[v3,1/5] libcamera: libipa: camera_sensor: Provide helper and properties for Sony IMX462
diff mbox series

Message ID 20241124192950.542686-2-geoffrey.vl@gmail.com
State Accepted
Headers show
Series
  • Add support for Sony IMX327 and IMX462 sensors
Related show

Commit Message

Geoffrey Van Landeghem Nov. 24, 2024, 7:29 p.m. UTC 
The sensor is largely compatible with the already supported
Sony IMX290 so we can reuse the same helpers for the analogue
gain conversion functions.

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(+)

Comments

Jacopo Mondi Nov. 25, 2024, 9:52 a.m. UTC | #1 
Hi Geoffrey
  thanks for the patch

On Sun, Nov 24, 2024 at 08:29:46PM +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.
>
> Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>

When you receive a tag on patch series version vX you should carry it
forward in version v(X + 1)

Feels a bit clunky doing that by hand, I know

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  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
>
Geoffrey Van Landeghem Nov. 25, 2024, 9:57 a.m. UTC | #2 
Hi Jacopo,

ah OK, so not every new version of a patch has to be reviewed entirely
again then?
Thanks for the clarification, I'll take it along with any other remarks
that you still have.

Geoffrey


Op ma 25 nov 2024 om 10:52 schreef Jacopo Mondi <
jacopo.mondi@ideasonboard.com>:

> Hi Geoffrey
>   thanks for the patch
>
> On Sun, Nov 24, 2024 at 08:29:46PM +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.
> >
> > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
>
> When you receive a tag on patch series version vX you should carry it
> forward in version v(X + 1)
>
> Feels a bit clunky doing that by hand, I know
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
>
> > ---
> >  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
> >
>
Jacopo Mondi Nov. 25, 2024, 10:10 a.m. UTC | #3 
Hi Geoffrey

On Mon, Nov 25, 2024 at 10:57:15AM +0100, Geoffrey Van Landeghem wrote:
> Hi Jacopo,
>
> ah OK, so not every new version of a patch has to be reviewed entirely
> again then?

Well, it depends. If you make substantial changes to the new version
you can decide to drop a tag because you think the patch is worth a
new review (and maybe mention it in the cover letter).

For me, as a reviewer, if I've sent a tag and the tag is new patch
version and the patch hasn't changed, I'll just gloss over to make
sure but won't go in a detailed review.

> Thanks for the clarification, I'll take it along with any other remarks
> that you still have.
>

Thanks for sticking with us and getting through this sometimes
unfriendly process.

> Geoffrey
>
>
> Op ma 25 nov 2024 om 10:52 schreef Jacopo Mondi <
> jacopo.mondi@ideasonboard.com>:
>
> > Hi Geoffrey
> >   thanks for the patch
> >
> > On Sun, Nov 24, 2024 at 08:29:46PM +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.
> > >
> > > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
> >
> > When you receive a tag on patch series version vX you should carry it
> > forward in version v(X + 1)
> >
> > Feels a bit clunky doing that by hand, I know
> >
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > Thanks
> >   j
> >
> > > ---
> > >  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
> > >
> >

Patch
diff mbox series 

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 = {},