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

[Jocelyn Falempe <jfalempe@redhat.com>]

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:
>>           /*
>