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

Sync object itself is never used as is so can be removed
from QemuDmaBuf struct. So now sync is only temporarily needed
when creating fence for the object which means what was done in
egl_dmabuf_create_sync can now be a part of egl_dmabuf_create_fence
function. And egl_dmabuf_create_fence returns fence_fd so the
better function name will be egl_dmabuf_create_fence_fd.

v3: create fence only if current QemuDmaBuf->fence_fd = -1
    to make sure there is no fence currently bound to the
    QemuDmaBuf

Dongwon Kim (2):
  ui/egl-helpers: Consolidates create-sync and create-fence
  ui/dmabuf: Remove 'sync' from QemuDmaBuf struct

 include/ui/dmabuf.h      |  2 --
 include/ui/egl-helpers.h |  3 +--
 ui/dmabuf.c              | 14 --------------
 ui/egl-helpers.c         | 24 +++++++++---------------
 ui/gtk-egl.c             | 17 ++++-------------
 ui/gtk-gl-area.c         | 12 +++---------
 6 files changed, 17 insertions(+), 55 deletions(-)

-- 
2.43.0

Hi

On Wed, Jul 24, 2024 at 2:05 AM <dongwon.kim@intel.com> wrote:

> From: Dongwon Kim <dongwon.kim@intel.com>
>
> Sync object itself is never used as is so can be removed
> from QemuDmaBuf struct. So now sync is only temporarily needed
> when creating fence for the object which means what was done in
> egl_dmabuf_create_sync can now be a part of egl_dmabuf_create_fence
> function. And egl_dmabuf_create_fence returns fence_fd so the
> better function name will be egl_dmabuf_create_fence_fd.
>
> v3: create fence only if current QemuDmaBuf->fence_fd = -1
>     to make sure there is no fence currently bound to the
>     QemuDmaBuf
>

Why not check it from egl_dmabuf_create_fence_fd() ? calling the function
twice can still potentially leak.

Also, please gather the v1/v2/v3/... summary on the cover letter.

thanks


> Dongwon Kim (2):
>   ui/egl-helpers: Consolidates create-sync and create-fence
>   ui/dmabuf: Remove 'sync' from QemuDmaBuf struct
>
>  include/ui/dmabuf.h      |  2 --
>  include/ui/egl-helpers.h |  3 +--
>  ui/dmabuf.c              | 14 --------------
>  ui/egl-helpers.c         | 24 +++++++++---------------
>  ui/gtk-egl.c             | 17 ++++-------------
>  ui/gtk-gl-area.c         | 12 +++---------
>  6 files changed, 17 insertions(+), 55 deletions(-)
>
> --
> 2.43.0
>
>
>

-- 
Marc-André Lureau

Hey Marc-André,

On 7/24/2024 3:37 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 24, 2024 at 2:05 AM <dongwon.kim@intel.com 
> <mailto:dongwon.kim@intel.com>> wrote:
> 
>     From: Dongwon Kim <dongwon.kim@intel.com <mailto:dongwon.kim@intel.com>>
> 
>     Sync object itself is never used as is so can be removed
>     from QemuDmaBuf struct. So now sync is only temporarily needed
>     when creating fence for the object which means what was done in
>     egl_dmabuf_create_sync can now be a part of egl_dmabuf_create_fence
>     function. And egl_dmabuf_create_fence returns fence_fd so the
>     better function name will be egl_dmabuf_create_fence_fd.
> 
>     v3: create fence only if current QemuDmaBuf->fence_fd = -1
>          to make sure there is no fence currently bound to the
>          QemuDmaBuf
> 
> 
> Why not check it from egl_dmabuf_create_fence_fd() ? calling the 
> function twice can still potentially leak.

It is called from only two locations in gtk-egl.c and gtk-gl-draw.c
and dmabuf->fence_fd == -1 is checked beforehand so I thought it's
not needed but I think your point is the completeness of the function
itself. Do you think we can do assert 'dmabuf->fence_fd >= 0'?

> 
> Also, please gather the v1/v2/v3/... summary on the cover letter.

Sure

> 
> thanks
> 
> 
>     Dongwon Kim (2):
>        ui/egl-helpers: Consolidates create-sync and create-fence
>        ui/dmabuf: Remove 'sync' from QemuDmaBuf struct
> 
>       include/ui/dmabuf.h      |  2 --
>       include/ui/egl-helpers.h |  3 +--
>       ui/dmabuf.c              | 14 --------------
>       ui/egl-helpers.c         | 24 +++++++++---------------
>       ui/gtk-egl.c             | 17 ++++-------------
>       ui/gtk-gl-area.c         | 12 +++---------
>       6 files changed, 17 insertions(+), 55 deletions(-)
> 
>     -- 
>     2.43.0
> 
> 
> 
> 
> -- 
> Marc-André Lureau