On Wed, Sep 23, 2020 at 2:15 PM Lukas Wunner <lukas@xxxxxxxxx> wrote: > > [cc += Mario Limonciello @ Dell] > > On Tue, Sep 22, 2020 at 10:39:17AM -0400, Alex Deucher wrote: > > On Tue, Sep 22, 2020 at 2:54 AM Lukas Wunner <lukas@xxxxxxxxx> wrote: > > > On Mon, Sep 21, 2020 at 07:10:55PM -0400, Alex Deucher wrote: > > > > Recent AMD laptops which have iGPU + dGPU have been non-functional on > > > > Linux. The issue is that the laptops rely on ACPI to control the dGPU > > > > power and that is not happening because the bridges are hotplug > > > > capable, and the current pci code does not allow runtime pm on hotplug > > > > capable bridges. This worked on previous laptops presumably because > > > > the bridges did not support hotplug or they hit one of the allowed > > > > cases. The driver enables runtime power management, but since the > > > > dGPU does not actually get powered down via the platform ACPI > > > > controls, no power is saved, and things fall apart on resume leading > > > > to an unusable GPU or a system hang. To work around this users can > > > > currently disable runtime pm in the GPU driver or specify > > > > pcie_port_pm=force to force d3 on bridges. I'm not sure what the best > > > > solution for this is. I'd rather not have to add device IDs to a > > > > whitelist every time we release a new platform. Suggestions? What > > > > about something like the attached patch work? > > > > > > What is Windows doing on these machines? Microsoft came up with an > > > ACPI _DSD property to tell OSPM that it's safe to suspend a hotplug > > > port to D3: > > > > > > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-pcie-root-ports-supporting-hot-plug-in-d3 > > I've just taken a look at the ACPI dumps provided by Michal Rostecki > and Arthur Borsboom in the Gitlab bugs linked below. The topology > looks like this: > > 00:01.1 Root Port [\_SB.PCI0.GPP0] > 01:00.0 Switch Upstream [\_SB.PCI0.GPP0.SWUS] > 02:00.0 Switch Downstream [\_SB.PCI0.GPP0.SWUS.SWDS] > 03:00.0 dGPU [\_SB.PCI0.GPP0.SWUS.SWDS.VGA] > 03:00.1 dGPU Audio [\_SB.PCI0.GPP0.SWUS.SWDS.HDAU] > > The Root Port is hotplug-capable but is not suspended because we only > allow that for Thunderbolt hotplug ports or root ports with Microsoft's > HotPlugSupportInD3 _DSD property. However, that _DSD is not present > in the ACPI dumps and the Root Port is obviously not a Thunderbolt > port either. > > Again, it would be good to know why it's working on Windows. Agreed. > Could you ask AMD Windows driver folks? The ACPI tables look very > similar even though they're from different vendors (Dell and MSI), > so they were probably supplied by AMD to those OEMs. > > It's quite possible that Windows is now using a BIOS cut-off and > allows D3 by default on newer BIOSes, so I'm not opposed to your patch. I would reorder this function, though, to avoid calling dmi_get_bios_year() twice. The blacklist can be checked before is_hotplug_bridge, so I would just make the cut-off date depend on the latter. > But it would be good to have some kind of confirmation before we risk > regressing machines which do not support D3 on hotplug-capable Root Ports. Certainly - if possible. Cheers! > > > > From 3a08cb6ac38c47b921b8b6f31b03fcd8f13c4018 Mon Sep 17 00:00:00 2001 > > > > From: Alex Deucher <alexander.deucher@xxxxxxx> > > > > Date: Mon, 21 Sep 2020 18:07:27 -0400 > > > > Subject: [PATCH] pci: allow d3 on hotplug bridges after 2018 > > > > > > > > Newer AMD laptops have hotplug capabe bridges with dGPUs behind them. > > > > If d3 is disabled on the bridge, the dGPU is never powered down even > > > > though the dGPU driver may think it is because power is handled by > > > > the pci core. Things fall apart when the driver attempts to resume > > > > a dGPU that was not properly powered down which leads to hangs. > > > > > > > > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1252 > > > > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1222 > > > > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1304 > > > > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > > > > --- > > > > drivers/pci/pci.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > > index a458c46d7e39..12927d5df4b9 100644 > > > > --- a/drivers/pci/pci.c > > > > +++ b/drivers/pci/pci.c > > > > @@ -2856,7 +2856,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > > > > * by vendors for runtime D3 at least until 2018 because there > > > > * was no OS support. > > > > */ > > > > - if (bridge->is_hotplug_bridge) > > > > + if (bridge->is_hotplug_bridge && (dmi_get_bios_year() <= 2018)) > > > > return false; > > > > > > > > if (dmi_check_system(bridge_d3_blacklist)) > > > > -- > > > > 2.25.4 > > > >