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.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).
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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
