[PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide CameraSensorHelper and CameraSensorProperties for the Sony IMX462 image sensor.

[Geoffrey Van Landeghem]

Hi Jacopo,

sorry for screwing up the git commit message. I took over the
suggestion entirely from the V1 patch, which also got me a bit
confused. The entire git format-patch and git email is still a bit
awkward and needs some more getting used to it from my side. At work
we rely on other systems for centralizing source control, but I learn
something new and I also appreciate your feedback (and patience!).

Regarding the pixel size, it's indeed 2.9u pixel. I guess the most
trustworthy resource is this one from Sony:
https://www.sony-semicon.com/files/62/flyer_security/IMX462LQR_LQR1_Flyer.pdf
(look for "unit cell size")

And regarding the black level offset setting (register 300Ah). I
checked the IMX290 and IMX327 datasheets and they match in that they
both have F0h as default value.
For IMX462 I couldn't find any datasheet, but since I own the sensor I
queried it using i2c-tools, and it seems it's also defaulting to F0h:

$ i2ctransfer -f -y 10 w2 at 0x1a 0x30 0x0A r1
0xf0

Kinds regards,
Geoffrey

Op di 19 nov 2024 om 13:56 schreef Jacopo Mondi <jacopo.mondi at ideasonboard.com>:


>
> Hi Geoffrey
>
> On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:
> > The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.
>
> I presume my suggestion to read https://cbea.ms/git-commit/ first
> wasn't helpful.
>
> Subject line should be shortened (I would not force it to 50 cols
> as the website suggests, but strive for staying in at least 72), and
> no full stop at the end.
>
> Same thing for the commit message.
>
> We can adjust it when applying the patch if you don't have to resend
> for other reasons.
>
> >
> > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl at gmail.com>
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++
> >  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +
> >  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++
> >  3 files changed, 10 insertions(+)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index c6169bdc..f870dc28 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -622,6 +622,11 @@ public:
> >  };
> >  REGISTER_CAMERA_SENSOR_HELPER("imx415", CameraSensorHelperImx415)
> >
> > +class CameraSensorHelperImx462 : public CameraSensorHelperImx290
> > +{
> > +};
> > +REGISTER_CAMERA_SENSOR_HELPER("imx462", CameraSensorHelperImx462)
>
> One thing we're missing for imx290 is the black level pedestal. The
> imx290 datasheet reports a (programmable) black level of
> 10 bits: 0x3c
> 12 bits: 0xf0
>
> Could you confirm the imx462 reports the same default values ?
>
> > +
> >  class CameraSensorHelperImx477 : public CameraSensorHelper
> >  {
> >  public:
> > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> > index e57ab538..0cc24a6d 100644
> > --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> > @@ -73,3 +73,4 @@ static CamHelper *create()
> >  }
> >
> >  static RegisterCamHelper reg("imx290", &create);
> > +static RegisterCamHelper reg462("imx462", &create);
> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > index 6d4136d0..e2305166 100644
> > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >                       .unitCellSize = { 1450, 1450 },
> >                       .testPatternModes = {},
> >               } },
> > +             { "imx462", {
> > +                     .unitCellSize = { 2900, 2900 },
> > +                     .testPatternModes = {},
> > +             } },
>
> I don't have a datasheet but a few online sources confirm a 2.9u pixel
> size.
>
> The patch looks good
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Thanks
>   j
>
> >               { "imx477", {
> >                       .unitCellSize = { 1550, 1550 },
> >                       .testPatternModes = {},
> > --
> > 2.43.0
> >