loader: disable virgl driver when no 3D for virtio

Submitted by Lepton Wu on April 5, 2018, 5:16 a.m.

Details

Message ID 20180405051600.57300-2-lepton@chromium.org
State New
Headers show
Series "loader: disable virgl driver when no 3D for virtio" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Lepton Wu April 5, 2018, 5:16 a.m.
If user are running mesa under old version of qemu or have turned off
GL at runtime, virtio gpu driver actually doesn't work. Adding a detection
here can make sure same disk image work with both cases.

Signed-off-by: Lepton Wu <lepton@chromium.org>
---
 src/loader/loader.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/loader/loader.c b/src/loader/loader.c
index 43275484cc..2a689c52d6 100644
--- a/src/loader/loader.c
+++ b/src/loader/loader.c
@@ -381,6 +381,27 @@  out:
    log_(driver ? _LOADER_DEBUG : _LOADER_WARNING,
          "pci id for fd %d: %04x:%04x, driver %s\n",
          fd, vendor_id, chip_id, driver);
+   if (!strcmp(driver, "virtio_gpu")) {
+      struct drm_virtgpu_getparam {
+         uint64_t param;
+         uint64_t value;
+      };
+      #define VIRTGPU_PARAM_3D_FEATURES 1 /* do we have 3D features in the hw */
+      struct drm_virtgpu_getparam args;
+      uint32_t gl = 0;
+      args.param = VIRTGPU_PARAM_3D_FEATURES;
+      args.value = (uint64_t)(uintptr_t)&gl;
+      #define DRM_VIRTGPU_GETPARAM    0x03
+      #define DRM_IOCTL_VIRTGPU_GETPARAM \
+         DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_GETPARAM,\
+                  struct drm_virtgpu_getparam)
+      int ret = drmIoctl(fd, DRM_IOCTL_VIRTGPU_GETPARAM, &args);
+      if (ret || !gl) {
+         /* Actually there is no virtio_2d driver, mesa will
+          * fallback to software driver */
+         return strdup("virtio_gpu_2d");
+      }
+   }
    return driver;
 }
 

Comments

Shouldn't this just be handled as in, e.g.,

https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c#n97

i.e. return an error in the driver-specific loader? This create
function should fail:

https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c#n897

On Thu, Apr 5, 2018 at 1:16 AM, Lepton Wu <lepton@chromium.org> wrote:
> If user are running mesa under old version of qemu or have turned off
> GL at runtime, virtio gpu driver actually doesn't work. Adding a detection
> here can make sure same disk image work with both cases.
>
> Signed-off-by: Lepton Wu <lepton@chromium.org>
> ---
>  src/loader/loader.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/src/loader/loader.c b/src/loader/loader.c
> index 43275484cc..2a689c52d6 100644
> --- a/src/loader/loader.c
> +++ b/src/loader/loader.c
> @@ -381,6 +381,27 @@ out:
>     log_(driver ? _LOADER_DEBUG : _LOADER_WARNING,
>           "pci id for fd %d: %04x:%04x, driver %s\n",
>           fd, vendor_id, chip_id, driver);
> +   if (!strcmp(driver, "virtio_gpu")) {
> +      struct drm_virtgpu_getparam {
> +         uint64_t param;
> +         uint64_t value;
> +      };
> +      #define VIRTGPU_PARAM_3D_FEATURES 1 /* do we have 3D features in the hw */
> +      struct drm_virtgpu_getparam args;
> +      uint32_t gl = 0;
> +      args.param = VIRTGPU_PARAM_3D_FEATURES;
> +      args.value = (uint64_t)(uintptr_t)&gl;
> +      #define DRM_VIRTGPU_GETPARAM    0x03
> +      #define DRM_IOCTL_VIRTGPU_GETPARAM \
> +         DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_GETPARAM,\
> +                  struct drm_virtgpu_getparam)
> +      int ret = drmIoctl(fd, DRM_IOCTL_VIRTGPU_GETPARAM, &args);
> +      if (ret || !gl) {
> +         /* Actually there is no virtio_2d driver, mesa will
> +          * fallback to software driver */
> +         return strdup("virtio_gpu_2d");
> +      }
> +   }
>     return driver;
>  }
>
> --
> 2.17.0.484.g0c8726318c-goog
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 5 April 2018 at 14:45, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> Shouldn't this just be handled as in, e.g.,
>
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c#n97
>
> i.e. return an error in the driver-specific loader? This create
> function should fail:
>
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c#n897
>
Indeed. The radeon and amdgpu drivers also use a similar approach.
Be that to check if the kernel module is too old, or a required
feature is missing.

Keeping the ioctl in virgl_drm_winsys_create, alongside it's brethren
would help with consistency.

-Emil