[alsa-devel] [PATCH v2] 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") Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/gpu/vga/vga_switcheroo.c | 2 + include/linux/vga_switcheroo.h | 3 ++ sound/pci/hda/hda_intel.c | 86 +++++++++++++++++++++++--------- sound/pci/hda/hda_intel.h | 1 + 4 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index a96bf46bc483..cf2a18571d48 100644 --- 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 a34539b7f750..7e6ac0114d55 100644 --- 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 1b2ce304152a..aa4c672dbaf7 100644 --- 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 e3a3d318d2e5..f59719e06b91 100644 --- 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 */
2018-09-13 14:56 GMT+08:00 Takashi Iwai tiwai@suse.de:
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.
Tested-by: Jian-Hong Pan jian-hong@endlessm.com
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200945 Fixes: 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") Signed-off-by: Takashi Iwai tiwai@suse.de
drivers/gpu/vga/vga_switcheroo.c | 2 + include/linux/vga_switcheroo.h | 3 ++ sound/pci/hda/hda_intel.c | 86 +++++++++++++++++++++++--------- sound/pci/hda/hda_intel.h | 1 + 4 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index a96bf46bc483..cf2a18571d48 100644 --- 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 a34539b7f750..7e6ac0114d55 100644 --- 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 processhas 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 1b2ce304152a..aa4c672dbaf7 100644 --- 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 e3a3d318d2e5..f59719e06b91 100644 --- 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 */-- 2.18.0
On Thu, 13 Sep 2018 15:27:16 +0200, Jian-Hong Pan wrote:
2018-09-13 14:56 GMT+08:00 Takashi Iwai tiwai@suse.de:
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.
Tested-by: Jian-Hong Pan jian-hong@endlessm.com
Thanks. I should put you also as the original reporter.
Lukas, could you check the vga_switcheroo change whether it's acceptable?
Takashi
On Thu, Sep 13, 2018 at 03:35:36PM +0200, Takashi Iwai wrote:
On Thu, 13 Sep 2018 15:27:16 +0200, Jian-Hong Pan wrote:
2018-09-13 14:56 GMT+08:00 Takashi Iwai tiwai@suse.de:
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.
Tested-by: Jian-Hong Pan jian-hong@endlessm.com
Thanks. I should put you also as the original reporter.
Lukas, could you check the vga_switcheroo change whether it's acceptable?
I looked over it this morning and didn't get every little detail, e.g. why chip->running = 0 in azx_free() (seems like an unrelated change, but I'm not sure). But I do understand the idea underlying the patch and it LGTM. It's somewhat regrettable that we have to make things more complicated again to properly differentiate between integrated and discrete GPUs. I wish we could later on come up with a scheme that is lightweight and easy to follow. Obviously, such a scheme will likely not be eligible as a fix for 4.19. Anyway,
Acked-by: Lukas Wunner lukas@wunner.de
and thanks a lot for looking into it. Please cc dri-devel for vga_switcheroo-related patches in the future (now added, + Alex, links for context: https://patchwork.kernel.org/patch/10598735/ https://bugzilla.kernel.org/show_bug.cgi?id=200945 ).
Lukas
On Thu, 13 Sep 2018 15:58:13 +0200, Lukas Wunner wrote:
On Thu, Sep 13, 2018 at 03:35:36PM +0200, Takashi Iwai wrote:
On Thu, 13 Sep 2018 15:27:16 +0200, Jian-Hong Pan wrote:
2018-09-13 14:56 GMT+08:00 Takashi Iwai tiwai@suse.de:
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.
Tested-by: Jian-Hong Pan jian-hong@endlessm.com
Thanks. I should put you also as the original reporter.
Lukas, could you check the vga_switcheroo change whether it's acceptable?
I looked over it this morning and didn't get every little detail, e.g. why chip->running = 0 in azx_free() (seems like an unrelated change, but I'm not sure).
It's to avoid the possible call of set_default_power_save() in azx_vs_gpu_bound() call, i.e. when a GPU is bound during the audio driver is being deregistered. It matters just theoretically.
But I do understand the idea underlying the patch and it LGTM. It's somewhat regrettable that we have to make things more complicated again to properly differentiate between integrated and discrete GPUs. I wish we could later on come up with a scheme that is lightweight and easy to follow. Obviously, such a scheme will likely not be eligible as a fix for 4.19. Anyway,
Acked-by: Lukas Wunner lukas@wunner.de
and thanks a lot for looking into it. Please cc dri-devel for vga_switcheroo-related patches in the future (now added, + Alex, links for context: https://patchwork.kernel.org/patch/10598735/ https://bugzilla.kernel.org/show_bug.cgi?id=200945 ).
Oh right, I seem to have forgotten Cc, sorry for that.
thanks,
Takashi
participants (3)
-
Jian-Hong Pan -
Lukas Wunner -
Takashi Iwai