[PATCH v2] drm/msm/dpu: fix mismatch between power and frequency

[PATCH v2] drm/msm/dpu: fix mismatch between power and frequency

[yuanjie yang]

From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>

During DPU runtime suspend, calling dev_pm_opp_set_rate(dev, 0) drops
the MMCX rail to MIN_SVS while the core clock frequency remains at its
original (highest) rate. When runtime resume re-enables the clock, this
may result in a mismatch between the rail voltage and the clock rate.

For example, in the DPU bind path, the sequence could be:
  cpu0: dev_sync_state -> rpmhpd_sync_state
  cpu1:                                     dpu_kms_hw_init
timeline 0 ------------------------------------------------> t

After rpmhpd_sync_state, the voltage performance is no longer guaranteed
to stay at the highest level. During dpu_kms_hw_init, calling
dev_pm_opp_set_rate(dev, 0) drops the voltage, causing the MMCX rail to
fall to MIN_SVS while the core clock is still at its maximum frequency.
When the power is re-enabled, only the clock is enabled, leading to a
situation where the MMCX rail is at MIN_SVS but the core clock is at its
highest rate. In this state, the rail cannot sustain the clock rate,
which may cause instability or system crash.

Remove the call to dev_pm_opp_set_rate(dev, 0) from dpu_runtime_suspend
to ensure the correct vote is restored when DPU resumes.

Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 449552513997..327881056dd1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1463,8 +1463,6 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
 	struct msm_drm_private *priv = platform_get_drvdata(pdev);
 	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
 
-	/* Drop the performance state vote */
-	dev_pm_opp_set_rate(dev, 0);
 	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
 
 	for (i = 0; i < dpu_kms->num_paths; i++)
-- 
2.43.0

Claude review: drm/msm/dpu: fix mismatch between power and frequency

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm/dpu: fix mismatch between power and frequency
Author: yuanjie yang <yuanjie.yang@oss.qualcomm.com>
Patches: 1
Reviewed: 2026-03-10T12:42:46.926861

---

This is a single-patch fix for a real power/clock mismatch bug in the DPU (Display Processing Unit) driver on Qualcomm SoCs. The problem is well-described: during runtime suspend, `dev_pm_opp_set_rate(dev, 0)` drops the MMCX power rail voltage to MIN_SVS, but the clock rate is not correspondingly lowered. On resume, `clk_bulk_prepare_enable()` re-enables the clock at its previous (high) rate while the rail is still at the low voltage, causing potential instability.

The fix is simple — remove the `dev_pm_opp_set_rate(dev, 0)` call from `dpu_runtime_suspend()`. This is a reasonable approach because:

1. The resume path (`dpu_runtime_resume`) never calls `dev_pm_opp_set_rate()` to restore the performance state, so there was already an asymmetry.
2. The performance state vote will be naturally cleaned up when clocks are disabled via `clk_bulk_disable_unprepare()`, since the OPP framework and runtime PM handle genpd voltage scaling when the device is fully suspended.
3. The active OPP rate is managed through `dpu_core_perf_crtc_update()` → `_dpu_core_perf_set_core_clk_rate()` during normal operation.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/dpu: fix mismatch between power and frequency

[Claude Code Review Bot]

Patch Review

**Commit message:** The commit message is well-written with a clear explanation of the race condition between `rpmhpd_sync_state` and `dpu_kms_hw_init`, and the resulting voltage/clock mismatch. The ASCII timeline diagram is a nice touch for illustrating the concurrency issue.

**Code change:**

```c
-	/* Drop the performance state vote */
-	dev_pm_opp_set_rate(dev, 0);
 	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
```

The removal is clean and correct. Two observations:

1. **Asymmetry concern is addressed:** The resume path at `dpu_kms.c:1485` only calls `clk_bulk_prepare_enable()` and never restores the OPP rate, so removing the drop in suspend eliminates the unpaired voltage reduction.

2. **Potential concern — performance state leak during suspend:** By not calling `dev_pm_opp_set_rate(dev, 0)`, the performance state vote from the last `dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate)` call in `dpu_core_perf.c:398` remains active even during suspend. However, since the clocks are being disabled and the ICC bandwidth is zeroed (`icc_set_bw(dpu_kms->path[i], 0, 0)` at line 1469), runtime PM should still allow the power domain to scale down appropriately once the device is fully suspended. The OPP framework will reconcile when genpd aggregates votes. This should be fine in practice, but **the maintainers should confirm** that not explicitly dropping the OPP vote doesn't prevent the power domain from reaching a low-power state during suspend on all affected platforms.

3. **Fixes tag is correct:** The referenced commit `b0530eb11913` is the one that introduced the OPP API usage in this driver.

**Verdict:** The patch looks correct and addresses a real bug. The fix is minimal and appropriate. The only question worth raising with the maintainers is whether the stale OPP vote could prevent optimal power savings during runtime suspend, but given that genpd aggregation handles this when the device is fully powered off, this should be fine.

---
Generated by Claude Code Patch Reviewer