Hi Rafael,
On Sat, Jul 15, 2017 at 1:45 AM, Rafael Antognolli
<rafael.antogno...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
