[Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

Emil Velikov emil.l.velikov at gmail.com
Tue Jul 25 14:16:16 UTC 2017


Hi Zhongmin,

Thanks you for the update. There's a couple of important comments -
dri2_make_current + droid_window_enqueue_buffer.
The rest is just nitpiks.

Tomasz, hats down for the immense help and guidance.

On the potential performance hit (due to the extra fence), I think we
could do some tests before adding extra infra.
No obvious benchmarks come to mind - any suggestions?

On 25 July 2017 at 03:07, Zhongmin Wu <zhongmin.wu at intel.com> wrote:
> Before we queued the buffer with a invalid fence (-1), it will
> make some benchmarks failed to test such as flatland.
>
> Now we get the out fence during the flushing buffer and then pass
> it to SurfaceFlinger in eglSwapbuffer function.
>
> 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 breaked into two patches.
>
> v4.2: a) Add a deinit interface for surface to clear the out fence
>
> Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> Signed-off-by: Zhongmin Wu <zhongmin.wu at intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c             |   51 +++++++++++++++++++++++++++
>  src/egl/drivers/dri2/egl_dri2.h             |    8 +++++
>  src/egl/drivers/dri2/platform_android.c     |   12 ++++---
>  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, 77 insertions(+), 9 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 020a0bc..ffd3a8a 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay *disp, _EGLContext *ctx)
>     return EGL_TRUE;
>  }
>
> +EGLBoolean
> +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
nit: s/dri2_surf_init/dri2_init_surface/

> +        _EGLConfig *conf, const EGLint *attrib_list)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surf->out_fence_fd = -1;
> +   return _eglInitSurface(surf, dpy, type, conf, attrib_list);
> +}
> +
> +static void
> +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd)
nit: s/dri2_surface_set_out_fence/dri2_surface_set_out_fence_fd/

> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   if (dri2_surf->out_fence_fd >=0)
nit: space between = and 0

> +      close(dri2_surf->out_fence_fd);
> +
> +   dri2_surf->out_fence_fd = fence_fd;
> +}
> +
> +void
> +dri2_surf_deinit(_EGLSurface *surf)
> +{
nit: s/dri2_surf_deinit/dri2_fini_surface/

> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surface_set_out_fence(surf, -1);
> +}
> +
>  static EGLBoolean
>  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>  {
> @@ -1318,6 +1344,22 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>     return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);
>  }
>
> +static void
> +dri2_surf_get_fence_fd(_EGLContext *ctx,
> +                       _EGLDisplay *dpy, _EGLSurface *surf)
> +{
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   int fence_fd = -1;
> +   __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context;
> +   void * fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1);
Add blank line between declarations and code.

> +   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(surf, fence_fd);
> +}
> +
>  /**
>   * Called via eglMakeCurrent(), drv->API.MakeCurrent().
>   */
> @@ -1352,8 +1394,11 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
>     rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
>     cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
>
> +   int fence_fd = -1;
Unused variable?

>     if (old_ctx) {
>        __DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
> +      if (old_dsurf)
> +         dri2_surf_get_fence_fd(old_ctx, disp, old_dsurf);
Are you sure we should be using the old surface and not the new one?
Can you elaborate why?

>        dri2_dpy->core->unbindContext(old_cctx);
>     }
>
> @@ -1490,6 +1535,9 @@ static EGLBoolean
>  dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>  {
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   _EGLContext *ctx = _eglGetCurrentContext();
Blank line.

> +   if (ctx && surf)
> +      dri2_surf_get_fence_fd(ctx, dpy, surf);
>     return dri2_dpy->vtbl->swap_buffers(drv, dpy, surf);
>  }
>
> @@ -1499,6 +1547,9 @@ 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();
Blank line.

> +   if (ctx && surf)
> +      dri2_surf_get_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 bbba7c0..4ea3adb 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -320,6 +320,7 @@ struct dri2_egl_surface
>        __DRIimage           *front;
>        unsigned int         visual;
>  #endif
> +   int out_fence_fd;
>  };
>
>  struct dri2_egl_config
> @@ -446,4 +447,11 @@ dri2_set_WL_bind_wayland_display(_EGLDriver *drv, _EGLDisplay *disp)
>  void
>  dri2_display_destroy(_EGLDisplay *disp);
>
> +EGLBoolean
> +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> +        _EGLConfig *conf, const EGLint *attrib_list);
> +
> +void
> +dri2_surf_deinit(_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 cc2e4a6..4112a0a 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -305,7 +305,8 @@ droid_window_enqueue_buffer(_EGLDisplay *disp, struct dri2_egl_surface *dri2_sur
>      *    is passed to queueBuffer, and the ANativeWindow implementation
>      *    is responsible for closing it.
>      */
> -   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);
>
The comment above the function needs updating.

> @@ -327,8 +328,10 @@ static void
>  droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
>  {
>     int ret;
> -
> -   ret = dri2_surf->window->cancelBuffer(dri2_surf->window, dri2_surf->buffer, -1);
> +   int fence_fd = dri2_surf->out_fence_fd;
Blank line.

> +   dri2_surf->out_fence_fd = -1;
> +   ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
> +                                         dri2_surf->buffer, fence_fd);


Thanks
Emil


More information about the mesa-dev mailing list