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

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

On Mon, Oct 06, 2025 at 09:58:49AM +0530, Ankit Nautiyal wrote:
> As we move towards using a shorter, optimized guardband, we need to adjust
> how the delayed vblank start is computed.
> 
> Introduce intel_crtc_compute_vrr_guardband() to handle guardband
> computation and apply vblank_start adjustment for platforms that always use
> the VRR timing generator.
> 
> This function wraps the existing intel_vrr_compute_guardband() and adjusts
> crtc_vblank_start using (vblank_length - guardband) only when
> intel_vrr_always_use_vrr_tg() is true. Since the guardband is not yet
> optimized, the adjustment currently evaluates to zero, preserving existing
> behavior.
> 
> This paves way for guardband optimization, by handling the movement of
> the crtc_vblank_start for platforms that have VRR TG always active.
> 
> Also update allow_vblank_delay_fastset() to permit vblank delay adjustments
> during fastboot when VRR TG is always active, even without inherited state.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 33 ++++++++++++++++++--
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b2d4e24fd7c6..1964e41b5704 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2403,6 +2403,27 @@ static int intel_crtc_compute_set_context_latency(struct intel_atomic_state *sta
>  	return 0;
>  }
>  
> +static void intel_crtc_compute_vrr_guardband(struct intel_atomic_state *state,
> +					     struct intel_crtc *crtc)

Why this wrapper? You could just stick the adjustemnt into
intel_vrr_compute_guardband().

> +{
> +	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;
> +
> +	intel_vrr_compute_guardband(crtc_state);
> +
> +	if (intel_vrr_always_use_vrr_tg(display)) {
> +		int vblank_length = adjusted_mode->crtc_vtotal -
> +				    (crtc_state->set_context_latency +
> +				     adjusted_mode->crtc_vdisplay);
> +
> +		adjusted_mode->crtc_vblank_start +=
> +			vblank_length - crtc_state->vrr.guardband;

Why aren't you using the same 'vblank_start = vtotal-guardband' here as
during readout?

> +	}
> +}
> +
>  static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  				     struct intel_crtc *crtc)
>  {
> @@ -2414,7 +2435,7 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  	if (ret)
>  		return ret;
>  
> -	intel_vrr_compute_guardband(crtc_state);
> +	intel_crtc_compute_vrr_guardband(state, crtc);
>  
>  	ret = intel_dpll_crtc_compute_clock(state, crtc);
>  	if (ret)
> @@ -5105,9 +5126,15 @@ static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_s
>  	 * Allow fastboot to fix up vblank delay (handled via LRR
>  	 * codepaths), a bit dodgy as the registers aren't
>  	 * double buffered but seems to be working more or less...
> +	 *
> +	 * Also allow this when the VRR timing generator is always on,
> +	 * which implies optimized guardband is used. In such cases,
> +	 * vblank delay may vary even without inherited state, but it's
> +	 * still safe as VRR guardband is still same.
>  	 */
> -	return HAS_LRR(display) && old_crtc_state->inherited &&
> -		!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);
> +	return HAS_LRR(display) &&
> +	       (old_crtc_state->inherited || intel_vrr_always_use_vrr_tg(display)) &&
> +	       !intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);

This part doesn't seem directly related to the making crtc_vblank_start
correct. We still use the non-optimzied guardband so crtc_vblank_start
should not be changing during normal runtime operation.

>  }
>  
>  bool
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel