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

Tomasz Figa tfiga at chromium.org
Wed Aug 23 14:47:24 UTC 2017


Hi Yogesh,

Sorry for being late with review. Please see some comments inline.

On Fri, Aug 18, 2017 at 7:08 PM,  <yogesh.marathe at intel.com> wrote:
> 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;
> +   }

nit: It might not change anything in practice, but it would be more
logical if the code always initialized enable_out_fence to some value.
So maybe let's add dri2_surf->enable_out_fence = 0; above the if.

> +
> +   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);

We don't need to call this if enable_out_fence is 0. Actually, We can
invert the if above and return early if !enable_out_fence.

> +}
> +
>  /**
>   * 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;

Since this is declared as EGLBoolean in dri2_init_surface(), let's use
the same type here too.

>  };
>
>  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);

^ EGLBoolean used for enable_out_fence here.

With these fixed (and no changes other than that), feel free to add:

Reviewed-by: Tomasz Figa <tfiga at chromium.org>

Best regards,
Tomasz


More information about the mesa-dev mailing list