ALSA: hda: intel: Allow repeatedly probing on codec configuration errors

It seems that a few recent AMD systems show the codec configuration
errors at the early boot, while loading the driver at a later stage
works magically.  Although the root cause of the error isn't clear,
it's certainly not bad to allow retrying the codec probe in such a
case if that helps.

This patch adds the capability for retrying the probe upon codec probe
errors on the certain AMD platforms.  The probe_work is changed to a
delayed work, and at the secondary call, it'll jump to the codec
probing.

Note that, not only adding the re-probing, this includes the behavior
changes in the codec configuration function.  Namely,
snd_hda_codec_configure() won't unregister the codec at errors any
longer.  Instead, its caller, azx_codec_configure() unregisters the
codecs with the probe failures *if* any codec has been successfully
configured.  If all codec probe failed, it doesn't unregister but let
it re-probed -- which is the most case we're seeing and this patch
tries to improve.

Even if the driver doesn't re-probe or give up, it will go to the
"free-all" error path, hence the leftover codecs shall be disabled /
deleted in anyway.

BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1190801
Link: https://lore.kernel.org/r/20211006141940.2897-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 01570dbda503850a0a7eb5fb2da4cc3d806401e1..0e45963bb767f767f85eeb59e8bc6dc8603959a2 100644 (file)
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -224,6 +224,7 @@ struct hda_codec {
 #endif
 
        /* misc flags */
+       unsigned int configured:1; /* codec was configured */
        unsigned int in_freeing:1; /* being released */
        unsigned int registered:1; /* codec was registered */
        unsigned int display_power_control:1; /* needs display power */

diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
index 2523b23389e97dba45ff600b88535c47c6b13d56..1c8bffc3eec6e674b36065bf447400db71aa0835 100644 (file)
--- a/sound/pci/hda/hda_bind.c
+++ b/sound/pci/hda/hda_bind.c
@@ -298,29 +298,31 @@ int snd_hda_codec_configure(struct hda_codec *codec)
 {
        int err;
 
+       if (codec->configured)
+               return 0;
+
        if (is_generic_config(codec))
                codec->probe_id = HDA_CODEC_ID_GENERIC;
        else
                codec->probe_id = 0;
 
-       err = snd_hdac_device_register(&codec->core);
-       if (err < 0)
-               return err;
+       if (!device_is_registered(&codec->core.dev)) {
+               err = snd_hdac_device_register(&codec->core);
+               if (err < 0)
+                       return err;
+       }
 
        if (!codec->preset)
                codec_bind_module(codec);
        if (!codec->preset) {
                err = codec_bind_generic(codec);
                if (err < 0) {
-                       codec_err(codec, "Unable to bind the codec\n");
-                       goto error;
+                       codec_dbg(codec, "Unable to bind the codec\n");
+                       return err;
                }
        }
 
+       codec->configured = 1;
        return 0;
-
- error:
-       snd_hdac_device_unregister(&codec->core);
-       return err;
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_configure);

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index a9ebefd60cf68a0e773af8fdad276226566456ea..0c4a337c9fc0d266db5afd4d37210a33c889e61e 100644 (file)
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -791,6 +791,7 @@ void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
        snd_array_free(&codec->nids);
        remove_conn_list(codec);
        snd_hdac_regmap_exit(&codec->core);
+       codec->configured = 0;
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_cleanup_for_unbind);
 

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 7cd452831fd30956fcf6fff20e989f684b94fe95..930ae4002a8187addcc97defc7b75573c804fd88 100644 (file)
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -25,6 +25,7 @@
 #include <sound/core.h>
 #include <sound/initval.h>
 #include "hda_controller.h"
+#include "hda_local.h"
 
 #define CREATE_TRACE_POINTS
 #include "hda_controller_trace.h"
@@ -1248,17 +1249,24 @@ EXPORT_SYMBOL_GPL(azx_probe_codecs);
 int azx_codec_configure(struct azx *chip)
 {
        struct hda_codec *codec, *next;
+       int success = 0;
 
-       /* use _safe version here since snd_hda_codec_configure() deregisters
-        * the device upon error and deletes itself from the bus list.
-        */
-       list_for_each_codec_safe(codec, next, &chip->bus) {
-               snd_hda_codec_configure(codec);
+       list_for_each_codec(codec, &chip->bus) {
+               if (!snd_hda_codec_configure(codec))
+                       success++;
        }
 
-       if (!azx_bus(chip)->num_codecs)
-               return -ENODEV;
-       return 0;
+       if (success) {
+               /* unregister failed codecs if any codec has been probed */
+               list_for_each_codec_safe(codec, next, &chip->bus) {
+                       if (!codec->configured) {
+                               codec_err(codec, "Unable to configure, disabling\n");
+                               snd_hdac_device_unregister(&codec->core);
+                       }
+               }
+       }
+
+       return success ? 0 : -ENODEV;
 }
 EXPORT_SYMBOL_GPL(azx_codec_configure);
 

diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index 3062f87380b1b41f5f273f4c26b2ceda0aed3575..f5bf295eb83078d2586a5985aaa1af4eb2840116 100644 (file)
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -41,7 +41,7 @@
 /* 24 unused */
 #define AZX_DCAPS_COUNT_LPIB_DELAY  (1 << 25)  /* Take LPIB as delay */
 #define AZX_DCAPS_PM_RUNTIME   (1 << 26)       /* runtime PM support */
-/* 27 unused */
+#define AZX_DCAPS_RETRY_PROBE  (1 << 27)       /* retry probe if no codec is configured */
 #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)  /* CORBRP clears itself after reset */
 #define AZX_DCAPS_NO_MSI64      (1 << 29)      /* Stick to 32-bit MSIs */
 #define AZX_DCAPS_SEPARATE_STREAM_TAG  (1 << 30) /* capture and playback use separate stream tag */

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 47777439961c36320f194c6dcc5c1e31f20b6d65..4d22e7adeee8ecf11006c951f18ed10cf8e220f9 100644 (file)
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -307,7 +307,8 @@ enum {
 /* quirks for AMD SB */
 #define AZX_DCAPS_PRESET_AMD_SB \
        (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_AMD_WORKAROUND |\
-        AZX_DCAPS_SNOOP_TYPE(ATI) | AZX_DCAPS_PM_RUNTIME)
+        AZX_DCAPS_SNOOP_TYPE(ATI) | AZX_DCAPS_PM_RUNTIME |\
+        AZX_DCAPS_RETRY_PROBE)
 
 /* quirks for Nvidia */
 #define AZX_DCAPS_PRESET_NVIDIA \
@@ -1723,7 +1724,7 @@ static void azx_check_snoop_available(struct azx *chip)
 
 static void azx_probe_work(struct work_struct *work)
 {
-       struct hda_intel *hda = container_of(work, struct hda_intel, probe_work);
+       struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
        azx_probe_continue(&hda->chip);
 }
 
@@ -1828,7 +1829,7 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
        }
 
        /* continue probing in work context as may trigger request module */
-       INIT_WORK(&hda->probe_work, azx_probe_work);
+       INIT_DELAYED_WORK(&hda->probe_work, azx_probe_work);
 
        *rchip = chip;
 
@@ -2142,7 +2143,7 @@ static int azx_probe(struct pci_dev *pci,
 #endif
 
        if (schedule_probe)
-               schedule_work(&hda->probe_work);
+               schedule_delayed_work(&hda->probe_work, 0);
 
        dev++;
        if (chip->disabled)
@@ -2228,6 +2229,11 @@ static int azx_probe_continue(struct azx *chip)
        int dev = chip->dev_index;
        int err;
 
+       if (chip->disabled || hda->init_failed)
+               return -EIO;
+       if (hda->probe_retry)
+               goto probe_retry;
+
        to_hda_bus(bus)->bus_probing = 1;
        hda->probe_continued = 1;
 
@@ -2289,10 +2295,20 @@ static int azx_probe_continue(struct azx *chip)
 #endif
        }
 #endif
+
+ probe_retry:
        if (bus->codec_mask && !(probe_only[dev] & 1)) {
                err = azx_codec_configure(chip);
-               if (err < 0)
+               if (err) {
+                       if ((chip->driver_caps & AZX_DCAPS_RETRY_PROBE) &&
+                           ++hda->probe_retry < 60) {
+                               schedule_delayed_work(&hda->probe_work,
+                                                     msecs_to_jiffies(1000));
+                               return 0; /* keep things up */
+                       }
+                       dev_err(chip->card->dev, "Cannot probe codecs, giving up\n");
                        goto out_free;
+               }
        }
 
        err = snd_card_register(chip->card);
@@ -2322,6 +2338,7 @@ out_free:
                display_power(chip, false);
        complete_all(&hda->probe_wait);
        to_hda_bus(bus)->bus_probing = 0;
+       hda->probe_retry = 0;
        return 0;
 }
 
@@ -2347,7 +2364,7 @@ static void azx_remove(struct pci_dev *pci)
                 * device during cancel_work_sync() call.
                 */
                device_unlock(&pci->dev);
-               cancel_work_sync(&hda->probe_work);
+               cancel_delayed_work_sync(&hda->probe_work);
                device_lock(&pci->dev);
 
                snd_card_free(card);

diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
index 3fb119f090408865e2a380292807469b9bd8d369..0f39418f9328b08e7c34c2db540eed30f734100c 100644 (file)
--- a/sound/pci/hda/hda_intel.h
+++ b/sound/pci/hda/hda_intel.h
@@ -14,7 +14,7 @@ struct hda_intel {
 
        /* sync probing */
        struct completion probe_wait;
-       struct work_struct probe_work;
+       struct delayed_work probe_work;
 
        /* card list (for power_save trigger) */
        struct list_head list;
@@ -30,6 +30,8 @@ struct hda_intel {
        unsigned int freed:1; /* resources already released */
 
        bool need_i915_power:1; /* the hda controller needs i915 power */
+
+       int probe_retry;        /* being probe-retry */
 };
 
 #endif