[PATCH 4/6] drm/i915: Eliminate extra frame from skl-glk sync->async flip change

[Murthy, Arun R]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, March 20, 2024 9:34 PM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 4/6] drm/i915: Eliminate extra frame from skl-glk sync->async
> flip change
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> 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 at linux.intel.com>

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