Re: [PATCH v2 04/13] drm/format-helper: Store RGB565 in little-endian order

[Maíra Canal <mcanal@xxxxxxxxxx>]

On 12/20/22 13:11, Thomas Zimmermann wrote:> Fix to-RGB565 conversion
helpers to store the result in little-
> endian byte order. Update test cases as well.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

Reviewed-by: Maíra Canal <mcanal@xxxxxxxxxx>

Best Regards,
- Maíra Canal

> ---
>  drivers/gpu/drm/drm_format_helper.c           |  9 +++++----
>  .../gpu/drm/tests/drm_format_helper_test.c    | 20 ++++++++++++++++++-
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index b98bd7c5caee..f3f3b3809a3e 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -322,7 +322,7 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
>  
>  static void drm_fb_xrgb8888_to_rgb565_line(void *dbuf, const void *sbuf, unsigned int pixels)
>  {
> -	u16 *dbuf16 = dbuf;
> +	__le16 *dbuf16 = dbuf;
>  	const __le32 *sbuf32 = sbuf;
>  	unsigned int x;
>  	u16 val16;
> @@ -333,14 +333,15 @@ static void drm_fb_xrgb8888_to_rgb565_line(void *dbuf, const void *sbuf, unsigne
>  		val16 = ((pix & 0x00F80000) >> 8) |
>  			((pix & 0x0000FC00) >> 5) |
>  			((pix & 0x000000F8) >> 3);
> -		dbuf16[x] = val16;
> +		dbuf16[x] = cpu_to_le16(val16);
>  	}
>  }
>  
> +/* TODO: implement this helper as conversion to RGB565|BIG_ENDIAN */
>  static void drm_fb_xrgb8888_to_rgb565_swab_line(void *dbuf, const void *sbuf,
>  						unsigned int pixels)
>  {
> -	u16 *dbuf16 = dbuf;
> +	__le16 *dbuf16 = dbuf;
>  	const __le32 *sbuf32 = sbuf;
>  	unsigned int x;
>  	u16 val16;
> @@ -351,7 +352,7 @@ static void drm_fb_xrgb8888_to_rgb565_swab_line(void *dbuf, const void *sbuf,
>  		val16 = ((pix & 0x00F80000) >> 8) |
>  			((pix & 0x0000FC00) >> 5) |
>  			((pix & 0x000000F8) >> 3);
> -		dbuf16[x] = swab16(val16);
> +		dbuf16[x] = cpu_to_le16(swab16(val16));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
> index e7c49e6d3f6d..04fe373c9d97 100644
> --- a/drivers/gpu/drm/tests/drm_format_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
> @@ -264,6 +264,21 @@ static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
>  	return dst_pitch * drm_rect_height(clip);
>  }
>  
> +static u16 *le16buf_to_cpu(struct kunit *test, const __le16 *buf, size_t buf_size)
> +{
> +	u16 *dst = NULL;
> +	int n;
> +
> +	dst = kunit_kzalloc(test, sizeof(*dst) * buf_size, GFP_KERNEL);
> +	if (!dst)
> +		return NULL;
> +
> +	for (n = 0; n < buf_size; n++)
> +		dst[n] = le16_to_cpu(buf[n]);
> +
> +	return dst;
> +}
> +
>  static u32 *le32buf_to_cpu(struct kunit *test, const u32 *buf, size_t buf_size)
>  {
>  	u32 *dst = NULL;
> @@ -368,7 +383,7 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
>  	const struct convert_xrgb8888_case *params = test->param_value;
>  	const struct convert_to_rgb565_result *result = &params->rgb565_result;
>  	size_t dst_size;
> -	__u16 *buf = NULL;
> +	u16 *buf = NULL;
>  	__le32 *xrgb8888 = NULL;
>  	struct iosys_map dst, src;
>  
> @@ -390,9 +405,12 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
>  	iosys_map_set_vaddr(&src, xrgb8888);
>  
>  	drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, false);
> +	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
>  	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
>  
> +	buf = dst.vaddr; /* restore original value of buf */
>  	drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, true);
> +	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
>  	KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, dst_size), 0);
>  }
>