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

[Rafael Antognolli]

On Sat, Jul 15, 2017 at 01:52:43AM +0900, Tomasz Figa wrote:
> Hi Rafael,
> 
> On Sat, Jul 15, 2017 at 1:45 AM, Rafael Antognolli
> <rafael.antognolli at intel.com> wrote:
> > On Fri, Jul 14, 2017 at 09:13:49AM +0100, Chris Wilson wrote:
> >> Quoting Zhongmin Wu (2017-07-14 07:55:45)
> [snip]
> >> >  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.
> 
> Yes, that's a problem. But it should be solved at the source and not
> by digging around it. IMHO it's a limitation of EGL DRI2 code and
> that's where it should be fixed, by using all the tools already
> available (i.e. DRI2 fence extension).

Sure, I'm not saying it shouldn't, was just trying to help.

> >
> > Still, it looks to me that they would need to store the previous fence to
> > solve this.
> 
> Makes sense.
> 
> >
> > 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?
> 
> I'd say these fences are not 100% Android-specific anymore. They are
> an upstream Linux feature and can be used on other platforms as well.
> I think wiring this properly in EGL DRI2 core would make sense,
> especially since this is the place where the context is being
> destroyed (platform_android doesn't handle context destruction).

Ah, yes, that makes sense. I'll see if I can help.

Thanks!
Rafael