Re: [EXT] Re: [RFC PATCH 6/8] media: i2c: ov2312: add Omnivison OV2312 driver
From: Mirela Rabulea
Date: Tue Oct 06 2026 - 08:33:28 EST
On 10/6/26 08:19, Rishikesh Donadkar wrote:
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
On 02/10/26 21:46, Mirela Rabulea wrote:
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>
---
drivers/media/i2c/Kconfig | 12 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/ov2312.c | 939 +++++++++++++++++++++++++++++++++++++
drivers/media/i2c/ov2312.h | 285 +++++++++++
4 files changed, 1237 insertions(+)
create mode 100644 drivers/media/i2c/ov2312.c
create mode 100644 drivers/media/i2c/ov2312.h
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 4d9946479160..8d9d8d491b2e 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -496,6 +496,18 @@ config VIDEO_OV13B10
This is a Video4Linux2 sensor driver for the OmniVision
OV13B10 camera.
+I've received in the past feedback to use cci_* helpers instead of
+static int ov2312_read(struct ov2312 *ov2312, u16 addr, u32 *val,
size_t nbytes)
+{
+ int ret;
+ __le32 val_le = 0;
+
+ ret = regmap_bulk_read(ov2312->regmap, addr, &val_le, nbytes);
regmap, see drivers/media/v4l2-core/v4l2-cci.c
Sure, I will use them.
+ if (ret < 0) {
+ dev_err(ov2312->dev, "%s: failed to read reg 0x%04x:
%d\n",
+ __func__, addr, ret);
+ return ret;
+ }
+
+ *val = le32_to_cpu(val_le);
+ return 0;
+}
+
+static int ov2312_write(struct ov2312 *ov2312, u16 addr, u32 val,
size_t nbytes)
+{
+ int ret;
+ __le32 val_le = cpu_to_le32(val);
+
+ ret = regmap_bulk_write(ov2312->regmap, addr, &val_le, nbytes);
+ if (ret < 0)
+ dev_err(ov2312->dev, "%s: failed to write reg 0x%04x:
%d\n",
+ __func__, addr, ret);
+ return ret;
+}
+
+static int ov2312_write_table(struct ov2312 *ov2312,
+ const struct reg_sequence *regs,
+ unsigned int nr_regs)
+{
+ int ret, i;
+
+ for (i = 0; i < nr_regs; i++) {
+ ret = regmap_write(ov2312->regmap, regs[i].reg,
regs[i].def);
+ if (ret < 0) {
+ dev_err(ov2312->dev,
+ "%s: failed to write reg[%d] 0x%04x =
0x%02x (%d)!\n",
+ __func__, i, regs[i].reg, regs[i].def,
ret);
+ return ret;
+ }
+ }
+ return 0;
+}
+
+ */
+static int ov2312_set_group_a(struct ov2312 *ov2312)
+{
+ u32 ir_exposure = ov2312->exposure_multi->p_new.p_u32[1];
+ u32 ir_again = ov2312->again_multi->p_new.p_u32[1];
+ u32 ir_dgain = ov2312->dgain_multi->p_new.p_u32[1];
+ u32 ir_strobe_start = OV2312_VTS - ir_exposure - 7;
+ int ret;
+
+ struct reg_sequence ov2312_groupA[] = {
+ {0x3208, 0x00},/* Group A (IR Dominant VC0) */
+ {OV2312_AEC_PK_EXPO_HI, (ir_exposure >> 8) & 0xff},
+ {OV2312_AEC_PK_EXPO_LO, ir_exposure & 0xff},
+ {OV2312_AEC_PK_AGAIN_HI, (ir_again >> 4) & 0xff},
+ {OV2312_AEC_PK_AGAIN_LO, (ir_again & 0x0f) << 4},
+ {OV2312_AEC_PK_DGAIN_HI, (ir_dgain >> 8) & 0xff},
+ {OV2312_AEC_PK_DGAIN_LO, ir_dgain & 0xff},
+ {0x3920, 0xff},/* IR Strobe duty cycle */
+ {0x3927, (ir_exposure >> 8) & 0xff},
+ {0x3928, ir_exposure & 0xff},
+ {0x3929, (ir_strobe_start >> 8) & 0xff},
+ {0x392a, ir_strobe_start & 0xff},
+ {0x4813, 0x01},/* VC=1. This register takes effect
from next frame */
+ {0x3208, 0x10},
+ {0x320D, 0x00},/* Auto mode switch between group0 and
group1 ;setting to switch */
+ {0x320D, 0x31},
+ {0x3208, 0xA0},
+ };
+
+ ret = regmap_register_patch(ov2312->regmap, ov2312_groupA,
+ ARRAY_SIZE(ov2312_groupA));
+ if (ret < 0)
+ dev_err(ov2312->dev,
+ "%s: failed to apply Group A register patch
(%d)!\n",
+ __func__, ret);
+ return ret;
+}
+
+static int ov2312_set_group_b(struct ov2312 *ov2312)
+{
+ u32 rgb_exposure = ov2312->exposure_multi->p_new.p_u32[0];
+ u32 rgb_again = ov2312->again_multi->p_new.p_u32[0];
+ u32 rgb_dgain = ov2312->dgain_multi->p_new.p_u32[0];
+ int ret;
+
+ struct reg_sequence ov2312_groupB[] = {
+ {0x3208, 0x01},/* Group B (RGB Dominant VC1) */
+ {OV2312_AEC_PK_EXPO_HI, (rgb_exposure >> 8) & 0xff},
+ {OV2312_AEC_PK_EXPO_LO, rgb_exposure & 0xff},
+ {OV2312_AEC_PK_AGAIN_HI, (rgb_again >> 4) & 0xff},
+ {OV2312_AEC_PK_AGAIN_LO, (rgb_again & 0x0f) << 4},
+ {OV2312_AEC_PK_DGAIN_HI, (rgb_dgain >> 8) & 0xff},
+ {OV2312_AEC_PK_DGAIN_LO, rgb_dgain & 0xff},
+ {0x3920, 0x00},
+ {0x4813, 0x00},/* VC=0. This register takes effect
from next frame */
+ {0x3208, 0x11},
+ {0x320D, 0x00},/* Auto mode switch between group0 and
group1 ;setting to switch */
+ {0x320D, 0x30},
+ {0x3208, 0xA0},
+ };
+
+ ret = regmap_register_patch(ov2312->regmap, ov2312_groupB,
+ ARRAY_SIZE(ov2312_groupB));
+ if (ret < 0)
+ dev_err(ov2312->dev,
+ "%s: failed to apply Group B register patch
(%d)!\n",
+ __func__, ret);
+ return ret;
+}
+
+static int ov2312_set_AB_mode(struct ov2312 *ov2312)
+{
+ bool ir_ready = ov2312->exposure_multi->p_new.p_u32[1] &&
+ ov2312->again_multi->p_new.p_u32[1] &&
+ ov2312->dgain_multi->p_new.p_u32[1];
+ bool rgb_ready = ov2312->exposure_multi->p_new.p_u32[0] &&
+ ov2312->again_multi->p_new.p_u32[0] &&
+ ov2312->dgain_multi->p_new.p_u32[0];
+ int ret;
+
+ if (ir_ready) {
+ ret = ov2312_set_group_a(ov2312);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (rgb_ready) {
+ ret = ov2312_set_group_b(ov2312);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Wait for 1 frame duration after setting AB mode registers */
+ if (ir_ready || rgb_ready)
+ msleep(33);
+
+ return 0;
+}
+
+static int ov2312_set_orientation(struct ov2312 *ov2312)
+{
+ bool v_flip = ov2312->v_flip->val;
+ bool h_flip = ov2312->h_flip->val;
+ u32 reg = (v_flip ? 0x4400 : 0) | (h_flip ? 0x0004 : 0);
+
+ return ov2312_write(ov2312, OV2312_TIMING_VFLIP,
be16_to_cpu(reg), 2);
+}
+
+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).
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.
Just to clarify, when you say "update each group only when its context
is active" you mean when the frame for that group is being captured right?
Lets say group0 is launched, do we need to set the exposure for group0
before the next group i.e. group1 is launched?
Rishikesh
Yes. For more exact details on the timings required, you may check "Context switch (AB mode) group write timeline" here:
https://github.com/nxp-imx/linux-imx/blob/lf-6.18.y/drivers/media/i2c/ox05b1s/ox05b1s_mipi.c#L795
Group0 update (exp0, vc0) needs to be between group0 launch points (t0), but the sensor driver is unaware when the launch points take place, so instead, we try to update group 0 in the first half of t0, which we can determine by querying the status register for the active context.
Check if your 0x322d register provides information about the active group/context.
Maybe it makes sense to do an even more extensive study on the level of compatibility on the register set between ox05b1s and OV2312? I am wondering if it makes sense for these 2 to share a driver, with a compatible for each.
Regards,
Mirela
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.
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).
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.
Regards,
Mirela
+ break;
+
+ case V4L2_CID_HFLIP:
+ case V4L2_CID_VFLIP:
+ ret = ov2312_set_orientation(ov2312);
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+