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

[Kai-Heng Feng]

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