Re: [PATCH 1/6] drm/i915/display: Deprecate TRANS_VSYNC where VRR TG is always on

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

On Tue, May 12, 2026 at 07:02:44PM +0530, Ankit Nautiyal wrote:
> The VRR Timing generator does not use TRANS_VSYNC register, instead it
> use TRANS_VRR_VSYNC registers for both variable and fixed timings.
> 
> Avoid using TRANS_VSYNC registers for platforms that always use VRR
> timing generator. The crtc_vsync_{start, end} fields of the adjusted
> mode can still be filled with the Vsync start/end values, while readback
> these can be derived from TRANS_VRR_VSYNC. Since the TRANS_VRR_VSYNC
> register has vrr_vsync_{start,end} measured from the Vtotal, to get the
> crtc_vsync_{start, end} we need to subtract the vrr values from the
> Vtotal.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 19 +++++++++---
>  drivers/gpu/drm/i915/display/intel_vrr.c     | 31 ++++++++++++++------
>  2 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index d5cf1476c7b9..548a12aff88f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2653,6 +2653,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end;
> +	u32 crtc_vsync_start, crtc_vsync_end;
>  	int vsyncshift = 0;
>  
>  	drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder));
> @@ -2727,9 +2728,17 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
>  	intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
>  		       VBLANK_START(crtc_vblank_start - 1) |
>  		       VBLANK_END(crtc_vblank_end - 1));
> +	if (intel_vrr_always_use_vrr_tg(display)) {
> +		crtc_vsync_start = 1;
> +		crtc_vsync_end = 1;

At least the vsync interrupt still uses TRANS_VSYNC on LNL, even when
using the VRR timing generator. So I don't think we want to do this
until the hardware has really stopped using TRANS_VSYNC, which perhaps
means NVL+. I still need to check how PTL behaves...

> +	} else {
> +		crtc_vsync_start = adjusted_mode->crtc_vsync_start;
> +		crtc_vsync_end = adjusted_mode->crtc_vsync_end;
> +	}
> +
>  	intel_de_write(display, TRANS_VSYNC(display, cpu_transcoder),
> -		       VSYNC_START(adjusted_mode->crtc_vsync_start - 1) |
> -		       VSYNC_END(adjusted_mode->crtc_vsync_end - 1));
> +		       VSYNC_START(crtc_vsync_start - 1) |
> +		       VSYNC_END(crtc_vsync_end - 1));
>  
>  	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
>  	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
> @@ -5162,8 +5171,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	PIPE_CONF_CHECK_I(name.crtc_vdisplay); \
>  	if (!fastset || !allow_vblank_delay_fastset(current_config)) \
>  		PIPE_CONF_CHECK_I(name.crtc_vblank_start); \
> -	PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
> -	PIPE_CONF_CHECK_I(name.crtc_vsync_end); \
> +	if (!intel_vrr_always_use_vrr_tg(display)) { \
> +		PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
> +		PIPE_CONF_CHECK_I(name.crtc_vsync_end); \

IMO we should just handle these through the LRR codepath, just like
vtotal and vblank_end.

> +	} \
>  	if (!fastset || !pipe_config->update_lrr) { \
>  		PIPE_CONF_CHECK_I(name.crtc_vtotal); \
>  		PIPE_CONF_CHECK_I(name.crtc_vblank_end); \
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 1b09992ce9fd..24aa74475e64 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -1099,24 +1099,37 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>  			crtc_state->vrr.vmin += intel_vrr_vmin_flipline_offset(display);
>  		}
>  
> +		if (HAS_AS_SDP(display)) {
> +			trans_vrr_vsync =
> +				intel_de_read(display,
> +					      TRANS_VRR_VSYNC(display, cpu_transcoder));
> +			crtc_state->vrr.vsync_start =
> +				REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
> +			crtc_state->vrr.vsync_end =
> +				REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
> +		}
> +
>  		/*
>  		 * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
>  		 * bits are not filled. Since for these platforms TRAN_VMIN is always
>  		 * filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal for
>  		 * adjusted_mode.
> +		 *
> +		 * Similarly Vsync start/end are also not used when VRR TG is used.
> +		 * Use the TRANS_VRR_VSYNC to fill these. Since these are relative
> +		 * from the Vtotal, subtract from the crtc_vtotal to get the correct
> +		 * value.
>  		 */
> -		if (intel_vrr_always_use_vrr_tg(display))
> +		if (intel_vrr_always_use_vrr_tg(display)) {
>  			crtc_state->hw.adjusted_mode.crtc_vtotal =
>  				intel_vrr_vmin_vtotal(crtc_state);
>  
> -		if (HAS_AS_SDP(display)) {
> -			trans_vrr_vsync =
> -				intel_de_read(display,
> -					      TRANS_VRR_VSYNC(display, cpu_transcoder));
> -			crtc_state->vrr.vsync_start =
> -				REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
> -			crtc_state->vrr.vsync_end =
> -				REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
> +			crtc_state->hw.adjusted_mode.crtc_vsync_start =
> +				crtc_state->hw.adjusted_mode.crtc_vtotal -
> +				crtc_state->vrr.vsync_start;
> +			crtc_state->hw.adjusted_mode.crtc_vsync_end =
> +				crtc_state->hw.adjusted_mode.crtc_vtotal -
> +				crtc_state->vrr.vsync_end;
>  		}
>  	}
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel