Re: [PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers
On Wed, Apr 17, 2024 at 8:14 AM wrote:
>
> From: Dongwon Kim
>
> This commit introduces utility functions for the creation and deallocation
> of QemuDmaBuf instances. Additionally, it updates all relevant sections
> of the codebase to utilize these new utility functions.
>
> Suggested-by: Marc-André Lureau
> Cc: Philippe Mathieu-Daudé
> Cc: Vivek Kasireddy
> Signed-off-by: Dongwon Kim
> ---
> include/hw/vfio/vfio-common.h | 2 +-
> include/hw/virtio/virtio-gpu.h | 4 ++--
> include/ui/console.h| 8 +++-
> hw/display/vhost-user-gpu.c | 32 +--
> hw/display/virtio-gpu-udmabuf.c | 24 +--
> hw/vfio/display.c | 26 -
> ui/console.c| 34 +
> ui/dbus-listener.c | 28 ---
> 8 files changed, 95 insertions(+), 63 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..d66e27db02 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -148,7 +148,7 @@ typedef struct VFIOGroup {
> } VFIOGroup;
>
> typedef struct VFIODMABuf {
> -QemuDmaBuf buf;
> +QemuDmaBuf *buf;
> uint32_t pos_x, pos_y, pos_updates;
> uint32_t hot_x, hot_y, hot_updates;
> int dmabuf_id;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ed44cdad6b..56d6e821bf 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
> DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
>
> typedef struct VGPUDMABuf {
> -QemuDmaBuf buf;
> +QemuDmaBuf *buf;
> uint32_t scanout_id;
> QTAILQ_ENTRY(VGPUDMABuf) next;
> } VGPUDMABuf;
> @@ -238,7 +238,7 @@ struct VhostUserGPU {
> VhostUserBackend *vhost;
> int vhost_gpu_fd; /* closed by the chardev */
> CharBackend vhost_chr;
> -QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> +QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> bool backend_blocked;
> };
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 3d9d8b9fce..6d7c03b7c5 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf
> *dmabuf,
>bool have_hot, uint32_t hot_x, uint32_t hot_y);
> void dpy_gl_cursor_position(QemuConsole *con,
> uint32_t pos_x, uint32_t pos_y);
> -
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> + uint32_t stride, uint32_t x,
> + uint32_t y, uint32_t backing_width,
> + uint32_t backing_height, uint32_t fourcc,
> + uint64_t modifier, uint32_t dmabuf_fd,
An fd is an int.
> + bool allow_fences, bool y0_top);
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
> uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 87dcfbca10..4d8461e94a 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -250,6 +250,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> VhostUserGpuMsg *msg)
> VhostUserGpuDMABUFScanout *m = >payload.dmabuf_scanout;
> int fd = qemu_chr_fe_get_msgfd(>vhost_chr);
> int old_fd;
> +uint64_t modifier = 0;
> QemuDmaBuf *dmabuf;
>
> if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
> @@ -262,31 +263,34 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> VhostUserGpuMsg *msg)
>
> g->parent_obj.enable = 1;
> con = g->parent_obj.scanout[m->scanout_id].con;
> -dmabuf = >dmabuf[m->scanout_id];
> -old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> -if (old_fd >= 0) {
> -close(old_fd);
> -dmabuf->fd = -1;
> +dmabuf = g->dmabuf[m->scanout_id];
> +if (dmabuf) {
> +old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> +if (old_fd >= 0) {
> +close(old_fd);
> +dpy_gl_qemu_dmabuf_set_fd(dmabuf, -1);
> +}
> }
> dpy_gl_release_dmabuf(con, dmabuf);
> +g_clear_pointer(, dpy_gl_qemu_dmabuf_free);
> if (fd == -1) {
> dpy_gl_scanout_disable(con);
> break;
> }
> -*dmabuf = (QemuDmaBuf) {
> -.fd = fd,
> -.width = m->fd_width,
> -.height = m->fd_height,
> -.stride = m->fd_stride,
> -.fourcc = m->fd_drm_fourcc,
> -.y0_top = m->fd_flags &
Re: [PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers
On Tue, Apr 16, 2024 at 09:09:54PM -0700, dongwon@intel.com wrote:
> From: Dongwon Kim
>
> This commit introduces utility functions for the creation and deallocation
> of QemuDmaBuf instances. Additionally, it updates all relevant sections
> of the codebase to utilize these new utility functions.
>
> Suggested-by: Marc-André Lureau
> Cc: Philippe Mathieu-Daudé
> Cc: Vivek Kasireddy
> Signed-off-by: Dongwon Kim
> ---
> include/hw/vfio/vfio-common.h | 2 +-
> include/hw/virtio/virtio-gpu.h | 4 ++--
> include/ui/console.h| 8 +++-
> hw/display/vhost-user-gpu.c | 32 +--
> hw/display/virtio-gpu-udmabuf.c | 24 +--
> hw/vfio/display.c | 26 -
> ui/console.c| 34 +
> ui/dbus-listener.c | 28 ---
> 8 files changed, 95 insertions(+), 63 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..d66e27db02 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -148,7 +148,7 @@ typedef struct VFIOGroup {
> } VFIOGroup;
>
> typedef struct VFIODMABuf {
> -QemuDmaBuf buf;
> +QemuDmaBuf *buf;
> uint32_t pos_x, pos_y, pos_updates;
> uint32_t hot_x, hot_y, hot_updates;
> int dmabuf_id;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ed44cdad6b..56d6e821bf 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
> DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
>
> typedef struct VGPUDMABuf {
> -QemuDmaBuf buf;
> +QemuDmaBuf *buf;
> uint32_t scanout_id;
> QTAILQ_ENTRY(VGPUDMABuf) next;
> } VGPUDMABuf;
> @@ -238,7 +238,7 @@ struct VhostUserGPU {
> VhostUserBackend *vhost;
> int vhost_gpu_fd; /* closed by the chardev */
> CharBackend vhost_chr;
> -QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> +QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> bool backend_blocked;
> };
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 3d9d8b9fce..6d7c03b7c5 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf
> *dmabuf,
>bool have_hot, uint32_t hot_x, uint32_t hot_y);
> void dpy_gl_cursor_position(QemuConsole *con,
> uint32_t pos_x, uint32_t pos_y);
> -
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> + uint32_t stride, uint32_t x,
> + uint32_t y, uint32_t backing_width,
> + uint32_t backing_height, uint32_t fourcc,
> + uint64_t modifier, uint32_t dmabuf_fd,
> + bool allow_fences, bool y0_top);
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);
Since you're creating a new allocator/deallocator pair for
this, please also call G_DEFINE_AUTOPTR_CLEANUP_FUNC so this
struct becomes usable with g_autoptr(), even if we don't
currently have an immediate need to use g_autoptr.
> int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
> uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> diff --git a/ui/console.c b/ui/console.c
> index d4ca9e6e0f..ea23fd8af6 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1132,6 +1132,40 @@ void dpy_gl_cursor_position(QemuConsole *con,
> }
> }
>
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> + uint32_t stride, uint32_t x,
> + uint32_t y, uint32_t backing_width,
> + uint32_t backing_height, uint32_t fourcc,
> + uint64_t modifier, uint32_t dmabuf_fd,
> + bool allow_fences, bool y0_top) {
> +QemuDmaBuf *dmabuf;
> +
> +dmabuf = g_new0(QemuDmaBuf, 1);
> +
> +dmabuf->width = width;
> +dmabuf->height = height;
> +dmabuf->stride = stride;
> +dmabuf->x = x;
> +dmabuf->y = y;
> +dmabuf->backing_width = backing_width;
> +dmabuf->backing_height = backing_height;
> +dmabuf->fourcc = fourcc;
> +dmabuf->modifier = modifier;
> +dmabuf->fd = dmabuf_fd;
> +dmabuf->allow_fences = allow_fences;
> +dmabuf->y0_top = y0_top;
> +dmabuf->fence_fd = -1;
> +
> +return dmabuf;
> +}
> +
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> +{
> +assert(dmabuf != NULL);
It is normal practice for all XXX_free() funcs to
accept NULL as a valid argument, and operate as
a no-op. This makes code more robust, as it can
blindly call free without
[PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers
From: Dongwon Kim
This commit introduces utility functions for the creation and deallocation
of QemuDmaBuf instances. Additionally, it updates all relevant sections
of the codebase to utilize these new utility functions.
Suggested-by: Marc-André Lureau
Cc: Philippe Mathieu-Daudé
Cc: Vivek Kasireddy
Signed-off-by: Dongwon Kim
---
include/hw/vfio/vfio-common.h | 2 +-
include/hw/virtio/virtio-gpu.h | 4 ++--
include/ui/console.h| 8 +++-
hw/display/vhost-user-gpu.c | 32 +--
hw/display/virtio-gpu-udmabuf.c | 24 +--
hw/vfio/display.c | 26 -
ui/console.c| 34 +
ui/dbus-listener.c | 28 ---
8 files changed, 95 insertions(+), 63 deletions(-)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b9da6c08ef..d66e27db02 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -148,7 +148,7 @@ typedef struct VFIOGroup {
} VFIOGroup;
typedef struct VFIODMABuf {
-QemuDmaBuf buf;
+QemuDmaBuf *buf;
uint32_t pos_x, pos_y, pos_updates;
uint32_t hot_x, hot_y, hot_updates;
int dmabuf_id;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ed44cdad6b..56d6e821bf 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
typedef struct VGPUDMABuf {
-QemuDmaBuf buf;
+QemuDmaBuf *buf;
uint32_t scanout_id;
QTAILQ_ENTRY(VGPUDMABuf) next;
} VGPUDMABuf;
@@ -238,7 +238,7 @@ struct VhostUserGPU {
VhostUserBackend *vhost;
int vhost_gpu_fd; /* closed by the chardev */
CharBackend vhost_chr;
-QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
+QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
bool backend_blocked;
};
diff --git a/include/ui/console.h b/include/ui/console.h
index 3d9d8b9fce..6d7c03b7c5 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf
*dmabuf,
bool have_hot, uint32_t hot_x, uint32_t hot_y);
void dpy_gl_cursor_position(QemuConsole *con,
uint32_t pos_x, uint32_t pos_y);
-
+QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
+ uint32_t stride, uint32_t x,
+ uint32_t y, uint32_t backing_width,
+ uint32_t backing_height, uint32_t fourcc,
+ uint64_t modifier, uint32_t dmabuf_fd,
+ bool allow_fences, bool y0_top);
+void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);
int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 87dcfbca10..4d8461e94a 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -250,6 +250,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
VhostUserGpuMsg *msg)
VhostUserGpuDMABUFScanout *m = >payload.dmabuf_scanout;
int fd = qemu_chr_fe_get_msgfd(>vhost_chr);
int old_fd;
+uint64_t modifier = 0;
QemuDmaBuf *dmabuf;
if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
@@ -262,31 +263,34 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
VhostUserGpuMsg *msg)
g->parent_obj.enable = 1;
con = g->parent_obj.scanout[m->scanout_id].con;
-dmabuf = >dmabuf[m->scanout_id];
-old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
-if (old_fd >= 0) {
-close(old_fd);
-dmabuf->fd = -1;
+dmabuf = g->dmabuf[m->scanout_id];
+if (dmabuf) {
+old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
+if (old_fd >= 0) {
+close(old_fd);
+dpy_gl_qemu_dmabuf_set_fd(dmabuf, -1);
+}
}
dpy_gl_release_dmabuf(con, dmabuf);
+g_clear_pointer(, dpy_gl_qemu_dmabuf_free);
if (fd == -1) {
dpy_gl_scanout_disable(con);
break;
}
-*dmabuf = (QemuDmaBuf) {
-.fd = fd,
-.width = m->fd_width,
-.height = m->fd_height,
-.stride = m->fd_stride,
-.fourcc = m->fd_drm_fourcc,
-.y0_top = m->fd_flags & VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP,
-};
+
if (msg->request == VHOST_USER_GPU_DMABUF_SCANOUT2) {
VhostUserGpuDMABUFScanout2 *m2 = >payload.dmabuf_scanout2;
-dmabuf->modifier = m2->modifier;
+modifier = m2->modifier;
}
+dmabuf =
