Linux Virtualization - Apr 2017 - DRM_FORMAT_* byte order (was: Re: [PATCH] drm: virtio: fix virtio_gpu_cursor_formats)

When we use virtio-vga with a big-endian guest,
the mouse pointer disappears.

To fix that, on big-endian use DRM_FORMAT_BGRA8888
instead of DRM_FORMAT_ARGB8888.

Signed-off-by: Laurent Vivier <lvivier at redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 11288ff..3ed7174 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -39,7 +39,11 @@ static const uint32_t virtio_gpu_formats[] = {
 };
 
 static const uint32_t virtio_gpu_cursor_formats[] = {
+#ifdef __BIG_ENDIAN
+	DRM_FORMAT_BGRA8888,
+#else
 	DRM_FORMAT_ARGB8888,
+#endif
 };
 
 static void virtio_gpu_plane_destroy(struct drm_plane *plane)
-- 
2.9.3

On Wed, Apr 05, 2017 at 10:09:15AM +0200, Laurent Vivier
wrote:
> When we use virtio-vga with a big-endian guest,
> the mouse pointer disappears.
> 
> To fix that, on big-endian use DRM_FORMAT_BGRA8888
> instead of DRM_FORMAT_ARGB8888.
> 
> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 11288ff..3ed7174 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -39,7 +39,11 @@ static const uint32_t virtio_gpu_formats[] = {
>  };
>  
>  static const uint32_t virtio_gpu_cursor_formats[] = {
> +#ifdef __BIG_ENDIAN
> +	DRM_FORMAT_BGRA8888,
> +#else
>  	DRM_FORMAT_ARGB8888,
> +#endif
DRM formats are supposed to be little endian, so this isn't really
correct.
>  };
>  
>  static void virtio_gpu_plane_destroy(struct drm_plane *plane)
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Ville Syrj?l?
Intel OTC

On 05/04/2017 19:11, Ville Syrj?l? wrote:
> On Wed, Apr 05, 2017 at 10:09:15AM +0200, Laurent Vivier wrote:
>> When we use virtio-vga with a big-endian guest,
>> the mouse pointer disappears.
>>
>> To fix that, on big-endian use DRM_FORMAT_BGRA8888
>> instead of DRM_FORMAT_ARGB8888.
>>
>> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
>> ---
>>  drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> index 11288ff..3ed7174 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -39,7 +39,11 @@ static const uint32_t virtio_gpu_formats[] = {
>>  };
>>  
>>  static const uint32_t virtio_gpu_cursor_formats[] = {
>> +#ifdef __BIG_ENDIAN
>> +	DRM_FORMAT_BGRA8888,
>> +#else
>>  	DRM_FORMAT_ARGB8888,
>> +#endif
> 
> DRM formats are supposed to be little endian, so this isn't really
> correct.
In a big endian kernel, we need to swap the bytes order, where do you
think is the best place to do that?

Thanks,
Laurent

On Wed, Apr 05, 2017 at 08:11:25PM +0300, Ville Syrj?l?
wrote:
> On Wed, Apr 05, 2017 at 10:09:15AM +0200, Laurent Vivier wrote:
> > When we use virtio-vga with a big-endian guest,
> > the mouse pointer disappears.
> > 
> > To fix that, on big-endian use DRM_FORMAT_BGRA8888
> > instead of DRM_FORMAT_ARGB8888.
> > 
> > Signed-off-by: Laurent Vivier <lvivier at redhat.com>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > index 11288ff..3ed7174 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > @@ -39,7 +39,11 @@ static const uint32_t virtio_gpu_formats[] = {
> >  };
> >  
> >  static const uint32_t virtio_gpu_cursor_formats[] = {
> > +#ifdef __BIG_ENDIAN
> > +	DRM_FORMAT_BGRA8888,
> > +#else
> >  	DRM_FORMAT_ARGB8888,
> > +#endif
> 
> DRM formats are supposed to be little endian, so this isn't really
> correct.
Reality says they're native endian, and I asked Gerd Hoffman to do a
documentation update and get that reviewed by amd folks (the only other
ones who care).
-Daniel
> 
> >  };
> >  
> >  static void virtio_gpu_plane_destroy(struct drm_plane *plane)
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrj?l?
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Hi,
> >  static const uint32_t virtio_gpu_cursor_formats[] = {
> > +#ifdef __BIG_ENDIAN
> > +	DRM_FORMAT_BGRA8888,
> > +#else
> >  	DRM_FORMAT_ARGB8888,
> > +#endif
> 
> DRM formats are supposed to be little endian, so this isn't really
> correct.
Well, maybe they where *intended* to be little endian at some point in
the past.  The actual code appears to interpret them as native endian
though.

Lets take a simple example, the bochs driver (qemu sdvga).  It supports
32 bpp with depth 24 (DRM_FORMAT_XRGB8888) as the one and only
framebuffer format (see bochs_user_framebuffer_create).  We still had to
add a special register to the virtual hardware so the guest can signal
to the host whenever the framebuffer is big endian or little endian (see
bochs_hw_init), so both ppc64 and ppc64le guests work properly with the
qemu stdvga.

So, bigendian guests assume that DRM_FORMAT_XRGB8888 is big endian not
little endian.  And given that the fourcc codes are used in the
userspace/kernel API too (see DRM_IOCTL_MODE_ADDFB2) I think we can't
change that any more ...

cheers,
  Gerd