[1/3] libipa: camera_sensor_helper: Add OV9734

28768 diff mbox series

Add a camera sensor helper for the OmniVision OV9734, used as the front
camera of the Microsoft Surface Laptop 2.

The kernel ov9734 driver writes V4L2_CID_ANALOGUE_GAIN directly to the
0x350a gain register, with a range of 16 to 248. The register is a Q4
fixed point value, so 16 is unity gain, giving a linear model of
gain = code / 16.

The driver doesn't program the black level calibration target, so the
sensor default of 0x40 at 10 bits applies, which is 4096 at 16 bits.

Without a helper the IPU3 IPA can't be used with this sensor.

Signed-off-by: GuyliandeJong <guyliandejong@proton.me>
---
 src/ipa/libipa/camera_sensor_helper.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Hi

2026. 11. 03. 11:41 keltezéssel, GuyliandeJong írta:
> Add a camera sensor helper for the OmniVision OV9734, used as the front
> camera of the Microsoft Surface Laptop 2.
> 
> The kernel ov9734 driver writes V4L2_CID_ANALOGUE_GAIN directly to the
> 0x350a gain register, with a range of 16 to 248. The register is a Q4
> fixed point value, so 16 is unity gain, giving a linear model of
> gain = code / 16.

This is confirmed by the data sheet:

   1x gain and maximum (15.5x) gain of analog_gain[9:0] is 0x10 and 0xF8, respectively


> 
> The driver doesn't program the black level calibration target, so the

That's what I see as well.


> sensor default of 0x40 at 10 bits applies, which is 4096 at 16 bits.

But where do you see this default value of 0x40? I'm looking at
https://datasheet4u.com/pdf/1550047/OV9234.pdf and it says

0x4002	BLC CTRL 02	0x00	RW	Bit[7:2]: Debug mode
					Bit[1:0]: Blacklevel target[9:8]
0x4003	BLC CTRL 03	0x10	RW	Bit[7:0]: Blacklevel target[7:0]

which would correspond to a default value of 0x10 in my interpretation.


> 
> Without a helper the IPU3 IPA can't be used with this sensor.
> 
> Signed-off-by: GuyliandeJong <guyliandejong@proton.me>
> ---
>   src/ipa/libipa/camera_sensor_helper.cpp | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 359040ec..41266005 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -877,6 +877,19 @@ public:
>   };
>   REGISTER_CAMERA_SENSOR_HELPER("ov8865", CameraSensorHelperOv8865)
>   
> +class CameraSensorHelperOv9734 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperOv9734()
> +	{
> +		/* Sensor default black level target: 0x40 at 10 bits. */
> +		blackLevel_ = 4096;
> +		/* Analogue gain is a Q4 value, 16 is unity gain. */
> +		gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };
> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("ov9734", CameraSensorHelperOv9734)
> +
>   class CameraSensorHelperOv13858 : public CameraSensorHelper
>   {
>   public:

Hi Barnabás,

Thanks for the review.

On 03/11/2026 13:17, Barnabás Pőcze wrote:
>> sensor default of 0x40 at 10 bits applies, which is 4096 at 16 bits.
>
> But where do you see this default value of 0x40? I'm looking at
> https://datasheet4u.com/pdf/1550047/OV9234.pdf and it says
>
> 0x4002	BLC CTRL 02	0x00	RW	Bit[7:2]: Debug mode
> 					Bit[1:0]: Blacklevel target[9:8]
> 0x4003	BLC CTRL 03	0x10	RW	Bit[7:0]: Blacklevel target[7:0]
>
> which would correspond to a default value of 0x10 in my interpretation.

You're right, I don't have an OV9734 datasheet and the 0x40 was an
assumption I should not have stated as fact.

I measured it instead: with the lens covered, minimum exposure (4 lines)
and unity analogue and digital gain, raw frames from the CIO2 average
16.7 to 17.0 in all four Bayer channels (median 17). That matches the
0x10 default you found. At maximum analogue gain the mean rises to
19-24, which I take to be residual light and dark current being
amplified.

I'll change the helper to 1024 (0x10 at 10 bits) in v2.