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...@intel.com> > Reported-by: Li, Guangli <guangli...@intel.com> > Tested-by: Marathe, Yogesh <yogesh.mara...@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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev