Re: [PATCH 1/2] drm/i915/vrr: Hide the ICL/TGL intel_vrr_flipline_offset() mangling better

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 9/19/2025 4:52 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ICL/TGL VRR hardware won't allow us to program flipline==vmin. If we do
> that the actual effect will be the same as if we had programmed
> flipline=vmin+1, which would make the minimum vtotal one scanline taller
> than expected.
>
> To compensate for this we reduce vmin by one, and then program
> flipline=vmin+1. So we end up with a flipline value that matches
> the expected minimum vtotal. Currently this adjustment happens
> in intel_vrr_compute_config() which means that crtc_state->vrr.vmin
> will no longer be directly usable for the remainder of the high
> level VRR code. That is annoying at best, fragile at worst.
>
> Hide the adjustment in low level code instead. This will allow most
> of the higher level VRR code to remain blissfully ignorant about this
> fact. Afterwards crtc_state->vrr.{vmin,flipline} will be equal
> and match the minimum vtotal, exactly how things already work
> on ADL+.
>
> The only slight downside is that the actual register value will no
> longer match crtc_state->vrr.vmin on ICL/TGL, but that may already
> be the case on TGL because the register value will also have been
> adjusted by the SCL.
>
> Note that we must change the guardband calculation to account
> for intel_vrr_extra_vblank_delay() explicitly. Previously that
> was accidentally handled by the earlier vmin reduction by
> intel_vrr_flipline_offset().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


> ---
>   drivers/gpu/drm/i915/display/intel_vrr.c | 30 ++++++++++++++----------
>   1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 71a985d00604..e725b4581e81 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -108,15 +108,20 @@ int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state)
>   
>   static int intel_vrr_flipline_offset(struct intel_display *display)
>   {
> -	/* ICL/TGL hardware imposes flipline>=vmin+1 */
> +	/*
> +	 * ICL/TGL hardware imposes flipline>=vmin+1
> +	 *
> +	 * We reduce the vmin value to compensate when programming the
> +	 * hardware. This approach allows flipline to remain set at the
> +	 * original value, and thus the frame will have the desired
> +	 * minimum vtotal.
> +	 */
>   	return DISPLAY_VER(display) < 13 ? 1 : 0;
>   }
>   
>   static int intel_vrr_vmin_flipline(const struct intel_crtc_state *crtc_state)
>   {
> -	struct intel_display *display = to_intel_display(crtc_state);
> -
> -	return crtc_state->vrr.vmin + intel_vrr_flipline_offset(display);
> +	return crtc_state->vrr.vmin;
>   }
>   
>   static int intel_vrr_guardband_to_pipeline_full(const struct intel_crtc_state *crtc_state,
> @@ -400,13 +405,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>   	else
>   		intel_vrr_compute_fixed_rr_timings(crtc_state);
>   
> -	/*
> -	 * flipline determines the min vblank length the hardware will
> -	 * generate, and on ICL/TGL flipline>=vmin+1, hence we reduce
> -	 * vmin by one to make sure we can get the actual min vblank length.
> -	 */
> -	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
> -
>   	if (HAS_AS_SDP(display)) {
>   		crtc_state->vrr.vsync_start =
>   			(crtc_state->hw.adjusted_mode.crtc_vtotal -
> @@ -426,7 +424,8 @@ void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
>   		return;
>   
>   	crtc_state->vrr.guardband =
> -		crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start;
> +		crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
> +		intel_vrr_extra_vblank_delay(display);
>   
>   	if (DISPLAY_VER(display) < 13) {
>   		/* FIXME handle the limit in a proper way */
> @@ -597,7 +596,10 @@ void intel_vrr_set_db_point_and_transmission_line(const struct intel_crtc_state
>   
>   static int intel_vrr_hw_vmin(const struct intel_crtc_state *crtc_state)
>   {
> -	return intel_vrr_hw_value(crtc_state, crtc_state->vrr.vmin);
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	return intel_vrr_hw_value(crtc_state, crtc_state->vrr.vmin) -
> +		intel_vrr_flipline_offset(display);
>   }
>   
>   static int intel_vrr_hw_vmax(const struct intel_crtc_state *crtc_state)
> @@ -762,6 +764,8 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>   			crtc_state->vrr.flipline += intel_vrr_real_vblank_delay(crtc_state);
>   			crtc_state->vrr.vmax += intel_vrr_real_vblank_delay(crtc_state);
>   			crtc_state->vrr.vmin += intel_vrr_real_vblank_delay(crtc_state);
> +
> +			crtc_state->vrr.vmin += intel_vrr_flipline_offset(display);
>   		}
>   
>   		/*