Re: [PATCH 7/8] drm/i915/display: Add vblank_start adjustment logic for always-on VRR TG

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

On Thu, Oct 09, 2025 at 02:31:01PM +0530, Ankit Nautiyal wrote:
> As we move towards using a shorter, optimized guardband, we need to adjust
> how the delayed vblank start is computed.
> 
> Adjust the crtc_vblank_start using Vmin Vtotal - guardband only when
> intel_vrr_always_use_vrr_tg() is true.
> 
> This also paves way for guardband optimization, by handling the movement of
> the crtc_vblank_start for platforms that have VRR TG always active.
> 
> v2: Drop the helper and add the adjustment directly to
> intel_vrr_compute_guardband(). Ville
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 221b25832e56..5f9b8e5c48be 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -436,7 +436,7 @@ intel_vrr_max_guardband(struct intel_crtc_state *crtc_state)
>  void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
> -	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  
>  	if (!intel_vrr_possible(crtc_state))
>  		return;
> @@ -444,6 +444,10 @@ void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
>  	crtc_state->vrr.guardband = min(crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay,
>  					intel_vrr_max_guardband(crtc_state));
>  
> +	if (intel_vrr_always_use_vrr_tg(display))
> +		adjusted_mode->crtc_vblank_start  =
> +			crtc_state->vrr.vmin - crtc_state->vrr.guardband;

Since this is for the fixed refresh rate timings I think we should use
adjusted_mode.crtc_vtotal here instead of vmin (yes the two should be
equivalent at least for now, but I think it's better to be consistent).

And this should be squashed with the readout equivalent to make sure
both sides stay in sync so there's no possibility of angering the state
checker by only having the changes on one side.

> +
>  	if (DISPLAY_VER(display) < 13)
>  		crtc_state->vrr.pipeline_full =
>  			intel_vrr_guardband_to_pipeline_full(crtc_state,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel