On Wed, 09 Feb 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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ä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> 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
