[PATCH i-g-t 1/2] tests/kms_async_flips: Extend the "async flip needs an extra frame" logic

[Kulkarni, Vandita]

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, March 23, 2024 2:07 AM
> To: igt-dev at lists.freedesktop.org
> Subject: [PATCH i-g-t 1/2] tests/kms_async_flips: Extend the "async flip
> needs an extra frame" logic
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Soon i915 will be converting the first async flip always to a sync flip on SKL+.
> The reason being that Xorg/modesetting typically attempts to change the
> modifier when it issues the first async flip, and we can't do that with an actual
> async flip.
> 
> Extend the logic to consider all platforms that need the extra frame, which
> will now be:
> - BDW-GLK due to the async flip hw double buffer fail
> - SKL+ to change the modifier
> - ADL+ to optimize watermarks/DDB
> 
> Note that the 'AT_LEAST_GEN(devid, 12)' already included TGL in this logic
> despite the kernel not actually forcing any sync flips on it previously. So only
> ICL was being correctly excluded here. Also BDW wasn't being included
> despite needing it due to the double buffer fail.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---

Looks good to me.
Reviewed-by: Vandita Kulkarni <vandita.kulkarni at intel.com>

>  tests/kms_async_flips.c | 46 +++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c index
> 2895168f7dc7..efbc8778d7e1 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -233,6 +233,28 @@ static void test_init_fbs(data_t *data)
>  	igt_plane_set_size(data->plane, width, height);  }
> 
> +static bool async_flip_needs_extra_frame(data_t *data) {
> +	uint32_t devid;
> +
> +	if (!is_intel_device(data->drm_fd))
> +		return false;
> +
> +	devid = intel_get_drm_devid(data->drm_fd);
> +
> +	/*
> +	 * On BDW-GLK async address update bit is double buffered
> +	 * on vblank. So the first async flip will in fact be
> +	 * performed as a sync flip by the hardware.
> +	 *
> +	 * In order to allow the first async flip to change the modifier
> +	 * on SKL+ (needed by Xorg/modesetting), and to optimize
> +	 * watermarks/ddb for faster response on ADL+, we convert the
> +	 * first async flip to a sync flip.
> +	 */
> +	return intel_display_ver(devid) >= 9 || IS_BROADWELL(devid); }
> +
>  static void test_async_flip(data_t *data)  {
>  	int ret, frame;
> @@ -259,26 +281,14 @@ static void test_async_flip(data_t *data)
> 
>  			flags |= DRM_MODE_PAGE_FLIP_ASYNC;
> 
> -			/*
> -			 * In older platforms (<= Gen10), async address
> update bit is double buffered.
> -			 * So flip timestamp can be verified only from the
> second flip.
> -			 * The first async flip just enables the async address
> update.
> -			 * In platforms greater than DISPLAY13 the first async
> flip will be discarded
> -			 * in order to change the watermark levels as per the
> optimization. Hence the
> -			 * subsequent async flips will actually do the
> asynchronous flips.
> -			 */
> -			if (is_intel_device(data->drm_fd)) {
> -				uint32_t devid = intel_get_drm_devid(data-
> >drm_fd);
> +			if (async_flip_needs_extra_frame(data)) {
> +				ret = drmModePageFlip(data->drm_fd, data-
> >crtc_id,
> +						      data->bufs[frame %
> NUM_FBS].fb_id,
> +						      flags, data);
> 
> -				if (IS_GEN9(devid) || IS_GEN10(devid) ||
> AT_LEAST_GEN(devid, 12)) {
> -					ret = drmModePageFlip(data-
> >drm_fd, data->crtc_id,
> -							      data->bufs[frame
> % NUM_FBS].fb_id,
> -							      flags, data);
> +				igt_assert(ret == 0);
> 
> -					igt_assert(ret == 0);
> -
> -					wait_flip_event(data);
> -				}
> +				wait_flip_event(data);
>  			}
>  		}
> 
> --
> 2.43.2