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

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

On Tue, Sep 23, 2025 at 06:40:36PM +0530, Ankit Nautiyal wrote:
> 'Set context latency' (SCL, 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, formally introduce the
> 'set context latency' or SCL and track it as a new member
> `set_context_latency` of the structure 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.
> 
> Note: for TGL the TRANS_SET_CONTEXT_LATENCY doesn't exist to account for
> the SCL. However, the VBLANK_START-VACTIVE diffence plays an identical role
> here ie. it can be used to create the SCL window ahead of the undelayed
> vblank.
> 
> While readback there is no specific register to read out the SCL, so use
> the difference between vblank start and vactive to populate the new
> member for TGL.
> 
> v2:
> - Use u16 for set_context_latency. (Ville)
> - s/vblank_delay/set_context_latency. (Ville)
> - Meld the changes for TGL with this change. (Ville)
> 
> 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  | 53 ++++++++++++-------
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  3 files changed, 41 insertions(+), 20 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..aceafe4478d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2361,39 +2361,44 @@ 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,
> -					   struct intel_crtc *crtc)
> +static int intel_crtc_compute_set_context_latency(struct intel_atomic_state *state,
> +						  struct intel_crtc *crtc)
>  {
>  	struct intel_display *display = to_intel_display(state);
>  	struct intel_crtc_state *crtc_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 set_context_latency, max_vblank_delay;
> +
> +	set_context_latency = intel_crtc_set_context_latency(crtc_state);
>  
> -	vblank_delay = intel_crtc_vblank_delay(crtc_state);
>  	max_vblank_delay = adjusted_mode->crtc_vblank_end - adjusted_mode->crtc_vblank_start - 1;
>  
> -	if (vblank_delay > max_vblank_delay) {
> -		drm_dbg_kms(display->drm, "[CRTC:%d:%s] vblank delay (%d) exceeds max (%d)\n",
> -			    crtc->base.base.id, crtc->base.name, vblank_delay, max_vblank_delay);
> +	if (set_context_latency > max_vblank_delay) {
> +		drm_dbg_kms(display->drm, "[CRTC:%d:%s] set context latency (%d) exceeds max (%d)\n",
> +			    crtc->base.base.id, crtc->base.name,
> +			    set_context_latency,
> +			    max_vblank_delay);
>  		return -EINVAL;
>  	}
>  
> -	adjusted_mode->crtc_vblank_start += vblank_delay;
> +	crtc_state->set_context_latency = set_context_latency;
> +	adjusted_mode->crtc_vblank_start += set_context_latency;
>  
>  	return 0;
>  }
> @@ -2405,7 +2410,7 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	int ret;
>  
> -	ret = intel_crtc_compute_vblank_delay(state, crtc);
> +	ret = intel_crtc_compute_set_context_latency(state, crtc);
>  	if (ret)
>  		return ret;
>  
> @@ -2617,7 +2622,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 +2712,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 +2825,21 @@ 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;
> +	} else if (DISPLAY_VER(display) == 12) {
> +		/*
> +		 * There is no specific register for SCL for TGL.
> +		 * Derive the value from the difference between Vblank start and Vactive.
> +		 */

Importantly it is the *hardware* that derives it from those register
values. I think we should state that explicitly so that people don't
the wrong impression that this is just some random software thing.

Apart from that:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		pipe_config->set_context_latency =
> +			adjusted_mode->crtc_vblank_start - adjusted_mode->crtc_vdisplay;
> +	}
>  
>  	if (DISPLAY_VER(display) >= 30)
>  		pipe_config->min_hblank = intel_de_read(display,
> @@ -5387,6 +5402,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..029c47743f8b 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 */
> +	u16 set_context_latency;
>  };
>  
>  enum intel_pipe_crc_source {
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel