[wayland-protocols,v7] Add zwp_linux_explicit_synchronization_v1

Submitted by Tomek Bury on Dec. 17, 2018, 12:44 p.m.

Details

Message ID CAO1ALzm=s6Gn8vvYvrK7T=hOZk7AiSnZFsu_RvRFpGwgvrbzWQ@mail.gmail.com
State Superseded
Series "Add zwp_linux_explicit_synchronization_v1"
Headers show

Commit Message

Tomek Bury Dec. 17, 2018, 12:44 p.m.
Hi Pekka, Alexandros,

Here's a patch containing all I had to do in order to test the explicit
sync support Alexandros has implemented in Weston.

Thanks,
Tomek


On Wed, 28 Nov 2018 at 14:35, Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Pekka,
>
> > I suppose the compositor-side copy of buffers you mentioned is for the
> > lack of release fences, not acquire fences?
> Yes, the lack of release fences is the very reason for the copy. I could
> cook something up for the acquire fence, but that wouldn't synchronise the
> buffer access anyway. The only way I can be sure the client doesn't
> overwrite a buffer still in use by the GPU was to texture from a copy and
> let the compositor release the original without waiting for the GPU and
> without a fence.
>
> Cheers,
> Tomek
>
>

Patch hide | download patch | download mbox

From ee74382c9a9b04c0fd7ca30bee9a3b98a314e942 Mon Sep 17 00:00:00 2001
From: Tomek Bury <tomek.bury@broadcom.com>
Date: Mon, 17 Dec 2018 12:33:07 +0000
Subject: [PATCH] clients: Support explicit synchronization in opaque EGL
 buffers

Signed-off-by: Tomek Bury <tomek.bury@broadcom.com>
---
 libweston/compositor.c  | 9 ---------
 libweston/gl-renderer.c | 3 ---
 2 files changed, 12 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index d939b833..d6860362 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -3418,15 +3418,6 @@  surface_commit(struct wl_client *client, struct wl_resource *resource)
 				wl_resource_get_id(resource));
 			return;
 		}
-
-		if (!linux_dmabuf_buffer_get(surface->pending.buffer->resource)) {
-			fd_clear(&surface->pending.acquire_fence_fd);
-			wl_resource_post_error(surface->synchronization_resource,
-				ZWP_LINUX_SURFACE_SYNCHRONIZATION_V1_ERROR_UNSUPPORTED_BUFFER,
-				"wl_surface@%"PRIu32" unsupported buffer for synchronization",
-				wl_resource_get_id(resource));
-			return;
-		}
 	}
 
 	if (surface->pending.buffer_release_ref.buffer_release &&
diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index 18858949..ac490322 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -895,9 +895,6 @@  ensure_surface_buffer_is_ready(struct gl_renderer *gr,
 	/* We should only get a fence if we support EGLSyncKHR, since
 	 * we don't advertise the explicit sync protocol otherwise. */
 	assert(gr->has_native_fence_sync);
-	/* We should only get a fence for zwp_linux_dmabuf buffers, since
-	 * surface commit would have failed otherwise. */
-	assert(linux_dmabuf_buffer_get(buffer->resource) != NULL);
 
 	attribs[1] = dup(surface->acquire_fence_fd);
 	if (attribs[1] == -1) {
-- 
2.17.1


Comments

Alexandros Frantzis Dec. 17, 2018, 3:37 p.m.
On Monday, December 17, 2018 12:44 GMT, Tomek Bury <tomek.bury@gmail.com> wrote: 
> On Wed, 28 Nov 2018 at 14:35, Tomek Bury <tomek.bury@gmail.com> wrote:
> > Hi Pekka,
> >
> > > I suppose the compositor-side copy of buffers you mentioned is for the
> > > lack of release fences, not acquire fences?
> > Yes, the lack of release fences is the very reason for the copy. I could
> > cook something up for the acquire fence, but that wouldn't synchronise the
> > buffer access anyway. The only way I can be sure the client doesn't
> > overwrite a buffer still in use by the GPU was to texture from a copy and
> > let the compositor release the original without waiting for the GPU and
> > without a fence.
> >
> > Cheers,
> > Tomek
> >
> Hi Pekka, Alexandros,
> 
> Here's a patch containing all I had to do in order to test the explicit
> sync support Alexandros has implemented in Weston.
> 
> Thanks,
> Tomek

Hi Tomek,

I have now updated the weston explicit-sync gitlab MR [1] to support,
among other things, minor version 2 of the protocol. Instead of
completely removing the checks, I have updated them to check for and
accept all non-SHM buffers, which is adequate for current needs. There
are other ways to deal with these checks if we think we need a more
versatile approach (e.g., asking the renderer to tell us if it support fences
for a particular buffer). Please see the MR comments for more info and
discussion.

Thanks,
Alexandros
 
[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Tomek Bury Dec. 17, 2018, 5:25 p.m.
Thanks! That looks better than my patch.

On Mon, 17 Dec 2018 at 15:37, Alexandros Frantzis <
alexandros.frantzis@collabora.com> wrote:

> On Monday, December 17, 2018 12:44 GMT, Tomek Bury <tomek.bury@gmail.com>
> wrote:
> > On Wed, 28 Nov 2018 at 14:35, Tomek Bury <tomek.bury@gmail.com> wrote:
> > > Hi Pekka,
> > >
> > > > I suppose the compositor-side copy of buffers you mentioned is for
> the
> > > > lack of release fences, not acquire fences?
> > > Yes, the lack of release fences is the very reason for the copy. I
> could
> > > cook something up for the acquire fence, but that wouldn't synchronise
> the
> > > buffer access anyway. The only way I can be sure the client doesn't
> > > overwrite a buffer still in use by the GPU was to texture from a copy
> and
> > > let the compositor release the original without waiting for the GPU and
> > > without a fence.
> > >
> > > Cheers,
> > > Tomek
> > >
> > Hi Pekka, Alexandros,
> >
> > Here's a patch containing all I had to do in order to test the explicit
> > sync support Alexandros has implemented in Weston.
> >
> > Thanks,
> > Tomek
>
> Hi Tomek,
>
> I have now updated the weston explicit-sync gitlab MR [1] to support,
> among other things, minor version 2 of the protocol. Instead of
> completely removing the checks, I have updated them to check for and
> accept all non-SHM buffers, which is adequate for current needs. There
> are other ways to deal with these checks if we think we need a more
> versatile approach (e.g., asking the renderer to tell us if it support
> fences
> for a particular buffer). Please see the MR comments for more info and
> discussion.
>
> Thanks,
> Alexandros
>
> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
>
>