[PATCH 8/8] libcamera: Add new atomisp pipeline handler

[Hans de Goede]

Hi Laurent,

On 5-Nov-24 12:53 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> (CC'ing Sakari)
> 
> Thank you for the patch.
> 
> A few high-level questions first.
> 
> On Sun, Nov 03, 2024 at 04:22:05PM +0100, Hans de Goede wrote:
>> Add a basic atomisp pipeline handler which supports configuring
>> the pipeline, capturing frames and selecting front/back sensor.
>>
>> The atomisp ISP needs some extra lines/columns when debayering and also
>> has some max resolution limitations, this causes the available output
>> resolutions to differ from the sensor resolutions.
>>
>> The atomisp driver's Android heritage means that it mostly works as a non
>> media-controller centric v4l2 device, primarily controlled through its
>> /dev/video# node.
> 
> Could that be fixed on the kernel side (assuming someone would be able
> to do the work of course) ?

Yes, note that the current kernel driver already uses the media-controller
and has separate subdevs for the ISP, CSI receivers, sensors and VCM,
see e.g. the 2 attached pngs for 2 different setups (generated by dot).

And the atomisp pipeline handler e.g. already configures mc-links to
select which sensor to use.

So we are already part way there. The thing which currently is not
very mc-centric is that a single set_fmt call is made on /dev/video#
after setting the mc-links and then that configures the fmts
on all the subdevs taking the special resolution-padding requirements
of the ISP into account.

Currently atomisp kernel code already allocates and initializes
a bunch of ISP contexts at this set_fmt call time (rather then
at request-buffers time) and more importantly it selects which
pipeline program (since the ISP is not fixed function) to run on
the ISP at this time. Changing that is very much no trivial.

I guess we could keep allocating those at that time and have
a flag (ioctl / v4l2-ctrl?) to skip the propagating of the fmts
to the subdevs and instead having the pipeline handler set
the subdev fmts itself, but I do not see much added value in that
atm.

>> The driver takes care of setting up the pipeline itself
>> propagating try / set fmt calls down from its single /dev/video# node to
>> the selected sensor taking the necessary padding, etc. into account.
>>
>> Therefor things like getting the list of support formats / sizes and
>> setFmt() calls are all done on the /dev/video# node instead of on subdevs,
>> this avoids having to duplicate the padding, etc. logic in the pipeline
>> handler.
>>
>> Since the statistics buffers which we get from the ISP2 are not documented
> 
> Could the stats format be reverse-engineered ? Or alternatively, could
> Intel provide documentation (waving at Sakari) ?

I have asked Salari about this already, but with these kinda things
it is going to take a while to get an official yes / no answer.

>> this uses the swstats_cpu and simple-IPA from the swisp. At the moment only
>> aec/agc is supported.
>>
>> awb support will be added in a follow-up patch.

Regards,

Hans

-------------- next part --------------
A non-text attachment was scrubbed...
Name: atomisp-graph.png
Type: image/png
Size: 2155 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20241106/7f4abc50/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: atomisp-graph2.png
Type: image/png
Size: 34460 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20241106/7f4abc50/attachment-0001.png>