[PATCH] ALSA: hda/i915 - skip acomp init if no matching display

[Lucas De Marchi]

On Mon, Apr 04, 2022 at 04:03:56PM +0300, Kai Vehmanen wrote:
>In systems with only a discrete i915 GPU, the acomp init will
>always timeout for the PCH HDA controller instance.
>
>Avoid the timeout by checking the PCI device hierarchy
>whether any display class PCI device can be found on the system,
>and at the same level as the HDA PCI device. If found, proceed
>with the acomp init, which will wait until i915 probe is complete
>and component binding can proceed. If no matching display
>device is found, the audio component bind can be safely skipped.
>
>The bind timeout will still be hit if the display is present
>in the system, but i915 driver does not bind to it by configuration
>choice or probe error. In this case the 60sec timeout will be
>hit.
>
>Signed-off-by: Kai Vehmanen <kai.vehmanen at linux.intel.com>
>Acked-by: Lucas De Marchi <lucas.demarchi at intel.com>
>---
> sound/hda/hdac_i915.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
>diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
>index efe810af28c5..55b61b1a0ef9 100644
>--- a/sound/hda/hdac_i915.c
>+++ b/sound/hda/hdac_i915.c
>@@ -116,16 +116,25 @@ static int i915_component_master_match(struct device *dev, int subcomponent,
> 	return 0;
> }
>
>-/* check whether intel graphics is present */
>-static bool i915_gfx_present(void)
>+/* check whether Intel graphics is present and reachable */
>+static int i915_gfx_present(struct pci_dev *hdac_pci)
> {
>-	static const struct pci_device_id ids[] = {
>-		{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_ANY_ID),
>-		  .class = PCI_BASE_CLASS_DISPLAY << 16,
>-		  .class_mask = 0xff << 16 },
>-		{}
>-	};
>-	return pci_dev_present(ids);
>+	unsigned int class = PCI_BASE_CLASS_DISPLAY << 16;
>+	struct pci_dev *display_dev = NULL;
>+	bool match = false;
>+
>+	do {
>+		display_dev = pci_get_class(class, display_dev);
>+		if (display_dev)
>+			if (display_dev->vendor == PCI_VENDOR_ID_INTEL &&
>+			    connectivity_check(display_dev, hdac_pci))

why not put this all in a single if level?

		if (display_dev && display_dev->vendor == PCI_VENDOR_ID_INTEL &&
		    connectivity_check(display_dev, hdac_pci))

basically the first check protects dereferencing the variable.

Lucas De Marchi

>+				match = true;
>+
>+		pci_dev_put(display_dev);
>+
>+	} while (!match && display_dev);
>+
>+	return match;
> }
>
> /**
>@@ -145,7 +154,7 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
> 	struct drm_audio_component *acomp;
> 	int err;
>
>-	if (!i915_gfx_present())
>+	if (!i915_gfx_present(to_pci_dev(bus->dev)))
> 		return -ENODEV;
>
> 	err = snd_hdac_acomp_init(bus, NULL,
>
>base-commit: bfa1e1a62c8bdbe3d8c915fbb7a078dc783b2ee8
>-- 
>2.35.1
>