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

[Tomasz Figa]

[resend from right email address, without bouncing recipients and with
all the people involved in v1... Zhongmin, please remember to add all
people participating in the discussion to CC next time.]

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).

>
> 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).

Best regards,
Tomasz