[PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Arpit Saini]

pm_clk_suspend()/resume() only call clk_disable()/clk_enable() for the
AHB ("iface") clock, never clk_unprepare(), so prepare_count never
reaches 0. The clock (and its source) stays reported as enabled
through s2idle suspend even though the display stack is fully
suspended:

  disp_cc_mdss_ahb_clk [19200000] ->
    disp_cc_mdss_ahb_clk_src [19200000] ->
    bi_tcxo [19200000] -> xo-board [76800000]
  disp_cc_mdss_ahb_clk_src [19200000] ->
    bi_tcxo [19200000] -> xo-board [76800000]

Calling clk_prepare_enable()/clk_disable_unprepare() directly from the
PHY's runtime_suspend/runtime_resume is not safe either, since it can
deadlock against the PHY's own PLL clocks taking the global CCF
prepare_lock during clk_prepare()/clk_unprepare().

Fix this by moving clk_prepare()/clk_unprepare() out of the runtime PM
path. clk_enable()/clk_disable() (spinlock only) stay in
runtime_suspend/runtime_resume. clk_prepare()/clk_unprepare() move to
suspend_late/resume_early, avoiding the prepare_lock collision and
actually dropping prepare_count to 0. DPM_FLAG_NO_DIRECT_COMPLETE
ensures these hooks always run on sleep.

Fixes: 0b3ccb76b95b ("drm/msm/dsi: Fix 14nm DSI PHY PLL Lock issue")
Signed-off-by: Arpit Saini <arpit.saini@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 132 +++++++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h |   4 ++
 2 files changed, 127 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 1fb3899b88bf..3a8b5e23f601 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -3,9 +3,9 @@
  * Copyright (c) 2015, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/platform_device.h>
-#include <linux/pm_clock.h>
 #include <linux/pm_runtime.h>
 #include <dt-bindings/phy/phy.h>
 
@@ -610,6 +610,105 @@ static int dsi_phy_get_id(struct msm_dsi_phy *phy)
 	return -EINVAL;
 }
 
+static int dsi_phy_prepare_ahb_clk(struct msm_dsi_phy *phy)
+{
+	int ret;
+
+	if (clk_is_enabled_when_prepared(phy->ahb_clk) || phy->ahb_clk_prepared)
+		return 0;
+
+	ret = clk_prepare(phy->ahb_clk);
+	if (!ret)
+		phy->ahb_clk_prepared = true;
+
+	return ret;
+}
+
+static void dsi_phy_unprepare_ahb_clk(struct msm_dsi_phy *phy)
+{
+	if (phy->ahb_clk_prepared) {
+		clk_unprepare(phy->ahb_clk);
+		phy->ahb_clk_prepared = false;
+	}
+}
+
+static int dsi_phy_runtime_suspend(struct device *dev)
+{
+	struct msm_dsi_phy *phy = dev_get_drvdata(dev);
+
+	if (phy->ahb_clk_enabled) {
+		clk_disable(phy->ahb_clk);
+		phy->ahb_clk_enabled = false;
+	}
+
+	if (clk_is_enabled_when_prepared(phy->ahb_clk))
+		dsi_phy_unprepare_ahb_clk(phy);
+
+	return 0;
+}
+
+static int dsi_phy_runtime_resume(struct device *dev)
+{
+	struct msm_dsi_phy *phy = dev_get_drvdata(dev);
+	int ret;
+
+	if (clk_is_enabled_when_prepared(phy->ahb_clk)) {
+		ret = clk_prepare_enable(phy->ahb_clk);
+		if (ret)
+			return ret;
+
+		phy->ahb_clk_prepared = true;
+	} else {
+		/* Do not enable an unprepared clock after a failed system resume. */
+		if (!phy->ahb_clk_prepared)
+			return -EIO;
+
+		ret = clk_enable(phy->ahb_clk);
+		if (ret)
+			return ret;
+	}
+
+	phy->ahb_clk_enabled = true;
+
+	return 0;
+}
+
+static int dsi_phy_suspend_late(struct device *dev)
+{
+	struct msm_dsi_phy *phy = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	/* Runtime PM is quiesced, so it is safe to release preparation now. */
+	dsi_phy_unprepare_ahb_clk(phy);
+
+	return 0;
+}
+
+static int dsi_phy_resume_early(struct device *dev)
+{
+	struct msm_dsi_phy *phy = dev_get_drvdata(dev);
+	int ret, resume_ret;
+
+	ret = dsi_phy_prepare_ahb_clk(phy);
+	/* Balance force_suspend even if restoring preparation failed. */
+	resume_ret = pm_runtime_force_resume(dev);
+
+	return ret ?: resume_ret;
+}
+
+static void dsi_phy_release_ahb_clk(void *data)
+{
+	struct msm_dsi_phy *phy = data;
+
+	/* Runtime PM is either not enabled yet or has already been quiesced. */
+	dsi_phy_runtime_suspend(&phy->pdev->dev);
+	dsi_phy_unprepare_ahb_clk(phy);
+}
+
 static int dsi_phy_driver_probe(struct platform_device *pdev)
 {
 	struct msm_dsi_phy *phy;
@@ -683,17 +782,31 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	ret = devm_pm_runtime_enable(dev);
+	phy->ahb_clk = msm_clk_get(pdev, "iface");
+	if (IS_ERR(phy->ahb_clk))
+		return dev_err_probe(dev, PTR_ERR(phy->ahb_clk),
+				     "Unable to get iface clk\n");
+
+	/*
+	 * As a clock provider, the PHY can be runtime-resumed with the CCF
+	 * prepare_lock held. Keep clocks with separate enable operations
+	 * prepared across runtime PM, and only unprepare them during system
+	 * sleep (dsi_phy_suspend_late()/dsi_phy_resume_early()).
+	 */
+	ret = dsi_phy_prepare_ahb_clk(phy);
 	if (ret)
-		return ret;
+		return dev_err_probe(dev, ret, "Unable to prepare iface clk\n");
 
-	ret = devm_pm_clk_create(dev);
+	ret = devm_add_action_or_reset(dev, dsi_phy_release_ahb_clk, phy);
 	if (ret)
 		return ret;
 
-	ret = pm_clk_add(dev, "iface");
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "Unable to get iface clk\n");
+	/* Even a runtime-suspended PHY must release preparation for sleep. */
+	dev_pm_set_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
 
 	if (phy->cfg->ops.pll_init) {
 		ret = phy->cfg->ops.pll_init(phy);
@@ -712,7 +825,8 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
 }
 
 static const struct dev_pm_ops dsi_phy_pm_ops = {
-	SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+	RUNTIME_PM_OPS(dsi_phy_runtime_suspend, dsi_phy_runtime_resume, NULL)
+	LATE_SYSTEM_SLEEP_PM_OPS(dsi_phy_suspend_late, dsi_phy_resume_early)
 };
 
 static struct platform_driver dsi_phy_platform_driver = {
@@ -720,7 +834,7 @@ static struct platform_driver dsi_phy_platform_driver = {
 	.driver     = {
 		.name   = "msm_dsi_phy",
 		.of_match_table = dsi_phy_dt_match,
-		.pm = &dsi_phy_pm_ops,
+		.pm = pm_ptr(&dsi_phy_pm_ops),
 	},
 };
 
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index f5d3e806f8fd..77ae08dcff4b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -106,6 +106,10 @@ struct msm_dsi_phy {
 	phys_addr_t lane_size;
 	int id;
 
+	struct clk *ahb_clk;
+	bool ahb_clk_prepared;
+	bool ahb_clk_enabled;
+
 	struct regulator_bulk_data *supplies;
 
 	struct msm_dsi_dphy_timing timing;

---
base-commit: 22430ae5d90ab288b0ee2ad99ae941f4a666b694
change-id: 20261006-dsi-ahb-clk-suspend-fix-51cd279cccb4

Best regards,
-- 
Arpit Saini <arpit.saini@oss.qualcomm.com>

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Alexey Minnekhanov]

On 06.10.2026 16:13, Arpit Saini wrote:
> pm_clk_suspend()/resume() only call clk_disable()/clk_enable() for the
> AHB ("iface") clock, never clk_unprepare(), so prepare_count never
> reaches 0. The clock (and its source) stays reported as enabled
> through s2idle suspend even though the display stack is fully
> suspended:
> 
>    disp_cc_mdss_ahb_clk [19200000] ->
>      disp_cc_mdss_ahb_clk_src [19200000] ->
>      bi_tcxo [19200000] -> xo-board [76800000]
>    disp_cc_mdss_ahb_clk_src [19200000] ->
>      bi_tcxo [19200000] -> xo-board [76800000]
> 
> Calling clk_prepare_enable()/clk_disable_unprepare() directly from the
> PHY's runtime_suspend/runtime_resume is not safe either, since it can
> deadlock against the PHY's own PLL clocks taking the global CCF
> prepare_lock during clk_prepare()/clk_unprepare().
> 
> Fix this by moving clk_prepare()/clk_unprepare() out of the runtime PM
> path. clk_enable()/clk_disable() (spinlock only) stay in
> runtime_suspend/runtime_resume. clk_prepare()/clk_unprepare() move to
> suspend_late/resume_early, avoiding the prepare_lock collision and
> actually dropping prepare_count to 0. DPM_FLAG_NO_DIRECT_COMPLETE
> ensures these hooks always run on sleep.
> 
> Fixes: 0b3ccb76b95b ("drm/msm/dsi: Fix 14nm DSI PHY PLL Lock issue")
> Signed-off-by: Arpit Saini <arpit.saini@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 132 +++++++++++++++++++++++++++++++---
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h |   4 ++
>   2 files changed, 127 insertions(+), 9 deletions(-)
> 

Hi,

is this supposed to fix the message: "mdss_ahb_clk status stuck at 'on'"
during suspending?

--
Regards,
Alexey Minnekhanov

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Arpit Saini]

On 10/9/2026 5:28 AM, Alexey Minnekhanov wrote:
> On 06.10.2026 16:13, Arpit Saini wrote:
>> pm_clk_suspend()/resume() only call clk_disable()/clk_enable() for the
>> AHB ("iface") clock, never clk_unprepare(), so prepare_count never
>> reaches 0. The clock (and its source) stays reported as enabled
>> through s2idle suspend even though the display stack is fully
>> suspended:
>>
>>    disp_cc_mdss_ahb_clk [19200000] ->
>>      disp_cc_mdss_ahb_clk_src [19200000] ->
>>      bi_tcxo [19200000] -> xo-board [76800000]
>>    disp_cc_mdss_ahb_clk_src [19200000] ->
>>      bi_tcxo [19200000] -> xo-board [76800000]
>>
>> Calling clk_prepare_enable()/clk_disable_unprepare() directly from the
>> PHY's runtime_suspend/runtime_resume is not safe either, since it can
>> deadlock against the PHY's own PLL clocks taking the global CCF
>> prepare_lock during clk_prepare()/clk_unprepare().
>>
>> Fix this by moving clk_prepare()/clk_unprepare() out of the runtime PM
>> path. clk_enable()/clk_disable() (spinlock only) stay in
>> runtime_suspend/runtime_resume. clk_prepare()/clk_unprepare() move to
>> suspend_late/resume_early, avoiding the prepare_lock collision and
>> actually dropping prepare_count to 0. DPM_FLAG_NO_DIRECT_COMPLETE
>> ensures these hooks always run on sleep.
>>
>> Fixes: 0b3ccb76b95b ("drm/msm/dsi: Fix 14nm DSI PHY PLL Lock issue")
>> Signed-off-by: Arpit Saini <arpit.saini@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 132 +++++++++++++++++++++++++++++++---
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h |   4 ++
>>   2 files changed, 127 insertions(+), 9 deletions(-)
>>
> 
> Hi,
> 
> is this supposed to fix the message: "mdss_ahb_clk status stuck at 'on'"
> during suspending?
> 
> -- 
> Regards,
> Alexey Minnekhanov

Hi Alexey, 

Yes it will fix the 
mdss_ahb_clk status stuck at 'on' during suspend.

Thanks,
Arpit

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Joonhoe Kim]

(Resending with the lists on Cc; my earlier reply only went to Arpit.)

On Tue, Oct 06, 2026 at 06:43:53PM +0530, Arpit Saini wrote:
> pm_clk_suspend()/resume() only call clk_disable()/clk_enable() for the
> AHB ("iface") clock, never clk_unprepare(), so prepare_count never
> reaches 0. The clock (and its source) stays reported as enabled
> through s2idle suspend even though the display stack is fully
> suspended:

I looked at this on a Lenovo Legion Tab Y700 Gen 5 (SM8850, bonded DSI,
v7.3-rc4), comparing the current code, this patch's approach, and
preparing the clock only in the PHY runtime callbacks.

In s2idle with the display on I see no difference: in all three cases
both PHYs are already runtime suspended at system suspend,
disp_cc_mdss_ahb_clk has enable_count 0, and CX collapse during a 100 s
s2idle is the same. Is the clock really enabled during suspend on your
board, or only prepared? Reading clk_summary runtime-resumes the clock
controllers, so it can show a different state.

The difference here is with the display off and the system awake: with
the clock prepared from probe (current code and this patch), dispcc
stays runtime active and MMCX stays on. Preparing it in the runtime
callbacks lets both go off. I have not measured the power difference.

That variant survived 30 DPMS off/on cycles with clk_summary read in a
loop. Did you hit the prepare_lock deadlock, or is it from code
reading?

Thanks,
Joonhoe Kim

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Arpit Saini]

(Resending with the lists on Cc; I earlier replied only to Kim.)

On 10/16/2026 6:20 PM, Joonhoe Kim wrote:
> (Resending with the lists on Cc; my earlier reply only went to Arpit.)
> 
> On Tue, Oct 06, 2026 at 06:43:53PM +0530, Arpit Saini wrote:
>> pm_clk_suspend()/resume() only call clk_disable()/clk_enable() for the
>> AHB ("iface") clock, never clk_unprepare(), so prepare_count never
>> reaches 0. The clock (and its source) stays reported as enabled
>> through s2idle suspend even though the display stack is fully
>> suspended:
> 
> I looked at this on a Lenovo Legion Tab Y700 Gen 5 (SM8850, bonded DSI,
> v7.3-rc4), comparing the current code, this patch's approach, and
> preparing the clock only in the PHY runtime callbacks.
> 
> In s2idle with the display on I see no difference: in all three cases
> both PHYs are already runtime suspended at system suspend,
> disp_cc_mdss_ahb_clk has enable_count 0, and CX collapse during a 100 s
> s2idle is the same. Is the clock really enabled during suspend on your
> board, or only prepared? Reading clk_summary runtime-resumes the clock
> controllers, so it can show a different state.
> 

Hi Kim,

Thanks for your observation,

The failure is that :

[   55.005539]  disp_cc_mdss_ahb_clk [19200000] (enable=0 prepare=1) ->

enable count is indeed zero, but the prepare is still non-zero.

The enable_count is correctly reduced to zero by pm_clk_suspend().

However, pm_clk_suspend() only calls clk_disable() and does not call clk_unprepare(). 

Therefore, the preparation reference acquired by pm_clk_add() remains held during s2idle.

> The difference here is with the display off and the system awake: with
> the clock prepared from probe (current code and this patch), dispcc
> stays runtime active and MMCX stays on. Preparing it in the runtime
> callbacks lets both go off. I have not measured the power difference.
> 
> That variant survived 30 DPMS off/on cycles with clk_summary read in a
> loop. Did you hit the prepare_lock deadlock, or is it from code
> reading?
> 

The reason for avoiding clk_prepare()/clk_unprepare() in the PHY
runtime callbacks is that the PHY is also a clock provider, so its
runtime resume could potentially be invoked while CCF holds
prepare_lock.

Agreed the CCF prepare_lock deadlock is currently a theoretical
concern. I have not reproduced it.

Thanks,
Arpit

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[Qianhui Xu]

Hi Joonhoe,

Thanks for the detailed data points in this thread.  We are bringing up an
SM8750P board with a bonded dual-DSI panel (NT36536) on a v7.3-rc7 based tree
(this file is unchanged in 7.3 final), and your "display off, system awake"
observation matches what we see.

Measured with DPMS off (org.gnome.ScreenSaver.SetActive(true)), reading
power/runtime_status before and after, in this order:

  ae00000.display-subsystem                     active -> suspended
  .../ae01000.display-controller                active -> suspended
  .../ae94000.dsi                               active -> suspended
  af00000.clock-controller (dispcc)             active -> ACTIVE

So the DSI/MDSS devices do runtime-suspend, but dispcc keeps
runtime_status=active.  We then read clk_summary last (we are aware it
runtime-resumes clock controllers, hence last) and while blanked it shows
disp_cc_mdss_mdp_clk with enable=prepare=1 still held by
display-controller@ae01000, and disp_cc_mdss_ahb_clk at 3 with one consumer
being af54000.displayport-controller.

Question: with your runtime-callback variant, does dispcc end up suspended
after DPMS off, and does the panel still come back on DPMS on?  We are trying
to tell whether the remaining dispcc reference is a DSI/PHY artifact or
something on the DPU/DP side.

Separately, on a fraction of cold boots our panel init fails (first DCS
command DMA timeout) and in that state DPU core_irq stops incrementing within
~3 s while DSI interrupts stay at 0.  We have no causal link to the dispcc
runtime state yet; just noting it in case it is useful.

No logs needed from you -- just curious whether "dispcc stays active" is a
shared property.

Thanks,
Qianhui

Re: [PATCH] drm/msm/dsi: fix AHB clock staying enabled across suspend

[joonhoekim]

From: Joonhoe Kim <26rote@gmail.com>

On Mon, Oct 19, 2026 at 09:10:28PM +0800, Qianhui Xu wrote:
> Question: with your runtime-callback variant, does dispcc end up suspended
> after DPMS off, and does the panel still come back on DPMS on?

Yes to both, on SM8850 with v7.3-rc4. With the display off and the
system awake:

  current code and Arpit's patch: disp_cc_mdss_ahb_clk prepared twice,
                                  dispcc runtime active, MMCX on
  prepare only in the PHY runtime callbacks: prepare count 0,
                                  dispcc runtime suspended, MMCX off

The panel came back on every one of 30 DPMS off/on cycles with that
variant. I repeated it today with the same variant on v7.3-rc4,
blanking both outputs from the compositor, with nothing on
DisplayPort and with four different sinks (a 4K TV and monitors at
5120x2160, 2560x1600 and 1920x2400):

  nothing on DP: DPU, both DSI and their PHYs, the DP controller, MDSS
                 and dispcc all runtime suspended,
                 disp_cc_mdss_ahb_clk enable=0 prepare=0
  sink on DP:    DPU, DSI and PHYs suspended, but the DP controller,
                 MDSS and dispcc stay runtime active,
                 disp_cc_mdss_ahb_clk enable=2 prepare=2

So here the remaining dispcc reference comes from the DP side while a
sink is connected, and the display controller does not keep the MDP
clock when blanked. This tree carries my DP patch that holds one
runtime PM reference per plugged sink, so the DP part may differ from
mainline.

Arpit, thanks for confirming it is prepare=1 with enable=0 on your
board during s2idle; that matches what I see. I could not measure a
difference from it in s2idle (CX collapse counts were the same in all
three cases), only in the display-off, system-awake case above.

Thanks,
Joonhoe Kim