Re: [PATCH 0/3] PCI: Notify PCI drivers about powerdown during suspend

From: Bjorn Helgaas <helgaas@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: bhelgaas@google.com, lorenzo.pieralisi@arm.com,
	kbusch@kernel.org, hch@lst.de, linux-nvme@lists.infradead.org,
	linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, svarbanov@mm-sol.com,
	bjorn.andersson@linaro.org, axboe@fb.com,
	quic_vbadigan@quicinc.com, quic_krichai@quicinc.com,
	quic_nitirawa@quicinc.com, vidyas@nvidia.com, sagi@grimberg.me,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH 0/3] PCI: Notify PCI drivers about powerdown during suspend
Date: Mon, 16 May 2022 15:15:45 -0500	[thread overview]
Message-ID: <20220516201545.GA1046888@bhelgaas> (raw)
In-Reply-To: <20220513110027.31015-1-manivannan.sadhasivam@linaro.org>

[+cc Rafael, linux-pm, since this is power management stuff
https://lore.kernel.org/all/20220513110027.31015-1-manivannan.sadhasivam@linaro.org/]

On Fri, May 13, 2022 at 04:30:24PM +0530, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series adds support for notifying the PCI drivers like NVMe about the
> transition of PCI devices into powerdown mode during system suspend.
> 
> Background
> ----------
> 
> On Qcom SC7280 based Chrome platforms, the OS will turn off the power to all
> PCIe devices during system suspend for aggressive powersaving. Currently, there
> is no way for the PCI device drivers to learn about this situation. Some of the
> drivers assume that the power will be retained and some others assume that the
> power may be taken down.
> 
> We faced the issue with NVMe PCI driver, where the driver expects the NVMe
> device to be in APST (Autonomous Power State Transition) state for power saving
> during system suspend. So when the power goes down, the NVMe driver fails to
> bringup the device during resume.
> 
> Previous work
> -------------
> 
> We tried to fix this issue in a couple of ways:
> 
> 1. The NVMe PCI driver checks for the existence of "StorageD3Enable" ACPI
> property in the suspend path. If the property is found, the driver assumes that
> the device may go to poweroff state and shutdowns the device accordingly.
> 
> As like the ACPI based systems, we also tried to get the support in place for
> DT based systems. But that didn't get accepted:
> https://lore.kernel.org/all/Yl+6V3pWuyRYuVV8@infradead.org/T/
> 
> 2. Keith Busch proposed a module params based approach. The parameter when set,
> will allow the driver to support APST during suspend. Absence of that parameter
> will let the driver shutdown the device.
> 
> This also did not get accepted:
> https://lore.kernel.org/linux-nvme/20220201165006.3074615-1-kbusch@kernel.org/
> 
> Proposal
> --------
> 
> Christoph suggested to add a notification in the PCI/PM core to let the NVMe
> driver know that the device will go into powerdown state during suspend.
> https://lore.kernel.org/all/Yg0wklcJ3ed76Jbk@infradead.org/ 
> 
> Hence in this series, a "suspend_poweroff" flag is introduced in the host bridge
> struct. When this flag is set by the PCI RC drivers, the PCI device driver like
> NVMe can shutdown the device during suspend.
> 
> In the coming days, the usage of this flag could be extended to other PCI
> drivers as well.
> 
> Testing
> -------
> 
> This series has been tested on SC7280 IDP board connected to a NVMe PCI device.
> 
> Thanks,
> Mani
> 
> Manivannan Sadhasivam (3):
>   PCI: Add a flag to notify PCI drivers about powerdown during suspend
>   PCI: dwc: qcom: Set suspend_poweroff flag for SC7280
>   nvme-pci: Make use of "suspend_poweroff" flag during system suspend
> 
>  drivers/nvme/host/pci.c                | 3 ++-
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
>  include/linux/pci.h                    | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> -- 
> 2.25.1
>