On Mon 12 Jun 2017, Chris Wilson wrote: > From KHR_fence_sync: > > When the condition of the sync object is satisfied by the fence > command, the sync is signaled by the associated client API context, > causing any eglClientWaitSyncKHR commands (see below) blocking on > <sync> to unblock. The only condition currently supported is > EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR, which is satisfied by > completion of the fence command corresponding to the sync object, > and all preceding commands in the associated client API context's > command stream. The sync object will not be signaled until all > effects from these commands on the client API's internal and > framebuffer state are fully realized. No other state is affected by > execution of the fence command. > > If clients are passing the fence fd (from EGL_ANDROID_native_fence_sync) > to a compositor, that fence must only be signaled once the framebuffer > is resolved and not before as is currently the case.
The intent is correct, but some details are wrong. Comments below. > Reported-by: Sergi Granell <[email protected]> > Fixes: c636284ee8ee ("i965/sync: Implement DRI2_Fence extension") > Signed-off-by: Chris Wilson <[email protected]> > Cc: Sergi Granell <[email protected]> > Cc: Rob Clark <[email protected]> > Cc: Chad Versace <[email protected]> > Cc: Daniel Stone <[email protected]> > Cc: Kenneth Graunke <[email protected]> > --- > src/mesa/drivers/dri/i965/brw_sync.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_sync.c > b/src/mesa/drivers/dri/i965/brw_sync.c > index 427108610d..ec4836ce07 100644 > --- a/src/mesa/drivers/dri/i965/brw_sync.c > +++ b/src/mesa/drivers/dri/i965/brw_sync.c > @@ -110,6 +110,34 @@ brw_fence_finish(struct brw_fence *fence) > static bool MUST_CHECK > brw_fence_insert_locked(struct brw_context *brw, struct brw_fence *fence) > { > + __DRIcontext *driContext = brw->driContext; > + __DRIdrawable *driDrawable = driContext->driDrawablePriv; > + > + /* > + * From KHR_fence_sync: > + * > + * When the condition of the sync object is satisfied by the fence > + * command, the sync is signaled by the associated client API context, > + * causing any eglClientWaitSyncKHR commands (see below) blocking on > + * <sync> to unblock. The only condition currently supported is > + * EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR, which is satisfied by > + * completion of the fence command corresponding to the sync object, > + * and all preceding commands in the associated client API context's > + * command stream. The sync object will not be signaled until all > + * effects from these commands on the client API's internal and > + * framebuffer state are fully realized. No other state is affected by > + * execution of the fence command. > + * > + * Note the emphasis there on ensuring that the framebuffer is fully > + * realised before the fence is signal. We cannot just flush the batch, > + * but must also resolve the drawable first. The importance of this is, > + * for example, in creating a fence for a frame to be passed to a > + * remote compositor. Otherwise, the resolve will be in a following batch > + * (when the client finally calls SwapBuffers, or forces a resolve via > + * some other path) and the compositor may read the fraembuffer > beforehand. > + */ > + if (driDrawable) > + intel_resolve_for_dri2_flush(brw, driDrawable); This hunk looks good. Nice find. > brw_emit_mi_flush(brw); > > switch (fence->type) { > @@ -335,6 +363,8 @@ brw_gl_fence_sync(struct gl_context *ctx, struct > gl_sync_object *_sync, > struct brw_context *brw = brw_context(ctx); > struct brw_gl_sync *sync = (struct brw_gl_sync *) _sync; > > + assert(condition == EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR); > + This function is the entrypoint for glFenceSync; brw_dri_client_wait_sync() is the entry point for eglClientWaitSync. So the assertion should be on GL_SYNC_GPU_COMMANDS_COMPLETE. For the record, GL_SYNC_GPU_COMMANDS_COMPLETE != EGL_SYNC_PRIOR_COMMANDS_COMPLETE. In fact, this assertion should probably go into a separate patch, as this codepath is unrelated to EGL_KHR_fence_sync. Fix the assertion; and optionally move the assertion to a separate patch; and then this will be Reviewed-by: Chad Versace <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
