[PATCH libdrm v2 00/10] Big-endian fixes

[PATCH libdrm v2 00/10] Big-endian fixes

[Geert Uytterhoeven]

	Hi all,

This patch series fixes some endianness issues in libdrm.
It has been tested on ARAnyM using a work-in-progress Atari DRM driver.

Changes compared to v1:
  - Consider arm, aarch64, microblaze, s390, and sh in endianness
    checks,
  - Add Acked-by,
  - Add swap32() intermediate helper,
  - Fix 16 bpp formats on big-endian,
  - Add support for big-endian XRGB1555 and RGB565,
  - Fix printing of big-endian fourcc values,
  - Fix pwetty on big-endian.

Please refer to [2] for related Linux DRM patches and background
information.

Thanks for your comments!

[1] "[PATCH RFC libdrm 0/2] Big-endian fixes"
    https://lore.kernel.org/r/cover.1646684158.git.geert@linux-m68k.org
[2] "[PATCH v3 00/10] drm: Add support for low-color frame buffer formats"
    https://lore.kernel.org/r/cover.1657294931.git.geert@linux-m68k.org

Geert Uytterhoeven (11):
  intel: Improve checks for big-endian
  util: Fix 32 bpp patterns on big-endian
  util: Fix 16 bpp patterns on big-endian
  util: Add missing big-endian RGB16 frame buffer formats
  modetest: Fix printing of big-endian fourcc values
  modetest: Add support for parsing big-endian formats
  util: Add test pattern support for big-endian XRGB1555/RGB565
  util: Fix pwetty on big-endian
  util: Add pwetty support for big-endian RGB565
  modetest: Add support for big-endian XRGB1555/RGB565

 intel/uthash.h            |   2 +-
 tests/modetest/buffers.c  |   4 ++
 tests/modetest/modetest.c |  25 ++++---
 tests/util/format.c       |   3 +
 tests/util/pattern.c      | 115 +++++++++++++++++++++++++++++++-------
 5 files changed, 115 insertions(+), 29 deletions(-)

-- 
2.25.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH libdrm v2 01/10] intel: Improve checks for big-endian

[Geert Uytterhoeven]

  - sparc64-linux-gnu-gcc does not define __BIG_ENDIAN__ or SPARC, but
    does define __sparc__, hence replace the check for SPARC by a check
    for __sparc__,
  - powerpc{,64,64}-linux-gnu-gcc does not define __ppc__ or __ppc64__,
    but does define __BIG_ENDIAN__.
    powerpc64le-linux-gnu-gcc does not define __ppc__ or __ppc64__, but
    does define __LITTLE_ENDIAN__.
    Hence remove the checks for __ppc__ and __ppc64__.
  - arm-linux-gnueabi-gcc and aarch64-linux-gnu-gcc do not define
    __BIG_ENDIAN__ for targets in big-endian mode, but do define
    __ARM_BIG_ENDIAN, so add a check for the latter,
  - m68k-linux-gnu-gcc does not define __BIG_ENDIAN__, but does define
    __mc68000__, so add a check for the latter,
  - mips{,64}-linux-gnu{,abi64}-gcc does not define __BIG_ENDIAN__, but
    does define __MIPSEB__, so add a check for the latter,
  - s390x-linux-gnu-gcc does not define __BIG_ENDIAN__, but does define
    __s390__, so add a check for the latter,
  - hppa{,64}-linux-gnu-gcc, microblaze-linux-gcc, and sh4-linux-gnu-gcc
    in big-endian mode do define __BIG_ENDIAN__, and thus should work
    out-of-the-box.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Untested due to lack of hardware.

v2:
  - Add arm, aarch64, microblaze, s390, and sh.
---
 intel/uthash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/intel/uthash.h b/intel/uthash.h
index 45d1f9fc12a1d6f9..a8465f23ff9fbcac 100644
--- a/intel/uthash.h
+++ b/intel/uthash.h
@@ -648,7 +648,7 @@ do {
 #define MUR_PLUS2_ALIGNED(p) (((unsigned long)p & 3UL) == 2UL)
 #define MUR_PLUS3_ALIGNED(p) (((unsigned long)p & 3UL) == 3UL)
 #define WP(p) ((uint32_t*)((unsigned long)(p) & ~3UL))
-#if (defined(__BIG_ENDIAN__) || defined(SPARC) || defined(__ppc__) || defined(__ppc64__))
+#if (defined(__BIG_ENDIAN__) || defined(__ARM_BIG_ENDIAN) || defined(__mc68000__) || defined(__MIPSEB__) || defined(__s390__) || defined(__sparc__))
 #define MUR_THREE_ONE(p) ((((*WP(p))&0x00ffffff) << 8) | (((*(WP(p)+1))&0xff000000) >> 24))
 #define MUR_TWO_TWO(p)   ((((*WP(p))&0x0000ffff) <<16) | (((*(WP(p)+1))&0xffff0000) >> 16))
 #define MUR_ONE_THREE(p) ((((*WP(p))&0x000000ff) <<24) | (((*(WP(p)+1))&0xffffff00) >>  8))
-- 
2.25.1

[PATCH libdrm v2 02/10] util: Fix 32 bpp patterns on big-endian

[Geert Uytterhoeven]

DRM formats are defined to be little-endian, unless the
DRM_FORMAT_BIG_ENDIAN flag is set.  Hence writes of multi-byte pixel
values need to take endianness into account.

Introduce a swap32() helper to byteswap 32-bit values, and a
cpu_to_le32() helper to convert 32-bit values from CPU-endian to
little-endian, and use the latter in the various pattern fill functions
for 32-bit formats.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---
Works now with Linux' drm_fb_xrgb8888_to_rgb332_line(), which uses
le32_to_cpu() to read pixel values from memory.

v2:
  - Add Acked-by,
  - Add swap32() intermediate helper,
  - Add __ARM_BIG_ENDIAN and __s390__.
---
 tests/util/pattern.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 631114563fa011c2..26e0614bd4faa620 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -61,6 +61,20 @@ struct color_yuv {
 	  .u = MAKE_YUV_601_U(r, g, b), \
 	  .v = MAKE_YUV_601_V(r, g, b) }
 
+static inline uint32_t swap32(uint32_t x)
+{
+	return ((x & 0x000000ffU) << 24) |
+	       ((x & 0x0000ff00U) <<  8) |
+	       ((x & 0x00ff0000U) >>  8) |
+	       ((x & 0xff000000U) >> 24);
+}
+
+#if defined(__BIG_ENDIAN__) || defined(__ARM_BIG_ENDIAN) || defined(__mc68000__) || defined(__MIPSEB__) || defined(__s390__) || defined(__sparc__)
+#define cpu_to_le32(x)	swap32(x)
+#else
+#define cpu_to_le32(x)	(x)
+#endif
+
 /* This function takes 8-bit color values */
 static inline uint32_t shiftcolor8(const struct util_color_component *comp,
 				  uint32_t value)
@@ -520,26 +534,26 @@ static void fill_smpte_rgb32(const struct util_rgb_info *rgb, void *mem,
 
 	for (y = 0; y < height * 6 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint32_t *)mem)[x] = colors_top[x * 7 / width];
+			((uint32_t *)mem)[x] = cpu_to_le32(colors_top[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint32_t *)mem)[x] = colors_middle[x * 7 / width];
+			((uint32_t *)mem)[x] = cpu_to_le32(colors_middle[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height; ++y) {
 		for (x = 0; x < width * 5 / 7; ++x)
 			((uint32_t *)mem)[x] =
-				colors_bottom[x * 4 / (width * 5 / 7)];
+				cpu_to_le32(colors_bottom[x * 4 / (width * 5 / 7)]);
 		for (; x < width * 6 / 7; ++x)
 			((uint32_t *)mem)[x] =
-				colors_bottom[(x - width * 5 / 7) * 3
-					      / (width / 7) + 4];
+				cpu_to_le32(colors_bottom[(x - width * 5 / 7) * 3
+							  / (width / 7) + 4]);
 		for (; x < width; ++x)
-			((uint32_t *)mem)[x] = colors_bottom[7];
+			((uint32_t *)mem)[x] = cpu_to_le32(colors_bottom[7]);
 		mem += stride;
 	}
 }
@@ -1307,7 +1321,7 @@ static void fill_tiles_rgb32(const struct util_format_info *info, void *mem,
 					  (rgb32 >> 8) & 0xff, rgb32 & 0xff,
 					  alpha);
 
-			((uint32_t *)mem)[x] = color;
+			((uint32_t *)mem)[x] = cpu_to_le32(color);
 		}
 		mem += stride;
 	}
@@ -1454,7 +1468,7 @@ static void fill_gradient_rgb32(const struct util_rgb_info *rgb,
 
 		for (j = 0; j < width / 2; j++) {
 			uint32_t value = MAKE_RGBA10(rgb, j & 0x3ff, j & 0x3ff, j & 0x3ff, 0);
-			row[2*j] = row[2*j+1] = value;
+			row[2*j] = row[2*j+1] = cpu_to_le32(value);
 		}
 		mem += stride;
 	}
@@ -1464,7 +1478,7 @@ static void fill_gradient_rgb32(const struct util_rgb_info *rgb,
 
 		for (j = 0; j < width / 2; j++) {
 			uint32_t value = MAKE_RGBA10(rgb, j & 0x3fc, j & 0x3fc, j & 0x3fc, 0);
-			row[2*j] = row[2*j+1] = value;
+			row[2*j] = row[2*j+1] = cpu_to_le32(value);
 		}
 		mem += stride;
 	}
-- 
2.25.1

[PATCH libdrm v2 03/10] util: Fix 16 bpp patterns on big-endian

[Geert Uytterhoeven]

DRM formats are defined to be little-endian, unless the
DRM_FORMAT_BIG_ENDIAN flag is set.  Hence writes of multi-byte pixel
values need to take endianness into account.

Introduce a swap16() helper to byteswap 16-bit values, and a
cpu_to_le16() helper to convert 16-bit values from CPU-endian to
little-endian, and use the latter in the various pattern fill functions
for 16-bit formats.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Works now with both Linux' drm_fb_swab() and my own
drm_fb_rgb565_to_rgb332_line() (the latter uses le16_to_cpu() to read
pixel values from memory).

v2:
  - New.
---
 tests/util/pattern.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 26e0614bd4faa620..c0bd382fbced90fb 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -61,6 +61,11 @@ struct color_yuv {
 	  .u = MAKE_YUV_601_U(r, g, b), \
 	  .v = MAKE_YUV_601_V(r, g, b) }
 
+static inline uint16_t swap16(uint16_t x)
+{
+	return ((x & 0x00ffU) << 8) | ((x & 0xff00U) >> 8);
+}
+
 static inline uint32_t swap32(uint32_t x)
 {
 	return ((x & 0x000000ffU) << 24) |
@@ -70,8 +75,10 @@ static inline uint32_t swap32(uint32_t x)
 }
 
 #if defined(__BIG_ENDIAN__) || defined(__ARM_BIG_ENDIAN) || defined(__mc68000__) || defined(__MIPSEB__) || defined(__s390__) || defined(__sparc__)
+#define cpu_to_le16(x)	swap16(x)
 #define cpu_to_le32(x)	swap32(x)
 #else
+#define cpu_to_le16(x)	(x)
 #define cpu_to_le32(x)	(x)
 #endif
 
@@ -410,26 +417,26 @@ static void fill_smpte_rgb16(const struct util_rgb_info *rgb, void *mem,
 
 	for (y = 0; y < height * 6 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint16_t *)mem)[x] = colors_top[x * 7 / width];
+			((uint16_t *)mem)[x] = cpu_to_le16(colors_top[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint16_t *)mem)[x] = colors_middle[x * 7 / width];
+			((uint16_t *)mem)[x] = cpu_to_le16(colors_middle[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height; ++y) {
 		for (x = 0; x < width * 5 / 7; ++x)
 			((uint16_t *)mem)[x] =
-				colors_bottom[x * 4 / (width * 5 / 7)];
+				cpu_to_le16(colors_bottom[x * 4 / (width * 5 / 7)]);
 		for (; x < width * 6 / 7; ++x)
 			((uint16_t *)mem)[x] =
-				colors_bottom[(x - width * 5 / 7) * 3
-					      / (width / 7) + 4];
+				cpu_to_le16(colors_bottom[(x - width * 5 / 7) * 3
+							  / (width / 7) + 4]);
 		for (; x < width; ++x)
-			((uint16_t *)mem)[x] = colors_bottom[7];
+			((uint16_t *)mem)[x] = cpu_to_le16(colors_bottom[7]);
 		mem += stride;
 	}
 }
@@ -1272,7 +1279,7 @@ static void fill_tiles_rgb16(const struct util_format_info *info, void *mem,
 					  (rgb32 >> 8) & 0xff, rgb32 & 0xff,
 					  255);
 
-			((uint16_t *)mem)[x] = color;
+			((uint16_t *)mem)[x] = cpu_to_le16(color);
 		}
 		mem += stride;
 	}
-- 
2.25.1

[PATCH libdrm v2 04/10] util: Add missing big-endian RGB16 frame buffer formats

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Any better suggestion than appending "be"?

v2:
  - New.
---
 tests/util/format.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/util/format.c b/tests/util/format.c
index a5464de6fc1ac70f..42a652c9a402a654 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -76,6 +76,9 @@ static const struct util_format_info format_info[] = {
 	{ DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
 	{ DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
 	{ DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
+	/* Big-endian RGB16 */
+	{ DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, "XR15be", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
+	{ DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, "RG16be", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
 	/* RGB24 */
 	{ DRM_FORMAT_BGR888, "BG24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) },
 	{ DRM_FORMAT_RGB888, "RG24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) },
-- 
2.25.1

Re: [PATCH libdrm v2 04/10] util: Add missing big-endian RGB16 frame buffer formats

[Ville Syrjälä]

On Fri, Jul 08, 2022 at 08:21:43PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Any better suggestion than appending "be"?
> 
> v2:
>   - New.
> ---
>  tests/util/format.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/util/format.c b/tests/util/format.c
> index a5464de6fc1ac70f..42a652c9a402a654 100644
> --- a/tests/util/format.c
> +++ b/tests/util/format.c
> @@ -76,6 +76,9 @@ static const struct util_format_info format_info[] = {
>  	{ DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
>  	{ DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
>  	{ DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
> +	/* Big-endian RGB16 */
> +	{ DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, "XR15be", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
> +	{ DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, "RG16be", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },

How about just stripping the BE bit in util_format_info_find()
so we don't have to duplicate the entries in the table?

I guess util_format_fourcc() would end up being more a bit
complicated since you'd have to massage the string.

But I'm not sure why we even store the fourcc as a string in
the table anyway. Could just add some kind of string_to_fourcc()
thingy instead AFAICS.

>  	/* RGB24 */
>  	{ DRM_FORMAT_BGR888, "BG24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) },
>  	{ DRM_FORMAT_RGB888, "RG24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) },
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [PATCH libdrm v2 04/10] util: Add missing big-endian RGB16 frame buffer formats

[Geert Uytterhoeven]

Hi Ville,

On Mon, Jul 11, 2022 at 2:17 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, Jul 08, 2022 at 08:21:43PM +0200, Geert Uytterhoeven wrote:
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> > Any better suggestion than appending "be"?
> >
> > v2:
> >   - New.

> > --- a/tests/util/format.c
> > +++ b/tests/util/format.c
> > @@ -76,6 +76,9 @@ static const struct util_format_info format_info[] = {
> >       { DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
> >       { DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
> >       { DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
> > +     /* Big-endian RGB16 */
> > +     { DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, "XR15be", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
> > +     { DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, "RG16be", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
>
> How about just stripping the BE bit in util_format_info_find()
> so we don't have to duplicate the entries in the table?

There is no need to support big-endian variants of all formats.
E.g. big-endian [AX]RGB8888 just map to little-endian BGR[AX]8888.

XRGB1555 and RGB565 are probably the only RGB formats we care about.
Or perhaps some of the *30 formats?

> I guess util_format_fourcc() would end up being more a bit
> complicated since you'd have to massage the string.

True.

> But I'm not sure why we even store the fourcc as a string in
> the table anyway. Could just add some kind of string_to_fourcc()
> thingy instead AFAICS.

I guess that can be done.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH libdrm v2 04/10] util: Add missing big-endian RGB16 frame buffer formats

[Michel Dänzer]

On 2022-07-11 14:34, Geert Uytterhoeven wrote:
> Hi Ville,
> 
> On Mon, Jul 11, 2022 at 2:17 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Fri, Jul 08, 2022 at 08:21:43PM +0200, Geert Uytterhoeven wrote:
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> Any better suggestion than appending "be"?
>>>
>>> v2:
>>>   - New.
> 
>>> --- a/tests/util/format.c
>>> +++ b/tests/util/format.c
>>> @@ -76,6 +76,9 @@ static const struct util_format_info format_info[] = {
>>>       { DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
>>>       { DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
>>>       { DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
>>> +     /* Big-endian RGB16 */
>>> +     { DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, "XR15be", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
>>> +     { DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, "RG16be", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
>>
>> How about just stripping the BE bit in util_format_info_find()
>> so we don't have to duplicate the entries in the table?
> 
> There is no need to support big-endian variants of all formats.
> E.g. big-endian [AX]RGB8888 just map to little-endian BGR[AX]8888.
> 
> XRGB1555 and RGB565 are probably the only RGB formats we care about.
> Or perhaps some of the *30 formats?

I'd say all of those, and any other formats with components straddling byte boundaries (including formats with multi-byte components).


P.S. libdrm changes are now reviewed and merged as GitLab merge requests: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests

I guess CONTRIBUTING.rst could make this clearer.

-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer

Re: [PATCH libdrm v2 04/10] util: Add missing big-endian RGB16 frame buffer formats

[Geert Uytterhoeven]

Hi Ville,

On Mon, Jul 11, 2022 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, Jul 11, 2022 at 2:17 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Jul 08, 2022 at 08:21:43PM +0200, Geert Uytterhoeven wrote:
> > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > ---
> > > Any better suggestion than appending "be"?
> > >
> > > v2:
> > >   - New.
>
> > > --- a/tests/util/format.c
> > > +++ b/tests/util/format.c
> > > @@ -76,6 +76,9 @@ static const struct util_format_info format_info[] = {
> > >       { DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
> > >       { DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
> > >       { DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
> > > +     /* Big-endian RGB16 */
> > > +     { DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, "XR15be", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
> > > +     { DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, "RG16be", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
> >
> > But I'm not sure why we even store the fourcc as a string in
> > the table anyway. Could just add some kind of string_to_fourcc()
> > thingy instead AFAICS.
>
> I guess that can be done.

Nowadays we have drmGetFormatName(), which returns an allocated string
with the name for a format code.

Using that helper in string_to_fourcc() would mean looping over the
table, and for each entry, calling drmGetFormatName(), comparing the
name, and freeing the name again.
Would that be acceptable?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH libdrm v2 05/10] modetest: Fix printing of big-endian fourcc values

[Geert Uytterhoeven]

Big-endian fourcc values have the MSB set, as that is the
DRM_FORMAT_BIG_ENDIAN flag.  Hence printing the last byte unmodified
leads to weird characters.

Fix this by stripping the DRM_FORMAT_BIG_ENDIAN flag, and appending "be"
for big-endian formats.

Sample impact:

     Planes:
     id     crtc    fb      CRTC x,y        x,y     gamma size      possible crtcs
     32     34      36      0,0             0,0     0               0x00000001
    -  formats: C1   C2   C4   C8   RG16 RG1� XR24
    +  formats: C1   C2   C4   C8   RG16 RG16be XR24
       props:
            8 type:
                    flags: immutable enum
                    enums: Overlay=0 Primary=1 Cursor=2
                    value: 1
            30 IN_FORMATS:
                    flags: immutable blob
                    blobs:

                    value:
                            00000001000000000000000700000018
                            00000001000000382020314320203243
                            202034432020384336314752b6314752
                            3432525800000000000000000000007f
                            00000000000000000000000000000000
                    in_formats blob decoded:
                             C1  :  LINEAR
                             C2  :  LINEAR
                             C4  :  LINEAR
                             C8  :  LINEAR
                             RG16:  LINEAR
    -                        RG1�:  LINEAR
    +                        RG16be:  LINEAR
                             XR24:  LINEAR

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/modetest/modetest.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 1d87046f0fdfaf24..f70043d1e6815497 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -198,11 +198,15 @@ static bit_name_fn(mode_flag)
 
 static void dump_fourcc(uint32_t fourcc)
 {
-	printf(" %c%c%c%c",
+	bool be = fourcc & DRM_FORMAT_BIG_ENDIAN;
+
+	fourcc &= ~DRM_FORMAT_BIG_ENDIAN;
+	printf(" %c%c%c%c%s",
 		fourcc,
 		fourcc >> 8,
 		fourcc >> 16,
-		fourcc >> 24);
+		fourcc >> 24,
+		be ? "be" : "");
 }
 
 static void dump_encoders(struct device *dev)
-- 
2.25.1

[PATCH libdrm v2 06/10] modetest: Add support for parsing big-endian formats

[Geert Uytterhoeven]

When specifying a frame buffer format like "RG16be" (big-endian RG16),
modetest still uses the little-endian variant, as the format string is
truncated to four characters.

Fix this by increasing the format string size to 7 bytes (6 characters +
NUL terminator).

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/modetest/modetest.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index f70043d1e6815497..7c6c21ef5174c41a 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -821,7 +821,7 @@ struct pipe_arg {
 	unsigned int num_cons;
 	uint32_t crtc_id;
 	char mode_str[64];
-	char format_str[5];
+	char format_str[7];
 	float vrefresh;
 	unsigned int fourcc;
 	drmModeModeInfo *mode;
@@ -843,7 +843,7 @@ struct plane_arg {
 	unsigned int old_fb_id;
 	struct bo *bo;
 	struct bo *old_bo;
-	char format_str[5]; /* need to leave room for terminating \0 */
+	char format_str[7]; /* need to leave room for "be" and terminating \0 */
 	unsigned int fourcc;
 };
 
@@ -1917,8 +1917,8 @@ static int parse_connector(struct pipe_arg *pipe, const char *arg)
 	}
 
 	if (*p == '@') {
-		strncpy(pipe->format_str, p + 1, 4);
-		pipe->format_str[4] = '\0';
+		strncpy(pipe->format_str, p + 1, 6);
+		pipe->format_str[6] = '\0';
 	}
 
 	pipe->fourcc = util_format_fourcc(pipe->format_str);
@@ -1970,8 +1970,8 @@ static int parse_plane(struct plane_arg *plane, const char *p)
 	}
 
 	if (*end == '@') {
-		strncpy(plane->format_str, end + 1, 4);
-		plane->format_str[4] = '\0';
+		strncpy(plane->format_str, end + 1, 6);
+		plane->format_str[6] = '\0';
 	} else {
 		strcpy(plane->format_str, "XR24");
 	}
-- 
2.25.1

[PATCH libdrm v2 07/10] util: Add test pattern support for big-endian XRGB1555/RGB565

[Geert Uytterhoeven]

Add support for drawing the SMPTE and tiles test patterns in buffers
using big-endian formats.

For now this is limited to XRGB1555 and RGB565, which are the most
common big-endian formats.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/util/pattern.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index c0bd382fbced90fb..e693892bb3cf93c1 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -23,6 +23,7 @@
  * IN THE SOFTWARE.
  */
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -75,13 +76,17 @@ static inline uint32_t swap32(uint32_t x)
 }
 
 #if defined(__BIG_ENDIAN__) || defined(__ARM_BIG_ENDIAN) || defined(__mc68000__) || defined(__MIPSEB__) || defined(__s390__) || defined(__sparc__)
+#define cpu_to_be16(x)	(x)
 #define cpu_to_le16(x)	swap16(x)
 #define cpu_to_le32(x)	swap32(x)
 #else
+#define cpu_to_be16(x)	swap16(x)
 #define cpu_to_le16(x)	(x)
 #define cpu_to_le32(x)	(x)
 #endif
 
+#define cpu_to_fb16(x)	(fb_be ? cpu_to_be16(x) : cpu_to_le16(x))
+
 /* This function takes 8-bit color values */
 static inline uint32_t shiftcolor8(const struct util_color_component *comp,
 				  uint32_t value)
@@ -382,7 +387,7 @@ static void fill_smpte_yuv_packed(const struct util_yuv_info *yuv, void *mem,
 
 static void fill_smpte_rgb16(const struct util_rgb_info *rgb, void *mem,
 			     unsigned int width, unsigned int height,
-			     unsigned int stride)
+			     unsigned int stride, bool fb_be)
 {
 	const uint16_t colors_top[] = {
 		MAKE_RGBA(rgb, 192, 192, 192, 255),	/* grey */
@@ -417,26 +422,26 @@ static void fill_smpte_rgb16(const struct util_rgb_info *rgb, void *mem,
 
 	for (y = 0; y < height * 6 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint16_t *)mem)[x] = cpu_to_le16(colors_top[x * 7 / width]);
+			((uint16_t *)mem)[x] = cpu_to_fb16(colors_top[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint16_t *)mem)[x] = cpu_to_le16(colors_middle[x * 7 / width]);
+			((uint16_t *)mem)[x] = cpu_to_fb16(colors_middle[x * 7 / width]);
 		mem += stride;
 	}
 
 	for (; y < height; ++y) {
 		for (x = 0; x < width * 5 / 7; ++x)
 			((uint16_t *)mem)[x] =
-				cpu_to_le16(colors_bottom[x * 4 / (width * 5 / 7)]);
+				cpu_to_fb16(colors_bottom[x * 4 / (width * 5 / 7)]);
 		for (; x < width * 6 / 7; ++x)
 			((uint16_t *)mem)[x] =
-				cpu_to_le16(colors_bottom[(x - width * 5 / 7) * 3
+				cpu_to_fb16(colors_bottom[(x - width * 5 / 7) * 3
 							  / (width / 7) + 4]);
 		for (; x < width; ++x)
-			((uint16_t *)mem)[x] = cpu_to_le16(colors_bottom[7]);
+			((uint16_t *)mem)[x] = cpu_to_fb16(colors_bottom[7]);
 		mem += stride;
 	}
 }
@@ -1081,9 +1086,11 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 	case DRM_FORMAT_BGRA4444:
 	case DRM_FORMAT_BGRX4444:
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
 	case DRM_FORMAT_RGBA5551:
@@ -1091,7 +1098,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 	case DRM_FORMAT_BGRA5551:
 	case DRM_FORMAT_BGRX5551:
 		return fill_smpte_rgb16(&info->rgb, planes[0],
-					width, height, stride);
+					width, height, stride,
+					info->format & DRM_FORMAT_BIG_ENDIAN);
 
 	case DRM_FORMAT_BGR888:
 	case DRM_FORMAT_RGB888:
@@ -1263,7 +1271,7 @@ static void fill_tiles_yuv_packed(const struct util_format_info *info,
 
 static void fill_tiles_rgb16(const struct util_format_info *info, void *mem,
 			     unsigned int width, unsigned int height,
-			     unsigned int stride)
+			     unsigned int stride, bool fb_be)
 {
 	const struct util_rgb_info *rgb = &info->rgb;
 	void *mem_base = mem;
@@ -1279,7 +1287,7 @@ static void fill_tiles_rgb16(const struct util_format_info *info, void *mem,
 					  (rgb32 >> 8) & 0xff, rgb32 & 0xff,
 					  255);
 
-			((uint16_t *)mem)[x] = cpu_to_le16(color);
+			((uint16_t *)mem)[x] = cpu_to_fb16(color);
 		}
 		mem += stride;
 	}
@@ -1401,9 +1409,11 @@ static void fill_tiles(const struct util_format_info *info, void *planes[3],
 	case DRM_FORMAT_BGRA4444:
 	case DRM_FORMAT_BGRX4444:
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
 	case DRM_FORMAT_RGBA5551:
@@ -1411,7 +1421,8 @@ static void fill_tiles(const struct util_format_info *info, void *planes[3],
 	case DRM_FORMAT_BGRA5551:
 	case DRM_FORMAT_BGRX5551:
 		return fill_tiles_rgb16(info, planes[0],
-					width, height, stride);
+					width, height, stride,
+					info->format & DRM_FORMAT_BIG_ENDIAN);
 
 	case DRM_FORMAT_BGR888:
 	case DRM_FORMAT_RGB888:
-- 
2.25.1

[PATCH libdrm v2 08/10] util: Fix pwetty on big-endian

[Geert Uytterhoeven]

Cairo always uses native byte order for rendering.

Hence if the byte order of the frame buffer differs from the byte order
of the CPU, the frame buffer contents need to be byteswapped twice: once
before rendering, to convert to native byte order, and a second time
after rendering, to restore the frame buffer format's byte order.

Note that byte swapping is not done for ARGB32 formats, as for these
formats, byte order only affects the order of the red, green, and blue
channels, which we do not care about here.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
This is ugly, but I see no better solution.

v2:
  - RGB30 is untested.
---
 tests/util/pattern.c | 54 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index e693892bb3cf93c1..627f402be1d02e1c 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -76,13 +76,15 @@ static inline uint32_t swap32(uint32_t x)
 }
 
 #if defined(__BIG_ENDIAN__) || defined(__ARM_BIG_ENDIAN) || defined(__mc68000__) || defined(__MIPSEB__) || defined(__s390__) || defined(__sparc__)
-#define cpu_to_be16(x)	(x)
-#define cpu_to_le16(x)	swap16(x)
-#define cpu_to_le32(x)	swap32(x)
+#define cpu_to_be16(x)			(x)
+#define cpu_to_le16(x)			swap16(x)
+#define cpu_to_le32(x)			swap32(x)
+#define fb_foreign_endian(format)	(!((format) & DRM_FORMAT_BIG_ENDIAN))
 #else
-#define cpu_to_be16(x)	swap16(x)
-#define cpu_to_le16(x)	(x)
-#define cpu_to_le32(x)	(x)
+#define cpu_to_be16(x)			swap16(x)
+#define cpu_to_le16(x)			(x)
+#define cpu_to_le32(x)			(x)
+#define fb_foreign_endian(format)	((format) & DRM_FORMAT_BIG_ENDIAN)
 #endif
 
 #define cpu_to_fb16(x)	(fb_be ? cpu_to_be16(x) : cpu_to_le16(x))
@@ -1133,6 +1135,30 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 #define GREEN 1
 #define BLUE  0
 
+static void byteswap_buffer16(void *mem, unsigned int width, unsigned int height,
+			      unsigned int stride)
+{
+	unsigned int x, y;
+
+	for (y = 0; y < height; ++y) {
+		for (x = 0; x < width; ++x)
+			((uint16_t *)mem)[x] = swap16(((uint16_t *)mem)[x]);
+		mem += stride;
+	}
+}
+
+static void byteswap_buffer32(void *mem, unsigned int width, unsigned int height,
+			      unsigned int stride)
+{
+	unsigned int x, y;
+
+	for (y = 0; y < height; ++y) {
+		for (x = 0; x < width; ++x)
+			((uint32_t *)mem)[x] = swap32(((uint32_t *)mem)[x]);
+		mem += stride;
+	}
+}
+
 static void make_pwetty(void *data, unsigned int width, unsigned int height,
 			unsigned int stride, uint32_t format)
 {
@@ -1140,6 +1166,8 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height,
 	cairo_surface_t *surface;
 	cairo_t *cr;
 	cairo_format_t cairo_format;
+	bool swap16 = false;
+	bool swap32 = false;
 
 	/* we can ignore the order of R,G,B channels */
 	switch (format) {
@@ -1152,6 +1180,7 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height,
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_BGR565:
 		cairo_format = CAIRO_FORMAT_RGB16_565;
+		swap16 = fb_foreign_endian(format);
 		break;
 #if CAIRO_VERSION_MAJOR > 1 || (CAIRO_VERSION_MAJOR == 1 && CAIRO_VERSION_MINOR >= 12)
 	case DRM_FORMAT_ARGB2101010:
@@ -1159,12 +1188,19 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height,
 	case DRM_FORMAT_ABGR2101010:
 	case DRM_FORMAT_XBGR2101010:
 		cairo_format = CAIRO_FORMAT_RGB30;
+		swap32 = fb_foreign_endian(format);
 		break;
 #endif
 	default:
 		return;
 	}
 
+	/* Cairo uses native byte order, so we may have to byteswap before... */
+	if (swap16)
+		byteswap_buffer16(data, width, height, stride);
+	if (swap32)
+		byteswap_buffer32(data, width, height, stride);
+
 	surface = cairo_image_surface_create_for_data(data,
 						      cairo_format,
 						      width, height,
@@ -1200,6 +1236,12 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height,
 		}
 
 	cairo_destroy(cr);
+
+	/* ... and after */
+	if (swap16)
+		byteswap_buffer16(data, width, height, stride);
+	if (swap32)
+		byteswap_buffer32(data, width, height, stride);
 #endif
 }
 
-- 
2.25.1

[PATCH libdrm v2 09/10] util: Add pwetty support for big-endian RGB565

[Geert Uytterhoeven]

Add support for rendering the crosshairs in a buffer using the
big-endian RGB565 format.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/util/pattern.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 627f402be1d02e1c..1222b47ea3f14cd0 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -1178,6 +1178,7 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height,
 		cairo_format = CAIRO_FORMAT_ARGB32;
 		break;
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_BGR565:
 		cairo_format = CAIRO_FORMAT_RGB16_565;
 		swap16 = fb_foreign_endian(format);
-- 
2.25.1

[PATCH libdrm v2 10/10] modetest: Add support for big-endian XRGB1555/RGB565

[Geert Uytterhoeven]

Add support for creating buffers using big-endian formats.

For now this is limited to XRGB1555 and RGB565, which are the most
common big-endian formats.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/modetest/buffers.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index af7f60b4fb4d09ad..aa4a976a7eb19dbd 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -167,6 +167,7 @@ bo_create(int fd, unsigned int format,
 	case DRM_FORMAT_BGRX4444:
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
 	case DRM_FORMAT_RGBA5551:
@@ -174,6 +175,7 @@ bo_create(int fd, unsigned int format,
 	case DRM_FORMAT_BGRA5551:
 	case DRM_FORMAT_BGRX5551:
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
@@ -309,6 +311,7 @@ bo_create(int fd, unsigned int format,
 	case DRM_FORMAT_BGRX4444:
 	case DRM_FORMAT_ARGB1555:
 	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_ABGR1555:
 	case DRM_FORMAT_XBGR1555:
 	case DRM_FORMAT_RGBA5551:
@@ -316,6 +319,7 @@ bo_create(int fd, unsigned int format,
 	case DRM_FORMAT_BGRA5551:
 	case DRM_FORMAT_BGRX5551:
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_BGR888:
 	case DRM_FORMAT_RGB888:
-- 
2.25.1