[PATCH 0/3] media: i2c: ov5693: Fix mode changes, flips and autosuspend

[PATCH 0/3] media: i2c: ov5693: Fix mode changes, flips and autosuspend

[Marco Ziliani via B4 Relay]

Three fixes for the ov5693, found while getting the front camera of the
Microsoft Surface Pro 5 (IPU3) to work, and since tested on IPU6 and
IPU4P Surfaces as well.

Patch 1 programs the mode registers on every stream start. They are
only written on runtime PM resume, so a format change on a sensor that
stays powered never reaches the chip. This is the ov5693 counterpart of
commit 4e1446ac169a ("media: i2c: ov8865: Program the sensor on stream
start").

Patch 2 keeps the reported SBGGR10 order when a single flip is enabled
at full resolution: each flip moves the Bayer phase by one column, so
the output window is shifted by one column when hflip XOR vflip.

Patch 3 drops the usage count at stream stop with
pm_runtime_put_autosuspend(), so that the autosuspend delay counts from
stream stop rather than from the last resume.

Patch 1 has been public and in testing in the linux-surface tree since
2026-10-23 [2]. Simon Kruse's active state conversion [1], posted this
morning, makes the same change as part of a larger rework. Patch 1 is
kept here as a small standalone fix with a Fixes: tag, so that it can
be applied and backported on its own; should the rework land first, I
will rebase patches 2 and 3 on top of it.

Patches 1 and 2 were developed in the linux-surface tree and tested
there on a Surface Pro 5 (IPU3), a Surface Pro 7 (IPU4P) and a Surface
Pro 7+ and Pro 9 (IPU6); patch 3 follows a report by Dario Manresa in
the same thread [2]. The three patches as posted here have also been
run on a Surface Pro 7 (IPU4P) and a Surface Pro 9 (IPU6) there.

[1] https://lore.kernel.org/linux-media/20261027074933.21921-1-simon0x+git@proton.me/
[2] https://github.com/linux-surface/kernel/pull/181

Signed-off-by: Marco Ziliani <rylos78@gmail.com>
---
Marco Ziliani (3):
      media: i2c: ov5693: Program the mode registers on stream start
      media: i2c: ov5693: Keep the Bayer order when flipping at full resolution
      media: i2c: ov5693: Use autosuspend when stopping the stream

 drivers/media/i2c/ov5693.c | 71 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)
---
base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
change-id: 20261027-ov5693-fixes-8ec29752893a

Best regards,
--  
Marco Ziliani <rylos78@gmail.com>

[PATCH 1/3] media: i2c: ov5693: Program the mode registers on stream start

[Marco Ziliani via B4 Relay]

From: Marco Ziliani <rylos78@gmail.com>

The mode registers (readout window, binning and subsampling) are only
written from the runtime PM resume handler, and ov5693_set_fmt() merely
stores the requested mode. If the sensor stays powered between two
streams, the new mode never reaches the chip: the sensor keeps
streaming the previous mode while the CSI-2 receiver expects the newly
negotiated format.

This happens whenever the format is changed while the sensor is still
powered. On a Surface Pro 7+ (IPU6), with the sensor held powered
between two streams, OUTPUT_SIZE_X read back over I2C showed the sensor
stuck in the mode it was powered up in, and 0 of 16 streams that asked
for a different mode produced a frame. With this change, 32 of 32
streams completed and the register followed the requested mode. A
Surface Pro 9 (IPU6) gave the same result: without the change
OUTPUT_SIZE stayed at 2592x1944 in all 32 streams of 16 such switches,
with it the register matched the requested size in all 32. On a
Surface Pro 5 (IPU3), switching between 2592x1944 and a 1920x1080 crop
the same way, OUTPUT_SIZE_X stayed at 2592 and none of the 16 streams
in the new size produced a frame; with this change, 32 of 32 streams
gave 10 of 10 frames at the requested size.

Program the mode on every stream start, before the controls, so that
the mode registers don't clobber them. This matches what commit
4e1446ac169a ("media: i2c: ov8865: Program the sensor on stream start")
does for the ov8865.

Fixes: 89aef879cb53 ("media: i2c: Add support for ov5693 sensor")
Link: https://github.com/linux-surface/kernel/pull/181
Signed-off-by: Marco Ziliani <rylos78@gmail.com>
---
 drivers/media/i2c/ov5693.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 56599f9ca..579aa55b7 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -1000,6 +1000,19 @@ static int ov5693_s_stream(struct v4l2_subdev *sd, int enable)
 			return ret;
 
 		mutex_lock(&ov5693->lock);
+
+		/*
+		 * The mode registers are otherwise only written on runtime PM
+		 * resume, so a format change on a sensor that stays powered
+		 * would never reach the chip. Program the mode first, then the
+		 * controls, so the mode registers don't clobber them.
+		 */
+		ret = ov5693_mode_configure(ov5693);
+		if (ret) {
+			mutex_unlock(&ov5693->lock);
+			goto err_power_down;
+		}
+
 		ret = __v4l2_ctrl_handler_setup(&ov5693->ctrls.handler);
 		if (ret) {
 			mutex_unlock(&ov5693->lock);

-- 
2.56.0

[PATCH 2/3] media: i2c: ov5693: Keep the Bayer order when flipping at full resolution

[Marco Ziliani via B4 Relay]

From: Marco Ziliani <rylos78@gmail.com>

In the full (unbinned) readout each of the horizontal and vertical
flips moves the Bayer phase by one column. With exactly one flip
enabled the CSI-2 receiver gets GBRG instead of the SBGGR10 the driver
reports, which gives a magenta and green image; with both enabled the
two shifts cancel out.

The crop start is only honoured in steps of two pixels, so compensate
by shifting the output window by one column when hflip XOR vflip, and
widen the crop by two columns, within the pixel array, so that the
window still fits. Reprogram the window when either flip changes. The
binned readout is left unchanged.

With this change, raw 2592x1944 frames come out BGGR at all four
hflip/vflip settings on the Surface Pro 5 (IPU3); the same change in
the linux-surface tree gave BGGR at all four settings on a Surface
Pro 7 (IPU4P) and a Surface Pro 9 (IPU6). Without it, the order is only
right with both flips off or both on.

Fixes: 89aef879cb53 ("media: i2c: Add support for ov5693 sensor")
Link: https://github.com/linux-surface/kernel/pull/181
Signed-off-by: Marco Ziliani <rylos78@gmail.com>
---
 drivers/media/i2c/ov5693.c | 56 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 579aa55b7..ca0968498 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -361,6 +361,49 @@ static const u8 ov5693_test_pattern_bits[] = {
 
 /* V4L2 Controls Functions */
 
+/*
+ * In the full (unbinned) readout each flip moves the Bayer phase by one
+ * column, so with exactly one of them enabled the CSI-2 receiver gets GBRG
+ * instead of the SBGGR10 the driver reports; with both enabled the two shifts
+ * cancel out. The crop start is only honoured in steps of two pixels, so
+ * compensate by shifting the output window by one column instead, and widen
+ * the crop so that the window still fits.
+ */
+static bool ov5693_bayer_shift(struct ov5693_device *ov5693)
+{
+	const struct ov5693_mode *mode = &ov5693->mode;
+	bool hflip, vflip;
+
+	if (mode->binning_x || mode->binning_y)
+		return false;
+
+	hflip = ov5693->ctrls.hflip && ov5693->ctrls.hflip->val;
+	vflip = ov5693->ctrls.vflip && ov5693->ctrls.vflip->val;
+
+	return hflip ^ vflip;
+}
+
+static int ov5693_window_x_configure(struct ov5693_device *ov5693)
+{
+	const struct ov5693_mode *mode = &ov5693->mode;
+	bool shift = ov5693_bayer_shift(ov5693);
+	u32 crop_end = mode->crop.left + mode->crop.width;
+	int ret = 0;
+
+	/* Widen the crop for the shifted window, within the pixel array */
+	if (shift)
+		crop_end = min(crop_end + 2,
+			       max_t(u32, crop_end, OV5693_NATIVE_WIDTH - 1));
+
+	/* Offset X */
+	cci_write(ov5693->regmap, OV5693_OFFSET_START_X_REG, shift, &ret);
+
+	/* Crop End X */
+	cci_write(ov5693->regmap, OV5693_CROP_END_X_REG, crop_end, &ret);
+
+	return ret;
+}
+
 static int ov5693_flip_vert_configure(struct ov5693_device *ov5693,
 				      bool enable)
 {
@@ -373,7 +416,7 @@ static int ov5693_flip_vert_configure(struct ov5693_device *ov5693,
 	if (ret)
 		return ret;
 
-	return 0;
+	return ov5693_window_x_configure(ov5693);
 }
 
 static int ov5693_flip_horz_configure(struct ov5693_device *ov5693,
@@ -388,7 +431,7 @@ static int ov5693_flip_horz_configure(struct ov5693_device *ov5693,
 	if (ret)
 		return ret;
 
-	return 0;
+	return ov5693_window_x_configure(ov5693);
 }
 
 static int ov5693_get_exposure(struct ov5693_device *ov5693, s32 *value)
@@ -566,16 +609,13 @@ static int ov5693_mode_configure(struct ov5693_device *ov5693)
 	cci_write(ov5693->regmap, OV5693_CROP_START_X_REG, mode->crop.left,
 		  &ret);
 
-	/* Offset X */
-	cci_write(ov5693->regmap, OV5693_OFFSET_START_X_REG, 0, &ret);
-
 	/* Output Size X */
 	cci_write(ov5693->regmap, OV5693_OUTPUT_SIZE_X_REG, mode->format.width,
 		  &ret);
 
-	/* Crop End X */
-	cci_write(ov5693->regmap, OV5693_CROP_END_X_REG,
-		  mode->crop.left + mode->crop.width, &ret);
+	/* Offset X and Crop End X follow the flips, see ov5693_bayer_shift() */
+	if (!ret)
+		ret = ov5693_window_x_configure(ov5693);
 
 	/* Horizontal Total Size */
 	cci_write(ov5693->regmap, OV5693_TIMING_HTS_REG, OV5693_FIXED_PPL,

-- 
2.56.0

[PATCH 3/3] media: i2c: ov5693: Use autosuspend when stopping the stream

[Marco Ziliani via B4 Relay]

From: Marco Ziliani <rylos78@gmail.com>

The driver enables runtime PM autosuspend with a 1 second delay at
probe, but drops the usage count at stream stop with pm_runtime_put(),
which doesn't update the device's last busy time. The autosuspend
delay is then counted from the runtime resume at stream start rather
than from stream stop, so after any stream longer than the delay the
sensor is powered off right away.

Use pm_runtime_put_autosuspend(), which marks the device busy, so that
the delay counts from stream stop as intended, and a following stream
started within it doesn't power the sensor up from scratch again. With
the mode now programmed on every stream start, keeping the sensor
powered between streams no longer leaves it in a stale mode.

On a Surface Pro 9 (IPU6) with autosuspend_delay_ms set to 3000, the
sensor suspended 1-2 ms after a 5 s stream stopped; with this change,
3.7 s after it, i.e. the delay plus the autosuspend timer slack. On a
Surface Pro 5 (IPU3) with the default 1 s delay, the sensor suspended
as soon as a 2 s stream stopped; with this change, 1.1 s after it.

Fixes: 89aef879cb53 ("media: i2c: Add support for ov5693 sensor")
Reported-by: D. Manresa <dmanresa@gmail.com>
Closes: https://github.com/linux-surface/kernel/pull/181#issuecomment-6436016335
Signed-off-by: Marco Ziliani <rylos78@gmail.com>
---
 drivers/media/i2c/ov5693.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index ca0968498..ef5e7285c 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -1070,7 +1070,7 @@ static int ov5693_s_stream(struct v4l2_subdev *sd, int enable)
 		goto err_power_down;
 
 	if (!enable)
-		pm_runtime_put(ov5693->dev);
+		pm_runtime_put_autosuspend(ov5693->dev);
 
 	return 0;
 err_power_down:

-- 
2.56.0

Re: [PATCH 0/3] media: i2c: ov5693: Fix mode changes, flips and autosuspend

[Chris Simpson]

Tested on a Surface Pro 9 (Intel Alder Lake-U, i7-1255U, IPU6, front
OV5693) in a 7.3-based kernel, with the series applied to an ov5693.c
that matches the base-commit (d7e7bca71698) apart from the pad ops'
client_info argument, against the same file unpatched, using raw
captures from the ISYS video node:

- Patch 1: with the sensor held powered between streams, OUTPUT_SIZE
  (read over I2C during each stream) matched the requested size in
  32 of 32 streams requesting 2592x1944 or 1296x972, and every
  stream delivered image data. Unpatched, it stayed at 2592x1944 in all
  32 and the binned streams gave no image.
- Patch 2: raw 2592x1944 frames are BGGR at all four hflip/vflip
  settings. Unpatched, the order is GBRG with exactly one flip on.
- Patch 3: with autosuspend_delay_ms at 3000, the sensor suspended
  3.4-3.7 s after stream stop (the delay plus the 25% timer slack),
  after both 1 s and 5 s streams. Unpatched, 1-2 ms after a 5 s stream.

The test scripts, the analysis of their output and this summary were
prepared with an AI coding assistant (Claude); I ran the tests on my
Surface Pro 9.

Tested-by: Chris Simpson <chris@nilk.net>

Re: [PATCH 0/3] media: i2c: ov5693: Fix mode changes, flips and autosuspend

[Tony Garcia]

Tested on a Surface Pro 7 (Ice Lake). Its front OV5693 sits behind an
IPU4P, which I run with ruslanbay's IPU4P-on-IPU6 RFC series on
linux-surface 7.3.0-rc7. The three patches were applied to the 7.3
ov5693.c (they apply cleanly). Raw capture from the ISYS video node with
v4l2-ctl, frames saved and checked buffer by buffer (bytesused and pixel
data), no libcamera.

The patches tested were the ones posted in the linux-surface/kernel#181
thread. The code in this v1 is identical: applying each patch here and
each tested one to the same ov5693.c gives the same tree after every
patch.

Patch 1 (mode registers on stream start): with the sensor held powered
(power/control=on), 8 full->binned and 8 binned->full switches:
OUTPUT_SIZE_X (0x3808) read back over I2C during each stream matched the
requested width 16 of 16 times, and all 16 streams delivered frames. Out
of window (sensor runtime-suspended before each stream), 5 + 5 switches:
10 of 10. The unpatched driver, same in-window test: 0x3808 stayed at
2592 after each switch to 1296x972, and none of the 6 streams after the
first switch produced a frame.

Patch 2 (Bayer order with flips): 2592x1944 at all four hflip/vflip
settings, the sensor reported SBGGR10 and the 2x2 sub-plane test found
the green pair on the BGGR/RGGB diagonal in all four, with the same
blue/red plane order each time. Comparing image profiles against
hflip=0/vflip=0 confirmed each setting really mirrored the image. 45 of
45 buffers at each setting had full bytesused and non-constant data.

Patch 3 (autosuspend at stream stop): after a 60-frame stream, the
sensor stayed active for 1.2 s and was suspended at the 1.3 s sample
(100 ms sampling, 3 of 3 runs, 1 s autosuspend delay). With the old
pm_runtime_put() (my installed 7.3 driver, whose stop path is stock) it
was already suspended at the first sample (3 of 3).

Also 1296x972 binned: 10 module loads x 2 captures, 900 of 900 buffers
with full bytesused and real image data, no CSI-2 errors.

The tests were scripted and run on my Surface Pro 7 by an AI coding
assistant (Claude) at my direction, and it drafted this summary.

Tested-by: Tony Garcia <tony@loafecho.com>