[PATCH 0/2] media: ov13858: device tree support for the Surface Pro 11 (Snapdragon)

[PATCH 0/2] media: ov13858: device tree support for the Surface Pro 11 (Snapdragon)

[George Neacsu via B4 Relay]

Hi Sakari,

The Surface Pro 11 with a Snapdragon X Elite (X1E80100) has the same
OV13858 as its rear camera as the Intel model does. There it is wired to
the Qualcomm CAMSS over four D-PHY lanes and described in the device tree.
These two patches add the binding and the OF match the driver needs for
that, and the endpoint checks that come with a DT user.

They apply on top of Sergey's "media: i2c: ov13858: add regulator, clock
and reset GPIO handling" v3. The Snapdragon machine needs it too: nothing
there powers the sensor for the driver either.
https://lore.kernel.org/all/20260831130312.26296-1-lsa.uz@pm.me/

Tested on the Surface Pro 11 (X1E80100) with mainline ov13858.c, Sergey's
v3 and these two, through an out-of-tree CAMSS capturing raw frames over
CSIPHY1, CSID0 and an RDI interface. The in-tree register lists work
unchanged at 540 MHz:
- 4224x3136 at 29.95 fps;
- the colour-bar test pattern arrives intact;
- a scene captures;
- the binding validates with dt-doc-validate, and dt-validate accepts
  the example and rejects a two-lane endpoint.

Sergey, German: on the Intel machine the IPU bridge entry for OVTID858
already lists both link frequencies, so the new endpoint check should
pass there. A quick v4l2-compliance run would confirm it.

The device tree for the Surface Pro 11 cameras will follow once Bryan's
x1e80100 camera DT series has settled.

Thanks,
George

---
George Neacsu (2):
      dt-bindings: media: i2c: Add OmniVision OV13858
      media: i2c: ov13858: Add device tree support

 .../bindings/media/i2c/ovti,ov13858.yaml           | 113 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 drivers/media/i2c/ov13858.c                        |  62 +++++++++++
 3 files changed, 176 insertions(+)
---
base-commit: ef7407f04f644833b0d1200a2e261c1879597b94
change-id: 20261015-ov13858-x1e-88fbdb884f82

Best regards,
-- 
George Neacsu <geocausa@gmail.com>

[PATCH 1/2] dt-bindings: media: i2c: Add OmniVision OV13858

[George Neacsu via B4 Relay]

From: George Neacsu <geocausa@gmail.com>

The OV13858 is a 13 megapixel sensor with a four-lane MIPI CSI-2 D-PHY
output. So far its driver has only been probed through ACPI. The
Microsoft Surface Pro 11 with a Snapdragon X Elite uses it as the rear
camera, and that machine describes it in the device tree.

The supplies are the three the driver takes, and the reset GPIO drives
the XSHUTDN pad.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 .../bindings/media/i2c/ovti,ov13858.yaml           | 113 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 114 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov13858.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov13858.yaml
new file mode 100644
index 0000000..a1cbdb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov13858.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov13858.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OV13858 CMOS Sensor
+
+maintainers:
+  - George Neacsu <geocausa@gmail.com>
+
+description:
+  The OmniVision OV13858 is a 13 megapixel CMOS image sensor with an I2C
+  control interface. It outputs 10-bit Bayer data over a four-lane MIPI
+  CSI-2 D-PHY interface and takes a 19.2 MHz input clock.
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: ovti,ov13858
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: Input clock (19.2 MHz)
+    maxItems: 1
+
+  avdd-supply:
+    description: Analogue circuit voltage supply.
+
+  dovdd-supply:
+    description: I/O circuit voltage supply.
+
+  dvdd-supply:
+    description: Digital circuit voltage supply.
+
+  reset-gpios:
+    description: Active low GPIO connected to the XSHUTDN pad of the sensor.
+    maxItems: 1
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            items:
+              - const: 1
+              - const: 2
+              - const: 3
+              - const: 4
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - avdd-supply
+  - dovdd-supply
+  - dvdd-supply
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        camera@10 {
+            compatible = "ovti,ov13858";
+            reg = <0x10>;
+
+            clocks = <&cam_mclk>;
+            assigned-clocks = <&cam_mclk>;
+            assigned-clock-rates = <19200000>;
+
+            avdd-supply = <&vreg_cam_2p8>;
+            dovdd-supply = <&vreg_cam_1p8>;
+            dvdd-supply = <&vreg_cam_1p2>;
+
+            reset-gpios = <&tlmm 110 GPIO_ACTIVE_LOW>;
+
+            orientation = <1>;
+            rotation = <0>;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&csiphy1_ep>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 <540000000 270000000>;
+                };
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 5d39ec6..0cbb07e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20186,6 +20186,7 @@ M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media.git
+F:	Documentation/devicetree/bindings/media/i2c/ovti,ov13858.yaml
 F:	drivers/media/i2c/ov13858.c
 
 OMNIVISION OV13B10 SENSOR DRIVER

-- 
2.53.0

[PATCH 2/2] media: i2c: ov13858: Add device tree support

[George Neacsu via B4 Relay]

From: George Neacsu <geocausa@gmail.com>

The rear camera of the Surface Pro 11 with a Snapdragon X Elite is an
OV13858 on a four-lane D-PHY link, described in the device tree. Add an
OF match table.

Parse the endpoint as other sensor drivers that serve both ACPI and DT
do. Wait for the endpoint if a bridge driver has not created it yet,
require four data lanes, and require both link frequencies, since the
modes switch between 540 and 270 MHz. The IPU bridge entry for OVTID858
lists both, so an ACPI machine with that entry probes as before. Without
the entry the sensor now waits for the bridge instead of probing into an
empty graph.

Tested on the Surface Pro 11 (X1E80100), with an out-of-tree CAMSS
capturing raw frames over CSIPHY1, CSID0 and an RDI interface:
4224x3136 at 29.95 fps at 540 MHz, the colour-bar test pattern intact,
and a scene captures.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
 drivers/media/i2c/ov13858.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 83eefba..587ee38 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1713,6 +1713,57 @@ static int ov13858_power_off(struct device *dev)
 static DEFINE_RUNTIME_DEV_PM_OPS(ov13858_pm_ops, ov13858_power_off,
 				 ov13858_power_on, NULL);
 
+static int ov13858_check_hwcfg(struct device *dev)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY
+	};
+	unsigned long link_freq_bitmap;
+	struct fwnode_handle *ep;
+	int ret;
+
+	/*
+	 * Sometimes the fwnode graph is initialized by the bridge driver.
+	 * Bridge drivers doing this also add sensor properties, wait for this.
+	 */
+	ep = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+	if (!ep)
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "waiting for fwnode graph endpoint\n");
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	fwnode_handle_put(ep);
+	if (ret)
+		return dev_err_probe(dev, ret, "parsing endpoint failed\n");
+
+	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
+		ret = dev_err_probe(dev, -EINVAL,
+				    "%u data lanes are not supported, only 4\n",
+				    bus_cfg.bus.mipi_csi2.num_data_lanes);
+		goto out;
+	}
+
+	/* The modes use both link frequencies, so both have to be allowed. */
+	ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies,
+				       bus_cfg.nr_of_link_frequencies,
+				       link_freq_menu_items,
+				       ARRAY_SIZE(link_freq_menu_items),
+				       &link_freq_bitmap);
+	if (ret)
+		goto out;
+
+	if (link_freq_bitmap != GENMASK(ARRAY_SIZE(link_freq_menu_items) - 1, 0))
+		ret = dev_err_probe(dev, -EINVAL,
+				    "link frequencies %lld and %lld are both needed\n",
+				    link_freq_menu_items[0],
+				    link_freq_menu_items[1]);
+
+out:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+
+	return ret;
+}
+
 static int ov13858_probe(struct i2c_client *client)
 {
 	struct ov13858 *ov13858;
@@ -1725,6 +1776,10 @@ static int ov13858_probe(struct i2c_client *client)
 
 	ov13858->dev = &client->dev;
 
+	ret = ov13858_check_hwcfg(ov13858->dev);
+	if (ret)
+		return ret;
+
 	ov13858->clk = devm_v4l2_sensor_clk_get(ov13858->dev, NULL);
 	if (IS_ERR(ov13858->clk))
 		return dev_err_probe(ov13858->dev, PTR_ERR(ov13858->clk),
@@ -1845,10 +1900,17 @@ static const struct acpi_device_id ov13858_acpi_ids[] = {
 MODULE_DEVICE_TABLE(acpi, ov13858_acpi_ids);
 #endif
 
+static const struct of_device_id ov13858_of_match[] = {
+	{ .compatible = "ovti,ov13858" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ov13858_of_match);
+
 static struct i2c_driver ov13858_i2c_driver = {
 	.driver = {
 		.name = "ov13858",
 		.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
+		.of_match_table = ov13858_of_match,
 		.pm = pm_ptr(&ov13858_pm_ops),
 	},
 	.probe = ov13858_probe,

-- 
2.53.0

Re: [PATCH 0/2] media: ov13858: device tree support for the Surface Pro 11 (Snapdragon)

[Sergey Lebedev]

Hi George,

On the Intel model, with the power-on v3 and these two on media next, the
new endpoint check passes with the endpoint ipu-bridge builds (four lanes,
540 and 270 MHz). The sensor streams 4224x3136 at 29.95 fps, and
v4l2-compliance passes 46 of 46 on the subdev. Through the media device
one test fails, Active VIDIOC_SUBDEV_S_FMT, because
ov13858_update_pad_format() never sets the colorspace. That is in media
next's driver already, not in these patches.

Tested-by: Sergey Lebedev <lsa.uz@pm.me> # Surface Pro 11 (Intel)

Thank you for picking up the power patch as a base.

Sergey

Re: [PATCH 0/2] media: ov13858: device tree support for the Surface Pro 11 (Snapdragon)

[George Neacsu]

Hi Sergey,

On 16/10/2026 08:01, Sergey Lebedev wrote:
> Through the media device
> one test fails, Active VIDIOC_SUBDEV_S_FMT, because
> ov13858_update_pad_format() never sets the colorspace. That is in media
> next's driver already, not in these patches.

Thank you for testing. That failure is fixed by "media: i2c: ov13858:
Set the colorspace in the pad format", which you have since tested
separately.

Thanks,
George

Re: [PATCH 2/2] media: i2c: ov13858: Add device tree support

[Bryan O'Donoghue]

On 15/10/2026 22:48, George Neacsu via B4 Relay wrote:
> From: George Neacsu <geocausa@gmail.com>
> 
> The rear camera of the Surface Pro 11 with a Snapdragon X Elite is an
> OV13858 on a four-lane D-PHY link, described in the device tree. Add an
> OF match table.
> 
> Parse the endpoint as other sensor drivers that serve both ACPI and DT
> do. Wait for the endpoint if a bridge driver has not created it yet,
> require four data lanes, and require both link frequencies, since the
> modes switch between 540 and 270 MHz. The IPU bridge entry for OVTID858
> lists both, so an ACPI machine with that entry probes as before. Without
> the entry the sensor now waits for the bridge instead of probing into an
> empty graph.
> 
> Tested on the Surface Pro 11 (X1E80100), with an out-of-tree CAMSS
> capturing raw frames over CSIPHY1, CSID0 and an RDI interface:
> 4224x3136 at 29.95 fps at 540 MHz, the colour-bar test pattern intact,
> and a scene captures.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: George Neacsu <geocausa@gmail.com>
This is a weird change.

You describe regulators, clocks and resets in the yaml file which then 
don't appear to be supported in this result code ?

- regulator_bulk_;
- clock_bulk_
- reset gpio optional

I don't think this driver would survive suspend/resume.

---
bod

Re: [PATCH 2/2] media: i2c: ov13858: Add device tree support

[George Neacsu]

Hi Bryan,

On 16/10/2026 17:06, Bryan O'Donoghue wrote:
> You describe regulators, clocks and resets in the yaml file which then
> don't appear to be supported in this result code ?
>
> - regulator_bulk_;
> - clock_bulk_
> - reset gpio optional
>
> I don't think this driver would survive suspend/resume.

The power handling comes from a prerequisite: this series applies on
top of Sergey's "media: i2c: ov13858: add regulator, clock and reset
GPIO handling" v3, as the cover letter says, but that is easy to miss
from 2/2 alone:
https://lore.kernel.org/all/20260831130312.26296-1-lsa.uz@pm.me/

With it, the driver gets the three supplies with
devm_regulator_bulk_get(), enables and disables the clock it already
takes with devm_v4l2_sensor_clk_get(), and takes an optional reset
GPIO. Power on and power off are the runtime PM callbacks through
DEFINE_RUNTIME_DEV_PM_OPS(), which also routes system sleep through
pm_runtime_force_suspend() and pm_runtime_force_resume(). So the binding
describes what the driver handles once both are applied. On its own,
2/2 only adds the OF match and the endpoint checks.

I cannot claim it survives system suspend on the Snapdragon model,
though. System suspend is still an open problem on the Surface Pro 11
under Linux and has not been validated, so I cannot exercise that path
here yet. What I can test is the
runtime PM path, which is where the supplies, clock and reset are
switched: the sensor powering off between sessions and back on for the
next stream. I will run repeated open, stream, close cycles and report
the result. Sergey, if suspend works on the Intel model, a suspend and
resume test there would cover the rest.

To make the dependency visible, I can carry Sergey's patch as 1/3 of
the next version, with his authorship. Sergey, would that be all right
with you, or would you rather post v4 yourself?

Thanks,
George

Re: [PATCH 2/2] media: i2c: ov13858: Add device tree support

[Bryan O'Donoghue]

On 16/10/2026 17:16, George Neacsu wrote:
> Hi Bryan,
> 
> On 16/10/2026 17:06, Bryan O'Donoghue wrote:
>> You describe regulators, clocks and resets in the yaml file which then
>> don't appear to be supported in this result code ?
>>
>> - regulator_bulk_;
>> - clock_bulk_
>> - reset gpio optional
>>
>> I don't think this driver would survive suspend/resume.
> 
> The power handling comes from a prerequisite: this series applies on
> top of Sergey's "media: i2c: ov13858: add regulator, clock and reset
> GPIO handling" v3, as the cover letter says, but that is easy to miss
> from 2/2 alone:
> https://lore.kernel.org/all/20260831130312.26296-1-lsa.uz@pm.me/


Hmm if you use b4 to generate your series, the tooling will pull this 
type of thing in for you and reviewers like me.
> With it, the driver gets the three supplies with
> devm_regulator_bulk_get(), enables and disables the clock it already
> takes with devm_v4l2_sensor_clk_get(), and takes an optional reset
> GPIO. Power on and power off are the runtime PM callbacks through
> DEFINE_RUNTIME_DEV_PM_OPS(), which also routes system sleep through
> pm_runtime_force_suspend() and pm_runtime_force_resume(). So the binding
> describes what the driver handles once both are applied. On its own,
> 2/2 only adds the OF match and the endpoint checks.
> 
> I cannot claim it survives system suspend on the Snapdragon model,
> though.

No its fine, I'll pull that series first. "Wouldn't survive reset" 
actually meant "I can't believe this works without powering on the 
rails" but the other series does that ;)

  System suspend is still an open problem on the Surface Pro 11
> under Linux and has not been validated, so I cannot exercise that path
> here yet. What I can test is the
> runtime PM path, which is where the supplies, clock and reset are
> switched: the sensor powering off between sessions and back on for the
> next stream. I will run repeated open, stream, close cycles and report
> the result. Sergey, if suspend works on the Intel model, a suspend and
> resume test there would cover the rest.
> 
> To make the dependency visible, I can carry Sergey's patch as 1/3 of
> the next version, with his authorship. Sergey, would that be all right
> with you, or would you rather post v4 yourself?

No need

b4 prep --edit-deps   # add: message-id: 
<20260831130312.26296-1-lsa.uz@pm.me>

b4 prep; b4 send

Then dependencies should be represented and sucked in automagically.

> 
> Thanks,
> George

Re: [PATCH 1/2] dt-bindings: media: i2c: Add OmniVision OV13858

[Bryan O'Donoghue]

On 15/10/2026 22:48, George Neacsu via B4 Relay wrote:
> +  - clocks
> +  - avdd-supply
> +  - dovdd-supply
> +  - dvdd-supply

The clocks, supplies and resets should all be optional as nothing says 
Linux must control any of those on every system.

i.e. you might have a co-processor or CPLD which owns the reset, 
power-rails and the clock might not come from the SoC.

---
bod

Re: [PATCH 2/2] media: i2c: ov13858: Add device tree support

[Sergey Lebedev]

Hi Bryan, George,

On 16/10/2026 21:43, Bryan O'Donoghue wrote:
> b4 prep --edit-deps   # add: message-id:
> <20260831130312.26296-1-lsa.uz@pm.me>

Please make that v4:
https://lore.kernel.org/all/20261016210246.58562-1-lsa.uz@pm.me/

The suspend test George asked for found one thing in v3. On the Intel
model a system suspend during a stream is refused by the ISYS (intel-ipu6
driver), as isys_suspend() returns -EBUSY while a stream is open. But
DEFINE_RUNTIME_DEV_PM_OPS() gives the driver system sleep handlers, so
pm_runtime_force_suspend() powered the sensor off before that refusal
(ftrace), pm_runtime_force_resume() powered it back without the stream,
and the stream stalled until restarted. v4 has runtime PM ops only, as
camera-sensor.rst asks of sensor drivers: the same refused suspend
reaches no ov13858 callback, and the stream completes all 900 frames.

Otherwise both behave the same here: 20 runtime PM cycles each, and two
s2idle suspends with the camera idle, after which the sensor streams
4224x3136 at 29.95 fps and v4l2-compliance passes 46 of 46. A suspend
that completes during a stream, as it may with CAMSS, cannot happen here.
v4 keeps v3's Tested-by from German and Simon.

Sergey

Re: [PATCH 2/2] media: i2c: ov13858: Add device tree support

[George Neacsu]

Hi Sergey,

On 16/10/2026 22:08, Sergey Lebedev wrote:
> Please make that v4:
> https://lore.kernel.org/all/20261016210246.58562-1-lsa.uz@pm.me/

Thank you for running the suspend test, and for tracking down and
fixing the stall. The next version will declare v4 as its dependency
through b4, as Bryan suggested, and will not describe system sleep
handling for the sensor.

Bryan, on 1/2: the next version also makes the clocks, supplies and
reset optional in the binding, as you asked.

Before sending it I will rebase on v4 and run runtime PM cycles on the
Surface Pro 11.

Thanks,
George