Hello Figa, Few caveats on that approach > -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf > Of Tomasz Figa > Sent: Tuesday, July 11, 2017 8:58 PM > To: Wu, Zhongmin <zhongmin...@intel.com> > Cc: Gao, Shuo <shuo....@intel.com>; Liu, Zhiquan <zhiquan....@intel.com>; > ML mesa-dev <mesa-dev@lists.freedesktop.org>; Emil Velikov > <emil.l.veli...@gmail.com>; Chad Versace <chadvers...@chromium.org>; Eric > Engestrom <e...@engestrom.ch>; Marathe, Yogesh > <yogesh.mara...@intel.com>; Rob Clark <robcl...@freedesktop.org>; Kenneth > Graunke <kenn...@whitecape.org>; Widawsky, Benjamin > <benjamin.widaw...@intel.com>; Kondapally, Kalyan > <kalyan.kondapa...@intel.com>; Kristian H . Kristensen > <hoegsb...@chromium.org>; Timothy Arceri <tarc...@itsqueeze.com> > Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: > Queue the buffer with a sync fence for Android OS > > Now for real, sorry guys... (Seriously gmail why you do this to me.) > > -chuanbo.w...@intel.com (bounces) > +"Gao, Shuo" <shuo....@intel.com> (forgot to add originally, sorry) > > On Wed, Jul 12, 2017 at 12:23 AM, Tomasz Figa <tf...@chromium.org> wrote: > > Hi Zhongmin, > > > > On Tue, Jul 11, 2017 at 11:02 AM, Wu, Zhongmin <zhongmin...@intel.com> > wrote: > >> By the way, > >> > >> For cancelBuffer, sorry I forget such function, thanks for notice. It > >> should > also pass the same fence fd as the queuebuffer. > >> > >> And Yogesh, you mentioned the gallium, is it another platform supported > >> by > mesa ? I am sorry I have no idea about this, could you please help to check > this > ? > >> > >> I think we can co-work with mesa team to work out an acceptable fix which > can meet the requirement of Android without any break on other platforms. > > > > One thing needs clarifying here. Release fences from EGL are _not_ a > > requirement. It is an optional feature. Android compliance suites pass > > fully without Android sync fence support in Mesa at all. > > > > Other than that, it's been taking long enough and I agree that we > > should finally wire both acquire and release fence support in EGL and > > related drivers. Otherwise we can forget about getting good user > > experience on Android. > > > > On a technical side, the EGL change needs to take into account that > > not all drivers support fences and so it needs to have a fallback to > > old behavior for those which don't. > > > > Other than that, correct me if I'm wrong, but could we just use the > > DRI2 fence extension instead of adding some custom callbacks? I can > > see that a normal client request to create a sync fence would end up > > calling dri2_dpy->fence->create_fence_fd() (if it's present) [1]. > > Could we do the same?
May be here we need to look at complete sequence eglCreateSyncKHR -> _eglCreateSync -> dri2_create_sync, as eglCreateSyncKHR seems entry point and its doing attrib/type checks before reaching dri2_create_sync(). Also, dri2_create_sync is static, can't be called directly, there needs to be an entry point / interface. > > > > [1] > > https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/drivers/dri2/egl_d > > ri2.c#n2772 > > > > + Kristian, Chad and Dominik who have been looking into sync fence > > integration with our EGL drivers. > > > > Best regards, > > Tomasz > > > >> > >> -----Original Message----- > >> From: Wu, Zhongmin > >> Sent: Tuesday, July 11, 2017 8:40 > >> To: 'Emil Velikov' <emil.l.veli...@gmail.com>; Marathe, Yogesh > >> <yogesh.mara...@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-dev@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 Emil and Yogesh > >> Thank you for your comments, and thanks Yogesh for giving the > >> detailed explanations > >> > >> > >> And according to the document of Android below > (https://source.android.com/devices/graphics/arch-bq-gralloc): > >> > >> Recent Android devices support the sync framework, which enables the > system to do nifty things when combined with hardware components that can > manipulate graphics data asynchronously. For example, a producer can submit a > series of OpenGL ES drawing commands and then enqueue the output buffer > before rendering completes. The buffer is accompanied by a fence that signals > when the contents are ready. > >> > >> > >> I think the things is very clear, that is if the rendering is completed > >> already > when we call queueBuffer() in mesa ? If not, we should queue the buffer > with a > fence which will be signaled when the buffer is ready. > >> > >> > >> > >> -----Original Message----- > >> From: Emil Velikov [mailto:emil.l.veli...@gmail.com] > >> Sent: Tuesday, July 11, 2017 1:18 > >> To: Marathe, Yogesh <yogesh.mara...@intel.com> > >> Cc: Wu, Zhongmin <zhongmin...@intel.com>; 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-dev@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 > >> > >> On 10 July 2017 at 15:26, Marathe, Yogesh <yogesh.mara...@intel.com> > wrote: > >>> 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. > >>> > >> What I'm saying is - seems like the app/framework does something silly or > >> at > least undocumented. > >> Fixing things in Mesa may be the right thing to do, but without more > information, its everyone's guess who's got it wrong. > >> > >> As Rob asked earlier - can we get an a simple test case or some pseudo code > illustrating the whole thing? > >> > >> Thanks > >> 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