drm/virtio: fix DRM_FORMAT_* handling
authorGerd Hoffmann <kraxel@redhat.com>
Fri, 21 Sep 2018 13:47:03 +0000 (15:47 +0200)
committerGerd Hoffmann <kraxel@redhat.com>
Tue, 25 Sep 2018 12:49:49 +0000 (14:49 +0200)
Use DRM_FORMAT_HOST_XRGB8888, so we are using the correct format code
on bigendian machines.  Also set the quirk_addfb_prefer_host_byte_order
mode_config bit so drm_mode_addfb() asks for the correct format code.

Both DRM_FORMAT_* and VIRTIO_GPU_FORMAT_* are defined to be little
endian, so using a different mapping on bigendian machines is wrong.
It's there because of broken drm_mode_addfb() behavior.  So with
drm_mode_addfb() being fixed we can fix this too.

While wading through the code I've noticed we have a little issue in
virtio:  We attach a format to the bo when it is created
(DRM_IOCTL_MODE_CREATE_DUMB), not when we map it as framebuffer
(DRM_IOCTL_MODE_ADDFB).  Easy way out:  Support a single format only.
Pick DRM_FORMAT_HOST_XRGB8888, it is the only one actually used in
practice.  Drop unused mappings in virtio_gpu_translate_format().

With this patch applied both ADDFB and ADDFB2 ioctls work correctly in
the virtio-gpu.ko driver on big endian machines.  Without the patch only
ADDFB (which still seems to be used by the majority of userspace) works
correctly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20180921134704.12826-6-kraxel@redhat.com
drivers/gpu/drm/virtio/virtgpu_display.c
drivers/gpu/drm/virtio/virtgpu_fb.c
drivers/gpu/drm/virtio/virtgpu_gem.c
drivers/gpu/drm/virtio/virtgpu_plane.c

index 0379d68976594b9f9aec084fc80e54da7224e669..8f8fed471e34b296d7d2efce36deb877656e61a6 100644 (file)
@@ -307,6 +307,10 @@ 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)
+               return ERR_PTR(-ENOENT);
+
        /* lookup object associated with res handle */
        obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
        if (!obj)
@@ -355,6 +359,7 @@ int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
        int i;
 
        drm_mode_config_init(vgdev->ddev);
+       vgdev->ddev->mode_config.quirk_addfb_prefer_host_byte_order = true;
        vgdev->ddev->mode_config.funcs = &virtio_gpu_mode_funcs;
        vgdev->ddev->mode_config.helper_private = &virtio_mode_config_helpers;
 
index 3364b0970dd2e82c31947bd70818b4d252f8fd31..e1e41efabec1d51c4b68b23b682d61d0c936c10d 100644 (file)
@@ -226,7 +226,7 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
        mode_cmd.width = sizes->surface_width;
        mode_cmd.height = sizes->surface_height;
        mode_cmd.pitches[0] = mode_cmd.width * 4;
-       mode_cmd.pixel_format = drm_mode_legacy_fb_format(32, 24);
+       mode_cmd.pixel_format = DRM_FORMAT_HOST_XRGB8888;
 
        format = virtio_gpu_translate_format(mode_cmd.pixel_format);
        if (format == 0)
index 0f2768eacaee58b12dfd4f48c8bacdc286f8209f..82c817f37cf7d68ebe2cf72fe022978fb1b02291 100644 (file)
@@ -90,7 +90,10 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
        uint32_t resid;
        uint32_t format;
 
-       pitch = args->width * ((args->bpp + 1) / 8);
+       if (args->bpp != 32)
+               return -EINVAL;
+
+       pitch = args->width * 4;
        args->size = pitch * args->height;
        args->size = ALIGN(args->size, PAGE_SIZE);
 
@@ -99,7 +102,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
        if (ret)
                goto fail;
 
-       format = virtio_gpu_translate_format(DRM_FORMAT_XRGB8888);
+       format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888);
        virtio_gpu_resource_id_get(vgdev, &resid);
        virtio_gpu_cmd_create_resource(vgdev, resid, format,
                                       args->width, args->height);
index 682a977d68c1485cbdd29cf75576966dfe7fde48..a9f4ae7d448395a8b8596499305fe872ab0b9bbe 100644 (file)
 #include <drm/drm_atomic_helper.h>
 
 static const uint32_t virtio_gpu_formats[] = {
-       DRM_FORMAT_XRGB8888,
-       DRM_FORMAT_ARGB8888,
-       DRM_FORMAT_BGRX8888,
-       DRM_FORMAT_BGRA8888,
-       DRM_FORMAT_RGBX8888,
-       DRM_FORMAT_RGBA8888,
-       DRM_FORMAT_XBGR8888,
-       DRM_FORMAT_ABGR8888,
+       DRM_FORMAT_HOST_XRGB8888,
 };
 
 static const uint32_t virtio_gpu_cursor_formats[] = {
-#ifdef __BIG_ENDIAN
-       DRM_FORMAT_BGRA8888,
-#else
-       DRM_FORMAT_ARGB8888,
-#endif
+       DRM_FORMAT_HOST_ARGB8888,
 };
 
 uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
@@ -51,32 +40,6 @@ uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
        uint32_t format;
 
        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;
-       case DRM_FORMAT_BGRX8888:
-               format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
-               break;
-       case DRM_FORMAT_BGRA8888:
-               format = VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM;
-               break;
-       case DRM_FORMAT_RGBX8888:
-               format = VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM;
-               break;
-       case DRM_FORMAT_RGBA8888:
-               format = VIRTIO_GPU_FORMAT_R8G8B8A8_UNORM;
-               break;
-       case DRM_FORMAT_XBGR8888:
-               format = VIRTIO_GPU_FORMAT_X8B8G8R8_UNORM;
-               break;
-       case DRM_FORMAT_ABGR8888:
-               format = VIRTIO_GPU_FORMAT_A8B8G8R8_UNORM;
-               break;
-#else
        case DRM_FORMAT_XRGB8888:
                format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM;
                break;
@@ -89,19 +52,6 @@ uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
        case DRM_FORMAT_BGRA8888:
                format = VIRTIO_GPU_FORMAT_A8R8G8B8_UNORM;
                break;
-       case DRM_FORMAT_RGBX8888:
-               format = VIRTIO_GPU_FORMAT_X8B8G8R8_UNORM;
-               break;
-       case DRM_FORMAT_RGBA8888:
-               format = VIRTIO_GPU_FORMAT_A8B8G8R8_UNORM;
-               break;
-       case DRM_FORMAT_XBGR8888:
-               format = VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM;
-               break;
-       case DRM_FORMAT_ABGR8888:
-               format = VIRTIO_GPU_FORMAT_R8G8B8A8_UNORM;
-               break;
-#endif
        default:
                /*
                 * This should not happen, we handle everything listed