On Wed, 09 Feb 2022, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > There are several reasons why we might have to do a vblank wait > between some of the pre_plane_update() steps and the actual > plane update. Currently we do a vblank wait for each of those > individually. Let's consolidate things so that we just do a > single vblank wait at the end of the pre_plane_update() step. > > Note that I don't think we should be hitting multiple vblank > waits here currently, at least in most cases. But no real > reason that couldn't happen in the future when some new > features/workarounds are introduced. > > Signed-off-by: Ville Syrjälä <[email protected]>
Reviewed-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_display.c | 35 ++++++++++++-------- > 1 file changed, 22 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 401a339973bf..7c37c4355606 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -742,6 +742,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc, > to_intel_crtc_state(crtc->base.state); > struct intel_plane_state *plane_state = > to_intel_plane_state(plane->base.state); > + bool need_vblank_wait = false; > > drm_dbg_kms(&dev_priv->drm, > "Disabling [PLANE:%d:%s] on [CRTC:%d:%s]\n", > @@ -756,7 +757,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc, > if ((crtc_state->active_planes & ~BIT(PLANE_CURSOR)) == 0 && > hsw_ips_disable(crtc_state)) { > crtc_state->ips_enabled = false; > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > } > > /* > @@ -770,7 +771,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc, > */ > if (HAS_GMCH(dev_priv) && > intel_set_memory_cxsr(dev_priv, false)) > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > @@ -779,6 +780,9 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc, > if (DISPLAY_VER(dev_priv) == 2 && !crtc_state->active_planes) > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, > false); > > + if (need_vblank_wait) > + intel_crtc_wait_for_next_vblank(crtc); > + > intel_plane_disable_arm(plane, crtc_state); > intel_crtc_wait_for_next_vblank(crtc); > } > @@ -1258,7 +1262,7 @@ static void intel_crtc_disable_flip_done(struct > intel_atomic_state *state, > } > } > > -static void intel_crtc_async_flip_disable_wa(struct intel_atomic_state > *state, > +static bool intel_crtc_async_flip_disable_wa(struct intel_atomic_state > *state, > struct intel_crtc *crtc) > { > const struct intel_crtc_state *old_crtc_state = > @@ -1268,7 +1272,7 @@ static void intel_crtc_async_flip_disable_wa(struct > intel_atomic_state *state, > u8 update_planes = new_crtc_state->update_planes; > const struct intel_plane_state *old_plane_state; > struct intel_plane *plane; > - bool need_vbl_wait = false; > + bool need_vblank_wait = false; > int i; > > for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { > @@ -1281,12 +1285,11 @@ static void intel_crtc_async_flip_disable_wa(struct > intel_atomic_state *state, > */ > plane->async_flip(plane, old_crtc_state, > old_plane_state, false); > - need_vbl_wait = true; > + need_vblank_wait = true; > } > } > > - if (need_vbl_wait) > - intel_crtc_wait_for_next_vblank(crtc); > + return need_vblank_wait; > } > > static void intel_pre_plane_update(struct intel_atomic_state *state, > @@ -1298,14 +1301,15 @@ static void intel_pre_plane_update(struct > intel_atomic_state *state, > const struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > enum pipe pipe = crtc->pipe; > + bool need_vblank_wait = false; > > intel_psr_pre_plane_update(state, crtc); > > if (hsw_ips_pre_update(state, crtc)) > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > > if (intel_fbc_pre_update(state, crtc)) > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > > if (!needs_async_flip_vtd_wa(old_crtc_state) && > needs_async_flip_vtd_wa(new_crtc_state)) > @@ -1337,7 +1341,7 @@ static void intel_pre_plane_update(struct > intel_atomic_state *state, > */ > if (HAS_GMCH(dev_priv) && old_crtc_state->hw.active && > new_crtc_state->disable_cxsr && intel_set_memory_cxsr(dev_priv, > false)) > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > > /* > * IVB workaround: must disable low power watermarks for at least > @@ -1348,7 +1352,7 @@ static void intel_pre_plane_update(struct > intel_atomic_state *state, > */ > if (old_crtc_state->hw.active && > new_crtc_state->disable_lp_wm && ilk_disable_lp_wm(dev_priv)) > - intel_crtc_wait_for_next_vblank(crtc); > + need_vblank_wait = true; > > /* > * If we're doing a modeset we don't need to do any > @@ -1389,8 +1393,13 @@ static void intel_pre_plane_update(struct > intel_atomic_state *state, > * WA for platforms where async address update enable bit > * is double buffered and only latched at start of vblank. > */ > - if (old_crtc_state->uapi.async_flip && !new_crtc_state->uapi.async_flip) > - intel_crtc_async_flip_disable_wa(state, crtc); > + if (old_crtc_state->uapi.async_flip && > + !new_crtc_state->uapi.async_flip && > + intel_crtc_async_flip_disable_wa(state, crtc)) > + need_vblank_wait = true; > + > + if (need_vblank_wait) > + intel_crtc_wait_for_next_vblank(crtc); > } > > static void intel_crtc_disable_planes(struct intel_atomic_state *state, -- Jani Nikula, Intel Open Source Graphics Center
