From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915: Consolidate all pre plane update vblank waits
Date: Wed, 09 Feb 2022 15:39:02 +0200 [thread overview]
Message-ID: <87o83gnoqx.fsf@intel.com> (raw)
In-Reply-To: <20220209113526.7595-7-ville.syrjala@linux.intel.com>
On Wed, 09 Feb 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> 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
next prev parent reply other threads:[~2022-02-09 13:39 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-09 11:35 [Intel-gfx] [PATCH 1/7] drm/i915: Move vblank waits out from IPS code Ville Syrjala
2022-02-09 11:35 ` [Intel-gfx] [PATCH 2/7] drm/i915: Change IPS calling convention Ville Syrjala
2022-02-09 13:17 ` Jani Nikula
2022-02-09 11:35 ` [Intel-gfx] [PATCH 3/7] drm/i915: Hoover the IPS enable/disable calls into the pre/post update hooks Ville Syrjala
2022-02-09 13:25 ` Jani Nikula
2022-02-09 11:35 ` [Intel-gfx] [PATCH 4/7] drm/i915: Move the IPS code to its own file Ville Syrjala
2022-02-09 13:34 ` Jani Nikula
2022-02-09 11:35 ` [Intel-gfx] [PATCH 5/7] drm/i915: Extract hsw_ips_get_config() Ville Syrjala
2022-02-09 13:35 ` Jani Nikula
2022-02-09 11:35 ` [Intel-gfx] [PATCH 6/7] drm/i915: Fix IPS disable in intel_plane_disable_noatomic() Ville Syrjala
2022-02-09 13:41 ` Jani Nikula
2022-02-09 11:35 ` [Intel-gfx] [PATCH 7/7] drm/i915: Consolidate all pre plane update vblank waits Ville Syrjala
2022-02-09 13:39 ` Jani Nikula [this message]
2022-02-09 16:36 ` Murthy, Arun R
2022-02-09 13:15 ` [Intel-gfx] [PATCH 1/7] drm/i915: Move vblank waits out from IPS code Jani Nikula
2022-02-09 14:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] " Patchwork
2022-02-09 14:08 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-02-09 14:39 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-09 17:36 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-02-09 19:13 ` Ville Syrjälä
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o83gnoqx.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox