Hello Emil, My two cents since I too spent some time on this. > -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf > Of Emil Velikov > Sent: Monday, July 10, 2017 4:41 PM > To: Wu, Zhongmin <zhongmin...@intel.com> > Cc: Widawsky, Benjamin <benjamin.widaw...@intel.com>; Liu, Zhiquan > <zhiquan....@intel.com>; Eric Engestrom <e...@engestrom.ch>; Rob Clark > <robcl...@freedesktop.org>; Tomasz Figa <tf...@chromium.org>; Kenneth > Graunke <kenn...@whitecape.org>; Kondapally, Kalyan > <kalyan.kondapa...@intel.com>; ML mesa-dev <mesa- > d...@lists.freedesktop.org>; Timothy Arceri <tarc...@itsqueeze.com>; Chuanbo > Weng <chuanbo.w...@intel.com> > Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: > Queue the buffer with a sync fence for Android OS > > Hi Zhongmin Wu, > > Above all, a bit of a disclaimer: I'm by no means an expert on the topic so > take > the following with a pinch of salt. > > On 10 July 2017 at 03:11, Zhongmin Wu <zhongmin...@intel.com> wrote: > > Before we queued the buffer with a invalid fence (-1), it will make > > some benchmarks failed to test such as flatland. > > > > Now we get the out fence during the flushing buffer and then pass it > > to SurfaceFlinger in eglSwapbuffer function. > > > Having a closer look it seems that the issue can be summarised as follows: > - flatland intercepts/interacts ANativeWindow::{de,}queueBuffer (how about > ANativeWindow::cancelBuffer?) > - the program expects that a sync fd is available for both dequeue and queue > > At the same time: > - the ANativeWindow documentation does _not_ state such requirement > - even if it did, that will be somewhat wrong, since > ANativeWindow::queueBuffer is called by eglSwapBuffers() Where the latter > documentation clearly states - "... performs an implicit flush ... glFlush ... > vgFlush" > > My take is that if flatland/Android framework does want an explicit sync > point it > should insert one with the EGL API. > There could be alternative solutions, but the proposed patch seems wrong > IMHO.
In fact, I could work this around in producer (Surface::queueBuffer) by ignoring the (-1) passed and by creating a sync using egl APIs. I see two problems with that. - Before getting a fd using eglDupNativeFenceFDANDROID(), you need a glFlush(), this costs additional cycles for each queueBuffer transaction on each BufferItem and I believe fd is also signaled due to this. (so I don’t know what we'll get by waiting on that fd on consumer side). - AFAIK, the whole idea of explicit sync revolves around being able to pass fds created by driver between processes and this one breaks that chain. If we work this around in upper layers, explicit sync feature will have to be fixed for every other lib that may use lib mesa underneath. For these reasons, I still believe we should fix it here. Of course, you and Rob have very valid points on cancelBuffer and about not breaking gallium respectively, those need to be taken care of. > > Regards, > Emil > _______________________________________________ > 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
