[Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)
Rafael Antognolli
rafael.antognolli at intel.com
Fri Jul 14 16:45:12 UTC 2017
On Fri, Jul 14, 2017 at 09:13:49AM +0100, Chris Wilson wrote:
> Quoting Zhongmin Wu (2017-07-14 07:55:45)
> > 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.
> >
> > 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);
> > 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)
> > {
> > 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);
> > 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;
> > + }
>
> No. Use the correct interfaces for creating a fence, stop imposing the
> cost upon everyone else.
Yeah, that's correct, it definitely shouldn't be inside intel_batchbuffer.
Chris, the main problem that Zhongmin raised was that on
droid_window_cancel_buffer the context has been previously destroyed already,
so they can't create a new fence for that old context. I haven't checked this
myself as I don't have an android build.
Still, it looks to me that they would need to store the previous fence to
solve this.
So, the right place to do so would be inside platform_android.c,
right? And since I don't see any private struct that could store such fence
there, one option would be to extend the struct dri2_egl_surface for android,
adding private data there. Does that make sense?
(just trying to help clarify this discussion)
Rafael
More information about the mesa-dev
mailing list