[Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)
Wu, Zhongmin
zhongmin.wu at intel.com
Mon Jul 17 03:10:08 UTC 2017
Hi Tomasz and Rafael:
Thanks very much for your comments. And I am not familiar with the whole architecture of the mesa, but is the implementation of mesa with dri2 core interface something like below ?
It is not a formal patch, but to show my idea according to my understanding on your discussion.
(cancelBuffer is still a tricky thing to handle..)
1. The last fence is saved in DRIcontext.
2. Return the last fence if the batch buffer is empty in _intel_batchbuffer_flush_fence
3. We can use the interface create_fence and get_fence_fd to get the acquire fence in queue buffer..
4 . For cancel buffer, we have to get the fence before context is rest, and save it in dri2_egl_surface, and then get it out in cancel buffer ..
Thanks very much..
---
src/egl/drivers/dri2/egl_dri2.c | 9 ++++++++-
src/egl/drivers/dri2/egl_dri2.h | 1 +
src/egl/drivers/dri2/platform_android.c | 19 ++++++++++++++-----
src/mesa/drivers/dri/common/dri_util.h | 2 ++
src/mesa/drivers/dri/i965/intel_batchbuffer.c | 25 ++++++++++++++++++++++---
src/mesa/drivers/dri/i965/intel_screen.c | 5 +++++
6 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 020a0bc..12e8946 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1351,9 +1351,16 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) : NULL;
rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
-
+ int fence_fd = -1;
if (old_ctx) {
__DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
+ void * fence = dri2_dpy->fence->create_fence(old_cctx);
+ fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen, fence);
+ dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
+ struct dri2_egl_surface *dri2_surf = dri2_egl_surface(old_dsurf);
+ if (dri2_surf->retrieve_fd != -1)
+ close(dri2_surf->retrieve_fd);
+ dri2_surf->retrieve_fd = fence_fd;
dri2_dpy->core->unbindContext(old_cctx);
}
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index bbba7c0..b470250 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -314,6 +314,7 @@ struct dri2_egl_surface
struct ANativeWindowBuffer *buffer;
int age;
} color_buffers[3], *back;
+ int retrieve_fd;
#endif
#if defined(HAVE_SURFACELESS_PLATFORM)
diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
index cc2e4a6..90cf1d3 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -306,9 +306,13 @@ droid_window_enqueue_buffer(_EGLDisplay *disp, struct dri2_egl_surface *dri2_sur
* is responsible for closing it.
*/
int fence_fd = -1;
+ _EGLContext *ctx = _eglGetCurrentContext();
+ struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
+ void * fence = dri2_dpy->fence->create_fence(dri2_ctx->dri_context);
+ fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen, fence);
dri2_surf->window->queueBuffer(dri2_surf->window, dri2_surf->buffer,
fence_fd);
-
+ dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
dri2_surf->buffer->common.decRef(&dri2_surf->buffer->common);
dri2_surf->buffer = NULL;
dri2_surf->back = NULL;
@@ -324,11 +328,16 @@ droid_window_enqueue_buffer(_EGLDisplay *disp, struct dri2_egl_surface *dri2_sur
}
static void
-droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
+droid_window_cancel_buffer(_EGLDisplay *disp,
+ struct dri2_egl_surface *dri2_surf)
{
int ret;
-
- ret = dri2_surf->window->cancelBuffer(dri2_surf->window, dri2_surf->buffer, -1);
+ int fd = -1;
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ int fence_fd = -1;
+ fence_fd = dup(dri2_surf->retrieve_fd);
+ ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
+ dri2_surf->buffer, fence_fd);
if (ret < 0) {
_eglLog(_EGL_WARNING, "ANativeWindow::cancelBuffer failed");
dri2_surf->base.Lost = EGL_TRUE;
@@ -469,7 +478,7 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
if (dri2_surf->buffer)
- droid_window_cancel_buffer(dri2_surf);
+ droid_window_cancel_buffer(disp, dri2_surf);
dri2_surf->window->common.decRef(&dri2_surf->window->common);
}
diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
index 8fcd632..f723a90 100644
--- a/src/mesa/drivers/dri/common/dri_util.h
+++ b/src/mesa/drivers/dri/common/dri_util.h
@@ -218,6 +218,8 @@ struct __DRIcontextRec {
int draw_stamp;
int read_stamp;
} dri2;
+
+ int retrieve_fd;
};
/**
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 62d2fe8..77bf8f6 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 &&
+ brw->driContext->retrieve_fd != -1) {
+ close(brw->driContext->retrieve_fd);
+ brw->driContext->retrieve_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);
+
+ if (out_fence_fd != NULL) {
+ *out_fence_fd = fd;
+ if (brw->driContext)
+ brw->driContext->retrieve_fd = dup(fd);
+ } else {
+ if (brw->driContext)
+ brw->driContext->retrieve_fd = fd;
+ }
}
throttle(brw);
@@ -684,8 +700,11 @@ _intel_batchbuffer_flush_fence(struct brw_context *brw,
{
int ret;
- if (USED_BATCH(brw->batch) == 0)
- return 0;
+ if (USED_BATCH(brw->batch) == 0) {
+ if (out_fence_fd && brw->driContext->retrieve_fd != -1)
+ *out_fence_fd = dup(brw->driContext->retrieve_fd);
+ return 0;
+ }
if (brw->throttle_batch[0] == NULL) {
brw->throttle_batch[0] = brw->batch.bo;
--
1.7.9.5
________________________________________
From: Tomasz Figa [tfiga at chromium.org]
Sent: Friday, July 14, 2017 9:58 AM
To: Antognolli, Rafael
Cc: Chris Wilson; ML mesa-dev; Marathe, Yogesh; Wu, Zhongmin; Liu, Zhiquan; Kondapally, Kalyan; Chad Versace; Eric Engestrom; Emil Velikov; Rob Clark; Kenneth Graunke; Widawsky, Benjamin; Kristian H . Kristensen; Timothy Arceri; Dominik Behr
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)
[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
More information about the mesa-dev
mailing list