[v4,1/3] ui/console: Introduce dpy_gl_dmabuf_get_height/width() helpers

From: Dongwon Kim <dongwon.kim@intel.com>

dpy_gl_dmabuf_get_height() and dpy_gl_dmabuf_get_width() are helpers for
retrieving width and height fields from QemuDmaBuf struct.

Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/console.h            |  2 ++
 hw/display/virtio-gpu-udmabuf.c |  7 ++++---
 hw/vfio/display.c               |  9 ++++++---
 ui/console.c                    | 18 ++++++++++++++++++
 4 files changed, 30 insertions(+), 6 deletions(-)

Hi Kim

On Fri, Mar 22, 2024 at 3:45 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> dpy_gl_dmabuf_get_height() and dpy_gl_dmabuf_get_width() are helpers for
> retrieving width and height fields from QemuDmaBuf struct.
>

There are many places left where width/height fields are still
accessed directly.

If we want to make the whole structure private, we will probably need setters.

I don't see why the function should silently return 0 when given NULL.
Imho an assert(dmabuf != NULL) is appropriate (or
g_return_val_if_fail).





> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/ui/console.h            |  2 ++
>  hw/display/virtio-gpu-udmabuf.c |  7 ++++---
>  hw/vfio/display.c               |  9 ++++++---
>  ui/console.c                    | 18 ++++++++++++++++++
>  4 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 0bc7a00ac0..6064487fc4 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,6 +358,8 @@ 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);
> +uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf);
>  void dpy_gl_release_dmabuf(QemuConsole *con,
>                             QemuDmaBuf *dmabuf);
>  void dpy_gl_update(QemuConsole *con,
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d51184d658..a4ebf828ec 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -206,6 +206,7 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>  {
>      struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
>      VGPUDMABuf *new_primary, *old_primary = NULL;
> +    uint32_t width, height;
>
>      new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r);
>      if (!new_primary) {
> @@ -216,10 +217,10 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>          old_primary = g->dmabuf.primary[scanout_id];
>      }
>
> +    width = dpy_gl_dmabuf_get_width(&new_primary->buf);
> +    height = dpy_gl_dmabuf_get_height(&new_primary->buf);
>      g->dmabuf.primary[scanout_id] = new_primary;
> -    qemu_console_resize(scanout->con,
> -                        new_primary->buf.width,
> -                        new_primary->buf.height);
> +    qemu_console_resize(scanout->con, width, height);
>      dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
>
>      if (old_primary) {
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 1aa440c663..c962e5f88f 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -286,6 +286,7 @@ static void vfio_display_dmabuf_update(void *opaque)
>      VFIOPCIDevice *vdev = opaque;
>      VFIODisplay *dpy = vdev->dpy;
>      VFIODMABuf *primary, *cursor;
> +    uint32_t width, height;
>      bool free_bufs = false, new_cursor = false;
>
>      primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
> @@ -296,10 +297,12 @@ static void vfio_display_dmabuf_update(void *opaque)
>          return;
>      }
>
> +    width = dpy_gl_dmabuf_get_width(&primary->buf);
> +    height = dpy_gl_dmabuf_get_height(&primary->buf);
> +
>      if (dpy->dmabuf.primary != primary) {
>          dpy->dmabuf.primary = primary;
> -        qemu_console_resize(dpy->con,
> -                            primary->buf.width, primary->buf.height);
> +        qemu_console_resize(dpy->con, width, height);
>          dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
>          free_bufs = true;
>      }
> @@ -328,7 +331,7 @@ static void vfio_display_dmabuf_update(void *opaque)
>          cursor->pos_updates = 0;
>      }
>
> -    dpy_gl_update(dpy->con, 0, 0, primary->buf.width, primary->buf.height);
> +    dpy_gl_update(dpy->con, 0, 0, width, height);
>
>      if (free_bufs) {
>          vfio_display_free_dmabufs(vdev);
> diff --git a/ui/console.c b/ui/console.c
> index 43226c5c14..1d0513a733 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1132,6 +1132,24 @@ void dpy_gl_cursor_position(QemuConsole *con,
>      }
>  }
>
> +uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf)
> +{
> +    if (dmabuf) {
> +        return dmabuf->width;
> +    }
> +
> +    return 0;
> +}
> +
> +uint32_t dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf)
> +{
> +    if (dmabuf) {
> +        return dmabuf->height;
> +    }
> +
> +    return 0;
> +}
> +
>  void dpy_gl_release_dmabuf(QemuConsole *con,
>                            QemuDmaBuf *dmabuf)
>  {
> --
> 2.34.1
>
>


--
Marc-André Lureau

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@gmail.com>
> Sent: Friday, March 22, 2024 2:06 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org; philmd@linaro.org
> Subject: Re: [PATCH v4 1/3] ui/console: Introduce
> dpy_gl_dmabuf_get_height/width() helpers
> 
> Hi Kim
> 
> On Fri, Mar 22, 2024 at 3:45 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > dpy_gl_dmabuf_get_height() and dpy_gl_dmabuf_get_width() are helpers
> > for retrieving width and height fields from QemuDmaBuf struct.
> >
> 
> There are many places left where width/height fields are still accessed directly.
> 
> If we want to make the whole structure private, we will probably need setters.
[Kim, Dongwon]  I am wondering if you are saying we need setters and getters for all individual fields and use those new functions in any places in QEMU where any of those fields are accessed including ui/* (e.g. gtk-egl.c)? 

> 
> I don't see why the function should silently return 0 when given NULL.
> Imho an assert(dmabuf != NULL) is appropriate (or g_return_val_if_fail).
[Kim, Dongwon] Yeah I can do that. I will update that part.

> 
> 
> 
> 
> 
> > Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  include/ui/console.h            |  2 ++
> >  hw/display/virtio-gpu-udmabuf.c |  7 ++++---
> >  hw/vfio/display.c               |  9 ++++++---
> >  ui/console.c                    | 18 ++++++++++++++++++
> >  4 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h index
> > 0bc7a00ac0..6064487fc4 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -358,6 +358,8 @@ 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);
> > +uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf);
> >  void dpy_gl_release_dmabuf(QemuConsole *con,
> >                             QemuDmaBuf *dmabuf);  void
> > dpy_gl_update(QemuConsole *con, diff --git
> > a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> > index d51184d658..a4ebf828ec 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -206,6 +206,7 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,  {
> >      struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
> >      VGPUDMABuf *new_primary, *old_primary = NULL;
> > +    uint32_t width, height;
> >
> >      new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r);
> >      if (!new_primary) {
> > @@ -216,10 +217,10 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
> >          old_primary = g->dmabuf.primary[scanout_id];
> >      }
> >
> > +    width = dpy_gl_dmabuf_get_width(&new_primary->buf);
> > +    height = dpy_gl_dmabuf_get_height(&new_primary->buf);
> >      g->dmabuf.primary[scanout_id] = new_primary;
> > -    qemu_console_resize(scanout->con,
> > -                        new_primary->buf.width,
> > -                        new_primary->buf.height);
> > +    qemu_console_resize(scanout->con, width, height);
> >      dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
> >
> >      if (old_primary) {
> > diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> > index 1aa440c663..c962e5f88f 100644
> > --- a/hw/vfio/display.c
> > +++ b/hw/vfio/display.c
> > @@ -286,6 +286,7 @@ static void vfio_display_dmabuf_update(void
> *opaque)
> >      VFIOPCIDevice *vdev = opaque;
> >      VFIODisplay *dpy = vdev->dpy;
> >      VFIODMABuf *primary, *cursor;
> > +    uint32_t width, height;
> >      bool free_bufs = false, new_cursor = false;
> >
> >      primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
> > @@ -296,10 +297,12 @@ static void vfio_display_dmabuf_update(void
> *opaque)
> >          return;
> >      }
> >
> > +    width = dpy_gl_dmabuf_get_width(&primary->buf);
> > +    height = dpy_gl_dmabuf_get_height(&primary->buf);
> > +
> >      if (dpy->dmabuf.primary != primary) {
> >          dpy->dmabuf.primary = primary;
> > -        qemu_console_resize(dpy->con,
> > -                            primary->buf.width, primary->buf.height);
> > +        qemu_console_resize(dpy->con, width, height);
> >          dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
> >          free_bufs = true;
> >      }
> > @@ -328,7 +331,7 @@ static void vfio_display_dmabuf_update(void
> *opaque)
> >          cursor->pos_updates = 0;
> >      }
> >
> > -    dpy_gl_update(dpy->con, 0, 0, primary->buf.width, primary->buf.height);
> > +    dpy_gl_update(dpy->con, 0, 0, width, height);
> >
> >      if (free_bufs) {
> >          vfio_display_free_dmabufs(vdev);
> > diff --git a/ui/console.c b/ui/console.c
> > index 43226c5c14..1d0513a733 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -1132,6 +1132,24 @@ void dpy_gl_cursor_position(QemuConsole *con,
> >      }
> >  }
> >
> > +uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf)
> > +{
> > +    if (dmabuf) {
> > +        return dmabuf->width;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +uint32_t dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf)
> > +{
> > +    if (dmabuf) {
> > +        return dmabuf->height;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  void dpy_gl_release_dmabuf(QemuConsole *con,
> >                            QemuDmaBuf *dmabuf)
> >  {
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Marc-André Lureau