[RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

Ok, this is really a kickstart for a discussion.  While working on
graphics support for virtual machines on ppc64 (which exists in both
little and big endian variants) I've figured the comments in the header
file don't match reality.  They are not considered little endian (as
suggested by the comments) but in practice are used as native endian.

So, go update the comments.

This patch switches all 32bpp / 8 bpc formats over to native endian.
Those used/supported by ppc64 virtual machines (virtio-gpu/bochs-drm
drivers).

Given that DRM_FORMAT_BIG_ENDIAN isn't used anywhere in the codebase
I suspect this problem applies to more formats.  When looking at
drm_mode_legacy_fb_format it seems *all* RGB formats are actually
native endian not little endian.

Dunno where we stand in terms of YCbCr.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/uapi/drm/drm_fourcc.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 995c8f9..a7fc81d 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -85,15 +85,15 @@ extern "C" {
 #define DRM_FORMAT_BGR888	fourcc_code('B', 'G', '2', '4') /* [23:0] B:G:R little endian */
 
 /* 32 bpp RGB */
-#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
-#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 little endian */
-#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 little endian */
-#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 little endian */
+#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
+#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
+#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
+#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
 
-#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 little endian */
-#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 little endian */
-#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 little endian */
-#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 little endian */
+#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
+#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
+#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
+#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
 
 #define DRM_FORMAT_XRGB2101010	fourcc_code('X', 'R', '3', '0') /* [31:0] x:R:G:B 2:10:10:10 little endian */
 #define DRM_FORMAT_XBGR2101010	fourcc_code('X', 'B', '3', '0') /* [31:0] x:B:G:R 2:10:10:10 little endian */
-- 
2.9.3

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Daniel Vetter]

On Mon, Apr 10, 2017 at 12:12:01PM +0200, Gerd Hoffmann wrote:
> Ok, this is really a kickstart for a discussion.  While working on
> graphics support for virtual machines on ppc64 (which exists in both
> little and big endian variants) I've figured the comments in the header
> file don't match reality.  They are not considered little endian (as
> suggested by the comments) but in practice are used as native endian.
> 
> So, go update the comments.
> 
> This patch switches all 32bpp / 8 bpc formats over to native endian.
> Those used/supported by ppc64 virtual machines (virtio-gpu/bochs-drm
> drivers).
> 
> Given that DRM_FORMAT_BIG_ENDIAN isn't used anywhere in the codebase
> I suspect this problem applies to more formats.  When looking at
> drm_mode_legacy_fb_format it seems *all* RGB formats are actually
> native endian not little endian.
> 
> Dunno where we stand in terms of YCbCr.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I think we should go all-in on this and essentially remove the _BIG_ENDIAN
stuff. But in the end this is to be decided be the people who care about
big endian, afaik that's only you (for the pile of virt drivers we have)
and amdgpu/radeon.
-Daniel

> ---
>  include/uapi/drm/drm_fourcc.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 995c8f9..a7fc81d 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -85,15 +85,15 @@ extern "C" {
>  #define DRM_FORMAT_BGR888	fourcc_code('B', 'G', '2', '4') /* [23:0] B:G:R little endian */
>  
>  /* 32 bpp RGB */
> -#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
> -#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 little endian */
> -#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 little endian */
> -#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 little endian */
> +#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
> +#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
>  
> -#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 little endian */
> -#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 little endian */
> -#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 little endian */
> -#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 little endian */
> +#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
> +#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
>  
>  #define DRM_FORMAT_XRGB2101010	fourcc_code('X', 'R', '3', '0') /* [31:0] x:R:G:B 2:10:10:10 little endian */
>  #define DRM_FORMAT_XBGR2101010	fourcc_code('X', 'B', '3', '0') /* [31:0] x:B:G:R 2:10:10:10 little endian */
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Alex Deucher]

On Mon, Apr 10, 2017 at 8:02 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Apr 10, 2017 at 12:12:01PM +0200, Gerd Hoffmann wrote:
>> Ok, this is really a kickstart for a discussion.  While working on
>> graphics support for virtual machines on ppc64 (which exists in both
>> little and big endian variants) I've figured the comments in the header
>> file don't match reality.  They are not considered little endian (as
>> suggested by the comments) but in practice are used as native endian.
>>
>> So, go update the comments.
>>
>> This patch switches all 32bpp / 8 bpc formats over to native endian.
>> Those used/supported by ppc64 virtual machines (virtio-gpu/bochs-drm
>> drivers).
>>
>> Given that DRM_FORMAT_BIG_ENDIAN isn't used anywhere in the codebase
>> I suspect this problem applies to more formats.  When looking at
>> drm_mode_legacy_fb_format it seems *all* RGB formats are actually
>> native endian not little endian.
>>
>> Dunno where we stand in terms of YCbCr.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: amd-gfx@lists.freedesktop.org
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> I think we should go all-in on this and essentially remove the _BIG_ENDIAN
> stuff. But in the end this is to be decided be the people who care about
> big endian, afaik that's only you (for the pile of virt drivers we have)
> and amdgpu/radeon.

For radeon/amdgpu, we try to be good endian citizens when writing the
code, but as far as testing and support, it's all LE.  For a lot of hw
blocks, the surface format is the least of your worries.

Alex

> -Daniel
>
>> ---
>>  include/uapi/drm/drm_fourcc.h | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 995c8f9..a7fc81d 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -85,15 +85,15 @@ extern "C" {
>>  #define DRM_FORMAT_BGR888    fourcc_code('B', 'G', '2', '4') /* [23:0] B:G:R little endian */
>>
>>  /* 32 bpp RGB */
>> -#define DRM_FORMAT_XRGB8888  fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
>> -#define DRM_FORMAT_XBGR8888  fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 little endian */
>> -#define DRM_FORMAT_RGBX8888  fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 little endian */
>> -#define DRM_FORMAT_BGRX8888  fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 little endian */
>> +#define DRM_FORMAT_XRGB8888  fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
>> +#define DRM_FORMAT_XBGR8888  fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
>> +#define DRM_FORMAT_RGBX8888  fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
>> +#define DRM_FORMAT_BGRX8888  fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
>>
>> -#define DRM_FORMAT_ARGB8888  fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 little endian */
>> -#define DRM_FORMAT_ABGR8888  fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 little endian */
>> -#define DRM_FORMAT_RGBA8888  fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 little endian */
>> -#define DRM_FORMAT_BGRA8888  fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 little endian */
>> +#define DRM_FORMAT_ARGB8888  fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
>> +#define DRM_FORMAT_ABGR8888  fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
>> +#define DRM_FORMAT_RGBA8888  fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
>> +#define DRM_FORMAT_BGRA8888  fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
>>
>>  #define DRM_FORMAT_XRGB2101010       fourcc_code('X', 'R', '3', '0') /* [31:0] x:R:G:B 2:10:10:10 little endian */
>>  #define DRM_FORMAT_XBGR2101010       fourcc_code('X', 'B', '3', '0') /* [31:0] x:B:G:R 2:10:10:10 little endian */
>> --
>> 2.9.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Mon, 10 Apr 2017 12:12:01 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Ok, this is really a kickstart for a discussion.  While working on
> graphics support for virtual machines on ppc64 (which exists in both
> little and big endian variants) I've figured the comments in the header
> file don't match reality.  They are not considered little endian (as
> suggested by the comments) but in practice are used as native endian.
> 
> So, go update the comments.
> 
> This patch switches all 32bpp / 8 bpc formats over to native endian.
> Those used/supported by ppc64 virtual machines (virtio-gpu/bochs-drm
> drivers).
> 
> Given that DRM_FORMAT_BIG_ENDIAN isn't used anywhere in the codebase
> I suspect this problem applies to more formats.  When looking at
> drm_mode_legacy_fb_format it seems *all* RGB formats are actually
> native endian not little endian.
> 
> Dunno where we stand in terms of YCbCr.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Hi,

which software have you used as representative of "reality"?

I worry is that there is potential to have endianess bugs in every
layer of the graphics stack: driver, Mesa, display servers,
compositors, toolkits, and applications. What's worse, two wrongs might
sometimes make a right - incorrectly swapping twice gives you what you
expected in the first place.

We just had a detailed review to get a new pixel format
definitions list in Weston right for LE vs. BE and Pixman vs. DRM vs.
OpenGL. We took the DRM definition literally to be always described in
LE regardless of machine endianess:
https://cgit.freedesktop.org/wayland/weston/commit/?id=903721a6215f474787b5daf02761fbcb1d3a0bb5

We also have this fun problem in Wayland:
https://lists.freedesktop.org/archives/wayland-devel/2017-March/033492.html

To solve that problem, we would like to know if anything existing would
break for each possible solution, but no developers using BE have really
turned up.

We have known for a long time that at least Weston is likely broken on
BE wrt. pixel formats.

We also have a bug for Mesa EGL/Wayland:
https://bugs.freedesktop.org/show_bug.cgi?id=99638

> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 995c8f9..a7fc81d 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -85,15 +85,15 @@ extern "C" {
>  #define DRM_FORMAT_BGR888	fourcc_code('B', 'G', '2', '4') /* [23:0] B:G:R little endian */
>  
>  /* 32 bpp RGB */
> -#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
> -#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 little endian */
> -#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 little endian */
> -#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 little endian */
> +#define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
> +#define DRM_FORMAT_BGRX8888	fourcc_code('B', 'X', '2', '4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
>  
> -#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 little endian */
> -#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 little endian */
> -#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 little endian */
> -#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 little endian */
> +#define DRM_FORMAT_ARGB8888	fourcc_code('A', 'R', '2', '4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_ABGR8888	fourcc_code('A', 'B', '2', '4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_RGBA8888	fourcc_code('R', 'A', '2', '4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
> +#define DRM_FORMAT_BGRA8888	fourcc_code('B', 'A', '2', '4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
>  
>  #define DRM_FORMAT_XRGB2101010	fourcc_code('X', 'R', '3', '0') /* [31:0] x:R:G:B 2:10:10:10 little endian */
>  #define DRM_FORMAT_XBGR2101010	fourcc_code('X', 'B', '3', '0') /* [31:0] x:B:G:R 2:10:10:10 little endian */

These format definitions are directly referenced by Wayland protocol
extension for dmabufs and Mesa (wl_drm), and they have been copied into
the Wayland core protocol for wl_shm. Changing their definition would
be... awkward.


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> which software have you used as representative of "reality"?

ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
only format supported by bochs-drm), and we have to interpret that in
bigendian byte order on the host side to get a correct display.

Didn't try wayland.  Can do, but will take a while.  Don't have a
wayland-capable guest install at hand, and installing one takes a while
because I don't have a physical pseries and emulation is slooooww.

> To solve that problem, we would like to know if anything existing would
> break for each possible solution, but no developers using BE have really
> turned up.

That is part of the problem.
And even ppc is moving to little endian these days ...

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Mon, Apr 10, 2017 at 10:17 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> which software have you used as representative of "reality"?
>
> ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
> driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
> only format supported by bochs-drm), and we have to interpret that in
> bigendian byte order on the host side to get a correct display.
>
> Didn't try wayland.  Can do, but will take a while.  Don't have a
> wayland-capable guest install at hand, and installing one takes a while
> because I don't have a physical pseries and emulation is slooooww.
>
>> To solve that problem, we would like to know if anything existing would
>> break for each possible solution, but no developers using BE have really
>> turned up.
>
> That is part of the problem.
> And even ppc is moving to little endian these days ...

The poor saps who are still using PPC G4's and G5's with NVIDIA and
ATI boards constantly get their working setups broken by people trying
to "fix" BE. I think it's important to keep them in mind, and test
those setups, whenever one tries to make a large sweeping change.

Cheers,

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Alex Deucher]

On Mon, Apr 10, 2017 at 10:45 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Mon, Apr 10, 2017 at 10:17 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>   Hi,
>>
>>> which software have you used as representative of "reality"?
>>
>> ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
>> driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
>> only format supported by bochs-drm), and we have to interpret that in
>> bigendian byte order on the host side to get a correct display.
>>
>> Didn't try wayland.  Can do, but will take a while.  Don't have a
>> wayland-capable guest install at hand, and installing one takes a while
>> because I don't have a physical pseries and emulation is slooooww.
>>
>>> To solve that problem, we would like to know if anything existing would
>>> break for each possible solution, but no developers using BE have really
>>> turned up.
>>
>> That is part of the problem.
>> And even ppc is moving to little endian these days ...
>
> The poor saps who are still using PPC G4's and G5's with NVIDIA and
> ATI boards constantly get their working setups broken by people trying
> to "fix" BE. I think it's important to keep them in mind, and test
> those setups, whenever one tries to make a large sweeping change.

I think part of the problem is that the higher levels of the stack
have different expectations than the kernel does.  Mesa breaks every
time someone fixes something endian related.  Most userspace stuff
assumes host endianness.  Someone that has the hw and cares about BE
should really audit the stack top to bottom and make sure all the
expectations are met for each level.  I doubt anyone will.

Alex

>
> Cheers,
>
>   -ilia
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Mon, 10 Apr 2017 16:17:27 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > which software have you used as representative of "reality"?  
> 
> ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
> driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
> only format supported by bochs-drm), and we have to interpret that in
> bigendian byte order on the host side to get a correct display.

I wonder if that is just an oversight from trying to match OpenGL
formats to DRM formats. It's full of gotcha's.

Did you try with GLAMOR? Do you see a difference with and without
GLAMOR? Hmm, but you have no GPU support, so GLAMOR would be through a
Mesa software renderer? I think I heard someone say something about
Mesa software on BE...

But even if this actually is a valid example of software we must keep
working as is, well, ouch. But if I cannot show that your fix breaks
anything, then I suppose you win. Your proposal would certainly solve
the dilemma we have with wl_shm formats in Wayland.

I also wonder if a real BE machine could have different results than
the virtual machine.


Thanks,
pq

> Didn't try wayland.  Can do, but will take a while.  Don't have a
> wayland-capable guest install at hand, and installing one takes a while
> because I don't have a physical pseries and emulation is slooooww.
> 
> > To solve that problem, we would like to know if anything existing would
> > break for each possible solution, but no developers using BE have really
> > turned up.  
> 
> That is part of the problem.
> And even ppc is moving to little endian these days ...
> 
> cheers,
>   Gerd
> 


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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Mon, Apr 10, 2017 at 11:09 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Mon, 10 Apr 2017 16:17:27 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>>   Hi,
>>
>> > which software have you used as representative of "reality"?
>>
>> ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
>> driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
>> only format supported by bochs-drm), and we have to interpret that in
>> bigendian byte order on the host side to get a correct display.
>
> I wonder if that is just an oversight from trying to match OpenGL
> formats to DRM formats. It's full of gotcha's.
>
> Did you try with GLAMOR? Do you see a difference with and without
> GLAMOR? Hmm, but you have no GPU support, so GLAMOR would be through a
> Mesa software renderer? I think I heard someone say something about
> Mesa software on BE...
>
> But even if this actually is a valid example of software we must keep
> working as is, well, ouch. But if I cannot show that your fix breaks
> anything, then I suppose you win. Your proposal would certainly solve
> the dilemma we have with wl_shm formats in Wayland.
>
> I also wonder if a real BE machine could have different results than
> the virtual machine.

I have a PPC G5 with an AGP GeForce FX 5200 that I can test things on,
if necessary. (I got it specifically for this purpose, as the people
who use this type of hw daily tend to perform updates rarely... in no
small part due to the fact that updates tend to break the HW.)

Just let know what you need tested, I should be able to turn it around
within a couple of days.

Cheers,

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Mon, 10 Apr 2017 12:10:14 -0400
Ilia Mirkin <imirkin@alum.mit.edu> wrote:

> On Mon, Apr 10, 2017 at 11:09 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> > I also wonder if a real BE machine could have different results than
> > the virtual machine.  
> 
> I have a PPC G5 with an AGP GeForce FX 5200 that I can test things on,
> if necessary. (I got it specifically for this purpose, as the people
> who use this type of hw daily tend to perform updates rarely... in no
> small part due to the fact that updates tend to break the HW.)
> 
> Just let know what you need tested, I should be able to turn it around
> within a couple of days.

That's part of my problem. I don't really know what should be tested.
What do people do with their BE machines that we should avoid breaking?

I suppose correlating with what Gerd was testing would be nice, i.e.
does X.org with modesetting look right with and without GLAMOR? Maybe
you also want to throw in xf86-video-nouveau for comparison.

As to what apps to test those with... Gerd? I presume anything that does
*not* use OpenGL, since we have doubts of Mesa's correctness on BE,
right? So maybe some of the traditional ancient X11 apps? xterm with
colours?

However, I totally agree with Alex that someone with a BE machine
should review the whole stack before we could be confident with anything.


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> > Just let know what you need tested, I should be able to turn it around
> > within a couple of days.
> 
> That's part of my problem. I don't really know what should be tested.
> What do people do with their BE machines that we should avoid breaking?

For the virtual machine use case the bar is pretty low, it's mostly
about a graphical server console.  Anaconda installer.  Gnome desktop
with browser and standard xorg (xterm) + gtk apps.  No heavy OpenGL
stuff.  No hardware acceleration, so if opengl is used then it'll be
llvmpipe.

Right now Xorg is important.  Not sure whenever wayland ever will be,
possibly the ppc64 -> ppc64le switch goes faster than the xorg ->
wayland switch.

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Tue, 11 Apr 2017 13:23:53 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > > Just let know what you need tested, I should be able to turn it around
> > > within a couple of days.  
> > 
> > That's part of my problem. I don't really know what should be tested.
> > What do people do with their BE machines that we should avoid breaking?  
> 
> For the virtual machine use case the bar is pretty low, it's mostly
> about a graphical server console.  Anaconda installer.  Gnome desktop
> with browser and standard xorg (xterm) + gtk apps.  No heavy OpenGL
> stuff.  No hardware acceleration, so if opengl is used then it'll be
> llvmpipe.
> 
> Right now Xorg is important.  Not sure whenever wayland ever will be,
> possibly the ppc64 -> ppc64le switch goes faster than the xorg ->
> wayland switch.

Hi,

IMHO you can ignore Wayland for now I suppose, I just wanted to point
out that we have similar problems there and whatever you do with the
DRM format codes will affect things on Wayland too.

Once you get things hashed out on an X.org based stack, we can look
what it means for Wayland software.

After all, BE users are scarce and allegedly favouring old software to
avoid breakage; Wayland is new, and Wayland compositors still "rare",
so the intersection of people using both BE and Wayland and relying on
it to work is... minuscule? insignificant?

I don't mean to belittle people that use Wayland on BE, but by that one
bug report EGL is and probably has been broken, and it's unclear if
anything has ever worked.


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Tue, Apr 11, 2017 at 3:31 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Mon, 10 Apr 2017 12:10:14 -0400
> Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>
>> On Mon, Apr 10, 2017 at 11:09 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
>> > I also wonder if a real BE machine could have different results than
>> > the virtual machine.
>>
>> I have a PPC G5 with an AGP GeForce FX 5200 that I can test things on,
>> if necessary. (I got it specifically for this purpose, as the people
>> who use this type of hw daily tend to perform updates rarely... in no
>> small part due to the fact that updates tend to break the HW.)
>>
>> Just let know what you need tested, I should be able to turn it around
>> within a couple of days.
>
> That's part of my problem. I don't really know what should be tested.
> What do people do with their BE machines that we should avoid breaking?
>
> I suppose correlating with what Gerd was testing would be nice, i.e.
> does X.org with modesetting look right with and without GLAMOR? Maybe
> you also want to throw in xf86-video-nouveau for comparison.

GLAMOR? That's funny. No way that works. Pretty sure it doesn't work
on nv30/nv40 GPUs on LE boxes (at least I get bugs about it). I've
gotten mesa to the point where simple demos work again (after it got
"fixed" for BE, thus breaking all previously working setups). However
there's too much confusion in my head about how to store buffer data,
and precisely what the "BE" mode on the GPUs does for anything complex
to work.

> As to what apps to test those with... Gerd? I presume anything that does
> *not* use OpenGL, since we have doubts of Mesa's correctness on BE,
> right? So maybe some of the traditional ancient X11 apps? xterm with
> colours?

Mesa's fine on BE again, at least the nv30 driver. I had to do some
slightly dubious things to get it to that state, but it does function
mostly properly (as long as you don't try to use VBOs ... or index
buffers ... something ends up getting byteswapped one too many times,
however that's a wholly nv30-private issue).

> However, I totally agree with Alex that someone with a BE machine
> should review the whole stack before we could be confident with anything.

Here's what I'm confident about: xf86-video-nouveau worked just fine
on top of kernel 4.3 on an AGP GeForce FX 5200 (with AGPGART turned
off because ... well ... uninorth). fbcon/fbdev accel worked,
xf86-video-nouveau's 2d accel worked, and simple demos (ala glxgears)
worked after I fixed up mesa and nv30 driver items in version ... 11.1
it seems. As I recall it had gotten all broken in 10.0 or so by Adam
Jackson in the name of making llvmpipe work on BE, declaring all other
drivers broken, with various fixes by Michel Dänzer to get it back to
working over the years.

Anyone "fixing" the stack has to maintain that level of functioning
through their various fixing.

I will double-check that the above still works with the latest
kernel/xorg/xf86-video-nouveau/mesa and report back (hopefully by this
weekend). If there are any patches you'd like me to test, now's the
time to ask -- getting the box up and running is the hard part,
booting up an extra kernel -- easy.

Cheers,

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Tue, Apr 11, 2017 at 10:18 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> However, I totally agree with Alex that someone with a BE machine
>> should review the whole stack before we could be confident with anything.
>
> Here's what I'm confident about: xf86-video-nouveau worked just fine
> on top of kernel 4.3 on an AGP GeForce FX 5200 (with AGPGART turned
> off because ... well ... uninorth). fbcon/fbdev accel worked,
> xf86-video-nouveau's 2d accel worked, and simple demos (ala glxgears)
> worked after I fixed up mesa and nv30 driver items in version ... 11.1
> it seems. As I recall it had gotten all broken in 10.0 or so by Adam
> Jackson in the name of making llvmpipe work on BE, declaring all other
> drivers broken, with various fixes by Michel Dänzer to get it back to
> working over the years.
>
> Anyone "fixing" the stack has to maintain that level of functioning
> through their various fixing.
>
> I will double-check that the above still works with the latest
> kernel/xorg/xf86-video-nouveau/mesa and report back (hopefully by this
> weekend). If there are any patches you'd like me to test, now's the
> time to ask -- getting the box up and running is the hard part,
> booting up an extra kernel -- easy.

OK, so I revived my PowerMac7,3 G5 setup (PPC64 BE, NV34 GPU). Booted
it with an upstream 4.11-rc7 kernel, loaded up the nouveau kernel
module (which is included in that kernel), updated X to 1.19.2 and
mesa to 17.0.3. Everything works fine. Specifically:

fbcon on top of fbdev provided by nouveau -- colors are fine
glxgears hw-accelerated by mesa on top of xf86-video-nouveau using
DRI2 -- colors are fine
glxgears softpipe-accelerated by mesa on top of xf86-video-nouveau --
colors are fine
glxgears softpipe-accelerated by mesa on top of xf86-video-modesetting
-- colors are fine
xterm on top of xf86-video-nouveau -- colors are fine
xterm on top of xf86-video-modesetting -- colors are fine

I couldn't test anything with GLAMOR since GLAMOR requires GL 2.1 or
higher, whereas nouveau's NV3x acceleration only provides GL 1.5 (due
to lacking NPOT and a handful of other things).

The modetest utility did have trouble with AR24 and I'm pretty sure
the XR24 pattern was off too. However I wouldn't be surprised if the
modetest utility itself had endian issues in the pattern generation
logic. (Seems to be the case, based on a quick glance at the
tests/util/format.c logic and how it's used in pattern.c.)

So in short, I think the current definitions of format are fine.

Cheers,

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Michel Dänzer]

On 17/04/17 03:43 PM, Ilia Mirkin wrote:
> On Tue, Apr 11, 2017 at 10:18 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> However, I totally agree with Alex that someone with a BE machine
>>> should review the whole stack before we could be confident with anything.
>>
>> Here's what I'm confident about: xf86-video-nouveau worked just fine
>> on top of kernel 4.3 on an AGP GeForce FX 5200 (with AGPGART turned
>> off because ... well ... uninorth). fbcon/fbdev accel worked,
>> xf86-video-nouveau's 2d accel worked, and simple demos (ala glxgears)
>> worked after I fixed up mesa and nv30 driver items in version ... 11.1
>> it seems. As I recall it had gotten all broken in 10.0 or so by Adam
>> Jackson in the name of making llvmpipe work on BE, declaring all other
>> drivers broken, with various fixes by Michel Dänzer to get it back to
>> working over the years.
>>
>> Anyone "fixing" the stack has to maintain that level of functioning
>> through their various fixing.
>>
>> I will double-check that the above still works with the latest
>> kernel/xorg/xf86-video-nouveau/mesa and report back (hopefully by this
>> weekend). If there are any patches you'd like me to test, now's the
>> time to ask -- getting the box up and running is the hard part,
>> booting up an extra kernel -- easy.
> 
> OK, so I revived my PowerMac7,3 G5 setup (PPC64 BE, NV34 GPU). Booted
> it with an upstream 4.11-rc7 kernel, loaded up the nouveau kernel
> module (which is included in that kernel), updated X to 1.19.2 and
> mesa to 17.0.3. Everything works fine. Specifically:
> 
> fbcon on top of fbdev provided by nouveau -- colors are fine
> glxgears hw-accelerated by mesa on top of xf86-video-nouveau using
> DRI2 -- colors are fine
> glxgears softpipe-accelerated by mesa on top of xf86-video-nouveau --
> colors are fine
> glxgears softpipe-accelerated by mesa on top of xf86-video-modesetting
> -- colors are fine
> xterm on top of xf86-video-nouveau -- colors are fine
> xterm on top of xf86-video-modesetting -- colors are fine
> 
> I couldn't test anything with GLAMOR since GLAMOR requires GL 2.1 or
> higher, whereas nouveau's NV3x acceleration only provides GL 1.5 (due
> to lacking NPOT and a handful of other things).
> 
> The modetest utility did have trouble with AR24 and I'm pretty sure
> the XR24 pattern was off too. However I wouldn't be surprised if the
> modetest utility itself had endian issues in the pattern generation
> logic. (Seems to be the case, based on a quick glance at the
> tests/util/format.c logic and how it's used in pattern.c.)
> 
> So in short, I think the current definitions of format are fine.

I agree with Pekka that it's not that simple. What you've established is
that things look fine after going through several layers of abstraction.
It's possible that multiple bugs in those layers cancel each other out;
in particular, it's quite likely that the code dealing with DRM formats
is treating them as using native endianness (one possible giveaway for
that is using shifts for (un)packing colour components).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Mon, Apr 17, 2017 at 10:53 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 17/04/17 03:43 PM, Ilia Mirkin wrote:
>> On Tue, Apr 11, 2017 at 10:18 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>> However, I totally agree with Alex that someone with a BE machine
>>>> should review the whole stack before we could be confident with anything.
>>>
>>> Here's what I'm confident about: xf86-video-nouveau worked just fine
>>> on top of kernel 4.3 on an AGP GeForce FX 5200 (with AGPGART turned
>>> off because ... well ... uninorth). fbcon/fbdev accel worked,
>>> xf86-video-nouveau's 2d accel worked, and simple demos (ala glxgears)
>>> worked after I fixed up mesa and nv30 driver items in version ... 11.1
>>> it seems. As I recall it had gotten all broken in 10.0 or so by Adam
>>> Jackson in the name of making llvmpipe work on BE, declaring all other
>>> drivers broken, with various fixes by Michel Dänzer to get it back to
>>> working over the years.
>>>
>>> Anyone "fixing" the stack has to maintain that level of functioning
>>> through their various fixing.
>>>
>>> I will double-check that the above still works with the latest
>>> kernel/xorg/xf86-video-nouveau/mesa and report back (hopefully by this
>>> weekend). If there are any patches you'd like me to test, now's the
>>> time to ask -- getting the box up and running is the hard part,
>>> booting up an extra kernel -- easy.
>>
>> OK, so I revived my PowerMac7,3 G5 setup (PPC64 BE, NV34 GPU). Booted
>> it with an upstream 4.11-rc7 kernel, loaded up the nouveau kernel
>> module (which is included in that kernel), updated X to 1.19.2 and
>> mesa to 17.0.3. Everything works fine. Specifically:
>>
>> fbcon on top of fbdev provided by nouveau -- colors are fine
>> glxgears hw-accelerated by mesa on top of xf86-video-nouveau using
>> DRI2 -- colors are fine
>> glxgears softpipe-accelerated by mesa on top of xf86-video-nouveau --
>> colors are fine
>> glxgears softpipe-accelerated by mesa on top of xf86-video-modesetting
>> -- colors are fine
>> xterm on top of xf86-video-nouveau -- colors are fine
>> xterm on top of xf86-video-modesetting -- colors are fine
>>
>> I couldn't test anything with GLAMOR since GLAMOR requires GL 2.1 or
>> higher, whereas nouveau's NV3x acceleration only provides GL 1.5 (due
>> to lacking NPOT and a handful of other things).
>>
>> The modetest utility did have trouble with AR24 and I'm pretty sure
>> the XR24 pattern was off too. However I wouldn't be surprised if the
>> modetest utility itself had endian issues in the pattern generation
>> logic. (Seems to be the case, based on a quick glance at the
>> tests/util/format.c logic and how it's used in pattern.c.)
>>
>> So in short, I think the current definitions of format are fine.
>
> I agree with Pekka that it's not that simple. What you've established is
> that things look fine after going through several layers of abstraction.
> It's possible that multiple bugs in those layers cancel each other out;
> in particular, it's quite likely that the code dealing with DRM formats
> is treating them as using native endianness (one possible giveaway for
> that is using shifts for (un)packing colour components).

Quite true that this proves nothing. However one should note that
fbcon -> fbdev works, and both mesa hw driver and softpipe driver
work, in addition to regular DDX accel. Which means that the bugs, if
they exist, are pretty consistent amongst each other, spanning
multiple layers, all agreeing as to what the proper bugginess is. One
could go so far as to declare it to be a feature.

It does show is that things generally work today in at least some, if
not many, setups, and one can't go around breaking them willy nilly.

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Michel Dänzer]

On 18/04/17 02:04 PM, Ilia Mirkin wrote:
> On Mon, Apr 17, 2017 at 10:53 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 17/04/17 03:43 PM, Ilia Mirkin wrote:
>>> On Tue, Apr 11, 2017 at 10:18 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>> However, I totally agree with Alex that someone with a BE machine
>>>>> should review the whole stack before we could be confident with anything.
>>>>
>>>> Here's what I'm confident about: xf86-video-nouveau worked just fine
>>>> on top of kernel 4.3 on an AGP GeForce FX 5200 (with AGPGART turned
>>>> off because ... well ... uninorth). fbcon/fbdev accel worked,
>>>> xf86-video-nouveau's 2d accel worked, and simple demos (ala glxgears)
>>>> worked after I fixed up mesa and nv30 driver items in version ... 11.1
>>>> it seems. As I recall it had gotten all broken in 10.0 or so by Adam
>>>> Jackson in the name of making llvmpipe work on BE, declaring all other
>>>> drivers broken, with various fixes by Michel Dänzer to get it back to
>>>> working over the years.
>>>>
>>>> Anyone "fixing" the stack has to maintain that level of functioning
>>>> through their various fixing.
>>>>
>>>> I will double-check that the above still works with the latest
>>>> kernel/xorg/xf86-video-nouveau/mesa and report back (hopefully by this
>>>> weekend). If there are any patches you'd like me to test, now's the
>>>> time to ask -- getting the box up and running is the hard part,
>>>> booting up an extra kernel -- easy.
>>>
>>> OK, so I revived my PowerMac7,3 G5 setup (PPC64 BE, NV34 GPU). Booted
>>> it with an upstream 4.11-rc7 kernel, loaded up the nouveau kernel
>>> module (which is included in that kernel), updated X to 1.19.2 and
>>> mesa to 17.0.3. Everything works fine. Specifically:
>>>
>>> fbcon on top of fbdev provided by nouveau -- colors are fine
>>> glxgears hw-accelerated by mesa on top of xf86-video-nouveau using
>>> DRI2 -- colors are fine
>>> glxgears softpipe-accelerated by mesa on top of xf86-video-nouveau --
>>> colors are fine
>>> glxgears softpipe-accelerated by mesa on top of xf86-video-modesetting
>>> -- colors are fine
>>> xterm on top of xf86-video-nouveau -- colors are fine
>>> xterm on top of xf86-video-modesetting -- colors are fine
>>>
>>> I couldn't test anything with GLAMOR since GLAMOR requires GL 2.1 or
>>> higher, whereas nouveau's NV3x acceleration only provides GL 1.5 (due
>>> to lacking NPOT and a handful of other things).
>>>
>>> The modetest utility did have trouble with AR24 and I'm pretty sure
>>> the XR24 pattern was off too. However I wouldn't be surprised if the
>>> modetest utility itself had endian issues in the pattern generation
>>> logic. (Seems to be the case, based on a quick glance at the
>>> tests/util/format.c logic and how it's used in pattern.c.)
>>>
>>> So in short, I think the current definitions of format are fine.
>>
>> I agree with Pekka that it's not that simple. What you've established is
>> that things look fine after going through several layers of abstraction.
>> It's possible that multiple bugs in those layers cancel each other out;
>> in particular, it's quite likely that the code dealing with DRM formats
>> is treating them as using native endianness (one possible giveaway for
>> that is using shifts for (un)packing colour components).
> 
> Quite true that this proves nothing. However one should note that
> fbcon -> fbdev works,

BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
native endian packed colour values.


> and both mesa hw driver and softpipe driver work, in addition to regular
> DDX accel.

Similarly, the X11 protocol uses native endian packed colour values.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> > Quite true that this proves nothing. However one should note that
> > fbcon -> fbdev works,
> 
> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
> native endian packed colour values.

Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
fourcc formats directly).

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Michel Dänzer]

On 18/04/17 07:14 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> Quite true that this proves nothing. However one should note that
>>> fbcon -> fbdev works,
>>
>> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
>> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
>> native endian packed colour values.
> 
> Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
> DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
> fourcc formats directly).

Right, and since all major Xorg drivers use DRM_IOCTL_MODE_ADDFB,
they're effectively using DRM_FORMAT_XRGB8888 as native endianness as well.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Tue, Apr 18, 2017 at 9:01 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 18/04/17 07:14 PM, Gerd Hoffmann wrote:
>>   Hi,
>>
>>>> Quite true that this proves nothing. However one should note that
>>>> fbcon -> fbdev works,
>>>
>>> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
>>> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
>>> native endian packed colour values.
>>
>> Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
>> DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
>> fourcc formats directly).
>
> Right, and since all major Xorg drivers use DRM_IOCTL_MODE_ADDFB,
> they're effectively using DRM_FORMAT_XRGB8888 as native endianness as well.

In the meanwhile, it has been pointed out to me that pre-nv50 display
code actually doesn't use DRM_FORMAT_* at all -- it uses some helpers
which end up advertising XR24 / AR24. However from what I can tell,
that's not a well-reasoned selection. Either way, I'm going to test
Gerd's patch, hopefully during the week, or weekend at the latest. My
current suspicion is that it will have no effect on nouveau either
way. We'll find out.

  -ilia

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Ilia Mirkin]

On Tue, Apr 18, 2017 at 11:19 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Tue, Apr 18, 2017 at 9:01 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 18/04/17 07:14 PM, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>>> Quite true that this proves nothing. However one should note that
>>>>> fbcon -> fbdev works,
>>>>
>>>> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
>>>> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
>>>> native endian packed colour values.
>>>
>>> Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
>>> DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
>>> fourcc formats directly).
>>
>> Right, and since all major Xorg drivers use DRM_IOCTL_MODE_ADDFB,
>> they're effectively using DRM_FORMAT_XRGB8888 as native endianness as well.
>
> In the meanwhile, it has been pointed out to me that pre-nv50 display
> code actually doesn't use DRM_FORMAT_* at all -- it uses some helpers
> which end up advertising XR24 / AR24. However from what I can tell,
> that's not a well-reasoned selection. Either way, I'm going to test
> Gerd's patch, hopefully during the week, or weekend at the latest. My
> current suspicion is that it will have no effect on nouveau either
> way. We'll find out.

(And as Michel points out, the patch doesn't actually touch anything,
just comments. I originally thought it changed format -> fourcc
mapping.)

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Wed, 19 Apr 2017 10:01:47 +0900
Michel Dänzer <michel@daenzer.net> wrote:

> On 18/04/17 07:14 PM, Gerd Hoffmann wrote:
> >   Hi,
> >   
> >>> Quite true that this proves nothing. However one should note that
> >>> fbcon -> fbdev works,  
> >>
> >> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
> >> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
> >> native endian packed colour values.  
> > 
> > Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
> > DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
> > fourcc formats directly).  
> 
> Right, and since all major Xorg drivers use DRM_IOCTL_MODE_ADDFB,
> they're effectively using DRM_FORMAT_XRGB8888 as native endianness as well.

I sincerely hope this doesn't actually force us into a place where we
have XRGB8888 (and ARGB8888?) as native-endian, but the other format
codes - since being used explicitly - must be kept as little-endian
because they were used like that honouring the documentation we have
atm. It's starting to resemble the wl_shm format codes problem we have
on Wayland for BE.

Has this now turned into a question of what the kernel drivers do
with the DRM pixel format codes?

Hmm, I suppose that has been the question all along... userspace uses
whatever formats that look right, new kernel drivers come along and fix
themselves to work with whatever userspace uses? And the people who get
it wrong are the ones who only ever test on LE and trust the docs...


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> > >> BTW, this supports Gerd's patch, since the KMS fbdev emulation code uses
> > >> e.g. DRM_FORMAT_XRGB8888 for depth/bpp 24/32, and the fbdev API uses
> > >> native endian packed colour values.  
> > > 
> > > Same is true for DRM_IOCTL_MODE_ADDFB, with depth/bpp 24/32 you'll get
> > > DRM_FORMAT_XRGB8888 (only DRM_IOCTL_MODE_ADDFB2 allows userspace specify
> > > fourcc formats directly).  
> > 
> > Right, and since all major Xorg drivers use DRM_IOCTL_MODE_ADDFB,
> > they're effectively using DRM_FORMAT_XRGB8888 as native endianness as well.
> 
> I sincerely hope this doesn't actually force us into a place where we
> have XRGB8888 (and ARGB8888?) as native-endian, but the other format
> codes - since being used explicitly - must be kept as little-endian
> because they were used like that honouring the documentation we have
> atm.

My expectation is that the other formats are (almost) unused in
practice.  cairo for example supports XRGB8888 + ARGB8888 (native
endian) only from all depth/bpp 24/32 formats.

IIRC there was a brief discussion how we should handle endianness in
qemu stdvga / bochsdrm.ko before we've added the new (virtual) hardware
register to switch endianness.  The idea to simply run with fixed
endianness (framebuffer is always little endian) was shot down quickly
with the argument that this isn't going to fly due to lack of support
for XRGB8888 in non-native byte order in the whole graphics stack.

> It's starting to resemble the wl_shm format codes problem we have
> on Wayland for BE.
> 
> Has this now turned into a question of what the kernel drivers do
> with the DRM pixel format codes?
> 
> Hmm, I suppose that has been the question all along...

Yep, basically.  I have the impression that drivers are either consider
those formats being native endian or simply don't care because they are
never used in systems with bigendian (-capable) cpus.

Anyone aware of anything else?

Guess I'll go prepare a new version of the patch, declaring all rgb
formats as native endian and putting a bunch of points from this thread
into the commit message.

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> > ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
> > driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
> > only format supported by bochs-drm), and we have to interpret that in
> > bigendian byte order on the host side to get a correct display.
> 
> I wonder if that is just an oversight from trying to match OpenGL
> formats to DRM formats. It's full of gotcha's.
> 
> Did you try with GLAMOR? Do you see a difference with and without
> GLAMOR? Hmm, but you have no GPU support, so GLAMOR would be through a
> Mesa software renderer? I think I heard someone say something about
> Mesa software on BE...

So, did some more testing to see where we stand.

Historical note:  RHEL-6.9 (gnome 2) works fine.  Not of much interest
here, it drives the qemu stdvga with offb, not bochs-drm.

More interesting:  RHEL-7.3 (gnome 3.14) works fine too.  kernel 3.10,
but drm drivers updated to roughly 4.6 level.  Runs bochs-drm.  mesa
11.2.2.  glamour not used.

Most recent:  Fedora 25 (gnome 3.22) looks mostly ok, but there are
rendering glitches, for example in the gnome activities screen (the one
you get when you press the windows key).  kernel 4.10, mesa 13.0.4.
glamor not used, but I think gnome-shell uses opengl (via llvmpipe) for
compositing.

btw: is there some way to start a wayland session from a shell (i.e.
what startx does for xorg)?

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Tue, 18 Apr 2017 12:00:17 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > > ppc64 (big endian) virtual machine, running with qemu stdvga & bochs-drm
> > > driver.  Xorg with modesetting driver uses DRM_FORMAT_XRGB8888 (one and
> > > only format supported by bochs-drm), and we have to interpret that in
> > > bigendian byte order on the host side to get a correct display.  
> > 
> > I wonder if that is just an oversight from trying to match OpenGL
> > formats to DRM formats. It's full of gotcha's.
> > 
> > Did you try with GLAMOR? Do you see a difference with and without
> > GLAMOR? Hmm, but you have no GPU support, so GLAMOR would be through a
> > Mesa software renderer? I think I heard someone say something about
> > Mesa software on BE...  
> 
> So, did some more testing to see where we stand.
> 
> Historical note:  RHEL-6.9 (gnome 2) works fine.  Not of much interest
> here, it drives the qemu stdvga with offb, not bochs-drm.

I suppose this proves the virtual machine itself is correct about
framebuffer endianess? Except you are running it on a little-endian
host machine I presume...

> More interesting:  RHEL-7.3 (gnome 3.14) works fine too.  kernel 3.10,
> but drm drivers updated to roughly 4.6 level.  Runs bochs-drm.  mesa
> 11.2.2.  glamour not used.
> 
> Most recent:  Fedora 25 (gnome 3.22) looks mostly ok, but there are
> rendering glitches, for example in the gnome activities screen (the one
> you get when you press the windows key).  kernel 4.10, mesa 13.0.4.
> glamor not used, but I think gnome-shell uses opengl (via llvmpipe) for
> compositing.

I believe glitches are irrelevant for this topic, what we are
interested in is if the colors are right or byte-swapped (also mind
alpha/blue etc. swaps).

> btw: is there some way to start a wayland session from a shell (i.e.
> what startx does for xorg)?

Depends on which display server you want to start I believe. I don't
know about anything else than Weston, which is 'weston' for a
logind-enabled system.


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> > Historical note:  RHEL-6.9 (gnome 2) works fine.  Not of much interest
> > here, it drives the qemu stdvga with offb, not bochs-drm.
> 
> I suppose this proves the virtual machine itself is correct about
> framebuffer endianess? Except you are running it on a little-endian
> host machine I presume...

Yes, little endian host, qemu interprets the framebuffer as
PIXMAN_b8g8r8x8.  Which should be correct for a xrgb bigendian
framebuffer as pixman formats are native endian.

> > More interesting:  RHEL-7.3 (gnome 3.14) works fine too.  kernel 3.10,
> > but drm drivers updated to roughly 4.6 level.  Runs bochs-drm.  mesa
> > 11.2.2.  glamour not used.
> > 
> > Most recent:  Fedora 25 (gnome 3.22) looks mostly ok, but there are
> > rendering glitches, for example in the gnome activities screen (the one
> > you get when you press the windows key).  kernel 4.10, mesa 13.0.4.
> > glamor not used, but I think gnome-shell uses opengl (via llvmpipe) for
> > compositing.
> 
> I believe glitches are irrelevant for this topic, what we are
> interested in is if the colors are right or byte-swapped (also mind
> alpha/blue etc. swaps).

Well, I mean color glitches.  But it isn't consistent.  As if some
operations operate with the correct byteorder and some don't.
alpha/blue being swapped is a problem in some areas.

https://www.kraxel.org/tmp/

cheers,
  Gerd

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Pekka Paalanen]

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

On Tue, 18 Apr 2017 15:39:53 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > > Historical note:  RHEL-6.9 (gnome 2) works fine.  Not of much interest
> > > here, it drives the qemu stdvga with offb, not bochs-drm.  
> > 
> > I suppose this proves the virtual machine itself is correct about
> > framebuffer endianess? Except you are running it on a little-endian
> > host machine I presume...  
> 
> Yes, little endian host, qemu interprets the framebuffer as
> PIXMAN_b8g8r8x8.  Which should be correct for a xrgb bigendian
> framebuffer as pixman formats are native endian.

Right. Very nice if we can trust the virtual machine at least getting
things right, gives some chance for people to test anything. Except...
that's a question of what kind of hardware the virtual machine
emulates. The display device defines what endianess it uses on
framebuffers, not the CPU, right?

> > > More interesting:  RHEL-7.3 (gnome 3.14) works fine too.  kernel 3.10,
> > > but drm drivers updated to roughly 4.6 level.  Runs bochs-drm.  mesa
> > > 11.2.2.  glamour not used.
> > > 
> > > Most recent:  Fedora 25 (gnome 3.22) looks mostly ok, but there are
> > > rendering glitches, for example in the gnome activities screen (the one
> > > you get when you press the windows key).  kernel 4.10, mesa 13.0.4.
> > > glamor not used, but I think gnome-shell uses opengl (via llvmpipe) for
> > > compositing.  
> > 
> > I believe glitches are irrelevant for this topic, what we are
> > interested in is if the colors are right or byte-swapped (also mind
> > alpha/blue etc. swaps).  
> 
> Well, I mean color glitches.  But it isn't consistent.  As if some
> operations operate with the correct byteorder and some don't.
> alpha/blue being swapped is a problem in some areas.
> 
> https://www.kraxel.org/tmp/

Ooh, yeah, that's definitely bonkers.

Maybe the 100% blue things are supposed to be a transparent blended
overlays, like highlights.

The icons look somehow... not completely right to me. Somehow washed
out?

Opaque gray shades are hard to tell right from wrong.

gnome-terminal and the wallpaper look right, but those might be the
only things.

Having a compositing manager complicates things.


Thanks,
pq

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

Re: [RfC PATCH] drm: fourcc byteorder: brings header file comments in line with reality.

[Gerd Hoffmann]

  Hi,

> Right. Very nice if we can trust the virtual machine at least getting
> things right, gives some chance for people to test anything. Except...
> that's a question of what kind of hardware the virtual machine
> emulates. The display device defines what endianess it uses on
> framebuffers, not the CPU, right?

The display device supports switching the endianess for the framebuffer,
at least with kernel 3.19+ and qemu 2.2+.  Default endianness depends on
the machine type, i.e. ppc64 guests get a bigendian framebuffer and
pretty much everything else a little endian framebuffer on reset.

The bochs-drm driver switches the display into native endian mode, i.e.
big endian for ppc64 and little endian for ppc64le kernels.

See commit 9ecdb039b7517dc10b8c3e6dbeb40859178ac28e

> > Well, I mean color glitches.  But it isn't consistent.  As if some
> > operations operate with the correct byteorder and some don't.
> > alpha/blue being swapped is a problem in some areas.
> > 
> > https://www.kraxel.org/tmp/
> 
> Ooh, yeah, that's definitely bonkers.
> 
> Maybe the 100% blue things are supposed to be a transparent blended
> overlays, like highlights.
> 
> The icons look somehow... not completely right to me. Somehow washed
> out?
> 
> Opaque gray shades are hard to tell right from wrong.
> 
> gnome-terminal and the wallpaper look right, but those might be the
> only things.
> 
> Having a compositing manager complicates things.

In some way yes, in some way no.  Tried wayland meanwhile (using
"gnome-shell --wayland").  Looks pretty much the same.  Window
decorations look a bit different (good on xorg, broken on wayland),
probably because window decorations work completely different.
Otherwise it is bug compatible to xorg.  Probably because gnome-shell
composites everything using llvmpipe, so it's largely the same code
running on both xorg and wayland, which then finally scans out to a dumb
framebuffer.

cheers,
  Gerd