RE: [PATCH v4 3/3] ui/console: Introduce dpy_gl_create_dmabuf() helper

2024-03-22 Thread Kim, Dongwon
Hi Marc-André,

> -Original Message-
> From: Marc-André Lureau 
> Sent: Friday, March 22, 2024 2:06 AM
> To: Kim, Dongwon 
> Cc: qemu-devel@nongnu.org; phi...@linaro.org
> Subject: Re: [PATCH v4 3/3] ui/console: Introduce dpy_gl_create_dmabuf()
> helper
> 
> Hi
> 
> On Fri, Mar 22, 2024 at 3:45 AM  wrote:
> >
> > From: Dongwon Kim 
> >
> > dpy_gl_create_dmabuf() allocates QemuDmaBuf and initialize fields.
> > hw/display modules, hw/vfio and ui/dbus-listener now use this method
> > to create QemuDmaBuf instead of declaring and initializing it on their
> > own.
> >
> > Cc: Philippe Mathieu-Daudé 
> > Cc: Marc-André Lureau 
> > 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|  6 ++
> >  hw/display/vhost-user-gpu.c | 33 ++---
> >  hw/display/virtio-gpu-udmabuf.c | 23 ---
> >  hw/vfio/display.c   | 26 +++---
> >  ui/console.c| 28 
> >  ui/dbus-listener.c  | 28 
> >  8 files changed, 86 insertions(+), 64 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
> > d5334a806c..01e998264b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -358,6 +358,12 @@ 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_create_dmabuf(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);
> >  uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf);  uint32_t
> > dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf);  int32_t
> > dpy_gl_dmabuf_get_fd(QemuDmaBuf *dmabuf); diff --git
> > a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c index
> > 709c8a02a1..0e49a934ed 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);
> > +uint64_t modifier = 0;
> >  QemuDmaBuf *dmabuf;
> >
> >  if (m->scanout_id >= g->parent_obj.conf.max_outputs) { @@
> > -261,30 +262,32 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
> > VhostUserGpuMsg *msg)
> >
> >  g->parent_obj.enable = 1;
> >  con = g->

Re: [PATCH v4 3/3] ui/console: Introduce dpy_gl_create_dmabuf() helper

2024-03-22 Thread Marc-André Lureau
Hi

On Fri, Mar 22, 2024 at 3:45 AM  wrote:
>
> From: Dongwon Kim 
>
> dpy_gl_create_dmabuf() allocates QemuDmaBuf and initialize fields.
> hw/display modules, hw/vfio and ui/dbus-listener now use this method
> to create QemuDmaBuf instead of declaring and initializing it on their
> own.
>
> Cc: Philippe Mathieu-Daudé 
> Cc: Marc-André Lureau 
> 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|  6 ++
>  hw/display/vhost-user-gpu.c | 33 ++---
>  hw/display/virtio-gpu-udmabuf.c | 23 ---
>  hw/vfio/display.c   | 26 +++---
>  ui/console.c| 28 
>  ui/dbus-listener.c  | 28 
>  8 files changed, 86 insertions(+), 64 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 d5334a806c..01e998264b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,6 +358,12 @@ 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_create_dmabuf(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);
>  uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf);
>  int32_t dpy_gl_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 709c8a02a1..0e49a934ed 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);
> +uint64_t modifier = 0;
>  QemuDmaBuf *dmabuf;
>
>  if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
> @@ -261,30 +262,32 @@ 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);
> -dmabuf->fd = -1;
> +dmabuf = g->dmabuf[m->scanout_id];
> +if (dmabuf) {
> +int dmabuf_fd = dpy_gl_dmabuf_get_fd(dmabuf);
> +if (dmabuf_fd >= 0) {
> +close(dmabuf_fd);
> +}
> +dpy_gl_release_dmabuf(con, dmabuf);
>  }
> -dpy_gl_release_dmabuf(con, dmabuf);
> +
>  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 = &msg->payload.dmabuf_scano