On Mon, Apr 22, 2024 at 07:22:50PM -0700, dongwon....@intel.com wrote:
> From: Dongwon Kim <dongwon....@intel.com>
>
> This commit updates all instances where fields within the QemuDmaBuf
> struct are directly accessed, replacing them with calls to these new
> helper functions.
>
> v6: fix typos in helper names in ui/spice-display.c
>
> v7: removed prefix, "dpy_gl_" from all helpers
>
> v8: Introduction of helpers was removed as those were already added
> by the previous commit
>
> Suggested-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Philippe Mathieu-Daudé <phi...@linaro.org>
> Cc: Daniel P. Berrangé <berra...@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> Signed-off-by: Dongwon Kim <dongwon....@intel.com>
> ---
> hw/display/vhost-user-gpu.c | 6 ++--
> hw/display/virtio-gpu-udmabuf.c | 7 +++--
> hw/vfio/display.c | 15 +++++++---
> ui/console.c | 4 +--
> ui/dbus-console.c | 9 ++++--
> ui/dbus-listener.c | 43 +++++++++++++++++-----------
> ui/egl-headless.c | 23 ++++++++++-----
> ui/egl-helpers.c | 47 ++++++++++++++++++-------------
> ui/gtk-egl.c | 48 ++++++++++++++++++++-----------
> ui/gtk-gl-area.c | 37 ++++++++++++++++--------
> ui/gtk.c | 6 ++--
> ui/spice-display.c | 50 +++++++++++++++++++--------------
> 12 files changed, 187 insertions(+), 108 deletions(-)
>
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 709c8a02a1..ea9a6c5d10 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -249,6 +249,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> VhostUserGpuMsg *msg)
> case VHOST_USER_GPU_DMABUF_SCANOUT: {
> VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
> int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
> + int old_fd;
> QemuDmaBuf *dmabuf;
>
> if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
> @@ -262,8 +263,9 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> VhostUserGpuMsg *msg)
> g->parent_obj.enable = 1;
> con = g->parent_obj.scanout[m->scanout_id].con;
> dmabuf = &g->dmabuf[m->scanout_id];
> - if (dmabuf->fd >= 0) {
> - close(dmabuf->fd);
> + old_fd = qemu_dmabuf_get_fd(dmabuf);
> + if (old_fd >= 0) {
> + close(old_fd);
> dmabuf->fd = -1;
I think we shoul hdave a qemu_dmabuf_close() method for this logic
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 1aa440c663..4861c8161d 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -259,9 +259,13 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice
> *vdev,
>
> static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf
> *dmabuf)
> {
> + int fd;
> +
> QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
> +
> + fd = qemu_dmabuf_get_fd(&dmabuf->buf);
> dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
> - close(dmabuf->buf.fd);
> + close(fd);
Again, we should have a qemu_dmabuf_close() method
> g_free(dmabuf);
> }
>
> } else {
> - trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id,
> - dmabuf->width,
> dmabuf->height);
> + stride = qemu_dmabuf_get_stride(dmabuf);
> + fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> + y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> + fd = qemu_dmabuf_get_fd(dmabuf);
This is where it would use a qemu_dmabuf_dup_fd(), avoiding
the subsequent dup() in the method call
> +
> + trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> /* note: spice server will close the fd, so hand over a dup */
> - spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
> - dmabuf->width, dmabuf->height,
> - dmabuf->stride, dmabuf->fourcc,
> - dmabuf->y0_top);
> + spice_qxl_gl_scanout(&ssd->qxl, dup(fd), width, height,
> + stride, fourcc, y_0_top);
> }
> - qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width,
> dmabuf->height);
> + qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> ssd->guest_dmabuf_refresh = false;
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
