Re: [PATCH v2 01/14] drm/i915: Drop the cached per-pipe min_cdclk[] from bw state

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 26 Mar 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_bw_crtc_min_cdclk() only depends on the pipe data rate,
> which we already have stashed in bw_state->data_rate[]. So
> stashing the resulting min_cdclk[] as well is redundant. Get
> rid of it.

Yay, always a fan of removing intermediate state.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 17 ++++++++---------
>  drivers/gpu/drm/i915/display/intel_bw.h |  1 -
>  2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index bb81efec08a0..15c2377193f7 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -825,14 +825,13 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
>  }
>  
>  /* "Maximum Pipe Read Bandwidth" */
> -static int intel_bw_crtc_min_cdclk(const struct intel_crtc_state *crtc_state)
> +static int intel_bw_crtc_min_cdclk(struct intel_display *display,
> +				   unsigned int data_rate)
>  {
> -	struct intel_display *display = to_intel_display(crtc_state);
> -
>  	if (DISPLAY_VER(display) < 12)
>  		return 0;
>  
> -	return DIV_ROUND_UP_ULL(mul_u32_u32(intel_bw_crtc_data_rate(crtc_state), 10), 512);
> +	return DIV_ROUND_UP_ULL(mul_u32_u32(data_rate, 10), 512);
>  }
>  
>  static unsigned int intel_bw_num_active_planes(struct intel_display *display,
> @@ -1170,7 +1169,8 @@ static bool intel_bw_state_changed(struct intel_display *display,
>  				return true;
>  		}
>  
> -		if (old_bw_state->min_cdclk[pipe] != new_bw_state->min_cdclk[pipe])
> +		if (intel_bw_crtc_min_cdclk(display, old_bw_state->data_rate[pipe]) !=
> +		    intel_bw_crtc_min_cdclk(display, new_bw_state->data_rate[pipe]))
>  			return true;
>  	}
>  
> @@ -1271,7 +1271,9 @@ int intel_bw_min_cdclk(struct intel_display *display,
>  	min_cdclk = intel_bw_dbuf_min_cdclk(display, bw_state);
>  
>  	for_each_pipe(display, pipe)
> -		min_cdclk = max(min_cdclk, bw_state->min_cdclk[pipe]);
> +		min_cdclk = max(min_cdclk,
> +				intel_bw_crtc_min_cdclk(display,
> +							bw_state->data_rate[pipe]));
>  
>  	return min_cdclk;
>  }
> @@ -1299,9 +1301,6 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state,
>  		old_bw_state = intel_atomic_get_old_bw_state(state);
>  
>  		skl_crtc_calc_dbuf_bw(new_bw_state, crtc_state);
> -
> -		new_bw_state->min_cdclk[crtc->pipe] =
> -			intel_bw_crtc_min_cdclk(crtc_state);
>  	}
>  
>  	if (!old_bw_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index c18126c83d2e..3e4397c85774 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -54,7 +54,6 @@ struct intel_bw_state {
>  	 */
>  	bool force_check_qgv;
>  
> -	int min_cdclk[I915_MAX_PIPES];
>  	unsigned int data_rate[I915_MAX_PIPES];
>  	u8 num_active_planes[I915_MAX_PIPES];
>  };

-- 
Jani Nikula, Intel