[RFC,v1,0/1] ui: Add a Wayland backend for Qemu UI

Message

Kasireddy, Vivek June 24, 2021, 4:10 a.m. UTC

Why does Qemu need a new Wayland UI backend?
The main reason why there needs to be a plain and simple Wayland backend
for Qemu UI is to eliminate the Blit (aka GPU copy) that happens if using
a toolkit like GTK or SDL (because they use EGL). The Blit can be eliminated
by sharing the dmabuf fd -- associated with the Guest scanout buffer --
directly with the Host compositor via the linux-dmabuf (unstable) protocol.
Once properly integrated, it would be potentially possible to have the
scanout buffer created by the Guest compositor be placed directly on a
hardware plane on the Host thereby improving performance. Only Guest 
compositors that use multiple back buffers (at-least 1 front and 1 back)
and virtio-gpu would benefit from this work.

The patch(es) are still WIP and the only reason why I am sending them now
is to get feedback and see if anyone thinks this work is interesting. And,
even after this work is complete, it is not meant to be merged and can be
used for performance testing purposes. Given Qemu UI's new direction, the
proper way to add new backends is to create a separate UI/display module
that is part of the dbus/pipewire infrastructure that Marc-Andre is
working on:
https://lists.nongnu.org/archive/html/qemu-devel/2021-03/msg04331.html

Cc: Gerd Hoffmann <[e-mail]>
Cc: Marc-André Lureau <[e-mail]>
Cc: Dongwon Kim <[e-mail]>
Cc: Tina Zhang <[e-mail]>

Vivek Kasireddy (1):
  ui: Add a plain Wayland backend for Qemu UI

 configure         |  17 ++
 meson.build       |  25 +++
 meson_options.txt |   2 +
 qapi/ui.json      |  19 ++-
 ui/meson.build    |  52 ++++++
 ui/wayland.c      | 402 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 516 insertions(+), 1 deletion(-)
 create mode 100644 ui/wayland.c

Comments

[e-mail] June 24, 2021, 4:30 a.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [e-mail]
Subject: [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
547ce45 ui: Add a plain Wayland backend for Qemu UI

=== OUTPUT BEGIN ===
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#260: 
new file mode 100644

WARNING: line over 80 characters
#266: FILE: ui/wayland.c:2:
+ * Wayland UI -- A simple Qemu UI backend to share buffers with Wayland compositors

ERROR: use QEMU instead of Qemu or QEmu
#266: FILE: ui/wayland.c:2:
+ * Wayland UI -- A simple Qemu UI backend to share buffers with Wayland compositors

ERROR: line over 90 characters
#271: FILE: ui/wayland.c:7:
+ * Mostly (boilerplate) based on cgit.freedesktop.org/wayland/weston/tree/clients/simple-dmabuf-egl.c

ERROR: code indent should never use tabs
#318: FILE: ui/wayland.c:54:
+^I^I^I     uint32_t serial)$

ERROR: code indent should never use tabs
#329: FILE: ui/wayland.c:65:
+^I^I^I      int32_t width, int32_t height,$

ERROR: code indent should never use tabs
#330: FILE: ui/wayland.c:66:
+^I^I^I      struct wl_array *states)$

ERROR: code indent should never use tabs
#459: FILE: ui/wayland.c:195:
+^Ierror_report("Can't find free buffer\n");$

ERROR: Error messages should not contain newlines
#459: FILE: ui/wayland.c:195:
+       error_report("Can't find free buffer\n");

ERROR: code indent should never use tabs
#519: FILE: ui/wayland.c:255:
+^I^I uint32_t format, uint32_t modifier_hi, uint32_t modifier_lo)$

ERROR: code indent should never use tabs
#552: FILE: ui/wayland.c:288:
+^I^I^I                 id, &wl_compositor_interface, 1);$

ERROR: code indent should never use tabs
#554: FILE: ui/wayland.c:290:
+^Id->wm_base = wl_registry_bind(registry,$

ERROR: code indent should never use tabs
#555: FILE: ui/wayland.c:291:
+^I^I^I^I      id, &xdg_wm_base_interface, 1);$

ERROR: code indent should never use tabs
#556: FILE: ui/wayland.c:292:
+^Ixdg_wm_base_add_listener(d->wm_base, &wm_base_listener, d);$

ERROR: code indent should never use tabs
#558: FILE: ui/wayland.c:294:
+^Id->fshell = wl_registry_bind(registry,$

ERROR: code indent should never use tabs
#559: FILE: ui/wayland.c:295:
+^I                             id, &zwp_fullscreen_shell_v1_interface,$

ERROR: code indent should never use tabs
#560: FILE: ui/wayland.c:296:
+^I                             1);$

ERROR: code indent should never use tabs
#562: FILE: ui/wayland.c:298:
+^Id->dmabuf = wl_registry_bind(registry,$

ERROR: code indent should never use tabs
#563: FILE: ui/wayland.c:299:
+^I                             id, &zwp_linux_dmabuf_v1_interface, 3);$

ERROR: code indent should never use tabs
#564: FILE: ui/wayland.c:300:
+^Izwp_linux_dmabuf_v1_add_listener(d->dmabuf, &dmabuf_listener,$

ERROR: code indent should never use tabs
#565: FILE: ui/wayland.c:301:
+^I                                 d);$

ERROR: code indent should never use tabs
#593: FILE: ui/wayland.c:329:
+^Ierror_report("No zwp_linux_dmabuf global\n");$

ERROR: Error messages should not contain newlines
#593: FILE: ui/wayland.c:329:
+       error_report("No zwp_linux_dmabuf global\n");

ERROR: code indent should never use tabs
#594: FILE: ui/wayland.c:330:
+^Iexit(1);$

ERROR: code indent should never use tabs
#609: FILE: ui/wayland.c:345:
+^I                                                  window->surface);$

ERROR: code indent should never use tabs
#621: FILE: ui/wayland.c:357:
+^I                                        window->surface,$

ERROR: line over 90 characters
#622: FILE: ui/wayland.c:358:
+                                               ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_DEFAULT,

ERROR: code indent should never use tabs
#622: FILE: ui/wayland.c:358:
+^I^I                                ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_DEFAULT,$

ERROR: code indent should never use tabs
#623: FILE: ui/wayland.c:359:
+^I^I                                NULL);$

total: 27 errors, 2 warnings, 607 lines checked

Commit 547ce45b800d (ui: Add a plain Wayland backend for Qemu UI) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Gerd Hoffmann June 24, 2021, 8:28 a.m. UTC | #2

On Wed, Jun 23, 2021 at 09:10:39PM -0700, Vivek Kasireddy wrote:
> Why does Qemu need a new Wayland UI backend?
> The main reason why there needs to be a plain and simple Wayland backend
> for Qemu UI is to eliminate the Blit (aka GPU copy) that happens if using
> a toolkit like GTK or SDL (because they use EGL). The Blit can be eliminated
> by sharing the dmabuf fd -- associated with the Guest scanout buffer --
> directly with the Host compositor via the linux-dmabuf (unstable) protocol.

Hmm, that probably means no window decorations (and other UI elements),
right?  Also the code seems to not (yet?) handle mouse and kbd input.

> The patch(es) are still WIP and the only reason why I am sending them now
> is to get feedback and see if anyone thinks this work is interesting. And,
> even after this work is complete, it is not meant to be merged and can be
> used for performance testing purposes. Given Qemu UI's new direction, the
> proper way to add new backends is to create a separate UI/display module
> that is part of the dbus/pipewire infrastructure that Marc-Andre is
> working on:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-03/msg04331.html

Separating emulation and UI has the big advantage that the guest
lifecycle is decoupled from the desktop session lifecycle, i.e.
the guest can continue to run when the desktop session ends.

Works today with spice (when using unix socket to connect it can pass
dma-buf handles from qemu to spice client).

Using dbus instead certainly makes sense.  Whenever we'll just go send
dma-buf handles over dbus or integrate with pipewire for display/sound
not clear yet.  Marc-André thinks using pipewire doesn't bring benefits
and I havn't found the time yet to learn more about pipewire ...

take care,
  Gerd

Kasireddy, Vivek June 24, 2021, 6:17 p.m. UTC | #3

Hi Gerd,

> > Why does Qemu need a new Wayland UI backend?
> > The main reason why there needs to be a plain and simple Wayland backend
> > for Qemu UI is to eliminate the Blit (aka GPU copy) that happens if using
> > a toolkit like GTK or SDL (because they use EGL). The Blit can be eliminated
> > by sharing the dmabuf fd -- associated with the Guest scanout buffer --
> > directly with the Host compositor via the linux-dmabuf (unstable) protocol.
> 
> Hmm, that probably means no window decorations (and other UI elements),
[Kasireddy, Vivek] Right, unfortunately, no decorations or other UI elements. For
that we can use GTK. 
> right?  Also the code seems to not (yet?) handle mouse and kbd input.
[Kasireddy, Vivek] Yes, kbd and mouse support not added yet and that is why I
tagged it as WIP. But it should not be too hard to add that.

> 
> > The patch(es) are still WIP and the only reason why I am sending them now
> > is to get feedback and see if anyone thinks this work is interesting. And,
> > even after this work is complete, it is not meant to be merged and can be
> > used for performance testing purposes. Given Qemu UI's new direction, the
> > proper way to add new backends is to create a separate UI/display module
> > that is part of the dbus/pipewire infrastructure that Marc-Andre is
> > working on:
> > https://lists.nongnu.org/archive/html/qemu-devel/2021-03/msg04331.html
> 
> Separating emulation and UI has the big advantage that the guest
> lifecycle is decoupled from the desktop session lifecycle, i.e.
> the guest can continue to run when the desktop session ends.
> 
> Works today with spice (when using unix socket to connect it can pass
> dma-buf handles from qemu to spice client).
> 
> Using dbus instead certainly makes sense.  Whenever we'll just go send
> dma-buf handles over dbus or integrate with pipewire for display/sound
> not clear yet.  Marc-André thinks using pipewire doesn't bring benefits
> and I havn't found the time yet to learn more about pipewire ...
[Kasireddy, Vivek] On our side, we'll also try to learn how dbus and pipewire
fit in and work. Having said that, can Marc-Andre's work be merged in 
stages -- first only dbus and no pipewire?

Thanks,
Vivek
> 
> take care,
>   Gerd