RE: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-23 Thread Kim, Dongwon
Hi Daniel,

> -Original Message-
> From: Daniel P. Berrangé 
> Sent: Tuesday, April 23, 2024 7:07 AM
> To: Kim, Dongwon 
> Cc: qemu-devel@nongnu.org; marcandre.lur...@redhat.com;
> phi...@linaro.org
> Subject: Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for
> QemuDmaBuf struct and helpers
> 
> On Mon, Apr 22, 2024 at 07:22:49PM -0700, dongwon@intel.com wrote:
> > From: Dongwon Kim 
> >
> > New header and source files are added for containing QemuDmaBuf struct
> > definition and newly introduced helpers for creating/freeing the
> > struct and accessing its data.
> >
> > v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
> >  GPL to be in line with QEMU's default license
> >  (Daniel P. Berrangé )
> >
> > Suggested-by: Marc-André Lureau 
> > Cc: Philippe Mathieu-Daudé 
> > Cc: Daniel P. Berrangé 
> > Cc: Vivek Kasireddy 
> > Signed-off-by: Dongwon Kim 
> > ---
> >  include/ui/console.h |  20 +
> >  include/ui/dmabuf.h  |  64 +++
> >  ui/dmabuf.c  | 189 +++
> >  ui/meson.build   |   1 +
> >  4 files changed, 255 insertions(+), 19 deletions(-)  create mode
> > 100644 include/ui/dmabuf.h  create mode 100644 ui/dmabuf.c
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h index
> > 0bc7a00ac0..a208a68b88 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -7,6 +7,7 @@
> >  #include "qapi/qapi-types-ui.h"
> >  #include "ui/input.h"
> >  #include "ui/surface.h"
> > +#include "ui/dmabuf.h"
> >
> >  #define TYPE_QEMU_CONSOLE "qemu-console"
> >  OBJECT_DECLARE_TYPE(QemuConsole, QemuConsoleClass, QEMU_CONSOLE)
> @@
> > -185,25 +186,6 @@ struct QEMUGLParams {
> >  int minor_ver;
> >  };
> >
> > -typedef struct QemuDmaBuf {
> > -int   fd;
> > -uint32_t  width;
> > -uint32_t  height;
> > -uint32_t  stride;
> > -uint32_t  fourcc;
> > -uint64_t  modifier;
> > -uint32_t  texture;
> > -uint32_t  x;
> > -uint32_t  y;
> > -uint32_t  backing_width;
> > -uint32_t  backing_height;
> > -bool  y0_top;
> > -void  *sync;
> > -int   fence_fd;
> > -bool  allow_fences;
> > -bool  draw_submitted;
> > -} QemuDmaBuf;
> > -
> >  enum display_scanout {
> >  SCANOUT_NONE,
> >  SCANOUT_SURFACE,
> > diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h new file mode
> > 100644 index 00..7a60116ee6
> > --- /dev/null
> > +++ b/include/ui/dmabuf.h
> > @@ -0,0 +1,64 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * QemuDmaBuf struct and helpers used for accessing its data
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef DMABUF_H
> > +#define DMABUF_H
> > +
> > +typedef struct QemuDmaBuf {
> > +int   fd;
> > +uint32_t  width;
> > +uint32_t  height;
> > +uint32_t  stride;
> > +uint32_t  fourcc;
> > +uint64_t  modifier;
> > +uint32_t  texture;
> > +uint32_t  x;
> > +uint32_t  y;
> > +uint32_t  backing_width;
> > +uint32_t  backing_height;
> > +bool  y0_top;
> > +void  *sync;
> > +int   fence_fd;
> > +bool  allow_fences;
> > +bool  draw_submitted;
> > +} QemuDmaBuf;
> > +
> > +QemuDmaBuf *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, int32_t
> > +dmabuf_fd,
> 
> Should be 'int' not 'int32_t' for FDs.
> 
> > +   bool allow_fences, bool y0_top);
> > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> > +
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf, qemu_dmabuf_free);
> > +
> > +int32_t qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> 
> Again should be 'int' not 'int42_t'
> 
> I think there ought to also be a
> 
>   int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf);
> 
> to do the dup() call in one go too
> 
> > diff --git a/ui/dmabuf.c b/ui/dmabuf.c new file mode 100644 index
> > 00..f447cce4fe
> > --- /dev/null
> > +++ b/ui/dmabuf.c
> 
> > +
> > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> > +{
> > +if (dmabuf == NULL) {
> > +return;
> > +}
> > +
> 
> I think this method should be made to call
> 
>   qemu_dmabuf_close()
> 
> to release the FD, if not already released, otherwise
> this method could be a resource leak.

[Kim, Dongwon]  Daniel, I initially agreed with you on the idea that is we 
should close dmabuf->fd here in freeing routine and I already submitted v11 
containing that change but I just found it causes some regression at least for 
virtio-gpu case because this fd is for udmabuf bound to the actual resource. 

Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-23 Thread Daniel P . Berrangé
On Tue, Apr 23, 2024 at 07:05:20PM +, Kim, Dongwon wrote:
> Hi Daniel,
> 
> > -Original Message-
> > From: Daniel P. Berrangé 
> > Sent: Tuesday, April 23, 2024 7:07 AM
> > To: Kim, Dongwon 
> > Cc: qemu-devel@nongnu.org; marcandre.lur...@redhat.com;
> > phi...@linaro.org
> > Subject: Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for
> > QemuDmaBuf struct and helpers
> > 
> > On Mon, Apr 22, 2024 at 07:22:49PM -0700, dongwon@intel.com wrote:
> > > From: Dongwon Kim 
> > >
> > > New header and source files are added for containing QemuDmaBuf struct
> > > definition and newly introduced helpers for creating/freeing the
> > > struct and accessing its data.
> > >
> > > v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
> > >  GPL to be in line with QEMU's default license
> > >  (Daniel P. Berrangé )
> > >
> > > Suggested-by: Marc-André Lureau 
> > > Cc: Philippe Mathieu-Daudé 
> > > Cc: Daniel P. Berrangé 
> > > Cc: Vivek Kasireddy 
> > > Signed-off-by: Dongwon Kim 
> > > ---
> > >  include/ui/console.h |  20 +
> > >  include/ui/dmabuf.h  |  64 +++
> > >  ui/dmabuf.c  | 189
> > +++
> > >  ui/meson.build   |   1 +
> > >  4 files changed, 255 insertions(+), 19 deletions(-)  create mode
> > > 100644 include/ui/dmabuf.h  create mode 100644 ui/dmabuf.c
> > >
> > > diff --git a/include/ui/console.h b/include/ui/console.h index
> > > 0bc7a00ac0..a208a68b88 100644
> > > --- a/include/ui/console.h
> > > +++ b/include/ui/console.h
> > > @@ -7,6 +7,7 @@
> > >  #include "qapi/qapi-types-ui.h"
> > >  #include "ui/input.h"
> > >  #include "ui/surface.h"
> > > +#include "ui/dmabuf.h"
> > >
> > >  #define TYPE_QEMU_CONSOLE "qemu-console"
> > >  OBJECT_DECLARE_TYPE(QemuConsole, QemuConsoleClass,
> > QEMU_CONSOLE) @@
> > > -185,25 +186,6 @@ struct QEMUGLParams {
> > >  int minor_ver;
> > >  };
> > >
> > > -typedef struct QemuDmaBuf {
> > > -int   fd;
> > > -uint32_t  width;
> > > -uint32_t  height;
> > > -uint32_t  stride;
> > > -uint32_t  fourcc;
> > > -uint64_t  modifier;
> > > -uint32_t  texture;
> > > -uint32_t  x;
> > > -uint32_t  y;
> > > -uint32_t  backing_width;
> > > -uint32_t  backing_height;
> > > -bool  y0_top;
> > > -void  *sync;
> > > -int   fence_fd;
> > > -bool  allow_fences;
> > > -bool  draw_submitted;
> > > -} QemuDmaBuf;
> > > -
> > >  enum display_scanout {
> > >  SCANOUT_NONE,
> > >  SCANOUT_SURFACE,
> > > diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h new file mode
> > > 100644 index 00..7a60116ee6
> > > --- /dev/null
> > > +++ b/include/ui/dmabuf.h
> > > @@ -0,0 +1,64 @@
> > > +/*
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + *
> > > + * QemuDmaBuf struct and helpers used for accessing its data
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > > later.
> > > + * See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#ifndef DMABUF_H
> > > +#define DMABUF_H
> > > +
> > > +typedef struct QemuDmaBuf {
> > > +int   fd;
> > > +uint32_t  width;
> > > +uint32_t  height;
> > > +uint32_t  stride;
> > > +uint32_t  fourcc;
> > > +uint64_t  modifier;
> > > +uint32_t  texture;
> > > +uint32_t  x;
> > > +uint32_t  y;
> > > +uint32_t  backing_width;
> > > +uint32_t  backing_height;
> > > +bool  y0_top;
> > > +void  *sync;
> > > +int   fence_fd;
> > > +bool  allow_fences;
> > > +bool  draw_submitted;
> > > +} QemuDmaBuf;
> > > +
> > > +QemuDmaBuf *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, int32_t
> > > +dmabuf_fd,
> > 
> > Should be 'int' not 'int32_t' for FDs.
> > 
> > > +   bool allow_fences, bool y0_top);
> > > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> > > +
> > > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf,
> > qemu_dmabuf_free);
> > > +
> > > +int32_t qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> > 
> > Again should be 'int' not 'int42_t'
> > 
> > I think there ought to also be a
> > 
> >   int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf);
> > 
> > to do the dup() call in one go too
> > 
> > > diff --git a/ui/dmabuf.c b/ui/dmabuf.c new file mode 100644 index
> > > 00..f447cce4fe
> > > --- /dev/null
> > > +++ b/ui/dmabuf.c
> > 
> > > +
> > > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> > > +{
> > > +if (dmabuf == NULL) {
> > > +return;
> > > +}
> > > +
> > 
> > I think this method should be made to call
> > 
> >   qemu_dmabuf_close()
> > 
> > to release the FD, if not 

RE: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-23 Thread Kim, Dongwon
Hi Daniel,

> -Original Message-
> From: Daniel P. Berrangé 
> Sent: Tuesday, April 23, 2024 7:07 AM
> To: Kim, Dongwon 
> Cc: qemu-devel@nongnu.org; marcandre.lur...@redhat.com;
> phi...@linaro.org
> Subject: Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for
> QemuDmaBuf struct and helpers
> 
> On Mon, Apr 22, 2024 at 07:22:49PM -0700, dongwon@intel.com wrote:
> > From: Dongwon Kim 
> >
> > New header and source files are added for containing QemuDmaBuf struct
> > definition and newly introduced helpers for creating/freeing the
> > struct and accessing its data.
> >
> > v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
> >  GPL to be in line with QEMU's default license
> >  (Daniel P. Berrangé )
> >
> > Suggested-by: Marc-André Lureau 
> > Cc: Philippe Mathieu-Daudé 
> > Cc: Daniel P. Berrangé 
> > Cc: Vivek Kasireddy 
> > Signed-off-by: Dongwon Kim 
> > ---
> >  include/ui/console.h |  20 +
> >  include/ui/dmabuf.h  |  64 +++
> >  ui/dmabuf.c  | 189
> +++
> >  ui/meson.build   |   1 +
> >  4 files changed, 255 insertions(+), 19 deletions(-)  create mode
> > 100644 include/ui/dmabuf.h  create mode 100644 ui/dmabuf.c
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h index
> > 0bc7a00ac0..a208a68b88 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -7,6 +7,7 @@
> >  #include "qapi/qapi-types-ui.h"
> >  #include "ui/input.h"
> >  #include "ui/surface.h"
> > +#include "ui/dmabuf.h"
> >
> >  #define TYPE_QEMU_CONSOLE "qemu-console"
> >  OBJECT_DECLARE_TYPE(QemuConsole, QemuConsoleClass,
> QEMU_CONSOLE) @@
> > -185,25 +186,6 @@ struct QEMUGLParams {
> >  int minor_ver;
> >  };
> >
> > -typedef struct QemuDmaBuf {
> > -int   fd;
> > -uint32_t  width;
> > -uint32_t  height;
> > -uint32_t  stride;
> > -uint32_t  fourcc;
> > -uint64_t  modifier;
> > -uint32_t  texture;
> > -uint32_t  x;
> > -uint32_t  y;
> > -uint32_t  backing_width;
> > -uint32_t  backing_height;
> > -bool  y0_top;
> > -void  *sync;
> > -int   fence_fd;
> > -bool  allow_fences;
> > -bool  draw_submitted;
> > -} QemuDmaBuf;
> > -
> >  enum display_scanout {
> >  SCANOUT_NONE,
> >  SCANOUT_SURFACE,
> > diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h new file mode
> > 100644 index 00..7a60116ee6
> > --- /dev/null
> > +++ b/include/ui/dmabuf.h
> > @@ -0,0 +1,64 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * QemuDmaBuf struct and helpers used for accessing its data
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef DMABUF_H
> > +#define DMABUF_H
> > +
> > +typedef struct QemuDmaBuf {
> > +int   fd;
> > +uint32_t  width;
> > +uint32_t  height;
> > +uint32_t  stride;
> > +uint32_t  fourcc;
> > +uint64_t  modifier;
> > +uint32_t  texture;
> > +uint32_t  x;
> > +uint32_t  y;
> > +uint32_t  backing_width;
> > +uint32_t  backing_height;
> > +bool  y0_top;
> > +void  *sync;
> > +int   fence_fd;
> > +bool  allow_fences;
> > +bool  draw_submitted;
> > +} QemuDmaBuf;
> > +
> > +QemuDmaBuf *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, int32_t
> > +dmabuf_fd,
> 
> Should be 'int' not 'int32_t' for FDs.
> 
> > +   bool allow_fences, bool y0_top);
> > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> > +
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf,
> qemu_dmabuf_free);
> > +
> > +int32_t qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> 
> Again should be 'int' not 'int42_t'
> 
> I think there ought to also be a
> 
>   int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf);
> 
> to do the dup() call in one go too
> 
> > diff --git a/ui/dmabuf.c b/ui/dmabuf.c new file mode 100644 index
> > 00..f447cce4fe
> > --- /dev/null
> > +++ b/ui/dmabuf.c
> 
> > +
> > +void qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> > +{
> > +if (dmabuf == NULL) {
> > +return;
> > +}
> > +
> 
> I think this method should be made to call
> 
>   qemu_dmabuf_close()
> 
> to release the FD, if not already released, otherwise
> this method could be a resource leak.
 
[Kim, Dongwon]  So you mean this close call should close all FDs including 
duped FDs, which implies we should include the list of duped FD and its 
management?

 > 
> > +g_free(dmabuf);
> > +}
> > +
> 
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-
> 

Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-23 Thread Daniel P . Berrangé
On Mon, Apr 22, 2024 at 07:22:49PM -0700, dongwon@intel.com wrote:
> From: Dongwon Kim 
> 
> New header and source files are added for containing QemuDmaBuf struct
> definition and newly introduced helpers for creating/freeing the struct
> and accessing its data.
> 
> v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
>  GPL to be in line with QEMU's default license
>  (Daniel P. Berrangé )
> 
> Suggested-by: Marc-André Lureau 
> Cc: Philippe Mathieu-Daudé 
> Cc: Daniel P. Berrangé 
> Cc: Vivek Kasireddy 
> Signed-off-by: Dongwon Kim 
> ---
>  include/ui/console.h |  20 +
>  include/ui/dmabuf.h  |  64 +++
>  ui/dmabuf.c  | 189 +++
>  ui/meson.build   |   1 +
>  4 files changed, 255 insertions(+), 19 deletions(-)
>  create mode 100644 include/ui/dmabuf.h
>  create mode 100644 ui/dmabuf.c
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 0bc7a00ac0..a208a68b88 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -7,6 +7,7 @@
>  #include "qapi/qapi-types-ui.h"
>  #include "ui/input.h"
>  #include "ui/surface.h"
> +#include "ui/dmabuf.h"
>  
>  #define TYPE_QEMU_CONSOLE "qemu-console"
>  OBJECT_DECLARE_TYPE(QemuConsole, QemuConsoleClass, QEMU_CONSOLE)
> @@ -185,25 +186,6 @@ struct QEMUGLParams {
>  int minor_ver;
>  };
>  
> -typedef struct QemuDmaBuf {
> -int   fd;
> -uint32_t  width;
> -uint32_t  height;
> -uint32_t  stride;
> -uint32_t  fourcc;
> -uint64_t  modifier;
> -uint32_t  texture;
> -uint32_t  x;
> -uint32_t  y;
> -uint32_t  backing_width;
> -uint32_t  backing_height;
> -bool  y0_top;
> -void  *sync;
> -int   fence_fd;
> -bool  allow_fences;
> -bool  draw_submitted;
> -} QemuDmaBuf;
> -
>  enum display_scanout {
>  SCANOUT_NONE,
>  SCANOUT_SURFACE,
> diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h
> new file mode 100644
> index 00..7a60116ee6
> --- /dev/null
> +++ b/include/ui/dmabuf.h
> @@ -0,0 +1,64 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * QemuDmaBuf struct and helpers used for accessing its data
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef DMABUF_H
> +#define DMABUF_H
> +
> +typedef struct QemuDmaBuf {
> +int   fd;
> +uint32_t  width;
> +uint32_t  height;
> +uint32_t  stride;
> +uint32_t  fourcc;
> +uint64_t  modifier;
> +uint32_t  texture;
> +uint32_t  x;
> +uint32_t  y;
> +uint32_t  backing_width;
> +uint32_t  backing_height;
> +bool  y0_top;
> +void  *sync;
> +int   fence_fd;
> +bool  allow_fences;
> +bool  draw_submitted;
> +} QemuDmaBuf;
> +
> +QemuDmaBuf *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, int32_t dmabuf_fd,

Should be 'int' not 'int32_t' for FDs.

> +   bool allow_fences, bool y0_top);
> +void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf, qemu_dmabuf_free);
> +
> +int32_t qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);

Again should be 'int' not 'int42_t'

I think there ought to also be a

  int qemu_dmabuf_dup_fd(QemuDmaBuf *dmabuf);

to do the dup() call in one go too

> diff --git a/ui/dmabuf.c b/ui/dmabuf.c
> new file mode 100644
> index 00..f447cce4fe
> --- /dev/null
> +++ b/ui/dmabuf.c

> +
> +void qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> +{
> +if (dmabuf == NULL) {
> +return;
> +}
> +

I think this method should be made to call

  qemu_dmabuf_close()

to release the FD, if not already released, otherwise
this method could be a resource leak.

> +g_free(dmabuf);
> +}
> +

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 :|




Re: [PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-23 Thread Daniel P . Berrangé
On Mon, Apr 22, 2024 at 07:22:49PM -0700, dongwon@intel.com wrote:
> From: Dongwon Kim 
> 
> New header and source files are added for containing QemuDmaBuf struct
> definition and newly introduced helpers for creating/freeing the struct
> and accessing its data.
> 
> v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
>  GPL to be in line with QEMU's default license
>  (Daniel P. Berrangé )

FYI, in future, notes about changes made in each version would
best go after the '---', since they're not something we need to
record in the commit message once merged. Don't re-send the series
now just for that reason though - whomever merges can trim this.

> 
> Suggested-by: Marc-André Lureau 
> Cc: Philippe Mathieu-Daudé 
> Cc: Daniel P. Berrangé 
> Cc: Vivek Kasireddy 
> Signed-off-by: Dongwon Kim 
> ---
>  include/ui/console.h |  20 +
>  include/ui/dmabuf.h  |  64 +++
>  ui/dmabuf.c  | 189 +++
>  ui/meson.build   |   1 +
>  4 files changed, 255 insertions(+), 19 deletions(-)
>  create mode 100644 include/ui/dmabuf.h
>  create mode 100644 ui/dmabuf.c

Reviewed-by: Daniel P. Berrangé 


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 :|




[PATCH v10 2/6] ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and helpers

2024-04-22 Thread dongwon . kim
From: Dongwon Kim 

New header and source files are added for containing QemuDmaBuf struct
definition and newly introduced helpers for creating/freeing the struct
and accessing its data.

v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
 GPL to be in line with QEMU's default license
 (Daniel P. Berrangé )

Suggested-by: Marc-André Lureau 
Cc: Philippe Mathieu-Daudé 
Cc: Daniel P. Berrangé 
Cc: Vivek Kasireddy 
Signed-off-by: Dongwon Kim 
---
 include/ui/console.h |  20 +
 include/ui/dmabuf.h  |  64 +++
 ui/dmabuf.c  | 189 +++
 ui/meson.build   |   1 +
 4 files changed, 255 insertions(+), 19 deletions(-)
 create mode 100644 include/ui/dmabuf.h
 create mode 100644 ui/dmabuf.c

diff --git a/include/ui/console.h b/include/ui/console.h
index 0bc7a00ac0..a208a68b88 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -7,6 +7,7 @@
 #include "qapi/qapi-types-ui.h"
 #include "ui/input.h"
 #include "ui/surface.h"
+#include "ui/dmabuf.h"
 
 #define TYPE_QEMU_CONSOLE "qemu-console"
 OBJECT_DECLARE_TYPE(QemuConsole, QemuConsoleClass, QEMU_CONSOLE)
@@ -185,25 +186,6 @@ struct QEMUGLParams {
 int minor_ver;
 };
 
-typedef struct QemuDmaBuf {
-int   fd;
-uint32_t  width;
-uint32_t  height;
-uint32_t  stride;
-uint32_t  fourcc;
-uint64_t  modifier;
-uint32_t  texture;
-uint32_t  x;
-uint32_t  y;
-uint32_t  backing_width;
-uint32_t  backing_height;
-bool  y0_top;
-void  *sync;
-int   fence_fd;
-bool  allow_fences;
-bool  draw_submitted;
-} QemuDmaBuf;
-
 enum display_scanout {
 SCANOUT_NONE,
 SCANOUT_SURFACE,
diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h
new file mode 100644
index 00..7a60116ee6
--- /dev/null
+++ b/include/ui/dmabuf.h
@@ -0,0 +1,64 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * QemuDmaBuf struct and helpers used for accessing its data
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef DMABUF_H
+#define DMABUF_H
+
+typedef struct QemuDmaBuf {
+int   fd;
+uint32_t  width;
+uint32_t  height;
+uint32_t  stride;
+uint32_t  fourcc;
+uint64_t  modifier;
+uint32_t  texture;
+uint32_t  x;
+uint32_t  y;
+uint32_t  backing_width;
+uint32_t  backing_height;
+bool  y0_top;
+void  *sync;
+int   fence_fd;
+bool  allow_fences;
+bool  draw_submitted;
+} QemuDmaBuf;
+
+QemuDmaBuf *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, int32_t dmabuf_fd,
+   bool allow_fences, bool y0_top);
+void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf, qemu_dmabuf_free);
+
+int32_t qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf);
+uint64_t qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_x(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_y(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
+uint32_t qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf);
+bool qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf);
+void *qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
+int32_t qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
+bool qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
+bool qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
+void qemu_dmabuf_set_texture(QemuDmaBuf *dmabuf, uint32_t texture);
+void qemu_dmabuf_set_fence_fd(QemuDmaBuf *dmabuf, int32_t fence_fd);
+void qemu_dmabuf_set_sync(QemuDmaBuf *dmabuf, void *sync);
+void qemu_dmabuf_set_draw_submitted(QemuDmaBuf *dmabuf, bool draw_submitted);
+void qemu_dmabuf_set_fd(QemuDmaBuf *dmabuf, int32_t fd);
+
+#endif
diff --git a/ui/dmabuf.c b/ui/dmabuf.c
new file mode 100644
index 00..f447cce4fe
--- /dev/null
+++ b/ui/dmabuf.c
@@ -0,0 +1,189 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * QemuDmaBuf struct and helpers used for accessing its data
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "ui/dmabuf.h"
+
+QemuDmaBuf *qemu_dmabuf_new(uint32_t width, uint32_t height,
+uint32_t stride, uint32_t x,
+uint32_t y, uint32_t backing_width,
+