| Message ID | 20240820090908.151042-1-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/virtio: Use XRGB8888 also for big endian systems | expand |
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
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.
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