> -----Original Message----- > From: Wu, Zhongmin > Sent: Monday, August 7, 2017 11:27 AM > To: Marathe, Yogesh <yogesh.mara...@intel.com>; Tomasz Figa > <tf...@chromium.org> > Cc: Gao, Shuo <shuo....@intel.com>; Antognolli, Rafael > <rafael.antogno...@intel.com>; Emil Velikov <emil.l.veli...@gmail.com>; > Kenneth Graunke <kenn...@whitecape.org>; ML mesa-dev <mesa- > d...@lists.freedesktop.org>; Kondapally, Kalyan > <kalyan.kondapa...@intel.com>; Timothy Arceri <tarc...@itsqueeze.com> > 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: > http://oorja.iind.intel.com/mediawiki/index.php/Flatland > can you also try the flatland on this page. > > For AOSP flatland, yes, the EGL patch may solve the issue.
That should suffice. > > However, I met one case that the batch buffer is empty just at the > swapbuffer > (glfush is just called before that), then you might not get a fence fd. So I > had to > record the last fence fd. > > I am not sure, you had better try it on the board If you can get a valid fd > on any > situation. I think aosp flatland is more relevant here. I haven’t face the issue you have mentioned so far. > > > > > > -----Original Message----- > From: Marathe, Yogesh > Sent: Monday, August 7, 2017 13:25 > To: Tomasz Figa <tf...@chromium.org>; Wu, Zhongmin > <zhongmin...@intel.com> > Cc: Gao, Shuo <shuo....@intel.com>; Antognolli, Rafael > <rafael.antogno...@intel.com>; Emil Velikov <emil.l.veli...@gmail.com>; > Kenneth Graunke <kenn...@whitecape.org>; ML mesa-dev <mesa- > d...@lists.freedesktop.org>; Kondapally, Kalyan > <kalyan.kondapa...@intel.com>; Timothy Arceri <tarc...@itsqueeze.com> > 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. > > This can be dropped. I'm running with egl patch alone and things seem fine. > > Zhongmin, please comment if you don’t think so. > > > -----Original Message----- > > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > > Behalf Of Marathe, Yogesh > > Sent: Friday, August 4, 2017 9:18 PM > > > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
