PCI: qcom: Prevent GDSC power down on suspend

[PATCH] PCI: qcom: Prevent GDSC power down on suspend

Posted by Krishna Chaitanya Chundru 1 week, 2 days ago

Currently, the driver expects the devices to remain in D0 across system
suspend, but the genpd framework may still power down the associated
GDSC during suspend. When that happens, the PCIe link goes down and
cannot be recovered on resume.

Prevent genpd from turning off the PCIe GDSC by using
dev_pm_genpd_rpm_always_on() so that the power domain stays on while
the controller is suspended. This preserves the link state across
suspend/resume and avoids unrecoverable link failures.

Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5a318487b2b3f6c61d8f5b1fd5cdf2738a1f1dcd..314cf334a313dff35efaf0c023597e6eef483925 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -25,6 +25,7 @@
 #include <linux/pci.h>
 #include <linux/pci-ecam.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/phy/pcie.h>
@@ -2052,6 +2053,11 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
 		pcie->suspended = true;
 	}
 
+	if (pcie->suspended)
+		dev_pm_genpd_rpm_always_on(dev, false);
+	else
+		dev_pm_genpd_rpm_always_on(dev, true);
+
 	/*
 	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
 	 * Because on some platforms, DBI access can happen very late during the

---
base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
change-id: 20260128-genpd_fix-3aa413d9a383

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

Posted by Bjorn Andersson 1 week, 2 days ago

On Wed, Jan 28, 2026 at 05:52:42PM +0530, Krishna Chaitanya Chundru wrote:
> Currently, the driver expects the devices to remain in D0 across system
> suspend, but the genpd framework may still power down the associated
> GDSC during suspend. When that happens, the PCIe link goes down and
> cannot be recovered on resume.
> 

The GDSC is a child of CX, so by keeping it always-on, you effectively
put an always-on vote on CX, forever preventing CXPC.

In fact, this is one of the reasons why the PCIe GDSCs on most targets
is marked PWRSTS_RET_ON (in the clock driver) so that the "off state"
doesn't actually turn off the GDSC, but it relinquishes the inherited
vote on CX.

> Prevent genpd from turning off the PCIe GDSC by using
> dev_pm_genpd_rpm_always_on() so that the power domain stays on while
> the controller is suspended. This preserves the link state across
> suspend/resume and avoids unrecoverable link failures.
> 

We are able to suspend/resume a whole bunch of platforms today, which
one are you on?

That said, while we can suspend/resume, we're not allowing CXPC today.
On many systems the main culprit is the icc_set_bw() vote in
qcom_pcie_suspend_noirq().

Regards,
Bjorn

> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5a318487b2b3f6c61d8f5b1fd5cdf2738a1f1dcd..314cf334a313dff35efaf0c023597e6eef483925 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -25,6 +25,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci-ecam.h>
>  #include <linux/pm_opp.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy/pcie.h>
> @@ -2052,6 +2053,11 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  		pcie->suspended = true;
>  	}
>  
> +	if (pcie->suspended)
> +		dev_pm_genpd_rpm_always_on(dev, false);
> +	else
> +		dev_pm_genpd_rpm_always_on(dev, true);
> +
>  	/*
>  	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
>  	 * Because on some platforms, DBI access can happen very late during the
> 
> ---
> base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
> change-id: 20260128-genpd_fix-3aa413d9a383
> 
> Best regards,
> -- 
> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> 
>

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

Posted by Konrad Dybcio 1 week, 2 days ago

On 1/28/26 1:22 PM, Krishna Chaitanya Chundru wrote:
> Currently, the driver expects the devices to remain in D0 across system
> suspend, but the genpd framework may still power down the associated
> GDSC during suspend. When that happens, the PCIe link goes down and
> cannot be recovered on resume.
> 
> Prevent genpd from turning off the PCIe GDSC by using
> dev_pm_genpd_rpm_always_on() so that the power domain stays on while
> the controller is suspended. This preserves the link state across
> suspend/resume and avoids unrecoverable link failures.
> 
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---

How does this play along with your D3Cold series?

Is this patch supposed to be applied first?

Konrad