Hi Tomasz, > -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf > Of Tomasz Figa > Sent: Tuesday, August 8, 2017 7:45 AM > To: Marathe, Yogesh <yogesh.mara...@intel.com> > >> > >> Changing the topic, the patch doesn't seem to change the > >> > >> implementation of swapBuffers to stop doing a flush on the > >> > >> buffer, which defeats the purpose of the fence, as the it is > >> > >> likely already signaled at the time it is passed to queueBuffer. > >> > >> Shouldn't > we fix this? > >> > >> > >> > > > >> > > I have been wondering about it all the while, when I had prints > >> > > in > >> > > Fence::getSignalTime() to check finfo->status from consumer side > >> > > during initial revisions, I always found it to be signaled! > >> > > > >> > > Can we really remove that flush in swapBuffers? In that case I > >> > > believe the consumer _must_ wait on fence before really accessing > >> > > it, so that would trigger a change in buffer consumer / application! > >> > > >> > The consumer must _always_ wait on the acquire fence if it's a > >> > valid FD, as this is how the ANativeWindow interface is defined. > >> > You can see Mesa already does it in droid_dequeue_buffer(). If you > >> > find a consumer that is not doing so, it's a bug in the consumer. > >> > There is no compatibility concern here, as it's strictly regulated by > >> > Android > specifications. > >> > >> I checked this, yes, BufferConsumer waits on fence provided its valid. > > > > Hi Tomasz, > > > > Is it ok to move that 'flush' removal change to separate commit? I > > would opt for that. This gflush removal change is going to trigger > > additional tests, while this one fixes the issue for now and has list > > of review comments done. If this is fine, I'll push v6 for this. > > I'm okay with either. >
I found GLConsumer aosp has glFlush() is already, it means we had two flush calls in the path, one in swapBuffers and other in libgui on consumer. I went ahead and removed dri2_flush_drawable_for_swapbuffer. Functionally, things seem to be ok. I assume this will be valid only for android with valid fence changes and not for other platforms. Is this right expectation? Diff below. diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 8bca753..80da021 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -706,7 +706,6 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw) if (dri2_surf->back) dri2_surf->back->age = 1; - dri2_flush_drawable_for_swapbuffers(disp, draw); /* dri2_surf->buffer can be null even when no error has occured. For * example, if the user has called no GL rendering commands since the If this is only change, I don’t think we need separate patch here. Please correct me if I'm wrong. > Best regards, > Tomasz > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev