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

[José Expósito]

On Tue, Dec 20, 2022 at 05:11:36PM +0100, 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 <tzimmerm...@suse.de>

Reviewed-by: José Expósito <jose.exposit...@gmail.com>

> ---
>  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);
>  }
>  
> -- 
> 2.39.0
>