[PATCH 0/2] drm/msm: fix mismatch between power and frequency

[PATCH 0/2] drm/msm: 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.

Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
---
Yuanjie Yang (2):
  drm/msm/dpu: fix mismatch between power and frequency
  drm/msm/dpu: use max_freq replace max_core_clk_rate

 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 ++++++++++++++--------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
 2 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH 1/2] 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.

Fix this by setting the corresponding OPP corner during both power-on
and power-off sequences to ensure proper alignment of rail voltage and
clock frequency.

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 | 16 ++++++++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0623f1dbed97..c31488335f2b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
 	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
 	struct dev_pm_opp *opp;
 	int ret = 0;
-	unsigned long max_freq = ULONG_MAX;
+	dpu_kms->max_freq = ULONG_MAX;
+	dpu_kms->min_freq = 0;
 
-	opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
+	opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
+	if (!IS_ERR(opp))
+		dev_pm_opp_put(opp);
+
+	opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
 	if (!IS_ERR(opp))
 		dev_pm_opp_put(opp);
 
@@ -1461,8 +1466,8 @@ 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);
+	/* adjust the performance state vote to low performance state */
+	dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
 	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
 
 	for (i = 0; i < dpu_kms->num_paths; i++)
@@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 	struct drm_device *ddev;
 
 	ddev = dpu_kms->dev;
+	/* adjust the performance state vote to high performance state */
+	if (dpu_kms->max_freq != ULONG_MAX)
+		dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
 
 	rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks);
 	if (rc) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 993cf512f8c5..8d2595d8a5f6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -92,6 +92,9 @@ struct dpu_kms {
 	struct clk_bulk_data *clocks;
 	size_t num_clocks;
 
+	unsigned long max_freq;
+	unsigned long min_freq;
+
 	/* reference count bandwidth requests, so we know when we can
 	 * release bandwidth.  Each atomic update increments, and frame-
 	 * done event decrements.  Additionally, for video mode, the
-- 
2.34.1

[PATCH 2/2] drm/msm/dpu: use max_freq replace max_core_clk_rate

[yuanjie yang]

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

Enable power will use max_freq to set opp, so remove redundant
opp setting, and use max_freq to replace max_core_clk_rate.

Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index c31488335f2b..973fec1c328c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1233,9 +1233,9 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 		dpu_kms->hw_vbif[vbif->id] = hw;
 	}
 
-	/* TODO: use the same max_freq as in dpu_kms_hw_init */
-	max_core_clk_rate = dpu_kms_get_clk_rate(dpu_kms, "core");
-	if (!max_core_clk_rate) {
+	if (dpu_kms->max_freq != ULONG_MAX) {
+		max_core_clk_rate = dpu_kms->max_freq;
+	} else {
 		DPU_DEBUG("max core clk rate not determined, using default\n");
 		max_core_clk_rate = DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE;
 	}
@@ -1317,8 +1317,6 @@ static int dpu_kms_init(struct drm_device *ddev)
 	if (!IS_ERR(opp))
 		dev_pm_opp_put(opp);
 
-	dev_pm_opp_set_rate(dev, max_freq);
-
 	ret = msm_kms_init(&dpu_kms->base, &kms_funcs);
 	if (ret) {
 		DPU_ERROR("failed to init kms, ret=%d\n", ret);
-- 
2.34.1

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 1/9/26 9:38 AM, yuanjie yang wrote:
> 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.

So what this message essentially says is that dev_pm_opp_set_rate(dev, 0)
doesn't actually set the rate of "0" (or any other rate, unless opp-level
is at play), nor does it disable the clock.

Seems like a couple of our drivers make this oversight..

I see that originally calling dev_pm_opp_set_rate(dev, 0) was forbidden,
up until Commit cd7ea582866f ("opp: Make dev_pm_opp_set_rate() handle freq
= 0 to drop performance votes")..

In fact,

$ rg 'dev_pm_opp_set_rate\(.*, 0\)'

returns exclusively Qualcomm drivers where I believe the intention is always
to disable the clock.. perhaps we should just do that instead. We don't have
to worry about setting F_MIN beforehand, because a disabled clock won't be
eating up power and when enabling it back up, calling opp_set_rate with a
non-zero frequency will bring back the rails to a suitable level

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> 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.

Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
doesn't do anything with clocks. I think we should have a call to
clk_disable_unprepare() before that and clk_prepare_enable() in the
resume path.

> 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.
> 
> Fix this by setting the corresponding OPP corner during both power-on
> and power-off sequences to ensure proper alignment of rail voltage and
> clock frequency.
> 
> Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> 
> Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>

No empty lines between the tags. Also please cc stable.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 0623f1dbed97..c31488335f2b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
>  	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>  	struct dev_pm_opp *opp;
>  	int ret = 0;
> -	unsigned long max_freq = ULONG_MAX;
> +	dpu_kms->max_freq = ULONG_MAX;
> +	dpu_kms->min_freq = 0;
>  
> -	opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> +	opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> +	if (!IS_ERR(opp))
> +		dev_pm_opp_put(opp);
> +
> +	opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
>  	if (!IS_ERR(opp))
>  		dev_pm_opp_put(opp);
>  
> @@ -1461,8 +1466,8 @@ 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);
> +	/* adjust the performance state vote to low performance state */
> +	dev_pm_opp_set_rate(dev, dpu_kms->min_freq);

Here min_freq is the minumum working frequency, which will keep it
ticking at a high frequency.  I think we are supposed to turn it off
(well, switch to XO). Would it be enough to swap these two lines
instead?

>  	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>  
>  	for (i = 0; i < dpu_kms->num_paths; i++)
> @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
>  	struct drm_device *ddev;
>  
>  	ddev = dpu_kms->dev;
> +	/* adjust the performance state vote to high performance state */
> +	if (dpu_kms->max_freq != ULONG_MAX)
> +		dev_pm_opp_set_rate(dev, dpu_kms->max_freq);

This one should not be necessary, we should be setting the performance
point while comitting the DRM state.

>  
>  	rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks);
>  	if (rc) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 993cf512f8c5..8d2595d8a5f6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -92,6 +92,9 @@ struct dpu_kms {
>  	struct clk_bulk_data *clocks;
>  	size_t num_clocks;
>  
> +	unsigned long max_freq;
> +	unsigned long min_freq;
> +
>  	/* reference count bandwidth requests, so we know when we can
>  	 * release bandwidth.  Each atomic update increments, and frame-
>  	 * done event decrements.  Additionally, for video mode, the
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[yuanjiey]

On Fri, Jan 09, 2026 at 11:44:48AM +0100, Konrad Dybcio wrote:
> On 1/9/26 9:38 AM, yuanjie yang wrote:
> > 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.
> 
> So what this message essentially says is that dev_pm_opp_set_rate(dev, 0)
> doesn't actually set the rate of "0" (or any other rate, unless opp-level
> is at play), nor does it disable the clock.
> 
> Seems like a couple of our drivers make this oversight..
> 
> I see that originally calling dev_pm_opp_set_rate(dev, 0) was forbidden,
> up until Commit cd7ea582866f ("opp: Make dev_pm_opp_set_rate() handle freq
> = 0 to drop performance votes")..
> 
> In fact,
> 
> $ rg 'dev_pm_opp_set_rate\(.*, 0\)'
> 
> returns exclusively Qualcomm drivers where I believe the intention is always
> to disable the clock.. perhaps we should just do that instead. We don't have
> to worry about setting F_MIN beforehand, because a disabled clock won't be
> eating up power and when enabling it back up, calling opp_set_rate with a
> non-zero frequency will bring back the rails to a suitable level

Yes, calling opp_set_rate with a non-zero frequency will bring back 
the rails to a suitable level. The other steps are unnecessary.

And here I just choose the highest freq, because I see 
dpu binding patch:
dpu_kms_init(struct drm_device *ddev) use highest freq to do initialization. 

I want to keep it consistent with the previous initialization logic.

Thanks,
Yuanjie

> 
> Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[yuanjiey]

On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> > 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.
> 
> Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
> doesn't do anything with clocks. I think we should have a call to
> clk_disable_unprepare() before that and clk_prepare_enable() in the
> resume path.

I do check the backtrace on kaanapali: 

active_corner = 3 (MIN_SVS)
rpmhpd_aggregate_corner+0x168/0x1d0
rpmhpd_set_performance_state+0x84/0xd0
_genpd_set_performance_state+0x50/0x198
genpd_set_performance_state.isra.0+0xbc/0xdc
genpd_dev_pm_set_performance_state+0x60/0xc4
dev_pm_domain_set_performance_state+0x24/0x3c
_set_opp+0x370/0x584
dev_pm_opp_set_rate+0x258/0x2b4
dpu_runtime_suspend+0x50/0x11c [msm]
pm_generic_runtime_suspend+0x2c/0x44
genpd_runtime_suspend+0xac/0x2d0
__rpm_callback+0x48/0x19c
rpm_callback+0x74/0x80
rpm_suspend+0x108/0x588
rpm_idle+0xe8/0x190
__pm_runtime_idle+0x50/0x94
dpu_kms_hw_init+0x3a0/0x4a8 

dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS. 
And freq MDP: 650MHz


And I try change the order:
from: 
	dev_pm_opp_set_rate(dev, 0);
	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
to:
	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
	dev_pm_opp_set_rate(dev, 0);

But the issue is still here.

 
> > 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.
> > 
> > Fix this by setting the corresponding OPP corner during both power-on
> > and power-off sequences to ensure proper alignment of rail voltage and
> > clock frequency.
> > 
> > Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> > 
> > Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> 
> No empty lines between the tags. Also please cc stable.

Sure, will fix.

> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
> >  2 files changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 0623f1dbed97..c31488335f2b 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
> >  	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> >  	struct dev_pm_opp *opp;
> >  	int ret = 0;
> > -	unsigned long max_freq = ULONG_MAX;
> > +	dpu_kms->max_freq = ULONG_MAX;
> > +	dpu_kms->min_freq = 0;
> >  
> > -	opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> > +	opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> > +	if (!IS_ERR(opp))
> > +		dev_pm_opp_put(opp);
> > +
> > +	opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
> >  	if (!IS_ERR(opp))
> >  		dev_pm_opp_put(opp);
> >  
> > @@ -1461,8 +1466,8 @@ 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);
> > +	/* adjust the performance state vote to low performance state */
> > +	dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
> 
> Here min_freq is the minumum working frequency, which will keep it
> ticking at a high frequency.  I think we are supposed to turn it off
> (well, switch to XO). Would it be enough to swap these two lines
> instead? 

Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
disable clk is OK. 
we can drop change here.

> >  	clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >  
> >  	for (i = 0; i < dpu_kms->num_paths; i++)
> > @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
> >  	struct drm_device *ddev;
> >  
> >  	ddev = dpu_kms->dev;
> > +	/* adjust the performance state vote to high performance state */
> > +	if (dpu_kms->max_freq != ULONG_MAX)
> > +		dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
> 
> This one should not be necessary, we should be setting the performance
> point while comitting the DRM state.

No, issue is during dpu binding path. And in msm_drm_kms_init driver just
pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
But We do not set the appropriate OPP each time the power is enabled. 
As a result, a situation may occur where the rail stays at MIN_SVS while the 
MDP is running at a high frequency.

rpm_idle+0xe8/0x190
__pm_runtime_idle+0x50/0x94
dpu_kms_hw_init+0x3a0/0x4a8 [msm]
msm_drm_kms_init+0xb8/0x340 [msm]
msm_drm_init+0x16c/0x22c [msm]
msm_drm_bind+0x30/0x3c [msm]
try_to_bring_up_aggregate_device+0x168/0x1d4
__component_add+0xa4/0x170
component_add+0x14/0x20
dsi_dev_attach+0x20/0x2c [msm]
dsi_host_attach+0x58/0x98 [msm]
mipi_dsi_attach+0x30/0x54
novatek_nt37801_probe+0x13c/0x1c8 [panel_novatek_nt37801]

And I found a a similar bug.
https://lore.kernel.org/all/20220915205559.14574-1-quic_bjorande@quicinc.com/

Since the panel driver does not hold the property power-domains = <&rpmhpd RPMHPD_MMCX> 
once all drivers that do own the RPMHPD_MMCX power domain finish probing, 
the interconnect’s dev_sync_state is triggered, which eventually runs rpmhpd_sync_state 
and starts dynamic voltage adjustment. This is when the issue occurs.


if do change below, this issue can also be fixed.
&mdss_dsi0 {
    ...
	panel@0 {
		compatible = "novatek,nt37801";
        	...
	++	power-domains = <&rpmhpd RPMHPD_MMCX>;
    }
}
But I don't think panel driver should own a power-domains = <&rpmhpd RPMHPD_MMCX>;



Thanks,
Yuanjie

> >  
> >  	rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks);
> >  	if (rc) {
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > index 993cf512f8c5..8d2595d8a5f6 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > @@ -92,6 +92,9 @@ struct dpu_kms {
> >  	struct clk_bulk_data *clocks;
> >  	size_t num_clocks;
> >  
> > +	unsigned long max_freq;
> > +	unsigned long min_freq;
> > +
> >  	/* reference count bandwidth requests, so we know when we can
> >  	 * release bandwidth.  Each atomic update increments, and frame-
> >  	 * done event decrements.  Additionally, for video mode, the
> > -- 
> > 2.34.1
> > 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>
> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> > On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> > > 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.
> >
> > Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
> > doesn't do anything with clocks. I think we should have a call to
> > clk_disable_unprepare() before that and clk_prepare_enable() in the
> > resume path.
>
> I do check the backtrace on kaanapali:
>
> active_corner = 3 (MIN_SVS)
> rpmhpd_aggregate_corner+0x168/0x1d0
> rpmhpd_set_performance_state+0x84/0xd0
> _genpd_set_performance_state+0x50/0x198
> genpd_set_performance_state.isra.0+0xbc/0xdc
> genpd_dev_pm_set_performance_state+0x60/0xc4
> dev_pm_domain_set_performance_state+0x24/0x3c
> _set_opp+0x370/0x584
> dev_pm_opp_set_rate+0x258/0x2b4
> dpu_runtime_suspend+0x50/0x11c [msm]
> pm_generic_runtime_suspend+0x2c/0x44
> genpd_runtime_suspend+0xac/0x2d0
> __rpm_callback+0x48/0x19c
> rpm_callback+0x74/0x80
> rpm_suspend+0x108/0x588
> rpm_idle+0xe8/0x190
> __pm_runtime_idle+0x50/0x94
> dpu_kms_hw_init+0x3a0/0x4a8
>
> dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS.
> And freq MDP: 650MHz
>
>
> And I try change the order:
> from:
>         dev_pm_opp_set_rate(dev, 0);
>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> to:
>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>         dev_pm_opp_set_rate(dev, 0);
>
> But the issue is still here.

But which clock is still demanding higher MMCX voltage? All DPU clocks
should be turned off at this point.

>
>
> > > 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.
> > >
> > > Fix this by setting the corresponding OPP corner during both power-on
> > > and power-off sequences to ensure proper alignment of rail voltage and
> > > clock frequency.
> > >
> > > Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> > >
> > > Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >
> > No empty lines between the tags. Also please cc stable.
>
> Sure, will fix.
>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
> > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > index 0623f1dbed97..c31488335f2b 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
> > >     struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> > >     struct dev_pm_opp *opp;
> > >     int ret = 0;
> > > -   unsigned long max_freq = ULONG_MAX;
> > > +   dpu_kms->max_freq = ULONG_MAX;
> > > +   dpu_kms->min_freq = 0;
> > >
> > > -   opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> > > +   opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> > > +   if (!IS_ERR(opp))
> > > +           dev_pm_opp_put(opp);
> > > +
> > > +   opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
> > >     if (!IS_ERR(opp))
> > >             dev_pm_opp_put(opp);
> > >
> > > @@ -1461,8 +1466,8 @@ 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);
> > > +   /* adjust the performance state vote to low performance state */
> > > +   dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
> >
> > Here min_freq is the minumum working frequency, which will keep it
> > ticking at a high frequency.  I think we are supposed to turn it off
> > (well, switch to XO). Would it be enough to swap these two lines
> > instead?
>
> Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
> disable clk is OK.
> we can drop change here.
>
> > >     clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> > >
> > >     for (i = 0; i < dpu_kms->num_paths; i++)
> > > @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
> > >     struct drm_device *ddev;
> > >
> > >     ddev = dpu_kms->dev;
> > > +   /* adjust the performance state vote to high performance state */
> > > +   if (dpu_kms->max_freq != ULONG_MAX)
> > > +           dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
> >
> > This one should not be necessary, we should be setting the performance
> > point while comitting the DRM state.
>
> No, issue is during dpu binding path. And in msm_drm_kms_init driver just
> pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
> But We do not set the appropriate OPP each time the power is enabled.
> As a result, a situation may occur where the rail stays at MIN_SVS while the
> MDP is running at a high frequency.

Please move dev_pm_opp_set_rate() from dpu_kms_init() to dpu_kms_hw_init().

>
> rpm_idle+0xe8/0x190
> __pm_runtime_idle+0x50/0x94
> dpu_kms_hw_init+0x3a0/0x4a8 [msm]
> msm_drm_kms_init+0xb8/0x340 [msm]
> msm_drm_init+0x16c/0x22c [msm]
> msm_drm_bind+0x30/0x3c [msm]
> try_to_bring_up_aggregate_device+0x168/0x1d4
> __component_add+0xa4/0x170
> component_add+0x14/0x20
> dsi_dev_attach+0x20/0x2c [msm]
> dsi_host_attach+0x58/0x98 [msm]
> mipi_dsi_attach+0x30/0x54
> novatek_nt37801_probe+0x13c/0x1c8 [panel_novatek_nt37801]
>
> And I found a a similar bug.
> https://lore.kernel.org/all/20220915205559.14574-1-quic_bjorande@quicinc.com/
>
> Since the panel driver does not hold the property power-domains = <&rpmhpd RPMHPD_MMCX>
> once all drivers that do own the RPMHPD_MMCX power domain finish probing,
> the interconnect’s dev_sync_state is triggered, which eventually runs rpmhpd_sync_state
> and starts dynamic voltage adjustment. This is when the issue occurs.
>
>
> if do change below, this issue can also be fixed.
> &mdss_dsi0 {
>     ...
>         panel@0 {
>                 compatible = "novatek,nt37801";
>                 ...
>         ++      power-domains = <&rpmhpd RPMHPD_MMCX>;
>     }
> }
> But I don't think panel driver should own a power-domains = <&rpmhpd RPMHPD_MMCX>;

That's not related.

>
>
>
> Thanks,
> Yuanjie
>
> > >
> > >     rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks);
> > >     if (rc) {
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > index 993cf512f8c5..8d2595d8a5f6 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > @@ -92,6 +92,9 @@ struct dpu_kms {
> > >     struct clk_bulk_data *clocks;
> > >     size_t num_clocks;
> > >
> > > +   unsigned long max_freq;
> > > +   unsigned long min_freq;
> > > +
> > >     /* reference count bandwidth requests, so we know when we can
> > >      * release bandwidth.  Each atomic update increments, and frame-
> > >      * done event decrements.  Additionally, for video mode, the
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[yuanjiey]

On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >
> > On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> > > On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> > > > 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.
> > >
> > > Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
> > > doesn't do anything with clocks. I think we should have a call to
> > > clk_disable_unprepare() before that and clk_prepare_enable() in the
> > > resume path.
> >
> > I do check the backtrace on kaanapali:
> >
> > active_corner = 3 (MIN_SVS)
> > rpmhpd_aggregate_corner+0x168/0x1d0
> > rpmhpd_set_performance_state+0x84/0xd0
> > _genpd_set_performance_state+0x50/0x198
> > genpd_set_performance_state.isra.0+0xbc/0xdc
> > genpd_dev_pm_set_performance_state+0x60/0xc4
> > dev_pm_domain_set_performance_state+0x24/0x3c
> > _set_opp+0x370/0x584
> > dev_pm_opp_set_rate+0x258/0x2b4
> > dpu_runtime_suspend+0x50/0x11c [msm]
> > pm_generic_runtime_suspend+0x2c/0x44
> > genpd_runtime_suspend+0xac/0x2d0
> > __rpm_callback+0x48/0x19c
> > rpm_callback+0x74/0x80
> > rpm_suspend+0x108/0x588
> > rpm_idle+0xe8/0x190
> > __pm_runtime_idle+0x50/0x94
> > dpu_kms_hw_init+0x3a0/0x4a8
> >
> > dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS.
> > And freq MDP: 650MHz
> >
> >
> > And I try change the order:
> > from:
> >         dev_pm_opp_set_rate(dev, 0);
> >         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> > to:
> >         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >         dev_pm_opp_set_rate(dev, 0);
> >
> > But the issue is still here.
> 
> But which clock is still demanding higher MMCX voltage? All DPU clocks
> should be turned off at this point.
Yes, no DPU clock demand higher MMCX voltage here. But next time pm_runtime_get_sync
need higher MMCX voltagei due to high freq.
 
> >
> >
> > > > 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.
> > > >
> > > > Fix this by setting the corresponding OPP corner during both power-on
> > > > and power-off sequences to ensure proper alignment of rail voltage and
> > > > clock frequency.
> > > >
> > > > Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> > > >
> > > > Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> > >
> > > No empty lines between the tags. Also please cc stable.
> >
> > Sure, will fix.
> >
> > > > ---
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
> > > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > > index 0623f1dbed97..c31488335f2b 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > > @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
> > > >     struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> > > >     struct dev_pm_opp *opp;
> > > >     int ret = 0;
> > > > -   unsigned long max_freq = ULONG_MAX;
> > > > +   dpu_kms->max_freq = ULONG_MAX;
> > > > +   dpu_kms->min_freq = 0;
> > > >
> > > > -   opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> > > > +   opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> > > > +   if (!IS_ERR(opp))
> > > > +           dev_pm_opp_put(opp);
> > > > +
> > > > +   opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
> > > >     if (!IS_ERR(opp))
> > > >             dev_pm_opp_put(opp);
> > > >
> > > > @@ -1461,8 +1466,8 @@ 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);
> > > > +   /* adjust the performance state vote to low performance state */
> > > > +   dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
> > >
> > > Here min_freq is the minumum working frequency, which will keep it
> > > ticking at a high frequency.  I think we are supposed to turn it off
> > > (well, switch to XO). Would it be enough to swap these two lines
> > > instead?
> >
> > Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
> > disable clk is OK.
> > we can drop change here.
> >
> > > >     clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> > > >
> > > >     for (i = 0; i < dpu_kms->num_paths; i++)
> > > > @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
> > > >     struct drm_device *ddev;
> > > >
> > > >     ddev = dpu_kms->dev;
> > > > +   /* adjust the performance state vote to high performance state */
> > > > +   if (dpu_kms->max_freq != ULONG_MAX)
> > > > +           dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
> > >
> > > This one should not be necessary, we should be setting the performance
> > > point while comitting the DRM state.
> >
> > No, issue is during dpu binding path. And in msm_drm_kms_init driver just
> > pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
> > But We do not set the appropriate OPP each time the power is enabled.
> > As a result, a situation may occur where the rail stays at MIN_SVS while the
> > MDP is running at a high frequency.
> 
> Please move dev_pm_opp_set_rate() from dpu_kms_init() to dpu_kms_hw_init().

During dpu_kms_hw_init and msm_drm_kms_init and msm_drm_kms_post_init.

There are multiple places where pm_runtime_get_sync(pm_runtime_resume_and_get)and pm_runtime_put_sync are called.
And each time after pm_runtime_get_sync(pm_runtime_resume_and_get) will access register depond on MDP clk.

Do I need to add dev_pm_opp_set_rate after every pm_runtime_get_sync and pm_runtime_resume_and_get?

pm_runtime_get_sync
dev_pm_opp_set_rate
"access register"
pm_runtime_put_sync

pm_runtime_resume_and_get
dev_pm_opp_set_rate
"access register"
pm_runtime_put_sync

Thanks,
Yuanjie

> >
> > rpm_idle+0xe8/0x190
> > __pm_runtime_idle+0x50/0x94
> > dpu_kms_hw_init+0x3a0/0x4a8 [msm]
> > msm_drm_kms_init+0xb8/0x340 [msm]
> > msm_drm_init+0x16c/0x22c [msm]
> > msm_drm_bind+0x30/0x3c [msm]
> > try_to_bring_up_aggregate_device+0x168/0x1d4
> > __component_add+0xa4/0x170
> > component_add+0x14/0x20
> > dsi_dev_attach+0x20/0x2c [msm]
> > dsi_host_attach+0x58/0x98 [msm]
> > mipi_dsi_attach+0x30/0x54
> > novatek_nt37801_probe+0x13c/0x1c8 [panel_novatek_nt37801]
> >
> > And I found a a similar bug.
> > https://lore.kernel.org/all/20220915205559.14574-1-quic_bjorande@quicinc.com/
> >
> > Since the panel driver does not hold the property power-domains = <&rpmhpd RPMHPD_MMCX>
> > once all drivers that do own the RPMHPD_MMCX power domain finish probing,
> > the interconnect’s dev_sync_state is triggered, which eventually runs rpmhpd_sync_state
> > and starts dynamic voltage adjustment. This is when the issue occurs.
> >
> >
> > if do change below, this issue can also be fixed.
> > &mdss_dsi0 {
> >     ...
> >         panel@0 {
> >                 compatible = "novatek,nt37801";
> >                 ...
> >         ++      power-domains = <&rpmhpd RPMHPD_MMCX>;
> >     }
> > }
> > But I don't think panel driver should own a power-domains = <&rpmhpd RPMHPD_MMCX>;
> 
> That's not related.
> 
> >
> >
> >
> > Thanks,
> > Yuanjie
> >
> > > >
> > > >     rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks);
> > > >     if (rc) {
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > > index 993cf512f8c5..8d2595d8a5f6 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > > @@ -92,6 +92,9 @@ struct dpu_kms {
> > > >     struct clk_bulk_data *clocks;
> > > >     size_t num_clocks;
> > > >
> > > > +   unsigned long max_freq;
> > > > +   unsigned long min_freq;
> > > > +
> > > >     /* reference count bandwidth requests, so we know when we can
> > > >      * release bandwidth.  Each atomic update increments, and frame-
> > > >      * done event decrements.  Additionally, for video mode, the
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> 
> 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 1/12/26 9:25 AM, yuanjiey wrote:
> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>
>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>> 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.
>>>>
>>>> Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
>>>> doesn't do anything with clocks. I think we should have a call to
>>>> clk_disable_unprepare() before that and clk_prepare_enable() in the
>>>> resume path.
>>>
>>> I do check the backtrace on kaanapali:
>>>
>>> active_corner = 3 (MIN_SVS)
>>> rpmhpd_aggregate_corner+0x168/0x1d0
>>> rpmhpd_set_performance_state+0x84/0xd0
>>> _genpd_set_performance_state+0x50/0x198
>>> genpd_set_performance_state.isra.0+0xbc/0xdc
>>> genpd_dev_pm_set_performance_state+0x60/0xc4
>>> dev_pm_domain_set_performance_state+0x24/0x3c
>>> _set_opp+0x370/0x584
>>> dev_pm_opp_set_rate+0x258/0x2b4
>>> dpu_runtime_suspend+0x50/0x11c [msm]
>>> pm_generic_runtime_suspend+0x2c/0x44
>>> genpd_runtime_suspend+0xac/0x2d0
>>> __rpm_callback+0x48/0x19c
>>> rpm_callback+0x74/0x80
>>> rpm_suspend+0x108/0x588
>>> rpm_idle+0xe8/0x190
>>> __pm_runtime_idle+0x50/0x94
>>> dpu_kms_hw_init+0x3a0/0x4a8
>>>
>>> dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS.
>>> And freq MDP: 650MHz
>>>
>>>
>>> And I try change the order:
>>> from:
>>>         dev_pm_opp_set_rate(dev, 0);
>>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>>> to:
>>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>>>         dev_pm_opp_set_rate(dev, 0);
>>>
>>> But the issue is still here.
>>
>> But which clock is still demanding higher MMCX voltage? All DPU clocks
>> should be turned off at this point.
> Yes, no DPU clock demand higher MMCX voltage here. But next time pm_runtime_get_sync
> need higher MMCX voltagei due to high freq.
>  
>>>
>>>
>>>>> 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.
>>>>>
>>>>> Fix this by setting the corresponding OPP corner during both power-on
>>>>> and power-off sequences to ensure proper alignment of rail voltage and
>>>>> clock frequency.
>>>>>
>>>>> Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
>>>>>
>>>>> Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>>>>
>>>> No empty lines between the tags. Also please cc stable.
>>>
>>> Sure, will fix.
>>>
>>>>> ---
>>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
>>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
>>>>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>>>> index 0623f1dbed97..c31488335f2b 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>>>> @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
>>>>>     struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>>>>>     struct dev_pm_opp *opp;
>>>>>     int ret = 0;
>>>>> -   unsigned long max_freq = ULONG_MAX;
>>>>> +   dpu_kms->max_freq = ULONG_MAX;
>>>>> +   dpu_kms->min_freq = 0;
>>>>>
>>>>> -   opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
>>>>> +   opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
>>>>> +   if (!IS_ERR(opp))
>>>>> +           dev_pm_opp_put(opp);
>>>>> +
>>>>> +   opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
>>>>>     if (!IS_ERR(opp))
>>>>>             dev_pm_opp_put(opp);
>>>>>
>>>>> @@ -1461,8 +1466,8 @@ 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);
>>>>> +   /* adjust the performance state vote to low performance state */
>>>>> +   dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
>>>>
>>>> Here min_freq is the minumum working frequency, which will keep it
>>>> ticking at a high frequency.  I think we are supposed to turn it off
>>>> (well, switch to XO). Would it be enough to swap these two lines
>>>> instead?
>>>
>>> Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
>>> disable clk is OK.
>>> we can drop change here.
>>>
>>>>>     clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>>>>>
>>>>>     for (i = 0; i < dpu_kms->num_paths; i++)
>>>>> @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
>>>>>     struct drm_device *ddev;
>>>>>
>>>>>     ddev = dpu_kms->dev;
>>>>> +   /* adjust the performance state vote to high performance state */
>>>>> +   if (dpu_kms->max_freq != ULONG_MAX)
>>>>> +           dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
>>>>
>>>> This one should not be necessary, we should be setting the performance
>>>> point while comitting the DRM state.
>>>
>>> No, issue is during dpu binding path. And in msm_drm_kms_init driver just
>>> pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
>>> But We do not set the appropriate OPP each time the power is enabled.
>>> As a result, a situation may occur where the rail stays at MIN_SVS while the
>>> MDP is running at a high frequency.
>>
>> Please move dev_pm_opp_set_rate() from dpu_kms_init() to dpu_kms_hw_init().
> 
> During dpu_kms_hw_init and msm_drm_kms_init and msm_drm_kms_post_init.
> 
> There are multiple places where pm_runtime_get_sync(pm_runtime_resume_and_get)and pm_runtime_put_sync are called.
> And each time after pm_runtime_get_sync(pm_runtime_resume_and_get) will access register depond on MDP clk.
> 
> Do I need to add dev_pm_opp_set_rate after every pm_runtime_get_sync and pm_runtime_resume_and_get?

So I took another look at this thread and I think the correct resolution
here would be to stop calling dev_pm_opp_set_rate(dev, 0) altogether if
the clock is getting disabled, since the PM APIs seem to take care of
removing the vote during runtime suspend:

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 61d7e65469b3..ddc6aeae8f55 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1462,7 +1462,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
        struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
 
        /* Drop the performance state vote */
-       dev_pm_opp_set_rate(dev, 0);
+       pr_err("!!!! SUSPENDING DPU\n");
        clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
 
        for (i = 0; i < dpu_kms->num_paths; i++)


[root@sc8280xp-crd ~]# XDG_RUNTIME_DIR=/run/user/1000 DISPLAY=:0 xset dpms force off
[  163.099585] [drm:dpu_runtime_suspend:1465] !!!! SUSPENDING DPU
[root@sc8280xp-crd ~]# grep ^ /sys/kernel/debug/pm_genpd/mmcx/*
/sys/kernel/debug/pm_genpd/mmcx/active_time:159146 ms
/sys/kernel/debug/pm_genpd/mmcx/current_state:off-0
/sys/kernel/debug/pm_genpd/mmcx/devices:ad00000.clock-controller
/sys/kernel/debug/pm_genpd/mmcx/devices:af00000.clock-controller
/sys/kernel/debug/pm_genpd/mmcx/devices:ae01000.display-controller
/sys/kernel/debug/pm_genpd/mmcx/devices:aea0000.displayport-controller
/sys/kernel/debug/pm_genpd/mmcx/devices:ae90000.displayport-controller
/sys/kernel/debug/pm_genpd/mmcx/devices:ae98000.displayport-controller
/sys/kernel/debug/pm_genpd/mmcx/idle_states:State  Time(ms)       Usage      Rejected   Above      Below      S2idle
/sys/kernel/debug/pm_genpd/mmcx/idle_states:S0     67845          3          0          0          0          0
/sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:State  Latency(us)  Residency(us)  Name
/sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:S0     0            0              N/A
/sys/kernel/debug/pm_genpd/mmcx/perf_state:0
/sys/kernel/debug/pm_genpd/mmcx/sub_domains:titan_top_gdsc
/sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_gdsc
/sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_int2_gdsc
/sys/kernel/debug/pm_genpd/mmcx/total_idle_time:67846 ms

(and the correct vote comes back up as the DPU resumes)

At the same time, we *do* need to ensure the correct level is set *before*
clk_prepare_enable and any set_rate operations (the latter is already done
via dev_pm_opp_set_rate())

clk_prepare_enable() happens in:
	msm_mdss.c : msm_mdss_enable()
	dpu_kms.c : dpu_runtime_resume()

(they refer to two disjoint sets of clocks but both cases need the fix)

I think the easiest solution in the MDP case would be to set the clocks to
the highest available OPP (lowest or *any* would work too, but going turbo
seems like it's going to give us a stronger foundation for adopting
cont_splash one day, avoiding potentially momentarily undervolting running
hw) during probe and count on these votes being adjusted either through
suspend (if unused) or to the actually needed frequency (via dpu_perf)

For MDSS, we're currently generally describing the MDSS_AHB clock, the
GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
table.. The GCC clock is sourced from (and scaled by) the NoC, but the
MDSS_AHB one seems to have 3 actually configurable performance points
that neither we nor seemingly the downstream driver seem to really care
about (i.e. both just treat it as on/off). If we need to scale it, we
should add an OPP table, if we don't, we should at least add required-opps.

The MDP4/MDP5 drivers are probably wrong too in this regard, but many
targets supported by these may not even have OPP tables and are generally
not super high priority for spending time on.. that can, I'd kick down the
road unless someone really wants to step up

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[yuanjiey]

On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> On 1/12/26 9:25 AM, yuanjiey wrote:
> > On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>
> >>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>> 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.
> >>>>
> >>>> Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
> >>>> doesn't do anything with clocks. I think we should have a call to
> >>>> clk_disable_unprepare() before that and clk_prepare_enable() in the
> >>>> resume path.
> >>>
> >>> I do check the backtrace on kaanapali:
> >>>
> >>> active_corner = 3 (MIN_SVS)
> >>> rpmhpd_aggregate_corner+0x168/0x1d0
> >>> rpmhpd_set_performance_state+0x84/0xd0
> >>> _genpd_set_performance_state+0x50/0x198
> >>> genpd_set_performance_state.isra.0+0xbc/0xdc
> >>> genpd_dev_pm_set_performance_state+0x60/0xc4
> >>> dev_pm_domain_set_performance_state+0x24/0x3c
> >>> _set_opp+0x370/0x584
> >>> dev_pm_opp_set_rate+0x258/0x2b4
> >>> dpu_runtime_suspend+0x50/0x11c [msm]
> >>> pm_generic_runtime_suspend+0x2c/0x44
> >>> genpd_runtime_suspend+0xac/0x2d0
> >>> __rpm_callback+0x48/0x19c
> >>> rpm_callback+0x74/0x80
> >>> rpm_suspend+0x108/0x588
> >>> rpm_idle+0xe8/0x190
> >>> __pm_runtime_idle+0x50/0x94
> >>> dpu_kms_hw_init+0x3a0/0x4a8
> >>>
> >>> dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS.
> >>> And freq MDP: 650MHz
> >>>
> >>>
> >>> And I try change the order:
> >>> from:
> >>>         dev_pm_opp_set_rate(dev, 0);
> >>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>> to:
> >>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>>         dev_pm_opp_set_rate(dev, 0);
> >>>
> >>> But the issue is still here.
> >>
> >> But which clock is still demanding higher MMCX voltage? All DPU clocks
> >> should be turned off at this point.
> > Yes, no DPU clock demand higher MMCX voltage here. But next time pm_runtime_get_sync
> > need higher MMCX voltagei due to high freq.
> >  
> >>>
> >>>
> >>>>> 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.
> >>>>>
> >>>>> Fix this by setting the corresponding OPP corner during both power-on
> >>>>> and power-off sequences to ensure proper alignment of rail voltage and
> >>>>> clock frequency.
> >>>>>
> >>>>> Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> >>>>>
> >>>>> Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >>>>
> >>>> No empty lines between the tags. Also please cc stable.
> >>>
> >>> Sure, will fix.
> >>>
> >>>>> ---
> >>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
> >>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
> >>>>>  2 files changed, 15 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> index 0623f1dbed97..c31488335f2b 100644
> >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
> >>>>>     struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> >>>>>     struct dev_pm_opp *opp;
> >>>>>     int ret = 0;
> >>>>> -   unsigned long max_freq = ULONG_MAX;
> >>>>> +   dpu_kms->max_freq = ULONG_MAX;
> >>>>> +   dpu_kms->min_freq = 0;
> >>>>>
> >>>>> -   opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> >>>>> +   opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> >>>>> +   if (!IS_ERR(opp))
> >>>>> +           dev_pm_opp_put(opp);
> >>>>> +
> >>>>> +   opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
> >>>>>     if (!IS_ERR(opp))
> >>>>>             dev_pm_opp_put(opp);
> >>>>>
> >>>>> @@ -1461,8 +1466,8 @@ 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);
> >>>>> +   /* adjust the performance state vote to low performance state */
> >>>>> +   dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
> >>>>
> >>>> Here min_freq is the minumum working frequency, which will keep it
> >>>> ticking at a high frequency.  I think we are supposed to turn it off
> >>>> (well, switch to XO). Would it be enough to swap these two lines
> >>>> instead?
> >>>
> >>> Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
> >>> disable clk is OK.
> >>> we can drop change here.
> >>>
> >>>>>     clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>>>>
> >>>>>     for (i = 0; i < dpu_kms->num_paths; i++)
> >>>>> @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
> >>>>>     struct drm_device *ddev;
> >>>>>
> >>>>>     ddev = dpu_kms->dev;
> >>>>> +   /* adjust the performance state vote to high performance state */
> >>>>> +   if (dpu_kms->max_freq != ULONG_MAX)
> >>>>> +           dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
> >>>>
> >>>> This one should not be necessary, we should be setting the performance
> >>>> point while comitting the DRM state.
> >>>
> >>> No, issue is during dpu binding path. And in msm_drm_kms_init driver just
> >>> pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
> >>> But We do not set the appropriate OPP each time the power is enabled.
> >>> As a result, a situation may occur where the rail stays at MIN_SVS while the
> >>> MDP is running at a high frequency.
> >>
> >> Please move dev_pm_opp_set_rate() from dpu_kms_init() to dpu_kms_hw_init().
> > 
> > During dpu_kms_hw_init and msm_drm_kms_init and msm_drm_kms_post_init.
> > 
> > There are multiple places where pm_runtime_get_sync(pm_runtime_resume_and_get)and pm_runtime_put_sync are called.
> > And each time after pm_runtime_get_sync(pm_runtime_resume_and_get) will access register depond on MDP clk.
> > 
> > Do I need to add dev_pm_opp_set_rate after every pm_runtime_get_sync and pm_runtime_resume_and_get?
> 
> So I took another look at this thread and I think the correct resolution
> here would be to stop calling dev_pm_opp_set_rate(dev, 0) altogether if
> the clock is getting disabled, since the PM APIs seem to take care of
> removing the vote during runtime suspend:
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 61d7e65469b3..ddc6aeae8f55 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1462,7 +1462,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
>         struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>  99
>         /* Drop the performance state vote */
> -       dev_pm_opp_set_rate(dev, 0);
> +       pr_err("!!!! SUSPENDING DPU\n");
>
drop dev_pm_opp_set_rate(dev, 0) is OK. Test it OK on Kaanapali.

And I do some test:
1.drop dev_pm_opp_set_rate(dev, 0),

2. when rpmhpd_sync_state runs before dpu_kms_hw_init initialization.
[   11.123830]  rpmhpd_sync_state+0x9c/0xec
 
When pm_runtime_put_sync is called: it can set corner to MIN_SVS.
[   11.546084]  rpmhpd_aggregate_corner+0x170/0x1d8
[   11.546086]  rpmhpd_set_performance_state+0xc4/0xec
[   11.546087]  _genpd_set_performance_state+0xd0/0x1ac
[   11.546089]  genpd_set_performance_state.isra.0+0xbc/0xdc
[   11.546091]  genpd_runtime_suspend+0x144/0x294
[   11.546093]  __rpm_callback+0x48/0x1d8
[   11.546095]  rpm_callback+0x74/0x80
[   11.546096]  rpm_suspend+0x10c/0x56c
[   11.546097]  rpm_idle+0x11c/0x1a8
[   11.546098]  __pm_runtime_idle+0x54/0x98
[   11.546099]  dpu_kms_hw_init+0x3c8/0x4ac [msm]
 
When pm_runtime_get_sync is called: it can set corner to correct corner(here is TURBO_SVS)
[   11.784091]  rpmhpd_aggregate_corner+0x170/0x1d8
[   11.784093]  rpmhpd_set_performance_state+0xc4/0xec
[   11.784093]  _genpd_set_performance_state+0x190/0x1ac
[   11.784096]  genpd_set_performance_state.isra.0+0xbc/0xdc
[   11.784098]  genpd_runtime_resume+0x228/0x288
[   11.784099]  __rpm_callback+0x48/0x1d8
[   11.784100]  rpm_callback+0x74/0x80
[   11.784101]  rpm_resume+0x480/0x6b8
[   11.784102]  __pm_runtime_resume+0x50/0x94
[   11.784103]  msm_drm_kms_init+0x1a4/0x340 [msm]

>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>  
>         for (i = 0; i < dpu_kms->num_paths; i++)
> 
> 
> [root@sc8280xp-crd ~]# XDG_RUNTIME_DIR=/run/user/1000 DISPLAY=:0 xset dpms force off
> [  163.099585] [drm:dpu_runtime_suspend:1465] !!!! SUSPENDING DPU
> [root@sc8280xp-crd ~]# grep ^ /sys/kernel/debug/pm_genpd/mmcx/*
> /sys/kernel/debug/pm_genpd/mmcx/active_time:159146 ms
> /sys/kernel/debug/pm_genpd/mmcx/current_state:off-0
> /sys/kernel/debug/pm_genpd/mmcx/devices:ad00000.clock-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:af00000.clock-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae01000.display-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:aea0000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae90000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae98000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/idle_states:State  Time(ms)       Usage      Rejected   Above      Below      S2idle
> /sys/kernel/debug/pm_genpd/mmcx/idle_states:S0     67845          3          0          0          0          0
> /sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:State  Latency(us)  Residency(us)  Name
> /sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:S0     0            0              N/A
> /sys/kernel/debug/pm_genpd/mmcx/perf_state:0
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:titan_top_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_int2_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/total_idle_time:67846 ms
> 
> (and the correct vote comes back up as the DPU resumes)
> 
> At the same time, we *do* need to ensure the correct level is set *before*
> clk_prepare_enable and any set_rate operations (the latter is already done
> via dev_pm_opp_set_rate())
> 
> clk_prepare_enable() happens in:
> 	msm_mdss.c : msm_mdss_enable()
> 	dpu_kms.c : dpu_runtime_resume()
> 
> (they refer to two disjoint sets of clocks but both cases need the fix)
> 
> I think the easiest solution in the MDP case would be to set the clocks to
> the highest available OPP (lowest or *any* would work too, but going turbo
> seems like it's going to give us a stronger foundation for adopting
> cont_splash one day, avoiding potentially momentarily undervolting running
> hw) during probe and count on these votes being adjusted either through
> suspend (if unused) or to the actually needed frequency (via dpu_perf)

Agree it.

Thanks,
Yuanjue


> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> MDSS_AHB one seems to have 3 actually configurable performance points
> that neither we nor seemingly the downstream driver seem to really care
> about (i.e. both just treat it as on/off). If we need to scale it, we
> should add an OPP table, if we don't, we should at least add required-opps.
> 
> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> targets supported by these may not even have OPP tables and are generally
> not super high priority for spending time on.. that can, I'd kick down the
> road unless someone really wants to step up
> 
> Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> On 1/12/26 9:25 AM, yuanjiey wrote:
> > On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>
> >>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>> 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.
> >>>>
> >>>> Ah, I see. dev_pm_set_rate(0) transforms to  _disable_opp_table(), which
> >>>> doesn't do anything with clocks. I think we should have a call to
> >>>> clk_disable_unprepare() before that and clk_prepare_enable() in the
> >>>> resume path.
> >>>
> >>> I do check the backtrace on kaanapali:
> >>>
> >>> active_corner = 3 (MIN_SVS)
> >>> rpmhpd_aggregate_corner+0x168/0x1d0
> >>> rpmhpd_set_performance_state+0x84/0xd0
> >>> _genpd_set_performance_state+0x50/0x198
> >>> genpd_set_performance_state.isra.0+0xbc/0xdc
> >>> genpd_dev_pm_set_performance_state+0x60/0xc4
> >>> dev_pm_domain_set_performance_state+0x24/0x3c
> >>> _set_opp+0x370/0x584
> >>> dev_pm_opp_set_rate+0x258/0x2b4
> >>> dpu_runtime_suspend+0x50/0x11c [msm]
> >>> pm_generic_runtime_suspend+0x2c/0x44
> >>> genpd_runtime_suspend+0xac/0x2d0
> >>> __rpm_callback+0x48/0x19c
> >>> rpm_callback+0x74/0x80
> >>> rpm_suspend+0x108/0x588
> >>> rpm_idle+0xe8/0x190
> >>> __pm_runtime_idle+0x50/0x94
> >>> dpu_kms_hw_init+0x3a0/0x4a8
> >>>
> >>> dev_pm_opp_set_rate(dev, 0); just low power to MIN_SVS.
> >>> And freq MDP: 650MHz
> >>>
> >>>
> >>> And I try change the order:
> >>> from:
> >>>         dev_pm_opp_set_rate(dev, 0);
> >>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>> to:
> >>>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>>         dev_pm_opp_set_rate(dev, 0);
> >>>
> >>> But the issue is still here.
> >>
> >> But which clock is still demanding higher MMCX voltage? All DPU clocks
> >> should be turned off at this point.
> > Yes, no DPU clock demand higher MMCX voltage here. But next time pm_runtime_get_sync
> > need higher MMCX voltagei due to high freq.
> >  
> >>>
> >>>
> >>>>> 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.
> >>>>>
> >>>>> Fix this by setting the corresponding OPP corner during both power-on
> >>>>> and power-off sequences to ensure proper alignment of rail voltage and
> >>>>> clock frequency.
> >>>>>
> >>>>> Fixes: b0530eb11913 ("drm/msm/dpu: Use OPP API to set clk/perf state")
> >>>>>
> >>>>> Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >>>>
> >>>> No empty lines between the tags. Also please cc stable.
> >>>
> >>> Sure, will fix.
> >>>
> >>>>> ---
> >>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++++++----
> >>>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  3 +++
> >>>>>  2 files changed, 15 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> index 0623f1dbed97..c31488335f2b 100644
> >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >>>>> @@ -1306,9 +1306,14 @@ static int dpu_kms_init(struct drm_device *ddev)
> >>>>>     struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> >>>>>     struct dev_pm_opp *opp;
> >>>>>     int ret = 0;
> >>>>> -   unsigned long max_freq = ULONG_MAX;
> >>>>> +   dpu_kms->max_freq = ULONG_MAX;
> >>>>> +   dpu_kms->min_freq = 0;
> >>>>>
> >>>>> -   opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> >>>>> +   opp = dev_pm_opp_find_freq_floor(dev, &dpu_kms->max_freq);
> >>>>> +   if (!IS_ERR(opp))
> >>>>> +           dev_pm_opp_put(opp);
> >>>>> +
> >>>>> +   opp = dev_pm_opp_find_freq_ceil(dev, &dpu_kms->min_freq);
> >>>>>     if (!IS_ERR(opp))
> >>>>>             dev_pm_opp_put(opp);
> >>>>>
> >>>>> @@ -1461,8 +1466,8 @@ 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);
> >>>>> +   /* adjust the performance state vote to low performance state */
> >>>>> +   dev_pm_opp_set_rate(dev, dpu_kms->min_freq);
> >>>>
> >>>> Here min_freq is the minumum working frequency, which will keep it
> >>>> ticking at a high frequency.  I think we are supposed to turn it off
> >>>> (well, switch to XO). Would it be enough to swap these two lines
> >>>> instead?
> >>>
> >>> Yes, just clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks) to
> >>> disable clk is OK.
> >>> we can drop change here.
> >>>
> >>>>>     clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
> >>>>>
> >>>>>     for (i = 0; i < dpu_kms->num_paths; i++)
> >>>>> @@ -1481,6 +1486,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
> >>>>>     struct drm_device *ddev;
> >>>>>
> >>>>>     ddev = dpu_kms->dev;
> >>>>> +   /* adjust the performance state vote to high performance state */
> >>>>> +   if (dpu_kms->max_freq != ULONG_MAX)
> >>>>> +           dev_pm_opp_set_rate(dev, dpu_kms->max_freq);
> >>>>
> >>>> This one should not be necessary, we should be setting the performance
> >>>> point while comitting the DRM state.
> >>>
> >>> No, issue is during dpu binding path. And in msm_drm_kms_init driver just
> >>> pm_runtime_resume_and_get enable power and pm_runtime_put_sync disable power.
> >>> But We do not set the appropriate OPP each time the power is enabled.
> >>> As a result, a situation may occur where the rail stays at MIN_SVS while the
> >>> MDP is running at a high frequency.
> >>
> >> Please move dev_pm_opp_set_rate() from dpu_kms_init() to dpu_kms_hw_init().
> > 
> > During dpu_kms_hw_init and msm_drm_kms_init and msm_drm_kms_post_init.
> > 
> > There are multiple places where pm_runtime_get_sync(pm_runtime_resume_and_get)and pm_runtime_put_sync are called.
> > And each time after pm_runtime_get_sync(pm_runtime_resume_and_get) will access register depond on MDP clk.
> > 
> > Do I need to add dev_pm_opp_set_rate after every pm_runtime_get_sync and pm_runtime_resume_and_get?
> 
> So I took another look at this thread and I think the correct resolution
> here would be to stop calling dev_pm_opp_set_rate(dev, 0) altogether if
> the clock is getting disabled, since the PM APIs seem to take care of
> removing the vote during runtime suspend:

LGTM.

> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 61d7e65469b3..ddc6aeae8f55 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1462,7 +1462,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
>         struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>  
>         /* Drop the performance state vote */
> -       dev_pm_opp_set_rate(dev, 0);
> +       pr_err("!!!! SUSPENDING DPU\n");
>         clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
>  
>         for (i = 0; i < dpu_kms->num_paths; i++)
> 
> 
> [root@sc8280xp-crd ~]# XDG_RUNTIME_DIR=/run/user/1000 DISPLAY=:0 xset dpms force off
> [  163.099585] [drm:dpu_runtime_suspend:1465] !!!! SUSPENDING DPU
> [root@sc8280xp-crd ~]# grep ^ /sys/kernel/debug/pm_genpd/mmcx/*
> /sys/kernel/debug/pm_genpd/mmcx/active_time:159146 ms
> /sys/kernel/debug/pm_genpd/mmcx/current_state:off-0
> /sys/kernel/debug/pm_genpd/mmcx/devices:ad00000.clock-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:af00000.clock-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae01000.display-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:aea0000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae90000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/devices:ae98000.displayport-controller
> /sys/kernel/debug/pm_genpd/mmcx/idle_states:State  Time(ms)       Usage      Rejected   Above      Below      S2idle
> /sys/kernel/debug/pm_genpd/mmcx/idle_states:S0     67845          3          0          0          0          0
> /sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:State  Latency(us)  Residency(us)  Name
> /sys/kernel/debug/pm_genpd/mmcx/idle_states_desc:S0     0            0              N/A
> /sys/kernel/debug/pm_genpd/mmcx/perf_state:0
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:titan_top_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/sub_domains:disp0_mdss_int2_gdsc
> /sys/kernel/debug/pm_genpd/mmcx/total_idle_time:67846 ms
> 
> (and the correct vote comes back up as the DPU resumes)
> 
> At the same time, we *do* need to ensure the correct level is set *before*
> clk_prepare_enable and any set_rate operations (the latter is already done
> via dev_pm_opp_set_rate())
> 
> clk_prepare_enable() happens in:
> 	msm_mdss.c : msm_mdss_enable()
> 	dpu_kms.c : dpu_runtime_resume()
> 
> (they refer to two disjoint sets of clocks but both cases need the fix)
> 
> I think the easiest solution in the MDP case would be to set the clocks to
> the highest available OPP (lowest or *any* would work too, but going turbo
> seems like it's going to give us a stronger foundation for adopting
> cont_splash one day, avoiding potentially momentarily undervolting running
> hw) during probe and count on these votes being adjusted either through
> suspend (if unused) or to the actually needed frequency (via dpu_perf)

Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
dpu_runtime_suspend, then we should be able to also skip setting OPP
corner in dpu_runtime_resume(), because the previously set corner should
be viable until drm/msm driver commits new state / new modes.

The only important issue is to set the corner before starting up the
DPU, where we already have code to set MDP_CLK to the max frequency.

Which means, we only need to drop the dev_pm_set_rate call from the
dpu_runtime_suspend().

> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP

No. As far as I remember, MDP_CLK is necessary to access MDSS registers
(see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
platforms")), I don't remember if accessing HW_REV without MDP_CLK
resulted in a zero reads or in a crash. At the same time it needs to be
enabled to any rate, which means that for most of the operations
msm_mdss.c can rely on DPU keeping the clock up and running.

> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> MDSS_AHB one seems to have 3 actually configurable performance points
> that neither we nor seemingly the downstream driver seem to really care
> about (i.e. both just treat it as on/off). If we need to scale it, we
> should add an OPP table, if we don't, we should at least add required-opps.

I think, dispcc already has a minimal vote on the MMCX, which fulfill
these needs.

> 
> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> targets supported by these may not even have OPP tables and are generally
> not super high priority for spending time on.. that can, I'd kick down the
> road unless someone really wants to step up

I'd really not bother with those two drivers, unless we start seing
crashes. For MDP4 platforms we don't implement power domains at all, no
interconnects, etc., which means that the system will never go to a
lower power state.

MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
should start there by adding missing bits adding corresponding
dev_pm_set_rate() calls as required (as demostrated by the DPU driver).

A note to myself to also add OPP tables support to the HDMI driver.

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>> On 1/12/26 9:25 AM, yuanjiey wrote:
>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>>>
>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>

[...]

> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> dpu_runtime_suspend, then we should be able to also skip setting OPP
> corner in dpu_runtime_resume(), because the previously set corner should
> be viable until drm/msm driver commits new state / new modes.

That matches my understanding.

> The only important issue is to set the corner before starting up the
> DPU, where we already have code to set MDP_CLK to the max frequency.
> 
> Which means, we only need to drop the dev_pm_set_rate call from the
> dpu_runtime_suspend().

I concur.

>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> 
> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> platforms")), I don't remember if accessing HW_REV without MDP_CLK
> resulted in a zero reads or in a crash. At the same time it needs to be
> enabled to any rate, which means that for most of the operations
> msm_mdss.c can rely on DPU keeping the clock up and running.
> 
>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>> MDSS_AHB one seems to have 3 actually configurable performance points
>> that neither we nor seemingly the downstream driver seem to really care
>> about (i.e. both just treat it as on/off). If we need to scale it, we
>> should add an OPP table, if we don't, we should at least add required-opps.
> 
> I think, dispcc already has a minimal vote on the MMCX, which fulfill
> these needs.

I have slightly mixed feelings, but I suppose that as we accepted Commit
e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
we can generally agree that it makes sense that calling genpd->on() actually
turns on the power indeed

What I'm worried about is if the clock is pre-configured to run at a high
frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
and doesn't impact the state of M/N/D at a glance), we may get a brownout

This rings the "downstream really did it better with putting clock dvfs states
into the clk driver" bell, but I suppose the way to fight this would be to
simply set_rate(fmax) there too..

I attempted an experiment with pulling out the plug. MMCX enabled with the
AHB clock off results in a read-as-zero. I tried really hard to disable the
mdp clock, but it seems like the "shared_ops" reflect some sort of "you
*really* can't just disable it" type behavior (verified with debugcc)


There's a possible race condition if we don't do it:

------- bootloader --------
configure display, mdp_clk=turbo
------- linux -------------
load rpmhpd     |
load venus      |
set mmcx=lowsvs | mdp_clk is @ turbo
                | brownout
                | 
                | <mdss would only probe here>

*but* that should be made impossible because of .sync_state().

This may impact hacky setups like simplefb, but as the name implies,
that's hacky.

Relying on .sync_state() however will not cover the case if the mdss
module is removed and re-inserted later, possibly with mmcx disabled
entirely but the clock not parked at a sufficiently low rate.


TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
plug the MDP opp table into it and set_rate(fmax) during mdss init

>> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
>> targets supported by these may not even have OPP tables and are generally
>> not super high priority for spending time on.. that can, I'd kick down the
>> road unless someone really wants to step up
> 
> I'd really not bother with those two drivers, unless we start seing
> crashes. For MDP4 platforms we don't implement power domains at all, no
> interconnects, etc., which means that the system will never go to a
> lower power state.

Right. Might be that 2030-something arrives and armv7 is gone before someone
randomly decides to work on that!

> MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> should start there by adding missing bits adding corresponding
> dev_pm_set_rate() calls as required (as demostrated by the DPU driver).

A bit off-topic, but:

drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
1101:   { .revision = 0, .config = { .hw = &msm8x74v1_config } },
1102:   { .revision = 1, .config = { .hw = &msm8x26_config } },
1103:   { .revision = 2, .config = { .hw = &msm8x74v2_config } },
1104:   { .revision = 3, .config = { .hw = &apq8084_config } },
1105:   { .revision = 6, .config = { .hw = &msm8x16_config } },
1106:   { .revision = 8, .config = { .hw = &msm8x36_config } },
1107:   { .revision = 9, .config = { .hw = &msm8x94_config } },
1108:   { .revision = 7, .config = { .hw = &msm8x96_config } },
1109:   { .revision = 11, .config = { .hw = &msm8x76_config } },
1110:   { .revision = 14, .config = { .hw = &msm8937_config } },
1111:   { .revision = 15, .config = { .hw = &msm8917_config } },
1112:   { .revision = 16, .config = { .hw = &msm8x53_config } },

96 is in DPU. any other candidates that should be moved over?

> A note to myself to also add OPP tables support to the HDMI driver.

Eliza!

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> > On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >> On 1/12/26 9:25 AM, yuanjiey wrote:
> >>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>>>
> >>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> 
> [...]
> 
> > Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> > dpu_runtime_suspend, then we should be able to also skip setting OPP
> > corner in dpu_runtime_resume(), because the previously set corner should
> > be viable until drm/msm driver commits new state / new modes.
> 
> That matches my understanding.
> 
> > The only important issue is to set the corner before starting up the
> > DPU, where we already have code to set MDP_CLK to the max frequency.
> > 
> > Which means, we only need to drop the dev_pm_set_rate call from the
> > dpu_runtime_suspend().
> 
> I concur.
> 
> >> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> > 
> > No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> > (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> > platforms")), I don't remember if accessing HW_REV without MDP_CLK
> > resulted in a zero reads or in a crash. At the same time it needs to be
> > enabled to any rate, which means that for most of the operations
> > msm_mdss.c can rely on DPU keeping the clock up and running.
> > 
> >> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >> MDSS_AHB one seems to have 3 actually configurable performance points
> >> that neither we nor seemingly the downstream driver seem to really care
> >> about (i.e. both just treat it as on/off). If we need to scale it, we
> >> should add an OPP table, if we don't, we should at least add required-opps.
> > 
> > I think, dispcc already has a minimal vote on the MMCX, which fulfill
> > these needs.
> 
> I have slightly mixed feelings, but I suppose that as we accepted Commit
> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> we can generally agree that it makes sense that calling genpd->on() actually
> turns on the power indeed
> 
> What I'm worried about is if the clock is pre-configured to run at a high
> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> 
> This rings the "downstream really did it better with putting clock dvfs states
> into the clk driver" bell, but I suppose the way to fight this would be to
> simply set_rate(fmax) there too..
> 
> I attempted an experiment with pulling out the plug. MMCX enabled with the
> AHB clock off results in a read-as-zero. I tried really hard to disable the
> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> *really* can't just disable it" type behavior (verified with debugcc)

I think, in 8996 it was possible to disable it. Not sure about
8998/630/660.

> 
> 
> There's a possible race condition if we don't do it:
> 
> ------- bootloader --------
> configure display, mdp_clk=turbo
> ------- linux -------------
> load rpmhpd     |
> load venus      |
> set mmcx=lowsvs | mdp_clk is @ turbo
>                 | brownout
>                 | 
>                 | <mdss would only probe here>
> 
> *but* that should be made impossible because of .sync_state().

Yep, sync_state should prevent MMCX or CX from dropping under the boot
level.

> 
> This may impact hacky setups like simplefb, but as the name implies,
> that's hacky.
> 
> Relying on .sync_state() however will not cover the case if the mdss
> module is removed and re-inserted later, possibly with mmcx disabled
> entirely but the clock not parked at a sufficiently low rate.
> 
> 
> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> plug the MDP opp table into it and set_rate(fmax) during mdss init

And what will drop it afterwards? MDSS will still vote on the MMCX / CX
level even though DPU will change the clock freq.

> 
> >> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> >> targets supported by these may not even have OPP tables and are generally
> >> not super high priority for spending time on.. that can, I'd kick down the
> >> road unless someone really wants to step up
> > 
> > I'd really not bother with those two drivers, unless we start seing
> > crashes. For MDP4 platforms we don't implement power domains at all, no
> > interconnects, etc., which means that the system will never go to a
> > lower power state.
> 
> Right. Might be that 2030-something arrives and armv7 is gone before someone
> randomly decides to work on that!
> 
> > MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> > SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> > has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> > should start there by adding missing bits adding corresponding
> > dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
> 
> A bit off-topic, but:
> 
> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> 
> 96 is in DPU. any other candidates that should be moved over?

Let's go through them.

All SoC except those currently supported in DPU require SMP (shared
memory pool) support to be ported from the MDP5 driver.

Most of the remaining platforms (except MSM8994/92) also had HW cursor
implemented in a fancy way, in the LM rather than in a separate pipe.
I'd really like to postpone those, possibly first completing migration
of the other platforms and dropping support for them from MDP5.

1.0  - old MSM8974
       I'd rather not touch it, it had bugs and I don't have HW
1.1  - MSM8x26
       Probably Luca can better comment on it. Should be doable, but I
       don't see upstream devices using display on it.
1.2  - MSM8974
       I think it also had issues, no IOMMU support in upstream, etc.
1.3  - APQ8084
       Had hw issues, no testing base, no MDSS in upstream DT
1.6  - MSM8916 / MSM8939
       Can be done, low-hanging fruit for testing
1.7  - MSM8996
       Supported in DPU
1.8  - MSM8936
       No upsteram testing base
1.9  - MSM8994
       No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
1.10 - MSM8992
       Even less testing base, no MDSS in upstream DT, normal CURSOR planes
1.11 - MSM8956 / 76
       No complete display configurations upstream
1.14 - MSM8937
       Supported in DPU
1.15 - MSM8917
       Supported in DPU
1.16 - MSM8953
       Supported in DPU
1.17 - QCS405
       Zero testing base, no MDSS in upstream DT

MSM8994/92 would have been an ideal testbeds for SMP testing, but...
they mostly don't exist (please correct me if I'm wrong). Which means
that the next viable targets are MSM8916, MSM8x26 and MSM8956/76. All of
them require SMP support and don't make sense without cursor handling.

> > A note to myself to also add OPP tables support to the HDMI driver.
> 
> Eliza!

MSM8996 / 98!

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>>>>>
>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>>
>> [...]
>>
>>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
>>> dpu_runtime_suspend, then we should be able to also skip setting OPP
>>> corner in dpu_runtime_resume(), because the previously set corner should
>>> be viable until drm/msm driver commits new state / new modes.
>>
>> That matches my understanding.
>>
>>> The only important issue is to set the corner before starting up the
>>> DPU, where we already have code to set MDP_CLK to the max frequency.
>>>
>>> Which means, we only need to drop the dev_pm_set_rate call from the
>>> dpu_runtime_suspend().
>>
>> I concur.
>>
>>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
>>>
>>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
>>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
>>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
>>> resulted in a zero reads or in a crash. At the same time it needs to be
>>> enabled to any rate, which means that for most of the operations
>>> msm_mdss.c can rely on DPU keeping the clock up and running.
>>>
>>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>>>> MDSS_AHB one seems to have 3 actually configurable performance points
>>>> that neither we nor seemingly the downstream driver seem to really care
>>>> about (i.e. both just treat it as on/off). If we need to scale it, we
>>>> should add an OPP table, if we don't, we should at least add required-opps.
>>>
>>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
>>> these needs.
>>
>> I have slightly mixed feelings, but I suppose that as we accepted Commit
>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
>> we can generally agree that it makes sense that calling genpd->on() actually
>> turns on the power indeed
>>
>> What I'm worried about is if the clock is pre-configured to run at a high
>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
>>
>> This rings the "downstream really did it better with putting clock dvfs states
>> into the clk driver" bell, but I suppose the way to fight this would be to
>> simply set_rate(fmax) there too..
>>
>> I attempted an experiment with pulling out the plug. MMCX enabled with the
>> AHB clock off results in a read-as-zero. I tried really hard to disable the
>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
>> *really* can't just disable it" type behavior (verified with debugcc)
> 
> I think, in 8996 it was possible to disable it. Not sure about
> 8998/630/660.
> 
>>
>>
>> There's a possible race condition if we don't do it:
>>
>> ------- bootloader --------
>> configure display, mdp_clk=turbo
>> ------- linux -------------
>> load rpmhpd     |
>> load venus      |
>> set mmcx=lowsvs | mdp_clk is @ turbo
>>                 | brownout
>>                 | 
>>                 | <mdss would only probe here>
>>
>> *but* that should be made impossible because of .sync_state().
> 
> Yep, sync_state should prevent MMCX or CX from dropping under the boot
> level.
> 
>>
>> This may impact hacky setups like simplefb, but as the name implies,
>> that's hacky.
>>
>> Relying on .sync_state() however will not cover the case if the mdss
>> module is removed and re-inserted later, possibly with mmcx disabled
>> entirely but the clock not parked at a sufficiently low rate.
>>
>>
>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
>> plug the MDP opp table into it and set_rate(fmax) during mdss init
> 
> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> level even though DPU will change the clock freq.

That's a good point. Perhaps the easiest resolution will be to leave a
comment before the prepare_enable() explaining that this should have a
vote, but it's easier to rely on the providers' .sync_state() keeping them
online until the consumers fully probe.

[...]

> Let's go through them.
> 
> All SoC except those currently supported in DPU require SMP (shared
> memory pool) support to be ported from the MDP5 driver.
> 
> Most of the remaining platforms (except MSM8994/92) also had HW cursor
> implemented in a fancy way, in the LM rather than in a separate pipe.
> I'd really like to postpone those, possibly first completing migration
> of the other platforms and dropping support for them from MDP5.
> 
> 1.0  - old MSM8974
>        I'd rather not touch it, it had bugs and I don't have HW

I have reasons to believe msm8974 v1.0 never reached store shelves.
Let's remove this.

> 1.1  - MSM8x26
>        Probably Luca can better comment on it. Should be doable, but I
>        don't see upstream devices using display on it.

Because there's no iommu support for these

> 1.2  - MSM8974
>        I think it also had issues, no IOMMU support in upstream, etc.
> 1.3  - APQ8084
>        Had hw issues, no testing base, no MDSS in upstream DT
> 1.6  - MSM8916 / MSM8939
>        Can be done, low-hanging fruit for testing
> 1.7  - MSM8996
>        Supported in DPU
> 1.8  - MSM8936
>        No upsteram testing base

8936 is 39 with some CPUs fused off (unless you have info suggesting
otherwise)

> 1.9  - MSM8994
>        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
> 1.10 - MSM8992
>        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
> 1.11 - MSM8956 / 76
>        No complete display configurations upstream

+Marijn, is your computer museum still running?

> 1.14 - MSM8937
>        Supported in DPU
> 1.15 - MSM8917
>        Supported in DPU
> 1.16 - MSM8953
>        Supported in DPU
> 1.17 - QCS405
>        Zero testing base, no MDSS in upstream DT

No upstream MDP5 support either. And it doesn't seem like that SoC had
much uses that didn't end up with the thing glued shut..

> MSM8994/92 would have been an ideal testbeds for SMP testing, but...
> they mostly don't exist (please correct me if I'm wrong). Which means
> that the next viable targets are MSM8916, MSM8x26 and MSM8956/76. All of
> them require SMP support and don't make sense without cursor handling.

We can think about poking at some of these it one day, but certainly not
high prio..

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Mon, Mar 02, 2026 at 11:41:59AM +0100, Konrad Dybcio wrote:
> On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
> > On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> >> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> >>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >>>> On 1/12/26 9:25 AM, yuanjiey wrote:
> >>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>>>>>
> >>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >>
> >> [...]
> >>
> >>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> >>> dpu_runtime_suspend, then we should be able to also skip setting OPP
> >>> corner in dpu_runtime_resume(), because the previously set corner should
> >>> be viable until drm/msm driver commits new state / new modes.
> >>
> >> That matches my understanding.
> >>
> >>> The only important issue is to set the corner before starting up the
> >>> DPU, where we already have code to set MDP_CLK to the max frequency.
> >>>
> >>> Which means, we only need to drop the dev_pm_set_rate call from the
> >>> dpu_runtime_suspend().
> >>
> >> I concur.
> >>
> >>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> >>>
> >>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> >>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> >>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
> >>> resulted in a zero reads or in a crash. At the same time it needs to be
> >>> enabled to any rate, which means that for most of the operations
> >>> msm_mdss.c can rely on DPU keeping the clock up and running.
> >>>
> >>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >>>> MDSS_AHB one seems to have 3 actually configurable performance points
> >>>> that neither we nor seemingly the downstream driver seem to really care
> >>>> about (i.e. both just treat it as on/off). If we need to scale it, we
> >>>> should add an OPP table, if we don't, we should at least add required-opps.
> >>>
> >>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
> >>> these needs.
> >>
> >> I have slightly mixed feelings, but I suppose that as we accepted Commit
> >> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> >> we can generally agree that it makes sense that calling genpd->on() actually
> >> turns on the power indeed
> >>
> >> What I'm worried about is if the clock is pre-configured to run at a high
> >> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> >> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> >>
> >> This rings the "downstream really did it better with putting clock dvfs states
> >> into the clk driver" bell, but I suppose the way to fight this would be to
> >> simply set_rate(fmax) there too..
> >>
> >> I attempted an experiment with pulling out the plug. MMCX enabled with the
> >> AHB clock off results in a read-as-zero. I tried really hard to disable the
> >> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> >> *really* can't just disable it" type behavior (verified with debugcc)
> > 
> > I think, in 8996 it was possible to disable it. Not sure about
> > 8998/630/660.
> > 
> >>
> >>
> >> There's a possible race condition if we don't do it:
> >>
> >> ------- bootloader --------
> >> configure display, mdp_clk=turbo
> >> ------- linux -------------
> >> load rpmhpd     |
> >> load venus      |
> >> set mmcx=lowsvs | mdp_clk is @ turbo
> >>                 | brownout
> >>                 | 
> >>                 | <mdss would only probe here>
> >>
> >> *but* that should be made impossible because of .sync_state().
> > 
> > Yep, sync_state should prevent MMCX or CX from dropping under the boot
> > level.
> > 
> >>
> >> This may impact hacky setups like simplefb, but as the name implies,
> >> that's hacky.
> >>
> >> Relying on .sync_state() however will not cover the case if the mdss
> >> module is removed and re-inserted later, possibly with mmcx disabled
> >> entirely but the clock not parked at a sufficiently low rate.
> >>
> >>
> >> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> >> plug the MDP opp table into it and set_rate(fmax) during mdss init
> > 
> > And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> > level even though DPU will change the clock freq.
> 
> That's a good point. Perhaps the easiest resolution will be to leave a
> comment before the prepare_enable() explaining that this should have a
> vote, but it's easier to rely on the providers' .sync_state() keeping them
> online until the consumers fully probe.

Yep.

> 
> [...]
> 
> > Let's go through them.
> > 
> > All SoC except those currently supported in DPU require SMP (shared
> > memory pool) support to be ported from the MDP5 driver.
> > 
> > Most of the remaining platforms (except MSM8994/92) also had HW cursor
> > implemented in a fancy way, in the LM rather than in a separate pipe.
> > I'd really like to postpone those, possibly first completing migration
> > of the other platforms and dropping support for them from MDP5.
> > 
> > 1.0  - old MSM8974
> >        I'd rather not touch it, it had bugs and I don't have HW
> 
> I have reasons to believe msm8974 v1.0 never reached store shelves.
> Let's remove this.

Please send a patch ;-)

> 
> > 1.1  - MSM8x26
> >        Probably Luca can better comment on it. Should be doable, but I
> >        don't see upstream devices using display on it.
> 
> Because there's no iommu support for these

I promised to put it on my todo list, but the list is very long.

> 
> > 1.2  - MSM8974
> >        I think it also had issues, no IOMMU support in upstream, etc.
> > 1.3  - APQ8084
> >        Had hw issues, no testing base, no MDSS in upstream DT
> > 1.6  - MSM8916 / MSM8939
> >        Can be done, low-hanging fruit for testing
> > 1.7  - MSM8996
> >        Supported in DPU
> > 1.8  - MSM8936
> >        No upsteram testing base
> 
> 8936 is 39 with some CPUs fused off (unless you have info suggesting
> otherwise)

Hmm, you added 8x36 to mdp5_cfg.c, stating it is 1.8. See commit
81c4389e4835 ("drm/msm/mdp5: Add MDP5 configuration for MSM8x36.")
Author: Konrad Dybcio <konradybcio@gmail.com>

Please remove it from the mdp5_cfg to avoid confusion.

> 
> > 1.9  - MSM8994
> >        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
> > 1.10 - MSM8992
> >        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
> > 1.11 - MSM8956 / 76
> >        No complete display configurations upstream
> 
> +Marijn, is your computer museum still running?

Should we open a Qualcomm Virtual Museum?

> 
> > 1.14 - MSM8937
> >        Supported in DPU
> > 1.15 - MSM8917
> >        Supported in DPU
> > 1.16 - MSM8953
> >        Supported in DPU
> > 1.17 - QCS405
> >        Zero testing base, no MDSS in upstream DT
> 
> No upstream MDP5 support either. And it doesn't seem like that SoC had
> much uses that didn't end up with the thing glued shut..

I saw and touched devices, but that was display-less version.

> 
> > MSM8994/92 would have been an ideal testbeds for SMP testing, but...
> > they mostly don't exist (please correct me if I'm wrong). Which means
> > that the next viable targets are MSM8916, MSM8x26 and MSM8956/76. All of
> > them require SMP support and don't make sense without cursor handling.
> 
> We can think about poking at some of these it one day, but certainly not
> high prio..

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 3/2/26 2:28 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 11:41:59AM +0100, Konrad Dybcio wrote:
>> On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>>>>>>>
>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>

[...]

>>> 1.0  - old MSM8974
>>>        I'd rather not touch it, it had bugs and I don't have HW
>>
>> I have reasons to believe msm8974 v1.0 never reached store shelves.
>> Let's remove this.
> 
> Please send a patch ;-)

done!

> 
>>
>>> 1.1  - MSM8x26
>>>        Probably Luca can better comment on it. Should be doable, but I
>>>        don't see upstream devices using display on it.
>>
>> Because there's no iommu support for these
> 
> I promised to put it on my todo list, but the list is very long.
> 
>>
>>> 1.2  - MSM8974
>>>        I think it also had issues, no IOMMU support in upstream, etc.
>>> 1.3  - APQ8084
>>>        Had hw issues, no testing base, no MDSS in upstream DT
>>> 1.6  - MSM8916 / MSM8939
>>>        Can be done, low-hanging fruit for testing
>>> 1.7  - MSM8996
>>>        Supported in DPU
>>> 1.8  - MSM8936
>>>        No upsteram testing base
>>
>> 8936 is 39 with some CPUs fused off (unless you have info suggesting
>> otherwise)
> 
> Hmm, you added 8x36 to mdp5_cfg.c, stating it is 1.8. See commit
> 81c4389e4835 ("drm/msm/mdp5: Add MDP5 configuration for MSM8x36.")
> Author: Konrad Dybcio <konradybcio@gmail.com>
> 
> Please remove it from the mdp5_cfg to avoid confusion.

v1.6 is strictly for 8916. 8936/39 both use v1.8.

>>> 1.9  - MSM8994
>>>        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
>>> 1.10 - MSM8992
>>>        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
>>> 1.11 - MSM8956 / 76
>>>        No complete display configurations upstream
>>
>> +Marijn, is your computer museum still running?
> 
> Should we open a Qualcomm Virtual Museum?

Maybe someone has a drawer full of QSD8250s!

> 
>>
>>> 1.14 - MSM8937
>>>        Supported in DPU
>>> 1.15 - MSM8917
>>>        Supported in DPU
>>> 1.16 - MSM8953
>>>        Supported in DPU
>>> 1.17 - QCS405
>>>        Zero testing base, no MDSS in upstream DT
>>
>> No upstream MDP5 support either. And it doesn't seem like that SoC had
>> much uses that didn't end up with the thing glued shut..
> 
> I saw and touched devices, but that was display-less version.

Only further confirming it's not worth pursuing

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Mon, Mar 02, 2026 at 02:46:33PM +0100, Konrad Dybcio wrote:
> On 3/2/26 2:28 PM, Dmitry Baryshkov wrote:
> > On Mon, Mar 02, 2026 at 11:41:59AM +0100, Konrad Dybcio wrote:
> >> On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
> >>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> >>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> >>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
> >>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>>>>>>>
> >>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> 
> [...]
> 
> > 
> >>
> >>> 1.1  - MSM8x26
> >>>        Probably Luca can better comment on it. Should be doable, but I
> >>>        don't see upstream devices using display on it.
> >>
> >> Because there's no iommu support for these
> > 
> > I promised to put it on my todo list, but the list is very long.
> > 
> >>
> >>> 1.2  - MSM8974
> >>>        I think it also had issues, no IOMMU support in upstream, etc.
> >>> 1.3  - APQ8084
> >>>        Had hw issues, no testing base, no MDSS in upstream DT
> >>> 1.6  - MSM8916 / MSM8939
> >>>        Can be done, low-hanging fruit for testing
> >>> 1.7  - MSM8996
> >>>        Supported in DPU
> >>> 1.8  - MSM8936
> >>>        No upsteram testing base
> >>
> >> 8936 is 39 with some CPUs fused off (unless you have info suggesting
> >> otherwise)
> > 
> > Hmm, you added 8x36 to mdp5_cfg.c, stating it is 1.8. See commit
> > 81c4389e4835 ("drm/msm/mdp5: Add MDP5 configuration for MSM8x36.")
> > Author: Konrad Dybcio <konradybcio@gmail.com>
> > 
> > Please remove it from the mdp5_cfg to avoid confusion.
> 
> v1.6 is strictly for 8916. 8936/39 both use v1.8.

I tend to agree with you. It's interesting that core list doesn't (and
likely is wrong).

> 
> >>> 1.9  - MSM8994
> >>>        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
> >>> 1.10 - MSM8992
> >>>        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
> >>> 1.11 - MSM8956 / 76
> >>>        No complete display configurations upstream
> >>
> >> +Marijn, is your computer museum still running?
> > 
> > Should we open a Qualcomm Virtual Museum?
> 
> Maybe someone has a drawer full of QSD8250s!

Only APQ8060 here, but it's not even online after the reorg.

Anyway, I assume that we have something alive and kicking for:
- 1.1, 8226 (currently unusable, no IOMMU)
- 1.2, 8974 (currently unusable, no IOMMU)
- 1.6, 8916
- 1.8, 8939
- 1.9, 8994 (almost dead, I'd say)
- 1.10, 8992 (almost dead, I'd say)
- 1.11, 8956/76

It seems we can ignore (or drop) apq8084 (no activity since Dec 2019).

Already in DPU:
- 1.7, 8996
- 1.14, 8937
- 1.15, 8917
- 1.16, 8953

Which means:
- port 8916, copy for 8939 / 8956/76 and hope it works
- add LM-cursor support
- deprecate mdp5, possibly moving it to EXPERT
- add 8226 / 8974 once we have IOMMU
- drop mdp5

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Luca Weiss]

On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>> > On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>> >> On 1/12/26 9:25 AM, yuanjiey wrote:
>> >>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>> >>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>> >>>>>
>> >>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>> >>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>> >>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>> 
>> [...]
>> 
>> > Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
>> > dpu_runtime_suspend, then we should be able to also skip setting OPP
>> > corner in dpu_runtime_resume(), because the previously set corner should
>> > be viable until drm/msm driver commits new state / new modes.
>> 
>> That matches my understanding.
>> 
>> > The only important issue is to set the corner before starting up the
>> > DPU, where we already have code to set MDP_CLK to the max frequency.
>> > 
>> > Which means, we only need to drop the dev_pm_set_rate call from the
>> > dpu_runtime_suspend().
>> 
>> I concur.
>> 
>> >> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>> >> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
>> > 
>> > No. As far as I remember, MDP_CLK is necessary to access MDSS registers
>> > (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
>> > platforms")), I don't remember if accessing HW_REV without MDP_CLK
>> > resulted in a zero reads or in a crash. At the same time it needs to be
>> > enabled to any rate, which means that for most of the operations
>> > msm_mdss.c can rely on DPU keeping the clock up and running.
>> > 
>> >> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>> >> MDSS_AHB one seems to have 3 actually configurable performance points
>> >> that neither we nor seemingly the downstream driver seem to really care
>> >> about (i.e. both just treat it as on/off). If we need to scale it, we
>> >> should add an OPP table, if we don't, we should at least add required-opps.
>> > 
>> > I think, dispcc already has a minimal vote on the MMCX, which fulfill
>> > these needs.
>> 
>> I have slightly mixed feelings, but I suppose that as we accepted Commit
>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
>> we can generally agree that it makes sense that calling genpd->on() actually
>> turns on the power indeed
>> 
>> What I'm worried about is if the clock is pre-configured to run at a high
>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
>> 
>> This rings the "downstream really did it better with putting clock dvfs states
>> into the clk driver" bell, but I suppose the way to fight this would be to
>> simply set_rate(fmax) there too..
>> 
>> I attempted an experiment with pulling out the plug. MMCX enabled with the
>> AHB clock off results in a read-as-zero. I tried really hard to disable the
>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
>> *really* can't just disable it" type behavior (verified with debugcc)
>
> I think, in 8996 it was possible to disable it. Not sure about
> 8998/630/660.
>
>> 
>> 
>> There's a possible race condition if we don't do it:
>> 
>> ------- bootloader --------
>> configure display, mdp_clk=turbo
>> ------- linux -------------
>> load rpmhpd     |
>> load venus      |
>> set mmcx=lowsvs | mdp_clk is @ turbo
>>                 | brownout
>>                 | 
>>                 | <mdss would only probe here>
>> 
>> *but* that should be made impossible because of .sync_state().
>
> Yep, sync_state should prevent MMCX or CX from dropping under the boot
> level.
>
>> 
>> This may impact hacky setups like simplefb, but as the name implies,
>> that's hacky.
>> 
>> Relying on .sync_state() however will not cover the case if the mdss
>> module is removed and re-inserted later, possibly with mmcx disabled
>> entirely but the clock not parked at a sufficiently low rate.
>> 
>> 
>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
>> plug the MDP opp table into it and set_rate(fmax) during mdss init
>
> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> level even though DPU will change the clock freq.
>
>> 
>> >> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
>> >> targets supported by these may not even have OPP tables and are generally
>> >> not super high priority for spending time on.. that can, I'd kick down the
>> >> road unless someone really wants to step up
>> > 
>> > I'd really not bother with those two drivers, unless we start seing
>> > crashes. For MDP4 platforms we don't implement power domains at all, no
>> > interconnects, etc., which means that the system will never go to a
>> > lower power state.
>> 
>> Right. Might be that 2030-something arrives and armv7 is gone before someone
>> randomly decides to work on that!
>> 
>> > MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
>> > SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
>> > has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
>> > should start there by adding missing bits adding corresponding
>> > dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
>> 
>> A bit off-topic, but:
>> 
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>> 
>> 96 is in DPU. any other candidates that should be moved over?
>
> Let's go through them.
>
> All SoC except those currently supported in DPU require SMP (shared
> memory pool) support to be ported from the MDP5 driver.
>
> Most of the remaining platforms (except MSM8994/92) also had HW cursor
> implemented in a fancy way, in the LM rather than in a separate pipe.
> I'd really like to postpone those, possibly first completing migration
> of the other platforms and dropping support for them from MDP5.
>
> 1.0  - old MSM8974
>        I'd rather not touch it, it had bugs and I don't have HW
> 1.1  - MSM8x26
>        Probably Luca can better comment on it. Should be doable, but I
>        don't see upstream devices using display on it.

I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
devices working with MDP5 fine. I should've probably upstreamed panel
driver & dts but they haven't been high priority..

Btw IOMMU support is here missing as well, so no GPU support anymore
since 6.17 if I'm not mistaken.

> 1.2  - MSM8974
>        I think it also had issues, no IOMMU support in upstream, etc.

Plenty of 'issues' for sure ;) but apart from GPU driver having dropped
no-IOMMU codepath they should be fairly fine, for what's currently
upstream.

I think also here, plenty of devices that do have panel support but not
much upstream. It's kind of all the same though, nothing exciting. Panel
driver with init sequence plus the same dts enablement bits.

Regards
Luca

> 1.3  - APQ8084
>        Had hw issues, no testing base, no MDSS in upstream DT
> 1.6  - MSM8916 / MSM8939
>        Can be done, low-hanging fruit for testing
> 1.7  - MSM8996
>        Supported in DPU
> 1.8  - MSM8936
>        No upsteram testing base
> 1.9  - MSM8994
>        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
> 1.10 - MSM8992
>        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
> 1.11 - MSM8956 / 76
>        No complete display configurations upstream
> 1.14 - MSM8937
>        Supported in DPU
> 1.15 - MSM8917
>        Supported in DPU
> 1.16 - MSM8953
>        Supported in DPU
> 1.17 - QCS405
>        Zero testing base, no MDSS in upstream DT
>
> MSM8994/92 would have been an ideal testbeds for SMP testing, but...
> they mostly don't exist (please correct me if I'm wrong). Which means
> that the next viable targets are MSM8916, MSM8x26 and MSM8956/76. All of
> them require SMP support and don't make sense without cursor handling.
>
>> > A note to myself to also add OPP tables support to the HDMI driver.
>> 
>> Eliza!
>
> MSM8996 / 98!

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
> > On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> >> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> >> > On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >> >> On 1/12/26 9:25 AM, yuanjiey wrote:
> >> >>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >> >>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >> >>>>>
> >> >>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >> >>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >> >>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >> 
> >> [...]
> >> 
> >> > Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> >> > dpu_runtime_suspend, then we should be able to also skip setting OPP
> >> > corner in dpu_runtime_resume(), because the previously set corner should
> >> > be viable until drm/msm driver commits new state / new modes.
> >> 
> >> That matches my understanding.
> >> 
> >> > The only important issue is to set the corner before starting up the
> >> > DPU, where we already have code to set MDP_CLK to the max frequency.
> >> > 
> >> > Which means, we only need to drop the dev_pm_set_rate call from the
> >> > dpu_runtime_suspend().
> >> 
> >> I concur.
> >> 
> >> >> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >> >> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> >> > 
> >> > No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> >> > (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> >> > platforms")), I don't remember if accessing HW_REV without MDP_CLK
> >> > resulted in a zero reads or in a crash. At the same time it needs to be
> >> > enabled to any rate, which means that for most of the operations
> >> > msm_mdss.c can rely on DPU keeping the clock up and running.
> >> > 
> >> >> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >> >> MDSS_AHB one seems to have 3 actually configurable performance points
> >> >> that neither we nor seemingly the downstream driver seem to really care
> >> >> about (i.e. both just treat it as on/off). If we need to scale it, we
> >> >> should add an OPP table, if we don't, we should at least add required-opps.
> >> > 
> >> > I think, dispcc already has a minimal vote on the MMCX, which fulfill
> >> > these needs.
> >> 
> >> I have slightly mixed feelings, but I suppose that as we accepted Commit
> >> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> >> we can generally agree that it makes sense that calling genpd->on() actually
> >> turns on the power indeed
> >> 
> >> What I'm worried about is if the clock is pre-configured to run at a high
> >> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> >> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> >> 
> >> This rings the "downstream really did it better with putting clock dvfs states
> >> into the clk driver" bell, but I suppose the way to fight this would be to
> >> simply set_rate(fmax) there too..
> >> 
> >> I attempted an experiment with pulling out the plug. MMCX enabled with the
> >> AHB clock off results in a read-as-zero. I tried really hard to disable the
> >> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> >> *really* can't just disable it" type behavior (verified with debugcc)
> >
> > I think, in 8996 it was possible to disable it. Not sure about
> > 8998/630/660.
> >
> >> 
> >> 
> >> There's a possible race condition if we don't do it:
> >> 
> >> ------- bootloader --------
> >> configure display, mdp_clk=turbo
> >> ------- linux -------------
> >> load rpmhpd     |
> >> load venus      |
> >> set mmcx=lowsvs | mdp_clk is @ turbo
> >>                 | brownout
> >>                 | 
> >>                 | <mdss would only probe here>
> >> 
> >> *but* that should be made impossible because of .sync_state().
> >
> > Yep, sync_state should prevent MMCX or CX from dropping under the boot
> > level.
> >
> >> 
> >> This may impact hacky setups like simplefb, but as the name implies,
> >> that's hacky.
> >> 
> >> Relying on .sync_state() however will not cover the case if the mdss
> >> module is removed and re-inserted later, possibly with mmcx disabled
> >> entirely but the clock not parked at a sufficiently low rate.
> >> 
> >> 
> >> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> >> plug the MDP opp table into it and set_rate(fmax) during mdss init
> >
> > And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> > level even though DPU will change the clock freq.
> >
> >> 
> >> >> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> >> >> targets supported by these may not even have OPP tables and are generally
> >> >> not super high priority for spending time on.. that can, I'd kick down the
> >> >> road unless someone really wants to step up
> >> > 
> >> > I'd really not bother with those two drivers, unless we start seing
> >> > crashes. For MDP4 platforms we don't implement power domains at all, no
> >> > interconnects, etc., which means that the system will never go to a
> >> > lower power state.
> >> 
> >> Right. Might be that 2030-something arrives and armv7 is gone before someone
> >> randomly decides to work on that!
> >> 
> >> > MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> >> > SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> >> > has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> >> > should start there by adding missing bits adding corresponding
> >> > dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
> >> 
> >> A bit off-topic, but:
> >> 
> >> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> >> 
> >> 96 is in DPU. any other candidates that should be moved over?
> >
> > Let's go through them.
> >
> > All SoC except those currently supported in DPU require SMP (shared
> > memory pool) support to be ported from the MDP5 driver.
> >
> > Most of the remaining platforms (except MSM8994/92) also had HW cursor
> > implemented in a fancy way, in the LM rather than in a separate pipe.
> > I'd really like to postpone those, possibly first completing migration
> > of the other platforms and dropping support for them from MDP5.
> >
> > 1.0  - old MSM8974
> >        I'd rather not touch it, it had bugs and I don't have HW
> > 1.1  - MSM8x26
> >        Probably Luca can better comment on it. Should be doable, but I
> >        don't see upstream devices using display on it.
> 
> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
> devices working with MDP5 fine. I should've probably upstreamed panel
> driver & dts but they haven't been high priority..

I think I have been saying this: having a panel & dsi enabled in DT is
the only way for me to know that the display is working on this
platform. I'd really kindly ask you and other activists to get at least
some panel drivers and corresponding DT bits upstream. It is really an
important flag for me.

I can propose some kind of bounty for those getting MDSS+panel supported
in Qcom DT. (Beer at the next conf we meet? Some stickers for the
laptops and phones? Mämmi?)

> 
> Btw IOMMU support is here missing as well, so no GPU support anymore
> since 6.17 if I'm not mistaken.
> 
> > 1.2  - MSM8974
> >        I think it also had issues, no IOMMU support in upstream, etc.
> 
> Plenty of 'issues' for sure ;) but apart from GPU driver having dropped
> no-IOMMU codepath they should be fairly fine, for what's currently
> upstream.
> 
> I think also here, plenty of devices that do have panel support but not
> much upstream. It's kind of all the same though, nothing exciting. Panel
> driver with init sequence plus the same dts enablement bits.

-- 
With best wishes
Dmitry

deliberations about the future of mdp5 (was: Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency)

[Konrad Dybcio]

On 3/2/26 3:27 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 02:46:33PM +0100, Konrad Dybcio wrote:
>> On 3/2/26 2:28 PM, Dmitry Baryshkov wrote:
>>> On Mon, Mar 02, 2026 at 11:41:59AM +0100, Konrad Dybcio wrote:
>>>> On 2/27/26 8:05 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>>>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>>>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>>>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
>>>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>>>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>>
>> [...]
>>
>>>
>>>>
>>>>> 1.1  - MSM8x26
>>>>>        Probably Luca can better comment on it. Should be doable, but I
>>>>>        don't see upstream devices using display on it.
>>>>
>>>> Because there's no iommu support for these
>>>
>>> I promised to put it on my todo list, but the list is very long.
>>>
>>>>
>>>>> 1.2  - MSM8974
>>>>>        I think it also had issues, no IOMMU support in upstream, etc.
>>>>> 1.3  - APQ8084
>>>>>        Had hw issues, no testing base, no MDSS in upstream DT
>>>>> 1.6  - MSM8916 / MSM8939
>>>>>        Can be done, low-hanging fruit for testing
>>>>> 1.7  - MSM8996
>>>>>        Supported in DPU
>>>>> 1.8  - MSM8936
>>>>>        No upsteram testing base
>>>>
>>>> 8936 is 39 with some CPUs fused off (unless you have info suggesting
>>>> otherwise)
>>>
>>> Hmm, you added 8x36 to mdp5_cfg.c, stating it is 1.8. See commit
>>> 81c4389e4835 ("drm/msm/mdp5: Add MDP5 configuration for MSM8x36.")
>>> Author: Konrad Dybcio <konradybcio@gmail.com>
>>>
>>> Please remove it from the mdp5_cfg to avoid confusion.
>>
>> v1.6 is strictly for 8916. 8936/39 both use v1.8.
> 
> I tend to agree with you. It's interesting that core list doesn't (and
> likely is wrong).
> 
>>
>>>>> 1.9  - MSM8994
>>>>>        No upstream testing base, no MDSS in upstream DT, normal CURSOR planes
>>>>> 1.10 - MSM8992
>>>>>        Even less testing base, no MDSS in upstream DT, normal CURSOR planes
>>>>> 1.11 - MSM8956 / 76
>>>>>        No complete display configurations upstream
>>>>
>>>> +Marijn, is your computer museum still running?
>>>
>>> Should we open a Qualcomm Virtual Museum?
>>
>> Maybe someone has a drawer full of QSD8250s!
> 
> Only APQ8060 here, but it's not even online after the reorg.
> 
> Anyway, I assume that we have something alive and kicking for:
> - 1.1, 8226 (currently unusable, no IOMMU)
> - 1.2, 8974 (currently unusable, no IOMMU)
> - 1.6, 8916
> - 1.8, 8939
> - 1.9, 8994 (almost dead, I'd say)
> - 1.10, 8992 (almost dead, I'd say)

My estimates for 8992/4 would be just a dozen or less enthusiasts, at most
a couple dozen. The platform is in a tragic/borderline unsupported state
upstream today and little effort has been seen to fix it, would probably
need to come from us anyway, with essentially a re-bring-up..

Plus those have no IOMMU either (same camp as 74) and even if they did,
I reckon any users would be happier to run the actually-maintained codebase.

I think I may have a partially-ported 94 branch somewhere on some SSD..

Definitely more of a 'weekend project' type thing and shouldn't be
considered a blocker.

> - 1.11, 8956/76
> 
> It seems we can ignore (or drop) apq8084 (no activity since Dec 2019).
> 
> Already in DPU:
> - 1.7, 8996
> - 1.14, 8937
> - 1.15, 8917
> - 1.16, 8953
> 
> Which means:
> - port 8916, copy for 8939 / 8956/76 and hope it works

I suppose it's not a high priority for you. Maybe some 8916 enthusiast
(+CC a couple) would like to try? (context: DPU1 support)

Konrad

> - add LM-cursor support
> - deprecate mdp5, possibly moving it to EXPERT
> - add 8226 / 8974 once we have IOMMU
> - drop mdp5
> 

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Luca Weiss]

On Mon Mar 2, 2026 at 4:17 PM CET, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
>> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
>> > On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>> >> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>> >> > On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>> >> >> On 1/12/26 9:25 AM, yuanjiey wrote:
>> >> >>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>> >> >>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>> >> >>>>>
>> >> >>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>> >> >>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>> >> >>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>> >> 
>> >> [...]
>> >> 
>> >> > Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
>> >> > dpu_runtime_suspend, then we should be able to also skip setting OPP
>> >> > corner in dpu_runtime_resume(), because the previously set corner should
>> >> > be viable until drm/msm driver commits new state / new modes.
>> >> 
>> >> That matches my understanding.
>> >> 
>> >> > The only important issue is to set the corner before starting up the
>> >> > DPU, where we already have code to set MDP_CLK to the max frequency.
>> >> > 
>> >> > Which means, we only need to drop the dev_pm_set_rate call from the
>> >> > dpu_runtime_suspend().
>> >> 
>> >> I concur.
>> >> 
>> >> >> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>> >> >> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
>> >> > 
>> >> > No. As far as I remember, MDP_CLK is necessary to access MDSS registers
>> >> > (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
>> >> > platforms")), I don't remember if accessing HW_REV without MDP_CLK
>> >> > resulted in a zero reads or in a crash. At the same time it needs to be
>> >> > enabled to any rate, which means that for most of the operations
>> >> > msm_mdss.c can rely on DPU keeping the clock up and running.
>> >> > 
>> >> >> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>> >> >> MDSS_AHB one seems to have 3 actually configurable performance points
>> >> >> that neither we nor seemingly the downstream driver seem to really care
>> >> >> about (i.e. both just treat it as on/off). If we need to scale it, we
>> >> >> should add an OPP table, if we don't, we should at least add required-opps.
>> >> > 
>> >> > I think, dispcc already has a minimal vote on the MMCX, which fulfill
>> >> > these needs.
>> >> 
>> >> I have slightly mixed feelings, but I suppose that as we accepted Commit
>> >> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
>> >> we can generally agree that it makes sense that calling genpd->on() actually
>> >> turns on the power indeed
>> >> 
>> >> What I'm worried about is if the clock is pre-configured to run at a high
>> >> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
>> >> and doesn't impact the state of M/N/D at a glance), we may get a brownout
>> >> 
>> >> This rings the "downstream really did it better with putting clock dvfs states
>> >> into the clk driver" bell, but I suppose the way to fight this would be to
>> >> simply set_rate(fmax) there too..
>> >> 
>> >> I attempted an experiment with pulling out the plug. MMCX enabled with the
>> >> AHB clock off results in a read-as-zero. I tried really hard to disable the
>> >> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
>> >> *really* can't just disable it" type behavior (verified with debugcc)
>> >
>> > I think, in 8996 it was possible to disable it. Not sure about
>> > 8998/630/660.
>> >
>> >> 
>> >> 
>> >> There's a possible race condition if we don't do it:
>> >> 
>> >> ------- bootloader --------
>> >> configure display, mdp_clk=turbo
>> >> ------- linux -------------
>> >> load rpmhpd     |
>> >> load venus      |
>> >> set mmcx=lowsvs | mdp_clk is @ turbo
>> >>                 | brownout
>> >>                 | 
>> >>                 | <mdss would only probe here>
>> >> 
>> >> *but* that should be made impossible because of .sync_state().
>> >
>> > Yep, sync_state should prevent MMCX or CX from dropping under the boot
>> > level.
>> >
>> >> 
>> >> This may impact hacky setups like simplefb, but as the name implies,
>> >> that's hacky.
>> >> 
>> >> Relying on .sync_state() however will not cover the case if the mdss
>> >> module is removed and re-inserted later, possibly with mmcx disabled
>> >> entirely but the clock not parked at a sufficiently low rate.
>> >> 
>> >> 
>> >> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
>> >> plug the MDP opp table into it and set_rate(fmax) during mdss init
>> >
>> > And what will drop it afterwards? MDSS will still vote on the MMCX / CX
>> > level even though DPU will change the clock freq.
>> >
>> >> 
>> >> >> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
>> >> >> targets supported by these may not even have OPP tables and are generally
>> >> >> not super high priority for spending time on.. that can, I'd kick down the
>> >> >> road unless someone really wants to step up
>> >> > 
>> >> > I'd really not bother with those two drivers, unless we start seing
>> >> > crashes. For MDP4 platforms we don't implement power domains at all, no
>> >> > interconnects, etc., which means that the system will never go to a
>> >> > lower power state.
>> >> 
>> >> Right. Might be that 2030-something arrives and armv7 is gone before someone
>> >> randomly decides to work on that!
>> >> 
>> >> > MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
>> >> > SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
>> >> > has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
>> >> > should start there by adding missing bits adding corresponding
>> >> > dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
>> >> 
>> >> A bit off-topic, but:
>> >> 
>> >> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>> >> 
>> >> 96 is in DPU. any other candidates that should be moved over?
>> >
>> > Let's go through them.
>> >
>> > All SoC except those currently supported in DPU require SMP (shared
>> > memory pool) support to be ported from the MDP5 driver.
>> >
>> > Most of the remaining platforms (except MSM8994/92) also had HW cursor
>> > implemented in a fancy way, in the LM rather than in a separate pipe.
>> > I'd really like to postpone those, possibly first completing migration
>> > of the other platforms and dropping support for them from MDP5.
>> >
>> > 1.0  - old MSM8974
>> >        I'd rather not touch it, it had bugs and I don't have HW
>> > 1.1  - MSM8x26
>> >        Probably Luca can better comment on it. Should be doable, but I
>> >        don't see upstream devices using display on it.
>> 
>> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
>> devices working with MDP5 fine. I should've probably upstreamed panel
>> driver & dts but they haven't been high priority..
>
> I think I have been saying this: having a panel & dsi enabled in DT is
> the only way for me to know that the display is working on this
> platform. I'd really kindly ask you and other activists to get at least
> some panel drivers and corresponding DT bits upstream. It is really an
> important flag for me.
>
> I can propose some kind of bounty for those getting MDSS+panel supported
> in Qcom DT. (Beer at the next conf we meet? Some stickers for the
> laptops and phones? Mämmi?)

Hm I realized yesterday (through trying it), that also MDSS is broken
since the no-IOMMU codepath was removed from drm/msm. I thought this was
only affecting GPU but it's also affecting MDSS/MDP5 :(

So I guess no panel enablement anytime soon until the IOMMU situation is
sorted out, for both 8226 and 8974...

Regards
Luca

>
>> 
>> Btw IOMMU support is here missing as well, so no GPU support anymore
>> since 6.17 if I'm not mistaken.
>> 
>> > 1.2  - MSM8974
>> >        I think it also had issues, no IOMMU support in upstream, etc.
>> 
>> Plenty of 'issues' for sure ;) but apart from GPU driver having dropped
>> no-IOMMU codepath they should be fairly fine, for what's currently
>> upstream.
>> 
>> I think also here, plenty of devices that do have panel support but not
>> much upstream. It's kind of all the same though, nothing exciting. Panel
>> driver with init sequence plus the same dts enablement bits.

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Konrad Dybcio]

On 3/17/26 9:07 AM, Luca Weiss wrote:
> On Mon Mar 2, 2026 at 4:17 PM CET, Dmitry Baryshkov wrote:
>> On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
>>> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
>>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
>>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>>>>>
>>>>> [...]
>>>>>
>>>>>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
>>>>>> dpu_runtime_suspend, then we should be able to also skip setting OPP
>>>>>> corner in dpu_runtime_resume(), because the previously set corner should
>>>>>> be viable until drm/msm driver commits new state / new modes.
>>>>>
>>>>> That matches my understanding.
>>>>>
>>>>>> The only important issue is to set the corner before starting up the
>>>>>> DPU, where we already have code to set MDP_CLK to the max frequency.
>>>>>>
>>>>>> Which means, we only need to drop the dev_pm_set_rate call from the
>>>>>> dpu_runtime_suspend().
>>>>>
>>>>> I concur.
>>>>>
>>>>>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>>>>>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
>>>>>>
>>>>>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
>>>>>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
>>>>>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
>>>>>> resulted in a zero reads or in a crash. At the same time it needs to be
>>>>>> enabled to any rate, which means that for most of the operations
>>>>>> msm_mdss.c can rely on DPU keeping the clock up and running.
>>>>>>
>>>>>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>>>>>>> MDSS_AHB one seems to have 3 actually configurable performance points
>>>>>>> that neither we nor seemingly the downstream driver seem to really care
>>>>>>> about (i.e. both just treat it as on/off). If we need to scale it, we
>>>>>>> should add an OPP table, if we don't, we should at least add required-opps.
>>>>>>
>>>>>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
>>>>>> these needs.
>>>>>
>>>>> I have slightly mixed feelings, but I suppose that as we accepted Commit
>>>>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
>>>>> we can generally agree that it makes sense that calling genpd->on() actually
>>>>> turns on the power indeed
>>>>>
>>>>> What I'm worried about is if the clock is pre-configured to run at a high
>>>>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
>>>>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
>>>>>
>>>>> This rings the "downstream really did it better with putting clock dvfs states
>>>>> into the clk driver" bell, but I suppose the way to fight this would be to
>>>>> simply set_rate(fmax) there too..
>>>>>
>>>>> I attempted an experiment with pulling out the plug. MMCX enabled with the
>>>>> AHB clock off results in a read-as-zero. I tried really hard to disable the
>>>>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
>>>>> *really* can't just disable it" type behavior (verified with debugcc)
>>>>
>>>> I think, in 8996 it was possible to disable it. Not sure about
>>>> 8998/630/660.
>>>>
>>>>>
>>>>>
>>>>> There's a possible race condition if we don't do it:
>>>>>
>>>>> ------- bootloader --------
>>>>> configure display, mdp_clk=turbo
>>>>> ------- linux -------------
>>>>> load rpmhpd     |
>>>>> load venus      |
>>>>> set mmcx=lowsvs | mdp_clk is @ turbo
>>>>>                 | brownout
>>>>>                 | 
>>>>>                 | <mdss would only probe here>
>>>>>
>>>>> *but* that should be made impossible because of .sync_state().
>>>>
>>>> Yep, sync_state should prevent MMCX or CX from dropping under the boot
>>>> level.
>>>>
>>>>>
>>>>> This may impact hacky setups like simplefb, but as the name implies,
>>>>> that's hacky.
>>>>>
>>>>> Relying on .sync_state() however will not cover the case if the mdss
>>>>> module is removed and re-inserted later, possibly with mmcx disabled
>>>>> entirely but the clock not parked at a sufficiently low rate.
>>>>>
>>>>>
>>>>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
>>>>> plug the MDP opp table into it and set_rate(fmax) during mdss init
>>>>
>>>> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
>>>> level even though DPU will change the clock freq.
>>>>
>>>>>
>>>>>>> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
>>>>>>> targets supported by these may not even have OPP tables and are generally
>>>>>>> not super high priority for spending time on.. that can, I'd kick down the
>>>>>>> road unless someone really wants to step up
>>>>>>
>>>>>> I'd really not bother with those two drivers, unless we start seing
>>>>>> crashes. For MDP4 platforms we don't implement power domains at all, no
>>>>>> interconnects, etc., which means that the system will never go to a
>>>>>> lower power state.
>>>>>
>>>>> Right. Might be that 2030-something arrives and armv7 is gone before someone
>>>>> randomly decides to work on that!
>>>>>
>>>>>> MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
>>>>>> SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
>>>>>> has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
>>>>>> should start there by adding missing bits adding corresponding
>>>>>> dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
>>>>>
>>>>> A bit off-topic, but:
>>>>>
>>>>> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>>>>>
>>>>> 96 is in DPU. any other candidates that should be moved over?
>>>>
>>>> Let's go through them.
>>>>
>>>> All SoC except those currently supported in DPU require SMP (shared
>>>> memory pool) support to be ported from the MDP5 driver.
>>>>
>>>> Most of the remaining platforms (except MSM8994/92) also had HW cursor
>>>> implemented in a fancy way, in the LM rather than in a separate pipe.
>>>> I'd really like to postpone those, possibly first completing migration
>>>> of the other platforms and dropping support for them from MDP5.
>>>>
>>>> 1.0  - old MSM8974
>>>>        I'd rather not touch it, it had bugs and I don't have HW
>>>> 1.1  - MSM8x26
>>>>        Probably Luca can better comment on it. Should be doable, but I
>>>>        don't see upstream devices using display on it.
>>>
>>> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
>>> devices working with MDP5 fine. I should've probably upstreamed panel
>>> driver & dts but they haven't been high priority..
>>
>> I think I have been saying this: having a panel & dsi enabled in DT is
>> the only way for me to know that the display is working on this
>> platform. I'd really kindly ask you and other activists to get at least
>> some panel drivers and corresponding DT bits upstream. It is really an
>> important flag for me.
>>
>> I can propose some kind of bounty for those getting MDSS+panel supported
>> in Qcom DT. (Beer at the next conf we meet? Some stickers for the
>> laptops and phones? Mämmi?)
> 
> Hm I realized yesterday (through trying it), that also MDSS is broken
> since the no-IOMMU codepath was removed from drm/msm. I thought this was
> only affecting GPU but it's also affecting MDSS/MDP5 :(
> 
> So I guess no panel enablement anytime soon until the IOMMU situation is
> sorted out, for both 8226 and 8974...

If you're sure the panel drivers are good (e.g. they worked on 6.whatever
prior to drm/msm saying sayonara to carveout), I think no one would object
to have them merged separately, so that you don't have to wait until
who-knows-when and keep rebasing them by hand

Konrad

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Tue, Mar 17, 2026 at 09:59:28AM +0100, Konrad Dybcio wrote:
> On 3/17/26 9:07 AM, Luca Weiss wrote:
> > On Mon Mar 2, 2026 at 4:17 PM CET, Dmitry Baryshkov wrote:
> >> On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
> >>> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
> >>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> >>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> >>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
> >>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> >>>>>> dpu_runtime_suspend, then we should be able to also skip setting OPP
> >>>>>> corner in dpu_runtime_resume(), because the previously set corner should
> >>>>>> be viable until drm/msm driver commits new state / new modes.
> >>>>>
> >>>>> That matches my understanding.
> >>>>>
> >>>>>> The only important issue is to set the corner before starting up the
> >>>>>> DPU, where we already have code to set MDP_CLK to the max frequency.
> >>>>>>
> >>>>>> Which means, we only need to drop the dev_pm_set_rate call from the
> >>>>>> dpu_runtime_suspend().
> >>>>>
> >>>>> I concur.
> >>>>>
> >>>>>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >>>>>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> >>>>>>
> >>>>>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> >>>>>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> >>>>>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
> >>>>>> resulted in a zero reads or in a crash. At the same time it needs to be
> >>>>>> enabled to any rate, which means that for most of the operations
> >>>>>> msm_mdss.c can rely on DPU keeping the clock up and running.
> >>>>>>
> >>>>>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >>>>>>> MDSS_AHB one seems to have 3 actually configurable performance points
> >>>>>>> that neither we nor seemingly the downstream driver seem to really care
> >>>>>>> about (i.e. both just treat it as on/off). If we need to scale it, we
> >>>>>>> should add an OPP table, if we don't, we should at least add required-opps.
> >>>>>>
> >>>>>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
> >>>>>> these needs.
> >>>>>
> >>>>> I have slightly mixed feelings, but I suppose that as we accepted Commit
> >>>>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> >>>>> we can generally agree that it makes sense that calling genpd->on() actually
> >>>>> turns on the power indeed
> >>>>>
> >>>>> What I'm worried about is if the clock is pre-configured to run at a high
> >>>>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> >>>>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> >>>>>
> >>>>> This rings the "downstream really did it better with putting clock dvfs states
> >>>>> into the clk driver" bell, but I suppose the way to fight this would be to
> >>>>> simply set_rate(fmax) there too..
> >>>>>
> >>>>> I attempted an experiment with pulling out the plug. MMCX enabled with the
> >>>>> AHB clock off results in a read-as-zero. I tried really hard to disable the
> >>>>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> >>>>> *really* can't just disable it" type behavior (verified with debugcc)
> >>>>
> >>>> I think, in 8996 it was possible to disable it. Not sure about
> >>>> 8998/630/660.
> >>>>
> >>>>>
> >>>>>
> >>>>> There's a possible race condition if we don't do it:
> >>>>>
> >>>>> ------- bootloader --------
> >>>>> configure display, mdp_clk=turbo
> >>>>> ------- linux -------------
> >>>>> load rpmhpd     |
> >>>>> load venus      |
> >>>>> set mmcx=lowsvs | mdp_clk is @ turbo
> >>>>>                 | brownout
> >>>>>                 | 
> >>>>>                 | <mdss would only probe here>
> >>>>>
> >>>>> *but* that should be made impossible because of .sync_state().
> >>>>
> >>>> Yep, sync_state should prevent MMCX or CX from dropping under the boot
> >>>> level.
> >>>>
> >>>>>
> >>>>> This may impact hacky setups like simplefb, but as the name implies,
> >>>>> that's hacky.
> >>>>>
> >>>>> Relying on .sync_state() however will not cover the case if the mdss
> >>>>> module is removed and re-inserted later, possibly with mmcx disabled
> >>>>> entirely but the clock not parked at a sufficiently low rate.
> >>>>>
> >>>>>
> >>>>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> >>>>> plug the MDP opp table into it and set_rate(fmax) during mdss init
> >>>>
> >>>> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> >>>> level even though DPU will change the clock freq.
> >>>>
> >>>>>
> >>>>>>> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> >>>>>>> targets supported by these may not even have OPP tables and are generally
> >>>>>>> not super high priority for spending time on.. that can, I'd kick down the
> >>>>>>> road unless someone really wants to step up
> >>>>>>
> >>>>>> I'd really not bother with those two drivers, unless we start seing
> >>>>>> crashes. For MDP4 platforms we don't implement power domains at all, no
> >>>>>> interconnects, etc., which means that the system will never go to a
> >>>>>> lower power state.
> >>>>>
> >>>>> Right. Might be that 2030-something arrives and armv7 is gone before someone
> >>>>> randomly decides to work on that!
> >>>>>
> >>>>>> MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> >>>>>> SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> >>>>>> has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> >>>>>> should start there by adding missing bits adding corresponding
> >>>>>> dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
> >>>>>
> >>>>> A bit off-topic, but:
> >>>>>
> >>>>> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> >>>>>
> >>>>> 96 is in DPU. any other candidates that should be moved over?
> >>>>
> >>>> Let's go through them.
> >>>>
> >>>> All SoC except those currently supported in DPU require SMP (shared
> >>>> memory pool) support to be ported from the MDP5 driver.
> >>>>
> >>>> Most of the remaining platforms (except MSM8994/92) also had HW cursor
> >>>> implemented in a fancy way, in the LM rather than in a separate pipe.
> >>>> I'd really like to postpone those, possibly first completing migration
> >>>> of the other platforms and dropping support for them from MDP5.
> >>>>
> >>>> 1.0  - old MSM8974
> >>>>        I'd rather not touch it, it had bugs and I don't have HW
> >>>> 1.1  - MSM8x26
> >>>>        Probably Luca can better comment on it. Should be doable, but I
> >>>>        don't see upstream devices using display on it.
> >>>
> >>> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
> >>> devices working with MDP5 fine. I should've probably upstreamed panel
> >>> driver & dts but they haven't been high priority..
> >>
> >> I think I have been saying this: having a panel & dsi enabled in DT is
> >> the only way for me to know that the display is working on this
> >> platform. I'd really kindly ask you and other activists to get at least
> >> some panel drivers and corresponding DT bits upstream. It is really an
> >> important flag for me.
> >>
> >> I can propose some kind of bounty for those getting MDSS+panel supported
> >> in Qcom DT. (Beer at the next conf we meet? Some stickers for the
> >> laptops and phones? Mämmi?)
> > 
> > Hm I realized yesterday (through trying it), that also MDSS is broken
> > since the no-IOMMU codepath was removed from drm/msm. I thought this was
> > only affecting GPU but it's also affecting MDSS/MDP5 :(
> > 
> > So I guess no panel enablement anytime soon until the IOMMU situation is
> > sorted out, for both 8226 and 8974...
> 
> If you're sure the panel drivers are good (e.g. they worked on 6.whatever
> prior to drm/msm saying sayonara to carveout), I think no one would object
> to have them merged separately, so that you don't have to wait until
> who-knows-when and keep rebasing them by hand

+1. Please submit them and cc me so that I don't miss those.

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Luca Weiss]

On Tue Mar 17, 2026 at 4:16 PM CET, Dmitry Baryshkov wrote:
> On Tue, Mar 17, 2026 at 09:59:28AM +0100, Konrad Dybcio wrote:
>> On 3/17/26 9:07 AM, Luca Weiss wrote:
>> > On Mon Mar 2, 2026 at 4:17 PM CET, Dmitry Baryshkov wrote:
>> >> On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
>> >>> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
>> >>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
>> >>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
>> >>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
>> >>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
>> >>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
>> >>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
>> >>>>>>>>>>
>> >>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
>> >>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
>> >>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
>> >>>>>
>> >>>>> [...]
>> >>>>>
>> >>>>>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
>> >>>>>> dpu_runtime_suspend, then we should be able to also skip setting OPP
>> >>>>>> corner in dpu_runtime_resume(), because the previously set corner should
>> >>>>>> be viable until drm/msm driver commits new state / new modes.
>> >>>>>
>> >>>>> That matches my understanding.
>> >>>>>
>> >>>>>> The only important issue is to set the corner before starting up the
>> >>>>>> DPU, where we already have code to set MDP_CLK to the max frequency.
>> >>>>>>
>> >>>>>> Which means, we only need to drop the dev_pm_set_rate call from the
>> >>>>>> dpu_runtime_suspend().
>> >>>>>
>> >>>>> I concur.
>> >>>>>
>> >>>>>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
>> >>>>>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
>> >>>>>>
>> >>>>>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
>> >>>>>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
>> >>>>>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
>> >>>>>> resulted in a zero reads or in a crash. At the same time it needs to be
>> >>>>>> enabled to any rate, which means that for most of the operations
>> >>>>>> msm_mdss.c can rely on DPU keeping the clock up and running.
>> >>>>>>
>> >>>>>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
>> >>>>>>> MDSS_AHB one seems to have 3 actually configurable performance points
>> >>>>>>> that neither we nor seemingly the downstream driver seem to really care
>> >>>>>>> about (i.e. both just treat it as on/off). If we need to scale it, we
>> >>>>>>> should add an OPP table, if we don't, we should at least add required-opps.
>> >>>>>>
>> >>>>>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
>> >>>>>> these needs.
>> >>>>>
>> >>>>> I have slightly mixed feelings, but I suppose that as we accepted Commit
>> >>>>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
>> >>>>> we can generally agree that it makes sense that calling genpd->on() actually
>> >>>>> turns on the power indeed
>> >>>>>
>> >>>>> What I'm worried about is if the clock is pre-configured to run at a high
>> >>>>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
>> >>>>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
>> >>>>>
>> >>>>> This rings the "downstream really did it better with putting clock dvfs states
>> >>>>> into the clk driver" bell, but I suppose the way to fight this would be to
>> >>>>> simply set_rate(fmax) there too..
>> >>>>>
>> >>>>> I attempted an experiment with pulling out the plug. MMCX enabled with the
>> >>>>> AHB clock off results in a read-as-zero. I tried really hard to disable the
>> >>>>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
>> >>>>> *really* can't just disable it" type behavior (verified with debugcc)
>> >>>>
>> >>>> I think, in 8996 it was possible to disable it. Not sure about
>> >>>> 8998/630/660.
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>> There's a possible race condition if we don't do it:
>> >>>>>
>> >>>>> ------- bootloader --------
>> >>>>> configure display, mdp_clk=turbo
>> >>>>> ------- linux -------------
>> >>>>> load rpmhpd     |
>> >>>>> load venus      |
>> >>>>> set mmcx=lowsvs | mdp_clk is @ turbo
>> >>>>>                 | brownout
>> >>>>>                 | 
>> >>>>>                 | <mdss would only probe here>
>> >>>>>
>> >>>>> *but* that should be made impossible because of .sync_state().
>> >>>>
>> >>>> Yep, sync_state should prevent MMCX or CX from dropping under the boot
>> >>>> level.
>> >>>>
>> >>>>>
>> >>>>> This may impact hacky setups like simplefb, but as the name implies,
>> >>>>> that's hacky.
>> >>>>>
>> >>>>> Relying on .sync_state() however will not cover the case if the mdss
>> >>>>> module is removed and re-inserted later, possibly with mmcx disabled
>> >>>>> entirely but the clock not parked at a sufficiently low rate.
>> >>>>>
>> >>>>>
>> >>>>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
>> >>>>> plug the MDP opp table into it and set_rate(fmax) during mdss init
>> >>>>
>> >>>> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
>> >>>> level even though DPU will change the clock freq.
>> >>>>
>> >>>>>
>> >>>>>>> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
>> >>>>>>> targets supported by these may not even have OPP tables and are generally
>> >>>>>>> not super high priority for spending time on.. that can, I'd kick down the
>> >>>>>>> road unless someone really wants to step up
>> >>>>>>
>> >>>>>> I'd really not bother with those two drivers, unless we start seing
>> >>>>>> crashes. For MDP4 platforms we don't implement power domains at all, no
>> >>>>>> interconnects, etc., which means that the system will never go to a
>> >>>>>> lower power state.
>> >>>>>
>> >>>>> Right. Might be that 2030-something arrives and armv7 is gone before someone
>> >>>>> randomly decides to work on that!
>> >>>>>
>> >>>>>> MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
>> >>>>>> SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
>> >>>>>> has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
>> >>>>>> should start there by adding missing bits adding corresponding
>> >>>>>> dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
>> >>>>>
>> >>>>> A bit off-topic, but:
>> >>>>>
>> >>>>> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>> >>>>>
>> >>>>> 96 is in DPU. any other candidates that should be moved over?
>> >>>>
>> >>>> Let's go through them.
>> >>>>
>> >>>> All SoC except those currently supported in DPU require SMP (shared
>> >>>> memory pool) support to be ported from the MDP5 driver.
>> >>>>
>> >>>> Most of the remaining platforms (except MSM8994/92) also had HW cursor
>> >>>> implemented in a fancy way, in the LM rather than in a separate pipe.
>> >>>> I'd really like to postpone those, possibly first completing migration
>> >>>> of the other platforms and dropping support for them from MDP5.
>> >>>>
>> >>>> 1.0  - old MSM8974
>> >>>>        I'd rather not touch it, it had bugs and I don't have HW
>> >>>> 1.1  - MSM8x26
>> >>>>        Probably Luca can better comment on it. Should be doable, but I
>> >>>>        don't see upstream devices using display on it.
>> >>>
>> >>> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
>> >>> devices working with MDP5 fine. I should've probably upstreamed panel
>> >>> driver & dts but they haven't been high priority..
>> >>
>> >> I think I have been saying this: having a panel & dsi enabled in DT is
>> >> the only way for me to know that the display is working on this
>> >> platform. I'd really kindly ask you and other activists to get at least
>> >> some panel drivers and corresponding DT bits upstream. It is really an
>> >> important flag for me.
>> >>
>> >> I can propose some kind of bounty for those getting MDSS+panel supported
>> >> in Qcom DT. (Beer at the next conf we meet? Some stickers for the
>> >> laptops and phones? Mämmi?)
>> > 
>> > Hm I realized yesterday (through trying it), that also MDSS is broken
>> > since the no-IOMMU codepath was removed from drm/msm. I thought this was
>> > only affecting GPU but it's also affecting MDSS/MDP5 :(
>> > 
>> > So I guess no panel enablement anytime soon until the IOMMU situation is
>> > sorted out, for both 8226 and 8974...
>> 
>> If you're sure the panel drivers are good (e.g. they worked on 6.whatever
>> prior to drm/msm saying sayonara to carveout), I think no one would object
>> to have them merged separately, so that you don't have to wait until
>> who-knows-when and keep rebasing them by hand
>
> +1. Please submit them and cc me so that I don't miss those.

Including dts?

The drivers themselves are bog-standard, I've generated them with
linux-mdss-dsi-panel-driver-generator like it's done for most qcom
phones/devices.

But I can work on it.

Regards
Luca

Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Tue, Mar 17, 2026 at 04:18:13PM +0100, Luca Weiss wrote:
> On Tue Mar 17, 2026 at 4:16 PM CET, Dmitry Baryshkov wrote:
> > On Tue, Mar 17, 2026 at 09:59:28AM +0100, Konrad Dybcio wrote:
> >> On 3/17/26 9:07 AM, Luca Weiss wrote:
> >> > On Mon Mar 2, 2026 at 4:17 PM CET, Dmitry Baryshkov wrote:
> >> >> On Mon, Mar 02, 2026 at 03:35:36PM +0100, Luca Weiss wrote:
> >> >>> On Fri Feb 27, 2026 at 8:05 PM CET, Dmitry Baryshkov wrote:
> >> >>>> On Fri, Feb 27, 2026 at 12:34:04PM +0100, Konrad Dybcio wrote:
> >> >>>>> On 2/27/26 4:48 AM, Dmitry Baryshkov wrote:
> >> >>>>>> On Thu, Feb 26, 2026 at 02:35:52PM +0100, Konrad Dybcio wrote:
> >> >>>>>>> On 1/12/26 9:25 AM, yuanjiey wrote:
> >> >>>>>>>> On Mon, Jan 12, 2026 at 09:38:41AM +0200, Dmitry Baryshkov wrote:
> >> >>>>>>>>> On Mon, 12 Jan 2026 at 08:23, yuanjiey <yuanjie.yang@oss.qualcomm.com> wrote:
> >> >>>>>>>>>>
> >> >>>>>>>>>> On Fri, Jan 09, 2026 at 05:22:37PM +0200, Dmitry Baryshkov wrote:
> >> >>>>>>>>>>> On Fri, Jan 09, 2026 at 04:38:07PM +0800, yuanjie yang wrote:
> >> >>>>>>>>>>>> From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> >> >>>>>
> >> >>>>> [...]
> >> >>>>>
> >> >>>>>> Please correct me if I'm wrong, if we drop dev_pm_opp_set() from
> >> >>>>>> dpu_runtime_suspend, then we should be able to also skip setting OPP
> >> >>>>>> corner in dpu_runtime_resume(), because the previously set corner should
> >> >>>>>> be viable until drm/msm driver commits new state / new modes.
> >> >>>>>
> >> >>>>> That matches my understanding.
> >> >>>>>
> >> >>>>>> The only important issue is to set the corner before starting up the
> >> >>>>>> DPU, where we already have code to set MDP_CLK to the max frequency.
> >> >>>>>>
> >> >>>>>> Which means, we only need to drop the dev_pm_set_rate call from the
> >> >>>>>> dpu_runtime_suspend().
> >> >>>>>
> >> >>>>> I concur.
> >> >>>>>
> >> >>>>>>> For MDSS, we're currently generally describing the MDSS_AHB clock, the
> >> >>>>>>> GCC_AHB clock and the MDP clock (sounds wrong?) - there's not even an OPP
> >> >>>>>>
> >> >>>>>> No. As far as I remember, MDP_CLK is necessary to access MDSS registers
> >> >>>>>> (see commit d2570ee67a47 ("drm/msm/mdss: generate MDSS data for MDP5
> >> >>>>>> platforms")), I don't remember if accessing HW_REV without MDP_CLK
> >> >>>>>> resulted in a zero reads or in a crash. At the same time it needs to be
> >> >>>>>> enabled to any rate, which means that for most of the operations
> >> >>>>>> msm_mdss.c can rely on DPU keeping the clock up and running.
> >> >>>>>>
> >> >>>>>>> table.. The GCC clock is sourced from (and scaled by) the NoC, but the
> >> >>>>>>> MDSS_AHB one seems to have 3 actually configurable performance points
> >> >>>>>>> that neither we nor seemingly the downstream driver seem to really care
> >> >>>>>>> about (i.e. both just treat it as on/off). If we need to scale it, we
> >> >>>>>>> should add an OPP table, if we don't, we should at least add required-opps.
> >> >>>>>>
> >> >>>>>> I think, dispcc already has a minimal vote on the MMCX, which fulfill
> >> >>>>>> these needs.
> >> >>>>>
> >> >>>>> I have slightly mixed feelings, but I suppose that as we accepted Commit
> >> >>>>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the domain"),
> >> >>>>> we can generally agree that it makes sense that calling genpd->on() actually
> >> >>>>> turns on the power indeed
> >> >>>>>
> >> >>>>> What I'm worried about is if the clock is pre-configured to run at a high
> >> >>>>> frequency from the bootloader (prepare_enable only sets the EN bit in the RCG,
> >> >>>>> and doesn't impact the state of M/N/D at a glance), we may get a brownout
> >> >>>>>
> >> >>>>> This rings the "downstream really did it better with putting clock dvfs states
> >> >>>>> into the clk driver" bell, but I suppose the way to fight this would be to
> >> >>>>> simply set_rate(fmax) there too..
> >> >>>>>
> >> >>>>> I attempted an experiment with pulling out the plug. MMCX enabled with the
> >> >>>>> AHB clock off results in a read-as-zero. I tried really hard to disable the
> >> >>>>> mdp clock, but it seems like the "shared_ops" reflect some sort of "you
> >> >>>>> *really* can't just disable it" type behavior (verified with debugcc)
> >> >>>>
> >> >>>> I think, in 8996 it was possible to disable it. Not sure about
> >> >>>> 8998/630/660.
> >> >>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> There's a possible race condition if we don't do it:
> >> >>>>>
> >> >>>>> ------- bootloader --------
> >> >>>>> configure display, mdp_clk=turbo
> >> >>>>> ------- linux -------------
> >> >>>>> load rpmhpd     |
> >> >>>>> load venus      |
> >> >>>>> set mmcx=lowsvs | mdp_clk is @ turbo
> >> >>>>>                 | brownout
> >> >>>>>                 | 
> >> >>>>>                 | <mdss would only probe here>
> >> >>>>>
> >> >>>>> *but* that should be made impossible because of .sync_state().
> >> >>>>
> >> >>>> Yep, sync_state should prevent MMCX or CX from dropping under the boot
> >> >>>> level.
> >> >>>>
> >> >>>>>
> >> >>>>> This may impact hacky setups like simplefb, but as the name implies,
> >> >>>>> that's hacky.
> >> >>>>>
> >> >>>>> Relying on .sync_state() however will not cover the case if the mdss
> >> >>>>> module is removed and re-inserted later, possibly with mmcx disabled
> >> >>>>> entirely but the clock not parked at a sufficiently low rate.
> >> >>>>>
> >> >>>>>
> >> >>>>> TLDR: reassess whether MDSS needs the MDP clock, if so, we should just
> >> >>>>> plug the MDP opp table into it and set_rate(fmax) during mdss init
> >> >>>>
> >> >>>> And what will drop it afterwards? MDSS will still vote on the MMCX / CX
> >> >>>> level even though DPU will change the clock freq.
> >> >>>>
> >> >>>>>
> >> >>>>>>> The MDP4/MDP5 drivers are probably wrong too in this regard, but many
> >> >>>>>>> targets supported by these may not even have OPP tables and are generally
> >> >>>>>>> not super high priority for spending time on.. that can, I'd kick down the
> >> >>>>>>> road unless someone really wants to step up
> >> >>>>>>
> >> >>>>>> I'd really not bother with those two drivers, unless we start seing
> >> >>>>>> crashes. For MDP4 platforms we don't implement power domains at all, no
> >> >>>>>> interconnects, etc., which means that the system will never go to a
> >> >>>>>> lower power state.
> >> >>>>>
> >> >>>>> Right. Might be that 2030-something arrives and armv7 is gone before someone
> >> >>>>> randomly decides to work on that!
> >> >>>>>
> >> >>>>>> MDP5 platforms mostly don't have OPP tables. (I'm not counting MSM8998 /
> >> >>>>>> SDM630 / SDM660 here). MSM8917 / MSM8937 have only DSI tables, MSM8976
> >> >>>>>> has both MDP and DSI tables (my favourite MSM8996 has none). Probably we
> >> >>>>>> should start there by adding missing bits adding corresponding
> >> >>>>>> dev_pm_set_rate() calls as required (as demostrated by the DPU driver).
> >> >>>>>
> >> >>>>> A bit off-topic, but:
> >> >>>>>
> >> >>>>> drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> >> >>>>>
> >> >>>>> 96 is in DPU. any other candidates that should be moved over?
> >> >>>>
> >> >>>> Let's go through them.
> >> >>>>
> >> >>>> All SoC except those currently supported in DPU require SMP (shared
> >> >>>> memory pool) support to be ported from the MDP5 driver.
> >> >>>>
> >> >>>> Most of the remaining platforms (except MSM8994/92) also had HW cursor
> >> >>>> implemented in a fancy way, in the LM rather than in a separate pipe.
> >> >>>> I'd really like to postpone those, possibly first completing migration
> >> >>>> of the other platforms and dropping support for them from MDP5.
> >> >>>>
> >> >>>> 1.0  - old MSM8974
> >> >>>>        I'd rather not touch it, it had bugs and I don't have HW
> >> >>>> 1.1  - MSM8x26
> >> >>>>        Probably Luca can better comment on it. Should be doable, but I
> >> >>>>        don't see upstream devices using display on it.
> >> >>>
> >> >>> I have at least two MSM8x26 (1x APQ8026 lg-lenok & 1x MSM8926 htc-memul)
> >> >>> devices working with MDP5 fine. I should've probably upstreamed panel
> >> >>> driver & dts but they haven't been high priority..
> >> >>
> >> >> I think I have been saying this: having a panel & dsi enabled in DT is
> >> >> the only way for me to know that the display is working on this
> >> >> platform. I'd really kindly ask you and other activists to get at least
> >> >> some panel drivers and corresponding DT bits upstream. It is really an
> >> >> important flag for me.
> >> >>
> >> >> I can propose some kind of bounty for those getting MDSS+panel supported
> >> >> in Qcom DT. (Beer at the next conf we meet? Some stickers for the
> >> >> laptops and phones? Mämmi?)
> >> > 
> >> > Hm I realized yesterday (through trying it), that also MDSS is broken
> >> > since the no-IOMMU codepath was removed from drm/msm. I thought this was
> >> > only affecting GPU but it's also affecting MDSS/MDP5 :(
> >> > 
> >> > So I guess no panel enablement anytime soon until the IOMMU situation is
> >> > sorted out, for both 8226 and 8974...
> >> 
> >> If you're sure the panel drivers are good (e.g. they worked on 6.whatever
> >> prior to drm/msm saying sayonara to carveout), I think no one would object
> >> to have them merged separately, so that you don't have to wait until
> >> who-knows-when and keep rebasing them by hand
> >
> > +1. Please submit them and cc me so that I don't miss those.
> 
> Including dts?

Up to you.

> The drivers themselves are bog-standard, I've generated them with
> linux-mdss-dsi-panel-driver-generator like it's done for most qcom
> phones/devices.

Getting panel drivers in would remove at least two patches from your
tree, would also lower the technical debt, etc.

From my PoV, I'm perfectly fine with submitting the panel drivers which
worked with the upstream kernels right before breaking the !IOMMU case
for MDP5 or which work currently with the relevant patchset being
reverted (although I assume it becomes more and more difficult).

Yes, it doesn't match our typical 'please test with the latest kernel'
mantra, but I think it is a good enough testing case (unless any of DRM
maintainers would tell me that I'm wrong here).

> 
> But I can work on it.

That would be appreciated. As we discussed at FOSDEM, I will attempt
taking a look at the IOMMU for those platforms once I have time
(currently I have about two major tasks and several smaller issues
before I can start looking at it, but it is in the queue).

-- 
With best wishes
Dmitry

Re: [PATCH 0/2] drm/msm: fix mismatch between power and frequency

[Dmitry Baryshkov]

On Fri, 09 Jan 2026 16:38:06 +0800, yuanjie yang wrote:
> 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.

Applied to msm-next, thanks!

[1/2] drm/msm/dpu: fix mismatch between power and frequency
      https://gitlab.freedesktop.org/lumag/msm/-/commit/bc1dccc518cc

Best regards,
-- 
With best wishes
Dmitry