[Mesa-dev] [PATCH v6.1] egl: Allow creation of per surface out fence

Marathe, Yogesh yogesh.marathe at intel.com
Mon Aug 21 15:45:25 UTC 2017


Can someone please comment on this? Are we done / still not done here?

Regards,
Yogesh.

> -----Original Message-----
> From: Marathe, Yogesh
> Sent: Friday, August 18, 2017 3:39 PM
> To: mesa-dev at lists.freedesktop.org
> Cc: tfiga at chromium.org; emil.l.velikov at gmail.com; Gao, Shuo
> <shuo.gao at intel.com>; Liu, Zhiquan <zhiquan.liu at intel.com>;
> daniels at collabora.com; nicolai.haehnle at amd.com; Antognolli, Rafael
> <rafael.antognolli at intel.com>; eric at engestrom.ch; kenneth at whitecape.org;
> fernetmenta at online.de; Kondapally, Kalyan <kalyan.kondapally at intel.com>;
> tarceri at itsqueeze.com; varad.gautam at collabora.com; Wu, Zhongmin
> <zhongmin.wu at intel.com>; Marathe, Yogesh <yogesh.marathe at intel.com>
> Subject: [PATCH v6.1] egl: Allow creation of per surface out fence
> 
> From: Zhongmin Wu <zhongmin.wu at intel.com>
> 
> Add plumbing to allow creation of per display surface out fence.
> 
> Currently enabled only on android, since the system expects a valid fd in
> ANativeWindow::{queue,cancel}Buffer. We pass a fd of -1 with which native
> applications such as flatland fail. The patch enables explicit sync on android and
> fixes one of the functional issue for apps or buffer consumers which depend
> upon fence and its timestamp.
> 
> v2: a) Also implement the fence in cancelBuffer.
>     b) The last sync fence is stored in drawable object
>        rather than brw context.
>     c) format clear.
> 
> v3: a) Save the last fence fd in DRI Context object.
>     b) Return the last fence if the batch buffer is empty and
>        nothing to be flushed when _intel_batchbuffer_flush_fence
>     c) Add the new interface in vbtl to set the retrieve fence
> 
> v3.1 a) close fd in the new vbtl interface on none Android platform
> 
> v4: a) The last fence is saved in brw context.
>     b) The retrieve fd is for all the platform but not just Android
>     c) Add a uniform dri2 interface to initialize the surface.
> 
> v4.1: a) make some changes of variable name.
>       b) the patch is broken into two patches.
> 
> v4.2: a) Add a deinit interface for surface to clear the out fence
> 
> v5: a) Add enable_out_fence to init, platform sets it true or
>        false
>     b) Change get fd to update fd and check for fence
>     c) Commit description updated
> 
> v6: a) Heading and commit description updated
>     b) enable_out_fence is set only if fence is supported
>     c) Review comments on function names
>     d) Test with standalone patch, resolves the bug
> 
> v6.1 a) Check for old display fence reverted back
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101655
> 
> Signed-off-by: Zhongmin Wu <zhongmin.wu at intel.com>
> Signed-off-by: Yogesh Marathe <yogesh.marathe at intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c             | 69 +++++++++++++++++++++++++++++
>  src/egl/drivers/dri2/egl_dri2.h             |  9 ++++
>  src/egl/drivers/dri2/platform_android.c     | 29 ++++++------
>  src/egl/drivers/dri2/platform_drm.c         |  3 +-
>  src/egl/drivers/dri2/platform_surfaceless.c |  3 +-
>  src/egl/drivers/dri2/platform_wayland.c     |  3 +-
>  src/egl/drivers/dri2/platform_x11.c         |  3 +-
>  src/egl/drivers/dri2/platform_x11_dri3.c    |  3 +-
>  8 files changed, 104 insertions(+), 18 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index
> ed79e0d..04d0332 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1354,6 +1354,44 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLContext *ctx)
>     return EGL_TRUE;
>  }
> 
> +EGLBoolean
> +dri2_init_surface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> +        _EGLConfig *conf, const EGLint *attrib_list, EGLBoolean
> +enable_out_fence) {
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +
> +   dri2_surf->out_fence_fd = -1;
> +   if (dri2_dpy->fence && dri2_dpy->fence->base.version >= 2 &&
> +       dri2_dpy->fence->get_capabilities &&
> +       (dri2_dpy->fence->get_capabilities(dri2_dpy->dri_screen) &
> +        __DRI_FENCE_CAP_NATIVE_FD)) {
> +      dri2_surf->enable_out_fence = enable_out_fence;
> +   }
> +
> +   return _eglInitSurface(surf, dpy, type, conf, attrib_list); }
> +
> +static void
> +dri2_surface_set_out_fence_fd( _EGLSurface *surf, int fence_fd) {
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +
> +   if (dri2_surf->out_fence_fd >=0)
> +      close(dri2_surf->out_fence_fd);
> +
> +   dri2_surf->out_fence_fd = fence_fd;
> +}
> +
> +void
> +dri2_deinit_surface(_EGLSurface *surf)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +
> +   dri2_surface_set_out_fence_fd(surf, -1);
> +   dri2_surf->enable_out_fence = false; }
> +
>  static EGLBoolean
>  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)  {
> @@ -1365,6 +1403,27 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *dpy, _EGLSurface *surf)
>     return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);  }
> 
> +static void
> +dri2_surf_update_fence_fd(_EGLContext *ctx,
> +                          _EGLDisplay *dpy, _EGLSurface *surf) {
> +   __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context;
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   int fence_fd = -1;
> +   void *fence;
> +
> +   if (dri2_surf->enable_out_fence) {
> +      fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1);
> +      if (fence) {
> +         fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
> +                                                  fence);
> +         dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
> +      }
> +   }
> +   dri2_surface_set_out_fence_fd(surf, fence_fd); }
> +
>  /**
>   * Called via eglMakeCurrent(), drv->API.MakeCurrent().
>   */
> @@ -1401,6 +1460,8 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *dsurf,
> 
>     if (old_ctx) {
>        __DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
> +      if (old_dsurf)
> +         dri2_surf_update_fence_fd(old_ctx, disp, old_dsurf);
>        dri2_dpy->core->unbindContext(old_cctx);
>     }
> 
> @@ -1539,6 +1600,10 @@ static EGLBoolean  dri2_swap_buffers(_EGLDriver
> *drv, _EGLDisplay *dpy, _EGLSurface *surf)  {
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   _EGLContext *ctx = _eglGetCurrentContext();
> +
> +   if (ctx && surf)
> +      dri2_surf_update_fence_fd(ctx, dpy, surf);
>     return dri2_dpy->vtbl->swap_buffers(drv, dpy, surf);  }
> 
> @@ -1548,6 +1613,10 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
> _EGLDisplay *dpy,
>                                const EGLint *rects, EGLint n_rects)  {
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   _EGLContext *ctx = _eglGetCurrentContext();
> +
> +   if (ctx && surf)
> +      dri2_surf_update_fence_fd(ctx, dpy, surf);
>     return dri2_dpy->vtbl->swap_buffers_with_damage(drv, dpy, surf,
>                                                     rects, n_rects);  } diff --git
> a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index
> f471561..f4f47af 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -328,6 +328,8 @@ struct dri2_egl_surface
>        __DRIimage           *front;
>        unsigned int         visual;
>  #endif
> +   int out_fence_fd;
> +   bool enable_out_fence;
>  };
> 
>  struct dri2_egl_config
> @@ -456,4 +458,11 @@ dri2_set_WL_bind_wayland_display(_EGLDriver *drv,
> _EGLDisplay *disp)  void  dri2_display_destroy(_EGLDisplay *disp);
> 
> +EGLBoolean
> +dri2_init_surface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> +        _EGLConfig *conf, const EGLint *attrib_list, EGLBoolean
> +enable_out_fence);
> +
> +void
> +dri2_deinit_surface(_EGLSurface *surf);
> +
>  #endif /* EGL_DRI2_INCLUDED */
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index beb4740..b878544 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -229,19 +229,18 @@ droid_window_enqueue_buffer(_EGLDisplay *disp,
> struct dri2_egl_surface *dri2_sur
>      */
>     mtx_unlock(&disp->Mutex);
> 
> -   /* Queue the buffer without a sync fence. This informs the ANativeWindow
> -    * that it may access the buffer immediately.
> +   /* Queue the buffer with stored out fence fd. The ANativeWindow or buffer
> +    * consumer may choose to wait for the fence to signal before accessing
> +    * it. If fence fd value is -1, buffer can be accessed by consumer
> +    * immediately. Consumer or application shouldn't rely on timestamp
> +    * associated with fence if the fence fd is -1.
>      *
> -    * From ANativeWindow::dequeueBuffer:
> -    *
> -    *    The fenceFd argument specifies a libsync fence file descriptor for
> -    *    a fence that must signal before the buffer can be accessed.  If
> -    *    the buffer can be accessed immediately then a value of -1 should
> -    *    be used.  The caller must not use the file descriptor after it
> -    *    is passed to queueBuffer, and the ANativeWindow implementation
> -    *    is responsible for closing it.
> +    * Ownership of fd is transferred to consumer after queueBuffer and the
> +    * consumer is responsible for closing it. Caller must not use the fd
> +    * after passing it to queueBuffer.
>      */
> -   int fence_fd = -1;
> +   int fence_fd = dri2_surf->out_fence_fd;
> +   dri2_surf->out_fence_fd = -1;
>     dri2_surf->window->queueBuffer(dri2_surf->window, dri2_surf->buffer,
>                                    fence_fd);
> 
> @@ -263,8 +262,11 @@ static void
>  droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)  {
>     int ret;
> +   int fence_fd = dri2_surf->out_fence_fd;
> 
> -   ret = dri2_surf->window->cancelBuffer(dri2_surf->window, dri2_surf->buffer,
> -1);
> +   dri2_surf->out_fence_fd = -1;
> +   ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
> +                                         dri2_surf->buffer, fence_fd);
>     if (ret < 0) {
>        _eglLog(_EGL_WARNING, "ANativeWindow::cancelBuffer failed");
>        dri2_surf->base.Lost = EGL_TRUE;
> @@ -323,7 +325,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
> +   if (!dri2_init_surface(&dri2_surf->base, disp, type, conf,
> + attrib_list, true))
>        goto cleanup_surface;
> 
>     if (type == EGL_WINDOW_BIT) {
> @@ -423,6 +425,7 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> 
>     dri2_dpy->core->destroyDrawable(dri2_surf->dri_drawable);
> 
> +   dri2_deinit_surface(surf);
>     free(dri2_surf);
> 
>     return EGL_TRUE;
> diff --git a/src/egl/drivers/dri2/platform_drm.c
> b/src/egl/drivers/dri2/platform_drm.c
> index ee75a9b..be2af30 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -110,7 +110,7 @@ dri2_drm_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri2_surf->base, disp, EGL_WINDOW_BIT, conf,
> attrib_list))
> +   if (!_eglInitSurface(&dri2_surf->base, disp, EGL_WINDOW_BIT, conf,
> + attrib_list, false))
>        goto cleanup_surf;
> 
>     surf = gbm_dri_surface(surface);
> @@ -182,6 +182,7 @@ dri2_drm_destroy_surface(_EGLDriver *drv,
> _EGLDisplay *disp, _EGLSurface *surf)
>                                         dri2_surf->dri_buffers[i]);
>     }
> 
> +   dri2_deinit_surface(surf);
>     free(surf);
> 
>     return EGL_TRUE;
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> index 1091b4f..8fbd20f 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -124,7 +124,7 @@ dri2_surfaceless_create_surface(_EGLDriver *drv,
> _EGLDisplay *disp, EGLint type,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
> +   if (!dri2_init_surface(&dri2_surf->base, disp, type, conf,
> + attrib_list, false))
>        goto cleanup_surface;
> 
>     config = dri2_get_dri_config(dri2_conf, type, @@ -165,6 +165,7 @@
> surfaceless_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface
> *sur
> 
>     dri2_dpy->core->destroyDrawable(dri2_surf->dri_drawable);
> 
> +   dri2_deinit_surface(surf);
>     free(dri2_surf);
>     return EGL_TRUE;
>  }
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index 3535c4b..b7eae55 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -142,7 +142,7 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri2_surf->base, disp, EGL_WINDOW_BIT, conf,
> attrib_list))
> +   if (!dri2_init_surface(&dri2_surf->base, disp, EGL_WINDOW_BIT, conf,
> + attrib_list, false))
>        goto cleanup_surf;
> 
>     if (dri2_dpy->wl_drm) {
> @@ -300,6 +300,7 @@ dri2_wl_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
>     wl_proxy_wrapper_destroy(dri2_surf->wl_dpy_wrapper);
>     wl_event_queue_destroy(dri2_surf->wl_queue);
> 
> +   dri2_deinit_surface(surf);
>     free(surf);
> 
>     return EGL_TRUE;
> diff --git a/src/egl/drivers/dri2/platform_x11.c
> b/src/egl/drivers/dri2/platform_x11.c
> index 062c8a4..663e1b7 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -232,7 +232,7 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
> +   if (!dri2_init_surface(&dri2_surf->base, disp, type, conf,
> + attrib_list, false))
>        goto cleanup_surf;
> 
>     dri2_surf->region = XCB_NONE;
> @@ -394,6 +394,7 @@ dri2_x11_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
>     if (surf->Type == EGL_PBUFFER_BIT)
>        xcb_free_pixmap (dri2_dpy->conn, dri2_surf->drawable);
> 
> +   dri2_deinit_surface(surf);
>     free(surf);
> 
>     return EGL_TRUE;
> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c
> b/src/egl/drivers/dri2/platform_x11_dri3.c
> index 290b150..9e32360 100644
> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
> @@ -101,6 +101,7 @@ dri3_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> 
>     loader_dri3_drawable_fini(&dri3_surf->loader_drawable);
> 
> +   dri2_deinit_surface(surf);
>     free(surf);
> 
>     return EGL_TRUE;
> @@ -137,7 +138,7 @@ dri3_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
>        return NULL;
>     }
> 
> -   if (!_eglInitSurface(&dri3_surf->base, disp, type, conf, attrib_list))
> +   if (!dri2_init_surface(&dri3_surf->base, disp, type, conf,
> + attrib_list, false))
>        goto cleanup_surf;
> 
>     if (type == EGL_PBUFFER_BIT) {
> --
> 2.7.4



More information about the mesa-dev mailing list