[Mesa-dev] [RFC 03/12] egl: add EGL_ANDROID_native_fence_sync

Rob Clark robdclark at gmail.com
Sat Oct 29 17:15:44 UTC 2016


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?

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

BR,
-R

> Regards,
> Rafael


More information about the mesa-dev mailing list