ALSA: hda - Enable runtime PM only for discrete GPU

The recent change of vga_switcheroo allowed the runtime PM for
HD-audio on AMD GPUs, but this also resulted in a regression.  When
the HD-audio controller driver gets runtime-suspended, HD-audio link
is turned off, and the hotplug notification is ignored.  This leads to
the inconsistent audio state (the connection isn't notified and ELD is
ignored).

The best fix would be to implement the proper ELD notification via the
audio component, but it's still not ready.  As a quick workaround,
this patch adds the check of runtime_idle and allows the runtime
suspend only when the vga_switcheroo is bound with discrete GPU.
That is, a system with a single GPU and APU would be again without
runtime PM to keep the HD-audio link for the hotplug notification and
ELD read out.

Also, the codec->auto_runtime_pm flag is set only for the discrete GPU
at the time GPU gets bound via vga_switcheroo (i.e. only dGPU is
forcibly runtime-PM enabled), so that APU can still get the ELD
notification.

For identifying which GPU is bound, a new vga_switcheroo client
callback, gpu_bound, is implemented.  The vga_switcheroo simply calls
this when GPU is bound, and tells whether it's dGPU or APU.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200945
Fixes: 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
Reported-by: Jian-Hong Pan <jian-hong@endlessm.com>
Tested-by: Jian-Hong Pan <jian-hong@endlessm.com>
Acked-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index a96bf46bc483fa42e2cd2e185a345ed24356b606..cf2a18571d484d078dc1eabc59a3d6ff0f11ab07 100644 (file)
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -215,6 +215,8 @@ static void vga_switcheroo_enable(void)
                        return;
 
                client->id = ret | ID_BIT_AUDIO;
+               if (client->ops->gpu_bound)
+                       client->ops->gpu_bound(client->pdev, ret);
        }
 
        vga_switcheroo_debugfs_init(&vgasr_priv);

diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index a34539b7f7508e1a7ae37580823b44858d66ea84..7e6ac0114d556bda8580fba760e223fb57e7fd14 100644 (file)
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -133,15 +133,18 @@ struct vga_switcheroo_handler {
  * @can_switch: check if the device is in a position to switch now.
  *     Mandatory. The client should return false if a user space process
  *     has one of its device files open
+ * @gpu_bound: notify the client id to audio client when the GPU is bound.
  *
  * Client callbacks. A client can be either a GPU or an audio device on a GPU.
  * The @set_gpu_state and @can_switch methods are mandatory, @reprobe may be
  * set to NULL. For audio clients, the @reprobe member is bogus.
+ * OTOH, @gpu_bound is only for audio clients, and not used for GPU clients.
  */
 struct vga_switcheroo_client_ops {
        void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state);
        void (*reprobe)(struct pci_dev *dev);
        bool (*can_switch)(struct pci_dev *dev);
+       void (*gpu_bound)(struct pci_dev *dev, enum vga_switcheroo_client_id);
 };
 
 #if defined(CONFIG_VGA_SWITCHEROO)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 1b2ce304152a65c52ce60322dbeec684965cf335..aa4c672dbaf73010aa26966173c403e6e8b0f4a7 100644 (file)
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -365,8 +365,10 @@ enum {
  */
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)       ((chip)->use_vga_switcheroo)
+#define needs_eld_notify_link(chip)    ((chip)->need_eld_notify_link)
 #else
 #define use_vga_switcheroo(chip)       0
+#define needs_eld_notify_link(chip)    false
 #endif
 
 #define CONTROLLER_IN_GPU(pci) (((pci)->device == 0x0a0c) || \
@@ -453,6 +455,7 @@ static inline void mark_runtime_wc(struct azx *chip, struct azx_dev *azx_dev,
 #endif
 
 static int azx_acquire_irq(struct azx *chip, int do_disconnect);
+static void set_default_power_save(struct azx *chip);
 
 /*
  * initialize the PCI registers
@@ -1201,6 +1204,10 @@ static int azx_runtime_idle(struct device *dev)
            azx_bus(chip)->codec_powered || !chip->running)
                return -EBUSY;
 
+       /* ELD notification gets broken when HD-audio bus is off */
+       if (needs_eld_notify_link(hda))
+               return -EBUSY;
+
        return 0;
 }
 
@@ -1298,6 +1305,36 @@ static bool azx_vs_can_switch(struct pci_dev *pci)
        return true;
 }
 
+/*
+ * The discrete GPU cannot power down unless the HDA controller runtime
+ * suspends, so activate runtime PM on codecs even if power_save == 0.
+ */
+static void setup_vga_switcheroo_runtime_pm(struct azx *chip)
+{
+       struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
+       struct hda_codec *codec;
+
+       if (hda->use_vga_switcheroo && !hda->need_eld_notify_link) {
+               list_for_each_codec(codec, &chip->bus)
+                       codec->auto_runtime_pm = 1;
+               /* reset the power save setup */
+               if (chip->running)
+                       set_default_power_save(chip);
+       }
+}
+
+static void azx_vs_gpu_bound(struct pci_dev *pci,
+                            enum vga_switcheroo_client_id client_id)
+{
+       struct snd_card *card = pci_get_drvdata(pci);
+       struct azx *chip = card->private_data;
+       struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
+
+       if (client_id == VGA_SWITCHEROO_DIS)
+               hda->need_eld_notify_link = 0;
+       setup_vga_switcheroo_runtime_pm(chip);
+}
+
 static void init_vga_switcheroo(struct azx *chip)
 {
        struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
@@ -1306,6 +1343,7 @@ static void init_vga_switcheroo(struct azx *chip)
                dev_info(chip->card->dev,
                         "Handle vga_switcheroo audio client\n");
                hda->use_vga_switcheroo = 1;
+               hda->need_eld_notify_link = 1; /* cleared in gpu_bound op */
                chip->driver_caps |= AZX_DCAPS_PM_RUNTIME;
                pci_dev_put(p);
        }
@@ -1314,6 +1352,7 @@ static void init_vga_switcheroo(struct azx *chip)
 static const struct vga_switcheroo_client_ops azx_vs_ops = {
        .set_gpu_state = azx_vs_set_state,
        .can_switch = azx_vs_can_switch,
+       .gpu_bound = azx_vs_gpu_bound,
 };
 
 static int register_vga_switcheroo(struct azx *chip)
@@ -1339,6 +1378,7 @@ static int register_vga_switcheroo(struct azx *chip)
 #define init_vga_switcheroo(chip)              /* NOP */
 #define register_vga_switcheroo(chip)          0
 #define check_hdmi_disabled(pci)       false
+#define setup_vga_switcheroo_runtime_pm(chip)  /* NOP */
 #endif /* SUPPORT_VGA_SWITCHER */
 
 /*
@@ -1352,6 +1392,7 @@ static int azx_free(struct azx *chip)
 
        if (azx_has_pm_runtime(chip) && chip->running)
                pm_runtime_get_noresume(&pci->dev);
+       chip->running = 0;
 
        azx_del_card_list(chip);
 
@@ -2230,6 +2271,25 @@ static struct snd_pci_quirk power_save_blacklist[] = {
 };
 #endif /* CONFIG_PM */
 
+static void set_default_power_save(struct azx *chip)
+{
+       int val = power_save;
+
+#ifdef CONFIG_PM
+       if (pm_blacklist) {
+               const struct snd_pci_quirk *q;
+
+               q = snd_pci_quirk_lookup(chip->pci, power_save_blacklist);
+               if (q && val) {
+                       dev_info(chip->card->dev, "device %04x:%04x is on the power_save blacklist, forcing power_save to 0\n",
+                                q->subvendor, q->subdevice);
+                       val = 0;
+               }
+       }
+#endif /* CONFIG_PM */
+       snd_hda_set_power_save(&chip->bus, val * 1000);
+}
+
 /* number of codec slots for each chipset: 0 = default slots (i.e. 4) */
 static unsigned int azx_max_codecs[AZX_NUM_DRIVERS] = {
        [AZX_DRIVER_NVIDIA] = 8,
@@ -2241,9 +2301,7 @@ static int azx_probe_continue(struct azx *chip)
        struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
        struct hdac_bus *bus = azx_bus(chip);
        struct pci_dev *pci = chip->pci;
-       struct hda_codec *codec;
        int dev = chip->dev_index;
-       int val;
        int err;
 
        hda->probe_continued = 1;
@@ -2322,31 +2380,13 @@ static int azx_probe_continue(struct azx *chip)
        if (err < 0)
                goto out_free;
 
+       setup_vga_switcheroo_runtime_pm(chip);
+
        chip->running = 1;
        azx_add_card_list(chip);
 
-       val = power_save;
-#ifdef CONFIG_PM
-       if (pm_blacklist) {
-               const struct snd_pci_quirk *q;
-
-               q = snd_pci_quirk_lookup(chip->pci, power_save_blacklist);
-               if (q && val) {
-                       dev_info(chip->card->dev, "device %04x:%04x is on the power_save blacklist, forcing power_save to 0\n",
-                                q->subvendor, q->subdevice);
-                       val = 0;
-               }
-       }
-#endif /* CONFIG_PM */
-       /*
-        * The discrete GPU cannot power down unless the HDA controller runtime
-        * suspends, so activate runtime PM on codecs even if power_save == 0.
-        */
-       if (use_vga_switcheroo(hda))
-               list_for_each_codec(codec, &chip->bus)
-                       codec->auto_runtime_pm = 1;
+       set_default_power_save(chip);
 
-       snd_hda_set_power_save(&chip->bus, val * 1000);
        if (azx_has_pm_runtime(chip))
                pm_runtime_put_autosuspend(&pci->dev);
 

diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
index e3a3d318d2e5f9fc57dbe9530a2470ab955b1765..f59719e06b91a38492957195dfda7328edeb80db 100644 (file)
--- a/sound/pci/hda/hda_intel.h
+++ b/sound/pci/hda/hda_intel.h
@@ -37,6 +37,7 @@ struct hda_intel {
 
        /* vga_switcheroo setup */
        unsigned int use_vga_switcheroo:1;
+       unsigned int need_eld_notify_link:1;
        unsigned int vga_switcheroo_registered:1;
        unsigned int init_failed:1; /* delayed init failed */