[PATCH] nvme-pci: set some AMD PCIe downstream storage device to D3 for s2idle

[Rafael J. Wysocki]

On Wednesday, May 26, 2021 4:45:28 PM CEST Keith Busch wrote:
> On Wed, May 26, 2021 at 10:52:35AM +0200, Hans de Goede wrote:
> > On 5/25/21 5:18 PM, Limonciello, Mario wrote:
> > > 
> > > So how about a publishing a notification chain that a platform driver can
> > > optionally pick up and set that flag when the device is probed?  Coming
> > > back to my idea to throw this in amd-pmc, that could also potentially
> > > mean moving out the Lenovo DMI quirk and let something like
> > > thinkpad-acpi behave as a notifier and handle it too.
> > > 
> > > Hans, would appreciate your thoughts here.
> > 
> > I see that the discussion has already continued without my thoughts (good),
> > reading the further discussion I see that pm_suspend_via_firmware() and
> > pm_suspend_no_platform() have already been mentioned. Using these (or
> > introducing something similar for this use-case) was also my first
> > thought on this (after the previous options were shot down).
> > 
> > So I think that looking at those is going in the right direction. I notice
> > that Rafael Wysocki is missing from the Cc (I've added him now) I believe
> > that this is something which is right up his alley and he might have some
> > ideas on this.
> > 
> > Mario, can you provide a summary of the discussion so far for Rafael
> > (I believe you have a better picture of this then I do).
> 
> Rafael,
> 
> For context, here's the summary from my understanding:
> 
> We (linux-nvme) received a bug report that a platform fails to resume
> after idle suspend due to mismatched behavior with the nvme driver.
> 
> When suspending, the nvme driver checks pm_suspend_via_firmware(). If
> false, the driver assumes platform firmware will not alter our device's
> power state after the kernel completes its suspend process.
> 
> But this platform's SMU firmware will remove power from the device.

How exactly does it do that?

In particular, how does it get a chance to run?

> Since the driver believed that wouldn't happen, the driver did not
> prepare the device for this powerloss event.
> 
> It seems that the kernel's assumptions around pm_suspend_via_firmware()
> and pm_suspend_no_platform() may not accurately reflect what the
> platform's firmware actually does.

Note that this is not about whether or not AML will remove power from devices. 

It is about passing control entirely to the platform firmware at the end of the
suspend transition.

If instead the kernel executes AML that happens to remove power from some
devices, that is a totally different case which should not be confused with
the above.

> I do not know of a better way to detect if the platform will remove power,
> so I'm looking at quirks to suppress PM_SUSPEND_FLAG_NO_PLATFORM for
> this platform. I'm hoping there's a better option, though :)

Honestly, I'm not sure about the clear understanding of what's really going on
here.