| Message ID | 20250130184518.22353-4-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/vrr: Fix DSB+VRR usage for PTL+ | expand |
On 1/31/2025 12:15 AM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@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@linux.intel.com> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@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);
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);