[Mesa-dev] [PATCH v2 2/7] egl: add support for EGL_ANDROID_blob_cache

[Tapani Pälli]

On 31.01.2018 15:07, Tapani Pälli wrote:
> 
> 
> On 31.01.2018 13:58, Emil Velikov wrote:
>> On 31 January 2018 at 07:17, Tapani Pälli <tapani.palli at intel.com> wrote:
>>> v2: cleanup, move callbacks to _egl_display struct (Emil Velikov)
>>>      adapt to earlier ctx->screen changes
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>>   src/egl/drivers/dri2/egl_dri2.c | 25 +++++++++++++++++++++++++
>>>   src/egl/drivers/dri2/egl_dri2.h |  1 +
>>>   src/egl/main/eglapi.c           | 30 ++++++++++++++++++++++++++++++
>>>   src/egl/main/eglapi.h           |  4 ++++
>>>   src/egl/main/egldisplay.h       |  4 ++++
>>>   src/egl/main/eglentrypoint.h    |  1 +
>>>   6 files changed, 65 insertions(+)
>>>
>>> diff --git a/src/egl/drivers/dri2/egl_dri2.c 
>>> b/src/egl/drivers/dri2/egl_dri2.c
>>> index d5a4f72e86..a54f8a4d96 100644
>>> --- a/src/egl/drivers/dri2/egl_dri2.c
>>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>>> @@ -458,6 +458,7 @@ static const struct dri2_extension_match 
>>> optional_core_extensions[] = {
>>>      { __DRI2_INTEROP, 1, offsetof(struct dri2_egl_display, interop) },
>>>      { __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
>>>      { __DRI2_FLUSH_CONTROL, 1, offsetof(struct dri2_egl_display, 
>>> flush_control) },
>>> +   { __DRI2_BLOB, 1, offsetof(struct dri2_egl_display, blob) },
>>>      { NULL, 0, 0 }
>>>   };
>>>
>>> @@ -727,6 +728,9 @@ dri2_setup_screen(_EGLDisplay *disp)
>>>         }
>>>      }
>>>
>>> +   if (dri2_dpy->blob)
>>> +      disp->Extensions.ANDROID_blob_cache = EGL_TRUE;
>>> +
>>>      disp->Extensions.KHR_reusable_sync = EGL_TRUE;
>>>
>>>      if (dri2_dpy->image) {
>>> @@ -3016,6 +3020,26 @@ dri2_dup_native_fence_fd(_EGLDriver *drv, 
>>> _EGLDisplay *dpy, _EGLSync *sync)
>>>      return dup(sync->SyncFd);
>>>   }
>>>
>>> +static void
>>> +dri2_set_blob_cache_funcs(_EGLDriver *drv, _EGLDisplay *dpy,
>>> +                          EGLSetBlobFuncANDROID set,
>>> +                          EGLGetBlobFuncANDROID get)
>>> +{
>>> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
>>> +
>>> +   /* No blob support. */
>>> +   if (!dri2_dpy->blob)
>>> +      return;
>>> +
>> Should never(tm) happen. As in: the extension won't be advertised and
>> thus applications shouldn't use the func. pointer they get from
>> eglGetProcAddress.
>> If we'd want to catch such abuse it ought to be in eglapi.c.
> 
> Fair enough, it should be unnecessary. I had it because I haven't seen 
> actual apps care too much about extension string but in this case it's 
> fine because it's Android EGL layer that utilizes this, not them buggy 
> apps.
> 
> 
>>> +   /* No functions to set. */
>>> +   if (!dpy->BlobCacheSet)
>>> +      return;
>>> +
>> Not needed - single caller that errors out if the pointer is NULL.
> 
> Will remove
> 
>>> +   dri2_dpy->blob->set_cache_funcs(dri2_dpy->dri_screen,
>>> +                                   dpy->BlobCacheSet,
>>> +                                   dpy->BlobCacheGet);
>>> +}
>>> +
>>>   static EGLint
>>>   dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync 
>>> *sync,
>>>                         EGLint flags, EGLTime timeout)
>>> @@ -3234,6 +3258,7 @@ _eglBuiltInDriver(void)
>>>      dri2_drv->API.GLInteropQueryDeviceInfo = 
>>> dri2_interop_query_device_info;
>>>      dri2_drv->API.GLInteropExportObject = dri2_interop_export_object;
>>>      dri2_drv->API.DupNativeFenceFDANDROID = dri2_dup_native_fence_fd;
>>> +   dri2_drv->API.SetBlobCacheFuncsANDROID = dri2_set_blob_cache_funcs;
>>>
>>>      dri2_drv->Name = "DRI2";
>>>
>>> diff --git a/src/egl/drivers/dri2/egl_dri2.h 
>>> b/src/egl/drivers/dri2/egl_dri2.h
>>> index cc76c73eab..c49156fbb6 100644
>>> --- a/src/egl/drivers/dri2/egl_dri2.h
>>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>>> @@ -171,6 +171,7 @@ struct dri2_egl_display
>>>      const __DRInoErrorExtension    *no_error;
>>>      const __DRI2configQueryExtension *config;
>>>      const __DRI2fenceExtension *fence;
>>> +   const __DRI2blobExtension *blob;
>>>      const __DRI2rendererQueryExtension *rendererQuery;
>>>      const __DRI2interopExtension *interop;
>>>      int                       fd;
>>> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>>> index 5110688f2d..f2ba260060 100644
>>> --- a/src/egl/main/eglapi.c
>>> +++ b/src/egl/main/eglapi.c
>>> @@ -476,6 +476,7 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
>>>      char *exts = dpy->ExtensionsString;
>>>
>>>      /* Please keep these sorted alphabetically. */
>>> +   _EGL_CHECK_EXTENSION(ANDROID_blob_cache);
>>>      _EGL_CHECK_EXTENSION(ANDROID_framebuffer_target);
>>>      _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
>>>      _EGL_CHECK_EXTENSION(ANDROID_native_fence_sync);
>>> @@ -2522,6 +2523,35 @@ eglQueryDmaBufModifiersEXT(EGLDisplay dpy, 
>>> EGLint format, EGLint max_modifiers,
>>>      RETURN_EGL_EVAL(disp, ret);
>>>   }
>>>
>>> +static void EGLAPIENTRY
>>> +eglSetBlobCacheFuncsANDROID(EGLDisplay *dpy, EGLSetBlobFuncANDROID set,
>>> +                            EGLGetBlobFuncANDROID get)
>>> +{
>>> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
>>> +   _EGLDriver *drv = _eglCheckDisplay(disp, __func__);
>>> +
>> This is the only EGL API which has no return type. Hence we cannot use
>> the _EGL_FUNC_START/_EGL_CHECK_DISPLAY macros ;-(
>> We'd want the _eglSetFuncName() call (from the former macro) though.
>>
>> I'd add a very small comment + [sort of] inline the macros.
> 
> True, I forgot _eglSetFuncName which can be useful for EGL_KHR_debug 
> when _eglError cases are hit. I will add this.
> 
Just noticed that there is no EGL_OBJECT type defined for those 
callbacks .. so it'll be just about the function name.

// Tapani