On Sat, Oct 29, 2016 at 01:15:44PM -0400, Rob Clark wrote: > On Fri, Oct 28, 2016 at 7:44 PM, Rafael Antognolli > <[email protected]> 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 <[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. > > 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 _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
