mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mirela Rabulea <mirela.rabulea@nxp.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Rishikesh Donadkar <r-donadkar@ti.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	tomi.valkeinen@ideasonboard.com, mchehab@kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	nm@ti.com, vigneshr@ti.com, kristo@kernel.org,
	mripard@kernel.org, jai.luthra@linux.dev,
	jai.luthra@ideasonboard.com, devarsht@ti.com,
	y-abhilashchandra@ti.com
Subject: Re: Re: [RFC PATCH 6/8] media: i2c: ov2312: add Omnivison OV2312 driver
Date: Tue, 6 Oct 2026 17:37:28 +0200	[thread overview]
Message-ID: <20261006153728.GA622105@killaraus.ideasonboard.com> (raw)
In-Reply-To: <577b34a1-4c2f-487d-a4fa-e7ede2b60091@nxp.com>

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).

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

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

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

  reply	other threads:[~2026-10-06 15:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 13:29 [RFC PATCH 0/8] Add OmniVision OV2312 RGB-IR sensor driver Rishikesh Donadkar
2026-09-25 13:29 ` [RFC PATCH 1/8] dt-bindings: media: Add bindings for Omnivision OV2312 Rishikesh Donadkar
2026-09-26 14:51   ` Laurent Pinchart
2026-09-25 13:29 ` [RFC PATCH 2/8] media: v4l: Add 10-bit RGBIr formats Rishikesh Donadkar
2026-09-26 14:30   ` Sakari Ailus
2026-09-26 14:40     ` Laurent Pinchart
2026-09-27  5:09       ` Rishikesh Donadkar
2026-09-27  5:08     ` Rishikesh Donadkar
2026-09-27  5:59       ` Sakari Ailus
2026-09-25 13:29 ` [RFC PATCH 3/8] media: i2c: ds90ub960: " Rishikesh Donadkar
2026-09-25 13:29 ` [RFC PATCH 4/8] media: cadence: csi2rx: Add RAW10 " Rishikesh Donadkar
2026-09-26 14:52   ` Laurent Pinchart
2026-09-27  5:20     ` Rishikesh Donadkar
2026-09-27 14:19       ` Laurent Pinchart
2026-09-25 13:29 ` [RFC PATCH 5/8] media: ti: j721e-csi2rx: " Rishikesh Donadkar
2026-09-25 13:29 ` [RFC PATCH 6/8] media: i2c: ov2312: add Omnivison OV2312 driver Rishikesh Donadkar
2026-10-02 16:16   ` Mirela Rabulea
2026-10-03  2:05     ` Jai Luthra
2026-10-05 18:12       ` Mirela Rabulea
2026-10-06  5:19     ` Rishikesh Donadkar
2026-10-06 12:33       ` [EXT] " Mirela Rabulea
2026-10-06  7:36     ` Sakari Ailus
2026-10-06 13:55       ` Mirela Rabulea
2026-10-06 15:37         ` Laurent Pinchart [this message]
2026-10-07  7:16           ` Mirela Rabulea
2026-09-25 13:30 ` [RFC PATCH 7/8] arm64: dts: ti: k3-am62a7: FPDLink overlays for LI OV2312 Rishikesh Donadkar
2026-09-26 14:57   ` Laurent Pinchart
2026-09-27  5:23     ` Rishikesh Donadkar
2026-09-25 13:30 ` [RFC PATCH 8/8] arm64: defconfig: Enable OV2312 Rishikesh Donadkar
2026-09-26 14:55   ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261006153728.GA622105@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=conor+dt@kernel.org \
    --cc=devarsht@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jai.luthra@ideasonboard.com \
    --cc=jai.luthra@linux.dev \
    --cc=kristo@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mirela.rabulea@nxp.com \
    --cc=mripard@kernel.org \
    --cc=nm@ti.com \
    --cc=r-donadkar@ti.com \
    --cc=robh@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=vigneshr@ti.com \
    --cc=y-abhilashchandra@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®