media: ipu-bridge: IVSC camera broken by c6b1b34b5090 ("media: pci: intel: Add CVS support for IPU bridge driver")

Hi,

the internal camera on my Dell XPS 16 9640 (Meteor Lake, IPU6 + IVSC) stopped working when going from 7.1.6 to 7.2. It is still broken in 7.2.2 and, from code inspection, in 7.3-rc1.

#regzbot introduced: c6b1b34b509032c7e7cef9efc63cab55c2ad309e

Hardware
--------

Dell XPS 16 9640, BIOS 1.4.1 (04/22/2024), board 029TJ2 Intel Core Ultra 9 185H (Meteor Lake)
IPU6: 00:05.0 Multimedia controller [8086:7d19] (rev 04)
Sensor: OVTI02C1 (ov02c10) behind IVSC
IVSC ACPI device: INTC10CF:00, path \_SB_.PC00.SPFD.CVFD
Transport: INTC10D0:00 (\_SB_.PC00.SPFD) on LJCA USB bridge (8086:0b63)

Good: 7.1.6
Bad: 7.2, 7.2.2 (CachyOS builds, but the relevant code is unmodified
mainline; ov02c10 / ivsc-csi / ivsc-ace / intel-ipu6 / intel-ipu6-isys / mei-vsc-hw all have identical srcversion in 7.1.6 and 7.2.2 -- only ipu-bridge and mei-vsc changed)

Symptom
-------

No sensor subdevice is ever created:

  $ cam -l
  [..] INFO Camera camera_manager.cpp:340 libcamera v0.7.2
  [..] INFO SimplePipeline simple.cpp:1911 No sensor found for /dev/media0
  Available cameras:
  (none)

  $ ls /dev/v4l-subdev*
  (none)

  $ media-ctl -d /dev/media0 -p | grep -E '^- entity' | grep -v Capture
  - entity 193: Intel IPU6 CSI2 0 (9 pads, 8 links, 0 routes)
  - entity 203: Intel IPU6 CSI2 1 (9 pads, 8 links, 0 routes)
  [.. CSI2 2..5, no sensor entity ..]

  $ ls /sys/bus/i2c/drivers/ov02c10/
  bind  module  uevent  unbind          # no bound device

  $ ls /sys/bus/acpi/devices/OVTI02C1:00/
  cid  hid  modalias  path  power  status  subsystem  uevent  uid
                                           # status=15, no physical_node

dmesg (loglevel=3 on the console, taken from the journal):

intel-ipu6 0000:00:05.0: Found supported sensor OVTI02C1:00 intel-ipu6 0000:00:05.0: Connected 1 cameras
intel-ipu6 0000:00:05.0: IPU6-v4[7d19] hardware version 6 intel_vsc intel_vsc: silicon stepping version is 0:2 ivsc_csi intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da: mei-csi probed
without device fwnode!

Note that ipu-bridge reports success ("Connected 1 cameras"), so nothing is retried afterwards.

Analysis
--------

c6b1b34b5090 added two fallbacks to ipu_bridge_get_ivsc_csi_dev():

	/* IVSC device on platform bus */
	dev = bus_find_device(&platform_bus_type, NULL, adev,
			      ipu_bridge_match_ivsc_dev);
	if (dev) {
		snprintf(name, sizeof(name), "%s-%pUl", dev_name(dev), &uuid);
		csi_dev = device_find_child_by_name(dev, name);
		put_device(dev);
		return csi_dev;
	}

- return NULL;
+ /* Try to locate CVS device on the I2C bus */
+ csi_dev = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
+ if (csi_dev)
+ return csi_dev;
+
+ /* Fallback to platform bus for CVS device */
+ return bus_find_device_by_acpi_dev(&platform_bus_type, adev);

The fallbacks are intended for CVS (INTC10DE/INTC10E0/INTC10E1), but they are reached for every entry of ivsc_acpi_ids[], including the IVSC IDs INTC1059/INTC1095/INTC100A/INTC10CF.

On this machine the IVSC ACPI device INTC10CF:00 has two physical nodes:

/sys/bus/acpi/devices/INTC10CF:00/physical_node
-> /sys/devices/platform/INTC10CF:00 (bare ACPI platform device,
no driver bound)
/sys/bus/acpi/devices/INTC10CF:00/physical_node1
-> /sys/devices/platform/intel_vsc (the real IVSC device created
by mei_vsc, parent of the mei-csi client)

and they appear in this order relative to the IPU6 probe:

  07:59:29.297  platform INTC10CF:00 created (ACPI scan)
  07:59:41      intel-ipu6 probe -> ipu_bridge_init()
  07:59:42.391  platform intel_vsc created (mei_vsc, after the LJCA USB

bridge and its SPI controller have come up)

So at ipu_bridge_init() time:

1. bus_find_device(&platform_bus_type, .., ipu_bridge_match_ivsc_dev) returns NULL, because the match requires dev_name() == "intel_vsc" and that device does not exist yet.
2. bus_find_device_by_acpi_dev(&i2c_bus_type, adev) returns NULL.
3. the new bus_find_device_by_acpi_dev(&platform_bus_type, adev) matches the bare INTC10CF:00 platform device and returns it.

sensor->csi_dev is therefore the wrong device, and

	static int ipu_bridge_instantiate_ivsc(struct ipu_sensor *sensor)
	{
		...
		set_secondary_fwnode(sensor->csi_dev, fwnode);
	}

attaches the IVSC software node to the bare platform device instead of the mei-csi client (intel_vsc-92335fcf-...). ipu_bridge_check_ivsc_dev() then returns 0, ipu-bridge reports "Connected 1 cameras", and the probe is not retried.

When ivsc_csi probes at 07:59:52 it finds no fwnode ("mei-csi probed without device fwnode!"), so the CSI-2 link is never described. The sensor ACPI device OVTI02C1:00 has a _DEP on INTC10CF, which is honoured (acpi_honor_dep_ids[] in drivers/acpi/scan.c), so it is never enumerated, no i2c client is created, ov02c10 never probes, no v4l2 subdev is registered, and libcamera finds no sensor.

Before c6b1b34b5090, step 3 did not exist: csi_dev stayed NULL, ipu_bridge_check_ivsc_dev() returned -ENODEV, ipu_bridge_init() failed and the probe was retried later, by which time platform/intel_vsc existed and the correct mei-csi child was found. That is why 7.1.6 works.

The commit message says the patch was tested on a Dell XPS 13 9350 with IPU7 and CVS; the IVSC path does not appear to have been covered.

I could not find a fix for this in v7.3-rc1 -- the ipu-bridge changes there are additions (Himax HM1092, NVL/IPU8 IDs, DMI quirks) rather than changes to this lookup. I have not booted v7.3-rc1, so that part is from reading the code only.

Happy to test any patch or to collect further debug output.

Disclosure of tool-generated content
------------------------------------

Per Documentation/process/generated-content.rst: this report was produced with the help of an AI coding assistant (Claude Code). I asked it to find out why the camera stopped working after a distro update. It gathered the sysfs/journal/media-ctl state shown above from the affected machine, compared module srcversions between 7.1.6 and 7.2.2 to narrow down what changed, and located c6b1b34b5090 and the code path described in the Analysis section. The wording of this mail is largely its output; I have reviewed it and can reproduce and defend every claim in it on my machine.

No reproducer beyond "boot the affected machine" exists. I have built and booted a patched kernel on the affected machine; the fix is posted as a reply to this mail.

Thanks,

Since commit c6b1b34b5090 ("media: pci: intel: Add CVS support for IPU bridge driver") the internal camera no longer works on laptops where the sensor sits behind an IVSC, for example a Dell XPS 16 9640 (IPU6, INTC10CF, ov02c10):

intel-ipu6 0000:00:05.0: Found supported sensor OVTI02C1:00 intel-ipu6 0000:00:05.0: Connected 1 cameras
ivsc_csi intel_vsc-92335fcf-3203-4472-af93-7b4453ac29da: mei-csi probed
without device fwnode!

No sensor subdevice is registered, the media graph has no sensor entity and userspace finds no camera at all.

ipu_bridge_get_ivsc_csi_dev() first looks for the platform device named "intel_vsc" and returns its mei-csi child. That device is created by mei_vsc, which on this machine only appears once the LJCA USB bridge and its SPI controller have probed, about a second after the IPU6 probe that runs the bridge:

  07:59:29.297  platform INTC10CF:00 created (ACPI scan)
  07:59:41      intel-ipu6 probe -> ipu_bridge_init()
  07:59:42.391  platform intel_vsc created (mei_vsc)

The commit above added two fallbacks for CVS which match on the ACPI companion alone. They are reached for every entry of ivsc_acpi_ids[], IVSC IDs included. The IVSC ACPI device has two physical nodes:

INTC10CF:00/physical_node -> platform/INTC10CF:00 (no driver bound)
INTC10CF:00/physical_node1 -> platform/intel_vsc (mei_vsc)

so bus_find_device_by_acpi_dev(&platform_bus_type, adev) returns the bare platform device. ipu_bridge_instantiate_ivsc() then attaches the IVSC software node to that device instead of to the mei-csi client, the bridge reports success, and the probe is never retried. mei_csi later probes without a fwnode, the CSI-2 link is never described, and the sensor ACPI device, which has an honoured _DEP on the IVSC device, is never enumerated.

Before those fallbacks existed the lookup returned NULL here, the bridge failed with -ENODEV and the probe was retried once the IVSC device had shown up.

Restrict the two fallbacks to the CVS IDs. CVS binds a driver to the ACPI device itself, so matching on the companion is unambiguous there.

---
Tested on the affected machine (Dell XPS 16 9640, IPU6 + IVSC + ov02c10) on top of 7.2.2, where drivers/media/pci/intel/ipu-bridge.c is byte-identical to v7.2. Without the patch libcamera finds no camera at all; with it:

  $ cam -l
  1: Internal front camera (\_SB_.PC00.LNK1)

  $ cam -c1 --capture=5
  202.794954 (30.05 fps) cam0-stream0 seq: 000003 bytesused: 8386560
  202.828224 (30.06 fps) cam0-stream0 seq: 000004 bytesused: 8386560

The media graph gains the entities that were missing:

- entity 349: Intel IVSC CSI (2 pads, 2 links, 0 routes)
- entity 368: ov02c10 21-0036 (1 pad, 1 link, 0 routes)

and the restored retry is visible in dmesg:

pci 0000:00:05.0: deferred probe pending: intel-ipu6: IPU6 bridge init failed intel-ipu6 0000:00:05.0: Found supported sensor OVTI02C1:00 intel-ipu6 0000:00:05.0: Connected 1 cameras

"mei-csi probed without device fwnode!" is gone, ov02c10 binds to i2c-OVTI02C1:00, and eight v4l-subdev nodes appear.

This patch is against v7.3-rc1. The version tested on 7.2.2 is the same change without the INTC10FA entry, which 7.2.x does not have. Building drivers/media/pci/intel/ipu-bridge.c with W=1 produces no new warnings.

Not covered: I have no CVS hardware, so the CVS path is only reasoned about, not tested, and I have not booted v7.3-rc1 itself.

This patch was produced with the help of an AI coding assistant; see the Assisted-by tag above and Documentation/process/generated-content.rst. The bug report this replies to describes what the tool did.

 drivers/media/pci/intel/ipu-bridge.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

index 1bb3a3e98d6b..2c3b9efb0b2f 100644
@@ -232,6 +232,15 @@ static const struct acpi_device_id ivsc_acpi_ids[] = {
 	{ "INTC10FA" }, /* NVL */
 };
 
+/* The subset of ivsc_acpi_ids[] which are CVS, rather than IVSC, devices. */
+static const struct acpi_device_id cvs_acpi_ids[] = {
+	{ "INTC10DE" }, /* LNL */
+	{ "INTC10E0" }, /* ARL */
+	{ "INTC10E1" }, /* PTL */
+	{ "INTC10FA" }, /* NVL */
+	{ }
+};
+
 static struct acpi_device *ipu_bridge_get_ivsc_acpi_dev(struct acpi_device *adev)
 {
 	unsigned int i;

@@ -283,6 +292,17 @@ static struct device *ipu_bridge_get_ivsc_csi_dev(struct acpi_device *adev)
 		return csi_dev;
 	}
 
+	/*
+	 * The lookups below match on the ACPI companion alone. That is fine for
+	 * CVS, which binds a driver to that very device, but not for IVSC: there
+	 * the ACPI device also has a driverless platform device, which would be
+	 * returned instead of the mei-csi client. Return NULL for IVSC so that
+	 * the caller fails and the probe is retried once the IVSC device shows
+	 * up.
+	 */
+	if (acpi_match_device_ids(adev, cvs_acpi_ids))
+		return NULL;
+
 	/* Try to locate CVS device on the I2C bus */
 	csi_dev = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
 	if (csi_dev)

2.55.0

Hi Sergey,

Regards,

Sakari Ailus

acpi_match_device_ids() returns 0 on a match, not a boolean:

int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids)
{
return __acpi_match_device(device, ids, NULL, NULL, NULL) ?
0 : -ENOENT;
}

so the bare "if (acpi_match_device_ids(adev, cvs_acpi_ids))" is true when adev is *not* in the list, which is the IVSC case. Both spellings are in tree, e.g. drivers/acpi/scan.c:1800 uses the negated form for "matched" and drivers/acpi/x86/utils.c:206 the bare one for "did not match".

On this machine adev is INTC10CF, which is not in cvs_acpi_ids[], so the early return is taken, the IPU6 probe fails with -ENODEV and is retried once the IVSC device exists. With the polarity you read, IVSC would fall through to the lookup that returns the driverless INTC10CF:00 platform device and the camera would stay dead. It does come up, so the code behaves as the changelog describes.

That said, you had to stop and ask, which says enough about how it reads. v2 wraps the match in a named helper so the polarity is visible at the call site:

  static bool ipu_bridge_is_cvs_dev(struct acpi_device *adev)
  {
          return !acpi_match_device_ids(adev, cvs_acpi_ids);
  }

if (!ipu_bridge_is_cvs_dev(adev))

          return NULL;

No functional change, so I rebuilt it but did not boot it again; the functional test in v1 stands:

https://lore.kernel.org/linux-media/[email protected]/

Thanks for the quick review.

Hi Sergey,

Ah, I missed you used acpi_match_device_ids(), not acpi_match_acpi_device(). v1 is still better than v2 though.

Can you match IVSC IDs instead? They're more stable as I don't expect new ones are created anymore, which isn't the case for CVS.

This is ok as a fix that is easily backported, but I think parsing CVS / IVSC should be separated so we wouldn't need a CVS fallback case at all. That's not necessarily part of the same set though.

Kidn regards,

Sakari Ailus