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

[Mirela Rabulea <mirela.rabulea@nxp.com>]

Hi Laurent,

On 10/6/26 18:37, Laurent Pinchart wrote:
> Hi Mirela,
>
> On Tue, Oct 06, 2026 at 04:55:25PM +0300, Mirela Rabulea wrote:
>> On 10/6/26 10:36, Sakari Ailus wrote:
>>> On Fri, Oct 02, 2026 at 07:16:11PM +0300, Mirela Rabulea wrote:
>>>> 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.
>>> In practice there's little the kernel overall can do about this: the timing
>>> of everything is handled by the userspace. Drivers that aren't directly in
>>> control of the data path don't even have frame timing information and even
>>> if we did pass that to drivers, I²C writes always have some uncertainty
>>> (system scheduling, I²C access failures etc.), so one needs to be prepared
>>> to failing to do the writes in time, which would further complicate the
>>> UAPI.
> I think an API to group multiple writes in a buffer and trigger the I2C
> operation would be useful. It could be software-based, but it can also
> be useful on platforms where the I2C controller supports hardware
> triggers. I have been told a while ago that some I2C controllers can do
> that (I think on Nvidia chips, but don't quote me on that).

I agree, I think s_ext_ctrls would be a good candidate to fit that 
purpose, the  problem is currently it is unavailable in the subdevice 
interface. Since most sensor drivers are implemented as v4l2 subdevices, 
at the present they get only the simple s_ctrl callbacks, so the sensor 
driver is unaware when a group of controls start or end. If we address 
this, we could also add a context-id or exposure-id in the group.

I have also experienced a bit with control clusters, it was for a 
different purpose, I was trying to keep single and multi control values 
in sync, but it might be another way to achieve a group. Benefit from 
the fact the v4l2 core will try/set together all the cluster (either all 
pass or all fail), and try_ctl/set_ctl is called only for the master 
control (first in the list).

This simplifies a lot the problem of the sensor driver managing group 
holds. It does not help though the issue of synchronization, the fact 
remains that the sensor driver is unaware of frame start event. I heard 
some ideas on that during yesterday LPC BoF session.

>> On the kerne side, what would help would be a way to make an atomic
>> operation out of an i2c read (the status register to determine the
>> active context) plus a group hold update (a few i2c register writes). I
>> don't know if that is possible.
> What do you mean by atomic operation here ? From an I2C point of view we
> can probably guarantee that nothing will perform I2C access on the same
> bus between the read and write, but only if we submit both operations
> together. Changing the values to be written based on the read value
> isn't possible.
>
> But I don't really see how that would help. The issue is about
> performing I2C accesses at the right time, not about something else
> preempting the I2C bus between the read and write, right ?
By atomic I mean that we need to be sure that between the time we query 
the current context (via an i2c register read) and the time we update 
the group hold (via a series of i2c register writes), the context does 
not change, otherwise we will update the wrong context (userspace will 
see the values applied on the wrong VC). This is a problem we observed 
under stress test conditions.
>
>> On the v4l2 API side, I believe the current expectation is that upon
>> s_ctrl, if the value was accepted by the kernel, it will return success,
>> but user-space should not assume frames will immediately reflect the new
>> values.
>>
>> Traditionally, with drivers I have seen so far, if s_ctrl is applied
>> while streaming, it is applied immediately (but fail in case of i2c
>> access failure). Upon success, captured frames will reflect the values
>> after N+1 or N+2.
> The point at which the control will take effect is device-dependent.
> Different sensors have different delays for exposure time and analog
> gain. Increasing the delay when interleaving two groups doesn't seem to
> be a fundamental problem. What is crucial, though, is for userspace to
> know when the controls have taken effect.

Is there a way for userspace to to know when the controls have taken 
effect without embedded data? From ISP statistics maybe...I'm not an 
expert on that :(

Thanks,

Mirela

>
>>> Do you have libcamera in userspace or something else?
>> Yes, we experienced with libcamera. I can also reproduce the unwanted
>> behavior with v4l2-ctl streaming + i2ctranfer script that stress the
>> group hold writes.
>>
>> If we were to place the responsibility on userspace/libcamera, than
>> libcamera should be able to handle this:
>>
>> - after a successful s_ctrl, captured frames will reflect the values
>> after not N+2 but  X+N+2, where X can be anything because we do not know
>> when we catch the right context to do the group hold access
>>
>> - do not expect s_ctrl to fail, unless the value was not accepted; i2c
>> access failures cannot be catched, because we cannot apply the control
>> value instantly
>>
>> - a control value may get accidentally applied to the wrong stream;
>> because VC takes effect at frame N+1 and exposure and gain settings take
>> effect at frame N+2, if the group write timing is improper, these may
>> get out of sync; so user-space may see frames optimized for RGB on the
>> stream that was supposed to be optimized for Ir or vice-versa, as
>> confirmed by RishiKesh and Jai on ov2312.
>>
>> - embedded data information may help identify what settings were
>> actually applied for a particular captured frame (if embedded data is
>> available reliably)
> We could decide that support for RGB/IR stream interleaving requires the
> ability to capture embedded data. Some people may complain though.
>
> --
> Regards,
>
> Laurent Pinchart