[Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)

Eric Engestrom eric.engestrom at imgtec.com
Fri Jul 14 10:37:12 UTC 2017


On Friday, 2017-07-14 14:55:45 +0800, Zhongmin Wu 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.

I don't know much about fences, sorry, shallow review.

> 
> Change-Id: Ic0773c19788d612a98d1402f5b5619dab64c1bc2
> Tracked-On: https://jira01.devtools.intel.com/browse/OAM-43936
> Reported-On: https://bugs.freedesktop.org/show_bug.cgi?id=101655
> Signed-off-by: Zhongmin Wu <zhongmin.wu at intel.com>
> Reported-by: Li, Guangli <guangli.li at intel.com>
> Tested-by: Marathe, Yogesh <yogesh.marathe at intel.com>
> ---
>  include/GL/internal/dri_interface.h           |    9 +++++++++
>  src/egl/drivers/dri2/platform_android.c       |   20 +++++++++++++-------
>  src/mesa/drivers/dri/common/dri_util.c        |    3 +++
>  src/mesa/drivers/dri/common/dri_util.h        |    2 ++
>  src/mesa/drivers/dri/i915/intel_screen.c      |    1 +
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c |   18 +++++++++++++++++-
>  src/mesa/drivers/dri/i965/intel_screen.c      |    6 ++++++
>  src/mesa/drivers/dri/nouveau/nouveau_screen.c |    1 +
>  src/mesa/drivers/dri/radeon/radeon_screen.c   |    1 +
>  9 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index fc2d4bb..7c6b08b 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -287,6 +287,15 @@ struct __DRI2flushExtensionRec {
>      void (*flush)(__DRIdrawable *drawable);
>  
>      /**
> +    * This function enables retrieving fence during enqueue / cancel buffer operations
> +    *
> +    * \param drawable the drawable to invalidate
> +    *
> +    * \since 3
> +    */
> +    int (*get_retrieve_fd)(__DRIdrawable *drawable);
> +
> +    /**
>       * Ask the driver to call getBuffers/getBuffersWithFormat before
>       * it starts rendering again.
>       *
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index cc2e4a6..aa99ed3 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -305,10 +305,11 @@ 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 fd = -1;
> +   if (dri2_dpy->flush->get_retrieve_fd)
> +      fd = dri2_dpy->flush->get_retrieve_fd(dri2_surf->dri_drawable);

Nit: these two lines would be the only diff in this hunk without the
rename, and `fence_fd` sounds like a better name than just `fd` :P

>     dri2_surf->window->queueBuffer(dri2_surf->window, dri2_surf->buffer,
> -                                  fence_fd);
> -
> +                                  fd);
>     dri2_surf->buffer->common.decRef(&dri2_surf->buffer->common);
>     dri2_surf->buffer = NULL;
>     dri2_surf->back = NULL;
> @@ -324,11 +325,16 @@ droid_window_enqueue_buffer(_EGLDisplay *disp, struct dri2_egl_surface *dri2_sur
>  }
>  
>  static void
> -droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
> +droid_window_cancel_buffer(_EGLDisplay *disp,
> +                           struct dri2_egl_surface *dri2_surf)

Do we really want to mix these?
You could pass `dri2_egl_display(disp)` when calling the function.

>  {
>     int ret;
> -
> -   ret = dri2_surf->window->cancelBuffer(dri2_surf->window, dri2_surf->buffer, -1);
> +   int fd = -1;
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> +   if (dri2_dpy->flush->get_retrieve_fd)
> +      fd = dri2_dpy->flush->get_retrieve_fd(dri2_surf->dri_drawable);
> +   ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
> +                                         dri2_surf->buffer, fd);
>     if (ret < 0) {
>        _eglLog(_EGL_WARNING, "ANativeWindow::cancelBuffer failed");
>        dri2_surf->base.Lost = EGL_TRUE;
> @@ -469,7 +475,7 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
>  
>     if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>        if (dri2_surf->buffer)
> -         droid_window_cancel_buffer(dri2_surf);
> +         droid_window_cancel_buffer(disp, dri2_surf);
>  
>        dri2_surf->window->common.decRef(&dri2_surf->window->common);
>     }
> diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
> index f6df488..21ee6aa 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -636,6 +636,8 @@ static void dri_put_drawable(__DRIdrawable *pdp)
>  	    return;
>  
>  	pdp->driScreenPriv->driver->DestroyBuffer(pdp);
> +    if (pdp->retrieve_fd != -1)
> +       close(pdp->retrieve_fd);

Indentation in this file needs fixing, but until then you should match
what's around, ie. tabs here, so it doesn't look too wrong.

>  	free(pdp);
>      }
>  }
> @@ -661,6 +663,7 @@ driCreateNewDrawable(__DRIscreen *screen,
>      pdraw->lastStamp = 0;
>      pdraw->w = 0;
>      pdraw->h = 0;
> +    pdraw->retrieve_fd = -1;
>  
>      dri_get_drawable(pdraw);
>  
> diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
> index 8fcd632..22a5887 100644
> --- a/src/mesa/drivers/dri/common/dri_util.h
> +++ b/src/mesa/drivers/dri/common/dri_util.h
> @@ -274,6 +274,8 @@ struct __DRIdrawableRec {
>      struct {
>  	unsigned int stamp;
>      } dri2;
> +
> +    int retrieve_fd;
>  };
>  
>  extern uint32_t
> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> index cba5434..38d0e63 100644
> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> @@ -182,6 +182,7 @@ static const struct __DRI2flushExtensionRec intelFlushExtension = {
>  
>      .flush              = intelDRI2Flush,
>      .invalidate         = dri2InvalidateDrawable,
> +    .get_retrieve_fd    = NULL,
>  };
>  
>  static struct intel_image_format intel_image_formats[] = {
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 62d2fe8..9813c8c 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -648,9 +648,25 @@ do_flush_locked(struct brw_context *brw, int in_fence_fd, int *out_fence_fd)
>           /* Add the batch itself to the end of the validation list */
>           add_exec_bo(batch, batch->bo);
>  
> +         if (brw->driContext->driDrawablePriv &&
> +            brw->driContext->driDrawablePriv->retrieve_fd != -1) {
> +            close(brw->driContext->driDrawablePriv->retrieve_fd);
> +            brw->driContext->driDrawablePriv->retrieve_fd = -1;
> +         }
> +
> +         int fd = -1;
>           ret = execbuffer(dri_screen->fd, batch, hw_ctx,
>                            4 * USED_BATCH(*batch),
> -                          in_fence_fd, out_fence_fd, flags);
> +                          in_fence_fd, &fd, flags);
> +
> +         if (out_fence_fd != NULL) {
> +            *out_fence_fd = fd;
> +            if (brw->driContext->driDrawablePriv)
> +               brw->driContext->driDrawablePriv->retrieve_fd = dup(fd);
> +         } else {
> +            if (brw->driContext->driDrawablePriv)
> +               brw->driContext->driDrawablePriv->retrieve_fd  = fd;
> +         }
>        }
>  
>        throttle(brw);
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 7d320c0..9f08845 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -138,6 +138,11 @@ static const __DRItexBufferExtension intelTexBufferExtension = {
>     .releaseTexBuffer    = NULL,
>  };
>  
> +static int intel_dri2_get_retreive_fd(__DRIdrawable *draw)
> +{
> +       return dup(draw->retrieve_fd);
> +}
> +
>  static void
>  intel_dri2_flush_with_flags(__DRIcontext *cPriv,
>                              __DRIdrawable *dPriv,
> @@ -184,6 +189,7 @@ static const struct __DRI2flushExtensionRec intelFlushExtension = {
>      .flush              = intel_dri2_flush,
>      .invalidate         = dri2InvalidateDrawable,
>      .flush_with_flags   = intel_dri2_flush_with_flags,
> +    .get_retrieve_fd    = intel_dri2_get_retreive_fd,
>  };
>  
>  static struct intel_image_format intel_image_formats[] = {
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_screen.c b/src/mesa/drivers/dri/nouveau/nouveau_screen.c
> index 375f640..11f17b5 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_screen.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_screen.c
> @@ -309,6 +309,7 @@ static const struct __DRI2flushExtensionRec nouveau_flush_extension = {
>  
>     .flush               = nouveau_drawable_flush,
>     .invalidate          = dri2InvalidateDrawable,
> +   .get_retrieve_fd     = NULL,
>  };
>  
>  static const struct __DRItexBufferExtensionRec nouveau_texbuffer_extension = {
> diff --git a/src/mesa/drivers/dri/radeon/radeon_screen.c b/src/mesa/drivers/dri/radeon/radeon_screen.c
> index 79e3889..d55b447 100644
> --- a/src/mesa/drivers/dri/radeon/radeon_screen.c
> +++ b/src/mesa/drivers/dri/radeon/radeon_screen.c
> @@ -185,6 +185,7 @@ static const struct __DRI2flushExtensionRec radeonFlushExtension = {
>  
>     .flush               = radeonDRI2Flush,
>     .invalidate          = dri2InvalidateDrawable,
> +   .get_retrieve_fd     = NULL,
>  };
>  
>  static __DRIimage *
> -- 
> 1.7.9.5
> 


More information about the mesa-dev mailing list