drivers/media/i2c/ov5693.c | 25 ++++++++++ drivers/media/pci/intel/ipu-bridge.c | 73 +++++++++++++++++++++++----- include/media/ipu-bridge.h | 51 ++++++++++++++++--- 3 files changed, 130 insertions(+), 19 deletions(-)
This enables the OV5693 front camera on Intel IPU6 Surface devices such
as the Microsoft Surface Pro 9. Six patches on a stock kernel:
1/6 ov5693: add the "OVTI5693" ACPI HID so the i2c core binds the
driver (these devices enumerate the sensor as OVTI5693, not
INT33BE).
2/6 ipu-bridge: add the same HID so the bridge builds the sensor's
fwnode graph.
3/6 ov5693: gate the MIPI clock lane when the endpoint requests a
non-continuous clock, which the IPU6 CSI-2 receiver needs to
lock.
4/6 ipu-bridge: assign endpoint property indices dynamically.
5/6 ipu-bridge: add per-IPU sensor config matching and config flags.
6/6 ipu-bridge: use those to request the non-continuous clock for
the ov5693 on the IPU6 variants that need it.
v4 restructures the bridge side per Sakari's review of v3. Instead of
the ad-hoc "is this the ov5693 on an IPU6" helper v3 had, struct
ipu_sensor_config gains an optional IPU PCI product ID and a flags
field, so a sensor that needs different treatment on different IPUs is
described by the table rather than by code. Patch 5 adds that mechanism
and patch 6 is its first user. Patch 4 is the prerequisite Sakari asked
for: endpoint property indices are now assigned through a running index
in the style of NEXT_PROPERTY() in mipi-disco-img.c, so a conditional
property no longer depends on the sensor having a link frequency.
Patch 5 introduces one semantic I would like checked: where both a
PCI-specific and a generic entry exist for the same HID, the specific one
wins and the generic one is skipped, so the sensor is connected once
rather than twice. Doing it the other way round would consume two of the
four IPU ports for one sensor.
On the register value: patch 3 now sets only bit 5 (clock-lane gate).
Sakari asked whether IPU6 worked without bit 2. It does, so v4 drops it.
For the record, sweeping MIPI_CTRL00 at runtime on a Surface Pro 9
(IPU6EP_ADLP, sensor as OVTI5693), 3 trials of 30 frames plus a
300-frame stability run per value:
0x20 bit5 300/300 frames, steady 28.6 fps
0x24 bit5+bit2 300/300 frames, steady 28.6 fps
0x2d vendor value 300/300 frames, steady 28.6 fps
0x00 power-on default 0 frames, "stream stop time out"
0x04 bit2 alone 0 frames, "stream stop time out"
0x10 bit4 alone 0 frames, "stream stop time out"
0x30 bit5+bit4 2 frames, stream collapses
0x34 bit5+bit4+bit2 2 frames, stream collapses
So bit 5 is necessary and sufficient, bit 2 makes no difference, and bit 4
actively breaks the link. That last one is why this differs from ov5647's
value for the same register even though the mechanism is copied from it. A
bit5-only value recovered to 300/300 in the same run after the bit 4
failures, so those are genuine value effects and not a wedged link.
Register read-back confirmed the power-on default is 0x00.
This was reproduced independently on a Surface Pro 8 (IPU6 0x9a19,
sensor as INT33BE) by a linux-surface user [3], 30 frames per value,
every value read back after writing:
0x2d vendor value (control) 30 frames, 28.65 fps
0x24 bit5+bit2 30 frames, 28.65 fps
0x20 bit5 30 frames, 28.65 fps
0x0d 0x2d with bit 5 clear 0 frames
0x08 bit3 alone 0 frames
0x04 bit2 alone 0 frames
0x01 bit0 alone 0 frames
0x00 power-on default 0 frames
The 0x0d row is the one I could not produce myself: everything in the
vendor value except bit 5 fails to stream, so bit 5 is necessary and not
just sufficient. That second set has two limits. It was produced by poking
the register over I2C into a stalled capture rather than by running this
patch, and the write lands after stream on rather than before. So it
confirms the value on a second device and IPU generation, but says nothing
about the plumbing. The patch itself is tested on the Pro 9.
Patch 3 sets the bit with cci_update_bits() rather than writing the whole
register. The ov5693 does not otherwise program MIPI_CTRL00, and this
driver also serves IPU3/CIO2 and Rockchip platforms, so modifying one bit
leaves anything the platform put there intact. On the devices here the
register reads 0x00 before the write, so in practice the two forms are
equivalent. I went with the narrower one because it claims less.
The write is issued at each stream on rather than once per power-on.
That placement seems to matter in practice: in the same thread [4],
setups that write MIPI_CTRL00 once from userspace lose the binned
1296x972 mode, which reprograms the sensor, while setups that write it
from the driver at every stream on keep it.
Patch 6 sets the flag for two IPU6 product IDs, the ones the gate has
been exercised on: IPU6 (Tiger Lake, 0x9a19) and IPU6EP_ADLP (Alder
Lake-P, 0x465d), for both HIDs, four entries in total. The combinations
covered by hardware are OVTI5693 on ADL-P (this series, Surface Pro 9)
and INT33BE on Tiger Lake (the register value, Surface Pro 8 [3] and
Pro 7+). v3 matched all of ipu6_pci_tbl; with the IDs now spelled out
in the table I would rather list only what is confirmed and add the
others as reports come in. Surface Go 4 (ADL-N, 0x462e) is the likely
next one.
Patches 1-2 are independent and useful on their own for enumeration,
and both carry Dan Scally's Reviewed-by (patch 1 from v1, patch 2 from
v3).
Dan, Jakob: patch 3 changed since you reviewed and tested it. 0x24
became 0x20 and the write became a read-modify-write, so I dropped Dan's
Reviewed-by and Jakob's Tested-by rather than carry them across a
behaviour change. Happy to add both back if you are still content with
the narrower write.
Dan, on your v3 4/4 question about matching on PCI ID and sensor being
quirky: agreed, and patches 5-6 replace that with per-IPU entries in
the sensor config table instead. That also settles the IPU3 question you
offered to test, since IPU3 keeps matching the generic entry and never
sees the flag. No need spend hardware time on it on my account.
Two checkpatch complaints in this series are deliberate, in case they
show up in a report: patch 4 trips "CHECK: Macro argument reuse
'index'", which upstream's NEXT_PROPERTY() trips identically and is
inherent to the pattern; and patch 5 trips "ERROR: Macros with complex
values should be enclosed in parentheses", which the existing
ipu-bridge.h macros already trip five times over. Parenthesising a
compound literal would break the designated initialisers.
Changes since v3 [1]:
- Patch 3: write only bit 5, dropping bit 2 (not needed on IPU6);
OV5693_MIPI_CTRL00_BUS_IDLE is gone rather than renamed to _LP11.
Set the bit with cci_update_bits() instead of writing the whole
register. Moved the register-sweep evidence from the commit message
to this cover letter, and added an independent sweep on a second
device and IPU generation. Dropped Dan's Reviewed-by and Jakob's
Tested-by (the patch changed behaviour since both were given).
- New patch 4: assign endpoint property indices dynamically.
- New patch 5: PCI ID + flags in struct ipu_sensor_config, with
IPU_SENSOR_CONFIG_MATCH_FL() and most-specific-entry-wins matching.
- Patch 6 rewritten on top of patch 5: the HID-strcmp plus
pci_match_id(ipu6_pci_tbl) helper is gone, replaced by table
entries. Scoped to the two tested IPU6 product IDs instead of all of
ipu6_pci_tbl.
- Rebased onto v7.3-rc1.
Changes since v2 [2]:
- 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 alone (IPU3 and others unaffected).
- Added the ipu-bridge side that supplies the property.
Tested on a Surface Pro 9 (IPU6EP_ADLP): with the series applied the
front camera streams 300/300 frames at 28.6 fps. That testing predates
the switch to cci_update_bits(). The register reads 0x00 before the write
on this hardware, so both forms resolve to the same value and I have not
re-measured it. Each patch builds without warnings on its own, at W=1 as
well, so the series is bisectable.
[1] https://lore.kernel.org/linux-media/20260720163819.104130-1-fernandorimoli11@gmail.com/
[2] https://lore.kernel.org/linux-media/20260717132021.18034-1-fernandorimoli11@gmail.com/
[3] https://github.com/linux-surface/linux-surface/pull/2171#issuecomment-5298352360
[4] https://github.com/linux-surface/linux-surface/pull/2171#issuecomment-5372616619
Fernando Rimoli (6):
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: Assign endpoint property indices dynamically
media: ipu-bridge: Match sensor configs per IPU and add config flags
media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6
drivers/media/i2c/ov5693.c | 25 ++++++++++
drivers/media/pci/intel/ipu-bridge.c | 73 +++++++++++++++++++++++-----
include/media/ipu-bridge.h | 51 ++++++++++++++++---
3 files changed, 130 insertions(+), 19 deletions(-)
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
--
2.43.0
The ov5693 front camera does not stream on IPU6 Surface devices. The sensor needs its MIPI clock lane gated while idle, which needs a driver change and a way for the ipu-bridge to ask for it per IPU. v4 is at [1]. This addresses Sakari's review of patches 3, 4 and 5, adds the DT bindings property he asked for, and adds an Alder Lake-N entry now that a Surface Go 4 has been measured. Changes since v4: - New patch 3 adds "clock-noncontinuous" to the ov5693 DT bindings. The series is now seven patches; v4's patches 3 to 6 are 4 to 7 here, and the Tested-by tags are scoped to those four. - Commit messages cut throughout, per your comments on v4. Patch 5 now carries your wording and nothing else. Patch 4 lost the ov5647 comparison and the per-machine failure detail, which is below instead. Patches 6 and 7 trimmed. - Patch 5: the running index starts at 0, each call site passes its own property's maximum index rather than a shared one, and the macro is IPU_BRIDGE_NEXT_PROPERTY with the enum renamed to match, following mipi-disco-img.c more closely. - Patch 6: struct ipu_sensor_config now carries a zero-terminated list of IPU PCI product IDs rather than a single ID, so one entry covers several IPUs. The flags macro argument is a plain value, so flags can be combined. The scan of the whole table is gone: entries for one HID are adjacent with the IPU-specific ones first, and the generic entry is skipped once a specific one has matched. - Patch 7: the four entries collapse to one shared ID list, with Alder Lake-N added. On the enum naming in patch 5: "s/SENSOR_EP/BRIDGE/" taken literally gives IPU_BRIDGE_BUS_TYPE, but since the macro is not endpoint-specific and may later serve dev_properties, I kept the group marker as IPU_BRIDGE_EP_*, which is also how mipi-disco-img.c separates its EP and DEV entries. Say the word if you meant the shorter form. Unpatched behaviour, which patch 4 now only summarises: Four people measured it on five machines and it is not uniform. It is absolute on three (a Pro 9, a Pro 7+ and a Go 4). On another Pro 7+ it is intermittent, 3 of 20 opens delivering frames inside a single boot, with the successes at opens 1, 7 and 14 and the sensor runtime-suspending in between. On a Pro 8 the first capture after a cold boot succeeds and every later one in that boot hangs. No mechanism explains the spread: it is not once per boot, given where that machine's successes fell, and it is not Tiger Lake versus Alder Lake, since two Tiger Lake machines fall on opposite sides. The sensor is up during the failures on every machine checked: the privacy LED lights, the failing runs log no CSI-2 errors where the working ones log ten or more, and setting bit 5 over i2c on an already stalled stream starts delivery immediately. **If you verify this, capture twice.** On one of the five machines a single capture immediately after a cold boot succeeds without the flagged entry, so the obvious check gives a false negative. Both testers who hit this asked for it to be written down. Two other things worth knowing: - The teardown "stream stop time out" appears identically with and without this series, so it is pre-existing and not introduced here. - checkpatch flags "Macros with complex values should be enclosed in parentheses" on patch 6. The pristine header already trips it five times and the patched header still trips it exactly five times, because IPU_SENSOR_CONFIG becomes a wrapper as IPU_SENSOR_CONFIG_MATCH_FL takes over the compound literal. No net new instance. Testing coverage, all scoped to patches 4 to 7. Patches 1 and 2 are not exercised by the testers, whose machines enumerate INT33BE; the Pro 9 is the OVTI5693 machine. Tiger Lake 0x9a19 Pro 7+ (Jakob), Pro 8 (Fil Dunsky), Pro 7+ (Lucas Lis) Alder Lake-P 0x465d Pro 9 (mine) Alder Lake-N 0x462e Go 4 (Kengo Oki), 0/300 without the entry, 300/300 with Kengo Oki's Alder Lake-N result is what justifies the third ID in patch 7, and his tag is given for the version that carries it. The reworked bridge in patches 5 to 7 was also run on the Pro 9 with all three of its sensors: three cameras connected once each with no double connect, link frequencies unchanged, and the front camera streaming at 28.63 fps, which is only possible if the flag reaches the sensor driver through the new per-IPU match. The Pro 9 measurement of 300 consecutive opens predates the switch from a full write to cci_update_bits(), but the register reads 0x00 first there, so it resolves to the same write. [1] https://lore.kernel.org/linux-media/20260831181858.325109-1-fernandorimoli11@gmail.com/ Fernando Rimoli (7): media: i2c: ov5693: Add OVTI5693 ACPI HID media: ipu-bridge: Add OVTI5693 to the list of supported sensors dt-bindings: media: ov5693: Add clock-noncontinuous media: i2c: ov5693: Gate the MIPI clock lane for non-continuous clock media: ipu-bridge: Assign endpoint property indices dynamically media: ipu-bridge: Match sensor configs per IPU and add config flags media: ipu-bridge: Request non-continuous clock for ov5693 on IPU6 .../bindings/media/i2c/ovti,ov5693.yaml | 2 + drivers/media/i2c/ov5693.c | 25 ++++++ drivers/media/pci/intel/ipu-bridge.c | 83 ++++++++++++++++--- include/media/ipu-bridge.h | 50 +++++++++-- 4 files changed, 141 insertions(+), 19 deletions(-) base-commit: cee9395acd8043be0644b25c34bfa86623f2b935 -- 2.43.0
On Wed, 2 Sep 2026, Fernando Rimoli wrote: > This series enables the OV5693 front camera on IPU6 Surface devices For patches 4-7: Tested-by: D. Manresa <dmanresa@gmail.com> # Surface Pro 7+, IPU6 TGL 8086:9a19, INT33BE; 4-7 backported to a 6.19 linux-surface tree Coverage that was still missing from the other reports, all on the same machine and boot: - With the downstream unconditional 0x4800 = 0x2d write removed and only your gate in place, MIPI_CTRL00 reads 0x20 over i2c mid-stream (bit 5 only), so the result below is your code path and nothing else. - The sensor's 2x2 binned 1296x972 readout (a downstream mode, not in mainline) streams through the IPU6 hardware ISP (PSYS) at ~28.7 fps: three fresh stream starts x 60 frames, all frames unique and non-black, Bayer phase unchanged. A raw binned capture straight off the ISYS node had zero CSI-2 errors. - The CSI-2 receiver error signature at stream start/stop (the usual "Transfer FIFO overflow" / "Inter-frame packet discarded" bursts) is identical to the 0x2d build over the same five capture cycles: 15/15/20 in both. Consistent with Fil's sweep - bits 0, 2 and 3 of 0x2d were doing nothing. - Exercised on top of my ipu-bridge rebind series, with the "clock-noncontinuous" property name routed through ipu_property_names as suggested in my reply to 7/7: PCI remove -> module unload -> rescan -> reload re-enumerates all three sensors without -EEXIST and captures afterwards. The IR (OV7251) and rear (OV8865) cameras are unaffected before and after. Backport notes, in case they help anyone else on 6.19: 2/7 and the table hunks of 7/7 need hand placement (no OVTI5675/HIMX1092 entries there), and 5/7 changes the ipu_bridge_init()/ipu_bridge_parse_ssdb() CRCs, so intel-ipu6 has to be rebuilt alongside ipu-bridge under MODVERSIONS; isys and psys only import ipu_bridge_instantiate_vcm and are fine. D. Manresa <dmanresa@gmail.com>
Hi Manresa, On Sun, Sep 06, 2026 at 09:39:32AM +0200, D. Manresa wrote: > - The sensor's 2x2 binned 1296x972 readout (a downstream mode, not in > mainline) streams through the IPU6 hardware ISP (PSYS) at ~28.7 fps Thank you, and sorry for the slow reply. Your tag is the only one that covers the binned readout and the PSYS path, which is how the Pro 7+ front camera is actually used, so it filled a real gap in the coverage. The point about the CSI-2 error signature being identical to the 0x2d build over the same five capture cycles is also the cleanest confirmation so far that bits 0, 2 and 3 were doing nothing. The series was applied to media-committers next on 2026-09-11, and your tag did not make it into the commits. I have pointed Sakari at your message in the follow-up fix, in case that commit can still be amended. You were also right about the property name, but I was slow and the literal went in as 8e3def7bf410 with the bug intact. I have sent the fix as a follow-up, with you as Reported-by: media: ipu-bridge: Keep the clock-noncontinuous property name out of rodata It routes the name through ipu_property_names exactly as you suggested, so it matches what you already had applied locally when you ran the rebind test. One detail: the dangling pointer does not depend on your series at all. The nodes are already never unregistered on a successful init, so the literal dangles in the tree as it stands today. Your 1/2 is still the fix for the remaining two, the link-frequencies values and the lens-focus name, both of which are still literals in next. Thanks again for the review and the testing. Fernando
On Mon, 14 Sep 2026, Fernando Rimoli wrote: > The series was applied to media-committers next on 2026-09-11, and your tag > did not make it into the commits. I have pointed Sakari at your message in > the follow-up fix, in case that commit can still be amended. Thanks, but please don't spend effort on that. The coverage was the point, not the line in the commit. Two practical things, because I am donating this Surface Pro 7+ shortly and will not have the hardware any more: - Don't hold anything waiting for a test from me. That includes a v3 of my ipu-bridge series: it stands as tested on 6.19 and on top of your v5, and anyone who wants to carry it is welcome to it (Sakari, on Cc). - The two remaining literals, the link-frequencies values and the lens-focus name, are what my 1/2 fixes. Take it, rework it, or rewrite it under your own name, whichever is least friction. Everything measured on this machine stays public, including the register tables extracted from the vendor driver, the Bayer-phase matrices and the raw captures: https://github.com/dmanresa-saes/surface-ipu6-cameras Good luck with the rest of the series. D. Manresa <dmanresa@gmail.com>
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.0The 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 1bb3a3e98..131c70844 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -95,6 +95,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.0The sensor can gate its MIPI clock lane while idle, which the CSI-2
receivers on some platforms require. Allow the endpoint property that
asks for it, as ov5647 already does.
Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com>
---
Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
index 3368b3bd8..0d8ca0a51 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
@@ -82,6 +82,8 @@ properties:
unevaluatedProperties: false
properties:
+ clock-noncontinuous: true
+
link-frequencies: true
data-lanes:
--
2.43.0Acked-by: Conor Dooley <conor.dooley@microchip.com> pw-bot: not-applicable
The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00
power-on default, which lets the MIPI clock run freely. The IPU3 CSI-2
receiver tolerates this, but the IPU6 receiver (e.g. on Microsoft
Surface Pro 7+, Pro 8, Pro 9 and Surface Go 4) fails to lock onto the
link, so the sensor streams but capture times out with "stream stop
time out". On most affected machines no frames arrive at all; on some
the failure is intermittent.
Gate the clock lane while idle at stream on when the endpoint requests a
non-continuous clock.
Only the gate bit is touched, so platforms that do not request it are
unaffected. No counterpart is needed at stream off, as the link is down
by then and the register returns to its default when the sensor is
powered off.
The property is supplied by the ipu-bridge 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> # Surface Pro 7+, IPU6 Tiger Lake
Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6 Tiger Lake (8086:9a19)
Tested-by: Lucas Lis <lucaseze.lis@gmail.com> # Surface Pro 7+, IPU6 Tiger Lake (0x9a19)
Tested-by: Kengo Oki <dev.kengo.fugu0141@gmail.com> # Surface Go 4, IPU6 Alder Lake-N 8086:462e
---
drivers/media/i2c/ov5693.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 02236f3db..cedc6ea03 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -35,6 +35,11 @@
#define OV5693_STOP_STREAMING 0x00
#define OV5693_SW_RESET 0x01
+/* MIPI transmitter control */
+#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800)
+/* Gate the clock lane when there is no packet to transmit */
+#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5)
+
#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 +149,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 +619,19 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable)
{
int ret = 0;
+ /*
+ * Gate the MIPI clock lane while idle if the CSI-2 link is configured
+ * for a non-continuous clock. Only that bit is touched, and only in
+ * that case, so the register keeps whatever the platform left in it
+ * and the clock stays free-running as before everywhere else. It
+ * needs no counterpart at stream off: the link is down by then, and
+ * the register returns to its default when the sensor is powered off.
+ */
+ if (enable && ov5693->clock_ncont)
+ cci_update_bits(ov5693->regmap, OV5693_MIPI_CTRL00_REG,
+ OV5693_MIPI_CTRL00_CLOCK_LANE_GATE,
+ OV5693_MIPI_CTRL00_CLOCK_LANE_GATE, &ret);
+
cci_write(ov5693->regmap, OV5693_SW_STREAM_REG,
enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING,
&ret);
@@ -1259,6 +1280,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.0Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6
Tiger Lake (8086:9a19)
Scope: patches 4-7, as in v4. Patches 1 and 2 are already in my tree and
this machine enumerates INT33BE, so neither is exercised here; patch 3 is
not either, there is no DT on this platform.
Retested rather than carried over, because patches 5 and 6 are not the
code I tagged in v4. Same machine and kernel as before: 7.2.2 plus the
linux-surface patch set. Patches 3 to 6 applied to it unchanged; patch 7
needed two hunks by hand, both because that tree's sensor table differs
from yours — it has no HIMX1092 entry, which the first hunk uses as
context, and it carries its own out-of-order OVTI5693 entry. I put the
flagged entry immediately before the generic one, as the new adjacency
rule requires.
Capture from the ISYS node, three times in a row with the sensor
runtime-suspending in between:
60 frames, SBGGR10 2592x1944, 604661760 bytes, 28.63-28.65 fps (x3)
MIPI_CTRL00 (0x4800) read over i2c while streaming: 0x20
MIPI_CTRL00 with no stream: i2c times out, the sensor is powered off
0x20 is bit 5 alone, so the property still reaches the sensor driver
through the reworked per-IPU match, and the whole path is exercised
rather than "the camera works". Three captures because of the warning in
your cover letter: on this machine the first one after a cold boot
succeeds even without the entry.
Patch 6's adjacency rule behaves as described. INT33BE appears twice in
this tree, the Tiger Lake entry and the generic one, and the sensor is
connected once: "Connected 3 cameras" and exactly one ov5693 subdev in
the media graph. I also ran it with the pristine upstream table, where
this machine has no third sensor: "Connected 2 cameras", one ov5693, and
the same three captures.
Patch 5 is exercised by the sensor probing at all. ov5693 refuses an
endpoint with no link frequencies, and then refuses one that does not
offer 419.2 MHz, so a probe that succeeds means the running index put
link-frequencies and clock-noncontinuous in the right slots and did not
displace anything. The other case, where nr_link_freqs is 0 and
clock-noncontinuous packs down into the freed slot, is not exercised
here: both flagged entries carry one frequency.
The other two sensors on this machine are unaffected: OV13858 rear
captures normally, and the VD55G0 IR camera still does face
authentication.
The "stream stop time out" and "stream close time out" at teardown appear
with and without the series, as you say.
Built clean, no new warnings.
One practical note for anyone testing this by swapping modules rather
than booting a whole kernel. With CONFIG_MODVERSIONS the series moves
symbol CRCs, because struct ipu_sensor changes size:
ipu_bridge_parse_ssdb 0x8730390b -> 0x222edd7f
ipu_bridge_init 0xbb0996a9 -> 0x573da924
So intel-ipu6 and intel-ipu6-isys have to come from the same build as
ipu-bridge. I compared Module.symvers before swapping and rebuilt all
three together, so I did not hit it, but installing ipu-bridge alone
would have left the other two refusing to load.
On 9/2/26 17:23, Fernando Rimoli wrote:
> The ov5693 never programs MIPI_CTRL00 (0x4800), leaving it at its 0x00
> power-on default, which lets the MIPI clock run freely. The IPU3 CSI-2
> receiver tolerates this, but the IPU6 receiver (e.g. on Microsoft
> Surface Pro 7+, Pro 8, Pro 9 and Surface Go 4) fails to lock onto the
> link, so the sensor streams but capture times out with "stream stop
> time out". On most affected machines no frames arrive at all; on some
> the failure is intermittent.
>
> Gate the clock lane while idle at stream on when the endpoint requests a
> non-continuous clock.
>
> Only the gate bit is touched, so platforms that do not request it are
> unaffected. No counterpart is needed at stream off, as the link is down
> by then and the register returns to its default when the sensor is
> powered off.
>
> The property is supplied by the ipu-bridge 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> # Surface Pro 7+, IPU6 Tiger Lake
> Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6 Tiger Lake (8086:9a19)
> Tested-by: Lucas Lis <lucaseze.lis@gmail.com> # Surface Pro 7+, IPU6 Tiger Lake (0x9a19)
> Tested-by: Kengo Oki <dev.kengo.fugu0141@gmail.com> # Surface Go 4, IPU6 Alder Lake-N 8086:462e
> ---
> drivers/media/i2c/ov5693.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
> index 02236f3db..cedc6ea03 100644
> --- a/drivers/media/i2c/ov5693.c
> +++ b/drivers/media/i2c/ov5693.c
> @@ -35,6 +35,11 @@
> #define OV5693_STOP_STREAMING 0x00
> #define OV5693_SW_RESET 0x01
>
> +/* MIPI transmitter control */
> +#define OV5693_MIPI_CTRL00_REG CCI_REG8(0x4800)
> +/* Gate the clock lane when there is no packet to transmit */
> +#define OV5693_MIPI_CTRL00_CLOCK_LANE_GATE BIT(5)
> +
> #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 +149,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 +619,19 @@ static int ov5693_enable_streaming(struct ov5693_device *ov5693, bool enable)
> {
> int ret = 0;
>
> + /*
> + * Gate the MIPI clock lane while idle if the CSI-2 link is configured
> + * for a non-continuous clock. Only that bit is touched, and only in
> + * that case, so the register keeps whatever the platform left in it
> + * and the clock stays free-running as before everywhere else. It
> + * needs no counterpart at stream off: the link is down by then, and
> + * the register returns to its default when the sensor is powered off.
> + */
> + if (enable && ov5693->clock_ncont)
> + cci_update_bits(ov5693->regmap, OV5693_MIPI_CTRL00_REG,
> + OV5693_MIPI_CTRL00_CLOCK_LANE_GATE,
> + OV5693_MIPI_CTRL00_CLOCK_LANE_GATE, &ret);
> +
> cci_write(ov5693->regmap, OV5693_SW_STREAM_REG,
> enable ? OV5693_START_STREAMING : OV5693_STOP_STREAMING,
> &ret);
> @@ -1259,6 +1280,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);
>
Index the ep_properties array dynamically instead of plain numerical
values as is done in mipi-disco-img.c.
Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com>
Tested-by: Jakob Berg Jespersen <dev@berg.pm> # Surface Pro 7+, IPU6 Tiger Lake
Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6 Tiger Lake (8086:9a19)
Tested-by: Lucas Lis <lucaseze.lis@gmail.com> # Surface Pro 7+, IPU6 Tiger Lake (0x9a19)
Tested-by: Kengo Oki <dev.kengo.fugu0141@gmail.com> # Surface Go 4, IPU6 Alder Lake-N 8086:462e
---
drivers/media/pci/intel/ipu-bridge.c | 27 ++++++++++++++-------------
include/media/ipu-bridge.h | 19 ++++++++++++++++++-
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 131c70844..eb7d1611b 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -451,6 +451,7 @@ static void ipu_bridge_create_fwnode_properties(
{
struct ipu_property_names *names = &sensor->prop_names;
struct software_node *nodes = sensor->swnodes;
+ unsigned int i = 0;
sensor->prop_names = prop_names;
@@ -508,21 +509,21 @@ static void ipu_bridge_create_fwnode_properties(
PROPERTY_ENTRY_REF_ARRAY("lens-focus", sensor->vcm_ref);
}
- sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
- sensor->prop_names.bus_type,
- V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
- sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN(
- sensor->prop_names.data_lanes,
- bridge->data_lanes, sensor->lanes);
- sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY(
- sensor->prop_names.remote_endpoint,
- sensor->local_ref);
+ sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_BUS_TYPE)] =
+ PROPERTY_ENTRY_U32(names->bus_type,
+ V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
+ sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_DATA_LANES)] =
+ PROPERTY_ENTRY_U32_ARRAY_LEN(names->data_lanes,
+ bridge->data_lanes, sensor->lanes);
+ sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_REMOTE_EP)] =
+ PROPERTY_ENTRY_REF_ARRAY(names->remote_endpoint,
+ sensor->local_ref);
if (cfg->nr_link_freqs > 0)
- sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
- sensor->prop_names.link_frequencies,
- cfg->link_freqs,
- cfg->nr_link_freqs);
+ sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_LINK_FREQUENCIES)] =
+ PROPERTY_ENTRY_U64_ARRAY_LEN(names->link_frequencies,
+ cfg->link_freqs,
+ cfg->nr_link_freqs);
sensor->ipu_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
sensor->prop_names.data_lanes,
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 16fac7654..06701d151 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -64,6 +64,23 @@ enum ipu_sensor_swnodes {
SWNODE_COUNT
};
+enum ipu_bridge_ep_props {
+ IPU_BRIDGE_EP_BUS_TYPE,
+ IPU_BRIDGE_EP_DATA_LANES,
+ IPU_BRIDGE_EP_REMOTE_EP,
+ IPU_BRIDGE_EP_LINK_FREQUENCIES,
+ IPU_BRIDGE_EP_NUM_OF,
+ IPU_BRIDGE_EP_NUM_ENTRIES
+};
+
+/*
+ * Get the index of the next property in a property array, with a given maximum
+ * value.
+ */
+#define IPU_BRIDGE_NEXT_PROPERTY(index, max) \
+ (WARN_ON((index) > IPU_BRIDGE_##max) ? \
+ IPU_BRIDGE_##max : (index)++)
+
/* Data representation as it is in ACPI SSDB buffer */
struct ipu_sensor_ssdb {
u8 version;
@@ -141,7 +158,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[IPU_BRIDGE_EP_NUM_ENTRIES];
struct property_entry dev_properties[5];
struct property_entry ipu_properties[3];
struct property_entry ivsc_properties[1];
--
2.43.0Some sensors need different treatment depending on which IPU they are
connected to, so the ACPI HID alone is not enough.
Match on an optional list of IPU PCI product IDs. Entries for one HID
must be adjacent, IPU-specific ones first, so the generic entry is
skipped once a specific one has matched.
Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com>
Tested-by: Jakob Berg Jespersen <dev@berg.pm> # Surface Pro 7+, IPU6 Tiger Lake
Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6 Tiger Lake (8086:9a19)
Tested-by: Lucas Lis <lucaseze.lis@gmail.com> # Surface Pro 7+, IPU6 Tiger Lake (0x9a19)
Tested-by: Kengo Oki <dev.kengo.fugu0141@gmail.com> # Surface Go 4, IPU6 Alder Lake-N 8086:462e
---
drivers/media/pci/intel/ipu-bridge.c | 35 ++++++++++++++++++++++++++++
include/media/ipu-bridge.h | 26 +++++++++++++++++----
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index eb7d1611b..5efdcb9c8 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>
@@ -853,8 +854,28 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg,
return ret;
}
+/*
+ * Whether a sensor config applies to the IPU this bridge sits on. A config
+ * listing PCI product IDs only applies to those IPUs.
+ */
+static bool ipu_bridge_config_matches(const struct ipu_sensor_config *cfg,
+ struct ipu_bridge *bridge)
+{
+ const u16 *id;
+
+ if (!cfg->pci_ids)
+ return true;
+
+ for (id = cfg->pci_ids; *id; id++)
+ if (*id == bridge->pci_id)
+ return true;
+
+ return false;
+}
+
static int ipu_bridge_connect_sensors(struct ipu_bridge *bridge)
{
+ const char *done_hid = NULL;
unsigned int i;
int ret;
@@ -862,9 +883,22 @@ static int ipu_bridge_connect_sensors(struct ipu_bridge *bridge)
const struct ipu_sensor_config *cfg =
&ipu_supported_sensors[i];
+ /*
+ * Entries for one HID are adjacent, IPU-specific ones first,
+ * so the generic entry is skipped once a specific one has
+ * matched and the sensor is not connected twice.
+ */
+ if (done_hid && !strcmp(cfg->hid, done_hid))
+ continue;
+
+ if (!ipu_bridge_config_matches(cfg, bridge))
+ continue;
+
ret = ipu_bridge_connect_sensor(cfg, bridge);
if (ret)
goto err_unregister_sensors;
+
+ done_hid = cfg->hid;
}
return 0;
@@ -948,6 +982,7 @@ int ipu_bridge_init(struct device *dev,
sizeof(bridge->ipu_node_name));
bridge->ipu_hid_node.name = bridge->ipu_node_name;
bridge->dev = dev;
+ bridge->pci_id = dev_is_pci(dev) ? to_pci_dev(dev)->device : 0;
bridge->parse_sensor_fwnode = parse_sensor_fwnode;
ret = software_node_register(&bridge->ipu_hid_node);
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 06701d151..aca76ff71 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -17,13 +17,24 @@
#define IPU_SENSOR_ROTATION_NORMAL 0
#define IPU_SENSOR_ROTATION_INVERTED 1
-#define IPU_SENSOR_CONFIG(_HID, _NR, ...) \
- (const struct ipu_sensor_config) { \
- .hid = _HID, \
- .nr_link_freqs = _NR, \
- .link_freqs = { __VA_ARGS__ } \
+/*
+ * Sensor config specific to one or more IPUs, identified by their PCI product
+ * IDs, with flags describing what the sensor needs there. Entries for one HID
+ * must be adjacent in ipu_supported_sensors[], with the IPU-specific ones
+ * before the generic one.
+ */
+#define IPU_SENSOR_CONFIG_MATCH_FL(_HID, _IDS, _FLAGS, _NR, ...) \
+ (const struct ipu_sensor_config) { \
+ .hid = _HID, \
+ .pci_ids = _IDS, \
+ .flags = _FLAGS, \
+ .nr_link_freqs = _NR, \
+ .link_freqs = { __VA_ARGS__ } \
}
+#define IPU_SENSOR_CONFIG(_HID, _NR, ...) \
+ IPU_SENSOR_CONFIG_MATCH_FL(_HID, NULL, 0, _NR, __VA_ARGS__)
+
#define NODE_SENSOR(_HID, _PROPS) \
(const struct software_node) { \
.name = _HID, \
@@ -132,6 +143,9 @@ struct ipu_node_names {
struct ipu_sensor_config {
const char *hid;
+ /* Zero-terminated list of IPU PCI product IDs, NULL for any IPU */
+ const u16 *pci_ids;
+ const u32 flags;
const u8 nr_link_freqs;
const u64 link_freqs[MAX_NUM_LINK_FREQS];
};
@@ -177,6 +191,8 @@ typedef int (*ipu_parse_sensor_fwnode_t)(struct acpi_device *adev,
struct ipu_bridge {
struct device *dev;
+ /* PCI product ID of the IPU, 0 if it is not a PCI device */
+ u16 pci_id;
ipu_parse_sensor_fwnode_t parse_sensor_fwnode;
char ipu_node_name[ACPI_ID_LEN];
struct software_node ipu_hid_node;
--
2.43.0The ov5693 (INT33BE / OVTI5693) needs its MIPI clock lane gated while
idle for the IPU6 CSI-2 receiver to lock onto the link, 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 request
it only for the IPU6 variants where it has been confirmed on hardware:
Tiger Lake, Alder Lake-P and Alder Lake-N.
Signed-off-by: Fernando Rimoli <fernandorimoli11@gmail.com>
Tested-by: Jakob Berg Jespersen <dev@berg.pm> # Surface Pro 7+, IPU6 Tiger Lake
Tested-by: Fil Dunsky <filipp.dunsky@gmail.com> # Surface Pro 8, IPU6 Tiger Lake (8086:9a19)
Tested-by: Lucas Lis <lucaseze.lis@gmail.com> # Surface Pro 7+, IPU6 Tiger Lake (0x9a19)
Tested-by: Kengo Oki <dev.kengo.fugu0141@gmail.com> # Surface Go 4, IPU6 Alder Lake-N 8086:462e
---
drivers/media/pci/intel/ipu-bridge.c | 19 +++++++++++++++++++
include/media/ipu-bridge.h | 5 +++++
2 files changed, 24 insertions(+)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 5efdcb9c8..12f1cccd8 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -16,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))
@@ -49,6 +50,14 @@
*
* Please keep the list sorted by ACPI HID.
*/
+/* IPU6 variants whose CSI-2 receiver needs the ov5693 clock lane gated */
+static const u16 ipu6_ov5693_ncont_clk[] = {
+ PCI_DEVICE_ID_INTEL_IPU6, /* Tiger Lake */
+ PCI_DEVICE_ID_INTEL_IPU6EP_ADLP, /* Alder Lake-P */
+ PCI_DEVICE_ID_INTEL_IPU6EP_ADLN, /* Alder Lake-N */
+ 0
+};
+
static const struct ipu_sensor_config ipu_supported_sensors[] = {
/* Himax HM1092 */
IPU_SENSOR_CONFIG("HIMX1092", 2, 180000000, 180480000),
@@ -61,6 +70,9 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
/* GalaxyCore GC0310 */
IPU_SENSOR_CONFIG("INT0310", 1, 55692000),
/* Omnivision OV5693 */
+ IPU_SENSOR_CONFIG_MATCH_FL("INT33BE", ipu6_ov5693_ncont_clk,
+ IPU_BR_FL_CSI2_CLK_NONCONTINUOUS,
+ 1, 419200000),
IPU_SENSOR_CONFIG("INT33BE", 1, 419200000),
/* Onsemi MT9M114 */
IPU_SENSOR_CONFIG("INT33F0", 1, 384000000),
@@ -97,6 +109,9 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
/* Omnivision OV5675 */
IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000),
/* Omnivision OV5693 */
+ IPU_SENSOR_CONFIG_MATCH_FL("OVTI5693", ipu6_ov5693_ncont_clk,
+ IPU_BR_FL_CSI2_CLK_NONCONTINUOUS,
+ 1, 419200000),
IPU_SENSOR_CONFIG("OVTI5693", 1, 419200000),
/* Omnivision OV8856 */
IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000),
@@ -526,6 +541,10 @@ static void ipu_bridge_create_fwnode_properties(
cfg->link_freqs,
cfg->nr_link_freqs);
+ if (cfg->flags & IPU_BR_FL_CSI2_CLK_NONCONTINUOUS)
+ sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_CLOCK_NONCONTINUOUS)] =
+ 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 aca76ff71..eec8c4688 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -17,6 +17,10 @@
#define IPU_SENSOR_ROTATION_NORMAL 0
#define IPU_SENSOR_ROTATION_INVERTED 1
+/* Flags for struct ipu_sensor_config */
+/* The sensor's CSI-2 transmitter needs a non-continuous clock */
+#define IPU_BR_FL_CSI2_CLK_NONCONTINUOUS BIT(0)
+
/*
* Sensor config specific to one or more IPUs, identified by their PCI product
* IDs, with flags describing what the sensor needs there. Entries for one HID
@@ -80,6 +84,7 @@ enum ipu_bridge_ep_props {
IPU_BRIDGE_EP_DATA_LANES,
IPU_BRIDGE_EP_REMOTE_EP,
IPU_BRIDGE_EP_LINK_FREQUENCIES,
+ IPU_BRIDGE_EP_CLOCK_NONCONTINUOUS,
IPU_BRIDGE_EP_NUM_OF,
IPU_BRIDGE_EP_NUM_ENTRIES
};
--
2.43.0Hi Fernando,
On Wed, 2 Sep 2026, Fernando Rimoli wrote:
> + if (cfg->flags & IPU_BR_FL_CSI2_CLK_NONCONTINUOUS)
> + sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, IPU_BRIDGE_EP_CLOCK_NONCONTINUOUS)] =
> + PROPERTY_ENTRY_BOOL("clock-noncontinuous");
One small thing, coming from the ipu-bridge series I have under review in
parallel ("media: ipu-bridge: survive module unload and reuse the software
nodes on rebind", <20260831140304.45940-1-dmanresa@gmail.com>): the software
nodes ipu-bridge registers are deliberately never unregistered and must
survive the module being unloaded, so every string a registered property
points at has to live in the bridge's own allocation, not in the module
image. That is why the other endpoint property names all go through the
char[] members of struct ipu_property_names, copied into sensor->prop_names.
"clock-noncontinuous" above is a string literal in ipu-bridge's rodata, so
after an unload the surviving node carries a dangling property name - the
same class of problem my 1/2 fixes for the "lens-focus" literal. The fix is
one line in your design: add a `char clock_noncontinuous[sizeof("clock-
noncontinuous")]` to struct ipu_property_names, initialise it in
prop_names, and use `sensor->prop_names.clock_noncontinuous` here. I have
that variant applied locally on top of your v5 and it is what I am testing.
Two related notes:
- Your 5/7 and my 1/2 touch the same link-frequencies block in
ipu_bridge_create_fwnode_properties(); the merge is trivial (your
IPU_BRIDGE_NEXT_PROPERTY() indexing, my copy of cfg->link_freqs into the
bridge allocation). Your series is further along, so I will rebase mine
on top of yours once it is applied - no action needed on your side.
- The MIPI_CTRL00 gate supersedes the unconditional 0x4800 = 0x2d write the
Surface Pro 7+ downstream drivers carry (mine included); Fil's Pro 8
sweep showing bit 5 is the only one that matters agrees with everything
I have measured here. What nobody has covered yet is bit 5 alone in the
sensor's 2x2 binned 1296x972 readout and through the IPU6 hardware ISP
(PSYS) path, which is how the Pro 7+ front camera is used in practice; I
am running exactly that on this machine with your v5 (backported to a
6.19 tree, with the downstream 0x2d write removed) and will follow up with
a Tested-by for 4-7 covering it if it holds.
Thanks for the series - it turns a hack several of us were carrying into the
right thing.
D. Manresa <dmanresa@gmail.com>
© 2016 - 2026 Red Hat, Inc.