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

[Limonciello, Mario]

[Public]

> 
> I think what we're all missing here is that the concept of requring devices
> to go to D3 for suspend to idle is a higher level concept.  

Ah.. so your argument being we should keep it a higher level concept in Linux
kernel too.  IOW maybe even nvme_acpi_storage_d3 shouldn't be living 
in drivers/nvme/host/pci.c, but somewhere else more acpi platform oriented.

>AFAIK this
> comes from this microsoft document:
> 
> https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro
> 
> and spread from there.  Note that this document explicitly mentions AHCI
> in addition to NVMe.  It also has some issues that I can spot:
> 
>  - PCIe slots are not specific to storage device, so this really needs to
>    apply to all devices

I don't disagree here but I'll point out that on the Windows side that page
mentions that there is also:
1) A "global" registry key option
2) A hardcoded allowlist

>  - it generall is a rather bad idea to start with as each shutdown not
>    only causes media progam/erase cycles, but also is not very power
>    efficient.
> 
> So what we need is a way for a driver to figure out if for a given
> device it should shut down the device fully or just do something that
> is efficient for saving as much as possible power.  That can be either
> in form of a flag 

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.

> or by splitting the suspend method in different ones
> for different use cases.  Platform-specific code (right now for Intel
> and AMD) can then make sure drivers do get the right requests instead of
> hardcoding platform information in every driver that wants to be able
> to implement intelligent suspend behavior.

This seems like a gross assumption though that evicting the quirks into a
central place that every driver needs to behave the same.  AMD's case is
specific to NVME, particularly because APST will be used otherwise.