Re: [PATCH 10/10] drm/i915/dsb: Inline dsb_vblank_delay() into intel_dsb_wait_for_delayed_vblank()

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

On Wed, Sep 24, 2025 at 07:45:42PM +0530, Ankit Nautiyal wrote:
> Drop the now single-use dsb_vblank_delay() helper and inline its logic
> directly into intel_dsb_wait_for_delayed_vblank().
> 
> This will help to keep all VRR related wait stuff in one place.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 56 ++++++++++--------------
>  1 file changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index ae8574880ef2..44a465ff52d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -115,24 +115,6 @@ static bool pre_commit_is_vrr_active(struct intel_atomic_state *state,
>  	return old_crtc_state->vrr.enable && !intel_crtc_vrr_disabling(state, crtc);
>  }
>  
> -static int dsb_vblank_delay(struct intel_atomic_state *state,
> -			    struct intel_crtc *crtc)
> -{
> -	const struct intel_crtc_state *crtc_state =
> -		intel_pre_commit_crtc_state(state, crtc);
> -
> -	if (pre_commit_is_vrr_active(state, crtc))
> -		/*
> -		 * When the push is sent during vblank it will trigger
> -		 * on the next scanline, hence we have up to one extra
> -		 * scanline until the delayed vblank occurs after
> -		 * TRANS_PUSH has been written.
> -		 */
> -		return crtc_state->set_context_latency + 1;
> -	else
> -		return intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
> -}
> -
>  static int dsb_vtotal(struct intel_atomic_state *state,
>  		      struct intel_crtc *crtc)
>  {
> @@ -821,24 +803,34 @@ void intel_dsb_wait_for_delayed_vblank(struct intel_atomic_state *state,
>  	struct intel_crtc *crtc = dsb->crtc;
>  	const struct intel_crtc_state *crtc_state =
>  		intel_pre_commit_crtc_state(state, crtc);
> -	int usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
> -					     dsb_vblank_delay(state, crtc));
> +	int usecs;
>  
> -	/*
> -	 * If the push happened before the vmin decision boundary
> -	 * we don't know how far we are from the undelayed vblank.
> -	 * Wait until we're past the vmin safe window, at which
> -	 * point we're SCL lines away from the delayed vblank.
> -	 *
> -	 * If the push happened after the vmin decision boundary
> -	 * the hardware itself guarantees that we're SCL lines
> -	 * away from the delayed vblank, and we won't be inside
> -	 * the vmin safe window so this extra wait does nothing.
> -	 */
> -	if (pre_commit_is_vrr_active(state, crtc))
> +	if (pre_commit_is_vrr_active(state, crtc)) {
> +		/*
> +		 * If the push happened before the vmin decision boundary
> +		 * we don't know how far we are from the undelayed vblank.
> +		 * Wait until we're past the vmin safe window, at which
> +		 * point we're SCL lines away from the delayed vblank.
> +		 *
> +		 * If the push happened after the vmin decision boundary
> +		 * the hardware itself guarantees that we're SCL lines
> +		 * away from the delayed vblank, and we won't be inside
> +		 * the vmin safe window so this extra wait does nothing.
> +		 */
>  		intel_dsb_wait_scanline_out(state, dsb,
>  					    intel_vrr_safe_window_start(crtc_state),
>  					    intel_vrr_vmin_safe_window_end(crtc_state));
> +		/*
> +		 * When the push is sent during vblank it will trigger
> +		 * on the next scanline, hence we have up to one extra
> +		 * scanline until the delayed vblank occurs after
> +		 * TRANS_PUSH has been written.
> +		 */
> +		usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
> +						 crtc_state->set_context_latency + 1);
> +	} else {
> +		usecs = intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);

That also returns the delay in scanlines.

> +	}
>  
>  	intel_dsb_wait_usec(dsb, usecs);

So I guess just do the scanline_to_usec() conversion here when calling
wait_usec().

I think eventually we may want to change this part to be:

if (something)
	intel_dsb_wait_hblanks(x);
else
	intel_dsb_wait_usec(intel_scanlines_to_usecs(x));

But need to figure out what that "something" is. IIRC bspec
might not actually say which platforms have the "wait hblanks"
command. I gave it quick try on LNL but it didn't seem to work
there.

-- 
Ville Syrjälä
Intel