ipa: libipa: camera_sensor_helper: add ov02c10

28362 diff mbox series

Without an entry for ov02c10, the software ISP used by the simple
pipeline handler can't run AGC for this sensor and logs:

  IPASoft: Failed to create camera sensor helper for ov02c10

The kernel driver (drivers/media/i2c/ov02c10.c) advertises analogue-gain
register values in the range 0x10..0xf8 with a default of 0x10, and
writes them to the 16-bit register at 0x3508 as (val << 4).  That
register is graded in 1/256 steps, so both the control range and the
register encoding agree on 1/16 steps: the default of 0x10 is unity gain
and the maximum of 0xf8 is 15.5x.

The sensor outputs 10-bit Bayer data (MEDIA_BUS_FMT_SGRBG10_1X10).  A
dark-frame measurement gives a mean pedestal of 64.2 LSB with a standard
deviation below 1.1 LSB, unchanged when the integration time is varied
by a factor of 356 (4 to 1425 lines), which confirms a fixed 0x40
pedestal rather than dark current.  That scales to 4096 at the 16-bit
width expected by blackLevel(), as for the other OmniVision sensors in
this file.

Add a helper that follows the same pattern.  Tested on the front camera
of a Microsoft Surface Pro 12 (Qualcomm X1P42100) driven by the qcom
CAMSS pipeline.  Before the change the IPA reported "Exposure 4-2320,
gain 16-248 (1)"; after it reports "Exposure 4-2320, gain 1-15.5
(0.145)".

Signed-off-by: François Roux <info@humanlearning.ch>
---
 src/ipa/libipa/camera_sensor_helper.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

2026. 09. 21. 16:56 keltezéssel, François Roux írta:
> Without an entry for ov02c10, the software ISP used by the simple
> pipeline handler can't run AGC for this sensor and logs:

That's not entirely true, it will still run some kind of AGC, although it
won't be particularly good.


> 
>    IPASoft: Failed to create camera sensor helper for ov02c10
> 
> The kernel driver (drivers/media/i2c/ov02c10.c) advertises analogue-gain
> register values in the range 0x10..0xf8 with a default of 0x10, and
> writes them to the 16-bit register at 0x3508 as (val << 4).  That
> register is graded in 1/256 steps, so both the control range and the
> register encoding agree on 1/16 steps: the default of 0x10 is unity gain
> and the maximum of 0xf8 is 15.5x.
> 
> The sensor outputs 10-bit Bayer data (MEDIA_BUS_FMT_SGRBG10_1X10).  A
> dark-frame measurement gives a mean pedestal of 64.2 LSB with a standard
> deviation below 1.1 LSB, unchanged when the integration time is varied
> by a factor of 356 (4 to 1425 lines), which confirms a fixed 0x40
> pedestal rather than dark current.  That scales to 4096 at the 16-bit
> width expected by blackLevel(), as for the other OmniVision sensors in
> this file.
> 
> Add a helper that follows the same pattern.  Tested on the front camera
> of a Microsoft Surface Pro 12 (Qualcomm X1P42100) driven by the qcom
> CAMSS pipeline.  Before the change the IPA reported "Exposure 4-2320,
> gain 16-248 (1)"; after it reports "Exposure 4-2320, gain 1-15.5
> (0.145)".
> 
> Signed-off-by: François Roux <info@humanlearning.ch>
> ---
>   src/ipa/libipa/camera_sensor_helper.cpp | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 9457d62b3..4b5c70f52 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -677,6 +677,18 @@ public:
>   };
>   REGISTER_CAMERA_SENSOR_HELPER("ov01a10", CameraSensorHelperOv01a10)
>   
> +class CameraSensorHelperOv02c10 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperOv02c10()
> +	{
> +		/* From dark frame measurement: 0x40 at 10bits. */
> +		blackLevel_ = 4096;
> +		gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };

The parameters here match what's in https://patchwork.libcamera.org/patch/28174/
so I tend to think these are indeed close to reality. Although that change refers
to the programmed black level in the kernel driver instead of measurements. And I
can indeed see

	{0x4002, 0x00},
	{0x4003, 0x40},

which are the BLC CTRL 02/03 registers on some other omnivison sensors. If you're feeling
adventurous, you could change 0x40 -> 0x80/etc. in the driver, and see if the measurements change.

In any case, I think the change itself is ok, not sure about which explanation
or source should used, though. Maybe we should just say that this is what the
kernel driver programs, instead of referencing measurements.


> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("ov02c10", CameraSensorHelperOv02c10)
> +
>   class CameraSensorHelperOv08x40 : public CameraSensorHelper
>   {
>   public:

Hi Barnabás,

Thanks for the review.

On "can't run AGC": you are right, the simple IPA still runs its fallback
AGC. I'll reword that.

I ran the experiment you suggested, without rebuilding the driver: while
streaming raw frames with the lens covered, I rewrote 0x4003 from 0x40 to
0x80 over I2C on the CCI bus. The black level followed on the very next
frame:

    frames 0-40   (0x4003 = 0x40):  64.19 LSB (10-bit)
    frames 41-149 (0x4003 = 0x80): 128.19 LSB

All four Bayer channels moved by +64.0 (Gr 64.20 -> 128.18, R 64.77 ->
128.74, B 64.13 -> 128.12, Gb 64.19 -> 128.18), with unchanged noise. So
the black level is exactly the BLC target the driver programs in
0x4002/0x4003. The measurement and the driver agree.

Since Martin's patch 28174 adds the same helper with the same values, I'm
happy for that one to be merged instead, whichever you prefer. In that
case, please feel free to add:

    Tested-by: François Roux <info@humanlearning.ch>

(front camera of a Surface Pro 12, X1P42100, CAMSS + simple pipeline).
Otherwise I'll send a v2 that cites the driver's BLC target as the source,
with the measurement as confirmation, and drops the AGC claim.

Best regards,
François

2026. 10. 03. 17:02 keltezéssel, François Roux írta:
> Hi Barnabás,
> 
> Thanks for the review.
> 
> On "can't run AGC": you are right, the simple IPA still runs its fallback
> AGC. I'll reword that.
> 
> I ran the experiment you suggested, without rebuilding the driver: while
> streaming raw frames with the lens covered, I rewrote 0x4003 from 0x40 to
> 0x80 over I2C on the CCI bus. The black level followed on the very next
> frame:
> 
>      frames 0-40   (0x4003 = 0x40):  64.19 LSB (10-bit)
>      frames 41-149 (0x4003 = 0x80): 128.19 LSB
> 
> All four Bayer channels moved by +64.0 (Gr 64.20 -> 128.18, R 64.77 ->
> 128.74, B 64.13 -> 128.12, Gb 64.19 -> 128.18), with unchanged noise. So
> the black level is exactly the BLC target the driver programs in
> 0x4002/0x4003. The measurement and the driver agree.
> 
> Since Martin's patch 28174 adds the same helper with the same values, I'm
> happy for that one to be merged instead, whichever you prefer. In that
> case, please feel free to add:
> 
>      Tested-by: François Roux <info@humanlearning.ch>
> 
> (front camera of a Surface Pro 12, X1P42100, CAMSS + simple pipeline).
> Otherwise I'll send a v2 that cites the driver's BLC target as the source,
> with the measurement as confirmation, and drops the AGC claim.

Given they have not replied to my earlier comments, nor sent a new version
for more than a month. I think it's fine if you send a new version and we
merge that.


> 
> Best regards,
> François