Message-Id: <20250126050309.7243-1-manivannan.sadhasivam@linaro.org>
Date: Sun, 26 Jan 2025 10:33:09 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: kbusch@...nel.org,
	axboe@...nel.dk,
	hch@....de,
	sagi@...mberg.me
Cc: linux-nvme@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	andersson@...nel.org,
	konradybcio@...nel.org,
	rafael@...nel.org,
	ulf.hansson@...aro.org,
	helgaas@...nel.org,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Subject: [PATCH] nvme-pci: Use NVME_QUIRK_SIMPLE_SUSPEND for Qualcomm Snapdragon 8cx Gen 3 platforms

On these platforms, power to the PCIe bus is not retained if the SoC enters
its own deep low power state called, CX power collapse state during system
suspend. Once the SoC resumes after going through CX power collapse, all
the PCIe bus state will be lost. So the NVMe devices on these platforms
won't resume properly, rendering the machines useless until forcefully
restarted by the users.

To workaround this issue, the PCIe controller driver is keeping a minimal
vote on the PCIe-MEM interconnect path in its system suspend callback
current currently. While this gives a working system suspend, it also
results in increased power consumption during suspend as the SoC never
enters its low power state. So with this change, the workaround can finally
be removed.

Also it should be noted that the actual fix to this issue lies in the
PCI/PM core. But currently there is no such fix exist as of now and the
consensus to reach it is also taking a lot of time.

So use this quirk to allow users to save battery life on these platforms
until the PCI/PM core comes up with an API that tells the PCI client
drivers when to turn off the devices.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
---

Hi,

I'm sending this patch as there seems to be no consensus so far (as been
discussed in this thread [1]) in coming up with a PCI/PM API that tells the
client drivers when to safely turn off the devices. Since coming up with these
kind of generic APIs are bound to take time (I do intend to pursue), it makes
sense to allow these platforms to make use of the existing
NVME_QUIRK_SIMPLE_SUSPEND quirk in the meantime. Once such API is upstreamed and
used in this driver, this quirk can be reverted.

It should be noted that this CX power collapse issue only exist on the platforms
based on the Qcom Snapdragon 8cx Gen 3 SoC. On other platforms, it is a more of
a policy decision to decide whether to shutdown the devices or not. And these
other platforms can wait for the generic API to be made available by the PCI/PM
core.

[1] https://lore.kernel.org/linux-pci/20241118082344.8146-1-manivannan.sadhasivam@linaro.org

 drivers/nvme/host/pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e2634f437f33..c52dced952a1 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3162,6 +3162,16 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
 	if (dmi_match(DMI_BOARD_NAME, "LXKT-ZXEG-N6"))
 		return NVME_QUIRK_NO_APST;
 
+	/*
+	 * Qualcomm Snapdragon 8cx Gen 3 (SC8280XP) platforms doesn't retain
+	 * power to the PCIe bus after entering low power CX power collapse
+	 * state during system suspend. So shutdown the NVMe devices to have a
+	 * working system suspend on these platforms.
+	 */
+	if (dmi_match(DMI_PRODUCT_FAMILY, "SCP_MAKENA") ||
+	    dmi_match(DMI_PRODUCT_FAMILY, "ThinkPad X13s Gen 1"))
+		return NVME_QUIRK_SIMPLE_SUSPEND;
+
 	return 0;
 }
 
-- 
2.25.1

Powered by blists - more mailing lists