Re: [igt-dev] [PATCH i-g-t v2 7/8] lib/igt_fb: Ignore the X component when computing CRC

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

On Mon, 28 Mar 2022 16:55:08 +0200
Maxime Ripard <maxime@cerno.tech> wrote:

> The igt_fb_get_fnv1a_crc() function will compute a FNV-1a hash over the
> content of the framebuffer. The sole user of this function is the
> writeback test suite, which will use it to compare an XRGB8888 buffer
> used in input to an XRGB8888 buffer filled by the writeback connector.
> 
> However, that function uses each bytes of each buffers to compute the
> hash, and therefore the writeback code assumes that the hardware will
> preserve the content of the X component through the writeback pipeline,
> which isn't true for all hardware. VC4 doesn't for example.
> 
> Since that function is only ever used for XRGB8888 buffers, let's just
> set the most significant to 0 (which is the X padding) for each pixel
> when computing the hash, and thus ignore whatever the hardware will
> return here.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Yes, this is absolutely a requirement when handling any pixel format
with X-channel.


Thanks,
pq

> ---
>  lib/igt_fb.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index eafbe7fd41cd..66c52bb83876 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -4392,15 +4392,19 @@ int igt_fb_get_fnv1a_crc(struct igt_fb *fb, igt_crc_t *crc)
>  {
>  	const uint32_t FNV1a_OFFSET_BIAS = 2166136261;
>  	const uint32_t FNV1a_PRIME = 16777619;
> +	uint32_t *line = NULL;
>  	uint32_t hash;
>  	void *map;
> -	char *ptr, *line = NULL;
> +	char *ptr;
>  	int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
>  	uint32_t stride = calc_plane_stride(fb, 0);
>  
>  	if (fb->num_planes != 1)
>  		return -EINVAL;
>  
> +	if (fb->drm_format != DRM_FORMAT_XRGB8888)
> +		return -EINVAL;
> +
>  	ptr = igt_fb_map_buffer(fb->fd, fb);
>  	igt_assert(ptr);
>  	map = ptr;
> @@ -4422,9 +4426,17 @@ int igt_fb_get_fnv1a_crc(struct igt_fb *fb, igt_crc_t *crc)
>  
>  		igt_memcpy_from_wc(line, ptr, fb->width * cpp);
>  
> -		for (x = 0; x < fb->width * cpp; x++) {
> -			hash ^= line[x];
> -			hash *= FNV1a_PRIME;
> +		for (x = 0; x < fb->width; x++) {
> +			unsigned int i;
> +			uint32_t pixel = le32_to_cpu(line[x]);
> +			pixel &= 0x00ffffff;
> +
> +			for (i = 0; i < sizeof(pixel); i++) {
> +				uint8_t component = (pixel >> (i * 8)) & 0xff;
> +
> +				hash ^= component;
> +				hash *= FNV1a_PRIME;
> +			}
>  		}
>  	}
>  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]