[1/2] libcamera: camera_sensor: Add Sony IMX676 sensor properties and helper
diff mbox series

Message ID 20261014-mainline-imx676-v1-1-f7550988d367@ideasonboard.com
State New
Headers show
Series
  • libcamera: sensor: Add support for Sony IMX676
Related show

Commit Message

Jai Luthra Oct. 14, 2026, 9:07 a.m. UTC
Add the camera sensor properties and camera helper entry for the Sony
IMX676 Starvis 2 sensor.

Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
---
 src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
 src/libcamera/sensor/camera_sensor_properties.cpp | 10 ++++++++++
 2 files changed, 22 insertions(+)

Comments

Kieran Bingham Oct. 14, 2026, 9:26 a.m. UTC | #1
Quoting Jai Luthra (2026-10-14 10:07:14)
> Add the camera sensor properties and camera helper entry for the Sony
> IMX676 Starvis 2 sensor.
> 
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
>  src/libcamera/sensor/camera_sensor_properties.cpp | 10 ++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 359040ec7418..46ea76a003d7 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -678,6 +678,18 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx662", CameraSensorHelperImx662)
>  
> +class CameraSensorHelperImx676 : public CameraSensorHelper
> +{
> +public:
> +       CameraSensorHelperImx676()
> +       {
> +               /* From datasheet: 0x32 at 10bits. */
> +               blackLevel_ = 3200;
> +               gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };
> +       }
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx676)

I wonder if this is also identical if we should just do:

+REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx678)

Or also similar with just a common CameraSensorHelperSonyStarvis

and maybe include the IMX415 (which is missing it's black level) and
IMX335, and IMX662.

Oh ... but now I see this in the file:

class CameraSensorHelperImx327 : public CameraSensorHelperImx290
{
};
REGISTER_CAMERA_SENSOR_HELPER("imx327", CameraSensorHelperImx327)

Which seems a bit ugly. Is there something preventing us registering
multiple names to a helper ?

Anyway, either way:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> +
>  class CameraSensorHelperImx678 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 20cfd0bc11a6..53bdf8b12f3e 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -350,6 +350,16 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 .hblankDelay = 2
>                         },
>                 } },
> +               { "imx676", {
> +                       .unitCellSize = { 2000, 2000 },
> +                       .testPatternModes = {},
> +                       .sensorDelays = {
> +                               .exposureDelay = 2,
> +                               .gainDelay = 2,
> +                               .vblankDelay = 2,
> +                               .hblankDelay = 2
> +                       },
> +               } },
>                 { "imx678", {
>                         .unitCellSize = { 2000, 2000 },
>                         .testPatternModes = {},
> 
> -- 
> 2.55.0
>
Barnabás Pőcze Oct. 14, 2026, 11:05 a.m. UTC | #2
2026. 10. 14. 11:26 keltezéssel, Kieran Bingham írta:
> Quoting Jai Luthra (2026-10-14 10:07:14)
>> Add the camera sensor properties and camera helper entry for the Sony
>> IMX676 Starvis 2 sensor.
>>
>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
>> ---
>>   src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
>>   src/libcamera/sensor/camera_sensor_properties.cpp | 10 ++++++++++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
>> index 359040ec7418..46ea76a003d7 100644
>> --- a/src/ipa/libipa/camera_sensor_helper.cpp
>> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
>> @@ -678,6 +678,18 @@ public:
>>   };
>>   REGISTER_CAMERA_SENSOR_HELPER("imx662", CameraSensorHelperImx662)
>>   
>> +class CameraSensorHelperImx676 : public CameraSensorHelper
>> +{
>> +public:
>> +       CameraSensorHelperImx676()
>> +       {
>> +               /* From datasheet: 0x32 at 10bits. */
>> +               blackLevel_ = 3200;
>> +               gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };
>> +       }
>> +};
>> +REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx676)
> 
> I wonder if this is also identical if we should just do:
> 
> +REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx678)

I think that makes sense.


> 
> Or also similar with just a common CameraSensorHelperSonyStarvis
> 
> and maybe include the IMX415 (which is missing it's black level) and
> IMX335, and IMX662.
> 
> Oh ... but now I see this in the file:
> 
> class CameraSensorHelperImx327 : public CameraSensorHelperImx290
> {
> };
> REGISTER_CAMERA_SENSOR_HELPER("imx327", CameraSensorHelperImx327)
> 
> Which seems a bit ugly. Is there something preventing us registering
> multiple names to a helper ?

Yes, the second argument is used to name a variable, so using the same
class will result in a redefinition error. So the inheritance approach
is the best that can be done at the moment, unless things are refactored
somehow.


> 
> Anyway, either way:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> 
>> +
>>   class CameraSensorHelperImx678 : public CameraSensorHelper
>>   {
>>   public:
>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
>> index 20cfd0bc11a6..53bdf8b12f3e 100644
>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
>> @@ -350,6 +350,16 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>>                                  .hblankDelay = 2
>>                          },
>>                  } },
>> +               { "imx676", {
>> +                       .unitCellSize = { 2000, 2000 },
>> +                       .testPatternModes = {},
>> +                       .sensorDelays = {
>> +                               .exposureDelay = 2,
>> +                               .gainDelay = 2,
>> +                               .vblankDelay = 2,
>> +                               .hblankDelay = 2
>> +                       },
>> +               } },
>>                  { "imx678", {
>>                          .unitCellSize = { 2000, 2000 },
>>                          .testPatternModes = {},
>>
>> -- 
>> 2.55.0
>>
Jai Luthra Oct. 14, 2026, 11:54 a.m. UTC | #3
Hi Barnabás, Kieran,

Quoting Barnabás Pőcze (2026-10-14 16:35:02)
> 2026. 10. 14. 11:26 keltezéssel, Kieran Bingham írta:
> > Quoting Jai Luthra (2026-10-14 10:07:14)
> >> Add the camera sensor properties and camera helper entry for the Sony
> >> IMX676 Starvis 2 sensor.
> >>
> >> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> >> ---
> >>   src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
> >>   src/libcamera/sensor/camera_sensor_properties.cpp | 10 ++++++++++
> >>   2 files changed, 22 insertions(+)
> >>
> >> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> >> index 359040ec7418..46ea76a003d7 100644
> >> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> >> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> >> @@ -678,6 +678,18 @@ public:
> >>   };
> >>   REGISTER_CAMERA_SENSOR_HELPER("imx662", CameraSensorHelperImx662)
> >>   
> >> +class CameraSensorHelperImx676 : public CameraSensorHelper
> >> +{
> >> +public:
> >> +       CameraSensorHelperImx676()
> >> +       {
> >> +               /* From datasheet: 0x32 at 10bits. */
> >> +               blackLevel_ = 3200;
> >> +               gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };
> >> +       }
> >> +};
> >> +REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx676)
> > 
> > I wonder if this is also identical if we should just do:
> > 
> > +REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx678)
> 
> I think that makes sense.
> 
> 
> > 
> > Or also similar with just a common CameraSensorHelperSonyStarvis
> > 
> > and maybe include the IMX415 (which is missing it's black level) and
> > IMX335, and IMX662.
> > 
> > Oh ... but now I see this in the file:
> > 
> > class CameraSensorHelperImx327 : public CameraSensorHelperImx290
> > {
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("imx327", CameraSensorHelperImx327)
> > 
> > Which seems a bit ugly. Is there something preventing us registering
> > multiple names to a helper ?
> 
> Yes, the second argument is used to name a variable, so using the same
> class will result in a redefinition error. So the inheritance approach
> is the best that can be done at the moment, unless things are refactored
> somehow.
> 

Indeed, I had tried using the same helper class for two sensors before and
it failed. I'll see if refactoring is straightforward before I post the v2.

Thanks,
  Jai

> 
> > 
> > Anyway, either way:
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > 
> >> +
> >>   class CameraSensorHelperImx678 : public CameraSensorHelper
> >>   {
> >>   public:
> >> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> >> index 20cfd0bc11a6..53bdf8b12f3e 100644
> >> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> >> @@ -350,6 +350,16 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >>                                  .hblankDelay = 2
> >>                          },
> >>                  } },
> >> +               { "imx676", {
> >> +                       .unitCellSize = { 2000, 2000 },
> >> +                       .testPatternModes = {},
> >> +                       .sensorDelays = {
> >> +                               .exposureDelay = 2,
> >> +                               .gainDelay = 2,
> >> +                               .vblankDelay = 2,
> >> +                               .hblankDelay = 2
> >> +                       },
> >> +               } },
> >>                  { "imx678", {
> >>                          .unitCellSize = { 2000, 2000 },
> >>                          .testPatternModes = {},
> >>
> >> -- 
> >> 2.55.0
> >>
>

Patch
diff mbox series

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 359040ec7418..46ea76a003d7 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -678,6 +678,18 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("imx662", CameraSensorHelperImx662)
 
+class CameraSensorHelperImx676 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperImx676()
+	{
+		/* From datasheet: 0x32 at 10bits. */
+		blackLevel_ = 3200;
+		gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("imx676", CameraSensorHelperImx676)
+
 class CameraSensorHelperImx678 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index 20cfd0bc11a6..53bdf8b12f3e 100644
--- a/src/libcamera/sensor/camera_sensor_properties.cpp
+++ b/src/libcamera/sensor/camera_sensor_properties.cpp
@@ -350,6 +350,16 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				.hblankDelay = 2
 			},
 		} },
+		{ "imx676", {
+			.unitCellSize = { 2000, 2000 },
+			.testPatternModes = {},
+			.sensorDelays = {
+				.exposureDelay = 2,
+				.gainDelay = 2,
+				.vblankDelay = 2,
+				.hblankDelay = 2
+			},
+		} },
 		{ "imx678", {
 			.unitCellSize = { 2000, 2000 },
 			.testPatternModes = {},