Re: [PATCH i-g-t v2 2/2] tests/gem|xe_ccs: Check surface ccs data instead of naive comparison

["Jahagirdar, Akshata" <akshata.jahagirdar@intel.com>]

Looks good to me:

Reviewed-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>

Best,
Akshata

On 4/10/2024 3:09 AM, Zbigniew Kempczyński wrote:
> Start using function which extracts ccs data from the surface and
> determines its compression.
>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Karolina Stolarek <karolina.stolarek@intel.com>
> Cc: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
> ---
> v2: rephrase comment (Karolina)
> ---
>   tests/intel/gem_ccs.c | 16 +++++++++++++---
>   tests/intel/xe_ccs.c  | 14 ++++++--------
>   2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/tests/intel/gem_ccs.c b/tests/intel/gem_ccs.c
> index e4126bcaa8..a8e7e8a747 100644
> --- a/tests/intel/gem_ccs.c
> +++ b/tests/intel/gem_ccs.c
> @@ -305,6 +305,10 @@ static int blt_block_copy3(int i915,
>   	return ret;
>   }
>   
> +#define CHECK_FROM_WIDTH 256
> +#define CHECK_FROM_HEIGHT 256
> +#define FROM_EXP_WH(w, h) ((w) >= CHECK_FROM_WIDTH && (h) >= CHECK_FROM_HEIGHT)
> +
>   static void block_copy(int i915,
>   		       const intel_ctx_t *ctx,
>   		       const struct intel_execution_engine2 *e,
> @@ -359,9 +363,15 @@ static void block_copy(int i915,
>   	blt_block_copy(i915, ctx, e, ahnd, &blt, pext);
>   	gem_sync(i915, mid->handle);
>   
> -	/* We expect mid != src if there's compression */
> -	if (mid->compression)
> -		igt_assert(memcmp(src->ptr, mid->ptr, src->size) != 0);
> +	/*
> +	 * If there's a compression we expect ctrl surface is not fully zeroed.
> +	 * Gradient image used as the reference may be not compressible for
> +	 * smaller sizes. Let's use some 'safe' size we're sure compression
> +	 * occurs and ctrl surface will be filled with some not-zeroed values.
> +	 */
> +	if (mid->compression && FROM_EXP_WH(width, height))
> +		igt_assert(blt_surface_is_compressed(i915, (intel_ctx_t *)ctx, e,
> +						     ahnd, mid));
>   
>   	WRITE_PNG(i915, run_id, "mid", &blt.dst, width, height, bpp);
>   
> diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
> index 7d0f2f2a11..b3bf97af7e 100644
> --- a/tests/intel/xe_ccs.c
> +++ b/tests/intel/xe_ccs.c
> @@ -359,15 +359,13 @@ static void block_copy(int xe,
>   	intel_ctx_xe_sync(ctx, true);
>   
>   	/*
> -	 * We expect mid != src if there's compression. Ignore this for small
> -	 * width x height for linear as compression for gradient occurs in the
> -	 * middle for bigger sizes. We also ignore 1x1 as this looks same for
> -	 * xmajor.
> +	 * If there's a compression we expect ctrl surface is not fully zeroed.
> +	 * Gradient image used as the reference may be not compressible for
> +	 * smaller sizes. Let's use some 'safe' size we're sure compression
> +	 * occurs and ctrl surface will be filled with some not-zeroed values.
>   	 */
> -	if (mid->compression && MIN_EXP_WH(width, height)) {
> -		if (mid_tiling != T_LINEAR || FROM_EXP_WH(width, height))
> -			igt_assert(memcmp(src->ptr, mid->ptr, src->size) != 0);
> -	}
> +	if (mid->compression && FROM_EXP_WH(width, height))
> +		igt_assert(blt_surface_is_compressed(xe, ctx, NULL, ahnd, mid));
>   
>   	WRITE_PNG(xe, run_id, "mid", &blt.dst, width, height, bpp);
>