Re: [PATCH 10/15] drm/i915/skl_watermark: Remove redundant latency checks from vblank validation

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

On Thu, Sep 11, 2025 at 08:15:49AM +0530, Ankit Nautiyal wrote:
> Drop DSC and scaler prefill latency checks from skl_is_vblank_too_short().
> These are now covered by the guardband validation added during the atomic
> CRTC check phase.
> 
> This cleanup prepares for future changes where the guardband will be
> optimized independently of vblank length, making vblank-based checks
> obsolete.

This looks very wrong, at least for platforms that don't have a
programmable guardband.

> 
> 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 | 79 --------------------
>  1 file changed, 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 83ac26004f05..07589096b143 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -28,7 +28,6 @@
>  #include "intel_flipq.h"
>  #include "intel_pcode.h"
>  #include "intel_plane.h"
> -#include "intel_vrr.h"
>  #include "intel_wm.h"
>  #include "skl_universal_plane_regs.h"
>  #include "skl_scaler.h"
> @@ -2159,93 +2158,15 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
>  	return 0;
>  }
>  
> -static int
> -cdclk_prefill_adjustment(const struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_display *display = to_intel_display(crtc_state);
> -	struct intel_atomic_state *state =
> -		to_intel_atomic_state(crtc_state->uapi.state);
> -	const struct intel_cdclk_state *cdclk_state;
> -
> -	cdclk_state = intel_atomic_get_cdclk_state(state);
> -	if (IS_ERR(cdclk_state)) {
> -		drm_WARN_ON(display->drm, PTR_ERR(cdclk_state));
> -		return 1;
> -	}
> -
> -	return min(1, DIV_ROUND_UP(crtc_state->pixel_rate,
> -				   2 * intel_cdclk_logical(cdclk_state)));
> -}
> -
> -static int
> -dsc_prefill_latency(const struct intel_crtc_state *crtc_state, int linetime)
> -{
> -	const struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
> -	int chroma_downscaling_factor = skl_scaler_chroma_downscale_factor(crtc_state);
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	int num_scaler_users = hweight32(scaler_state->scaler_users);
> -	u64 hscale_k[ARRAY_SIZE(scaler_state->scalers)];
> -	u64 vscale_k[ARRAY_SIZE(scaler_state->scalers)];
> -	u32 dsc_prefill_latency = 0;
> -
> -	if (!crtc_state->dsc.compression_enable ||
> -	    !num_scaler_users ||
> -	    num_scaler_users > crtc->num_scalers)
> -		return dsc_prefill_latency;
> -
> -	for (int i = 0; i < num_scaler_users; i++) {
> -		hscale_k[i] = max(1000, mul_u32_u32(scaler_state->scalers[i].hscale, 1000) >> 16);
> -		vscale_k[i] = max(1000, mul_u32_u32(scaler_state->scalers[i].vscale, 1000) >> 16);
> -	}
> -
> -	dsc_prefill_latency =
> -		intel_vrr_guardband_dsc_latency(num_scaler_users, hscale_k, vscale_k,
> -						chroma_downscaling_factor,
> -						cdclk_prefill_adjustment(crtc_state),
> -						linetime);
> -
> -	return dsc_prefill_latency;
> -}
> -
> -static int
> -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 chroma_downscaling_factor = skl_scaler_chroma_downscale_factor(crtc_state);
> -	int num_scaler_users = hweight32(scaler_state->scaler_users);
> -	u64 hscale_k = 0, vscale_k = 0;
> -	int scaler_prefill_latency = 0;
> -
> -	if (!num_scaler_users)
> -		return scaler_prefill_latency;
> -
> -	if (num_scaler_users > 1) {
> -		hscale_k = max(1000, mul_u32_u32(scaler_state->scalers[0].hscale, 1000) >> 16);
> -		vscale_k = max(1000, mul_u32_u32(scaler_state->scalers[0].vscale, 1000) >> 16);
> -	}
> -
> -	scaler_prefill_latency =
> -		intel_vrr_guardband_scaler_latency(num_scaler_users, hscale_k, vscale_k,
> -						   chroma_downscaling_factor,
> -						   cdclk_prefill_adjustment(crtc_state),
> -						   linetime);
> -
> -	return scaler_prefill_latency;
> -}
> -
>  static bool
>  skl_is_vblank_too_short(const struct intel_crtc_state *crtc_state,
>  			int wm0_lines, int latency)
>  {
>  	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) +
> -		DIV_ROUND_UP(scaler_prefill_latency(crtc_state, linetime), linetime) +
> -		DIV_ROUND_UP(dsc_prefill_latency(crtc_state, linetime), linetime) +
>  		wm0_lines >
>  		adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vblank_start;
>  }
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel