Re: [PATCH v22] PCI: Avoid D3 at suspend for AMD PCIe root ports w/ USB4 controllers

[Lukas Wunner <lukas@wunner.de>]

On Thu, Oct 05, 2023 at 01:14:40PM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 04, 2023 at 09:49:59AM -0500, Mario Limonciello wrote:
> > Iain reports that USB devices can't be used to wake a Lenovo Z13 from
> > suspend.  This occurs because on some AMD platforms, even though the Root
> > Ports advertise PME_Support for D3hot and D3cold, they don't handle PME
> > messages and generate wakeup interrupts from those states when amd-pmc has
> > put the platform in a hardware sleep state.
> > 
> > Iain reported this on an AMD Rembrandt platform, but it also affects
> > Phoenix SoCs.  On Iain's system, a USB4 router below the affected Root Port
> > generates the PME. To avoid this issue, disable D3 for the root port
> > associated with USB4 controllers at suspend time.
> > 
> > Restore D3 support at resume so that it can be used by runtime suspend.
> > The amd-pmc driver doesn't put the platform in a hardware sleep state for
> > runtime suspend, so PMEs work as advertised.
> > 
> > Cc: stable@vger.kernel.org
> > Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram [1]
> > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> > Reported-by: Iain Lane <iain@orangesquash.org.uk>
> > Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Applied to pci/pm for v6.7, thanks for all your patience!

One belated thought I have on this is that it might be a better fit in
arch/x86/pci/fixups.c rather than drivers/pci/quirks.c.

The latter contains quirks for cards which could appear in any machine,
regardless of the arch.  But this seems to be specific to x86 machines.
I understand these xhci controllers are built into the SoC.

We've had complaints in the past from developers working with
space-constrained Mips routers that the generic quirks in
drivers/pci/quirks.c occupy too much memory:

https://lore.kernel.org/all/1482306784-29224-1-git-send-email-john@phrozen.org/

To cater to their needs, we need to keep in mind that arch-specific
quirks are kept outside of drivers/pci/quirks.c.

I apologize that this didn't occur to me earlier.

Thanks,

Lukas