[PATCH 0/6] qcom: Refactor GPU GX GDSC handling and IFPC behavior on A8x GPUs

[PATCH 0/6] qcom: Refactor GPU GX GDSC handling and IFPC behavior on A8x GPUs

[Taniya Das]

This series fixes two closely related issues around GX GDSC ownership and
rail control on A8x GPUs, where recent hardware changes and runtime PM
interactions violate GMU and IFPC(Inter Frame Power Collapse) architectural
expectations.

Problem statement 1: Incorrect GX GDSC disable by OS
----------------------------------------------------
GX GDSC is a special GPU power domain that must be exclusively controlled
by GMU firmware during normal operation. The OS is expected to force‑
disable GX GDSC only during GPU/GMU recovery.

However, when GX GDSC is managed through the generic GenPD runtime PM
framework, it can be unintentionally disabled by the OS during system
resume (resume_noirq/complete phases) or runtime PM suspend paths. This
can race with GMU firmware usage and result in warnings such as:

  gx_clkctl_gx_gdsc status stuck at 'on'

and failures in gdsc_toggle_logic(), leading to broken GPU suspend/resume
behaviour.

Solution:
Introduce a custom disable callback for GX GDSC that prevents the OS from
touching GX GDSC hardware during normal runtime PM and system PM flows.
The callback relies on GenPD’s synced_poweroff flag, which is asserted by
the GMU driver only during recovery, explicitly allowing GX GDSC to be
disabled by the OS in that case.

This ensures strict GX GDSC ownership by GMU while still supporting
recovery use cases.

Problem statement 2: Unintended GX/GMxC rail votes from APPS RSC
--------------------------------------------------------------
On A8x platforms, GX GDSC has been moved to a dedicated GXCLKCTL block
under the GX power domain. Due to the current runtime PM device links
between supplier and consumer, when GMU device is moved to RPM_ACTIVE
state, GXCLKCTL device will also be moved to RPM_ACTIVE and result in
GX/GMxC rail votes from the OS.

This behavior conflicts with IFPC and Adreno architecture requirements,
which mandate that GMU firmware must be the sole voter of these
collapsible rails on behalf of the GPU. Linux is expected to intervene
only during GPU/GMU recovery.

Solution:
The runtime PM of GXCLKCTL can be disabled post the GX CLKCTL is runtime
suspended in probe, so the runtime PM requests of GMU device are not propagated
to its supplier GXCLKCTL.  To avoid incomplete runtime suspend during probe on
clock controllers using runtime PM, ensure pm_runtime_put_sync() is used so that
runtime PM suspend completes before returning from probe.

This along with GMU driver change to vote on GX GDSC only during GMU recovery will
prevent the votes on GX/GMXC rails from APPS RSC during normal GMU operation.

Patch overview:
--------------
1. clk: qcom: gdsc: Add custom disable callback for GX GDSC
   - Prevents unintended GX GDSC disable outside recovery.

2. clk: qcom: gxclkctl: Use custom disable callback for gx_gdsc
   - Applies the custom behavior to GXCLKCTL and fixes runtime PM warnings.

3. clk: qcom: common: ensure runtime PM suspend completes on probe
   - Guarantees synchronous runtime suspend during probe.

4. clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC
   - Eliminates unintended APPS RSC rail votes, restoring GMU ownership.

5. drm/msm/a8xx: Make a8xx_recover IFPC safe
   - Makes the A8xx recovery path IFPC‑aware by checking GX power‑domain
   state before accessing GX MMIO, matching a6xx behavior.

6. drm/msm/a6xx: Limit GXPD votes to recovery in A8x
   - Removes GXPD voting from normal GMU runtime PM and restricts it to
   recovery using the synced_poweroff mechanism.

Together, these changes restore strict GMU ownership of GX GDSC and GPU
rails, align Linux behavior with IFPC architecture, resolves reported
runtime warnings and failures, and enable correct power collapse of
GX/GMxC on A8x GPUs.

Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
Akhil P Oommen (2):
      drm/msm/a8xx: Make a8xx_recover IFPC safe
      drm/msm/a6xx: Limit GXPD votes to recovery in A8x

Jagadeesh Kona (1):
      clk: qcom: gdsc: Add custom disable callback for GX GDSC

Taniya Das (3):
      clk: qcom: gxclkctl: Use custom disable callback for gx_gdsc
      clk: qcom: common: ensure runtime PM suspend completes on probe
      clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC

 drivers/clk/qcom/common.c             |  2 +-
 drivers/clk/qcom/gdsc.c               | 22 ++++++++++++
 drivers/clk/qcom/gdsc.h               |  1 +
 drivers/clk/qcom/gxclkctl-kaanapali.c | 12 ++++++-
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 64 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/msm/adreno/a8xx_gpu.c | 15 +++++---
 6 files changed, 99 insertions(+), 17 deletions(-)
---
base-commit: 2febe6e6ee6e34c7754eff3c4d81aa7b0dcb7979
change-id: 20260406-gfx-clk-fixes-24a492bb7676

Best regards,
-- 
Taniya Das <taniya.das@oss.qualcomm.com>

[PATCH 1/6] clk: qcom: gdsc: Add custom disable callback for GX GDSC

[Taniya Das]

From: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>

The GX GDSC is a special power domain that should only be disabled
by OS during GMU recovery. In all other scenarios, the GMU firmware
is responsible for handling its disable sequence, and OS must not
interfere.

During the resume_noirq() phase of system resume, the GenPD framework
enables all power domains and later disables them in the complete()
phase if there are no active votes from OS. This behavior can
incorrectly disable the GX GDSC while the GMU firmware is still using
it.

To prevent this, implement a custom disable callback for GX GDSC that
relies on GenPD’s synced_poweroff flag. The GMU driver sets this flag
only during recovery, allowing OS to explicitly disable GX GDSC in
hardware in that case. In all other situations, the disable callback
will avoid touching GX GDSC hardware.

Signed-off-by: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>
Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 95aa071202455421d818171d04f95d15e2b581fa..ab5d741a2e2351bfac06a6814c5a8ba7355bc8bc 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -675,3 +675,25 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+/*
+ * GX GDSC is a special power domain. Normally, its disable sequence
+ * is managed by the GMU firmware, and high level OS must not attempt
+ * to disable it. The only exception is during GMU recovery, where the
+ * GMU driver can set GenPD’s synced_poweroff flag to allow explicitly
+ * disable GX GDSC in hardware.
+ */
+int gdsc_gx_disable(struct generic_pm_domain *domain)
+{
+	struct gdsc *sc = domain_to_gdsc(domain);
+
+	if (domain->synced_poweroff)
+		return gdsc_disable(domain);
+
+	/* Remove parent-supply placed in enable */
+	if (sc->rsupply)
+		return regulator_disable(sc->rsupply);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gdsc_gx_disable);
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index dd843e86c05b2f30e6d9e978681580016333839d..495daebaf99519ba0571070b41d133ee867c4fd3 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -88,6 +88,7 @@ int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
 		  struct regmap *);
 void gdsc_unregister(struct gdsc_desc *desc);
 int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_gx_disable(struct generic_pm_domain *domain);
 #else
 static inline int gdsc_register(struct gdsc_desc *desc,
 				struct reset_controller_dev *rcdev,

-- 
2.34.1

[PATCH 2/6] clk: qcom: gxclkctl: Use custom disable callback for gx_gdsc

[Taniya Das]

The GX GDSC represents a special GPU power domain that must not be
disabled during normal runtime PM flows. As per the GMU architecture,
GX GDSC should only be force-disabled during GMU/GPU recovery, where the
OS explicitly resets the GX power domain.

However, when managed by the generic GDSC runtime PM path, GX GDSC may be
disabled during GMU runtime suspend, resulting in warnings such as:

  gx_clkctl_gx_gdsc status stuck at 'on'

and failures in gdsc_toggle_logic() during rpm suspend.

Use the newly added custom disable callback for gx_gdsc to ensure the
GDSC is toggled only in recovery scenarios, while preventing unintended
disable attempts during normal GMU runtime PM operations.

Reported-by: Pengyu Luo <mitltlatltl@gmail.com>
Closes: https://lore.kernel.org/all/CAH2e8h4Vp9fJYAUUbOmoHSKB25wakPBvmpwa62BTRqgRQbMWuw@mail.gmail.com/
Reported-by: Alexander Koskovich <akoskovich@pm.me>
Closes: https://lore.kernel.org/all/gwVAH2mJerU4dBInw8pKmOs5aQK55Q7W6q_UQAlLFCsEgX6eyvSgXAWbNNMqAX4WmPlYCKUSMhfkr5Jry4Ps5EqnxYZqEEDd3Whwv7ZXGlc=@pm.me/
Fixes: 5af11acae660 ("clk: qcom: Add a driver for SM8750 GPU clocks")
Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/clk/qcom/gxclkctl-kaanapali.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gxclkctl-kaanapali.c b/drivers/clk/qcom/gxclkctl-kaanapali.c
index 40d856378a74aeb706c2f4a7a17a2c5602042af2..d7cf6834dd77c2a5320ffb8548cdb515be237bdc 100644
--- a/drivers/clk/qcom/gxclkctl-kaanapali.c
+++ b/drivers/clk/qcom/gxclkctl-kaanapali.c
@@ -26,6 +26,7 @@ static struct gdsc gx_clkctl_gx_gdsc = {
 	.pd = {
 		.name = "gx_clkctl_gx_gdsc",
 		.power_on = gdsc_gx_do_nothing_enable,
+		.power_off = gdsc_gx_disable,
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,

-- 
2.34.1

[PATCH 3/6] clk: qcom: common: ensure runtime PM suspend completes on probe

[Taniya Das]

When the clock controller is probed with 'use_rpm' enabled, the
runtime PM reference is currently released using pm_runtime_put(),
which may return before the runtime suspend has completed. This
can leave the power domain transition in progress while the probe
path continues or returns.

Use pm_runtime_put_sync() instead to ensure the runtime PM “put”
completes synchronously during probe, guaranteeing the device is
fully runtime‑suspended before returning.

Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/clk/qcom/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index eec369d2173b5ce24bc1ca860d2ac1bbdce04524..2c09abaf1d2a15b7fbbbfeb67c03075381185a00 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -428,7 +428,7 @@ int qcom_cc_really_probe(struct device *dev,
 
 put_rpm:
 	if (desc->use_rpm)
-		pm_runtime_put(dev);
+		pm_runtime_put_sync(dev);
 
 	return ret;
 }

-- 
2.34.1

[PATCH 4/6] clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC

[Taniya Das]

The GX GDSC control is handled through a dedicated clock controller,
and the enable/disable sequencing depends on correct rail voting.
The driver votes for the GX/GMxC rails and CX GDSC before toggling
the GX GDSC. Currently, during GMU runtime PM resume, rails remain
enabled due to upstream votes propagated via RPM-enabled devlinks
and explicit pm_runtime votes on GX GDSC.

This is not an expected behaviour of IFPC(Inter Frame Power Collapse)
requirements of GPU as GMU firmware is expected to control these rails,
except during the GPU/GMU recovery via the OS and that is where the GX
GDSC should be voting for the rails (GX/GMxC and CX GDSC) before
toggling the GX GDSC.

Thus, disable runtime PM after successfully registering the clock
controller.

Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/clk/qcom/gxclkctl-kaanapali.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gxclkctl-kaanapali.c b/drivers/clk/qcom/gxclkctl-kaanapali.c
index d7cf6834dd77c2a5320ffb8548cdb515be237bdc..d470ade11b0d11eb40843fe84c809e71646dce27 100644
--- a/drivers/clk/qcom/gxclkctl-kaanapali.c
+++ b/drivers/clk/qcom/gxclkctl-kaanapali.c
@@ -7,6 +7,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,kaanapali-gxclkctl.h>
@@ -61,7 +62,15 @@ MODULE_DEVICE_TABLE(of, gx_clkctl_kaanapali_match_table);
 
 static int gx_clkctl_kaanapali_probe(struct platform_device *pdev)
 {
-	return qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc);
+	int ret;
+
+	ret = qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc);
+	if (ret)
+		return ret;
+
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
 }
 
 static struct platform_driver gx_clkctl_kaanapali_driver = {

-- 
2.34.1

[PATCH 5/6] drm/msm/a8xx: Make a8xx_recover IFPC safe

[Taniya Das]

From: Akhil P Oommen <akhilpo@oss.qualcomm.com>

Similar to a6xx_recover(), check the GX power domain status before
accessing mmio in GX domain a8xx_recover().

Fixes: 288a93200892 ("drm/msm/adreno: Introduce A8x GPU Support")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a8xx_gpu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a8xx_gpu.c b/drivers/gpu/drm/msm/adreno/a8xx_gpu.c
index ccfccc45133fda53168d3475ebd3d543f10268af..9b99ec5ceeb5826fbd5cd1059febf5bc5ba468b5 100644
--- a/drivers/gpu/drm/msm/adreno/a8xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a8xx_gpu.c
@@ -886,17 +886,22 @@ void a8xx_recover(struct msm_gpu *gpu)
 
 	adreno_dump_info(gpu);
 
-	if (hang_debug)
-		a8xx_dump(gpu);
-
 	/*
 	 * To handle recovery specific sequences during the rpm suspend we are
 	 * about to trigger
 	 */
 	a6xx_gpu->hung = true;
 
-	/* Halt SQE first */
-	gpu_write(gpu, REG_A8XX_CP_SQE_CNTL, 3);
+	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
+		/*
+		 * Sometimes crashstate capture is skipped, so SQE should be
+		 * halted here again
+		 */
+		gpu_write(gpu, REG_A8XX_CP_SQE_CNTL, 3);
+
+		if (hang_debug)
+			a8xx_dump(gpu);
+	}
 
 	pm_runtime_dont_use_autosuspend(&gpu->pdev->dev);
 

-- 
2.34.1

[PATCH 6/6] drm/msm/a6xx: Limit GXPD votes to recovery in A8x

[Taniya Das]

From: Akhil P Oommen <akhilpo@oss.qualcomm.com>

In A8x GPUs, the GX GDSC is moved to a separate block called GXCLKCTL
which is under the GX power domain. Due to the way the support for this
block is implemented in its driver, pm_runtime votes result in a vote on
GX/GMxC/MxC rails from the APPS RSC. This is against the Adreno
architecture which require GMU to be the sole voter of these collapsible
rails on behalf of GPU, except during the GPU/GMU recovery.

To align with this architectural requirement and to realize the power
benefits of the IFPC feature, remove the GXPD votes during gmu resume
and suspend. And during the recovery sequence, enable/disable the GXPD
along with the 'synced_poweroff' genpd hint to force collapse this GDSC.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 64 +++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 1b44b9e21ad868e6454b9140284cc7ebedc4f59a..b7166a883b018f459caae742e9a589f32167f8d2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1250,6 +1250,56 @@ static int a6xx_gmu_secure_init(struct a6xx_gpu *a6xx_gpu)
 	return 0;
 }
 
+static int a6xx_gmu_gxpd_get(struct a6xx_gmu *gmu)
+{
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+
+	if (IS_ERR_OR_NULL(gmu->gxpd))
+		return 0;
+
+	/*
+	 * On A8xx HW, GX GDSC is moved to a new clk controller block under GX
+	 * power domain. The clock driver for this new block keeps the GX rail
+	 * voted when gxpd is voted. So, use the gxpd only during gpu recovery.
+	 */
+	if (adreno_gpu->info->family >= ADRENO_8XX_GEN1)
+		return 0;
+
+	/*
+	 * On A6x/A7x, "enable" the GX power domain which won't actually do
+	 * anything but it will make sure that the refcounting is correct in
+	 * case we need to bring down the GX after a GMU failure
+	 */
+	return pm_runtime_get_sync(gmu->gxpd);
+}
+
+static int a6xx_gmu_gxpd_put(struct a6xx_gmu *gmu)
+{
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+
+	if (IS_ERR_OR_NULL(gmu->gxpd))
+		return 0;
+
+	if (adreno_gpu->info->family < ADRENO_8XX_GEN1)
+		return pm_runtime_put_sync(gmu->gxpd);
+
+	/*
+	 * On A8x, GX GDSC collapse should be triggered only when it is stuck ON
+	 */
+	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
+		pm_runtime_get_sync(gmu->gxpd);
+		/*
+		 * Hint to gfxclkctl driver to do a hw collapse during the next
+		 * RPM PUT. This is a special behavior in the gfxclkctl driver
+		 */
+		dev_pm_genpd_synced_poweroff(gmu->gxpd);
+		pm_runtime_put_sync(gmu->gxpd);
+	}
+
+	return 0;
+}
 
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 {
@@ -1266,13 +1316,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 	/* Turn on the resources */
 	pm_runtime_get_sync(gmu->dev);
 
-	/*
-	 * "enable" the GX power domain which won't actually do anything but it
-	 * will make sure that the refcounting is correct in case we need to
-	 * bring down the GX after a GMU failure
-	 */
-	if (!IS_ERR_OR_NULL(gmu->gxpd))
-		pm_runtime_get_sync(gmu->gxpd);
+	a6xx_gmu_gxpd_get(gmu);
 
 	/* Use a known rate to bring up the GMU */
 	clk_set_rate(gmu->core_clk, 200000000);
@@ -1339,7 +1383,8 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 disable_clk:
 	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
 rpm_put:
-	pm_runtime_put(gmu->gxpd);
+	a6xx_gmu_gxpd_put(gmu);
+
 	pm_runtime_put(gmu->dev);
 
 	return ret;
@@ -1455,8 +1500,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
 	 * domain. Usually the GMU does this but only if the shutdown sequence
 	 * was successful
 	 */
-	if (!IS_ERR_OR_NULL(gmu->gxpd))
-		pm_runtime_put_sync(gmu->gxpd);
+	a6xx_gmu_gxpd_put(gmu);
 
 	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
 

-- 
2.34.1

Re: [PATCH 1/6] clk: qcom: gdsc: Add custom disable callback for GX GDSC

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> From: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>
> 
> The GX GDSC is a special power domain that should only be disabled
> by OS during GMU recovery. In all other scenarios, the GMU firmware
> is responsible for handling its disable sequence, and OS must not
> interfere.
> 
> During the resume_noirq() phase of system resume, the GenPD framework
> enables all power domains and later disables them in the complete()
> phase if there are no active votes from OS. This behavior can
> incorrectly disable the GX GDSC while the GMU firmware is still using
> it.
> 
> To prevent this, implement a custom disable callback for GX GDSC that
> relies on GenPD’s synced_poweroff flag. The GMU driver sets this flag
> only during recovery, allowing OS to explicitly disable GX GDSC in
> hardware in that case. In all other situations, the disable callback
> will avoid touching GX GDSC hardware.
> 
> Signed-off-by: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 95aa071202455421d818171d04f95d15e2b581fa..ab5d741a2e2351bfac06a6814c5a8ba7355bc8bc 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -675,3 +675,25 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> +
> +/*
> + * GX GDSC is a special power domain. Normally, its disable sequence
> + * is managed by the GMU firmware, and high level OS must not attempt

nit: "and high level OS must not attempt" -> "and the OS must not attempt"


> + * to disable it. The only exception is during GMU recovery, where the
> + * GMU driver can set GenPD’s synced_poweroff flag to allow explicitly
> + * disable GX GDSC in hardware.
> + */
> +int gdsc_gx_disable(struct generic_pm_domain *domain)
> +{
> +	struct gdsc *sc = domain_to_gdsc(domain);
> +
> +	if (domain->synced_poweroff)
> +		return gdsc_disable(domain);

Can we use this version to replace gdsc_gx_do_nothing_enable() too?

Konrad

Re: [PATCH 2/6] clk: qcom: gxclkctl: Use custom disable callback for gx_gdsc

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> The GX GDSC represents a special GPU power domain that must not be
> disabled during normal runtime PM flows. As per the GMU architecture,
> GX GDSC should only be force-disabled during GMU/GPU recovery, where the
> OS explicitly resets the GX power domain.
> 
> However, when managed by the generic GDSC runtime PM path, GX GDSC may be
> disabled during GMU runtime suspend, resulting in warnings such as:
> 
>   gx_clkctl_gx_gdsc status stuck at 'on'
> 
> and failures in gdsc_toggle_logic() during rpm suspend.
> 
> Use the newly added custom disable callback for gx_gdsc to ensure the
> GDSC is toggled only in recovery scenarios, while preventing unintended
> disable attempts during normal GMU runtime PM operations.
> 
> Reported-by: Pengyu Luo <mitltlatltl@gmail.com>
> Closes: https://lore.kernel.org/all/CAH2e8h4Vp9fJYAUUbOmoHSKB25wakPBvmpwa62BTRqgRQbMWuw@mail.gmail.com/
> Reported-by: Alexander Koskovich <akoskovich@pm.me>
> Closes: https://lore.kernel.org/all/gwVAH2mJerU4dBInw8pKmOs5aQK55Q7W6q_UQAlLFCsEgX6eyvSgXAWbNNMqAX4WmPlYCKUSMhfkr5Jry4Ps5EqnxYZqEEDd3Whwv7ZXGlc=@pm.me/
> Fixes: 5af11acae660 ("clk: qcom: Add a driver for SM8750 GPU clocks")
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 3/6] clk: qcom: common: ensure runtime PM suspend completes on probe

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> When the clock controller is probed with 'use_rpm' enabled, the
> runtime PM reference is currently released using pm_runtime_put(),
> which may return before the runtime suspend has completed. This
> can leave the power domain transition in progress while the probe
> path continues or returns.
> 
> Use pm_runtime_put_sync() instead to ensure the runtime PM “put”
> completes synchronously during probe, guaranteeing the device is
> fully runtime‑suspended before returning.

It's not immediately obvious why that's an issue, is there an
observable problem when the transition doesn't complete in time?

Konrad

Re: [PATCH 5/6] drm/msm/a8xx: Make a8xx_recover IFPC safe

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> From: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> 
> Similar to a6xx_recover(), check the GX power domain status before
> accessing mmio in GX domain a8xx_recover().
> 
> Fixes: 288a93200892 ("drm/msm/adreno: Introduce A8x GPU Support")
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 6/6] drm/msm/a6xx: Limit GXPD votes to recovery in A8x

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> From: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> 
> In A8x GPUs, the GX GDSC is moved to a separate block called GXCLKCTL
> which is under the GX power domain. Due to the way the support for this
> block is implemented in its driver, pm_runtime votes result in a vote on
> GX/GMxC/MxC rails from the APPS RSC. This is against the Adreno
> architecture which require GMU to be the sole voter of these collapsible
> rails on behalf of GPU, except during the GPU/GMU recovery.
> 
> To align with this architectural requirement and to realize the power
> benefits of the IFPC feature, remove the GXPD votes during gmu resume
> and suspend. And during the recovery sequence, enable/disable the GXPD
> along with the 'synced_poweroff' genpd hint to force collapse this GDSC.
> 
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---

Can we simply make this change unconditional on the gen, so as not to
maintain 2 separate code paths that try to achieve mostly the same thing?

Konrad

Re: [PATCH 4/6] clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC

[Konrad Dybcio]

On 4/7/26 11:30 AM, Taniya Das wrote:
> The GX GDSC control is handled through a dedicated clock controller,
> and the enable/disable sequencing depends on correct rail voting.
> The driver votes for the GX/GMxC rails and CX GDSC before toggling
> the GX GDSC. Currently, during GMU runtime PM resume, rails remain
> enabled due to upstream votes propagated via RPM-enabled devlinks
> and explicit pm_runtime votes on GX GDSC.
> 
> This is not an expected behaviour of IFPC(Inter Frame Power Collapse)
> requirements of GPU as GMU firmware is expected to control these rails,
> except during the GPU/GMU recovery via the OS and that is where the GX
> GDSC should be voting for the rails (GX/GMxC and CX GDSC) before
> toggling the GX GDSC.
> 
> Thus, disable runtime PM after successfully registering the clock
> controller.
> 
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/gxclkctl-kaanapali.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gxclkctl-kaanapali.c b/drivers/clk/qcom/gxclkctl-kaanapali.c
> index d7cf6834dd77c2a5320ffb8548cdb515be237bdc..d470ade11b0d11eb40843fe84c809e71646dce27 100644
> --- a/drivers/clk/qcom/gxclkctl-kaanapali.c
> +++ b/drivers/clk/qcom/gxclkctl-kaanapali.c
> @@ -7,6 +7,7 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  
>  #include <dt-bindings/clock/qcom,kaanapali-gxclkctl.h>
> @@ -61,7 +62,15 @@ MODULE_DEVICE_TABLE(of, gx_clkctl_kaanapali_match_table);
>  
>  static int gx_clkctl_kaanapali_probe(struct platform_device *pdev)
>  {
> -	return qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc);
> +	int ret;
> +
> +	ret = qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_disable(&pdev->dev);

My understanding is that this works because we have more than one domain
associated with the nod (so the generic code that would otherwise enable a
single one so long as the device is resumed doesn't apply) and your previous
patch ensures that after probe, the clock controller is being put to sleep,
right before pm_runtime_disable() executes.

Is that right?

Konrad

Re: [PATCH 6/6] drm/msm/a6xx: Limit GXPD votes to recovery in A8x

[Akhil P Oommen]

On 4/7/2026 4:31 PM, Konrad Dybcio wrote:
> On 4/7/26 11:30 AM, Taniya Das wrote:
>> From: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>
>> In A8x GPUs, the GX GDSC is moved to a separate block called GXCLKCTL
>> which is under the GX power domain. Due to the way the support for this
>> block is implemented in its driver, pm_runtime votes result in a vote on
>> GX/GMxC/MxC rails from the APPS RSC. This is against the Adreno
>> architecture which require GMU to be the sole voter of these collapsible
>> rails on behalf of GPU, except during the GPU/GMU recovery.
>>
>> To align with this architectural requirement and to realize the power
>> benefits of the IFPC feature, remove the GXPD votes during gmu resume
>> and suspend. And during the recovery sequence, enable/disable the GXPD
>> along with the 'synced_poweroff' genpd hint to force collapse this GDSC.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> ---
> 
> Can we simply make this change unconditional on the gen, so as not to
> maintain 2 separate code paths that try to achieve mostly the same thing?

We can skip the gdsc vote on A8x because the 'gdsc disable' callback is
dummy when the synced_poweroff hint is not set. Otherwise, gdsc may get
disabled during system resume while GMU assumes the ownership. Taniya
touched upon this point in the cover letter:

"""
when GX GDSC is managed through the generic GenPD runtime PM
framework, it can be unintentionally disabled by the OS during system
resume (resume_noirq/complete phases) or runtime PM suspend paths.
"""

-Akhil.

> 
> Konrad

Re: [PATCH 4/6] clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC

[Taniya Das]

On 4/7/2026 4:59 PM, Konrad Dybcio wrote:
>> 
>> #include <dt-bindings/clock/qcom,kaanapali-gxclkctl.h> @@ -61,7
>> +62,15 @@ MODULE_DEVICE_TABLE(of,
>> gx_clkctl_kaanapali_match_table);
>> 
>> static int gx_clkctl_kaanapali_probe(struct platform_device *pdev) 
>> { -	return qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc); +	int
>> ret; + +	ret = qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc); +
>> if (ret) +		return ret; + +	pm_runtime_disable(&pdev->dev);
> My understanding is that this works because we have more than one
> domain associated with the nod (so the generic code that would
> otherwise enable a single one so long as the device is resumed
> doesn't apply) and your previous patch ensures that after probe, the
> clock controller is being put to sleep, right before
> pm_runtime_disable() executes.

Konrad, by disabling runtime_pm will ensure that when GMU is moved to
runtime active the clock controller is not runtime resumed, which would
prevent the votes on the rails as per IFPC expectations.

-- 
Thanks,
Taniya Das

Re: [PATCH 3/6] clk: qcom: common: ensure runtime PM suspend completes on probe

[Taniya Das]

On 4/7/2026 4:28 PM, Konrad Dybcio wrote:
> On 4/7/26 11:30 AM, Taniya Das wrote:
>> When the clock controller is probed with 'use_rpm' enabled, the
>> runtime PM reference is currently released using pm_runtime_put(),
>> which may return before the runtime suspend has completed. This
>> can leave the power domain transition in progress while the probe
>> path continues or returns.
>>
>> Use pm_runtime_put_sync() instead to ensure the runtime PM “put”
>> completes synchronously during probe, guaranteeing the device is
>> fully runtime‑suspended before returning.
> 
> It's not immediately obvious why that's an issue, is there an
> observable problem when the transition doesn't complete in time?
> 
> Konrad


Calling pm_runtime_disable() immediately after pm_runtime_put() prevents
the runtime suspend from completing, leaving the clock controller active
and the HW rails in the ON state.


-- 
Thanks,
Taniya Das

Re: [PATCH 1/6] clk: qcom: gdsc: Add custom disable callback for GX GDSC

[Taniya Das]

On 4/7/2026 4:26 PM, Konrad Dybcio wrote:
>> @@ -675,3 +675,25 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>> +
>> +/*
>> + * GX GDSC is a special power domain. Normally, its disable sequence
>> + * is managed by the GMU firmware, and high level OS must not attempt
> nit: "and high level OS must not attempt" -> "and the OS must not attempt"
> 

Sure, will update in the next patch.

> 
>> + * to disable it. The only exception is during GMU recovery, where the
>> + * GMU driver can set GenPD’s synced_poweroff flag to allow explicitly
>> + * disable GX GDSC in hardware.
>> + */
>> +int gdsc_gx_disable(struct generic_pm_domain *domain)
>> +{
>> +	struct gdsc *sc = domain_to_gdsc(domain);
>> +
>> +	if (domain->synced_poweroff)
>> +		return gdsc_disable(domain);
> Can we use this version to replace gdsc_gx_do_nothing_enable() too?

No, Konrad, the enable should be always a no-op in every scenario.
synced_poweroff can't be used to handle enable of GDSC.


-- 
Thanks,
Taniya Das

Claude review: qcom: Refactor GPU GX GDSC handling and IFPC behavior on A8x GPUs

[Claude Code Review Bot]

Overall Series Review

Subject: qcom: Refactor GPU GX GDSC handling and IFPC behavior on A8x GPUs
Author: Taniya Das <taniya.das@oss.qualcomm.com>
Patches: 17
Reviewed: 2026-04-12T13:46:54.677976

---

This series addresses two real problems with GX GDSC ownership and rail voting on A8x (Kaanapali) GPUs. The cover letter is well-written and clearly explains both problem statements and their solutions. The approach of using `synced_poweroff` as a gate for OS-side GX collapse is sound and follows the existing pattern used for CX PD in the recovery path.

There are a few issues worth addressing:

1. **Missing `#else` stub in gdsc.h** (Patch 1) will cause build failures when `CONFIG_QCOM_GDSC` is not set.
2. **Patch 3 has broader scope than motivated** -- it changes behavior for all `use_rpm` clock controllers, not just gxclkctl.
3. **Unchecked `pm_runtime_get_sync()` return value** in Patch 6's `a6xx_gmu_gxpd_put()`.
4. **Ordering difference with a6xx_recover** in Patch 5, despite claiming to match that behavior.

---

---
Generated by Claude Code Patch Reviewer

Claude review: clk: qcom: gdsc: Add custom disable callback for GX GDSC

[Claude Code Review Bot]

Patch Review

The logic is clean and follows the existing `gdsc_gx_do_nothing_enable()` pattern nicely. The use of `domain->synced_poweroff` as the gate for actual hardware disable is a good design choice.

**Issue: Missing `#else` stub in gdsc.h.**

The header adds the declaration under `#ifdef CONFIG_QCOM_GDSC`:
```c
int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_gx_disable(struct generic_pm_domain *domain);
#else
```

But no corresponding `static inline` stub is added in the `#else` block (lines 92-100 of `gdsc.h`). The existing `gdsc_gx_do_nothing_enable()` also lacks an `#else` stub, so this is a pre-existing pattern gap, but since `gxclkctl-kaanapali.c` (Patch 2) references `gdsc_gx_disable` directly, this will cause a link error if `CONFIG_QCOM_GDSC=n`. Worth adding stubs for both functions.

**Minor:** The comment could be slightly more precise -- "allow explicitly disable" should read "allow explicit disable of".

---
Generated by Claude Code Patch Reviewer

Claude review: clk: qcom: gxclkctl: Use custom disable callback for gx_gdsc

[Claude Code Review Bot]

Patch Review

Straightforward one-liner. The assignment:
```c
.power_off = gdsc_gx_disable,
```
pairs correctly with the existing `.power_on = gdsc_gx_do_nothing_enable`. No issues.

The Fixes/Reported-by/Closes tags are well-placed on this patch since this is the one that actually fixes the reported symptoms.

---
Generated by Claude Code Patch Reviewer

Claude review: clk: qcom: common: ensure runtime PM suspend completes on probe

[Claude Code Review Bot]

Patch Review

```c
-		pm_runtime_put(dev);
+		pm_runtime_put_sync(dev);
```

**Concern: This is a broad change affecting all `use_rpm` clock controllers.** The `qcom_cc_really_probe()` function is shared infrastructure used by every Qualcomm clock controller with `use_rpm = true`. The commit message motivates this change specifically for gxclkctl/IFPC, but the change applies globally.

For most clock controllers, the async `pm_runtime_put()` is preferable because it avoids blocking probe unnecessarily. The synchronous variant adds latency to every such probe call. If the requirement is specific to gxclkctl (which Patch 4 then disables RPM on), consider whether this should be scoped more narrowly -- perhaps by adding a flag to `qcom_cc_desc` to opt into synchronous put, or by doing the sync in the gxclkctl probe itself.

Also, no Fixes tag is present. Is this a standalone fix or only needed as a prerequisite for Patch 4?

---
Generated by Claude Code Patch Reviewer

Claude review: clk: qcom: gxclkctl: Remove GX/GMxC rail votes to align with IFPC

[Claude Code Review Bot]

Patch Review

```c
+	ret = qcom_cc_probe(pdev, &gx_clkctl_kaanapali_desc);
+	if (ret)
+		return ret;
+
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
```

The approach of disabling runtime PM post-probe to prevent RPM request propagation from GMU's device links is reasonable.

**Minor:** `return ret;` where `ret` is guaranteed to be 0 -- `return 0;` would be marginally clearer, though this is a style nit.

**Concern: No cleanup path.** If the module is unbound/rebound, `pm_runtime_disable()` is called again on an already-disabled device. The PM core handles this gracefully (it's ref-counted via `disable_depth`), but repeated unload/reload could keep incrementing `disable_depth`. Since there's no `.remove` callback, there's no matching `pm_runtime_enable()`. This is probably fine in practice since this driver is essentially always-loaded, but worth noting.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/a8xx: Make a8xx_recover IFPC safe

[Claude Code Review Bot]

Patch Review

This correctly gates GX MMIO access behind a `gx_is_on()` check, preventing bus errors when GX is power-collapsed:

```c
+	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
+		gpu_write(gpu, REG_A8XX_CP_SQE_CNTL, 3);
+
+		if (hang_debug)
+			a8xx_dump(gpu);
+	}
```

**Ordering difference from a6xx_recover.** The commit message says "matching a6xx behavior", but the ordering differs. In `a6xx_recover()` (line 1648-1662 of `a6xx_gpu.c`), the sequence is:

```c
// a6xx_recover:
adreno_dump_info(gpu);
if (gx_is_on) { ... }     // GX check first
a6xx_gpu->hung = true;      // hung set after
```

In the patched `a8xx_recover`:
```c
// a8xx_recover (patched):
adreno_dump_info(gpu);
a6xx_gpu->hung = true;      // hung set first
if (gx_is_on) { ... }       // GX check after
```

The `hung` flag is set *before* the `gx_is_on` check in a8xx but *after* in a6xx. While this probably doesn't cause a functional problem (the flag is consumed during the subsequent RPM suspend), it would be better to match a6xx's ordering exactly if that's the stated intent. Consider moving `a6xx_gpu->hung = true` after the `gx_is_on` block.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/a6xx: Limit GXPD votes to recovery in A8x

[Claude Code Review Bot]

Patch Review

This is the most complex patch. The new helper functions cleanly separate the A6xx/A7xx and A8xx behaviors.

**Issue: Unchecked `pm_runtime_get_sync()` in `a6xx_gmu_gxpd_put()`.**

```c
	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
		pm_runtime_get_sync(gmu->gxpd);  // return value unchecked
		dev_pm_genpd_synced_poweroff(gmu->gxpd);
		pm_runtime_put_sync(gmu->gxpd);
	}
```

`pm_runtime_get_sync()` can fail (return negative). If it fails, the subsequent `synced_poweroff` + `put_sync` operate on a device with an incorrect refcount, potentially leaving GX in a bad state. At minimum, check the return value and log a warning, or propagate the error.

**Design observation:** The `a6xx_gmu_gxpd_put()` function returns `int` but the callers at the `a6xx_gmu_stop()` and `rpm_put:` error path in `a6xx_gmu_resume()` do not check the return value. For A6xx/A7xx, it returns the value from `pm_runtime_put_sync()`, which should be checked. For A8xx, it always returns 0 even if the internal `pm_runtime_get_sync()` failed. Consider making this void if return values aren't going to be checked, or actually check them at the call sites.

**Existing error path change:** In the `rpm_put:` label of `a6xx_gmu_resume()`:
```c
-	pm_runtime_put(gmu->gxpd);
+	a6xx_gmu_gxpd_put(gmu);
```

The old code called `pm_runtime_put()` (async) on `gmu->gxpd` directly, without NULL checking (the NULL check was only on the get side). If `gmu->gxpd` is NULL/ERR, calling `pm_runtime_put()` on it would crash. The new `a6xx_gmu_gxpd_put()` adds the `IS_ERR_OR_NULL` check, which is actually an improvement for the non-A8xx case too.

**Blank line removed between function definitions:**
```c
+}
+
 
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
```

The diff shows the extra blank line between `gdsc_gx_do_nothing_enable` and `a6xx_gmu_resume` being consumed, which is fine -- one blank line between functions is correct style.

---
Generated by Claude Code Patch Reviewer