RE: [PATCH 00/11] Add support for Blob resources feature

Kasireddy, Vivek posted 11 patches 3 years ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH 00/11] Add support for Blob resources feature
Posted by Kasireddy, Vivek 3 years ago
Hi Gerd,
While looking at the Qemu UI code, I noticed that there is a Blit operation
performed to copy the Guest FB (texture) into a Host buffer before it is presented
to the Host compositor. I was wondering if there are any elegant ways to
eliminate this Blit to further the goal of absolute zero-copy. One idea that I am 
familiar with involves the usage of this extension:
https://www.khronos.org/registry/EGL/extensions/WL/EGL_WL_create_wayland_buffer_from_image.txt

However, I do realize that this is very Wayland specific but we are also going to need
to add explicit sync support which is currently only available in upstream Weston. 
I did try adding explicit sync support to GTK but the maintainers are not particularly
thrilled about it:
https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3410

Any other ideas as to how to eliminate that Blit cleanly?

Thanks,
Vivek

> -----Original Message-----
> From: Kasireddy, Vivek <vivek.kasireddy@intel.com>
> Sent: Tuesday, March 30, 2021 8:10 PM
> To: qemu-devel@nongnu.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Marc-André Lureau <marcandre.lureau@redhat.com>; Kim,
> Dongwon <dongwon.kim@intel.com>; Zhang, Tina <tina.zhang@intel.com>
> Subject: [PATCH 00/11] Add support for Blob resources feature
> 
> Enabling this feature would eliminate data copies from the resource object in the Guest to
> the shadow resource in Qemu. This patch series however adds support only for Blobs of
> type VIRTIO_GPU_BLOB_MEM_GUEST with property
> VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE.
> 
> Most of the patches in this series are a rebased, refactored and bugfixed versions of Gerd
> Hoffmann's patches located here:
> https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next
> 
> TODO:
> - Enable the combination virgl + blob resources
> - Add support for VIRTGPU_BLOB_MEM_HOST3D blobs
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Cc: Tina Zhang <tina.zhang@intel.com>
> 
> Vivek Kasireddy (11):
>   ui: Get the fd associated with udmabuf driver
>   ui/pixman: Add qemu_pixman_to_drm_format()
>   virtio-gpu: Add udmabuf helpers
>   virtio-gpu: Add virtio_gpu_find_check_resource
>   virtio-gpu: Refactor virtio_gpu_set_scanout
>   virtio-gpu: Refactor virtio_gpu_create_mapping_iov
>   virtio-gpu: Add initial definitions for blob resources
>   virtio-gpu: Add virtio_gpu_resource_create_blob
>   virtio-gpu: Add helpers to create and destroy dmabuf objects
>   virtio-gpu: Add virtio_gpu_set_scanout_blob
>   virtio-gpu: Update cursor data using blob
> 
>  hw/display/meson.build                      |   2 +-
>  hw/display/trace-events                     |   2 +
>  hw/display/virtio-gpu-3d.c                  |   3 +-
>  hw/display/virtio-gpu-base.c                |   3 +
>  hw/display/virtio-gpu-udmabuf.c             | 276 +++++++++++++
>  hw/display/virtio-gpu.c                     | 423 +++++++++++++++-----
>  include/hw/virtio/virtio-gpu-bswap.h        |  16 +
>  include/hw/virtio/virtio-gpu.h              |  41 +-
>  include/standard-headers/linux/udmabuf.h    |  32 ++
>  include/standard-headers/linux/virtio_gpu.h |   1 +
>  include/ui/console.h                        |   3 +
>  include/ui/qemu-pixman.h                    |   1 +
>  scripts/update-linux-headers.sh             |   3 +
>  ui/meson.build                              |   1 +
>  ui/qemu-pixman.c                            |  35 +-
>  ui/udmabuf.c                                |  40 ++
>  16 files changed, 772 insertions(+), 110 deletions(-)  create mode 100644
> hw/display/virtio-gpu-udmabuf.c  create mode 100644 include/standard-
> headers/linux/udmabuf.h
>  create mode 100644 ui/udmabuf.c
> 
> --
> 2.26.2

Re: [PATCH 00/11] Add support for Blob resources feature
Posted by Gerd Hoffmann 3 years ago
  Hi,

> Any other ideas as to how to eliminate that Blit cleanly?

Well, "cleanly" pretty much implies "supported by toolkit".

gtk glarea for example sets up a framebuffer and expects the application
render to that framebuffer.  So qemu glarea code does a fb-to-fb blit.

Other reasons are scaling and cursor rendering.  Not all reasons apply
to all UIs.  I think when using spice qemu doesn't blit (not fully sure
what happens inside spice-server), but it could very well be that the
spice-client does the blit instead, i.e. we just shift the issue to
another place ...

take care,
  Gerd


RE: [PATCH 00/11] Add support for Blob resources feature
Posted by Kasireddy, Vivek 3 years ago
Hi Gerd,

> 
> > Any other ideas as to how to eliminate that Blit cleanly?
> 
> Well, "cleanly" pretty much implies "supported by toolkit".
[Kasireddy, Vivek] I was kind of hoping you'd not draw that implication :)
> 
> gtk glarea for example sets up a framebuffer and expects the application render to that
> framebuffer.  So qemu glarea code does a fb-to-fb blit.
[Kasireddy, Vivek] Right, that is how it works today but we'd like to not have that
blit and instead submit the Qemu application buffer (i.e Guest FB) directly to the
compositor  -- so that it can be placed on a hardware plane should the compositor
decide to do so. Only then it'd be absolute zero-copy but the compositor may decide
to composite it which would mean another blit that we cannot avoid. 

I am trying to make a concerted effort to accomplish this using GTK/GLArea:
https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3410

But I get a feeling that it is inadequate as GTK/GLArea does not manage the wl_buffers
submitted to the compositor -- EGL does. I suspect we either need to use a new GTK
mechanism -- that perhaps does not exist yet -- or not use GTK at all for this.

I do understand that adding a new purely Wayland backend would make it redundant given
that GTK, SDL, Spice, etc already support Wayland; however, I do not see any good options
available for eliminating that blit.

Thanks,
Vivek

> 
> Other reasons are scaling and cursor rendering.  Not all reasons apply to all UIs.  I think
> when using spice qemu doesn't blit (not fully sure what happens inside spice-server), but it
> could very well be that the spice-client does the blit instead, i.e. we just shift the issue to
> another place ...
> 
> take care,
>   Gerd


Re: [PATCH 00/11] Add support for Blob resources feature
Posted by Gerd Hoffmann 3 years ago
  Hi,

> But I get a feeling that it is inadequate as GTK/GLArea does not manage the wl_buffers
> submitted to the compositor -- EGL does. I suspect we either need to use a new GTK
> mechanism -- that perhaps does not exist yet -- or not use GTK at all for this.
> 
> I do understand that adding a new purely Wayland backend would make it redundant given
> that GTK, SDL, Spice, etc already support Wayland; however, I do not see any good options
> available for eliminating that blit.

Well, one idea is using dbus (discovery/setup) and pipewire (data
streams) to allow other applications access the guest display (also
audio, input, ...).  Simliar to gnome exporting the wayland display
that way for remote access and screen sharing.

pipewire supports using dmabufs, so that should allow to decouple user
interface code from qemu without making compromises on performance,
which is probably quite useful for a number of use cases, like a
zero-copy wayland client, or a client using drm directly.

Right now pipewire support is at "idea" level, there isn't even a
proof-of-concept for that.  So it surely doesn't help short-term, but
IMHO this makes alot of sense medium/long-term.

Marc-André has experimental code for a dbus-based UI for qemu.  It
doesn't use pipewire as data transport though.  At least the first
version posted a while ago @ qemu-devel doesn't.

take care,
  Gerd


RE: [PATCH 00/11] Add support for Blob resources feature
Posted by Kasireddy, Vivek 3 years ago
Hi Gerd,

> > I do understand that adding a new purely Wayland backend would make it
> > redundant given that GTK, SDL, Spice, etc already support Wayland;
> > however, I do not see any good options available for eliminating that blit.
> 
> Well, one idea is using dbus (discovery/setup) and pipewire (data
> streams) to allow other applications access the guest display (also audio, input, ...).
> Simliar to gnome exporting the wayland display that way for remote access and screen
> sharing.
> 
> pipewire supports using dmabufs, so that should allow to decouple user interface code
> from qemu without making compromises on performance, which is probably quite useful
> for a number of use cases, like a zero-copy wayland client, or a client using drm directly.
[Kasireddy, Vivek] We considered having a separate client but decided that adding the
relevant pieces to Qemu UI would be sufficient. We also felt that the interaction between
the client and Qemu for sharing the dmabuf (guest FB) would add to the overhead and
exacerbate synchronization issues. And, maintaining and distributing such a client would 
probably not be prudent for us right now.

> 
> Right now pipewire support is at "idea" level, there isn't even a proof-of-concept for that.
> So it surely doesn't help short-term, but IMHO this makes alot of sense medium/long-
> term.
[Kasireddy, Vivek] Ok, we'll explore this idea further to see if it would work for our use-case. 

> 
> Marc-André has experimental code for a dbus-based UI for qemu.  It doesn't use pipewire
> as data transport though.  At least the first version posted a while ago @ qemu-devel
> doesn't.
[Kasireddy, Vivek] What is the main motivation for a new dbus-based UI for Qemu?
Also, would you be able to review the patches in this series soon?

Thanks,
Vivek

> 
> take care,
>   Gerd


Re: [PATCH 00/11] Add support for Blob resources feature
Posted by Gerd Hoffmann 3 years ago
  Hi,

> > Marc-André has experimental code for a dbus-based UI for qemu.  It doesn't use pipewire
> > as data transport though.  At least the first version posted a while ago @ qemu-devel
> > doesn't.
> [Kasireddy, Vivek] What is the main motivation for a new dbus-based UI for Qemu?

Having UI and qemu run in separate processes has serveral advantages,
for starters the VM is independent from the desktop session.  Today you
can use vnc or spice, the dbus UI should allow better performance and
integration than that.  If it works out well we maybe we can even drop
the sdl/gtk UIs.

> Also, would you be able to review the patches in this series soon?

I expect there will be a v2 anyway due to the kernel-side changes?

take care,
  Gerd