[PATCH v3 0/5] ALSA/ASoC: Conditionally skip i915 init and cleanups
A small set of changes to improve initialization of the audio stack on HDAudio devices and pair of cleanups.
As the first change is the most important one here, following is the technical background for it:
Commit 78f613ba1efb ("drm/i915: finish removal of CNL") and its friends removed support for i915 for all CNL-based platforms. HDAudio library, however, still treats such platforms as valid candidates for i915 binding. Update query mechanism to reflect changes made in drm tree.
At the same time, i915 support for LKF-based platforms has not been provided so remove them from valid binding candidates.
The snd_soc_hda change is a follow up for the above and the cleanup patches do not bring any functional changes.
Changes in v3: - snd_soc_hda_codec now returns -ENODEV on attach() if i915 is not present - denylist now const - added new patch for the avs-driver to address -ENODEV during probe_codec() - note: retained reviewed-by for patch 1/4 as changes are minimal
Changes in v2: - list of problematic VGA devices is now declared locally, no more touching drm stuff
Cezary Rojewski (5): ALSA: hda: Skip i915 initialization on CNL/LKF-based platforms ASoC: codecs: hda: Skip HDMI/DP registration if i915 is missing ASoC: Intel: avs: Ignore codecs with no suppoting driver ASoC: codecs: hda: Cleanup error messages ALSA: hda: Reuse for_each_pcm_streams()
sound/hda/hdac_i915.c | 32 +++++++++++++++++++++++++++++--- sound/pci/hda/hda_codec.c | 2 +- sound/soc/codecs/hda.c | 15 ++++++++++----- sound/soc/intel/avs/core.c | 9 +++++---- 4 files changed, 45 insertions(+), 13 deletions(-)
Commit 78f613ba1efb ("drm/i915: finish removal of CNL") and its friends removed support for i915 for all CNL-based platforms. HDAudio library, however, still treats such platforms as valid candidates for i915 binding. Update query mechanism to reflect changes made in drm tree.
At the same time, i915 support for LKF-based platforms has not been provided so remove them from valid binding candidates.
Link: https://lore.kernel.org/all/20210728215946.1573015-1-lucas.demarchi@intel.co... Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/hda/hdac_i915.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c index 365c36fdf205..e9425213320e 100644 --- a/sound/hda/hdac_i915.c +++ b/sound/hda/hdac_i915.c @@ -127,15 +127,41 @@ static int i915_component_master_match(struct device *dev, int subcomponent, /* check whether Intel graphics is present and reachable */ static int i915_gfx_present(struct pci_dev *hdac_pci) { + /* List of known platforms with no i915 support. */ + static const struct pci_device_id denylist[] = { + /* CNL */ + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a40), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a41), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a42), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a44), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a49), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a4a), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a4c), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a50), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a51), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a52), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a54), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a59), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a5a), 0x030000, 0xff0000 }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a5c), 0x030000, 0xff0000 }, + /* LKF */ + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x9840), 0x030000, 0xff0000 }, + {} + }; struct pci_dev *display_dev = NULL;
if (!gpu_bind || (gpu_bind < 0 && video_firmware_drivers_only())) return false;
for_each_pci_dev(display_dev) { - if (display_dev->vendor == PCI_VENDOR_ID_INTEL && - (display_dev->class >> 16) == PCI_BASE_CLASS_DISPLAY && - connectivity_check(display_dev, hdac_pci)) { + if (display_dev->vendor != PCI_VENDOR_ID_INTEL || + (display_dev->class >> 16) != PCI_BASE_CLASS_DISPLAY) + continue; + + if (pci_match_id(denylist, display_dev)) + continue; + + if (connectivity_check(display_dev, hdac_pci)) { pci_dev_put(display_dev); return true; }
If i915 does not support given platform but the hardware i.e.: HDAudio codec is still there, the codec-probing procedure will succeed for such device but the follow up initialization will always end up with -ENODEV.
While bus could filter out address '2' which Intel's HDMI/DP codecs always enumerate on, more robust approach is to check for i915 presence before registering display codecs.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/codecs/hda.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/soc/codecs/hda.c b/sound/soc/codecs/hda.c index d2117e36ddd1..7c6bedeceaa2 100644 --- a/sound/soc/codecs/hda.c +++ b/sound/soc/codecs/hda.c @@ -350,6 +350,11 @@ static int hda_hdev_attach(struct hdac_device *hdev) struct hda_codec *codec = dev_to_hda_codec(&hdev->dev); struct snd_soc_component_driver *comp_drv;
+ if (hda_codec_is_display(codec) && !hdev->bus->audio_component) { + dev_dbg(&hdev->dev, "no i915, skip registration for 0x%08x\n", hdev->vendor_id); + return -ENODEV; + } + comp_drv = devm_kzalloc(&hdev->dev, sizeof(*comp_drv), GFP_KERNEL); if (!comp_drv) return -ENOMEM;
On Mon, Feb 26, 2024 at 01:44:29PM +0100, Cezary Rojewski wrote:
If i915 does not support given platform but the hardware i.e.: HDAudio codec is still there, the codec-probing procedure will succeed for such device but the follow up initialization will always end up with -ENODEV.
Acked-by: Mark Brown broonie@kernel.org
HDMI codecs which are present and functional from audio perspective lack i915 support on drm side what results in -ENODEV during the probing sequence. There is no reason to perform recovery procedure e.g.: reset the HDAudio controller if this is the case.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/core.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index b3e5a5012167..a61ce42b426c 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -150,7 +150,7 @@ static int probe_codec(struct hdac_bus *bus, int addr) /* configure effectively creates new ASoC component */ ret = snd_hda_codec_configure(codec); if (ret < 0) { - dev_err(bus->dev, "failed to config codec %d\n", ret); + dev_warn(bus->dev, "failed to config codec #%d: %d\n", addr, ret); return ret; }
@@ -159,15 +159,16 @@ static int probe_codec(struct hdac_bus *bus, int addr)
static void avs_hdac_bus_probe_codecs(struct hdac_bus *bus) { - int c; + int ret, c;
/* First try to probe all given codec slots */ for (c = 0; c < HDA_MAX_CODECS; c++) { if (!(bus->codec_mask & BIT(c))) continue;
- if (!probe_codec(bus, c)) - /* success, continue probing */ + ret = probe_codec(bus, c); + /* Ignore codecs with no supporting driver. */ + if (!ret || ret == -ENODEV) continue;
/*
On Mon, Feb 26, 2024 at 01:44:30PM +0100, Cezary Rojewski wrote:
HDMI codecs which are present and functional from audio perspective lack i915 support on drm side what results in -ENODEV during the probing sequence. There is no reason to perform recovery procedure e.g.: reset the HDAudio controller if this is the case.
Acked-by: Mark Brown broonie@kernel.org
Be cohesive and use same pattern in each error message.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/codecs/hda.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/hda.c b/sound/soc/codecs/hda.c index 7c6bedeceaa2..5a58723dc0e9 100644 --- a/sound/soc/codecs/hda.c +++ b/sound/soc/codecs/hda.c @@ -198,19 +198,19 @@ static int hda_codec_probe(struct snd_soc_component *component) ret = snd_hda_codec_device_new(codec->bus, component->card->snd_card, hdev->addr, codec, false); if (ret < 0) { - dev_err(&hdev->dev, "create hda codec failed: %d\n", ret); + dev_err(&hdev->dev, "codec create failed: %d\n", ret); goto device_new_err; }
ret = snd_hda_codec_set_name(codec, codec->preset->name); if (ret < 0) { - dev_err(&hdev->dev, "name failed %s\n", codec->preset->name); + dev_err(&hdev->dev, "set name: %s failed: %d\n", codec->preset->name, ret); goto err; }
ret = snd_hdac_regmap_init(&codec->core); if (ret < 0) { - dev_err(&hdev->dev, "regmap init failed\n"); + dev_err(&hdev->dev, "regmap init failed: %d\n", ret); goto err; }
@@ -223,13 +223,13 @@ static int hda_codec_probe(struct snd_soc_component *component)
ret = patch(codec); if (ret < 0) { - dev_err(&hdev->dev, "patch failed %d\n", ret); + dev_err(&hdev->dev, "codec init failed: %d\n", ret); goto err; }
ret = snd_hda_codec_parse_pcms(codec); if (ret < 0) { - dev_err(&hdev->dev, "unable to map pcms to dai %d\n", ret); + dev_err(&hdev->dev, "unable to map pcms to dai: %d\n", ret); goto parse_pcms_err; }
On Mon, Feb 26, 2024 at 01:44:31PM +0100, Cezary Rojewski wrote:
Be cohesive and use same pattern in each error message.
Acked-by: Mark Brown broonie@kernel.org
Use the macro to improve readability.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/pci/hda/hda_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 12f02cdc9659..2cac337f5263 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3313,7 +3313,7 @@ int snd_hda_codec_parse_pcms(struct hda_codec *codec) list_for_each_entry(cpcm, &codec->pcm_list_head, list) { int stream;
- for (stream = 0; stream < 2; stream++) { + for_each_pcm_streams(stream) { struct hda_pcm_stream *info = &cpcm->stream[stream];
if (!info->substreams)
On Mon, 26 Feb 2024 13:44:27 +0100, Cezary Rojewski wrote:
A small set of changes to improve initialization of the audio stack on HDAudio devices and pair of cleanups.
As the first change is the most important one here, following is the technical background for it:
Commit 78f613ba1efb ("drm/i915: finish removal of CNL") and its friends removed support for i915 for all CNL-based platforms. HDAudio library, however, still treats such platforms as valid candidates for i915 binding. Update query mechanism to reflect changes made in drm tree.
At the same time, i915 support for LKF-based platforms has not been provided so remove them from valid binding candidates.
The snd_soc_hda change is a follow up for the above and the cleanup patches do not bring any functional changes.
Changes in v3:
- snd_soc_hda_codec now returns -ENODEV on attach() if i915 is not present
- denylist now const
- added new patch for the avs-driver to address -ENODEV during probe_codec()
- note: retained reviewed-by for patch 1/4 as changes are minimal
Changes in v2:
- list of problematic VGA devices is now declared locally, no more touching drm stuff
Cezary Rojewski (5): ALSA: hda: Skip i915 initialization on CNL/LKF-based platforms ASoC: codecs: hda: Skip HDMI/DP registration if i915 is missing ASoC: Intel: avs: Ignore codecs with no suppoting driver ASoC: codecs: hda: Cleanup error messages ALSA: hda: Reuse for_each_pcm_streams()
Applied to for-next branch now. Thanks.
Takashi
participants (3)
-
Cezary Rojewski -
Mark Brown -
Takashi Iwai