Re: [PATCH 2/9] drm/i915/display: Add set_context_latency to crtc_state->vrr

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

On Sun, Sep 21, 2025 at 10:05:28AM +0530, Ankit Nautiyal wrote:
> 'Set context latency' (Window W2) is defined as the number of lines before
> the double buffering point, which are required to complete programming of
> the registers, typically when DSB is used to program the display
> registers.
> 
> Since we are not using this window for programming the registers, this
> is mostly set to 0, unless there is a requirement for few cases related
> to PSR/PR where the 'set context latency' should be at least 1.
> 
> Currently we are using the 'set context latency' (if required) implicitly
> by moving the vblank start by the required amount and then measuring the
> delay i.e. the difference between undelayed vblank start and delayed vblank
> start.
> 
> Since our guardband matches the vblank length, this was not a problem as
> the difference between the undelayed vblank and delayed vblank was at
> the most equal to the 'set context latency' lines.
> 
> However, if we want to optimize the guardband, the difference between the
> undelayed and the delayed vblank will be large and we cannot use this
> difference as the 'set context latency' lines.
> 
> To make way for this optimization of guardband, we formally introduce
> the 'set context latency' and track it as a new member
> `set_context_latency` of the intel_crtc_state.
> 
> Eventually, all references of vblank delay where we mean to use set
> context latency will be replaced by this new `set_context_latency`
> member.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_crtc_state_dump.c  |  5 ++--
>  drivers/gpu/drm/i915/display/intel_display.c  | 29 ++++++++++++-------
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  3 files changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 0c7f91046996..a14bcda4446c 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -289,10 +289,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>  	drm_printf(&p, "scanline offset: %d\n",
>  		   intel_crtc_scanline_offset(pipe_config));
>  
> -	drm_printf(&p, "vblank delay: %d, framestart delay: %d, MSA timing delay: %d\n",
> +	drm_printf(&p, "vblank delay: %d, framestart delay: %d, MSA timing delay: %d set context latency: %d\n",
>  		   pipe_config->hw.adjusted_mode.crtc_vblank_start -
>  		   pipe_config->hw.adjusted_mode.crtc_vdisplay,
> -		   pipe_config->framestart_delay, pipe_config->msa_timing_delay);
> +		   pipe_config->framestart_delay, pipe_config->msa_timing_delay,
> +		   pipe_config->set_context_latency);
>  
>  	drm_printf(&p, "vrr: %s, fixed rr: %s, vmin: %d, vmax: %d, flipline: %d, pipeline full: %d, guardband: %d vsync start: %d, vsync end: %d\n",
>  		   str_yes_no(pipe_config->vrr.enable),
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 679c2a9baaee..5a7794387ea2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2361,17 +2361,18 @@ static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> -static int intel_crtc_vblank_delay(const struct intel_crtc_state *crtc_state)
> +static int intel_crtc_set_context_latency(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
> -	int vblank_delay = 0;
> +	int set_context_latency = 0;
>  
>  	if (!HAS_DSB(display))
>  		return 0;
>  
> -	vblank_delay = max(vblank_delay, intel_psr_min_set_context_latency(crtc_state));
> +	set_context_latency = max(set_context_latency,
> +				  intel_psr_min_set_context_latency(crtc_state));
>  
> -	return vblank_delay;
> +	return set_context_latency;
>  }
>  
>  static int intel_crtc_compute_vblank_delay(struct intel_atomic_state *state,

I think we should just do a full s/vblank_delay/set_context_latency/
pass over this function. Then it'll be clear that we are calculating
only the SCL here.

> @@ -2382,9 +2383,10 @@ static int intel_crtc_compute_vblank_delay(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> -	int vblank_delay, max_vblank_delay;
> +	int vblank_delay = 0, max_vblank_delay;
>  
> -	vblank_delay = intel_crtc_vblank_delay(crtc_state);
> +	crtc_state->set_context_latency = intel_crtc_set_context_latency(crtc_state);
> +	vblank_delay += crtc_state->set_context_latency;
>  	max_vblank_delay = adjusted_mode->crtc_vblank_end - adjusted_mode->crtc_vblank_start - 1;
>  
>  	if (vblank_delay > max_vblank_delay) {
> @@ -2617,7 +2619,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	if (DISPLAY_VER(display) >= 13) {
>  		intel_de_write(display,
>  			       TRANS_SET_CONTEXT_LATENCY(display, cpu_transcoder),
> -			       crtc_vblank_start - crtc_vdisplay);
> +			       crtc_state->set_context_latency);
>  
>  		/*
>  		 * VBLANK_START not used by hw, just clear it
> @@ -2707,7 +2709,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
>  	if (DISPLAY_VER(display) >= 13) {
>  		intel_de_write(display,
>  			       TRANS_SET_CONTEXT_LATENCY(display, cpu_transcoder),
> -			       crtc_vblank_start - crtc_vdisplay);
> +			       crtc_state->set_context_latency);
>  
>  		/*
>  		 * VBLANK_START not used by hw, just clear it
> @@ -2820,11 +2822,14 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc,
>  		adjusted_mode->crtc_vblank_end += 1;
>  	}
>  
> -	if (DISPLAY_VER(display) >= 13 && !transcoder_is_dsi(cpu_transcoder))
> -		adjusted_mode->crtc_vblank_start =
> -			adjusted_mode->crtc_vdisplay +
> +	if (DISPLAY_VER(display) >= 13 && !transcoder_is_dsi(cpu_transcoder)) {
> +		pipe_config->set_context_latency =
>  			intel_de_read(display,
>  				      TRANS_SET_CONTEXT_LATENCY(display, cpu_transcoder));
> +		adjusted_mode->crtc_vblank_start =
> +			adjusted_mode->crtc_vdisplay +
> +			pipe_config->set_context_latency;
> +	}
>  
>  	if (DISPLAY_VER(display) >= 30)
>  		pipe_config->min_hblank = intel_de_read(display,
> @@ -5387,6 +5392,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  		PIPE_CONF_CHECK_I(vrr.guardband);
>  	}
>  
> +	PIPE_CONF_CHECK_I(set_context_latency);
> +
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_LLI
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 358ab922d7a7..a22fe77fcca1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1341,6 +1341,9 @@ struct intel_crtc_state {
>  
>  	/* LOBF flag */
>  	bool has_lobf;
> +
> +	/* W2 window or 'set context latency' lines */
> +	int set_context_latency;

This can be a u16. If there's a reasonable looking place with a
suitable hole I'd try to stick it there.

>  };
>  
>  enum intel_pipe_crc_source {
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel