[PATCH v2] media: ipu-bridge: add the OV13858 rear sensor

Sergey Lebedev posted 1 patch 1 week, 4 days ago
drivers/media/pci/intel/ipu-bridge.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2] media: ipu-bridge: add the OV13858 rear sensor
Posted by Sergey Lebedev 1 week, 4 days ago
The Microsoft Surface Pro 11 for Business (Intel Lunar Lake, IPU7) carries
an OV13858 as its rear sensor, enumerated as ACPI device OVTID858. That
HID is not in ipu_supported_sensors[], so ipu_bridge_connect_sensors()
passes over it and the IPU is left with an empty graph:

  intel-ipu7 0000:00:05.0: no subdev found in graph

Nothing is logged about the sensor having been passed over, which makes
the failure hard to attribute: the ACPI device is present and enabled and
the i2c client exists.

ov13858 declares two link frequencies, in this order:

  #define OV13858_NUM_OF_LINK_FREQS	2
  #define OV13858_LINK_FREQ_540MHZ	540000000ULL
  #define OV13858_LINK_FREQ_270MHZ	270000000ULL

With the entry added the bridge connects the sensor and the graph is
built:

  intel-ipu7: Found supported sensor OVTID858:00 (\_SB.PC00.I2C3.CAMR)
  intel_ipu7_isys: bind ov13858 1-0010 nlanes is 4 port is 0

  - entity 247: ov13858 1-0010 (1 pad, 1 link, 0 routes)
                device node name /dev/v4l-subdev5
    pad0: SOURCE
          [stream:0 fmt:SGRBG10_1X10/4224x3136 field:none]
          -> "Intel IPU7 CSI2 0":0 [ENABLED,IMMUTABLE]

and libcamera captures from it, 30 fps at its own choice of 4220x3136 out
of the sensor's 4224x3136.

The rear camera on this hardware was reported dead publicly by German
Pablo Lindo, who traced it to int3472 not recognising GPIO type 0x08:

  https://lore.kernel.org/linux-media/20260903081557.16603-1-germanpapulindez@gmail.com/

That is a different cause of the same symptom, fixed separately by
"platform/x86: int3472: support the POWER1 GPIO type", which is in
linux-next and not yet in media/next. The missing bridge entry is the
next one along. The sensor also needs power sequencing the in-tree
ov13858 driver does not do on this platform; that is a separate patch
and not a prerequisite for this one to be correct.

Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
---
v1: https://lore.kernel.org/linux-media/20260913100932.92087-1-lsa.uz@pm.me/

v2: no functional change. The Media CI robot flagged one over-75 line in the
    commit message, and fixing only that moved the warning to the next quoted
    log line, so both are shortened: the PCI address is dropped from the
    intel-ipu7 line and the full device path from the intel_ipu7_isys one.
    What the lines say is unchanged; only this machine's slot numbering is
    gone. checkpatch is now silent.

 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 7e65491..8a16c81 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -88,6 +88,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
 	IPU_SENSOR_CONFIG("OVTI08A1", 1, 500000000),
 	/* Omnivision OV08x40 */
 	IPU_SENSOR_CONFIG("OVTI08F4", 3, 400000000, 749000000, 800000000),
+	/* Omnivision OV13858 */
+	IPU_SENSOR_CONFIG("OVTID858", 2, 540000000, 270000000),
 	/* Omnivision OV13B10 */
 	IPU_SENSOR_CONFIG("OVTI13B1", 1, 560000000),
 	IPU_SENSOR_CONFIG("OVTIDB10", 1, 560000000),
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v2] media: ipu-bridge: add the OV13858 rear sensor
Posted by Fernando Rimoli 1 week, 3 days ago
Hi Sergey,

On Sun, Sep 13, 2026 at 02:20:40PM +0000, Sergey Lebedev wrote:
> +	/* Omnivision OV13858 */
> +	IPU_SENSOR_CONFIG("OVTID858", 2, 540000000, 270000000),

I have the same sensor on the other side of the IPU generation gap, a
Surface Pro 9, IPU6 Alder Lake-P, with OVTID858 as the rear sensor and
OVTI5693 as the front one. Your entry works there too:

Tested-by: Fernando Rimoli <fernandorimoli11@gmail.com> # Surface Pro 9, IPU6 Alder Lake-P (8086:465d)

Method, since this machine does not boot media/next as it stands: I took
the tree it does boot and replaced my own OVTID858 entry with yours, so 
your line is the only difference from a known-good build. Your
patch also applies clean to media/next on its own. Only ipu-bridge needed
rebuilding, as the entry does not touch the header, so the exported CRCs
are unchanged and the installed intel-ipu6 and intel-ipu6-isys load against
it untouched.

Cold boot:

  intel-ipu6 0000:00:05.0: Found supported sensor OVTI5693:00
  intel-ipu6 0000:00:05.0: Found supported sensor OVTID858:00
  intel-ipu6 0000:00:05.0: Found supported sensor SMO55F0:00
  intel-ipu6 0000:00:05.0: Connected 3 cameras

Rear sensor at its native 4224x3136 SGRBG10 off the ISYS node, 29.95 fps
over 60 frames, and the operating point is identical to my ascending
ordering, which is the part worth checking:

  link_frequency  value=0 (540000000)
  pixel_rate      432000000

To show the path carries pixels and not just buffers I used the sensor's
own vertical colour bar: stddev 479.5 on a 0-1023 range, min/max 64/1023,
bar edge where it belongs, and no CSI-2 errors on that run with the log
cleared first. Front and IR cameras unaffected.

One thing that may save you a review round is that ov13858 never reads the
link-frequencies property at all. It calls v4l2_fwnode_device_parse() for
the device properties and never v4l2_fwnode_endpoint_alloc_parse(), so the
array the bridge publishes is not consumed by this driver. It exposes its
own menu instead, and the receiver takes V4L2_CID_LINK_FREQ from that:

  link_frequency  0x009f0901 (intmenu): min=0 max=1 value=0 (540000000)
  pixel_rate      0x009f0902 (int64)  : value=432000000

That reads 540 MHz here even though my local entry happens to list 270
first, which is how I know the order is inert. Useful if anyone asks why
you ordered them that way, the other multi-frequency entries in that table
happen to be ascending.

What I am mainly writing about is your remark that the sensor also needs
power sequencing the in-tree driver does not do, and that it is a separate
patch. I have an ov13858 patch parked that touches the same probe path, and
I would rather we did not send two overlapping fixes into the same
function.

Mine retries the chip-id read in ov13858_identify_module(), five attempts
5 ms apart. Without it the rear camera fails probe with -EIO on every boot
here. The cause I identified is not a missing power sequence: on this
machine OVTID858 and the IR sensor sit on the same I2C controller
(ov13858 2-0010 and vd55g 2-0060), and the first chip-id read gets
corrupted by bus and power activity from the neighbour, so it returns
garbage rather than timing out. Reverting the retry on its own reproduces
the failure.

So, what does your power-sequencing patch do, and against which symptom?
If it orders or delays the sensor's own power-up then the two are probably
independent and both wanted, in which case I should say so in my commit
message so that it does not read as a duplicate of yours. If it subsumes
what I am seeing, I would rather drop mine and test yours. Either answer
suits me, I would just like to know before either of us sends.

I had held mine back because the failure was not reproducible on mainline,
there being no bridge entry to enumerate the sensor in the first place.
Your patch removes that objection, which is great.

On your rotation patch, separately: the SP9 rear sensor is mounted inverted
as well and its SSDB reports 0, so that machine wants the same treatment. I
will send the SP9 entries myself. Your DMI_SYS_VENDOR match is right for
Surface, for what it is worth: this SP9 reports "Microsoft Corporation"
with no leading space, so the exact match holds across two Surface
generations.

Thanks for doing this.

Fernando
Re: [PATCH v2] media: ipu-bridge: add the OV13858 rear sensor
Posted by Sergey Lebedev 1 week, 3 days ago
Thank you for the tag, and for replacing your own entry with mine so that my
line was the only difference. That is more care than a Tested-by usually
costs.

Your question is a better one than it looks, because our two failures produce
the same string:

  ov13858 i2c-OVTID858:00: failed to find sensor: -5
  ov13858 i2c-OVTID858:00: probe with driver ov13858 failed with error -5

Same -EIO, and necessarily so: ov13858_identify_module() has one caller and
one dev_err, so every cause that makes the chip-id read fail prints exactly
that. On this machine that line has already had two causes. In July it was
int3472 not knowing GPIO type 0x08, so none of the regulators existed; since
that was fixed upstream it has been the driver not asking for the ones that
do. Yours would be a third. So I checked rather than answering from memory.

They are independent, and neither subsumes the other.

Mine is that the sensor is never powered. The driver's assumption holds where
the rails are ACPI power resources. Here an INT3472 companion registers them
as regulators, a clock and a reset GPIO for the sensor driver to consume, and
ov13858 consumes none of them. The patch requests dovdd, avdd and dvdd and
the reset GPIO, and sequences them with the clock in the runtime PM
callbacks. It retries nothing.

What rules your mechanism out for mine is that there was no neighbour. The
before-and-after was measured on a media/next build, and no media/next build
carries a vd55g0 module - that driver is still in review - so SMO55F0 was
unbound while the read was failing. There was nothing on the bus to corrupt
the transfer, it failed anyway, and adding the supplies fixed it. The build I
am on today is the same in that respect:

  /sys/bus/i2c/devices, adapter i2c-1
    i2c-OVTID858:00   driver=ov13858
    i2c-SMO55F0:00    driver=none

which is your topology exactly - the rear sensor sharing a controller with
the infrared one - with the neighbour silent because nothing claims its HID.

Retrying a read on an unpowered part returns -EIO five times, and powering a
part that is already powered does not stop a neighbour corrupting a transfer.
So both, and your commit message can say so without it reading as a duplicate
of mine.

Since more than one build is involved above, which is which:

  media/next f9536a806, no vd55g0 module in the tree
    stock ov13858    failed to find sensor: -5 at boot
    with my patch    supply dovdd not found, using dummy regulator, binds
  a 7.3.0-rc1 built from media/next, same absence of vd55g0
    the bus listing above; SMO55F0 unbound
  Ubuntu 7.0.0-30-generic, my patch backported, out-of-tree vd55g0 installed
    SMO55F0 bound to vd55g0, and the rear sensor binds

All on one Surface Pro 11 for Business (Intel), firmware 17.105.143.

The last of those is the useful one for you: I have a kernel here where your
neighbour is live on the same controller, and one where it is absent. If you
would like the retry tested against either when you post it, say so.

Thank you also for the link-frequency observation, which I checked and which
holds in the source: ov13858 calls v4l2_fwnode_device_parse() and neither
v4l2_fwnode_endpoint_parse() nor the _alloc_ variant anywhere, so the array
the bridge publishes is indeed never read. I had ordered the two as measured
rather than ascending and wondered whether that would be queried. It cannot
be.

Sergey