Re: [RFC PATCH 6/8] media: i2c: ov2312: add Omnivison OV2312 driver

[Jai Luthra]

Hi Mirela,

Quoting Mirela Rabulea (2026-10-02 21:46:11)
> Hi Rishikesh, Jay, Laurent, Hans, Sakari,
> 
> On 9/25/26 16:29, Rishikesh Donadkar wrote:
> > From: Jai Luthra <j-luthra@xxxxxx>
> >
> > Omnivision OV2312 is an RGB-IR sensor, i.e. it uses a 4x4 R,G,B,Ir bayer
> > pattern to capture both visible and near-infrared light. Every alternate
> > frame, the sensor changes the exposure and IR flash strobe registers to
> > stream an -
> > A. IR-dominant frame on CSI-2 virtual channel 0
> > B. RGB-dominant frame on CSI-2 virtual channel 1
> >
> > These A/B frames are routed as separate v4l2 streams, which may be
> > mapped to two separate /dev/videoX nodes by the CSI-RX DMA driver.
> >
> > Both of these streams are captured at a resolution of 1600x1301, 30 fps
> > each (60fps total). The extra row (1301 vs 1300) is an embedded line
> > prepended to each frame by the sensor, containing the following register
> > values:
> >    0x4813 - VC (Virtual Channel)
> >    0x321A - Group ID
> >    0x3920 - Strobe
> >    0x3501 - Exposure HI
> >    0x3502 - Exposure LO
> >    0x3508 - Gain HI
> >    0x3509 - Gain LO
> >    0x350e - Current Exposure HI
> >    0x350f - Current Exposure LO
> >
> > This driver also supports a few v4l2 controls like horizontal/vertical
> > flip, multi exposure and multi gain controls.
> >
> > Signed-off-by: Jai Luthra <j-luthra@xxxxxx>
> > Signed-off-by: Rishikesh Donadkar <r-donadkar@xxxxxx>
[...]
> > +static int ov2312_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +       struct ov2312 *ov2312 = container_of(ctrl->handler,
> > +                                            struct ov2312, ctrls);
> > +       int ret;
> > +
> > +       /*
> > +        * If the device is not powered up by the host driver do
> > +        * not apply any controls to H/W at this time. Instead
> > +        * the controls will be restored right after power-up.
> > +        */
> > +       if (pm_runtime_suspended(ov2312->dev))
> > +               return 0;
> > +
> > +       switch (ctrl->id) {
> > +       case V4L2_CID_EXPOSURE_MULTI:
> > +       case V4L2_CID_AGAIN_MULTI:
> > +       case V4L2_CID_DGAIN_MULTI:
> > +               dev_dbg(ov2312->dev, "debug: %s: %s = [%u, %u]\n", __func__,
> > +                               ctrl->name, ctrl->p_new.p_u32[0], ctrl->p_new.p_u32[1]);
> > +
> > +               ret = ov2312_set_AB_mode(ov2312);
> 
> So, the group hold for A/B context is set right away, when the control 
> arrives.
> 
> While working with the Omnivision OX05B1S, which is also an RGB-IR 
> sensor, we run into this problem:
> 
> The normal expected sequence is that the sensor will output alternating 
> frames VC0, VC1, VC0, VC1,...
> 
> But  when user space tries to do automatic exposure and gain control via 
> v4l2 muti controls, if the driver applies the values immediately, if the 
> virtual channels are not switched within the proper timeframe, it is 
> possible to run into frame duplication (no more nice alternating frames 
> VC0, VC1, VC0, VC1,...but duplicate VC0,VC0 or VC1,VC1).

Yes I remember seeing a similar thing on OV2312 as well when the AE/AGC
algorithm is turned on, and thus exposure and gain controls are updated
frequently.

In our case the streams get mixed, so in the captured frames for VC0
(IR-dominant) we would occassionally see frames with the exposure/gain
values that we expect in VC1 (RGB-dominant) and vice-versa.

I had assumed this mixing was due to asynchronous nature of the control
updates coming in, where the exposure+gain latch on with a delay of 2
frames, but the rest of the registers in the group (like VC ID, and IR
strobe) have a variable delay depending upon when the group registers are
programmed.

Without the AE/AGC the frames are stable (no mixing), so we must be talking
about the same or very similar issues.

> The information we received from the sensor vendor is that group0 update 
> needs to be between 2 group0 launchpoints (similar for group 1). We can 
> use the status register to query the currently active context, and in 
> order to avoid frame duplication we can update each group only when its 
> context is active.
> 

Ah, thanks for the pointer, a status register for group launch points
sounds quite useful.

> This is problematic in the v4l2-api context, it implies that even while 
> streaming, a v4l2 control cannot be committed to sensor registers right 
> away.
> 
> Even with workarounds in the sensor driver, to defer for later the 
> updates for the inactive context, it is still problematic: defer for how 
> long, and problems with overloaded systems, a stress test can bring us 
> in a broken VC sequence, as there is no atomic way to determine the 
> current active context + update the right group.  A broken VC sequence 
> shows up for example in libcamera as lost frames.
> 

Indeed, I think we need some sort of a mechanism to enforce timings of
control updates in V4L2 framework. And I don't yet know if system stress
would make this impossible to ensure without enabling RT?

But knowing the sensor hardware provides status registers is a good sign
that it should be possible to fix.

> Rishikesh, Jai,
> 
>   did you notice this problem on OV2312? A way to reproduce this is to 
> stress the driver with frequent repeated set controls (for the 
> multi-controls), and observe broken VC sequence (I observed it with 
> libcamera and on the CSI analyzer).
> 

Yes frequent control updates reliably reproduce the stream mixing on OV2312
as well.

Would it be possible for you to share the downstream driver or the status
register you have used? We could experiment if the same solution helps on
OV2312 too.

> 
> Laurent, Hans, Sakari,
> 
> did you encounter similar situations? Any comments or proposals? The 
> concern here, to summarize, is: v4l2 control cannot be committed to 
> sensor registers right away (even when streaming) and we are also unsure 
> when the right moment to perform the register access may come.
> 

I think this is a good candidate for discussion in the upcoming LPC BoF
session [1]. Are you planning to travel to EOSS/LPC in Prague next week?

I won't be travelling unfortunately, but hopefully Rishikesh, Laurent and
others can discuss this.

[1]: https://lore.kernel.org/all/22a0424b-c67d-4dcc-a31c-26acd764c653@xxxxxx/
> 
> Regards,
> 
> Mirela
> 

Thanks,
Jai

> > +               break;
> > +
> > +       case V4L2_CID_HFLIP:
> > +       case V4L2_CID_VFLIP:
> > +               ret = ov2312_set_orientation(ov2312);
> > +               break;
> > +
> > +       default:
> > +               ret = -EINVAL;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >