Re: [PATCH 5/9] drm/i915/vrr: s/intel_vrr_vblank_delay/intel_vrr_scl_delay

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

On Tue, Sep 23, 2025 at 06:40:39PM +0530, Ankit Nautiyal wrote:
> The helper intel_vrr_vblank_delay() is used to account for scl lines
> + extra_vblank_delay (for ICL/TGL case) for:
> - evasion logic for vrr case
> - to wait for SCL+ lines after send push operation.
> 
> Rename the helper to intel_vrr_scl_delay since we are interested in the
> SCL+ lines for the VRR cases.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c    | 4 ++--
>  drivers/gpu/drm/i915/display/intel_vblank.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_vrr.c    | 2 +-
>  drivers/gpu/drm/i915/display/intel_vrr.h    | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index dee44d45b668..ca31e928ecb0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -128,7 +128,7 @@ static int dsb_vblank_delay(struct intel_atomic_state *state,
>  		 * scanline until the delayed vblank occurs after
>  		 * TRANS_PUSH has been written.
>  		 */
> -		return intel_vrr_vblank_delay(crtc_state) + 1;
> +		return intel_vrr_scl_delay(crtc_state) + 1;

I'd skip this renaming for now. I think after you've added the
safe window scanline wait you can replace all of these with
crtc_state->set_context_latency.

>  	else
>  		return intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
>  }
> @@ -723,7 +723,7 @@ void intel_dsb_vblank_evade(struct intel_atomic_state *state,
>  		intel_dsb_emit_wait_dsl(dsb, DSB_OPCODE_WAIT_DSL_OUT, 0, 0);
>  
>  	if (pre_commit_is_vrr_active(state, crtc)) {
> -		int vblank_delay = intel_vrr_vblank_delay(crtc_state);
> +		int vblank_delay = intel_vrr_scl_delay(crtc_state);
>  
>  		end = intel_vrr_vmin_vblank_start(crtc_state);
>  		start = end - vblank_delay - latency;
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index c15234c1d96e..9441b7bacd27 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -681,7 +681,7 @@ void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
>  		else
>  			evade->vblank_start = intel_vrr_vmax_vblank_start(crtc_state);
>  
> -		vblank_delay = intel_vrr_vblank_delay(crtc_state);
> +		vblank_delay = intel_vrr_scl_delay(crtc_state);

I was pondering about this case especially, but I *think* it should
also be changed to crtc_state->set_context_latency. We don't want to
perform the commit while in the SCL here because then we're not in
the safe window and the DSB we use for LUT updates wouldn't start
until the next safe window starts (== next frame's vactive), whereas
the double buffered registers would latch already in the upcoming
delayed vblank.

But performing the commit while we're between undelayed vblank
and SCL start should be fine since that is part of the safe
window. So we don't need to evade the actual undelayed vblank
when in VRR mode.

The only exception here would be the LRR and M/N cases since those
perhaps still need to evade the undlayed vblank proper. But we always
drop out of VRR mode for those types of updates so they won't be
taking this codepath anyway.

>  	} else {
>  		evade->vblank_start = intel_mode_vblank_start(adjusted_mode);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 1b90eaa6a776..40e256bce3cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -92,7 +92,7 @@ static int intel_vrr_extra_vblank_delay(struct intel_display *display)
>  	return DISPLAY_VER(display) < 13 ? 1 : 0;
>  }
>  
> -int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state)
> +int intel_vrr_scl_delay(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 38bf9996b883..b72e90b4abe5 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -35,7 +35,7 @@ int intel_vrr_vmax_vtotal(const struct intel_crtc_state *crtc_state);
>  int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state);
>  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
>  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
> -int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state);
> +int intel_vrr_scl_delay(const struct intel_crtc_state *crtc_state);
>  bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state);
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel