Re: [PATCH] drm/msm/dpu: compute the CRTC bandwidth from the state being checked

[Dmitry Baryshkov]

On Tue, Sep 29, 2026 at 10:06:33PM +0900, Joonhoe Kim wrote:
> _dpu_core_perf_calc_bw() is called with the new CRTC state but sums
> plane_fetch_bw over drm_atomic_crtc_for_each_plane(), i.e. the planes
> of the committed state. When the CRTC is re-enabled (DPMS on, system
> resume) the committed state has no planes attached, so the check
> computes bw_ctl = 0 and the display runs without an average bandwidth
> vote on the MDP path until some later commit changes a plane -- which
> may not happen for a long time on a static screen such as a lock
> screen.
> 
> Seen on a Lenovo TB323FU (SM8850) through the interconnect and DPU
> tracepoints: after DPMS off/on or s2idle, dpu_perf_crtc_update reported
> bw_ctl=0 and qnm_mdp was left at avg_bw=0 (peak 800000) instead of the
> 3728793 kBps voted before, until the next mode change.
> 
> Iterate the plane states of the CRTC state being checked instead.
> drm_atomic_crtc_state_for_each_plane_state() falls back to the current
> plane state for planes that are not part of the commit, so the result
> is unchanged for commits that do touch the planes.
> 
> With this, bw_ctl is 3728793600 right after DPMS on and after resume.
> Only tested on this device.
> 
> Fixes: c33b7c0389e1 ("drm/msm/dpu: add support for clk and bw scaling for display")
> Assisted-by: LLM
> Signed-off-by: Joonhoe Kim <26rote@xxxxxxxxx>
> ---
> _dpu_core_perf_calc_clk() walks the planes the same way; it is not
> touched here since I have not seen a wrong clock vote from it.
> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 24 ++++++++++---------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index fea173e37464..343c41550637 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -54,24 +54,26 @@ u64 dpu_core_perf_adjusted_mode_clk(u64 mode_clk_rate,
>  /**
>   * _dpu_core_perf_calc_bw() - to calculate BW per crtc
>   * @perf_cfg: performance configuration
> - * @crtc: pointer to a crtc
> + * @state: the CRTC state
>   * Return: returns aggregated BW for all planes in crtc.
>   */
>  static u64 _dpu_core_perf_calc_bw(const struct dpu_perf_cfg *perf_cfg,
> -		struct drm_crtc *crtc)
> +		struct drm_crtc_state *state)
>  {
>  	struct drm_plane *plane;
> -	struct dpu_plane_state *pstate;
> +	const struct drm_plane_state *plane_state;
>  	u64 crtc_plane_bw = 0;
>  	u32 bw_factor;
>  
> -	drm_atomic_crtc_for_each_plane(plane, crtc) {
> -		pstate = to_dpu_plane_state(plane->state);
> -		if (!pstate)
> -			continue;
> -
> -		crtc_plane_bw += pstate->plane_fetch_bw;
> -	}
> +	/*
> +	 * The planes of the CRTC state being checked: iterating the committed
> +	 * state (drm_atomic_crtc_for_each_plane()) summed nothing when the CRTC
> +	 * was being re-enabled (DPMS on, resume), whose old state has no
> +	 * planes, and the display ran without a bandwidth vote until the next
> +	 * plane update.
> +	 */

This again duplicates commit message. Please drop it. The patch LGTM.

> +	drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, state)
> +		crtc_plane_bw += to_dpu_plane_state(plane_state)->plane_fetch_bw;
>  
>  	bw_factor = perf_cfg->bw_inefficiency_factor;
>  	if (bw_factor) {
> @@ -131,7 +133,7 @@ static void _dpu_core_perf_calc_crtc(const struct dpu_core_perf *core_perf,
>  		return;
>  	}
>  
> -	perf->bw_ctl = _dpu_core_perf_calc_bw(perf_cfg, crtc);
> +	perf->bw_ctl = _dpu_core_perf_calc_bw(perf_cfg, state);
>  	perf->max_per_pipe_ib = perf_cfg->min_dram_ib;
>  	perf->core_clk_rate = _dpu_core_perf_calc_clk(perf_cfg, crtc, state);
>  	DRM_DEBUG_ATOMIC(
> 
> base-commit: 6375e61c01e93e35ee7acd336a689ac1fae4b509
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry