Re: [PATCH 14/18] drm/i915/vrr: Fix vmin/vmax/flipline on TGL when using vblank delay

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

On 12/11/2024 2:40 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Turns out that TGL needs its vmin/vmax/flipling adjusted based
typo: flipline
> on the vblank delay, otherwise the hardware pushes the vtotals
> fiurther out. Make it so.
Typo: further
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

LGTM.

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


> ---
>   drivers/gpu/drm/i915/display/intel_vrr.c | 32 +++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 15017254d250..c08dd1e5cffc 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -118,30 +118,41 @@ static int intel_vrr_vblank_exit_length(const struct intel_crtc_state *crtc_stat
>   	if (DISPLAY_VER(display) >= 13)
>   		return crtc_state->vrr.guardband;
>   	else
> -		/* The hw imposes the extra scanline before frame start */
> +		/* hardware imposes one extra scanline somewhere */
>   		return crtc_state->vrr.pipeline_full + crtc_state->framestart_delay + 1;
>   }
>   
>   int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state)
>   {
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
>   	/* Min vblank actually determined by flipline */
> -	return intel_vrr_vmin_flipline(crtc_state);
> +	if (DISPLAY_VER(display) >= 13)
> +		return intel_vrr_vmin_flipline(crtc_state);
> +	else
> +		return intel_vrr_vmin_flipline(crtc_state) +
> +			intel_vrr_vblank_delay(crtc_state);
>   }
>   
>   int intel_vrr_vmax_vtotal(const struct intel_crtc_state *crtc_state)
>   {
> -	return crtc_state->vrr.vmax;
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	if (DISPLAY_VER(display) >= 13)
> +		return crtc_state->vrr.vmax;
> +	else
> +		return crtc_state->vrr.vmax +
> +			intel_vrr_vblank_delay(crtc_state);
>   }
>   
>   int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state)
>   {
> -	/* Min vblank actually determined by flipline */
> -	return intel_vrr_vmin_flipline(crtc_state) - intel_vrr_vblank_exit_length(crtc_state);
> +	return intel_vrr_vmin_vtotal(crtc_state) - intel_vrr_vblank_exit_length(crtc_state);
>   }
>   
>   int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state)
>   {
> -	return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state);
> +	return intel_vrr_vmax_vtotal(crtc_state) - intel_vrr_vblank_exit_length(crtc_state);
>   }
>   
>   static bool
> @@ -290,9 +301,18 @@ void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
>   		crtc_state->vrr.guardband =
>   			crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start;
>   	} else {
> +		/* hardware imposes one extra scanline somewhere */
>   		crtc_state->vrr.pipeline_full =
>   			min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
>   			    crtc_state->framestart_delay - 1);
> +
> +		/*
> +		 * vmin/vmax/flipline also need to be adjusted by
> +		 * the vblank delay to maintain correct vtotals.
> +		 */
> +		crtc_state->vrr.vmin -= intel_vrr_vblank_delay(crtc_state);
> +		crtc_state->vrr.vmax -= intel_vrr_vblank_delay(crtc_state);
> +		crtc_state->vrr.flipline -= intel_vrr_vblank_delay(crtc_state);
>   	}
>   }
>