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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <kusxzlezvsuwcwwdtm7yqwnqea6gdeolkepxpx3estabaiqymo@edj7pgccli3y>
Date: Fri, 9 Jan 2026 17:22:37 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: yuanjie yang <yuanjie.yang@....qualcomm.com>
Cc: robin.clark@....qualcomm.com, lumag@...nel.org, abhinav.kumar@...ux.dev,
        jesszhan0024@...il.com, sean@...rly.run, marijn.suijten@...ainline.org,
        airlied@...il.com, simona@...ll.ch, krzysztof.kozlowski@...aro.org,
        konrad.dybcio@....qualcomm.com, linux-arm-msm@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, tingwei.zhang@....qualcomm.com,
        aiqun.yu@....qualcomm.com, yongxing.mou@....qualcomm.com
Subject: Re: [PATCH 1/2] drm/msm/dpu: fix mismatch between power and frequency

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