Re: [PATCH] nvme-pci: Use non-operational power state instead of D3 on Suspend-to-Idle

From: Kai-Heng Feng
Date: Thu May 09 2019 - 06:29:53 EST


at 17:56, Christoph Hellwig <hch@xxxxxx> wrote:

On Thu, May 09, 2019 at 05:42:30PM +0800, Kai-Heng Feng wrote:
That would be a set of 6 new suspend and resume callbacks, mind you,
and there's quite a few of them already. And the majority of drivers
would not need to use them anyway.

I think suspend_to_idle() and resume_from_idle() should be enough?
What are other 4 callbacks?

Also, please note that, possibly apart from the device power state
setting, the S2I and S2R handling really aren't that different at all.
You basically need to carry out the same preparations during suspend
and reverse them during resume in both cases.

But for this case, itâs quite different to the original suspend and
resume callbacks.

Let's think of what cases we needed.

The "classic" suspend in the nvme driver basically shuts down the
device entirely. This is useful for:

a) device that have no power management
b) System power states that eventually power off the entire PCIe bus.
I think that would:

- suspend to disk (hibernate)
- classic suspend to ram

The we have the sequence in your patch. This seems to be related to
some of the MS wording, but I'm not sure what for example tearing down
the queues buys us. Can you explain a bit more where those bits
make a difference?

Based on my testing if queues (IRQ) are not disabled, NVMe controller wonât be quiesced.
Symptoms can be high power drain or system freeze.

I can check with vendors whether this also necessary under Windows.


Otherwise I think we should use a "no-op" suspend, just leaving the
power management to the device, or a simple setting the device to the
deepest power state for everything else, where everything else is
suspend, or suspend to idle.

I am not sure I get your idea. Does this âno-opâ suspend happen in NVMe driver or PM core?


And of course than we have windows modern standby actually mandating
runtime D3 in some case, and vague handwaving mentions of this being
forced on the platforms, which I'm not entirely sure how they fit
into the above picture.

I was told that Windows doesnât use runtime D3, APST is used exclusively.

Kai-Heng