* [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices
@ 2026-07-08 21:36 Fernando Rimoli
2026-07-09 13:17 ` Dan Scally
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Fernando Rimoli @ 2026-07-08 21:36 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, linux-media
Cc: Mauro Carvalho Chehab, linux-kernel, Fernando Rimoli
The ov5693 driver only matches the "INT33BE" ACPI HID. Some Intel IPU6
Surface devices (e.g. Microsoft Surface Pro 9) enumerate the same OV5693
front camera with the ACPI HID "OVTI5693" instead. Without this HID the
i2c core never binds the driver, and the IPU bridge never builds the
firmware node graph for the sensor, so the front camera is unusable.
Add "OVTI5693" to the sensor driver's ACPI match table and to the IPU
bridge's supported-sensor list. The link frequency (419200000) matches
the existing INT33BE entry, as it is the same sensor. Devices that use
"INT33BE" (e.g. Surface Go 4) are unaffected.
Tested on Surface Pro 9 (IPU6): the sensor enumerates as OVTI5693:00
(ACPI path \_SB_.PC00.I2C3.CAMF) and binds with this change.
Link: https://github.com/linux-surface/linux-surface/pull/2171
Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com>
---
Note: this HID addition makes the sensor bind and enumerate, but a fully
working front camera on IPU6 also requires programming MIPI_CTRL00
(0x4800 = 0x2d) before stream-on, otherwise the IPU6 CSI-2 receiver never
locks and capture times out. That register change is being handled
separately (see the Link: above). This patch is independent and useful on
its own for enumeration, so I'm sending it standalone; happy to reorder if
you'd prefer it after the register fix.
drivers/media/i2c/ov5693.c | 1 +
drivers/media/pci/intel/ipu-bridge.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 4cc796bbe..02236f3db 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -1396,6 +1396,7 @@ static const struct dev_pm_ops ov5693_pm_ops = {
static const struct acpi_device_id ov5693_acpi_match[] = {
{"INT33BE"},
+ {"OVTI5693"},
{},
};
MODULE_DEVICE_TABLE(acpi, ov5693_acpi_match);
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 88581a4c0..b70d61099 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -93,6 +93,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000),
/* Omnivision OV5675 */
IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000),
+ /* Omnivision OV5693 */
+ IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000),
/* Omnivision OV8856 */
IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000),
/* Sony IMX471 */
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices 2026-07-08 21:36 [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices Fernando Rimoli @ 2026-07-09 13:17 ` Dan Scally 2026-07-14 9:32 ` Sakari Ailus 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli 2 siblings, 0 replies; 18+ messages in thread From: Dan Scally @ 2026-07-09 13:17 UTC (permalink / raw) To: Fernando Rimoli, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, linux-kernel Hi Fernando,thanks for the patch On 08/07/2026 22:36, Fernando Rimoli wrote: > The ov5693 driver only matches the "INT33BE" ACPI HID. Some Intel IPU6 > Surface devices (e.g. Microsoft Surface Pro 9) enumerate the same OV5693 > front camera with the ACPI HID "OVTI5693" instead. Without this HID the > i2c core never binds the driver, and the IPU bridge never builds the > firmware node graph for the sensor, so the front camera is unusable. > > Add "OVTI5693" to the sensor driver's ACPI match table and to the IPU > bridge's supported-sensor list. The link frequency (419200000) matches > the existing INT33BE entry, as it is the same sensor. Devices that use > "INT33BE" (e.g. Surface Go 4) are unaffected. > > Tested on Surface Pro 9 (IPU6): the sensor enumerates as OVTI5693:00 > (ACPI path \_SB_.PC00.I2C3.CAMF) and binds with this change. > > Link: https://github.com/linux-surface/linux-surface/pull/2171 > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > --- Looks good to me: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > Note: this HID addition makes the sensor bind and enumerate, but a fully > working front camera on IPU6 also requires programming MIPI_CTRL00 > (0x4800 = 0x2d) before stream-on, otherwise the IPU6 CSI-2 receiver never > locks and capture times out. That register change is being handled > separately (see the Link: above). This patch is independent and useful on > its own for enumeration, so I'm sending it standalone; happy to reorder if > you'd prefer it after the register fix. Standalone is fine :) Thanks Dan > > drivers/media/i2c/ov5693.c | 1 + > drivers/media/pci/intel/ipu-bridge.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c > index 4cc796bbe..02236f3db 100644 > --- a/drivers/media/i2c/ov5693.c > +++ b/drivers/media/i2c/ov5693.c > @@ -1396,6 +1396,7 @@ static const struct dev_pm_ops ov5693_pm_ops = { > > static const struct acpi_device_id ov5693_acpi_match[] = { > {"INT33BE"}, > + {"OVTI5693"}, > {}, > }; > MODULE_DEVICE_TABLE(acpi, ov5693_acpi_match); > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index 88581a4c0..b70d61099 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c > @@ -93,6 +93,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = { > IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000), > /* Omnivision OV5675 */ > IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000), > + /* Omnivision OV5693 */ > + IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000), > /* Omnivision OV8856 */ > IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000), > /* Sony IMX471 */ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices 2026-07-08 21:36 [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices Fernando Rimoli 2026-07-09 13:17 ` Dan Scally @ 2026-07-14 9:32 ` Sakari Ailus 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli 2 siblings, 0 replies; 18+ messages in thread From: Sakari Ailus @ 2026-07-14 9:32 UTC (permalink / raw) To: Fernando Rimoli Cc: Daniel Scally, linux-media, Mauro Carvalho Chehab, linux-kernel Hi Fernando, On Wed, Jul 08, 2026 at 11:36:33PM +0200, Fernando Rimoli wrote: > The ov5693 driver only matches the "INT33BE" ACPI HID. Some Intel IPU6 > Surface devices (e.g. Microsoft Surface Pro 9) enumerate the same OV5693 > front camera with the ACPI HID "OVTI5693" instead. Without this HID the > i2c core never binds the driver, and the IPU bridge never builds the > firmware node graph for the sensor, so the front camera is unusable. Could you split this into two, once for each? > > Add "OVTI5693" to the sensor driver's ACPI match table and to the IPU > bridge's supported-sensor list. The link frequency (419200000) matches > the existing INT33BE entry, as it is the same sensor. Devices that use > "INT33BE" (e.g. Surface Go 4) are unaffected. > > Tested on Surface Pro 9 (IPU6): the sensor enumerates as OVTI5693:00 > (ACPI path \_SB_.PC00.I2C3.CAMF) and binds with this change. > > Link: https://github.com/linux-surface/linux-surface/pull/2171 > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > --- > Note: this HID addition makes the sensor bind and enumerate, but a fully > working front camera on IPU6 also requires programming MIPI_CTRL00 > (0x4800 = 0x2d) before stream-on, otherwise the IPU6 CSI-2 receiver never > locks and capture times out. That register change is being handled > separately (see the Link: above). This patch is independent and useful on > its own for enumeration, so I'm sending it standalone; happy to reorder if > you'd prefer it after the register fix. Have you tested other values for 0x4800? Besides IPU3 (CIO2), the sensor is used on a Rockchip board, so changing the settings needs to be done with care. If you haven't, I'd be interested to know how 0x04 works -- it should be a safe change (LP00 -> LP11 in idle). If not, then testing other individual bits would be useful as well; hopefully changing one bit makes a difference. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/3] media: Enable the OV5693 front camera on IPU6 Surface devices 2026-07-08 21:36 [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices Fernando Rimoli 2026-07-09 13:17 ` Dan Scally 2026-07-14 9:32 ` Sakari Ailus @ 2026-07-17 13:20 ` Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 1/3] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli ` (3 more replies) 2 siblings, 4 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-17 13:20 UTC (permalink / raw) To: Daniel Scally, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, linux-kernel, Fernando Rimoli Apologies for the slow turnaround on v2, I wanted to properly characterise the MIPI_CTRL00 register before answering, and I was coordinating with Arsalan Naeem, who originally found the register write (linux-surface PR#2171); the third patch is co-developed with him. This enables the OV5693 front camera on Intel IPU6 Surface devices such as the Microsoft Surface Pro 9. Three independent pieces are needed on a stock kernel: 1/3 ov5693: add the "OVTI5693" ACPI HID so the i2c core binds the driver (these devices enumerate the sensor as OVTI5693, not INT33BE). 2/3 ipu-bridge: add the same HID so the bridge builds the sensor's fwnode graph. 3/3 ov5693: gate the MIPI clock lane, which the IPU6 CSI-2 receiver needs in order to lock (this is the register write from PR#2171). On the MIPI_CTRL00 value, in reply to Sakari's question on v1: I swept the register on a Surface Pro 9 (IPU6) with read-back. The power-on default reads 0x00. Every value with bit 5 (clock-lane gate) set streams (300/300 frames, steady ~28 fps); every value with bit 5 clear, including 0x00 and 0x04 (LP11-idle alone), fails with "stream stop time out" and no frames. So bit 5 is the decisive bit for IPU6, and 0x04 alone is not enough. Bit 5 is not an IPU6-specific hack: ov5640, ov5647 and ov5648 all gate the clock lane on every platform. Patch 3 sets bit 5 plus bit 2 (LP11-idle, i.e. 0x24, matching ov5640's value); I have only been able to test this on IPU6, not on IPU3 (CIO2) or Rockchip, so I have kept it to the conventional OV-family bits rather than the vendor's 0x2d (which also sets bit 3 and an undocumented bit 0). Patches 1-2 are independent of patch 3 and useful on their own for enumeration; patch 1 carries Dan Scally's Reviewed-by from v1. Changes since v1 [1]: - Split the single v1 patch into the two HID patches (1/3, 2/3), as requested by Sakari. - Added patch 3/3, the MIPI clock-lane gate, co-developed with Arsalan Naeem, with the characterisation above answering Sakari's v1 question. - Carried Daniel Scally's Reviewed-by on patch 1/3. - Used named bit macros instead of a bare register value. [1] https://lore.kernel.org/linux-media/20260708213633.18392-1-fernandorimoli11@gmail.com/ Fernando Rimoli (3): media: i2c: ov5693: Add OVTI5693 ACPI HID media: ipu-bridge: Add OVTI5693 to the list of supported sensors media: i2c: ov5693: Gate the MIPI clock lane for IPU6 drivers/media/i2c/ov5693.c | 21 +++++++++++++++++++++ drivers/media/pci/intel/ipu-bridge.c | 2 ++ 2 files changed, 23 insertions(+) -- 2.43.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] media: i2c: ov5693: Add OVTI5693 ACPI HID 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli @ 2026-07-17 13:20 ` Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 2/3] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-17 13:20 UTC (permalink / raw) To: Daniel Scally, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, linux-kernel, Fernando Rimoli The ov5693 driver only matches the "INT33BE" ACPI HID. Some Intel IPU6 Surface devices (e.g. Microsoft Surface Pro 9) enumerate the OV5693 front camera with the ACPI HID "OVTI5693" instead, so the i2c core never binds the driver. Add "OVTI5693" to the ACPI match table. Devices that use "INT33BE" (e.g. Surface Go 4) are unaffected. Tested on Surface Pro 9 (IPU6): the sensor enumerates as OVTI5693:00 (ACPI path \_SB_.PC00.I2C3.CAMF) and binds with this change. Link: https://github.com/linux-surface/linux-surface/pull/2171 Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> --- drivers/media/i2c/ov5693.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c index 4cc796bbe..02236f3db 100644 --- a/drivers/media/i2c/ov5693.c +++ b/drivers/media/i2c/ov5693.c @@ -1396,6 +1396,7 @@ static const struct dev_pm_ops ov5693_pm_ops = { static const struct acpi_device_id ov5693_acpi_match[] = { {"INT33BE"}, + {"OVTI5693"}, {}, }; MODULE_DEVICE_TABLE(acpi, ov5693_acpi_match); -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] media: ipu-bridge: Add OVTI5693 to the list of supported sensors 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 1/3] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli @ 2026-07-17 13:20 ` Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli 3 siblings, 0 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-17 13:20 UTC (permalink / raw) To: Daniel Scally, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, linux-kernel, Fernando Rimoli The IPU bridge builds the firmware node graph only for sensors listed in ipu_supported_sensors[]. The OV5693 is currently listed only under its legacy "INT33BE" HID, so on Intel IPU6 Surface devices that enumerate it as "OVTI5693" (e.g. Microsoft Surface Pro 9) the bridge never wires up the sensor and the front camera is unusable. Add an "OVTI5693" entry. The link frequency (419200000) matches the existing INT33BE entry, as it is the same sensor. Tested on Surface Pro 9 (IPU6). Link: https://github.com/linux-surface/linux-surface/pull/2171 Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> --- drivers/media/pci/intel/ipu-bridge.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c index 88581a4c0..b70d61099 100644 --- a/drivers/media/pci/intel/ipu-bridge.c +++ b/drivers/media/pci/intel/ipu-bridge.c @@ -93,6 +93,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = { IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000), /* Omnivision OV5675 */ IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000), + /* Omnivision OV5693 */ + IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000), /* Omnivision OV8856 */ IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000), /* Sony IMX471 */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 1/3] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 2/3] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli @ 2026-07-17 13:20 ` Fernando Rimoli 2026-07-19 16:25 ` Jakob Berg Jespersen 2026-07-19 22:42 ` Sakari Ailus 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli 3 siblings, 2 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-17 13:20 UTC (permalink / raw) To: Daniel Scally, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, linux-kernel, Fernando Rimoli The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00 power-on default (free-running MIPI clock). The IPU3 CSI-2 receiver tolerates this, but the IPU6 receiver (e.g. on Microsoft Surface Pro 8/9 and Surface Go 4) fails to lock onto the link, so the sensor streams but capture times out with "stream stop time out" and no frames arrive. Gating the clock lane when idle (MIPI_CTRL00 bit 5) makes the IPU6 receiver lock. This was determined by sweeping the register on a Surface Pro 9 (IPU6): every value with bit 5 set streams (300/300 frames, steady 28 fps), every value with bit 5 clear fails with the CSI-2 timeout; register read-back confirmed the power-on default is 0x00. Bit 5 is the conventional OmniVision clock-lane-gate setting (ov5640, ov5647 and ov5648 all set it on every platform), so it is not an IPU6-specific hack. Bit 2 (bus idle in LP11) is set as well, matching ov5640's value; this was tested only on IPU6. Link: https://github.com/linux-surface/linux-surface/pull/2171 Co-developed-by: Arsalan Naeem <naeemarsalan@gmail.com> Signed-off-by: Arsalan Naeem <naeemarsalan@gmail.com> Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> --- drivers/media/i2c/ov5693.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c index 02236f3db..d1b38348e 100644 --- a/drivers/media/i2c/ov5693.c +++ b/drivers/media/i2c/ov5693.c @@ -35,6 +35,12 @@ #define OV5693_STOP_STREAMING 0x00 #define OV5693_SW_RESET 0x01 +/* MIPI transmitter control */ +#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800) +/* bit 5: gate the clock lane when idle; bit 2: keep the bus in LP11 when idle */ +#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5) +#define OV5693_MIPI_CTRL00_BUS_IDLE BIT(2) + #define OV5693_REG_CHIP_ID CCI_REG16(0x300a) /* Yes, this is right. The datasheet for the OV5693 gives its ID as 0x5690 */ #define OV5693_CHIP_ID 0x5690 @@ -611,6 +617,20 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable) { int ret = 0; + /* + * Gate the MIPI clock lane when idle. The power-on default of + * MIPI_CTRL00 is 0x00 (free-running clock), which the IPU3 CSI-2 + * receiver tolerates but the IPU6 one does not: it fails to lock onto + * the link and capture times out. Gating the clock lane (bit 5) is + * required for IPU6 and is the conventional setting for OmniVision + * sensors (cf. ov5640, ov5647, ov5648); also keep the bus in LP11 when + * idle (bit 2). Harmless on IPU3. + */ + if (enable) + cci_write(ov5693->regmap, OV5693_MIPI_CTRL00_REG, + OV5693_MIPI_CTRL00_CLOCK_LANE_GATE | + OV5693_MIPI_CTRL00_BUS_IDLE, &ret); + cci_write(ov5693->regmap, OV5693_SW_STREAM_REG, enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING, &ret); -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 2026-07-17 13:20 ` [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 Fernando Rimoli @ 2026-07-19 16:25 ` Jakob Berg Jespersen 2026-07-19 22:42 ` Sakari Ailus 1 sibling, 0 replies; 18+ messages in thread From: Jakob Berg Jespersen @ 2026-07-19 16:25 UTC (permalink / raw) To: fernandorimoli11 Cc: dan.scally, sakari.ailus, naeemarsalan, mchehab, linux-media, linux-kernel, Jakob Berg Jespersen On Fri, 17 Jul 2026, Fernando Rimoli wrote: > The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00 > power-on default (free-running MIPI clock). The IPU3 CSI-2 receiver > tolerates this, but the IPU6 receiver [...] fails to lock onto the > link, so the sensor streams but capture times out [...] Confirmed on the Microsoft Surface Pro 7+ (Tiger Lake IPU6). Its front ov5693 enumerates as INT33BE, so just patch 3 seems relevant. With the unpatched driver, only 1/20 cold opens delivered frames, the rest timed out with zero frames. With this patch applied, 20/20 streamed at ~28-29 fps. Tested-by: Jakob Berg Jespersen <dev@berg.pm> (Surface Pro 7+, Fedora 44, kernel 7.0.12-200.fc44.x86_64; libcamera cam tool.) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 2026-07-17 13:20 ` [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 Fernando Rimoli 2026-07-19 16:25 ` Jakob Berg Jespersen @ 2026-07-19 22:42 ` Sakari Ailus 1 sibling, 0 replies; 18+ messages in thread From: Sakari Ailus @ 2026-07-19 22:42 UTC (permalink / raw) To: Fernando Rimoli Cc: Daniel Scally, linux-media, Mauro Carvalho Chehab, Arsalan Naeem, linux-kernel Hi Fernando, Thank you for the update. On Fri, Jul 17, 2026 at 03:20:21PM +0200, Fernando Rimoli wrote: > The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00 > power-on default (free-running MIPI clock). The IPU3 CSI-2 receiver > tolerates this, but the IPU6 receiver (e.g. on Microsoft Surface Pro 8/9 > and Surface Go 4) fails to lock onto the link, so the sensor streams but > capture times out with "stream stop time out" and no frames arrive. > > Gating the clock lane when idle (MIPI_CTRL00 bit 5) makes the IPU6 > receiver lock. This was determined by sweeping the register on a Surface > Pro 9 (IPU6): every value with bit 5 set streams (300/300 frames, steady Nice find! > 28 fps), every value with bit 5 clear fails with the CSI-2 timeout; > register read-back confirmed the power-on default is 0x00. Bit 5 is the > conventional OmniVision clock-lane-gate setting (ov5640, ov5647 and > ov5648 all set it on every platform), so it is not an IPU6-specific hack. > Bit 2 (bus idle in LP11) is set as well, matching ov5640's value; this > was tested only on IPU6. > > Link: https://github.com/linux-surface/linux-surface/pull/2171 > Co-developed-by: Arsalan Naeem <naeemarsalan@gmail.com> > Signed-off-by: Arsalan Naeem <naeemarsalan@gmail.com> > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > --- > drivers/media/i2c/ov5693.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c > index 02236f3db..d1b38348e 100644 > --- a/drivers/media/i2c/ov5693.c > +++ b/drivers/media/i2c/ov5693.c > @@ -35,6 +35,12 @@ > #define OV5693_STOP_STREAMING 0x00 > #define OV5693_SW_RESET 0x01 > > +/* MIPI transmitter control */ > +#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800) > +/* bit 5: gate the clock lane when idle; bit 2: keep the bus in LP11 when idle */ > +#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5) > +#define OV5693_MIPI_CTRL00_BUS_IDLE BIT(2) > + > #define OV5693_REG_CHIP_ID CCI_REG16(0x300a) > /* Yes, this is right. The datasheet for the OV5693 gives its ID as 0x5690 */ > #define OV5693_CHIP_ID 0x5690 > @@ -611,6 +617,20 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable) > { > int ret = 0; > > + /* > + * Gate the MIPI clock lane when idle. The power-on default of > + * MIPI_CTRL00 is 0x00 (free-running clock), which the IPU3 CSI-2 > + * receiver tolerates but the IPU6 one does not: it fails to lock onto > + * the link and capture times out. Gating the clock lane (bit 5) is > + * required for IPU6 and is the conventional setting for OmniVision > + * sensors (cf. ov5640, ov5647, ov5648); also keep the bus in LP11 when > + * idle (bit 2). Harmless on IPU3. > + */ > + if (enable) > + cci_write(ov5693->regmap, OV5693_MIPI_CTRL00_REG, > + OV5693_MIPI_CTRL00_CLOCK_LANE_GATE | > + OV5693_MIPI_CTRL00_BUS_IDLE, &ret); This configuration should in fact be dependent on "clock-noncontinuous" property; parsing the endpoint sets V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK flag when the property is present. Could you use that in v3? The interesting question is then why are other sensors working with IPU6 just fine without this? In general, the non-contiguous clock mode is something you enable explicitly and enabling it shouldn't make anything work but possibly you could save a little bit of power. It looks like drivers used with IPUs generally don't even support non-continuous clock mode. Ov2740 (on Thinkpads with IPU6) has similar configuration but there the same register is written 0, i.e. free running clock lane). Different IPU6 variants do have different PHYs though, so this could be the reason here. The ipu-bridge needs to set the property, too. I think this needs to be enabled only for the IPU6 variant in question (I'd just check the PCI ID; those are found in include/media/ipu6-pci-table.h). > + > cci_write(ov5693->regmap, OV5693_SW_STREAM_REG, > enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING, > &ret); -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli ` (2 preceding siblings ...) 2026-07-17 13:20 ` [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 Fernando Rimoli @ 2026-07-20 16:38 ` Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 1/4] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli ` (3 more replies) 3 siblings, 4 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 16:38 UTC (permalink / raw) To: Sakari Ailus, Daniel Scally, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel, Fernando Rimoli This enables the OV5693 front camera on Intel IPU6 Surface devices such as the Microsoft Surface Pro 9. Four pieces are needed on a stock kernel: 1/4 ov5693: add the "OVTI5693" ACPI HID so the i2c core binds the driver (these devices enumerate the sensor as OVTI5693, not INT33BE). 2/4 ipu-bridge: add the same HID so the bridge builds the sensor's fwnode graph. 3/4 ov5693: gate the MIPI clock lane when the endpoint requests a non-continuous clock, which the IPU6 CSI-2 receiver needs to lock. 4/4 ipu-bridge: request the non-continuous clock for the ov5693, but only on IPU6 (matched against ipu6_pci_tbl). v3 reworks the clock-lane gate per Sakari's review of v2: instead of writing MIPI_CTRL00 unconditionally, the sensor now gates the clock lane only when the "clock-noncontinuous" endpoint property is present (the V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK flag), the same way ov5647 does, and the ipu-bridge supplies that property, gated on the IPU6 PCI IDs. The property is set only for the ov5693 HIDs and only when the bridge sits on a PCI device matching ipu6_pci_tbl, so IPU3 users of the same sensor and the non-PCI IVSC bridge path are left untouched. I scoped it to all ipu6_pci_tbl IDs since the gate is confirmed needed across two IPU6 generations (see below), happy to narrow it to specific IDs if you would prefer. On the register value: unlike ov5647, patch 3 sets bit 5 (clock-lane gate) and bit 2 (LP11 idle) but not bit 4 (line sync). I re-swept the register at runtime on my Surface Pro 9 (IPU6) to check bit 4 specifically: 0x24 (bit5+bit2) streams 300/300 frames at ~28 fps, but adding bit 4 (i.e. ov5647's 0x34) collapses the stream to a couple of frames, and bit 4 alone fails outright. A bit5-only value recovered to 300/300 in the same run, so this is a value effect. This matches your point that the IPU6 PHYs differ (ov2740 writes this register to 0 on Thinkpad IPU6), the ov5693 on IPU6 wants exactly bit5+bit2. Register read-back confirmed the power-on default is 0x00. Patches 1-2 are independent and useful on their own for enumeration; patch 1 carries Dan Scally's Reviewed-by from v1. Changes since v2 [1]: - Reworked patch 3: gate the MIPI clock lane on the "clock-noncontinuous" endpoint property (V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK) rather than writing MIPI_CTRL00 unconditionally, following ov5647. When the flag is absent the register is left at its reset default (IPU3 and others unaffected). - New patch 4: ipu-bridge sets "clock-noncontinuous" for the ov5693, gated on the IPU6 PCI IDs (ipu6_pci_tbl), skipping non-PCI bridge instances. - Characterised bit 4 (line sync) on IPU6 and documented why the value stays bit5+bit2, differing from ov5647. - Patch 3 was reworked but is functionally identical on tested hardware (the same register value is written whenever the bridge sets the property), so I kept Jakob Berg Jespersen's Tested-by; Jakob, please shout if you would rather re-test v3. The two IPU6 units the gate has been tested on: Surface Pro 9 (IPU6EP_ADLP, 0x465d) and, via Jakob, Surface Pro 7+ (IPU6, Tiger Lake, 0x9a19). [1] https://lore.kernel.org/linux-media/20260717132021.18034-1-fernandorimoli11@gmail.com/ Fernando Rimoli (4): media: i2c: ov5693: Add OVTI5693 ACPI HID media: ipu-bridge: Add OVTI5693 to the list of supported sensors media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 drivers/media/i2c/ov5693.c | 28 ++++++++++++++++++++++++ drivers/media/pci/intel/ipu-bridge.c | 32 ++++++++++++++++++++++++++++ include/media/ipu-bridge.h | 2 +- 3 files changed, 61 insertions(+), 1 deletion(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 1/4] media: i2c: ov5693: Add OVTI5693 ACPI HID 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli @ 2026-07-20 16:38 ` Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 16:38 UTC (permalink / raw) To: Sakari Ailus, Daniel Scally, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel, Fernando Rimoli The ov5693 driver only matches the "INT33BE" ACPI HID. Some Intel IPU6 Surface devices (e.g. Microsoft Surface Pro 9) enumerate the OV5693 front camera with the ACPI HID "OVTI5693" instead, so the i2c core never binds the driver. Add "OVTI5693" to the ACPI match table. Devices that use "INT33BE" (e.g. Surface Go 4) are unaffected. Tested on Surface Pro 9 (IPU6): the sensor enumerates as OVTI5693:00 (ACPI path \_SB_.PC00.I2C3.CAMF) and binds with this change. Link: https://github.com/linux-surface/linux-surface/pull/2171 Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> --- drivers/media/i2c/ov5693.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c index 4cc796bbe..02236f3db 100644 --- a/drivers/media/i2c/ov5693.c +++ b/drivers/media/i2c/ov5693.c @@ -1396,6 +1396,7 @@ static const struct dev_pm_ops ov5693_pm_ops = { static const struct acpi_device_id ov5693_acpi_match[] = { {"INT33BE"}, + {"OVTI5693"}, {}, }; MODULE_DEVICE_TABLE(acpi, ov5693_acpi_match); -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 1/4] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli @ 2026-07-20 16:38 ` Fernando Rimoli 2026-07-20 21:09 ` Dan Scally 2026-07-20 16:38 ` [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 Fernando Rimoli 3 siblings, 1 reply; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 16:38 UTC (permalink / raw) To: Sakari Ailus, Daniel Scally, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel, Fernando Rimoli The IPU bridge builds the firmware node graph only for sensors listed in ipu_supported_sensors[]. The OV5693 is currently listed only under its legacy "INT33BE" HID, so on Intel IPU6 Surface devices that enumerate it as "OVTI5693" (e.g. Microsoft Surface Pro 9) the bridge never wires up the sensor and the front camera is unusable. Add an "OVTI5693" entry. The link frequency (419200000) matches the existing INT33BE entry, as it is the same sensor. Tested on Surface Pro 9 (IPU6). Link: https://github.com/linux-surface/linux-surface/pull/2171 Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> --- drivers/media/pci/intel/ipu-bridge.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c index 88581a4c0..b70d61099 100644 --- a/drivers/media/pci/intel/ipu-bridge.c +++ b/drivers/media/pci/intel/ipu-bridge.c @@ -93,6 +93,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = { IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000), /* Omnivision OV5675 */ IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000), + /* Omnivision OV5693 */ + IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000), /* Omnivision OV8856 */ IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000), /* Sony IMX471 */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors 2026-07-20 16:38 ` [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli @ 2026-07-20 21:09 ` Dan Scally 0 siblings, 0 replies; 18+ messages in thread From: Dan Scally @ 2026-07-20 21:09 UTC (permalink / raw) To: Fernando Rimoli, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel Hi Fernando - thanks for the set On 20/07/2026 17:38, Fernando Rimoli wrote: > The IPU bridge builds the firmware node graph only for sensors listed in > ipu_supported_sensors[]. The OV5693 is currently listed only under its > legacy "INT33BE" HID, so on Intel IPU6 Surface devices that enumerate it > as "OVTI5693" (e.g. Microsoft Surface Pro 9) the bridge never wires up > the sensor and the front camera is unusable. > > Add an "OVTI5693" entry. The link frequency (419200000) matches the > existing INT33BE entry, as it is the same sensor. > > Tested on Surface Pro 9 (IPU6). > > Link: https://github.com/linux-surface/linux-surface/pull/2171 > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > --- Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > drivers/media/pci/intel/ipu-bridge.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index 88581a4c0..b70d61099 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c > @@ -93,6 +93,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = { > IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000), > /* Omnivision OV5675 */ > IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000), > + /* Omnivision OV5693 */ > + IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000), > /* Omnivision OV8856 */ > IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000), > /* Sony IMX471 */ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 1/4] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli @ 2026-07-20 16:38 ` Fernando Rimoli 2026-07-20 21:49 ` Dan Scally 2026-07-20 16:38 ` [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 Fernando Rimoli 3 siblings, 1 reply; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 16:38 UTC (permalink / raw) To: Sakari Ailus, Daniel Scally, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel, Fernando Rimoli The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00 power-on default (free-running MIPI clock). The IPU3 CSI-2 receiver tolerates this, but the IPU6 receiver (e.g. on Microsoft Surface Pro 8/9 and Surface Go 4) fails to lock onto the link, so the sensor streams but capture times out with "stream stop time out" and no frames arrive. Parse the "clock-noncontinuous" endpoint property (which sets the V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK flag) and, when it is present, gate the clock lane while idle (bit 5) and keep the bus in LP11 (bit 2) at stream on, matching the approach ov5647 already uses for the same register. When the flag is absent the register is left at its reset default, so IPU3 and other users are unaffected. Gating the clock lane was determined to be necessary and sufficient by sweeping the register at runtime on a Surface Pro 9 (IPU6): every value with bit 5 set streams (300/300 frames, steady 28 fps), every value with bit 5 clear fails with the CSI-2 timeout; register read-back confirmed the power-on default is 0x00. Bit 2 (bus idle in LP11) is set as well, matching ov5640's value. Note that unlike ov5647 the line-sync bit (bit 4) is deliberately not set: adding it collapses the IPU6 stream to a couple of frames, so this value differs from ov5647's - the IPU6 D-PHY behaves differently, consistent with these settings being PHY-specific. The "clock-noncontinuous" property is supplied by the ipu-bridge for the affected IPU6 variants in a subsequent patch. Link: https://github.com/linux-surface/linux-surface/pull/2171 Co-developed-by: Arsalan Naeem <naeemarsalan@gmail.com> Signed-off-by: Arsalan Naeem <naeemarsalan@gmail.com> Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> Tested-by: Jakob Berg Jespersen <dev@berg.pm> --- drivers/media/i2c/ov5693.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c index 02236f3db..5468e89d6 100644 --- a/drivers/media/i2c/ov5693.c +++ b/drivers/media/i2c/ov5693.c @@ -35,6 +35,14 @@ #define OV5693_STOP_STREAMING 0x00 #define OV5693_SW_RESET 0x01 +/* MIPI transmitter control */ +#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800) +/* bit 5: gate the clock lane when idle; bit 2: keep the bus in LP11 when idle */ +#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5) +#define OV5693_MIPI_CTRL00_BUS_IDLE BIT(2) +#define OV5693_MIPI_CTRL00_NCONT_CLOCK (OV5693_MIPI_CTRL00_CLOCK_LANE_GATE | \ + OV5693_MIPI_CTRL00_BUS_IDLE) + #define OV5693_REG_CHIP_ID CCI_REG16(0x300a) /* Yes, this is right. The datasheet for the OV5693 gives its ID as 0x5690 */ #define OV5693_CHIP_ID 0x5690 @@ -144,6 +152,9 @@ struct ov5693_device { struct regulator_bulk_data supplies[OV5693_NUM_SUPPLIES]; struct clk *xvclk; + /* Gate the MIPI clock lane when idle (CSI-2 non-continuous clock) */ + bool clock_ncont; + struct ov5693_mode { struct v4l2_rect crop; struct v4l2_mbus_framefmt format; @@ -611,6 +622,19 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable) { int ret = 0; + /* + * When the CSI-2 link is configured for a non-continuous clock, gate + * the MIPI clock lane while idle (bit 5) and keep the bus in LP11 + * (bit 2). The power-on default of MIPI_CTRL00 is 0x00 (free-running + * clock): the IPU3 CSI-2 receiver tolerates that, but the IPU6 one + * fails to lock onto the link and capture times out. Only touch the + * register when the endpoint requests a non-continuous clock, leaving + * the reset default in place otherwise. + */ + if (enable && ov5693->clock_ncont) + cci_write(ov5693->regmap, OV5693_MIPI_CTRL00_REG, + OV5693_MIPI_CTRL00_NCONT_CLOCK, &ret); + cci_write(ov5693->regmap, OV5693_SW_STREAM_REG, enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING, &ret); @@ -1259,6 +1283,9 @@ static int ov5693_check_hwcfg(struct ov5693_device *ov5693) goto out_free_bus_cfg; } + ov5693->clock_ncont = bus_cfg.bus.mipi_csi2.flags & + V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; + out_free_bus_cfg: v4l2_fwnode_endpoint_free(&bus_cfg); -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock 2026-07-20 16:38 ` [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock Fernando Rimoli @ 2026-07-20 21:49 ` Dan Scally 0 siblings, 0 replies; 18+ messages in thread From: Dan Scally @ 2026-07-20 21:49 UTC (permalink / raw) To: Fernando Rimoli, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel Hi Fernando, thanks for the patch On 20/07/2026 17:38, Fernando Rimoli wrote: > The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00 > power-on default (free-running MIPI clock). The IPU3 CSI-2 receiver > tolerates this, but the IPU6 receiver (e.g. on Microsoft Surface Pro 8/9 > and Surface Go 4) fails to lock onto the link, so the sensor streams but > capture times out with "stream stop time out" and no frames arrive. > > Parse the "clock-noncontinuous" endpoint property (which sets the > V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK flag) and, when it is present, gate > the clock lane while idle (bit 5) and keep the bus in LP11 (bit 2) at > stream on, matching the approach ov5647 already uses for the same > register. When the flag is absent the register is left at its reset > default, so IPU3 and other users are unaffected. > > Gating the clock lane was determined to be necessary and sufficient by > sweeping the register at runtime on a Surface Pro 9 (IPU6): every value > with bit 5 set streams (300/300 frames, steady 28 fps), every value with > bit 5 clear fails with the CSI-2 timeout; register read-back confirmed > the power-on default is 0x00. Bit 2 (bus idle in LP11) is set as well, > matching ov5640's value. Note that unlike ov5647 the line-sync bit > (bit 4) is deliberately not set: adding it collapses the IPU6 stream to > a couple of frames, so this value differs from ov5647's - the IPU6 D-PHY > behaves differently, consistent with these settings being PHY-specific. > > The "clock-noncontinuous" property is supplied by the ipu-bridge for the > affected IPU6 variants in a subsequent patch. > This one looks good to me: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > Link: https://github.com/linux-surface/linux-surface/pull/2171 > Co-developed-by: Arsalan Naeem <naeemarsalan@gmail.com> > Signed-off-by: Arsalan Naeem <naeemarsalan@gmail.com> > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > Tested-by: Jakob Berg Jespersen <dev@berg.pm> > --- > drivers/media/i2c/ov5693.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c > index 02236f3db..5468e89d6 100644 > --- a/drivers/media/i2c/ov5693.c > +++ b/drivers/media/i2c/ov5693.c > @@ -35,6 +35,14 @@ > #define OV5693_STOP_STREAMING 0x00 > #define OV5693_SW_RESET 0x01 > > +/* MIPI transmitter control */ > +#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800) > +/* bit 5: gate the clock lane when idle; bit 2: keep the bus in LP11 when idle */ > +#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5) > +#define OV5693_MIPI_CTRL00_BUS_IDLE BIT(2) > +#define OV5693_MIPI_CTRL00_NCONT_CLOCK (OV5693_MIPI_CTRL00_CLOCK_LANE_GATE | \ > + OV5693_MIPI_CTRL00_BUS_IDLE) > + > #define OV5693_REG_CHIP_ID CCI_REG16(0x300a) > /* Yes, this is right. The datasheet for the OV5693 gives its ID as 0x5690 */ > #define OV5693_CHIP_ID 0x5690 > @@ -144,6 +152,9 @@ struct ov5693_device { > struct regulator_bulk_data supplies[OV5693_NUM_SUPPLIES]; > struct clk *xvclk; > > + /* Gate the MIPI clock lane when idle (CSI-2 non-continuous clock) */ > + bool clock_ncont; > + > struct ov5693_mode { > struct v4l2_rect crop; > struct v4l2_mbus_framefmt format; > @@ -611,6 +622,19 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable) > { > int ret = 0; > > + /* > + * When the CSI-2 link is configured for a non-continuous clock, gate > + * the MIPI clock lane while idle (bit 5) and keep the bus in LP11 > + * (bit 2). The power-on default of MIPI_CTRL00 is 0x00 (free-running > + * clock): the IPU3 CSI-2 receiver tolerates that, but the IPU6 one > + * fails to lock onto the link and capture times out. Only touch the > + * register when the endpoint requests a non-continuous clock, leaving > + * the reset default in place otherwise. > + */ > + if (enable && ov5693->clock_ncont) > + cci_write(ov5693->regmap, OV5693_MIPI_CTRL00_REG, > + OV5693_MIPI_CTRL00_NCONT_CLOCK, &ret); > + > cci_write(ov5693->regmap, OV5693_SW_STREAM_REG, > enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING, > &ret); > @@ -1259,6 +1283,9 @@ static int ov5693_check_hwcfg(struct ov5693_device *ov5693) > goto out_free_bus_cfg; > } > > + ov5693->clock_ncont = bus_cfg.bus.mipi_csi2.flags & > + V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; > + > out_free_bus_cfg: > v4l2_fwnode_endpoint_free(&bus_cfg); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli ` (2 preceding siblings ...) 2026-07-20 16:38 ` [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock Fernando Rimoli @ 2026-07-20 16:38 ` Fernando Rimoli 2026-07-20 21:56 ` Dan Scally 3 siblings, 1 reply; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 16:38 UTC (permalink / raw) To: Sakari Ailus, Daniel Scally, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel, Fernando Rimoli The ov5693 (INT33BE / OVTI5693) needs its MIPI clock lane gated while idle for the IPU6 CSI-2 receiver to lock onto the link; otherwise the sensor streams but capture times out and no frames arrive. The sensor driver gates the clock lane when the endpoint requests a non-continuous clock, so the bridge has to supply that property. The same sensor works with a free-running clock on the IPU3, and other sensors are unaffected, so add the "clock-noncontinuous" endpoint property only for the ov5693 and only when the bridge sits on an IPU6 (matched against ipu6_pci_tbl). Non-PCI bridge instances (e.g. the IVSC mei_csi path) are skipped. Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> --- drivers/media/pci/intel/ipu-bridge.c | 30 ++++++++++++++++++++++++++++ include/media/ipu-bridge.h | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c index b70d61099..98773a99a 100644 --- a/drivers/media/pci/intel/ipu-bridge.c +++ b/drivers/media/pci/intel/ipu-bridge.c @@ -8,6 +8,7 @@ #include <linux/dmi.h> #include <linux/i2c.h> #include <linux/mei_cl_bus.h> +#include <linux/pci.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/property.h> @@ -15,6 +16,7 @@ #include <linux/workqueue.h> #include <media/ipu-bridge.h> +#include <media/ipu6-pci-table.h> #include <media/v4l2-fwnode.h> #define ADEV_DEV(adev) ACPI_PTR(&((adev)->dev)) @@ -383,6 +385,25 @@ int ipu_bridge_parse_ssdb(struct acpi_device *adev, struct ipu_sensor *sensor) } EXPORT_SYMBOL_NS_GPL(ipu_bridge_parse_ssdb, "INTEL_IPU_BRIDGE"); +/* + * The ov5693 (INT33BE / OVTI5693, on Microsoft Surface devices) only locks + * onto the IPU6 CSI-2 receiver when its clock lane is gated while idle, which + * the sensor driver does when the endpoint requests a non-continuous clock. + * The same sensor works with a free-running clock on the IPU3, so only request + * the non-continuous clock on the IPU6 variants, keyed off the bridge's PCI ID. + */ +static bool ipu_bridge_needs_ncont_clock(struct ipu_bridge *bridge, + const struct ipu_sensor_config *cfg) +{ + if (strcmp(cfg->hid, "INT33BE") && strcmp(cfg->hid, "OVTI5693")) + return false; + + if (!dev_is_pci(bridge->dev)) + return false; + + return !!pci_match_id(ipu6_pci_tbl, to_pci_dev(bridge->dev)); +} + static void ipu_bridge_create_fwnode_properties( struct ipu_sensor *sensor, struct ipu_bridge *bridge, @@ -463,6 +484,15 @@ static void ipu_bridge_create_fwnode_properties( cfg->link_freqs, cfg->nr_link_freqs); + /* + * This must stay after the link-frequencies above: the property array + * is NULL-terminated, so the entry only takes effect when a preceding + * slot is populated. All IPU6 ov5693 configs define a link frequency. + */ + if (ipu_bridge_needs_ncont_clock(bridge, cfg)) + sensor->ep_properties[4] = + PROPERTY_ENTRY_BOOL("clock-noncontinuous"); + sensor->ipu_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN( sensor->prop_names.data_lanes, bridge->data_lanes, sensor->lanes); diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h index 16fac7654..48eb1e270 100644 --- a/include/media/ipu-bridge.h +++ b/include/media/ipu-bridge.h @@ -141,7 +141,7 @@ struct ipu_sensor { const char *vcm_type; struct ipu_property_names prop_names; - struct property_entry ep_properties[5]; + struct property_entry ep_properties[6]; struct property_entry dev_properties[5]; struct property_entry ipu_properties[3]; struct property_entry ivsc_properties[1]; -- 2.43.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 2026-07-20 16:38 ` [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 Fernando Rimoli @ 2026-07-20 21:56 ` Dan Scally 2026-07-20 23:50 ` Fernando Rimoli 0 siblings, 1 reply; 18+ messages in thread From: Dan Scally @ 2026-07-20 21:56 UTC (permalink / raw) To: Fernando Rimoli, Sakari Ailus, linux-media Cc: Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel Hi Fernando On 20/07/2026 17:38, Fernando Rimoli wrote: > The ov5693 (INT33BE / OVTI5693) needs its MIPI clock lane gated while > idle for the IPU6 CSI-2 receiver to lock onto the link; otherwise the > sensor streams but capture times out and no frames arrive. The sensor > driver gates the clock lane when the endpoint requests a non-continuous > clock, so the bridge has to supply that property. > > The same sensor works with a free-running clock on the IPU3, and other > sensors are unaffected, so add the "clock-noncontinuous" endpoint > property only for the ov5693 and only when the bridge sits on an IPU6 > (matched against ipu6_pci_tbl). Non-PCI bridge instances (e.g. the IVSC > mei_csi path) are skipped. Hm, I think this is functionally fine, but matching on PCI ID and sensor does seem a bit quirky...do you know if the IPU3 case is fine with the clock-noncontinuous flag too? If not I can test it tomorrow. Thanks Dan > > Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com> > --- > drivers/media/pci/intel/ipu-bridge.c | 30 ++++++++++++++++++++++++++++ > include/media/ipu-bridge.h | 2 +- > 2 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index b70d61099..98773a99a 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c > @@ -8,6 +8,7 @@ > #include <linux/dmi.h> > #include <linux/i2c.h> > #include <linux/mei_cl_bus.h> > +#include <linux/pci.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/property.h> > @@ -15,6 +16,7 @@ > #include <linux/workqueue.h> > > #include <media/ipu-bridge.h> > +#include <media/ipu6-pci-table.h> > #include <media/v4l2-fwnode.h> > > #define ADEV_DEV(adev) ACPI_PTR(&((adev)->dev)) > @@ -383,6 +385,25 @@ int ipu_bridge_parse_ssdb(struct acpi_device *adev, struct ipu_sensor *sensor) > } > EXPORT_SYMBOL_NS_GPL(ipu_bridge_parse_ssdb, "INTEL_IPU_BRIDGE"); > > +/* > + * The ov5693 (INT33BE / OVTI5693, on Microsoft Surface devices) only locks > + * onto the IPU6 CSI-2 receiver when its clock lane is gated while idle, which > + * the sensor driver does when the endpoint requests a non-continuous clock. > + * The same sensor works with a free-running clock on the IPU3, so only request > + * the non-continuous clock on the IPU6 variants, keyed off the bridge's PCI ID. > + */ > +static bool ipu_bridge_needs_ncont_clock(struct ipu_bridge *bridge, > + const struct ipu_sensor_config *cfg) > +{ > + if (strcmp(cfg->hid, "INT33BE") && strcmp(cfg->hid, "OVTI5693")) > + return false; > + > + if (!dev_is_pci(bridge->dev)) > + return false; > + > + return !!pci_match_id(ipu6_pci_tbl, to_pci_dev(bridge->dev)); > +} > + > static void ipu_bridge_create_fwnode_properties( > struct ipu_sensor *sensor, > struct ipu_bridge *bridge, > @@ -463,6 +484,15 @@ static void ipu_bridge_create_fwnode_properties( > cfg->link_freqs, > cfg->nr_link_freqs); > > + /* > + * This must stay after the link-frequencies above: the property array > + * is NULL-terminated, so the entry only takes effect when a preceding > + * slot is populated. All IPU6 ov5693 configs define a link frequency. > + */ > + if (ipu_bridge_needs_ncont_clock(bridge, cfg)) > + sensor->ep_properties[4] = > + PROPERTY_ENTRY_BOOL("clock-noncontinuous"); > + > sensor->ipu_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN( > sensor->prop_names.data_lanes, > bridge->data_lanes, sensor->lanes); > diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h > index 16fac7654..48eb1e270 100644 > --- a/include/media/ipu-bridge.h > +++ b/include/media/ipu-bridge.h > @@ -141,7 +141,7 @@ struct ipu_sensor { > const char *vcm_type; > > struct ipu_property_names prop_names; > - struct property_entry ep_properties[5]; > + struct property_entry ep_properties[6]; > struct property_entry dev_properties[5]; > struct property_entry ipu_properties[3]; > struct property_entry ivsc_properties[1]; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 2026-07-20 21:56 ` Dan Scally @ 2026-07-20 23:50 ` Fernando Rimoli 0 siblings, 0 replies; 18+ messages in thread From: Fernando Rimoli @ 2026-07-20 23:50 UTC (permalink / raw) To: Dan Scally, Sakari Ailus, linux-media Cc: Fernando Rimoli, Mauro Carvalho Chehab, Arsalan Naeem, Jakob Berg Jespersen, linux-kernel Hi Dan, Thanks for the reviews on 1-3. You're right that keying on both the PCI ID and the sensor is a bit awkward. My reasoning for scoping it that tightly was caution rather than a known IPU3 failure: I only have IPU6 hardware (Surface Pro 9), so I couldn't confirm that gating the ov5693's clock lane is safe on the IPU3 CSI-2 receiver, and I didn't want to risk regressing the existing cio2 + ov5693 users (the INT33BE Surface Pro/Book devices) that work today with the free-running default. For what it's worth, from the receiver side IPU3 looks agnostic to the flag: ipu3-cio2 only consumes bus.mipi_csi2.num_data_lanes from the parsed endpoint and programs its D-PHY Rx timing (clk_termen/clk_settle) the same way regardless of V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK, it never looks at that flag. So the open question is purely sensor-side: whether the ov5693 idling its clock lane in LP11 (bit 5) upsets the cio2 D-PHY's lock. I can't answer that without IPU3 hardware. If your test tomorrow shows cio2 + ov5693 still streams fine with clock-noncontinuous set, I'm happy to drop the ipu6_pci_tbl check entirely and just request the property for the ov5693 unconditionally in v4 which removes the PCI quirk and is much cleaner. (The sensor-driver side already no-ops when the flag is absent, so nothing else needs to change.) If it turns out IPU3 doesn't like it, then the PCI gate is doing real work and I'd keep it, but I can add a comment making that rationale explicit. Either way I'll respin once we know. Thanks a lot for offering to test on IPU3, that's the one platform I can't cover. Thanks, Fernando ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-07-20 23:50 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-08 21:36 [PATCH] media: ov5693: add OVTI5693 ACPI HID for IPU6 Surface devices Fernando Rimoli 2026-07-09 13:17 ` Dan Scally 2026-07-14 9:32 ` Sakari Ailus 2026-07-17 13:20 ` [PATCH v2 0/3] media: Enable the OV5693 front camera on " Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 1/3] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 2/3] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli 2026-07-17 13:20 ` [PATCH v2 3/3] media: i2c: ov5693: Gate the MIPI clock lane for IPU6 Fernando Rimoli 2026-07-19 16:25 ` Jakob Berg Jespersen 2026-07-19 22:42 ` Sakari Ailus 2026-07-20 16:38 ` [PATCH v3 0/4] media: Enable the OV5693 front camera on IPU6 Surface devices Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 1/4] media: i2c: ov5693: Add OVTI5693 ACPI HID Fernando Rimoli 2026-07-20 16:38 ` [PATCH v3 2/4] media: ipu-bridge: Add OVTI5693 to the list of supported sensors Fernando Rimoli 2026-07-20 21:09 ` Dan Scally 2026-07-20 16:38 ` [PATCH v3 3/4] media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock Fernando Rimoli 2026-07-20 21:49 ` Dan Scally 2026-07-20 16:38 ` [PATCH v3 4/4] media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 Fernando Rimoli 2026-07-20 21:56 ` Dan Scally 2026-07-20 23:50 ` Fernando Rimoli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox