RE: [PATCH] drm/i915/vrr: Refactor VRR live status wait into common helper

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Ankit
> Nautiyal
> Sent: 02 September 2025 17:59
> To: intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nauti...@intel.com>
> Subject: [PATCH] drm/i915/vrr: Refactor VRR live status wait into common
> helper
> 
> Add a helper to consolidate timeout handling and error logging when waiting
> for VRR live status to clear. Log an error message if the VRR live status bit 
> fails
> to clear within the timeout.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 3eed37f271b0..0797b5b375b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -627,6 +627,15 @@ void intel_vrr_enable(const struct intel_crtc_state
> *crtc_state)
>       }
>  }
> 
> +static void intel_vrr_wait_for_live_status_clear(struct intel_display 
> *display,
> +                                              enum transcoder
> cpu_transcoder)
> +{
> +     if (intel_de_wait_for_clear(display,
> +                                 TRANS_VRR_STATUS(display,
> cpu_transcoder),
> +                                 VRR_STATUS_VRR_EN_LIVE, 1000))
> +             drm_err(display->drm, "Timed out waiting for VRR live status
> to
> +clear\n"); }
> +
>  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)  {
>       struct intel_display *display = to_intel_display(old_crtc_state); @@ -
> 638,9 +647,7 @@ void intel_vrr_disable(const struct intel_crtc_state
> *old_crtc_state)
>       if (!intel_vrr_always_use_vrr_tg(display)) {
>               intel_de_write(display, TRANS_VRR_CTL(display,
> cpu_transcoder),
>                              trans_vrr_ctl(old_crtc_state));
> -             intel_de_wait_for_clear(display,
> -                                     TRANS_VRR_STATUS(display,
> cpu_transcoder),
> -                                     VRR_STATUS_VRR_EN_LIVE, 1000);
> +             intel_vrr_wait_for_live_status_clear(display, cpu_transcoder);
>               intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> 0);
>       }
> 
> @@ -686,8 +693,8 @@ void intel_vrr_transcoder_disable(const struct
> intel_crtc_state *crtc_state)
> 
>       intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 0);
> 
> -     intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display,
> cpu_transcoder),
> -                             VRR_STATUS_VRR_EN_LIVE, 1000);
> +     intel_vrr_wait_for_live_status_clear(display, cpu_transcoder);
> +

LGTM

Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com>

>       intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);  }
> 
> --
> 2.45.2