[PATCH 3/3] drm/i915/vrr: Poll for the push send bit to clear on the DSB

[Nautiyal, Ankit K]

On 1/31/2025 12:15 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Since we now do the "wait for safe window" before triggering
> the push send, there is a theoretical possibity that we may
> have screwed up the vblank evasion and the push has slipped
> past the vmax decision boundary. In that case the push would
> only happen after the next frame's vactive, while we've
> already signalled the flip to have completed via the DSB
> interrupt immediately after triggering the push.
>
> To make sure we catch such screwups let's poll for the push
> send bit to clear. Assuming vblank delay has been dealt with
> already it should clear within ~1 scanline. But for safety
> let's give it ~2 scanlines. If the bit does not clear within
> that time the DSB will raise the poll error interrupt to inform
> us that something went wrong.
>
> Also I suppose it might generally be a good idea to make sure
> the send bit has cleared before we complete the commit because
> we're not supposed to send a new push while the previous one
> is still pending.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

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


> ---
>   drivers/gpu/drm/i915/display/intel_color.c   |  1 +
>   drivers/gpu/drm/i915/display/intel_display.c |  1 +
>   drivers/gpu/drm/i915/display/intel_vrr.c     | 31 ++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
>   4 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index be2691a80227..964a4bde3c88 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1990,6 +1990,7 @@ void intel_color_prepare_commit(struct intel_atomic_state *state,
>   
>   	intel_vrr_send_push(crtc_state->dsb_color_vblank, crtc_state);
>   	intel_dsb_wait_vblank_delay(state, crtc_state->dsb_color_vblank);
> +	intel_vrr_dsb_wait_push_sent(crtc_state->dsb_color_vblank, crtc_state);
>   	intel_dsb_interrupt(crtc_state->dsb_color_vblank);
>   
>   	intel_dsb_finish(crtc_state->dsb_color_vblank);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 69dbb0eb5ca1..3fc61c1848b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7732,6 +7732,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state,
>   
>   			intel_vrr_send_push(new_crtc_state->dsb_commit, new_crtc_state);
>   			intel_dsb_wait_vblank_delay(state, new_crtc_state->dsb_commit);
> +			intel_vrr_dsb_wait_push_sent(new_crtc_state->dsb_commit, new_crtc_state);
>   			intel_dsb_interrupt(new_crtc_state->dsb_commit);
>   		}
>   	}
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index adb51609d0a3..2925a013f708 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -5,6 +5,7 @@
>    */
>   
>   #include "i915_reg.h"
> +#include "intel_crtc.h"
>   #include "intel_de.h"
>   #include "intel_display_types.h"
>   #include "intel_dp.h"
> @@ -416,6 +417,36 @@ void intel_vrr_send_push(struct intel_dsb *dsb,
>   		intel_dsb_nonpost_end(dsb);
>   }
>   
> +void intel_vrr_dsb_wait_push_sent(struct intel_dsb *dsb,
> +				  const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	int wait, count;
> +
> +	/* not to be used in mmio codepaths */
> +	if (drm_WARN_ON(display->drm, !dsb))
> +		return;
> +
> +	/*
> +	 * We always do the send after the "wait for safe window", thus
> +	 * the push should happen within ~1 scanline. Poll for ~2 scanlines
> +	 * to make sure it does. If the bit does not clear DSB will raise
> +	 * the poll error interrupt as an indication that we failed to
> +	 * sequence things correctly.
> +	 *
> +	 * Note that vblank delay does postpone the bit clearing, but
> +	 * we can ignore that by assuming that our caller has already
> +	 * dealt with it via intel_dsb_wait_vblank_delay() after
> +	 * triggering the push.
> +	 */
> +	wait = 2; /* usecs */
> +	count = DIV_ROUND_UP(intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode, 2), wait);
> +
> +	intel_dsb_poll(dsb, TRANS_PUSH(display, cpu_transcoder),
> +		       TRANS_PUSH_SEND, 0, wait, count);
> +}
> +
>   bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
>   {
>   	struct intel_display *display = to_intel_display(crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 899cbf40f880..19a5aa671eae 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -25,6 +25,8 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>   void intel_vrr_enable(const struct intel_crtc_state *crtc_state);
>   void intel_vrr_send_push(struct intel_dsb *dsb,
>   			 const struct intel_crtc_state *crtc_state);
> +void intel_vrr_dsb_wait_push_sent(struct intel_dsb *dsb,
> +				  const struct intel_crtc_state *crtc_state);
>   bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state);
>   void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
>   void intel_vrr_get_config(struct intel_crtc_state *crtc_state);