Re: [PATCH 1/6] drm/i915/display: Handle VSYNC timing in LRR path

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

On Fri, May 22, 2026 at 06:55:06PM +0530, Ankit Nautiyal wrote:
> LRR already updates crtc_vtotal/crtc_vblank_end seamlessly.
> Extend the same handling to crtc_vsync_start/crtc_vsync_end so
> VSYNC timing changes are programmed and accepted via the LRR path instead
> of forcing a full modeset.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 757a78c75bbf..75c998960864 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2797,6 +2797,9 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
>  	intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
>  		       VBLANK_START(crtc_vblank_start - 1) |
>  		       VBLANK_END(crtc_vblank_end - 1));

I'm thinking we probably want some kind of comment here. Perhaps
something like this:
/*
 * DP doesn't have vertical sync, so TRANS_VSYNC only affects
 * the position of the vsync interrupt (and does so even when
 * using the VRR timing generator!). Thus updating TRANS_VSYNC
 * here seems fine even if it isn't double buffered.
 */

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

> +	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));
>  	/*
>  	 * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal
>  	 * bits are not required. Since the support for these bits is going to
> @@ -5166,9 +5169,9 @@ 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 (!fastset || !pipe_config->update_lrr) { \
> +		PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
> +		PIPE_CONF_CHECK_I(name.crtc_vsync_end); \
>  		PIPE_CONF_CHECK_I(name.crtc_vtotal); \
>  		PIPE_CONF_CHECK_I(name.crtc_vblank_end); \
>  	} \
> @@ -5782,6 +5785,8 @@ static bool lrr_params_changed(const struct intel_crtc_state *old_crtc_state,
>  
>  	return old_adjusted_mode->crtc_vblank_start != new_adjusted_mode->crtc_vblank_start ||
>  		old_adjusted_mode->crtc_vblank_end != new_adjusted_mode->crtc_vblank_end ||
> +		old_adjusted_mode->crtc_vsync_start != new_adjusted_mode->crtc_vsync_start ||
> +		old_adjusted_mode->crtc_vsync_end != new_adjusted_mode->crtc_vsync_end ||
>  		old_adjusted_mode->crtc_vtotal != new_adjusted_mode->crtc_vtotal ||
>  		old_crtc_state->set_context_latency != new_crtc_state->set_context_latency;
>  }
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel