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

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, Dec 16, 2024 at 06:28:55PM +0100, Rafael J. Wysocki wrote:
> On Mon, Dec 16, 2024 at 5:48 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Mon, Dec 16, 2024 at 05:42:30PM +0100, Rafael J. Wysocki wrote:
> > > On Mon, Dec 16, 2024 at 5:23 PM Christoph Hellwig <hch@lst.de> wrote:
> > > >
> > > > On Sat, Dec 14, 2024 at 12:00:23PM +0530, Manivannan Sadhasivam wrote:
> > > > > We need a PM core API that tells the device drivers when it is safe to powerdown
> > > > > the devices. The usecase here is with PCIe based NVMe devices but the problem is
> > > > > applicable to other devices as well.
> > > >
> > > > Maybe I'm misunderstanding things, but I think the important part is
> > > > to indicate when a suspend actually MUST put the device into D3.  Because
> > > > doing that should always be safe, but not always optimal.
> > >
> > > I'm not aware of any cases when a device must be put into D3cold
> > > (which I think is what you mean) during system-wide suspend.
> > >
> > > Suspend-to-idle on x86 doesn't require this, at least not for
> > > correctness.  I don't think any platforms using DT require it either.
> > >
> >
> > On suspend-to-idle, yes D3Cold doesn't make sense,
> 
> Why?
> 

Because there is no requirement to remove power during S2Idle, isn't it?

From Documentation/admin-guide/pm/sleep-states.rst:

'This is a generic, pure software, light-weight variant of system suspend'.

> > but on suspend-to-ram it is pretty much required.
> 
> Well, I know for a fact that on x86 platforms ACPI S3 does not require
> putting devices into D3cold in general.
> 
> Why is it required for NVMe?
> 

But ACPI code currently calls pm_set_suspend_via_firmware() for S3 suspend. And
that causes NVMe to be powered down because of pm_suspend_via_firmware() check.

> > That applies to DT as well.
> 
> Again, why?

On DT systems if firmware supports both S2Idle and S2R, devices can be kept in
low power state during S2Idle and powered down during S2R.

The problem comes if the firmware only supports the former state.

- Mani

-- 
மணிவண்ணன் சதாசிவம்