[1/4] libcamera: libipa: camera_sensor: Add Sony IMX462 sensor properties

21892 diff mbox series

Hi,
This patch-set comes as part of an ongoing effort to support Sony IMX327 and IMX462 sensors better in the linux kernel.
For the kernel following changes are proposed:
https://github.com/raspberrypi/linux/pull/5859

After those changes have been applied, the 2 Starvis sensors will no longer be identied by libcamera as a IMX290 and therefore require this patch-set to work correctly. 

Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@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(+)

Hi Geoffrey
   thanks for the patch

On Wed, Nov 13, 2024 at 11:35:54PM +0100, Geoffrey Van Landeghem wrote:
> Hi,

Hi to you, but this is a commit message and the text in here will
remain in the git log history. It's nice to be greet when meeting, but
probably we don't want this in the commit history :)

> This patch-set comes as part of an ongoing effort to support Sony IMX327 and IMX462 sensors better in the linux kernel.
> For the kernel following changes are proposed:


I suggest to read https://cbea.ms/git-commit/. In example, I would
have:

Provide a CameraSensorHelper and CameraSensorProperties for
the Sony IMX462 image sensor.

The sensor is largely compatible with the already supported
Sony IMX290 so we can reuse the same helpers for the analogue
gain conversion functions.


> https://github.com/raspberrypi/linux/pull/5859

Do you plan to send the same patches to linux-media ? We're happy to
take in changes before they get merged in mainline linux, but they
should be at least posted to the mailing list to have them available
to everyone.

>
> After those changes have been applied, the 2 Starvis sensors will no longer be identied by libcamera as a IMX290 and therefore require this patch-set to work correctly.

If you want to provide context around a set of patches you can write a
cover letter. Generate it the --cover-letter option to git
format-patch and put there the context, while focusing on the single
changes in each patch's commit message.

Oh, and looking at the other patches, even if trivial, we always
require a commit message for each patch.

Thanks
  j

>
> Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@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)
> +
>  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 = {},
> +		} },
>  		{ "imx477", {
>  			.unitCellSize = { 1550, 1550 },
>  			.testPatternModes = {},
> --
> 2.43.0
>

Hi Jacopo

On Thu, 14 Nov 2024 at 08:59, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Geoffrey
>    thanks for the patch
>
> On Wed, Nov 13, 2024 at 11:35:54PM +0100, Geoffrey Van Landeghem wrote:
> > Hi,
>
> Hi to you, but this is a commit message and the text in here will
> remain in the git log history. It's nice to be greet when meeting, but
> probably we don't want this in the commit history :)
>
> > This patch-set comes as part of an ongoing effort to support Sony IMX327 and IMX462 sensors better in the linux kernel.
> > For the kernel following changes are proposed:
>
>
> I suggest to read https://cbea.ms/git-commit/. In example, I would
> have:
>
> Provide a CameraSensorHelper and CameraSensorProperties for
> the Sony IMX462 image sensor.
>
> The sensor is largely compatible with the already supported
> Sony IMX290 so we can reuse the same helpers for the analogue
> gain conversion functions.
>
>
> > https://github.com/raspberrypi/linux/pull/5859
>
> Do you plan to send the same patches to linux-media ? We're happy to
> take in changes before they get merged in mainline linux, but they
> should be at least posted to the mailing list to have them available
> to everyone.

imx327 will already be advertised by the kernel driver if that
compatible string is used.

imx462 I only looked at this week following on from checking out the
High Conversion Gain option.
I'll look to send the couple of patches
637c995d8763 media: i2c: imx290: Add configuration for IMX462
c21f2153831b media: dt-bindings: media: i2c: Add IMX462 to the bindings
22393df4e814 media: imx290: Limit analogue gain according to module
to linux-media next week.

  Dave

> >
> > After those changes have been applied, the 2 Starvis sensors will no longer be identied by libcamera as a IMX290 and therefore require this patch-set to work correctly.
>
> If you want to provide context around a set of patches you can write a
> cover letter. Generate it the --cover-letter option to git
> format-patch and put there the context, while focusing on the single
> changes in each patch's commit message.
>
> Oh, and looking at the other patches, even if trivial, we always
> require a commit message for each patch.
>
> Thanks
>   j
>
> >
> > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@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)
> > +
> >  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 = {},
> > +             } },
> >               { "imx477", {
> >                       .unitCellSize = { 1550, 1550 },
> >                       .testPatternModes = {},
> > --
> > 2.43.0
> >

On Thu, 14 Nov 2024 at 11:43, Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi Jacopo
>
> On Thu, 14 Nov 2024 at 08:59, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Geoffrey
> >    thanks for the patch
> >
> > On Wed, Nov 13, 2024 at 11:35:54PM +0100, Geoffrey Van Landeghem wrote:
> > > Hi,
> >
> > Hi to you, but this is a commit message and the text in here will
> > remain in the git log history. It's nice to be greet when meeting, but
> > probably we don't want this in the commit history :)
> >
> > > This patch-set comes as part of an ongoing effort to support Sony IMX327 and IMX462 sensors better in the linux kernel.
> > > For the kernel following changes are proposed:
> >
> >
> > I suggest to read https://cbea.ms/git-commit/. In example, I would
> > have:
> >
> > Provide a CameraSensorHelper and CameraSensorProperties for
> > the Sony IMX462 image sensor.
> >
> > The sensor is largely compatible with the already supported
> > Sony IMX290 so we can reuse the same helpers for the analogue
> > gain conversion functions.
> >
> >
> > > https://github.com/raspberrypi/linux/pull/5859
> >
> > Do you plan to send the same patches to linux-media ? We're happy to
> > take in changes before they get merged in mainline linux, but they
> > should be at least posted to the mailing list to have them available
> > to everyone.
>
> imx327 will already be advertised by the kernel driver if that
> compatible string is used.
>
> imx462 I only looked at this week following on from checking out the
> High Conversion Gain option.
> I'll look to send the couple of patches
> 637c995d8763 media: i2c: imx290: Add configuration for IMX462
> c21f2153831b media: dt-bindings: media: i2c: Add IMX462 to the bindings
> 22393df4e814 media: imx290: Limit analogue gain according to module
> to linux-media next week.

OK, they got done today instead -
https://lore.kernel.org/linux-media/20241114-media-imx290-imx462-v1-0-c538a2e24786@raspberrypi.com/

>   Dave
>
> > >
> > > After those changes have been applied, the 2 Starvis sensors will no longer be identied by libcamera as a IMX290 and therefore require this patch-set to work correctly.
> >
> > If you want to provide context around a set of patches you can write a
> > cover letter. Generate it the --cover-letter option to git
> > format-patch and put there the context, while focusing on the single
> > changes in each patch's commit message.
> >
> > Oh, and looking at the other patches, even if trivial, we always
> > require a commit message for each patch.
> >
> > Thanks
> >   j
> >
> > >
> > > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@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)
> > > +
> > >  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 = {},
> > > +             } },
> > >               { "imx477", {
> > >                       .unitCellSize = { 1550, 1550 },
> > >                       .testPatternModes = {},
> > > --
> > > 2.43.0
> > >