[Mesa-dev] [RFC 03/12] egl: add EGL_ANDROID_native_fence_sync
Rafael Antognolli
rafael.antognolli at intel.com
Mon Oct 31 15:58:26 UTC 2016
On Sat, Oct 29, 2016 at 01:15:44PM -0400, Rob Clark wrote:
> On Fri, Oct 28, 2016 at 7:44 PM, Rafael Antognolli
> <rafael.antognolli at intel.com> wrote:
> > Hi Chad and Rob,
> >
> > I took the liberty to run the piglit tests that I submitted against this
> > series, and it pointed out to a couple errors. Please disregard this if
> > you already have a newer version of these patches laying around with
> > those things fixed.
> >
> > On Mon, Oct 10, 2016 at 10:43:50AM -0700, Chad Versace wrote:
> >> From: Rob Clark <robclark at freedesktop.org>
> >>
> >> [chadv]: Resolve rebase conflicts.
> >>
> >> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> >> ---
> >> src/egl/drivers/dri2/egl_dri2.c | 49 +++++++++++++++++++++++++++++++++++++++++
> >> src/egl/main/eglapi.c | 36 +++++++++++++++++++++++++++---
> >> src/egl/main/eglapi.h | 2 ++
> >> src/egl/main/egldisplay.h | 1 +
> >> src/egl/main/eglfallbacks.c | 1 +
> >> src/egl/main/eglsync.c | 37 +++++++++++++++++++------------
> >> src/egl/main/eglsync.h | 1 +
> >> 7 files changed, 110 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> >> index cd0a2e9..0c15c85 100644
> >> --- a/src/egl/drivers/dri2/egl_dri2.c
> >> +++ b/src/egl/drivers/dri2/egl_dri2.c
> >> @@ -633,6 +633,12 @@ dri2_setup_screen(_EGLDisplay *disp)
> >> disp->Extensions.KHR_wait_sync = EGL_TRUE;
> >> if (dri2_dpy->fence->get_fence_from_cl_event)
> >> disp->Extensions.KHR_cl_event2 = EGL_TRUE;
> >> + if (dri2_dpy->fence->base.version >= 2) {
> >> + unsigned capabilities =
> >> + dri2_dpy->fence->get_capabilities(dri2_dpy->dri_screen);
> >> + disp->Extensions.ANDROID_native_fence_sync =
> >> + (capabilities & __DRI_FENCE_CAP_NATIVE_FD) != 0;
> >> + }
> >> }
> >>
> >> disp->Extensions.KHR_reusable_sync = EGL_TRUE;
> >> @@ -2589,6 +2595,19 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
> >> /* initial status of reusable sync must be "unsignaled" */
> >> dri2_sync->base.SyncStatus = EGL_UNSIGNALED_KHR;
> >> break;
> >> +
> >> + case EGL_SYNC_NATIVE_FENCE_ANDROID:
> >> + if (dri2_dpy->fence->create_fence_fd) {
> >> + dri2_sync->fence = dri2_dpy->fence->create_fence_fd(
> >> + dri2_ctx->dri_context,
> >> + dri2_sync->base.SyncFd);
> >> + }
> >> + if (!dri2_sync->fence) {
> >> + _eglError(EGL_BAD_ATTRIBUTE, "eglCreateSyncKHR");
> >> + free(dri2_sync);
> >> + return NULL;
> >> + }
> >> + break;
> >> }
> >>
> >> p_atomic_set(&dri2_sync->refcount, 1);
> >> @@ -2618,12 +2637,41 @@ dri2_destroy_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync)
> >> ret = EGL_FALSE;
> >> }
> >> }
> >> +
> >> + if (sync->SyncFd != EGL_NO_NATIVE_FENCE_FD_ANDROID)
> >> + close(sync->SyncFd);
> >> +
> >> dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> >>
> >> return ret;
> >> }
> >>
> >> static EGLint
> >> +dri2_dup_native_fence_fd(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync)
> >> +{
> >> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> >> + struct dri2_egl_sync *dri2_sync = dri2_egl_sync(sync);
> >> +
> >> + assert(sync->Type == EGL_SYNC_NATIVE_FENCE_ANDROID);
> >> +
> >> + if (sync->SyncFd == EGL_NO_NATIVE_FENCE_FD_ANDROID) {
> >> + /* try to retrieve the actual native fence fd.. if rendering is
> >> + * not flushed this will just return -1, aka NO_NATIVE_FENCE_FD:
> >> + */
> >> + sync->SyncFd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
> >> + dri2_sync->fence);
> >> + }
> >> +
> >> + if (sync->SyncFd == EGL_NO_NATIVE_FENCE_FD_ANDROID) {
> >> + /* if native fence fd still not created, return an error: */
> >> + _eglError(EGL_BAD_PARAMETER, "eglDupNativeFenceFDANDROID");
> >> + return EGL_NO_NATIVE_FENCE_FD_ANDROID;
> >> + }
> >> +
> >> + return dup(sync->SyncFd);
> >> +}
> >> +
> >> +static EGLint
> >> dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
> >> EGLint flags, EGLTime timeout)
> >
> > I believe dri2_client_wait_sync is missing a:
> > case EGL_SYNC_NATIVE_FENCE_ANDROID:
> >
> > which does the same as the EGL_SYNC_FENCE_KHR case.
> >
> > See
> > https://lists.freedesktop.org/archives/piglit/2016-October/021263.html
> >
> >> {
> >> @@ -2900,6 +2948,7 @@ _eglBuiltInDriverDRI2(const char *args)
> >> dri2_drv->base.API.SignalSyncKHR = dri2_signal_sync;
> >> dri2_drv->base.API.WaitSyncKHR = dri2_server_wait_sync;
> >> dri2_drv->base.API.DestroySyncKHR = dri2_destroy_sync;
> >> + dri2_drv->base.API.DupNativeFenceFDANDROID = dri2_dup_native_fence_fd;
> >> dri2_drv->base.API.GLInteropQueryDeviceInfo = dri2_interop_query_device_info;
> >> dri2_drv->base.API.GLInteropExportObject = dri2_interop_export_object;
> >>
> >> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> >> index 18071d7..3c0b017 100644
> >> --- a/src/egl/main/eglapi.c
> >> +++ b/src/egl/main/eglapi.c
> >> @@ -469,6 +469,7 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
> >> /* Please keep these sorted alphabetically. */
> >> _EGL_CHECK_EXTENSION(ANDROID_framebuffer_target);
> >> _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
> >> + _EGL_CHECK_EXTENSION(ANDROID_native_fence_sync);
> >> _EGL_CHECK_EXTENSION(ANDROID_recordable);
> >>
> >> _EGL_CHECK_EXTENSION(CHROMIUM_sync_control);
> >> @@ -1562,6 +1563,10 @@ _eglCreateSync(_EGLDisplay *disp, EGLenum type, const EGLAttrib *attrib_list,
> >> if (!disp->Extensions.KHR_cl_event2)
> >> RETURN_EGL_ERROR(disp, invalid_type_error, EGL_NO_SYNC_KHR);
> >> break;
> >> + case EGL_SYNC_NATIVE_FENCE_ANDROID:
> >> + if (!disp->Extensions.ANDROID_native_fence_sync)
> >> + RETURN_EGL_ERROR(disp, invalid_type_error, EGL_NO_SYNC_KHR);
> >> + break;
> >> default:
> >> RETURN_EGL_ERROR(disp, invalid_type_error, EGL_NO_SYNC_KHR);
> >> }
> >> @@ -1634,7 +1639,8 @@ eglDestroySync(EGLDisplay dpy, EGLSync sync)
> >>
> >> _EGL_CHECK_SYNC(disp, s, EGL_FALSE, drv);
> >> assert(disp->Extensions.KHR_reusable_sync ||
> >> - disp->Extensions.KHR_fence_sync);
> >> + disp->Extensions.KHR_fence_sync ||
> >> + disp->Extensions.ANDROID_native_fence_sync);
> >>
> >> _eglUnlinkSync(s);
> >> ret = drv->API.DestroySyncKHR(drv, disp, s);
> >> @@ -1655,7 +1661,8 @@ eglClientWaitSync(EGLDisplay dpy, EGLSync sync, EGLint flags, EGLTime timeout)
> >>
> >> _EGL_CHECK_SYNC(disp, s, EGL_FALSE, drv);
> >> assert(disp->Extensions.KHR_reusable_sync ||
> >> - disp->Extensions.KHR_fence_sync);
> >> + disp->Extensions.KHR_fence_sync ||
> >> + disp->Extensions.ANDROID_native_fence_sync);
> >>
> >> if (s->SyncStatus == EGL_SIGNALED_KHR)
> >> RETURN_EGL_EVAL(disp, EGL_CONDITION_SATISFIED_KHR);
> >> @@ -1754,7 +1761,8 @@ _eglGetSyncAttribCommon(_EGLDisplay *disp, _EGLSync *s, EGLint attribute, EGLAtt
> >>
> >> _EGL_CHECK_SYNC(disp, s, EGL_FALSE, drv);
> >> assert(disp->Extensions.KHR_reusable_sync ||
> >> - disp->Extensions.KHR_fence_sync);
> >> + disp->Extensions.KHR_fence_sync ||
> >> + disp->Extensions.ANDROID_native_fence_sync);
> >> ret = drv->API.GetSyncAttrib(drv, disp, s, attribute, value);
> >>
> >> RETURN_EGL_EVAL(disp, ret);
> >> @@ -1797,6 +1805,27 @@ eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *valu
> >> return result;
> >> }
> >>
> >> +static EGLint EGLAPIENTRY
> >> +eglDupNativeFenceFDANDROID(EGLDisplay dpy, EGLSync sync)
> >> +{
> >> + _EGLDisplay *disp = _eglLockDisplay(dpy);
> >> + _EGLSync *s = _eglLookupSync(sync, disp);
> >> + _EGLDriver *drv;
> >> + EGLBoolean ret;
> >> +
> >> + /* the spec doesn't seem to specify what happens if the fence
> >> + * type is not EGL_SYNC_NATIVE_FENCE_ANDROID, but this seems
> >> + * sensible:
> >> + */
> >> + if (s->Type != EGL_SYNC_NATIVE_FENCE_ANDROID)
> >
> > If sync is invalid, "s" will be null and will segfault here.
> >
> > See
> > https://lists.freedesktop.org/archives/piglit/2016-October/021257.html
> >
> >> + RETURN_EGL_ERROR(disp, EGL_BAD_PARAMETER, EGL_NO_NATIVE_FENCE_FD_ANDROID);
> >> +
> >> + _EGL_CHECK_SYNC(disp, s, EGL_NO_NATIVE_FENCE_FD_ANDROID, drv);
> >> + assert(disp->Extensions.ANDROID_native_fence_sync);
> >> + ret = drv->API.DupNativeFenceFDANDROID(drv, disp, s);
> >> +
> >> + RETURN_EGL_EVAL(disp, ret);
> >> +}
> >>
> >> static EGLBoolean EGLAPIENTRY
> >> eglSwapBuffersRegionNOK(EGLDisplay dpy, EGLSurface surface,
> >> @@ -2271,6 +2300,7 @@ eglGetProcAddress(const char *procname)
> >> { "eglLabelObjectKHR", (_EGLProc) eglLabelObjectKHR },
> >> { "eglDebugMessageControlKHR", (_EGLProc) eglDebugMessageControlKHR },
> >> { "eglQueryDebugKHR", (_EGLProc) eglQueryDebugKHR },
> >> + { "eglDupNativeFenceFDANDROID", (_EGLProc) eglDupNativeFenceFDANDROID },
> >> { NULL, NULL }
> >> };
> >> EGLint i;
> >> diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
> >> index b9bcc8e..2dc89fc 100644
> >> --- a/src/egl/main/eglapi.h
> >> +++ b/src/egl/main/eglapi.h
> >> @@ -146,6 +146,8 @@ struct _egl_api
> >> EGLBoolean (*GetSyncAttrib)(_EGLDriver *drv, _EGLDisplay *dpy,
> >> _EGLSync *sync, EGLint attribute,
> >> EGLAttrib *value);
> >> + EGLint (*DupNativeFenceFDANDROID)(_EGLDriver *drv, _EGLDisplay *dpy,
> >> + _EGLSync *sync);
> >>
> >> EGLBoolean (*SwapBuffersRegionNOK)(_EGLDriver *drv, _EGLDisplay *disp,
> >> _EGLSurface *surf, EGLint numRects,
> >> diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
> >> index 22fb5c8..dcec764 100644
> >> --- a/src/egl/main/egldisplay.h
> >> +++ b/src/egl/main/egldisplay.h
> >> @@ -94,6 +94,7 @@ struct _egl_extensions
> >> /* Please keep these sorted alphabetically. */
> >> EGLBoolean ANDROID_framebuffer_target;
> >> EGLBoolean ANDROID_image_native_buffer;
> >> + EGLBoolean ANDROID_native_fence_sync;
> >> EGLBoolean ANDROID_recordable;
> >>
> >> EGLBoolean CHROMIUM_sync_control;
> >> diff --git a/src/egl/main/eglfallbacks.c b/src/egl/main/eglfallbacks.c
> >> index d0fce8c..017d337 100644
> >> --- a/src/egl/main/eglfallbacks.c
> >> +++ b/src/egl/main/eglfallbacks.c
> >> @@ -92,6 +92,7 @@ _eglInitDriverFallbacks(_EGLDriver *drv)
> >> drv->API.WaitSyncKHR = NULL;
> >> drv->API.SignalSyncKHR = NULL;
> >> drv->API.GetSyncAttrib = _eglGetSyncAttrib;
> >> + drv->API.DupNativeFenceFDANDROID = NULL;
> >>
> >> drv->API.CreateDRMImageMESA = NULL;
> >> drv->API.ExportDRMImageMESA = NULL;
> >> diff --git a/src/egl/main/eglsync.c b/src/egl/main/eglsync.c
> >> index eb9950d..ccfa474 100644
> >> --- a/src/egl/main/eglsync.c
> >> +++ b/src/egl/main/eglsync.c
> >> @@ -49,24 +49,23 @@ _eglParseSyncAttribList(_EGLSync *sync, const EGLAttrib *attrib_list)
> >> for (i = 0; attrib_list[i] != EGL_NONE; i++) {
> >> EGLAttrib attr = attrib_list[i++];
> >> EGLAttrib val = attrib_list[i];
> >> - EGLint err = EGL_SUCCESS;
> >>
> >> switch (attr) {
> >> case EGL_CL_EVENT_HANDLE_KHR:
> >> - if (sync->Type == EGL_SYNC_CL_EVENT_KHR) {
> >> - sync->CLEvent = val;
> >> - break;
> >> - }
> >> - /* fall through */
> >> - default:
> >> - (void) val;
> >> - err = EGL_BAD_ATTRIBUTE;
> >> + if (sync->Type != EGL_SYNC_CL_EVENT_KHR)
> >> + goto bad_attrib;
> >> + sync->CLEvent = val;
> >> break;
> >> - }
> >> -
> >> - if (err != EGL_SUCCESS) {
> >> + case EGL_SYNC_NATIVE_FENCE_FD_ANDROID:
> >> + if (sync->Type != EGL_SYNC_NATIVE_FENCE_ANDROID)
> >> + goto bad_attrib;
> >> + /* we take ownership of the native fd, so no dup(): */
> >> + sync->SyncFd = val;
> >> + break;
> >> + bad_attrib:
> >> + default:
> >> _eglLog(_EGL_DEBUG, "bad sync attribute 0x%" PRIxPTR, attr);
> >> - return err;
> >> + return EGL_BAD_ATTRIBUTE;
> >> }
> >> }
> >>
> >> @@ -83,6 +82,7 @@ _eglInitSync(_EGLSync *sync, _EGLDisplay *dpy, EGLenum type,
> >> _eglInitResource(&sync->Resource, sizeof(*sync), dpy);
> >> sync->Type = type;
> >> sync->SyncStatus = EGL_UNSIGNALED_KHR;
> >> + sync->SyncFd = EGL_NO_NATIVE_FENCE_FD_ANDROID;
> >>
> >> err = _eglParseSyncAttribList(sync, attrib_list);
> >> if (err != EGL_SUCCESS)
> >> @@ -92,6 +92,13 @@ _eglInitSync(_EGLSync *sync, _EGLDisplay *dpy, EGLenum type,
> >> case EGL_SYNC_CL_EVENT_KHR:
> >> sync->SyncCondition = EGL_SYNC_CL_EVENT_COMPLETE_KHR;
> >> break;
> >> + case EGL_SYNC_NATIVE_FENCE_ANDROID:
> >> + sync->SyncCondition = EGL_SYNC_NATIVE_FENCE_SIGNALED_ANDROID;
> >> + if (sync->SyncFd == EGL_NO_NATIVE_FENCE_FD_ANDROID)
> >> + sync->SyncCondition = EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR;
> >> + else
> >> + sync->SyncCondition = EGL_SYNC_NATIVE_FENCE_SIGNALED_ANDROID;
> >> + break;
> >> default:
> >> sync->SyncCondition = EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR;
> >> }
> >> @@ -123,10 +130,12 @@ _eglGetSyncAttrib(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
> >> break;
> >
> > I think this function is missing a (sync->Type ==
> > EGL_SYNC_NATIVE_FENCE_ANDROID) inside the EGL_SYNC_STATUS_KHR case.
> > Without that, it will only return whatever is inside sync->SyncStatus,
> > which is not always up to date to the status of the sync fd.
> >
> > See
> > https://lists.freedesktop.org/archives/piglit/2016-October/021252.html
> >
> >> case EGL_SYNC_CONDITION_KHR:
> >> if (sync->Type != EGL_SYNC_FENCE_KHR &&
> >> - sync->Type != EGL_SYNC_CL_EVENT_KHR)
> >> + sync->Type != EGL_SYNC_CL_EVENT_KHR &&
> >> + sync->Type != EGL_SYNC_NATIVE_FENCE_ANDROID)
> >> return _eglError(EGL_BAD_ATTRIBUTE, "eglGetSyncAttribKHR");
> >> *value = sync->SyncCondition;
> >> break;
> >> +
> >> default:
> >> return _eglError(EGL_BAD_ATTRIBUTE, "eglGetSyncAttribKHR");
> >> break;
> >> diff --git a/src/egl/main/eglsync.h b/src/egl/main/eglsync.h
> >> index 83b6f72..5ac76f3 100644
> >> --- a/src/egl/main/eglsync.h
> >> +++ b/src/egl/main/eglsync.h
> >> @@ -48,6 +48,7 @@ struct _egl_sync
> >> EGLenum SyncStatus;
> >> EGLenum SyncCondition;
> >> EGLAttrib CLEvent;
> >> + EGLint SyncFd;
> >> };
> >>
> >>
> >> --
> >> 2.10.0
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > I hope that helps.
>
> Hey, thanks for this. I don't suppose you have a branch somewhere w/
> the piglit tests?
Ouch, I mentioned it on another email but should have mentioned it here
too. It's here:
https://github.com/rantogno/piglit/tree/fences
> I've rebased and pulled in Chad's squash patches (and also a squash
> patch based on the issues you pointed out), but not yet the i965
> patches:
>
> https://github.com/freedreno/mesa/commits/wip-fence
Awesome, I will check that one.
Thanks,
Rafael
> BR,
> -R
>
> > Regards,
> > Rafael
More information about the mesa-dev
mailing list