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

[Mirela Rabulea]

Hi Jay,

On 10/3/26 05:05, Jai Luthra wrote:
> 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.

Sure,

here is the latest driver in nxp tree, please be aware that it is ahead 
of the upstream version, with some experimental features:

https://github.com/nxp-imx/linux-imx/tree/lf-6.18.y/drivers/media/i2c/ox05b1s

This is the status register we query to determine the active context:

#define OX05B1S_REG_GH_SEL_REAL        CCI_REG8(0x322d)

Today I also sent the v5 of ox05b1s RGB-Ir driver upstream (less features):

https://lore.kernel.org/all/20261005175102.2358881-1-mirela.rabulea@xxxxxxx/

> > 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/

Thanks for letting me know. Unfortunately I will also not travel, but a 
few folks from NXP will be there (cc-ed).

Daniel Baluta will be at OSS+ELC, leaving on 6 october from Bucharest, I 
asked him to attend the RGB-Ir presentation at ELC
Tomas Babinec will be meeting IoB in Prague on Wednesday, but will not 
be at LPC/OSS+ELC
Frank Li, I asked him to attend RGB-Ir presentation at LPC

If it is possible for me to attend remotely these 2 RGB-Ir 
presentations, I will do so.

Best regards,

Mirela

> > Regards,
> >
> > Mirela
> Thanks,
> Jai
>
> > > +               break;
> > > +
> > > +       case V4L2_CID_HFLIP:
> > > +       case V4L2_CID_VFLIP:
> > > +               ret = ov2312_set_orientation(ov2312);
> > > +               break;
> > > +
> > > +       default:
> > > +               ret = -EINVAL;
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +