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 <[email protected]> > > [chadv]: Resolve rebase conflicts. > > Signed-off-by: Rob Clark <[email protected]> > --- > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev I hope that helps. Regards, Rafael _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
