2017-04-20 4:18 GMT+02:00 Tomasz Figa <[email protected]>: > On Wed, Apr 19, 2017 at 11:51 PM, Emil Velikov <[email protected]> > wrote: > > Hi Tomasz, > > > > On 19 April 2017 at 08:00, Tomasz Figa <[email protected]> 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 <[email protected]> > >> --- > >> 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. > > These are different refcounts, one for the window and one the buffer. > However according to the ANativeWindow spec, it might not be necessary > to increment the refcount if the driver references the buffer only > between dequeue and queue. Still, I'd say it's something for a > separate cleanup. > > > > > For the series > > Cc: <[email protected]> > > Reviewed-by: Emil Velikov <[email protected]> > > Thanks! > > I'd also like to hear from Mauro if this version, combined with the > patch to use cancelBuffer instead of queueBuffer for destroy surface, > by any chance helps with his wallpaper issue. > > Best regards, > Tomasz >
Hi Tomasz, I will check Black Wallpaper negative test case this evening or the next, I'll be back with results in a couple of days top. If you/Tapani/others have experience with Android-CTS 7 or other means to launch CTS/piglit test sessions on nougat, could you please share with me? Any attempt to launch Android CTS 7 has failed for me. In this moment I'm only able to launch Android CTS 6 on marshmallow-x86, and results diff is not as easy as for piglit on linux. I wish some GPU compatibility test suite had been integrated in Android Studio/GUI Mauro
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
