<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 23 Jan 2023 at 09:55, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Mon, Jan 23, 2023 at 09:11:31AM +0000, Naushir Patuck wrote:<br>
> On Sun, 22 Jan 2023 at 19:12, Laurent Pinchart wrote:<br>
> > On Thu, Jan 19, 2023 at 10:45:30AM +0000, Naushir Patuck via libcamera-devel wrote:<br>
> > > Hi,<br>
> > ><br>
> > > This series adds support for the Raspberry Pi Camera Module 3 (IMX708).<br>
> > > A new hybrid PDAF/CDAF autofocus algorithm has been implemented to drive the<br>
> > > autofocus mechanism available on this module. All libcamera focus related<br>
> > > controls are now hanled by the IPA.<br>
> ><br>
> > It's nice to see support for this camera module, as well as PDAF :-) I<br>
> > haven't tested the IMX708 personally, but Kieran told me how good he<br>
> > thinks it is.<br>
> ><br>
> > We have a rule in libcamera that support for new hardware can be merged<br>
> > only if the corresponding kernel drivers are on their way to upstream. I<br>
> > haven't seen an imx708 driver posted to the linux-media mailing list.<br>
> > Did I miss something ?<br>
> ><br>
> <br>
> We are in the process of getting the kernel driver ready to be posted on the<br>
> linux-media list. Hope to get it sent this week!<br>
> <br>
> We need to de-feature some bits of the driver - specifically variant detection<br>
> and embedded/pdaf/HDR stats metadata streams. We are not entirely sure if HDR<br>
> mode support is also something that may/not be accepted as-is.<br>
<br>
Embedded data and PDAF depend on the V4L2 streams API. The good news is<br>
that the API just got merged in the linux-media tree :-) The bad news is<br>
that a piece is still missing in order to properly support camera<br>
sensors. Tomi, Sakari and I are working on it, so dropping embedded data<br>
and PDAF support for now is fine.<br>
<br>
Regarding HDR, I suppose you need to drop sensor-side histogram support<br>
for the same reason, but I'm not sure about the rest of the HDR-related<br>
features. In particular, if there are any HDR controls, they should be<br>
included in the driver already.<br></blockquote><div><br></div><div>Agreed, we will leave the HDR control in-place as is. This does eventually<br>want to be replaced by a proper V4L2 format type, we will propose something<br>for that.<br><br>Of course doing any sort of metering without the HDR metadata on the sensor is<br>going to be very difficult!<br></div><div><br></div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
As for variant detection, someone has to make a proposal, so I think<br>
this is a great time to bite the bullet :-) I'll do my best to help, but<br>
I can't drive this development given that I'm already busy with the<br>
streams API as mentioned above.<br>
<br>
> > > Camera Module 3 is available either with an IR or without an IR (NoIR) filter,<br>
> > > and a normal or wide angle lens. This makes a total of 4 possible variants.<br>
> > > Detection of the variant is done through an OTP on the module, and signalled<br>
> > > back to userland through changing the suffix on the device entity name. As such,<br>
> > > we have 4 different tuning files and 4 duplicate instances of camera helpers and<br>
> > > sensor properties entires.<br>
> > ><br>
> > > There is a single HDR sensor mode available in the sensor. Because of current<br>
> > > limitations, this needs to be switched on/off externally before running<br>
> > > libcamera:<br>
> > ><br>
> > > v4l2-ctl --set-ctrl wide_dynamic_range=[0|1] -d /dev/v4l-subdev0<br>
> > ><br>
> > > in order for the CameraSensor class to cache the correct sensor mode. In our<br>
> > > libcamera-apps, you can simply use the --hdr argument to switch this on. This<br>
> > > is only a temporary measure, and we need to work on a more permanent solution by<br>
> > > adding HDR pixel formats into V4L2.<br>
> > ><br>
> > > Naushir Patuck (7):<br>
> > > pipeline: ipa: raspberrypi: Check if lens actuator is available<br>
> > > pipeline: ipa: raspberrypi: Remove unused streamConfig<br>
> > > pipeline: ipa: raspberrypi: Replace entityControls<br>
> > > pipeline: ipa: raspberrypi: Validate lens controls<br>
> > > ipa: raspberrypi: Reorder header file inclusion<br>
> > > ipa: raspberrypi: Include autofocus controls in the IPA ControlInfoMap<br>
> > > ipa: raspberrypi: Add lens position to DeviceStatus<br>
> > ><br>
> > > Nick Hollinghurst (7):<br>
> > > ipa: mojom: raspberrypi: Add setLensControls() function<br>
> > > ipa: raspberrypi: Add autofocus algorithm interface headers<br>
> > > ipa: raspberrypi: Handle autofocus controls<br>
> > > ipa: raspberrypi: Handle autofocus algorithm results<br>
> > > ipa: raspberrypi: First version of autofocus algorithm using PDAF<br>
> > > libcamera: camera_sensor: Add Sony IMX708 sensor properties<br>
> > > ipa: raspberrypi: Add support for the Sony IMX708 sensor<br>
> > ><br>
> > > include/libcamera/ipa/raspberrypi.mojom | 10 +-<br>
> > > src/ipa/raspberrypi/cam_helper_imx708.cpp | 350 ++++++++<br>
> > > src/ipa/raspberrypi/controller/af_algorithm.h | 76 ++<br>
> > > src/ipa/raspberrypi/controller/af_status.h | 35 +<br>
> > > .../raspberrypi/controller/device_status.h | 4 +-<br>
> > > src/ipa/raspberrypi/controller/pdaf_data.h | 21 +<br>
> > > src/ipa/raspberrypi/controller/rpi/af.cpp | 755 ++++++++++++++++++<br>
> > > src/ipa/raspberrypi/controller/rpi/af.h | 153 ++++<br>
> > > src/ipa/raspberrypi/data/imx708.json | 559 +++++++++++++<br>
> > > src/ipa/raspberrypi/data/imx708_noir.json | 559 +++++++++++++<br>
> > > src/ipa/raspberrypi/data/imx708_wide.json | 462 +++++++++++<br>
> > > .../raspberrypi/data/imx708_wide_noir.json | 462 +++++++++++<br>
> > > src/ipa/raspberrypi/data/meson.build | 4 +<br>
> > > src/ipa/raspberrypi/meson.build | 2 +<br>
> > > src/ipa/raspberrypi/raspberrypi.cpp | 267 ++++++-<br>
> > > src/libcamera/camera_sensor_properties.cpp | 16 +<br>
> > > .../pipeline/raspberrypi/raspberrypi.cpp | 35 +-<br>
> > > 17 files changed, 3729 insertions(+), 41 deletions(-)<br>
> > > create mode 100644 src/ipa/raspberrypi/cam_helper_imx708.cpp<br>
> > > create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.h<br>
> > > create mode 100644 src/ipa/raspberrypi/controller/af_status.h<br>
> > > create mode 100644 src/ipa/raspberrypi/controller/pdaf_data.h<br>
> > > create mode 100644 src/ipa/raspberrypi/controller/rpi/af.cpp<br>
> > > create mode 100644 src/ipa/raspberrypi/controller/rpi/af.h<br>
> > > create mode 100644 src/ipa/raspberrypi/data/imx708.json<br>
> > > create mode 100644 src/ipa/raspberrypi/data/imx708_noir.json<br>
> > > create mode 100644 src/ipa/raspberrypi/data/imx708_wide.json<br>
> > > create mode 100644 src/ipa/raspberrypi/data/imx708_wide_noir.json<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>