> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, March 20, 2024 9:34 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 4/6] drm/i915: Eliminate extra frame from skl-glk sync->async
> flip change
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> On bdw-glk the sync->async flip change takes an extra frame due to the double
> buffering behaviour of the async flip plane control bit.
>
> Since on skl+ we are now explicitly converting the first async flip to a sync flip
> (in order to allow changing the modifier and/or
> ddb/watermarks) we are now taking two extra frames until async flips are
> actually active. We can drop that back down to one frame by setting the async
> flip bit already during the sync flip.
>
> Note that on bdw we don't currently do the extra sync flip (see
> intel_plane_do_async_flip()) so technically we wouldn't have to deal with this in
> i9xx_plane_update_arm(). But I added the relevant snippet of code there as
> well, just in case we ever decide to go for the extra sync flip on pre-skl platforms
> as well (we might, for example, want to change the fb stride).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Logically changes looks good. I see failures in CI.IGT
Better to have this green or a Tested-by would be good.
Thanks and Regards,
Arun R Murthy
-------------------
> ---
> drivers/gpu/drm/i915/display/i9xx_plane.c | 5 +++++
> drivers/gpu/drm/i915/display/intel_atomic_plane.c | 15 +++++++++++----
> .../gpu/drm/i915/display/skl_universal_plane.c | 5 +++++
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 0279c8aabdd1..76fc7626051b 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -455,6 +455,11 @@ static void i9xx_plane_update_arm(struct intel_plane
> *plane,
>
> dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state);
>
> + /* see intel_plane_atomic_calc_changes() */
> + if (plane->need_async_flip_disable_wa &&
> + crtc_state->async_flip_planes & BIT(plane->id))
> + dspcntr |= DISP_ASYNC_FLIP;
> +
> linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
>
> if (DISPLAY_VER(dev_priv) >= 4)
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 769010d0ebc4..7098a34a17c8 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -437,10 +437,6 @@ static bool intel_plane_do_async_flip(struct
> intel_plane *plane,
> * only X-tile is supported with async flips, though we could
> * extend this so other scanout parameters (stride/etc) could
> * be changed as well...
> - *
> - * FIXME: Platforms with need_async_flip_disable_wa==true will
> - * now end up doing two sync flips initially. Would be nice to
> - * combine those into just the one sync flip...
> */
> return DISPLAY_VER(i915) < 9 || old_crtc_state->uapi.async_flip; } @@
> -604,6 +600,17 @@ static int intel_plane_atomic_calc_changes(const struct
> intel_crtc_state *old_cr
> if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state)) {
> new_crtc_state->do_async_flip = true;
> new_crtc_state->async_flip_planes |= BIT(plane->id);
> + } else if (plane->need_async_flip_disable_wa &&
> + new_crtc_state->uapi.async_flip) {
> + /*
> + * On platforms with double buffered async flip bit we
> + * set the bit already one frame early during the sync
> + * flip (see {i9xx,skl}_plane_update_arm()). The
> + * hardware will therefore be ready to perform a real
> + * async flip during the next commit, without having
> + * to wait yet another frame for the bit to latch.
> + */
> + new_crtc_state->async_flip_planes |= BIT(plane->id);
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 860574d04f88..ad4c90344f68 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1174,6 +1174,11 @@ skl_plane_update_arm(struct intel_plane *plane,
> plane_ctl = plane_state->ctl |
> skl_plane_ctl_crtc(crtc_state);
>
> + /* see intel_plane_atomic_calc_changes() */
> + if (plane->need_async_flip_disable_wa &&
> + crtc_state->async_flip_planes & BIT(plane->id))
> + plane_ctl |= PLANE_CTL_ASYNC_FLIP;
> +
> if (DISPLAY_VER(dev_priv) >= 10)
> plane_color_ctl = plane_state->color_ctl |
> glk_plane_color_ctl_crtc(crtc_state);
> --
> 2.43.2
