From: Takashi Iwai <tiwai@suse.de> To: "Kai-Heng Feng" <kai.heng.feng@canonical.com> Cc: Lukas Wunner <lukas@wunner.de>, <mario.limonciello@dell.com>, <pali.rohar@gmail.com>, <andy@infradead.org>, <dvhart@infradead.org>, <mjg59@srcf.ucam.org>, <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>, <platform-driver-x86@vger.kernel.org> Subject: Re: [PATCH v3 3/3] ALSA: hda: Disabled unused audio controller for Dell platforms with Switchable Graphics Date: Thu, 12 Apr 2018 12:50:02 +0200 [thread overview] Message-ID: <s5hr2nk8zs5.wl-tiwai@suse.de> (raw) In-Reply-To: <20180412104239.25584-3-kai.heng.feng@canonical.com> On Thu, 12 Apr 2018 12:42:39 +0200, Kai-Heng Feng wrote: > > Some Dell platforms (Preicsion 7510/7710/7520/7720) have a BIOS option > "Switchable Graphics" (SG). > > When SG is enabled, we have: > 00:02.0 VGA compatible controller: Intel Corporation Device 591b (rev 04) > 00:1f.3 Audio device: Intel Corporation CM238 HD Audio Controller (rev 31) > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Polaris10] > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 580] > > The Intel Audio outputs all the sound, including HDMI audio. The audio > controller comes with AMD graphics doesn't get used. > > When SG is disabled, we have: > 00:1f.3 Audio device: Intel Corporation CM238 HD Audio Controller (rev 31) > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Polaris10] > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 580] > > Now it's a typical discrete-only system. HDMI audio comes from AMD audio > controller, others from Intel audio controller. > > When SG is enabled, the unused AMD audio controller still exposes its > sysfs, so userspace still opens the control file and stream. If > userspace tries to output sound through the stream, it hangs when > runtime suspend kicks in: > [ 12.796265] snd_hda_intel 0000:01:00.1: Disabling via vga_switcheroo > [ 12.796367] snd_hda_intel 0000:01:00.1: Cannot lock devices! > > Since the discrete audio controller isn't useful when SG enabled, we > should just disable the device. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Adding Lukas to Cc. I thought we manage this better now with runtime PM by Lukas's recent patchset? thanks, Takashi > --- > v3: Simplify dell_switchable_gfx_is_enabled() by returning bool instead > of error code. > Use DMI_DEV_TYPE_OEM_STRING to match Dell System. > > v2: Mario suggested to squash the HDA part into the same series. > > sound/pci/hda/hda_intel.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index 7720e3102bcc..d9310616d5ca 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -49,6 +49,8 @@ > #include <linux/clocksource.h> > #include <linux/time.h> > #include <linux/completion.h> > +#include <linux/dell-common.h> > +#include <linux/dmi.h> > > #ifdef CONFIG_X86 > /* for snoop control */ > @@ -1629,6 +1631,38 @@ static void check_msi(struct azx *chip) > } > } > > +#if IS_ENABLED(CONFIG_DELL_LAPTOP) > +static bool check_dell_switchable_gfx(struct pci_dev *pdev) > +{ > + bool (*dell_switchable_gfx_is_enabled_func)(void); > + bool enabled; > + > + /* Only need to check for Dell laptops and AIOs */ > + if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL) || > + !(dmi_match(DMI_CHASSIS_TYPE, "10") || > + dmi_match(DMI_CHASSIS_TYPE, "13")) || > + !(pdev->vendor == PCI_VENDOR_ID_ATI || > + pdev->vendor == PCI_VENDOR_ID_NVIDIA)) > + return false; > + > + dell_switchable_gfx_is_enabled_func = > + symbol_request(dell_switchable_gfx_is_enabled); > + if (!dell_switchable_gfx_is_enabled_func) > + return false; > + > + enabled = dell_switchable_gfx_is_enabled_func(); > + > + symbol_put(dell_switchable_gfx_is_enabled); > + > + return enabled; > +} > +#else > +static bool check_dell_switchable_gfx(struct pci_dev *pdev) > +{ > + return false; > +} > +#endif > + > /* check the snoop mode availability */ > static void azx_check_snoop_available(struct azx *chip) > { > @@ -1711,6 +1745,11 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci, > if (err < 0) > return err; > > + if (check_dell_switchable_gfx(pci)) { > + pci_disable_device(pci); > + return -ENODEV; > + } > + > hda = kzalloc(sizeof(*hda), GFP_KERNEL); > if (!hda) { > pci_disable_device(pci); > -- > 2.17.0 > >
WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de> To: Kai-Heng Feng <kai.heng.feng@canonical.com> Cc: mjg59@srcf.ucam.org, alsa-devel@alsa-project.org, mario.limonciello@dell.com, linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, Lukas Wunner <lukas@wunner.de>, pali.rohar@gmail.com, dvhart@infradead.org, andy@infradead.org Subject: Re: [PATCH v3 3/3] ALSA: hda: Disabled unused audio controller for Dell platforms with Switchable Graphics Date: Thu, 12 Apr 2018 12:50:02 +0200 [thread overview] Message-ID: <s5hr2nk8zs5.wl-tiwai@suse.de> (raw) In-Reply-To: <20180412104239.25584-3-kai.heng.feng@canonical.com> On Thu, 12 Apr 2018 12:42:39 +0200, Kai-Heng Feng wrote: > > Some Dell platforms (Preicsion 7510/7710/7520/7720) have a BIOS option > "Switchable Graphics" (SG). > > When SG is enabled, we have: > 00:02.0 VGA compatible controller: Intel Corporation Device 591b (rev 04) > 00:1f.3 Audio device: Intel Corporation CM238 HD Audio Controller (rev 31) > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Polaris10] > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 580] > > The Intel Audio outputs all the sound, including HDMI audio. The audio > controller comes with AMD graphics doesn't get used. > > When SG is disabled, we have: > 00:1f.3 Audio device: Intel Corporation CM238 HD Audio Controller (rev 31) > 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Polaris10] > 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 580] > > Now it's a typical discrete-only system. HDMI audio comes from AMD audio > controller, others from Intel audio controller. > > When SG is enabled, the unused AMD audio controller still exposes its > sysfs, so userspace still opens the control file and stream. If > userspace tries to output sound through the stream, it hangs when > runtime suspend kicks in: > [ 12.796265] snd_hda_intel 0000:01:00.1: Disabling via vga_switcheroo > [ 12.796367] snd_hda_intel 0000:01:00.1: Cannot lock devices! > > Since the discrete audio controller isn't useful when SG enabled, we > should just disable the device. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Adding Lukas to Cc. I thought we manage this better now with runtime PM by Lukas's recent patchset? thanks, Takashi > --- > v3: Simplify dell_switchable_gfx_is_enabled() by returning bool instead > of error code. > Use DMI_DEV_TYPE_OEM_STRING to match Dell System. > > v2: Mario suggested to squash the HDA part into the same series. > > sound/pci/hda/hda_intel.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index 7720e3102bcc..d9310616d5ca 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -49,6 +49,8 @@ > #include <linux/clocksource.h> > #include <linux/time.h> > #include <linux/completion.h> > +#include <linux/dell-common.h> > +#include <linux/dmi.h> > > #ifdef CONFIG_X86 > /* for snoop control */ > @@ -1629,6 +1631,38 @@ static void check_msi(struct azx *chip) > } > } > > +#if IS_ENABLED(CONFIG_DELL_LAPTOP) > +static bool check_dell_switchable_gfx(struct pci_dev *pdev) > +{ > + bool (*dell_switchable_gfx_is_enabled_func)(void); > + bool enabled; > + > + /* Only need to check for Dell laptops and AIOs */ > + if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL) || > + !(dmi_match(DMI_CHASSIS_TYPE, "10") || > + dmi_match(DMI_CHASSIS_TYPE, "13")) || > + !(pdev->vendor == PCI_VENDOR_ID_ATI || > + pdev->vendor == PCI_VENDOR_ID_NVIDIA)) > + return false; > + > + dell_switchable_gfx_is_enabled_func = > + symbol_request(dell_switchable_gfx_is_enabled); > + if (!dell_switchable_gfx_is_enabled_func) > + return false; > + > + enabled = dell_switchable_gfx_is_enabled_func(); > + > + symbol_put(dell_switchable_gfx_is_enabled); > + > + return enabled; > +} > +#else > +static bool check_dell_switchable_gfx(struct pci_dev *pdev) > +{ > + return false; > +} > +#endif > + > /* check the snoop mode availability */ > static void azx_check_snoop_available(struct azx *chip) > { > @@ -1711,6 +1745,11 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci, > if (err < 0) > return err; > > + if (check_dell_switchable_gfx(pci)) { > + pci_disable_device(pci); > + return -ENODEV; > + } > + > hda = kzalloc(sizeof(*hda), GFP_KERNEL); > if (!hda) { > pci_disable_device(pci); > -- > 2.17.0 > >
next prev parent reply other threads:[~2018-04-12 10:50 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-12 10:42 [PATCH v3 1/3] dell-led: Change dell-led.h to dell-common.h Kai-Heng Feng 2018-04-12 10:42 ` [PATCH v3 2/3] platform/x86: dell-*: Add interface for switchable graphics status query Kai-Heng Feng 2018-04-12 10:42 ` Kai-Heng Feng 2018-04-12 10:42 ` [PATCH v3 3/3] ALSA: hda: Disabled unused audio controller for Dell platforms with Switchable Graphics Kai-Heng Feng 2018-04-12 10:50 ` Takashi Iwai [this message] 2018-04-12 10:50 ` Takashi Iwai 2018-04-12 10:59 ` Pali Rohár 2018-04-12 10:59 ` Pali Rohár 2018-04-12 14:15 ` Kai-Heng Feng 2018-04-12 14:15 ` Kai-Heng Feng 2018-04-13 16:08 ` Darren Hart 2018-04-14 10:45 ` Lukas Wunner 2018-04-14 10:45 ` Lukas Wunner 2018-04-14 10:49 ` Pali Rohár 2018-04-14 10:49 ` Pali Rohár 2018-04-14 11:17 ` Lukas Wunner 2018-04-14 11:17 ` Lukas Wunner 2018-04-15 17:17 ` Pali Rohár 2018-04-15 19:05 ` Lukas Wunner 2018-04-15 19:05 ` Lukas Wunner 2018-04-16 14:25 ` Pali Rohár 2018-04-16 14:25 ` Pali Rohár 2018-04-17 2:38 ` Lukas Wunner 2018-04-17 2:38 ` Lukas Wunner 2018-04-17 7:52 ` Pali Rohár 2018-04-12 14:12 ` Kai-Heng Feng 2018-04-14 11:25 ` Lukas Wunner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=s5hr2nk8zs5.wl-tiwai@suse.de \ --to=tiwai@suse.de \ --cc=alsa-devel@alsa-project.org \ --cc=andy@infradead.org \ --cc=dvhart@infradead.org \ --cc=kai.heng.feng@canonical.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lukas@wunner.de \ --cc=mario.limonciello@dell.com \ --cc=mjg59@srcf.ucam.org \ --cc=pali.rohar@gmail.com \ --cc=platform-driver-x86@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.