[v1] media: ipu-bridge: do not use the CVS device lookup for IVSC

[PATCH] media: ipu-bridge: do not use the CVS device lookup for IVSC

Posted by Sergey Zagursky 3 weeks, 3 days ago

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.

Fixes: c6b1b34b5090 ("media: pci: intel: Add CVS support for IPU bridge driver")
Link: https://lore.kernel.org/linux-media/20260901194526.6369-1-gvozdoder@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Sergey Zagursky <gvozdoder@gmail.com>
---
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(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 1bb3a3e98d6b..2c3b9efb0b2f 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -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

Re: [PATCH] media: ipu-bridge: do not use the CVS device lookup for IVSC

Posted by Sakari Ailus 3 weeks, 3 days ago

Hi Sergey,

On Tue, Sep 01, 2026 at 08:50:36PM +0100, Sergey Zagursky wrote:
> 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.
> 
> Fixes: c6b1b34b5090 ("media: pci: intel: Add CVS support for IPU bridge driver")
> Link: https://lore.kernel.org/linux-media/20260901194526.6369-1-gvozdoder@gmail.com/
> Cc: stable@vger.kernel.org
> Assisted-by: Claude Code:claude-opus-5
> Signed-off-by: Sergey Zagursky <gvozdoder@gmail.com>
> ---
> 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(+)
> 
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index 1bb3a3e98d6b..2c3b9efb0b2f 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -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;

I can confirm there's indeed an issue here. But considering the list
contains the CVS device HIDs, doesn't it mean you're returning NULL here
for CVS, i.e. not for IVSC?

> +
>  	/* Try to locate CVS device on the I2C bus */
>  	csi_dev = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
>  	if (csi_dev)

-- 
Regards,

Sakari Ailus

Re: [PATCH] media: ipu-bridge: do not use the CVS device lookup for IVSC

Posted by Sergey Zagursky 3 weeks, 2 days ago

Sakari Ailus wrote:
> I can confirm there's indeed an issue here. But considering the list
> contains the CVS device HIDs, doesn't it mean you're returning NULL here
> for CVS, i.e. not for IVSC?

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/20260902145440.1786297-1-gvozdoder@gmail.com/

Thanks for the quick review.

Re: [PATCH] media: ipu-bridge: do not use the CVS device lookup for IVSC

Posted by Sakari Ailus 3 weeks, 2 days ago

Hi Sergey,

On Wed, Sep 02, 2026 at 03:57:25PM +0100, Sergey Zagursky wrote:
> Sakari Ailus wrote:
> > I can confirm there's indeed an issue here. But considering the list
> > contains the CVS device HIDs, doesn't it mean you're returning NULL here
> > for CVS, i.e. not for IVSC?
> 
> 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;
>   }

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.

> 
> 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/20260902145440.1786297-1-gvozdoder@gmail.com/
> 
> Thanks for the quick review.

-- 
Kidn regards,

Sakari Ailus