Re: [PATCH 01/13] drm/i915/vrr: Fix intel_vrr_dcb_vmin_vblank_start()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 23 Oct 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_vrr_dcb_vmin_vblank_start() may evaluate
> intel_vrr_dcb_vmin_vblank_start_next() twice, and given that
> this is doing hardware access under the hood isn't really
> desired.

Feels like there's a word missing there. This? It?

> Even worse, the double evaluation might even cause
> intel_vrr_dcb_vmin_vblank_start() itself to finally
> return -EINVAL (if we race against the DMC hard enough),
> which we definitely never want to do.
>
> Eliminate the double evaluation, guaranteeing a sane
> final return value at the same time.

I think a Fixes: would be in order.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index b14750692ae0..e59345bd1d33 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -1202,9 +1202,13 @@ int intel_vrr_safe_window_start(const struct intel_crtc_state *crtc_state)
>  static int
>  intel_vrr_dcb_vmin_vblank_start(const struct intel_crtc_state *crtc_state)
>  {
> -	return (intel_vrr_dcb_vmin_vblank_start_next(crtc_state) < 0) ?
> -		intel_vrr_dcb_vmin_vblank_start_final(crtc_state) :
> -		intel_vrr_dcb_vmin_vblank_start_next(crtc_state);
> +	int vblank_start;
> +
> +	vblank_start = intel_vrr_dcb_vmin_vblank_start_next(crtc_state);
> +	if (vblank_start < 0)
> +		vblank_start = intel_vrr_dcb_vmin_vblank_start_final(crtc_state);
> +
> +	return vblank_start;
>  }
>  
>  int intel_vrr_vmin_safe_window_end(const struct intel_crtc_state *crtc_state)

-- 
Jani Nikula, Intel