spice: allow to specify drm rendernode

[Qemu-devel] [PATCH] spice: allow to specify drm rendernode

Posted by Marc-André Lureau 7 years, 2 months ago

When multiple GPU are available, picking the first one isn't always the
best choice. Learn to specify a device rendernode.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/egl-helpers.h |  3 +--
 ui/egl-helpers.c         | 10 +++++++---
 ui/spice-core.c          |  5 ++++-
 qemu-options.hx          |  6 +++++-
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 03fcf4bba2..88a13e827b 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -14,8 +14,7 @@ extern int qemu_egl_rn_fd;
 extern struct gbm_device *qemu_egl_rn_gbm_dev;
 extern EGLContext qemu_egl_rn_ctx;
 
-int qemu_egl_rendernode_open(void);
-int egl_rendernode_init(void);
+int egl_rendernode_init(const char *rendernode);
 int egl_get_fd_for_texture(uint32_t tex_id, EGLint *stride, EGLint *fourcc);
 
 #endif
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index cd24568a5e..417462b76d 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -44,13 +44,17 @@ int qemu_egl_rn_fd;
 struct gbm_device *qemu_egl_rn_gbm_dev;
 EGLContext qemu_egl_rn_ctx;
 
-int qemu_egl_rendernode_open(void)
+static int qemu_egl_rendernode_open(const char *rendernode)
 {
     DIR *dir;
     struct dirent *e;
     int r, fd;
     char *p;
 
+    if (rendernode) {
+        return open(rendernode, O_RDWR | O_CLOEXEC | O_NOCTTY | O_NONBLOCK);
+    }
+
     dir = opendir("/dev/dri");
     if (!dir) {
         return -1;
@@ -85,11 +89,11 @@ int qemu_egl_rendernode_open(void)
     return fd;
 }
 
-int egl_rendernode_init(void)
+int egl_rendernode_init(const char *rendernode)
 {
     qemu_egl_rn_fd = -1;
 
-    qemu_egl_rn_fd = qemu_egl_rendernode_open();
+    qemu_egl_rn_fd = qemu_egl_rendernode_open(rendernode);
     if (qemu_egl_rn_fd == -1) {
         error_report("egl: no drm render node available");
         goto err;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 1452e77fd1..39ccab7561 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -501,6 +501,9 @@ static QemuOptsList qemu_spice_opts = {
         },{
             .name = "gl",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "rendernode",
+            .type = QEMU_OPT_STRING,
 #endif
         },
         { /* end of list */ }
@@ -833,7 +836,7 @@ void qemu_spice_init(void)
                          "incompatible with -spice port/tls-port");
             exit(1);
         }
-        if (egl_rendernode_init() != 0) {
+        if (egl_rendernode_init(qemu_opt_get(opts, "rendernode")) != 0) {
             error_report("Failed to initialize EGL render node for SPICE GL");
             exit(1);
         }
diff --git a/qemu-options.hx b/qemu-options.hx
index ac036b4cbd..b64087d934 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1066,7 +1066,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
     "       [,streaming-video=[off|all|filter]][,disable-copy-paste]\n"
     "       [,disable-agent-file-xfer][,agent-mouse=[on|off]]\n"
     "       [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
-    "       [,gl=[on|off]]\n"
+    "       [,gl=[on|off]][,rendernode=<file>]\n"
     "   enable spice\n"
     "   at least one of {port, tls-port} is mandatory\n",
     QEMU_ARCH_ALL)
@@ -1161,6 +1161,10 @@ Enable/disable spice seamless migration. Default is off.
 @item gl=[on|off]
 Enable/disable OpenGL context. Default is off.
 
+@item rendernode=<file>
+DRM render node for OpenGL rendering. If not specified, it will pick
+the first available. (Since 2.9)
+
 @end table
 ETEXI
 
-- 
2.11.0.295.gd7dffce1c.dirty

Re: [Qemu-devel] [PATCH] spice: allow to specify drm rendernode

Posted by Marc-André Lureau 7 years, 2 months ago

Hi Gerd,

Rendernode are not stable across reboots, it depends on module order.

I have further patches to specify a pci address instead:
rendernode=<file|pciaddr>. Does that seem fine?

Do you want me to squash and resend?

The current branch: https://github.com/elmarco/qemu/commits/spice

thanks

On Sun, Feb 12, 2017 at 3:23 PM Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:

> When multiple GPU are available, picking the first one isn't always the
> best choice. Learn to specify a device rendernode.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/egl-helpers.h |  3 +--
>  ui/egl-helpers.c         | 10 +++++++---
>  ui/spice-core.c          |  5 ++++-
>  qemu-options.hx          |  6 +++++-
>  4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index 03fcf4bba2..88a13e827b 100644
> --- a/include/ui/egl-helpers.h
> +++ b/include/ui/egl-helpers.h
> @@ -14,8 +14,7 @@ extern int qemu_egl_rn_fd;
>  extern struct gbm_device *qemu_egl_rn_gbm_dev;
>  extern EGLContext qemu_egl_rn_ctx;
>
> -int qemu_egl_rendernode_open(void);
> -int egl_rendernode_init(void);
> +int egl_rendernode_init(const char *rendernode);
>  int egl_get_fd_for_texture(uint32_t tex_id, EGLint *stride, EGLint
> *fourcc);
>
>  #endif
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index cd24568a5e..417462b76d 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -44,13 +44,17 @@ int qemu_egl_rn_fd;
>  struct gbm_device *qemu_egl_rn_gbm_dev;
>  EGLContext qemu_egl_rn_ctx;
>
> -int qemu_egl_rendernode_open(void)
> +static int qemu_egl_rendernode_open(const char *rendernode)
>  {
>      DIR *dir;
>      struct dirent *e;
>      int r, fd;
>      char *p;
>
> +    if (rendernode) {
> +        return open(rendernode, O_RDWR | O_CLOEXEC | O_NOCTTY |
> O_NONBLOCK);
> +    }
> +
>      dir = opendir("/dev/dri");
>      if (!dir) {
>          return -1;
> @@ -85,11 +89,11 @@ int qemu_egl_rendernode_open(void)
>      return fd;
>  }
>
> -int egl_rendernode_init(void)
> +int egl_rendernode_init(const char *rendernode)
>  {
>      qemu_egl_rn_fd = -1;
>
> -    qemu_egl_rn_fd = qemu_egl_rendernode_open();
> +    qemu_egl_rn_fd = qemu_egl_rendernode_open(rendernode);
>      if (qemu_egl_rn_fd == -1) {
>          error_report("egl: no drm render node available");
>          goto err;
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 1452e77fd1..39ccab7561 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -501,6 +501,9 @@ static QemuOptsList qemu_spice_opts = {
>          },{
>              .name = "gl",
>              .type = QEMU_OPT_BOOL,
> +        },{
> +            .name = "rendernode",
> +            .type = QEMU_OPT_STRING,
>  #endif
>          },
>          { /* end of list */ }
> @@ -833,7 +836,7 @@ void qemu_spice_init(void)
>                           "incompatible with -spice port/tls-port");
>              exit(1);
>          }
> -        if (egl_rendernode_init() != 0) {
> +        if (egl_rendernode_init(qemu_opt_get(opts, "rendernode")) != 0) {
>              error_report("Failed to initialize EGL render node for SPICE
> GL");
>              exit(1);
>          }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ac036b4cbd..b64087d934 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1066,7 +1066,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
>      "       [,streaming-video=[off|all|filter]][,disable-copy-paste]\n"
>      "       [,disable-agent-file-xfer][,agent-mouse=[on|off]]\n"
>      "
>  [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
> -    "       [,gl=[on|off]]\n"
> +    "       [,gl=[on|off]][,rendernode=<file>]\n"
>      "   enable spice\n"
>      "   at least one of {port, tls-port} is mandatory\n",
>      QEMU_ARCH_ALL)
> @@ -1161,6 +1161,10 @@ Enable/disable spice seamless migration. Default is
> off.
>  @item gl=[on|off]
>  Enable/disable OpenGL context. Default is off.
>
> +@item rendernode=<file>
> +DRM render node for OpenGL rendering. If not specified, it will pick
> +the first available. (Since 2.9)
> +
>  @end table
>  ETEXI
>
> --
> 2.11.0.295.gd7dffce1c.dirty
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH] spice: allow to specify drm rendernode

Posted by Gerd Hoffmann 7 years, 2 months ago

On Mo, 2017-02-13 at 08:37 +0000, Marc-André Lureau wrote:
> Hi Gerd,
> 
> 
> Rendernode are not stable across reboots, it depends on module order.
> 
> I have further patches to specify a pci address instead:
> rendernode=<file|pciaddr>. Does that seem fine?

No, IMO we should use udev rules to generate stable paths then
(/dev/disk/by-* style).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] spice: allow to specify drm rendernode

Posted by Marc-André Lureau 7 years, 2 months ago

Hi

On Mon, Feb 13, 2017 at 5:32 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mo, 2017-02-13 at 08:37 +0000, Marc-André Lureau wrote:
> > Hi Gerd,
> >
> >
> > Rendernode are not stable across reboots, it depends on module order.
> >
> > I have further patches to specify a pci address instead:
> > rendernode=<file|pciaddr>. Does that seem fine?
>
> No, IMO we should use udev rules to generate stable paths then
> (/dev/disk/by-* style).
>

Yes, we are discussing this approach on libvirt mailing list, I will work
on it.

In the meantime, this patch should be good enough for 2.9 hopefully.

Thanks
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] spice: allow to specify drm rendernode

Posted by Gerd Hoffmann 7 years, 2 months ago

  Hi,

> In the meantime, this patch should be good enough for 2.9 hopefully.

Sure, I'll take it.

cheers,
  Gerd