Re: [PATCH 03/15] drm/i915/skl_watermark: Pass linetime as argument to latency helpers

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 11, 2025 at 08:15:42AM +0530, Ankit Nautiyal wrote:
> Refactor dsc_prefill_latency and scaler_prefill_latency to take
> linetime as an explicit parameter instead of computing it internally.
> 
> This avoids redundant calculations and simplifies scanline conversion
> logic in skl_is_vblank_too_short().
> 
> This change also facilitates future extraction of these helpers for use
> cases where latencies are computed for an optimized guardband, based on the
> highest resolution mode, rather than the current mode.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 51548447f4aa..49d424f5b305 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2176,13 +2176,11 @@ cdclk_prefill_adjustment(const struct intel_crtc_state *crtc_state)
>  }
>  
>  static int
> -dsc_prefill_latency(const struct intel_crtc_state *crtc_state)
> +dsc_prefill_latency(const struct intel_crtc_state *crtc_state, int linetime)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct intel_crtc_scaler_state *scaler_state =
>  					&crtc_state->scaler_state;
> -	int linetime = DIV_ROUND_UP(1000 * crtc_state->hw.adjusted_mode.htotal,
> -				    crtc_state->hw.adjusted_mode.clock);
>  	int num_scaler_users = hweight32(scaler_state->scaler_users);
>  	int chroma_downscaling_factor =
>  		crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ? 4 : 1;
> @@ -2206,18 +2204,16 @@ dsc_prefill_latency(const struct intel_crtc_state *crtc_state)
>  
>  	dsc_prefill_latency *= cdclk_prefill_adjustment(crtc_state);
>  
> -	return intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, dsc_prefill_latency);
> +	return dsc_prefill_latency;
>  }
>  
>  static int
> -scaler_prefill_latency(const struct intel_crtc_state *crtc_state)
> +scaler_prefill_latency(const struct intel_crtc_state *crtc_state, int linetime)
>  {
>  	const struct intel_crtc_scaler_state *scaler_state =
>  					&crtc_state->scaler_state;
>  	int num_scaler_users = hweight32(scaler_state->scaler_users);
>  	int scaler_prefill_latency = 0;
> -	int linetime = DIV_ROUND_UP(1000 * crtc_state->hw.adjusted_mode.htotal,
> -				    crtc_state->hw.adjusted_mode.clock);
>  
>  	if (!num_scaler_users)
>  		return scaler_prefill_latency;
> @@ -2238,7 +2234,7 @@ scaler_prefill_latency(const struct intel_crtc_state *crtc_state)
>  
>  	scaler_prefill_latency *= cdclk_prefill_adjustment(crtc_state);
>  
> -	return intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, scaler_prefill_latency);
> +	return scaler_prefill_latency;
>  }
>  
>  static bool
> @@ -2247,11 +2243,13 @@ skl_is_vblank_too_short(const struct intel_crtc_state *crtc_state,
>  {
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> +	int linetime = DIV_ROUND_UP(1000 * adjusted_mode->htotal,
> +				    adjusted_mode->clock);
>  
>  	return crtc_state->framestart_delay +
>  		intel_usecs_to_scanlines(adjusted_mode, latency) +
> -		scaler_prefill_latency(crtc_state) +
> -		dsc_prefill_latency(crtc_state) +
> +		DIV_ROUND_UP(scaler_prefill_latency(crtc_state, linetime), linetime) +
> +		DIV_ROUND_UP(dsc_prefill_latency(crtc_state, linetime), linetime) +

Looks like you could just add up all the usec latency numbers and
convert the whole thing with a single intel_usecs_to_scanlines() call
instead of this mismash of hand rolled and non-hand rolled stuff.

>  		wm0_lines >
>  		adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vblank_start;
>  }
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel