[Mesa-dev] [PATCH 3/3] egl/android: Dequeue buffers inside EGL calls (v2)
Mauro Rossi
issor.oruam at gmail.com
Sat Apr 22 14:53:21 UTC 2017
Hi Tomasz,
last minute doubt
2017-04-19 9:00 GMT+02:00 Tomasz Figa <tfiga at chromium.org>:
> Android buffer queues can be abandoned, which results in failing to
> dequeue next buffer. Currently this would fail somewhere deep within
> the DRI stack calling loader's getBuffers*(), without any error
> reporting to the client app. However Android framework code relies on
> proper signaling of this event, so we move buffer dequeue to
> createWindowSurface() and swapBuffers() call, which can generate proper
> EGL errors. To keep the performance benefits of delayed buffer handling,
> if any, fence wait and DRI image creation is kept delayed until
> getBuffers*() is called by the DRI driver.
>
> v2:
> - add missing fence wait in DRI loader case,
> - split simple code moving to a separate patch (Emil),
> - fix previous rebase error.
>
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 1 +
> src/egl/drivers/dri2/platform_android.c | 94
> +++++++++++++++++++--------------
> 2 files changed, 54 insertions(+), 41 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_
> dri2.h
> index f16663712d..92b234d622 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -288,6 +288,7 @@ struct dri2_egl_surface
> #ifdef HAVE_ANDROID_PLATFORM
> struct ANativeWindow *window;
> struct ANativeWindowBuffer *buffer;
> + int acquire_fence_fd;
> __DRIimage *dri_image_back;
> __DRIimage *dri_image_front;
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index 9a84a4c43d..0a75d790c5 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -189,15 +189,9 @@ droid_free_local_buffers(struct dri2_egl_surface
> *dri2_surf)
> }
> }
>
> -static EGLBoolean
> -droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
>
Here droid_window_dequeue_buffer() had one argument...TO BE CONTINUED
> +static void
> +wait_and_close_acquire_fence(struct dri2_egl_surface *dri2_surf)
> {
> - int fence_fd;
> -
> - if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
> &dri2_surf->buffer,
> - &fence_fd))
> - return EGL_FALSE;
> -
> /* If access to the buffer is controlled by a sync fence, then block
> on the
> * fence.
> *
> @@ -215,15 +209,25 @@ droid_window_dequeue_buffer(struct dri2_egl_surface
> *dri2_surf)
> * any value except -1) then the caller is responsible for closing
> the
> * file descriptor.
> */
> - if (fence_fd >= 0) {
> + if (dri2_surf->acquire_fence_fd >= 0) {
> /* From the SYNC_IOC_WAIT documentation in <linux/sync.h>:
> *
> * Waits indefinitely if timeout < 0.
> */
> int timeout = -1;
> - sync_wait(fence_fd, timeout);
> - close(fence_fd);
> + sync_wait(dri2_surf->acquire_fence_fd, timeout);
> + close(dri2_surf->acquire_fence_fd);
> + dri2_surf->acquire_fence_fd = -1;
> }
> +}
> +
> +static EGLBoolean
> +droid_window_dequeue_buffer(_EGLDisplay *disp,
> + struct dri2_egl_surface *dri2_surf)
>
...and here has two arguments, _EGLDisplay *disp was added
Is it necessary or wanted for future proof design or upcoming changes?
I'm asking because we have some swrast patches in android-x86 branches,
with swrastUpdateBuffer/ which I'm merging in android-x86 mesa for testing
purposes,
but its seams that _EGLDisplay *disp propagates in several unexpected
places,
and since I'm not sure if this is a bad sign, I wanted to check with you.
In case new argument is not strictly necessary,
would you evaluate to post-pone that change to when it would be necessary
and keep one argument for now?
Thanks
PS: To be completely honest I'm not even sure our swrast implementation
still requires swrastUpdateBuffers,
but there are past swastPutImage(), swarstPutImage2() and others would
require new argument,
but if new argument is needed, nevermind
> +{
> + if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
> &dri2_surf->buffer,
> + &dri2_surf->acquire_fence_fd))
> + return EGL_FALSE;
>
> dri2_surf->buffer->common.incRef(&dri2_surf->buffer->common);
>
> @@ -254,6 +258,14 @@ droid_window_dequeue_buffer(struct dri2_egl_surface
> *dri2_surf)
> dri2_surf->back = &dri2_surf->color_buffers[0];
> }
>
> + /* free outdated buffers and update the surface size */
> + if (dri2_surf->base.Width != dri2_surf->buffer->width ||
> + dri2_surf->base.Height != dri2_surf->buffer->height) {
> + droid_free_local_buffers(dri2_surf);
> + dri2_surf->base.Width = dri2_surf->buffer->width;
> + dri2_surf->base.Height = dri2_surf->buffer->height;
> + }
> +
> return EGL_TRUE;
> }
>
> @@ -263,6 +275,9 @@ droid_window_enqueue_buffer(_EGLDisplay *disp,
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>
> + /* In case we haven't done any rendering. */
> + wait_and_close_acquire_fence(dri2_surf);
> +
> /* To avoid blocking other EGL calls, release the display mutex before
> * we enter droid_window_enqueue_buffer() and re-acquire the mutex upon
> * return.
> @@ -319,6 +334,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
> _eglError(EGL_BAD_ALLOC, "droid_create_surface");
> return NULL;
> }
> + dri2_surf->acquire_fence_fd = -1;
>
> if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
> goto cleanup_surface;
> @@ -360,10 +376,18 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
> if (window) {
> window->common.incRef(&window->common);
> dri2_surf->window = window;
> + if (!droid_window_dequeue_buffer(disp, dri2_surf)) {
> + _eglError(EGL_BAD_SURFACE, "Could not dequeue buffer from native
> window");
> + goto cleanup_window:
> + }
> }
>
> return &dri2_surf->base;
>
> +cleanup_window:
> + window->common.decRef(&window->common);
> + (*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);
> +
> cleanup_surface:
> free(dri2_surf);
>
> @@ -422,29 +446,6 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> }
>
> static int
> -update_buffers(struct dri2_egl_surface *dri2_surf)
> -{
> - if (dri2_surf->base.Type != EGL_WINDOW_BIT)
> - return 0;
> -
> - /* try to dequeue the next back buffer */
> - if (!dri2_surf->buffer && !droid_window_dequeue_buffer(dri2_surf)) {
> - _eglLog(_EGL_WARNING, "Could not dequeue buffer from native
> window");
> - return -1;
> - }
> -
> - /* free outdated buffers and update the surface size */
> - if (dri2_surf->base.Width != dri2_surf->buffer->width ||
> - dri2_surf->base.Height != dri2_surf->buffer->height) {
> - droid_free_local_buffers(dri2_surf);
> - dri2_surf->base.Width = dri2_surf->buffer->width;
> - dri2_surf->base.Height = dri2_surf->buffer->height;
> - }
> -
> - return 0;
> -}
> -
> -static int
> get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
> {
> struct dri2_egl_display *dri2_dpy =
> @@ -494,6 +495,10 @@ get_back_bo(struct dri2_egl_surface *dri2_surf,
> unsigned int format)
> return -1;
> }
>
> + /* Android might have given us an acquire fence to wait for. If so,
> + * we need to wait for it and close the descriptor after that. */
> + wait_and_close_acquire_fence(dri2_surf);
> +
> fd = get_native_buffer_fd(dri2_surf->buffer);
> if (fd < 0) {
> _eglLog(_EGL_WARNING, "Could not get native buffer FD");
> @@ -564,9 +569,6 @@ droid_image_get_buffers(__DRIdrawable *driDrawable,
> images->front = NULL;
> images->back = NULL;
>
> - if (update_buffers(dri2_surf) < 0)
> - return 0;
> -
> if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> if (get_front_bo(dri2_surf, format) < 0)
> return 0;
> @@ -596,7 +598,7 @@ droid_query_buffer_age(_EGLDriver *drv,
> {
> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surface);
>
> - if (update_buffers(dri2_surf) < 0) {
> + if (!dri2_surf->buffer) {
> _eglError(EGL_BAD_ALLOC, "droid_query_buffer_age");
> return 0;
> }
> @@ -634,6 +636,12 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *draw)
>
> dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
>
> + /* try to dequeue the next back buffer */
> + if (!droid_window_dequeue_buffer(disp, dri2_surf)) {
> + _eglError(EGL_BAD_SURFACE, "Could not dequeue buffer from native
> window");
> + return EGL_FALSE;
> + }
> +
> return EGL_TRUE;
> }
>
> @@ -905,6 +913,13 @@ droid_get_buffers_parse_attachments(struct
> dri2_egl_surface *dri2_surf,
> switch (attachments[i]) {
> case __DRI_BUFFER_BACK_LEFT:
> if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> + if (!dri2_surf->buffer)
> + continue;
> +
> + /* Android might have given us an acquire fence to wait for.
> If so,
> + * we need to wait for it and close the descriptor after
> that. */
> + wait_and_close_acquire_fence(dri2_surf);
> +
> buf->attachment = attachments[i];
> buf->name = get_native_buffer_name(dri2_surf->buffer);
> buf->cpp = get_format_bpp(dri2_surf->buffer->format);
> @@ -952,9 +967,6 @@ droid_get_buffers_with_format(__DRIdrawable *
> driDrawable,
> {
> struct dri2_egl_surface *dri2_surf = loaderPrivate;
>
> - if (update_buffers(dri2_surf) < 0)
> - return NULL;
> -
> dri2_surf->buffer_count =
> droid_get_buffers_parse_attachments(dri2_surf, attachments, count);
>
> --
> 2.12.2.816.g2cccc81164-goog
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170422/ac880f5c/attachment-0001.html>
More information about the mesa-dev
mailing list