[PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI

[Deucher, Alexander]

[AMD Public Use]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> Rafael J. Wysocki
> Sent: Friday, October 2, 2020 10:17 AM
> To: Lukas Wunner <lukas at wunner.de>
> Cc: Aaron Zakhrov <aaron.zakhrov at gmail.com>; Michal Rostecki
> <mrostecki at suse.com>; Linux PCI <linux-pci at vger.kernel.org>; Rafael J.
> Wysocki <rjw at rjwysocki.net>; amd-gfx list <amd-
> gfx at lists.freedesktop.org>; ACPI Devel Maling List <linux-
> acpi at vger.kernel.org>; Shai Coleman <git at shaicoleman.com>; Bjorn
> Helgaas <helgaas at kernel.org>; Arthur Borsboom
> <arthurborsboom at gmail.com>; matoro <matoro at airmail.cc>; Deucher,
> Alexander <Alexander.Deucher at amd.com>; Mika Westerberg
> <mika.westerberg at linux.intel.com>; Len Brown <lenb at 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 at 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&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&data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&sd
> ata=cMj
> >
> LDIbjp8RQiWX8pgK2bDUH%2B0u3oquy3TqeT9QjZGE%3D&reserved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1252&data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&sd
> ata=iP9
> >
> EqNcM15Dj4Ax%2BE6e2HaMWHEX%2B0IO3cMoi0NXWGzM%3D&reser
> ved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1304&data=02%7C01%7C
> >
> alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> 7C3dd89
> >
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491&sd
> ata=VlT
> > UV2UCH4RvKgTXZcpGOpkjZpfijmPgwtvKx6HRT04%3D&reserved=0
> > Reported-and-tested-by: Arthur Borsboom <arthurborsboom at gmail.com>
> > Reported-and-tested-by: matoro <matoro at airmail.cc>
> > Reported-by: Aaron Zakhrov <aaron.zakhrov at gmail.com>
> > Reported-by: Michal Rostecki <mrostecki at suse.com>
> > Reported-by: Shai Coleman <git at shaicoleman.com>
> > Signed-off-by: Lukas Wunner <lukas at wunner.de>
> > Cc: stable at vger.kernel.org
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> > Cc: Mika Westerberg <mika.westerberg at 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