[PATCH v2 1/2] drm/virtio: Use XRGB8888 also for big endian systems

[PATCH v2 1/2] drm/virtio: Use XRGB8888 also for big endian systems

[Jocelyn Falempe]

Mesa doesn't support BGRX8888, that means most wayland compositors
don't work on big endian guests.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c |  4 ++--
 drivers/gpu/drm/virtio/virtgpu_gem.c     |  2 +-
 drivers/gpu/drm/virtio/virtgpu_plane.c   | 10 ++--------
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 64baf2f22d9f0..3572a53ea2061 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -299,8 +299,8 @@ virtio_gpu_user_framebuffer_create(struct drm_device *dev,
 	struct virtio_gpu_framebuffer *virtio_gpu_fb;
 	int ret;
 
-	if (mode_cmd->pixel_format != DRM_FORMAT_HOST_XRGB8888 &&
-	    mode_cmd->pixel_format != DRM_FORMAT_HOST_ARGB8888)
+	if (mode_cmd->pixel_format != DRM_FORMAT_XRGB8888 &&
+	    mode_cmd->pixel_format != DRM_FORMAT_ARGB8888)
 		return ERR_PTR(-ENOENT);
 
 	/* lookup object associated with res handle */
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 7db48d17ee3a8..601e06962530f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -75,7 +75,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	args->size = pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
 
-	params.format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888);
+	params.format = virtio_gpu_translate_format(DRM_FORMAT_XRGB8888);
 	params.width = args->width;
 	params.height = args->height;
 	params.size = args->size;
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a72a2dbda031c..860b5757ec3fc 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -30,11 +30,11 @@
 #include "virtgpu_drv.h"
 
 static const uint32_t virtio_gpu_formats[] = {
-	DRM_FORMAT_HOST_XRGB8888,
+	DRM_FORMAT_XRGB8888,
 };
 
 static const uint32_t virtio_gpu_cursor_formats[] = {
-	DRM_FORMAT_HOST_ARGB8888,
+	DRM_FORMAT_ARGB8888,
 };
 
 uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
@@ -48,12 +48,6 @@ uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
 	case DRM_FORMAT_ARGB8888:
 		format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
 		break;
-	case DRM_FORMAT_BGRX8888:
-		format = VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM;
-		break;
-	case DRM_FORMAT_BGRA8888:
-		format = VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM;
-		break;
 	default:
 		/*
 		 * This should not happen, we handle everything listed

base-commit: 8befe8fa5a4e4b30787b17e078d9d7b5cb92ea19
-- 
2.46.0

[PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Jocelyn Falempe]

The colors are inverted when testing a s390x VM on a s390x host.
Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
endian guests fixes the colors. But it may break big-endian guest on
little-endian host. In this case, the fix should be in qemu, because
the host endianess is not known in the guest VM.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 860b5757ec3fc..0ec6ecc96eb13 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -37,16 +37,24 @@ static const uint32_t virtio_gpu_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
 
+#ifdef __BIG_ENDIAN
+#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
+#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
+#else
+#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
+#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
+#endif
+
 uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
 {
 	uint32_t format;
 
 	switch (drm_fourcc) {
 	case DRM_FORMAT_XRGB8888:
-		format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
+		format = VIRTIO_GPU_HOST_XRGB8888;
 		break;
 	case DRM_FORMAT_ARGB8888:
-		format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
+		format = VIRTIO_GPU_HOST_ARGB8888;
 		break;
 	default:
 		/*
-- 
2.46.0

Re: [PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Thomas Zimmermann]

Hi

Am 20.08.24 um 11:07 schrieb Jocelyn Falempe:
> The colors are inverted when testing a s390x VM on a s390x host.
> Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
> endian guests fixes the colors. But it may break big-endian guest on
> little-endian host. In this case, the fix should be in qemu, because
> the host endianess is not known in the guest VM.
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>   drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 860b5757ec3fc..0ec6ecc96eb13 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -37,16 +37,24 @@ static const uint32_t virtio_gpu_cursor_formats[] = {
>   	DRM_FORMAT_ARGB8888,
>   };
>   
> +#ifdef __BIG_ENDIAN
> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
> +#else
> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
> +#endif

As these defines are only used here, would it be beneficial to put the 
__BIG_ENDIAN branch directly around the switch statement?

Best regards
Thomas

> +
>   uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
>   {
>   	uint32_t format;
>   
>   	switch (drm_fourcc) {
>   	case DRM_FORMAT_XRGB8888:
> -		format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
> +		format = VIRTIO_GPU_HOST_XRGB8888;
>   		break;
>   	case DRM_FORMAT_ARGB8888:
> -		format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
> +		format = VIRTIO_GPU_HOST_ARGB8888;
>   		break;
>   	default:
>   		/*

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Jocelyn Falempe]

On 20/08/2024 14:48, Thomas Zimmermann wrote:
> Hi
> 
> Am 20.08.24 um 11:07 schrieb Jocelyn Falempe:
>> The colors are inverted when testing a s390x VM on a s390x host.
>> Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
>> endian guests fixes the colors. But it may break big-endian guest on
>> little-endian host. In this case, the fix should be in qemu, because
>> the host endianess is not known in the guest VM.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
>> b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> index 860b5757ec3fc..0ec6ecc96eb13 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -37,16 +37,24 @@ static const uint32_t virtio_gpu_cursor_formats[] = {
>>       DRM_FORMAT_ARGB8888,
>>   };
>> +#ifdef __BIG_ENDIAN
>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
>> +#else
>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
>> +#endif
> 
> As these defines are only used here, would it be beneficial to put the 
> __BIG_ENDIAN branch directly around the switch statement?

That was my first version, but I found it difficult to read, when I mix 
#ifdef in a switch case.


or maybe something like the following would be better ?


  	switch (drm_fourcc) {
#ifdef _BIG_ENDIAN
  	case DRM_FORMAT_XRGB8888:
		format = VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM;
  		break;
  	case DRM_FORMAT_ARGB8888:
		format = VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM;
  		break;
#else
  	case DRM_FORMAT_XRGB8888:
		format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
  		break;
  	case DRM_FORMAT_ARGB8888:
		format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
  		break;
#endif
> 
> Best regards
> Thomas
> 
>> +
>>   uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
>>   {
>>       uint32_t format;
>>       switch (drm_fourcc) {
>>       case DRM_FORMAT_XRGB8888:
>> -        format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
>> +        format = VIRTIO_GPU_HOST_XRGB8888;
>>           break;
>>       case DRM_FORMAT_ARGB8888:
>> -        format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
>> +        format = VIRTIO_GPU_HOST_ARGB8888;
>>           break;
>>       default:
>>           /*
> 

Re: [PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Thomas Zimmermann]

Hi

Am 20.08.24 um 14:55 schrieb Jocelyn Falempe:
> On 20/08/2024 14:48, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 20.08.24 um 11:07 schrieb Jocelyn Falempe:
>>> The colors are inverted when testing a s390x VM on a s390x host.
>>> Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
>>> endian guests fixes the colors. But it may break big-endian guest on
>>> little-endian host. In this case, the fix should be in qemu, because
>>> the host endianess is not known in the guest VM.
>>>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>>> ---
>>>   drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
>>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
>>> b/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> index 860b5757ec3fc..0ec6ecc96eb13 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> @@ -37,16 +37,24 @@ static const uint32_t 
>>> virtio_gpu_cursor_formats[] = {
>>>       DRM_FORMAT_ARGB8888,
>>>   };
>>> +#ifdef __BIG_ENDIAN
>>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
>>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
>>> +#else
>>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
>>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
>>> +#endif
>>
>> As these defines are only used here, would it be beneficial to put 
>> the __BIG_ENDIAN branch directly around the switch statement?
>
> That was my first version, but I found it difficult to read, when I 
> mix #ifdef in a switch case.
>
>
> or maybe something like the following would be better ?
>
>
>      switch (drm_fourcc) {
> #ifdef _BIG_ENDIAN
>      case DRM_FORMAT_XRGB8888:
>         format = VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM;
>          break;
>      case DRM_FORMAT_ARGB8888:
>         format = VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM;
>          break;
> #else
>      case DRM_FORMAT_XRGB8888:
>         format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
>          break;
>      case DRM_FORMAT_ARGB8888:
>         format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
>          break;
> #endif

I have no preference. Maybe the virtio devs can comment.

Best regards
Thomas

>>
>> Best regards
>> Thomas
>>
>>> +
>>>   uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
>>>   {
>>>       uint32_t format;
>>>       switch (drm_fourcc) {
>>>       case DRM_FORMAT_XRGB8888:
>>> -        format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
>>> +        format = VIRTIO_GPU_HOST_XRGB8888;
>>>           break;
>>>       case DRM_FORMAT_ARGB8888:
>>> -        format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
>>> +        format = VIRTIO_GPU_HOST_ARGB8888;
>>>           break;
>>>       default:
>>>           /*
>>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Javier Martinez Canillas]

Jocelyn Falempe <jfalempe@redhat.com> writes:

> On 20/08/2024 14:48, Thomas Zimmermann wrote:
>> Hi
>> 
>> Am 20.08.24 um 11:07 schrieb Jocelyn Falempe:
>>> The colors are inverted when testing a s390x VM on a s390x host.
>>> Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
>>> endian guests fixes the colors. But it may break big-endian guest on
>>> little-endian host. In this case, the fix should be in qemu, because
>>> the host endianess is not known in the guest VM.
>>>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>>> ---
>>>   drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
>>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
>>> b/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> index 860b5757ec3fc..0ec6ecc96eb13 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>>> @@ -37,16 +37,24 @@ static const uint32_t virtio_gpu_cursor_formats[] = {
>>>       DRM_FORMAT_ARGB8888,
>>>   };
>>> +#ifdef __BIG_ENDIAN
>>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
>>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
>>> +#else
>>> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
>>> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
>>> +#endif
>> 
>> As these defines are only used here, would it be beneficial to put the 
>> __BIG_ENDIAN branch directly around the switch statement?
>
> That was my first version, but I found it difficult to read, when I mix 
> #ifdef in a switch case.
>
>
> or maybe something like the following would be better ?
>
>
>   	switch (drm_fourcc) {
> #ifdef _BIG_ENDIAN
>   	case DRM_FORMAT_XRGB8888:
> 		format = VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM;
>   		break;
>   	case DRM_FORMAT_ARGB8888:
> 		format = VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM;
>   		break;
> #else
>   	case DRM_FORMAT_XRGB8888:
> 		format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
>   		break;
>   	case DRM_FORMAT_ARGB8888:
> 		format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
>   		break;
> #endif

IMO your current patch is easier to read than having the ifdefery
in the switch statement.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 2/2] drm/virtio: Fix host color format for big endian guests

[Gerd Hoffmann]

On Tue, Aug 20, 2024 at 11:07:41AM GMT, Jocelyn Falempe wrote:
> The colors are inverted when testing a s390x VM on a s390x host.
> Changing the conversion from DRM_FORMAT -> VIRTIO_GPU_FORMAT on big
> endian guests fixes the colors. But it may break big-endian guest on
> little-endian host. In this case, the fix should be in qemu, because
> the host endianess is not known in the guest VM.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 860b5757ec3fc..0ec6ecc96eb13 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -37,16 +37,24 @@ static const uint32_t virtio_gpu_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
>  };
>  
> +#ifdef __BIG_ENDIAN
> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_X8R8G8B8_UNORM
> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM
> +#else
> +#define VIRTIO_GPU_HOST_XRGB8888 VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM
> +#define VIRTIO_GPU_HOST_ARGB8888 VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM
> +#endif

VIRTIO_GPU_FORMAT_* is little endian (like DRM_FORMAT_*), there should
be no need to do anything byte order specific here.  This looks like you
are papering over a bug somewhere else.

take care,
  Gerd

Re: [PATCH v2 1/2] drm/virtio: Use XRGB8888 also for big endian systems

[Gerd Hoffmann]

On Tue, Aug 20, 2024 at 11:07:40AM GMT, Jocelyn Falempe wrote:
> Mesa doesn't support BGRX8888, that means most wayland compositors
> don't work on big endian guests.

So you are doing a hard switch from native endian to little endian.

While this should be fine for modern userspace API (aka ADDFB2 ioctl) it
is not for older APIs (ADDFB ioctl, also fbdev emulation) where only
depth=32 is specified and userspace typically expects a framebuffer in
native byte order.

Ideally virtio-gpu would support both big endian and little endian
framebuffer formats (simliar to bochs drm driver).  That probably is a
somewhat more invasive change because the DRM_IOCTL_MODE_CREATE_DUMB
doesn't tell use the format which will be used.  Possible options I see:

  (1) Be lazy on creating host resources, i.e. call
      virtio_gpu_cmd_create_resource() not at DRM_IOCTL_MODE_CREATE_DUMB
      time but later when the resource will be actually be used (and
      specifically after DRM_IOCTL_MODE_ADDFB(2) ioctl so we know the
      format).  Needs additional state tracking (whenever the resource
      has been created or not) in possibly lots of places.

  (2) Support changing the resource format, i.e. in case
      DRM_IOCTL_MODE_ADDFB(2) is called with a format different from the
      current one go through a destroy-and-recreate cycle for the host
      resource.  Might have tricky corner cases (resource being in use
      when DRM_IOCTL_MODE_ADDFB(2) is called).

HTH & take care,
  Gerd

Re: [PATCH v2 1/2] drm/virtio: Use XRGB8888 also for big endian systems

[Jocelyn Falempe]

On 21/08/2024 13:12, Gerd Hoffmann wrote:
> On Tue, Aug 20, 2024 at 11:07:40AM GMT, Jocelyn Falempe wrote:
>> Mesa doesn't support BGRX8888, that means most wayland compositors
>> don't work on big endian guests.
> 
> So you are doing a hard switch from native endian to little endian.
> 
> While this should be fine for modern userspace API (aka ADDFB2 ioctl) it
> is not for older APIs (ADDFB ioctl, also fbdev emulation) where only
> depth=32 is specified and userspace typically expects a framebuffer in
> native byte order.
> 
> Ideally virtio-gpu would support both big endian and little endian
> framebuffer formats (simliar to bochs drm driver).  That probably is a
> somewhat more invasive change because the DRM_IOCTL_MODE_CREATE_DUMB
> doesn't tell use the format which will be used.  Possible options I see:
> 
>    (1) Be lazy on creating host resources, i.e. call
>        virtio_gpu_cmd_create_resource() not at DRM_IOCTL_MODE_CREATE_DUMB
>        time but later when the resource will be actually be used (and
>        specifically after DRM_IOCTL_MODE_ADDFB(2) ioctl so we know the
>        format).  Needs additional state tracking (whenever the resource
>        has been created or not) in possibly lots of places.
> 
>    (2) Support changing the resource format, i.e. in case
>        DRM_IOCTL_MODE_ADDFB(2) is called with a format different from the
>        current one go through a destroy-and-recreate cycle for the host
>        resource.  Might have tricky corner cases (resource being in use
>        when DRM_IOCTL_MODE_ADDFB(2) is called).

I've implemented (1), I will send a new series soon.

Thanks for your advice.

-- 

Jocelyn

> 
> HTH & take care,
>    Gerd
>