Tomasz, > -----Original Message----- > From: Tomasz Figa [mailto:tf...@chromium.org] > Sent: Friday, August 4, 2017 3:48 PM > To: Marathe, Yogesh <yogesh.mara...@intel.com> > Cc: Emil Velikov <emil.l.veli...@gmail.com>; Wu, Zhongmin > <zhongmin...@intel.com>; Gao, Shuo <shuo....@intel.com>; Antognolli, > Rafael <rafael.antogno...@intel.com>; Timothy Arceri > <tarc...@itsqueeze.com>; Kenneth Graunke <kenn...@whitecape.org>; > Kondapally, Kalyan <kalyan.kondapa...@intel.com>; ML mesa-dev <mesa- > d...@lists.freedesktop.org> > Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation 1/2] i965: > Return the last fence if the batch buffer is empty and nothing to be flushed > when > _intel_batchbuffer_flush_fence. > > Hi Yogesh, > > On Fri, Aug 4, 2017 at 7:12 PM, Marathe, Yogesh <yogesh.mara...@intel.com> > wrote: > > Hi Emil, > > > >> -----Original Message----- > >> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > >> Behalf Of Emil Velikov > >> Sent: Tuesday, July 25, 2017 8:19 PM > >> To: Wu, Zhongmin <zhongmin...@intel.com> > >> Cc: Gao, Shuo <shuo....@intel.com>; Antognolli, Rafael > >> <rafael.antogno...@intel.com>; Timothy Arceri > >> <tarc...@itsqueeze.com>; Marathe, Yogesh <yogesh.mara...@intel.com>; > >> Tomasz Figa <tf...@chromium.org>; Kenneth Graunke > >> <kenn...@whitecape.org>; Kondapally, Kalyan > >> <kalyan.kondapa...@intel.com>; ML mesa-dev <mesa- > >> d...@lists.freedesktop.org> > >> Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation 1/2] > i965: > >> Return the last fence if the batch buffer is empty and nothing to be > >> flushed when _intel_batchbuffer_flush_fence. > >> > >> Hi Zhongmin, > >> > >> Is the issue resolved by the EGL patch alone? Worth sticking with that for > now? > >> > >> I think this patch will cause some noticeable overhead - see below for > >> details. > >> > >> > >> On 21 July 2017 at 04:08, Zhongmin Wu <zhongmin...@intel.com> wrote: > >> > Always save the last fence in the brw context when flushing buffer. > >> > If the buffer is nothing to be flushed, then return the last fence > >> > when asked for. > >> > > >> > Change-Id: Ic47035bcd1a27e402609afd9e2d1e3972548b97d > >> > Signed-off-by: Zhongmin Wu <zhongmin...@intel.com> > >> > --- > >> > src/mesa/drivers/dri/i965/brw_context.c | 5 +++++ > >> > src/mesa/drivers/dri/i965/brw_context.h | 1 + > >> > src/mesa/drivers/dri/i965/intel_batchbuffer.c | 16 ++++++++++++++-- > >> > 3 files changed, 20 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > >> > b/src/mesa/drivers/dri/i965/brw_context.c > >> > index 5433f90..ed0b056 100644 > >> > --- a/src/mesa/drivers/dri/i965/brw_context.c > >> > +++ b/src/mesa/drivers/dri/i965/brw_context.c > >> > @@ -1086,6 +1086,8 @@ brwCreateContext(gl_api api, > >> > ctx->VertexProgram._MaintainTnlProgram = true; > >> > ctx->FragmentProgram._MaintainTexEnvProgram = true; > >> > > >> > + brw->out_fence_fd = -1; > >> > + > >> > brw_draw_init( brw ); > >> > > >> > if ((flags & __DRI_CTX_FLAG_DEBUG) != 0) { @@ -1169,6 +1171,9 > >> > @@ intelDestroyContext(__DRIcontext * driContextPriv) > >> > brw->throttle_batch[1] = NULL; > >> > brw->throttle_batch[0] = NULL; > >> > > >> > + if (brw->out_fence_fd >= 0) > >> > + close(brw->out_fence_fd); > >> > + > >> > driDestroyOptionCache(&brw->optionCache); > >> > > >> > /* free the Mesa context */ > >> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > >> > b/src/mesa/drivers/dri/i965/brw_context.h > >> > index dc4bc8f..692ea2c 100644 > >> > --- a/src/mesa/drivers/dri/i965/brw_context.h > >> > +++ b/src/mesa/drivers/dri/i965/brw_context.h > >> > @@ -1217,6 +1217,7 @@ struct brw_context > >> > > >> > __DRIcontext *driContext; > >> > struct intel_screen *screen; > >> > + int out_fence_fd; > >> > }; > >> > > >> > /* brw_clear.c */ > >> > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c > >> > b/src/mesa/drivers/dri/i965/intel_batchbuffer.c > >> > index 62d2fe8..d342e5d 100644 > >> > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c > >> > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c > >> > @@ -648,9 +648,18 @@ 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->out_fence_fd >= 0) { > >> > + close(brw->out_fence_fd); > >> > + brw->out_fence_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); > >> execbuffer() creates an out fence if the "out_fence_fd" pointer is > >> non-NULL. > >> Hence with this patch we'will create a fence for each > >> _intel_batchbuffer_flush_fence() invocation... > >> > >> Not sure how costly that will be though :-\ > >> > > > > I see this results into 1 get_unused_fd_flags() + 1 sync_file_create() > > and operations to store out fd in kernel for return arg. I doubt it > > will be very costly, the ioctl > > DRM_IOCTL_I915_GEM_EXECBUFFER2 or > DRM_IOCTL_I915_GEM_EXECBUFFER2_WR > > was anyways there, so nothing huge additional on ioctl front. > > > > Nonetheless, what other option do we have? This may sound absurd but > > is there a way / ioctl to call sync_file_create directly from mesa > > UMD and store fds instead of creating at every execbuffer()? We also > > need a way to associate those stored fds to buffers then. I know I'm > > adding more ioctls but intension is, if we can do this in some > > init() we'll save these operations during execbuffer(). > > > > IMHO, this is separate discussion, let this patch enable functionality > > first and then we can work on making it light. > > The question was whether the EGL patch alone solves the Flatland issue. We > need it answered first before we continue with further discussion.
Sorry, I assumed it won't, I tested flatland again with egl patch alone and it worked! Will check overall stability of system and confirm if we can drop this by Monday (shouldn’t break anything else). > > Best regards, > Tomasz _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
