Hi Tomasz,
On 19 April 2017 at 08:00, Tomasz Figa <tf...@chromium.org> wrote:
> 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 <tf...@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)
> +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)
> +{
> + 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)) {
Haven't checked the refcounting too close, mostly a gut feeling.
Do we need to refcount twice - once prior to calling
droid_window_dequeue_buffer and a second time within?
Hmm actually it's consistent with the teardown side - once in
droid_destroy_surface itself and again in droid_window_enqueue_buffer.
For the series
Cc: <mesa-sta...@lists.freedesktop.org>
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
