[v2] libcamera: libipa: camera_sensor: Add OmniVision OV08D10 sensor properties
diff mbox series

Message ID 20260618-ov08d10-v2-1-aa5a38e15bcc@emfend.at
State Accepted
Headers show
Series
  • [v2] libcamera: libipa: camera_sensor: Add OmniVision OV08D10 sensor properties
Related show

Commit Message

Matthias Fend June 18, 2026, 7:57 p.m. UTC 
Provide the OmniVision OV08D10 camera sensor properties and registration
with libipa for the gain code helpers.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
Changes in v2:
- Added black level
- Rebased
- Link to v1: https://lore.kernel.org/r/20260327-ov08d10-v1-1-b08eec3e1d21@emfend.at
---
 src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
 src/libcamera/sensor/camera_sensor_properties.cpp | 13 +++++++++++++
 2 files changed, 25 insertions(+)


---
base-commit: cde4eddf895cd09d05137bd99425d238cd8a6018
change-id: 20260327-ov08d10-078ce984b8a4

Best regards,

Comments

Kieran Bingham July 9, 2026, 1:09 p.m. UTC | #1 
Quoting Matthias Fend (2026-06-18 20:57:12)
> Provide the OmniVision OV08D10 camera sensor properties and registration
> with libipa for the gain code helpers.
> 
> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
> Changes in v2:
> - Added black level
> - Rebased
> - Link to v1: https://lore.kernel.org/r/20260327-ov08d10-v1-1-b08eec3e1d21@emfend.at
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
>  src/libcamera/sensor/camera_sensor_properties.cpp | 13 +++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 3028197e1cced6f96a7cb3ebfb14d976cac96a99..e89ea1808f5658b66c12a56af9d709fbe62fe69b 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -653,6 +653,18 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx708", CameraSensorHelperImx708)
>  
> +class CameraSensorHelperOv08d10 : public CameraSensorHelper
> +{
> +public:
> +       CameraSensorHelperOv08d10()
> +       {
> +               /* From Linux kernel driver: 0x40 at 10bits. */

Is this true? or copied?

I see two references to 0x40, at both register 0x04 and 0x05. But that's
hardly documentation.


I suspect it's reasonable and I have nothing to contradict so:

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


> +               blackLevel_ = 4096;
> +               gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };
> +       }
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("ov08d10", CameraSensorHelperOv08d10)
> +
>  class CameraSensorHelperOv2685 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index b217363d872a7cee81fbf50242e13cf0f6819da5..0e346edac66c7b85650b0e7c30496f1f10ab445e 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -325,6 +325,19 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 .hblankDelay = 3
>                         },
>                 } },
> +               { "ov08d10", {
> +                       .unitCellSize = { 1120, 1120 },
> +                       .testPatternModes =  {
> +                               { controls::draft::TestPatternModeOff, 0 },
> +                               { controls::draft::TestPatternModeCustom1, 1 },
> +                       },
> +                       .sensorDelays = {
> +                               .exposureDelay = 2,
> +                               .gainDelay = 2,
> +                               .vblankDelay = 2,
> +                               .hblankDelay = 2
> +                       },
> +               } },
>                 { "ov2685", {
>                         .unitCellSize = { 1750, 1750 },
>                         .testPatternModes = {
> 
> ---
> base-commit: cde4eddf895cd09d05137bd99425d238cd8a6018
> change-id: 20260327-ov08d10-078ce984b8a4
> 
> Best regards,
> -- 
> Matthias Fend <matthias.fend@emfend.at>
>
Matthias Fend July 11, 2026, 7:10 p.m. UTC | #2 
Hi Kieran,

thanks for your feedback!

Am 09.07.2026 um 15:09 schrieb Kieran Bingham:
> Quoting Matthias Fend (2026-06-18 20:57:12)
>> Provide the OmniVision OV08D10 camera sensor properties and registration
>> with libipa for the gain code helpers.
>>
>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
>> ---
>> Changes in v2:
>> - Added black level
>> - Rebased
>> - Link to v1: https://lore.kernel.org/r/20260327-ov08d10-v1-1-b08eec3e1d21@emfend.at
>> ---
>>   src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
>>   src/libcamera/sensor/camera_sensor_properties.cpp | 13 +++++++++++++
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
>> index 3028197e1cced6f96a7cb3ebfb14d976cac96a99..e89ea1808f5658b66c12a56af9d709fbe62fe69b 100644
>> --- a/src/ipa/libipa/camera_sensor_helper.cpp
>> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
>> @@ -653,6 +653,18 @@ public:
>>   };
>>   REGISTER_CAMERA_SENSOR_HELPER("imx708", CameraSensorHelperImx708)
>>   
>> +class CameraSensorHelperOv08d10 : public CameraSensorHelper
>> +{
>> +public:
>> +       CameraSensorHelperOv08d10()
>> +       {
>> +               /* From Linux kernel driver: 0x40 at 10bits. */
> 
> Is this true? or copied?
> 
> I see two references to 0x40, at both register 0x04 and 0x05. But that's
> hardly documentation.


It is this sequence in the Linux driver:
[..]
{0xfd, 0x07},
{0x00, 0xf8},
{0x01, 0x2b},
{0x05, 0x40},
[..]

{0xfd, 0x07} .. select page 7
{0x05, 0x40} .. set register P7:0x05 (blk_lvl_target[7:]) to 0x40

The access to register 0x04 that you mentioned is on page 5 and is 
unrelated.

And yes, the text is copied, of course ;)

Thanks
  ~Matthias

> 
> 
> I suspect it's reasonable and I have nothing to contradict so:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> 
>> +               blackLevel_ = 4096;
>> +               gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };
>> +       }
>> +};
>> +REGISTER_CAMERA_SENSOR_HELPER("ov08d10", CameraSensorHelperOv08d10)
>> +
>>   class CameraSensorHelperOv2685 : public CameraSensorHelper
>>   {
>>   public:
>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
>> index b217363d872a7cee81fbf50242e13cf0f6819da5..0e346edac66c7b85650b0e7c30496f1f10ab445e 100644
>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
>> @@ -325,6 +325,19 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>>                                  .hblankDelay = 3
>>                          },
>>                  } },
>> +               { "ov08d10", {
>> +                       .unitCellSize = { 1120, 1120 },
>> +                       .testPatternModes =  {
>> +                               { controls::draft::TestPatternModeOff, 0 },
>> +                               { controls::draft::TestPatternModeCustom1, 1 },
>> +                       },
>> +                       .sensorDelays = {
>> +                               .exposureDelay = 2,
>> +                               .gainDelay = 2,
>> +                               .vblankDelay = 2,
>> +                               .hblankDelay = 2
>> +                       },
>> +               } },
>>                  { "ov2685", {
>>                          .unitCellSize = { 1750, 1750 },
>>                          .testPatternModes = {
>>
>> ---
>> base-commit: cde4eddf895cd09d05137bd99425d238cd8a6018
>> change-id: 20260327-ov08d10-078ce984b8a4
>>
>> Best regards,
>> -- 
>> Matthias Fend <matthias.fend@emfend.at>
>>
Kieran Bingham July 12, 2026, 2:13 p.m. UTC | #3 
Hi Matthias,

Quoting Matthias Fend (2026-07-11 20:10:45)
> Hi Kieran,
> 
> thanks for your feedback!
> 
> Am 09.07.2026 um 15:09 schrieb Kieran Bingham:
> > Quoting Matthias Fend (2026-06-18 20:57:12)
> >> Provide the OmniVision OV08D10 camera sensor properties and registration
> >> with libipa for the gain code helpers.
> >>
> >> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> >> ---
> >> Changes in v2:
> >> - Added black level
> >> - Rebased
> >> - Link to v1: https://lore.kernel.org/r/20260327-ov08d10-v1-1-b08eec3e1d21@emfend.at
> >> ---
> >>   src/ipa/libipa/camera_sensor_helper.cpp           | 12 ++++++++++++
> >>   src/libcamera/sensor/camera_sensor_properties.cpp | 13 +++++++++++++
> >>   2 files changed, 25 insertions(+)
> >>
> >> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> >> index 3028197e1cced6f96a7cb3ebfb14d976cac96a99..e89ea1808f5658b66c12a56af9d709fbe62fe69b 100644
> >> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> >> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> >> @@ -653,6 +653,18 @@ public:
> >>   };
> >>   REGISTER_CAMERA_SENSOR_HELPER("imx708", CameraSensorHelperImx708)
> >>   
> >> +class CameraSensorHelperOv08d10 : public CameraSensorHelper
> >> +{
> >> +public:
> >> +       CameraSensorHelperOv08d10()
> >> +       {
> >> +               /* From Linux kernel driver: 0x40 at 10bits. */
> > 
> > Is this true? or copied?
> > 
> > I see two references to 0x40, at both register 0x04 and 0x05. But that's
> > hardly documentation.
> 
> 
> It is this sequence in the Linux driver:
> [..]
> {0xfd, 0x07},
> {0x00, 0xf8},
> {0x01, 0x2b},
> {0x05, 0x40},
> [..]
> 
> {0xfd, 0x07} .. select page 7
> {0x05, 0x40} .. set register P7:0x05 (blk_lvl_target[7:]) to 0x40
> 
> The access to register 0x04 that you mentioned is on page 5 and is 
> unrelated.
> 
> And yes, the text is copied, of course ;)

That's ok - as long as it wasn't a copy of the text without actually
verifying against the driver, which at the moment is quite opaque.

If you have the documentation for the driver please could you submit a
patch to document those registers? Perhaps changing the register
addresses for named defines at least?

Even updating just those registers as we go will help future users. Of
course improving the documentation for 'as many registers as possible'
is also welcome.

I've merged this patch regardless, but in the future I can envisage we
could introduce configurable pedestals.

--
Thanks

Kieran


> 
> Thanks
>   ~Matthias
> 
> > 
> > 
> > I suspect it's reasonable and I have nothing to contradict so:
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > 
> >> +               blackLevel_ = 4096;
> >> +               gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };
> >> +       }
> >> +};
> >> +REGISTER_CAMERA_SENSOR_HELPER("ov08d10", CameraSensorHelperOv08d10)
> >> +
> >>   class CameraSensorHelperOv2685 : public CameraSensorHelper
> >>   {
> >>   public:
> >> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> >> index b217363d872a7cee81fbf50242e13cf0f6819da5..0e346edac66c7b85650b0e7c30496f1f10ab445e 100644
> >> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> >> @@ -325,6 +325,19 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >>                                  .hblankDelay = 3
> >>                          },
> >>                  } },
> >> +               { "ov08d10", {
> >> +                       .unitCellSize = { 1120, 1120 },
> >> +                       .testPatternModes =  {
> >> +                               { controls::draft::TestPatternModeOff, 0 },
> >> +                               { controls::draft::TestPatternModeCustom1, 1 },
> >> +                       },
> >> +                       .sensorDelays = {
> >> +                               .exposureDelay = 2,
> >> +                               .gainDelay = 2,
> >> +                               .vblankDelay = 2,
> >> +                               .hblankDelay = 2
> >> +                       },
> >> +               } },
> >>                  { "ov2685", {
> >>                          .unitCellSize = { 1750, 1750 },
> >>                          .testPatternModes = {
> >>
> >> ---
> >> base-commit: cde4eddf895cd09d05137bd99425d238cd8a6018
> >> change-id: 20260327-ov08d10-078ce984b8a4
> >>
> >> Best regards,
> >> -- 
> >> Matthias Fend <matthias.fend@emfend.at>
> >>
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 3028197e1cced6f96a7cb3ebfb14d976cac96a99..e89ea1808f5658b66c12a56af9d709fbe62fe69b 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -653,6 +653,18 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("imx708", CameraSensorHelperImx708)
 
+class CameraSensorHelperOv08d10 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperOv08d10()
+	{
+		/* From Linux kernel driver: 0x40 at 10bits. */
+		blackLevel_ = 4096;
+		gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("ov08d10", CameraSensorHelperOv08d10)
+
 class CameraSensorHelperOv2685 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index b217363d872a7cee81fbf50242e13cf0f6819da5..0e346edac66c7b85650b0e7c30496f1f10ab445e 100644
--- a/src/libcamera/sensor/camera_sensor_properties.cpp
+++ b/src/libcamera/sensor/camera_sensor_properties.cpp
@@ -325,6 +325,19 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				.hblankDelay = 3
 			},
 		} },
+		{ "ov08d10", {
+			.unitCellSize = { 1120, 1120 },
+			.testPatternModes =  {
+				{ controls::draft::TestPatternModeOff, 0 },
+				{ controls::draft::TestPatternModeCustom1, 1 },
+			},
+			.sensorDelays = {
+				.exposureDelay = 2,
+				.gainDelay = 2,
+				.vblankDelay = 2,
+				.hblankDelay = 2
+			},
+		} },
 		{ "ov2685", {
 			.unitCellSize = { 1750, 1750 },
 			.testPatternModes = {