[v2,1/2] libcamera: libipa: camera_sensor: Add Sony IMX283 sensor properties
diff mbox series

Message ID 20240426132516.11085-2-umang.jain@ideasonboard.com
State Accepted
Commit 2ac544cfd653809d513c8100a28962852279eb01
Headers show
Series
  • libipa: Add IMX283 and IMx335 sensor helpers
Related show

Commit Message

Umang Jain April 26, 2024, 1:25 p.m. UTC 
From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Provide the IMX283 camera sensor properties and registration
with libipa for the gain code helpers.

The test patterns exposed by the IMX283 do not map well to the current
set of test pattern controls supplied by libcamera. These are left
inentionally unimplemented.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
 src/libcamera/sensor/camera_sensor_properties.cpp |  4 ++++
 2 files changed, 15 insertions(+)

Comments

Stefan Klug April 29, 2024, 9:23 a.m. UTC | #1 
Hi Umang,

thanks for reposting the patch.

Looks good to me.
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Cheers,
Stefan


On Fri, Apr 26, 2024 at 06:55:15PM +0530, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Provide the IMX283 camera sensor properties and registration
> with libipa for the gain code helpers.
> 
> The test patterns exposed by the IMX283 do not map well to the current
> set of test pattern controls supplied by libcamera. These are left
> inentionally unimplemented.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
>  src/libcamera/sensor/camera_sensor_properties.cpp |  4 ++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index ce29f423..f70d898f 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -417,6 +417,17 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
>  
> +class CameraSensorHelperImx283 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperImx283()
> +	{
> +		gainType_ = AnalogueGainLinear;
> +		gainConstants_.linear = { 0, 2048, -1, 2048 };
> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("imx283", CameraSensorHelperImx283)
> +
>  class CameraSensorHelperImx290 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 6e28b09e..4eabbbda 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -99,6 +99,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
>  		} },
> +		{ "imx283", {
> +			.unitCellSize = { 2400, 2400 },
> +			.testPatternModes = {},
> +		} },
>  		{ "imx290", {
>  			.unitCellSize = { 2900, 2900 },
>  			.testPatternModes = {},
> -- 
> 2.44.0
>
Jacopo Mondi May 2, 2024, 3:24 p.m. UTC | #2 
Hi Umang

On Fri, Apr 26, 2024 at 06:55:15PM +0530, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> Provide the IMX283 camera sensor properties and registration
> with libipa for the gain code helpers.
>
> The test patterns exposed by the IMX283 do not map well to the current
> set of test pattern controls supplied by libcamera. These are left
> inentionally unimplemented.

intentionally

>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
>  src/libcamera/sensor/camera_sensor_properties.cpp |  4 ++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index ce29f423..f70d898f 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -417,6 +417,17 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
>
> +class CameraSensorHelperImx283 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperImx283()
> +	{
> +		gainType_ = AnalogueGainLinear;
> +		gainConstants_.linear = { 0, 2048, -1, 2048 };

I see the gain-code to value in the datasheet expressed in dB
associated with a graph that suggest and exponential model.

Are we sure we should here use the linear one ?

> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("imx283", CameraSensorHelperImx283)
> +
>  class CameraSensorHelperImx290 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 6e28b09e..4eabbbda 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -99,6 +99,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
>  		} },
> +		{ "imx283", {
> +			.unitCellSize = { 2400, 2400 },
> +			.testPatternModes = {},
> +		} },
>  		{ "imx290", {
>  			.unitCellSize = { 2900, 2900 },
>  			.testPatternModes = {},
> --
> 2.44.0
>
Laurent Pinchart May 2, 2024, 3:41 p.m. UTC | #3 
On Thu, May 02, 2024 at 05:24:06PM +0200, Jacopo Mondi wrote:
> On Fri, Apr 26, 2024 at 06:55:15PM +0530, Umang Jain wrote:
> > From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >
> > Provide the IMX283 camera sensor properties and registration
> > with libipa for the gain code helpers.
> >
> > The test patterns exposed by the IMX283 do not map well to the current
> > set of test pattern controls supplied by libcamera. These are left
> > inentionally unimplemented.
> 
> intentionally
> 
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
> >  src/libcamera/sensor/camera_sensor_properties.cpp |  4 ++++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index ce29f423..f70d898f 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -417,6 +417,17 @@ public:
> >  };
> >  REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
> >
> > +class CameraSensorHelperImx283 : public CameraSensorHelper
> > +{
> > +public:
> > +	CameraSensorHelperImx283()
> > +	{
> > +		gainType_ = AnalogueGainLinear;
> > +		gainConstants_.linear = { 0, 2048, -1, 2048 };
> 
> I see the gain-code to value in the datasheet expressed in dB
> associated with a graph that suggest and exponential model.
> 
> Are we sure we should here use the linear one ?

The formula in the datasheet indicates that the gain in dB is equal to

-20 * log((2048 - gain) / 2048)

so the above is correct.

> > +	}
> > +};
> > +REGISTER_CAMERA_SENSOR_HELPER("imx283", CameraSensorHelperImx283)
> > +
> >  class CameraSensorHelperImx290 : public CameraSensorHelper
> >  {
> >  public:
> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > index 6e28b09e..4eabbbda 100644
> > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > @@ -99,6 +99,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >  				{ controls::draft::TestPatternModePn9, 4 },
> >  			},
> >  		} },
> > +		{ "imx283", {
> > +			.unitCellSize = { 2400, 2400 },
> > +			.testPatternModes = {},
> > +		} },
> >  		{ "imx290", {
> >  			.unitCellSize = { 2900, 2900 },
> >  			.testPatternModes = {},
Jacopo Mondi May 2, 2024, 4:25 p.m. UTC | #4 
Hi Laurent

On Thu, May 02, 2024 at 06:41:39PM GMT, Laurent Pinchart wrote:
> On Thu, May 02, 2024 at 05:24:06PM +0200, Jacopo Mondi wrote:
> > On Fri, Apr 26, 2024 at 06:55:15PM +0530, Umang Jain wrote:
> > > From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > >
> > > Provide the IMX283 camera sensor properties and registration
> > > with libipa for the gain code helpers.
> > >
> > > The test patterns exposed by the IMX283 do not map well to the current
> > > set of test pattern controls supplied by libcamera. These are left
> > > inentionally unimplemented.
> >
> > intentionally
> >
> > >
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > > ---
> > >  src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
> > >  src/libcamera/sensor/camera_sensor_properties.cpp |  4 ++++
> > >  2 files changed, 15 insertions(+)
> > >
> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > index ce29f423..f70d898f 100644
> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > @@ -417,6 +417,17 @@ public:
> > >  };
> > >  REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
> > >
> > > +class CameraSensorHelperImx283 : public CameraSensorHelper
> > > +{
> > > +public:
> > > +	CameraSensorHelperImx283()
> > > +	{
> > > +		gainType_ = AnalogueGainLinear;
> > > +		gainConstants_.linear = { 0, 2048, -1, 2048 };
> >
> > I see the gain-code to value in the datasheet expressed in dB
> > associated with a graph that suggest and exponential model.
> >
> > Are we sure we should here use the linear one ?
>
> The formula in the datasheet indicates that the gain in dB is equal to
>
> -20 * log((2048 - gain) / 2048)
>
> so the above is correct.
>

I see, so the "-20 * log(gain)" is just to convert to gain value to
db, while the actual code-to-gain formula is

        gain(db) = 20 * log (gain ^ -1)
        gain = 2048 / ( 2048 - gain)

(Thanks for the hint, I missed the - in front of -20)

which corresponds to m0 = 0, c0 = 2048, m1 = -1, c1 = 2048

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

Thanks
  j

> > > +	}
> > > +};
> > > +REGISTER_CAMERA_SENSOR_HELPER("imx283", CameraSensorHelperImx283)
> > > +
> > >  class CameraSensorHelperImx290 : public CameraSensorHelper
> > >  {
> > >  public:
> > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > index 6e28b09e..4eabbbda 100644
> > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > @@ -99,6 +99,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > >  				{ controls::draft::TestPatternModePn9, 4 },
> > >  			},
> > >  		} },
> > > +		{ "imx283", {
> > > +			.unitCellSize = { 2400, 2400 },
> > > +			.testPatternModes = {},
> > > +		} },
> > >  		{ "imx290", {
> > >  			.unitCellSize = { 2900, 2900 },
> > >  			.testPatternModes = {},
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index ce29f423..f70d898f 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -417,6 +417,17 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
 
+class CameraSensorHelperImx283 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperImx283()
+	{
+		gainType_ = AnalogueGainLinear;
+		gainConstants_.linear = { 0, 2048, -1, 2048 };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("imx283", CameraSensorHelperImx283)
+
 class CameraSensorHelperImx290 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index 6e28b09e..4eabbbda 100644
--- a/src/libcamera/sensor/camera_sensor_properties.cpp
+++ b/src/libcamera/sensor/camera_sensor_properties.cpp
@@ -99,6 +99,10 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				{ controls::draft::TestPatternModePn9, 4 },
 			},
 		} },
+		{ "imx283", {
+			.unitCellSize = { 2400, 2400 },
+			.testPatternModes = {},
+		} },
 		{ "imx290", {
 			.unitCellSize = { 2900, 2900 },
 			.testPatternModes = {},