Re: [PATCH] nvme-pci: Shutdown the device if D3Cold is allowed by the user

Ulf Hansson
2024-12-12T12:50:00Z

On Thu, 12 Dec 2024 at 13:21, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Thursday, December 12, 2024 6:59:20 AM CET Christoph Hellwig wrote:
> > On Mon, Dec 09, 2024 at 08:08:21PM +0530, Manivannan Sadhasivam wrote:
> > > >
> > > > The istory here is the the NVMe internal power states are significantly
> > > > better for the SSDs. It avoid shutting down the SSD frequently, which
> > > > creates a lot of extra erase cycles and reduces life time. It also
> > > > prevents the SSD from performing maintainance operations while the host
> > > > system is idle, which is the perfect time for them. But the idea of
> > > > putting all periphals into D3 is gaining a lot of ground because it
> > > > makes the platform vendors life a lot simpler at the cost of others.
> > >
> > > No, I disagree with the last comment. When the system goes to low power mode
> > > (like S2R/hibernate), it *does* makes a lot of sense to put the devices into
> > > D3Cold to save power.
> >
> > Yes. That's what the pm_suspend_via_firmware call in nvme_suspend is
> > supposed to catch.
>
> pm_suspend_via_firmware() is to distinguish different flavors of system
> suspend.

Right. This seems to somewhat work for ACPI types of systems, because
ACPI is controlling the low power state for all the devices. Based on
the requested system wide low power state, ACPI can then decide to
call pm_set_suspend_via_firmware() or not.

Still there is a problem with this for ACPI too.

How does ACPI know whether it's actually a good idea to keep the NVMe
storage powered in s2idle (ACPI calls pm_set_suspend_via_firmware()
only for S2R and S2disk!?)? Especially when my laptop only supports
s2idle and that's what I will use when I close the lid. In this way,
the NMVe storage will certainly contribute to draining the battery,
especially when I won't be using my laptop for a couple of days.

In my opinion, we need a better approach that is both flexible and
that dynamically adjusts based upon the use case.

[...]

Kind regards
Uffe