[PATCH v5 0/7] media: Enable the OV5693 front camera on IPU6 Surface devices

Fernando Rimoli posted 7 patches 3 weeks, 2 days ago
.../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(-)
[PATCH v5 0/7] media: Enable the OV5693 front camera on IPU6 Surface devices
Posted by Fernando Rimoli 3 weeks, 2 days ago
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
Re: [PATCH v5 0/7] media: Enable the OV5693 front camera on IPU6 Surface devices
Posted by D. Manresa 2 weeks, 6 days ago
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>
Re: [PATCH v5 0/7] media: Enable the OV5693 front camera on IPU6 Surface devices
Posted by Fernando Rimoli 1 week, 4 days ago
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
Re: [PATCH v5 0/7] media: Enable the OV5693 front camera on IPU6 Surface devices
Posted by D. Manresa 4 days, 10 hours ago
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>