linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
@ 2020-10-02  5:10 Lukas Wunner
  2020-10-02 13:15 ` Deucher, Alexander
  2020-10-02 14:17 ` Rafael J. Wysocki
  0 siblings, 2 replies; 6+ messages in thread
From: Lukas Wunner @ 2020-10-02  5:10 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki, Len Brown
  Cc: Alex Deucher, Mika Westerberg, linux-pci, linux-acpi, amd-gfx,
	Arthur Borsboom, matoro, Aaron Zakhrov, Michal Rostecki,
	Shai Coleman

Recent laptops with dual AMD GPUs fail to suspend the discrete GPU, thus
causing lockups on system sleep and high power consumption at runtime.
The discrete GPU would normally be suspended to D3cold by turning off
ACPI _PR3 Power Resources of the Root Port above the GPU.

However on affected systems, the Root Port is hotplug-capable and
pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
belong to a Thunderbolt device or if the Root Port possesses a
"HotPlugSupportInD3" ACPI property.  Neither is the case on affected
laptops.  The reason for whitelisting only specific, known to work
hotplug ports for D3 is that there have been reports of SkyLake Xeon-SP
systems raising Hardware Error NMIs upon suspending their hotplug ports:
https://lore.kernel.org/linux-pci/20170503180426.GA4058@otc-nc-03/

But if a hotplug port is power manageable by ACPI (as can be detected
through presence of Power Resources and corresponding _PS0 and _PS3
methods) then it ought to be safe to suspend it to D3.  To this end,
amend acpi_pci_bridge_d3() to whitelist such ports for D3.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1222
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1252
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1304
Reported-and-tested-by: Arthur Borsboom <arthurborsboom@gmail.com>
Reported-and-tested-by: matoro <matoro@airmail.cc>
Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
Reported-by: Michal Rostecki <mrostecki@suse.com>
Reported-by: Shai Coleman <git@shaicoleman.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/pci-acpi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index d5869a0..d9aa551 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
 	if (!dev->is_hotplug_bridge)
 		return false;
 
+	/* Assume D3 support if the bridge is power-manageable by ACPI. */
+	adev = ACPI_COMPANION(&dev->dev);
+	if (!adev && !pci_dev_is_added(dev)) {
+		adev = acpi_pci_find_companion(&dev->dev);
+		ACPI_COMPANION_SET(&dev->dev, adev);
+	}
+
+	if (adev && acpi_device_power_manageable(adev))
+		return true;
+
 	/*
 	 * Look for a special _DSD property for the root port and if it
 	 * is set we know the hierarchy behind it supports D3 just fine.
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
  2020-10-02  5:10 [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI Lukas Wunner
@ 2020-10-02 13:15 ` Deucher, Alexander
  2020-10-02 14:17 ` Rafael J. Wysocki
  1 sibling, 0 replies; 6+ messages in thread
From: Deucher, Alexander @ 2020-10-02 13:15 UTC (permalink / raw)
  To: Lukas Wunner, Bjorn Helgaas, Rafael J. Wysocki, Len Brown
  Cc: Mika Westerberg, linux-pci, linux-acpi, amd-gfx, Arthur Borsboom,
	matoro, Aaron Zakhrov, Michal Rostecki, Shai Coleman

[AMD Public Use]

> -----Original Message-----
> From: Lukas Wunner <lukas@wunner.de>
> Sent: Friday, October 2, 2020 1:10 AM
> To: Bjorn Helgaas <helgaas@kernel.org>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Mika Westerberg
> <mika.westerberg@linux.intel.com>; linux-pci@vger.kernel.org; linux-
> acpi@vger.kernel.org; amd-gfx@lists.freedesktop.org; Arthur Borsboom
> <arthurborsboom@gmail.com>; matoro <matoro@airmail.cc>; Aaron
> Zakhrov <aaron.zakhrov@gmail.com>; Michal Rostecki
> <mrostecki@suse.com>; Shai Coleman <git@shaicoleman.com>
> Subject: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed
> by ACPI
> 
> Recent laptops with dual AMD GPUs fail to suspend the discrete GPU, thus
> causing lockups on system sleep and high power consumption at runtime.
> The discrete GPU would normally be suspended to D3cold by turning off
> ACPI _PR3 Power Resources of the Root Port above the GPU.
> 
> However on affected systems, the Root Port is hotplug-capable and
> pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they belong
> to a Thunderbolt device or if the Root Port possesses a
> "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> laptops.  The reason for whitelisting only specific, known to work hotplug
> ports for D3 is that there have been reports of SkyLake Xeon-SP systems
> raising Hardware Error NMIs upon suspending their hotplug ports:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Flinux-pci%2F20170503180426.GA4058%40otc-nc-
> 03%2F&amp;data=02%7C01%7Calexander.deucher%40amd.com%7C712c82d
> abd82477e540408d8669172c3%7C3dd8961fe4884e608e11a82d994e183d%7C0
> %7C0%7C637372123238938344&amp;sdata=S1%2ByqWpkuZcilrl7kRXQyodrN
> P66MAjcECRDd7tEjpE%3D&amp;reserved=0
> 
> But if a hotplug port is power manageable by ACPI (as can be detected
> through presence of Power Resources and corresponding _PS0 and _PS3
> methods) then it ought to be safe to suspend it to D3.  To this end, amend
> acpi_pci_bridge_d3() to whitelist such ports for D3.
> 
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1222&amp;data=02%7C01%7Calexander.deucher%40amd.com
> %7C712c82dabd82477e540408d8669172c3%7C3dd8961fe4884e608e11a82d99
> 4e183d%7C0%7C0%7C637372123238938344&amp;sdata=k%2FRI8kSJclwhzYuc
> WSjAZNzNEaaU9JgIYNjJsV0dAWo%3D&amp;reserved=0
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1252&amp;data=02%7C01%7Calexander.deucher%40amd.com
> %7C712c82dabd82477e540408d8669172c3%7C3dd8961fe4884e608e11a82d99
> 4e183d%7C0%7C0%7C637372123238938344&amp;sdata=JvakgHaQNI6XMaM
> eTHqtPJ5ooZP83lN%2F6wcdHyt53yA%3D&amp;reserved=0
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1304&amp;data=02%7C01%7Calexander.deucher%40amd.com
> %7C712c82dabd82477e540408d8669172c3%7C3dd8961fe4884e608e11a82d99
> 4e183d%7C0%7C0%7C637372123238938344&amp;sdata=8clLa%2BCnwH4xtK
> Dc7F9%2FqPBSMFygpjP8EiPgVIz9LiE%3D&amp;reserved=0
> Reported-and-tested-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Reported-and-tested-by: matoro <matoro@airmail.cc>
> Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
> Reported-by: Michal Rostecki <mrostecki@suse.com>
> Reported-by: Shai Coleman <git@shaicoleman.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks for sorting this out.
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/pci/pci-acpi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> d5869a0..d9aa551 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
>  	if (!dev->is_hotplug_bridge)
>  		return false;
> 
> +	/* Assume D3 support if the bridge is power-manageable by ACPI. */
> +	adev = ACPI_COMPANION(&dev->dev);
> +	if (!adev && !pci_dev_is_added(dev)) {
> +		adev = acpi_pci_find_companion(&dev->dev);
> +		ACPI_COMPANION_SET(&dev->dev, adev);
> +	}
> +
> +	if (adev && acpi_device_power_manageable(adev))
> +		return true;
> +
>  	/*
>  	 * Look for a special _DSD property for the root port and if it
>  	 * is set we know the hierarchy behind it supports D3 just fine.
> --
> 2.27.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
  2020-10-02  5:10 [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI Lukas Wunner
  2020-10-02 13:15 ` Deucher, Alexander
@ 2020-10-02 14:17 ` Rafael J. Wysocki
  2020-10-02 14:20   ` Deucher, Alexander
  1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-10-02 14:17 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Len Brown, Alex Deucher,
	Mika Westerberg, Linux PCI, ACPI Devel Maling List, amd-gfx list,
	Arthur Borsboom, matoro, Aaron Zakhrov, Michal Rostecki,
	Shai Coleman

On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner <lukas@wunner.de> wrote:
>
> Recent laptops with dual AMD GPUs fail to suspend the discrete GPU, thus
> causing lockups on system sleep and high power consumption at runtime.
> The discrete GPU would normally be suspended to D3cold by turning off
> ACPI _PR3 Power Resources of the Root Port above the GPU.
>
> However on affected systems, the Root Port is hotplug-capable and
> pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
> belong to a Thunderbolt device or if the Root Port possesses a
> "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> laptops.  The reason for whitelisting only specific, known to work
> hotplug ports for D3 is that there have been reports of SkyLake Xeon-SP
> systems raising Hardware Error NMIs upon suspending their hotplug ports:
> https://lore.kernel.org/linux-pci/20170503180426.GA4058@otc-nc-03/
>
> But if a hotplug port is power manageable by ACPI (as can be detected
> through presence of Power Resources and corresponding _PS0 and _PS3
> methods) then it ought to be safe to suspend it to D3.  To this end,
> amend acpi_pci_bridge_d3() to whitelist such ports for D3.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1222
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1252
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1304
> Reported-and-tested-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Reported-and-tested-by: matoro <matoro@airmail.cc>
> Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
> Reported-by: Michal Rostecki <mrostecki@suse.com>
> Reported-by: Shai Coleman <git@shaicoleman.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/pci-acpi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index d5869a0..d9aa551 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
>         if (!dev->is_hotplug_bridge)
>                 return false;
>
> +       /* Assume D3 support if the bridge is power-manageable by ACPI. */
> +       adev = ACPI_COMPANION(&dev->dev);
> +       if (!adev && !pci_dev_is_added(dev)) {
> +               adev = acpi_pci_find_companion(&dev->dev);
> +               ACPI_COMPANION_SET(&dev->dev, adev);
> +       }
> +
> +       if (adev && acpi_device_power_manageable(adev))
> +               return true;
> +
>         /*
>          * Look for a special _DSD property for the root port and if it
>          * is set we know the hierarchy behind it supports D3 just fine.
> --

I'm going to apply this patch for 5.10 unless Bjorn would rather route
it through the PCI tree.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
  2020-10-02 14:17 ` Rafael J. Wysocki
@ 2020-10-02 14:20   ` Deucher, Alexander
  2020-10-02 14:24     ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Deucher, Alexander @ 2020-10-02 14:20 UTC (permalink / raw)
  To: Rafael J. Wysocki, Lukas Wunner
  Cc: Aaron Zakhrov, Michal Rostecki, Linux PCI, Rafael J. Wysocki,
	amd-gfx list, ACPI Devel Maling List, Shai Coleman,
	Bjorn Helgaas, Arthur Borsboom, matoro, Mika Westerberg,
	Len Brown

[AMD Public Use]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Rafael J. Wysocki
> Sent: Friday, October 2, 2020 10:17 AM
> To: Lukas Wunner <lukas@wunner.de>
> Cc: Aaron Zakhrov <aaron.zakhrov@gmail.com>; Michal Rostecki
> <mrostecki@suse.com>; Linux PCI <linux-pci@vger.kernel.org>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>; ACPI Devel Maling List <linux-
> acpi@vger.kernel.org>; Shai Coleman <git@shaicoleman.com>; Bjorn
> Helgaas <helgaas@kernel.org>; Arthur Borsboom
> <arthurborsboom@gmail.com>; matoro <matoro@airmail.cc>; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Mika Westerberg
> <mika.westerberg@linux.intel.com>; Len Brown <lenb@kernel.org>
> Subject: Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power
> managed by ACPI
> 
> On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner <lukas@wunner.de> wrote:
> >
> > Recent laptops with dual AMD GPUs fail to suspend the discrete GPU,
> > thus causing lockups on system sleep and high power consumption at
> runtime.
> > The discrete GPU would normally be suspended to D3cold by turning off
> > ACPI _PR3 Power Resources of the Root Port above the GPU.
> >
> > However on affected systems, the Root Port is hotplug-capable and
> > pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
> > belong to a Thunderbolt device or if the Root Port possesses a
> > "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> > laptops.  The reason for whitelisting only specific, known to work
> > hotplug ports for D3 is that there have been reports of SkyLake
> > Xeon-SP systems raising Hardware Error NMIs upon suspending their
> hotplug ports:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Flinux-pci%2F20170503180426.GA4058%40otc-nc-
> 03%2F&amp;dat
> >
> a=02%7C01%7Calexander.deucher%40amd.com%7C99ec20b6d4dc410baf800
> 8d866dd
> >
> e688%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373724505855
> 84491&a
> >
> mp;sdata=EPFyxPA0MDBuAkvH7bbp2wHYnpos8p%2BoZmzlUvvdAek%3D&a
> mp;reserved
> > =0
> >
> > But if a hotplug port is power manageable by ACPI (as can be detected
> > through presence of Power Resources and corresponding _PS0 and _PS3
> > methods) then it ought to be safe to suspend it to D3.  To this end,
> > amend acpi_pci_bridge_d3() to whitelist such ports for D3.
> >
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1222&amp;data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> ata=cMj
> >
> LDIbjp8RQiWX8pgK2bDUH%2B0u3oquy3TqeT9QjZGE%3D&amp;reserved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1252&amp;data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> ata=iP9
> >
> EqNcM15Dj4Ax%2BE6e2HaMWHEX%2B0IO3cMoi0NXWGzM%3D&amp;reser
> ved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1304&amp;data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> ata=VlT
> > UV2UCH4RvKgTXZcpGOpkjZpfijmPgwtvKx6HRT04%3D&amp;reserved=0
> > Reported-and-tested-by: Arthur Borsboom <arthurborsboom@gmail.com>
> > Reported-and-tested-by: matoro <matoro@airmail.cc>
> > Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
> > Reported-by: Michal Rostecki <mrostecki@suse.com>
> > Reported-by: Shai Coleman <git@shaicoleman.com>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > Cc: stable@vger.kernel.org
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/pci/pci-acpi.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> > d5869a0..d9aa551 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev
> *dev)
> >         if (!dev->is_hotplug_bridge)
> >                 return false;
> >
> > +       /* Assume D3 support if the bridge is power-manageable by ACPI. */
> > +       adev = ACPI_COMPANION(&dev->dev);
> > +       if (!adev && !pci_dev_is_added(dev)) {
> > +               adev = acpi_pci_find_companion(&dev->dev);
> > +               ACPI_COMPANION_SET(&dev->dev, adev);
> > +       }
> > +
> > +       if (adev && acpi_device_power_manageable(adev))
> > +               return true;
> > +
> >         /*
> >          * Look for a special _DSD property for the root port and if it
> >          * is set we know the hierarchy behind it supports D3 just fine.
> > --
> 
> I'm going to apply this patch for 5.10 unless Bjorn would rather route it
> through the PCI tree.

Any chance we can get this into stable at some point as well?  It would be nice to fix the laptops out there in the wild running older kernels.

Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
  2020-10-02 14:20   ` Deucher, Alexander
@ 2020-10-02 14:24     ` Rafael J. Wysocki
  2020-10-02 14:30       ` Deucher, Alexander
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-10-02 14:24 UTC (permalink / raw)
  To: Deucher, Alexander
  Cc: Rafael J. Wysocki, Lukas Wunner, Aaron Zakhrov, Michal Rostecki,
	Linux PCI, Rafael J. Wysocki, amd-gfx list,
	ACPI Devel Maling List, Shai Coleman, Bjorn Helgaas,
	Arthur Borsboom, matoro, Mika Westerberg, Len Brown

On Fri, Oct 2, 2020 at 4:20 PM Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>
> [AMD Public Use]
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Rafael J. Wysocki
> > Sent: Friday, October 2, 2020 10:17 AM
> > To: Lukas Wunner <lukas@wunner.de>
> > Cc: Aaron Zakhrov <aaron.zakhrov@gmail.com>; Michal Rostecki
> > <mrostecki@suse.com>; Linux PCI <linux-pci@vger.kernel.org>; Rafael J.
> > Wysocki <rjw@rjwysocki.net>; amd-gfx list <amd-
> > gfx@lists.freedesktop.org>; ACPI Devel Maling List <linux-
> > acpi@vger.kernel.org>; Shai Coleman <git@shaicoleman.com>; Bjorn
> > Helgaas <helgaas@kernel.org>; Arthur Borsboom
> > <arthurborsboom@gmail.com>; matoro <matoro@airmail.cc>; Deucher,
> > Alexander <Alexander.Deucher@amd.com>; Mika Westerberg
> > <mika.westerberg@linux.intel.com>; Len Brown <lenb@kernel.org>
> > Subject: Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power
> > managed by ACPI
> >
> > On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner <lukas@wunner.de> wrote:
> > >
> > > Recent laptops with dual AMD GPUs fail to suspend the discrete GPU,
> > > thus causing lockups on system sleep and high power consumption at
> > runtime.
> > > The discrete GPU would normally be suspended to D3cold by turning off
> > > ACPI _PR3 Power Resources of the Root Port above the GPU.
> > >
> > > However on affected systems, the Root Port is hotplug-capable and
> > > pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
> > > belong to a Thunderbolt device or if the Root Port possesses a
> > > "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> > > laptops.  The reason for whitelisting only specific, known to work
> > > hotplug ports for D3 is that there have been reports of SkyLake
> > > Xeon-SP systems raising Hardware Error NMIs upon suspending their
> > hotplug ports:
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Flinux-pci%2F20170503180426.GA4058%40otc-nc-
> > 03%2F&amp;dat
> > >
> > a=02%7C01%7Calexander.deucher%40amd.com%7C99ec20b6d4dc410baf800
> > 8d866dd
> > >
> > e688%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373724505855
> > 84491&a
> > >
> > mp;sdata=EPFyxPA0MDBuAkvH7bbp2wHYnpos8p%2BoZmzlUvvdAek%3D&a
> > mp;reserved
> > > =0
> > >
> > > But if a hotplug port is power manageable by ACPI (as can be detected
> > > through presence of Power Resources and corresponding _PS0 and _PS3
> > > methods) then it ought to be safe to suspend it to D3.  To this end,
> > > amend acpi_pci_bridge_d3() to whitelist such ports for D3.
> > >
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1222&amp;data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > ata=cMj
> > >
> > LDIbjp8RQiWX8pgK2bDUH%2B0u3oquy3TqeT9QjZGE%3D&amp;reserved=0
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1252&amp;data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > ata=iP9
> > >
> > EqNcM15Dj4Ax%2BE6e2HaMWHEX%2B0IO3cMoi0NXWGzM%3D&amp;reser
> > ved=0
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1304&amp;data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > ata=VlT
> > > UV2UCH4RvKgTXZcpGOpkjZpfijmPgwtvKx6HRT04%3D&amp;reserved=0
> > > Reported-and-tested-by: Arthur Borsboom <arthurborsboom@gmail.com>
> > > Reported-and-tested-by: matoro <matoro@airmail.cc>
> > > Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
> > > Reported-by: Michal Rostecki <mrostecki@suse.com>
> > > Reported-by: Shai Coleman <git@shaicoleman.com>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > Cc: stable@vger.kernel.org
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > ---
> > >  drivers/pci/pci-acpi.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> > > d5869a0..d9aa551 100644
> > > --- a/drivers/pci/pci-acpi.c
> > > +++ b/drivers/pci/pci-acpi.c
> > > @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev
> > *dev)
> > >         if (!dev->is_hotplug_bridge)
> > >                 return false;
> > >
> > > +       /* Assume D3 support if the bridge is power-manageable by ACPI. */
> > > +       adev = ACPI_COMPANION(&dev->dev);
> > > +       if (!adev && !pci_dev_is_added(dev)) {
> > > +               adev = acpi_pci_find_companion(&dev->dev);
> > > +               ACPI_COMPANION_SET(&dev->dev, adev);
> > > +       }
> > > +
> > > +       if (adev && acpi_device_power_manageable(adev))
> > > +               return true;
> > > +
> > >         /*
> > >          * Look for a special _DSD property for the root port and if it
> > >          * is set we know the hierarchy behind it supports D3 just fine.
> > > --
> >
> > I'm going to apply this patch for 5.10 unless Bjorn would rather route it
> > through the PCI tree.
>
> Any chance we can get this into stable at some point as well?  It would be nice to fix the laptops out there in the wild running older kernels.

I can mark it for stable, but I'm not sure which releases it should go back to.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI
  2020-10-02 14:24     ` Rafael J. Wysocki
@ 2020-10-02 14:30       ` Deucher, Alexander
  0 siblings, 0 replies; 6+ messages in thread
From: Deucher, Alexander @ 2020-10-02 14:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lukas Wunner, Aaron Zakhrov, Michal Rostecki, Linux PCI,
	Rafael J. Wysocki, amd-gfx list, ACPI Devel Maling List,
	Shai Coleman, Bjorn Helgaas, Arthur Borsboom, matoro,
	Mika Westerberg, Len Brown

[AMD Public Use]



> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Friday, October 2, 2020 10:25 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Lukas Wunner
> <lukas@wunner.de>; Aaron Zakhrov <aaron.zakhrov@gmail.com>; Michal
> Rostecki <mrostecki@suse.com>; Linux PCI <linux-pci@vger.kernel.org>;
> Rafael J. Wysocki <rjw@rjwysocki.net>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>; ACPI Devel Maling List <linux-
> acpi@vger.kernel.org>; Shai Coleman <git@shaicoleman.com>; Bjorn
> Helgaas <helgaas@kernel.org>; Arthur Borsboom
> <arthurborsboom@gmail.com>; matoro <matoro@airmail.cc>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Len Brown
> <lenb@kernel.org>
> Subject: Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power
> managed by ACPI
> 
> On Fri, Oct 2, 2020 at 4:20 PM Deucher, Alexander
> <Alexander.Deucher@amd.com> wrote:
> >
> > [AMD Public Use]
> >
> > > -----Original Message-----
> > > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > > Rafael J. Wysocki
> > > Sent: Friday, October 2, 2020 10:17 AM
> > > To: Lukas Wunner <lukas@wunner.de>
> > > Cc: Aaron Zakhrov <aaron.zakhrov@gmail.com>; Michal Rostecki
> > > <mrostecki@suse.com>; Linux PCI <linux-pci@vger.kernel.org>; Rafael J.
> > > Wysocki <rjw@rjwysocki.net>; amd-gfx list <amd-
> > > gfx@lists.freedesktop.org>; ACPI Devel Maling List <linux-
> > > acpi@vger.kernel.org>; Shai Coleman <git@shaicoleman.com>; Bjorn
> > > Helgaas <helgaas@kernel.org>; Arthur Borsboom
> > > <arthurborsboom@gmail.com>; matoro <matoro@airmail.cc>; Deucher,
> > > Alexander <Alexander.Deucher@amd.com>; Mika Westerberg
> > > <mika.westerberg@linux.intel.com>; Len Brown <lenb@kernel.org>
> > > Subject: Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if
> > > power managed by ACPI
> > >
> > > On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner <lukas@wunner.de> wrote:
> > > >
> > > > Recent laptops with dual AMD GPUs fail to suspend the discrete
> > > > GPU, thus causing lockups on system sleep and high power
> > > > consumption at
> > > runtime.
> > > > The discrete GPU would normally be suspended to D3cold by turning
> > > > off ACPI _PR3 Power Resources of the Root Port above the GPU.
> > > >
> > > > However on affected systems, the Root Port is hotplug-capable and
> > > > pci_bridge_d3_possible() only allows hotplug ports to go to D3 if
> > > > they belong to a Thunderbolt device or if the Root Port possesses
> > > > a "HotPlugSupportInD3" ACPI property.  Neither is the case on
> > > > affected laptops.  The reason for whitelisting only specific,
> > > > known to work hotplug ports for D3 is that there have been reports
> > > > of SkyLake Xeon-SP systems raising Hardware Error NMIs upon
> > > > suspending their
> > > hotplug ports:
> > > >
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re
> > > > .kernel.org%2Flinux-pci%2F20170503180426.GA4058%40otc-nc-
> > > 03%2F&amp;dat
> > > >
> > >
> a=02%7C01%7Calexander.deucher%40amd.com%7C99ec20b6d4dc410baf800
> > > 8d866dd
> > > >
> > >
> e688%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373724505855
> > > 84491&a
> > > >
> > >
> mp;sdata=EPFyxPA0MDBuAkvH7bbp2wHYnpos8p%2BoZmzlUvvdAek%3D&a
> > > mp;reserved
> > > > =0
> > > >
> > > > But if a hotplug port is power manageable by ACPI (as can be
> > > > detected through presence of Power Resources and corresponding
> > > > _PS0 and _PS3
> > > > methods) then it ought to be safe to suspend it to D3.  To this
> > > > end, amend acpi_pci_bridge_d3() to whitelist such ports for D3.
> > > >
> > > > Link:
> > > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > gitl
> > > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > > %2Fissues%2F1222&amp;data=02%7C01%7C
> > > >
> > >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > > 7C3dd89
> > > >
> > >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > > ata=cMj
> > > >
> > >
> LDIbjp8RQiWX8pgK2bDUH%2B0u3oquy3TqeT9QjZGE%3D&amp;reserved=0
> > > > Link:
> > > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > gitl
> > > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > > %2Fissues%2F1252&amp;data=02%7C01%7C
> > > >
> > >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > > 7C3dd89
> > > >
> > >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > > ata=iP9
> > > >
> > >
> EqNcM15Dj4Ax%2BE6e2HaMWHEX%2B0IO3cMoi0NXWGzM%3D&amp;reser
> > > ved=0
> > > > Link:
> > > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > gitl
> > > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > > %2Fissues%2F1304&amp;data=02%7C01%7C
> > > >
> > >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > > 7C3dd89
> > > >
> > >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&amp;sd
> > > ata=VlT
> > > >
> UV2UCH4RvKgTXZcpGOpkjZpfijmPgwtvKx6HRT04%3D&amp;reserved=0
> > > > Reported-and-tested-by: Arthur Borsboom
> <arthurborsboom@gmail.com>
> > > > Reported-and-tested-by: matoro <matoro@airmail.cc>
> > > > Reported-by: Aaron Zakhrov <aaron.zakhrov@gmail.com>
> > > > Reported-by: Michal Rostecki <mrostecki@suse.com>
> > > > Reported-by: Shai Coleman <git@shaicoleman.com>
> > > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > > Cc: stable@vger.kernel.org
> > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > > ---
> > > >  drivers/pci/pci-acpi.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> > > > d5869a0..d9aa551 100644
> > > > --- a/drivers/pci/pci-acpi.c
> > > > +++ b/drivers/pci/pci-acpi.c
> > > > @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev
> > > *dev)
> > > >         if (!dev->is_hotplug_bridge)
> > > >                 return false;
> > > >
> > > > +       /* Assume D3 support if the bridge is power-manageable by ACPI.
> */
> > > > +       adev = ACPI_COMPANION(&dev->dev);
> > > > +       if (!adev && !pci_dev_is_added(dev)) {
> > > > +               adev = acpi_pci_find_companion(&dev->dev);
> > > > +               ACPI_COMPANION_SET(&dev->dev, adev);
> > > > +       }
> > > > +
> > > > +       if (adev && acpi_device_power_manageable(adev))
> > > > +               return true;
> > > > +
> > > >         /*
> > > >          * Look for a special _DSD property for the root port and if it
> > > >          * is set we know the hierarchy behind it supports D3 just fine.
> > > > --
> > >
> > > I'm going to apply this patch for 5.10 unless Bjorn would rather
> > > route it through the PCI tree.
> >
> > Any chance we can get this into stable at some point as well?  It would be
> nice to fix the laptops out there in the wild running older kernels.
> 
> I can mark it for stable, but I'm not sure which releases it should go back to.

5.4 is the oldest kernel that supports the GPUs on this platform.

Alex

> 
> Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-02 14:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02  5:10 [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI Lukas Wunner
2020-10-02 13:15 ` Deucher, Alexander
2020-10-02 14:17 ` Rafael J. Wysocki
2020-10-02 14:20   ` Deucher, Alexander
2020-10-02 14:24     ` Rafael J. Wysocki
2020-10-02 14:30       ` Deucher, Alexander

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).