| Message ID | 20240723220258.3170957-1-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Consolidate create-sync and create-fence | expand |
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 > > >
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
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(-)