Hi Yogesh, On Wed, Jul 12, 2017 at 2:18 AM, Marathe, Yogesh <yogesh.mara...@intel.com> wrote: >> -----Original Message----- >> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf >> Of Tomasz Figa >> Sent: Tuesday, July 11, 2017 9:59 PM >> To: Marathe, Yogesh <yogesh.mara...@intel.com> >> Cc: Gao, Shuo <shuo....@intel.com>; Liu, Zhiquan <zhiquan....@intel.com>; >> Kondapally, Kalyan <kalyan.kondapa...@intel.com>; Chad Versace >> <chadvers...@chromium.org>; Eric Engestrom <e...@engestrom.ch>; Emil >> Velikov <emil.l.veli...@gmail.com>; Wu, Zhongmin >> <zhongmin...@intel.com>; Kenneth Graunke <kenn...@whitecape.org>; Rob >> Clark <robcl...@freedesktop.org>; Widawsky, Benjamin >> <benjamin.widaw...@intel.com>; ML mesa-dev <mesa- >> d...@lists.freedesktop.org>; 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 >> >> Hi Yogesh, >> >> On Wed, Jul 12, 2017 at 1:09 AM, Marathe, Yogesh >> <yogesh.mara...@intel.com> wrote: >> > Hello Figa, Few caveats on that approach >> >> (I'm Tomasz, by the way) > > My bad Tomasz.
No problem. :) > >> >> 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. >> > >> >> I think you misunderstood my suggestion. I didn't mean dri2_create_sync(), >> but >> rather using the DRI2 fence extension directly, just as dri2_create_sync() >> does. >> You can access dri2_egl_display from Android EGL code and in fact it already >> uses other extensions like this. > > Sorry, I'm still searching around. To try this out, can you please specify, > which > functions did you mean by DRI2 fence extension? An example within EGL code > would help. Please note we need to call these from platform_android.c finally. I meant using dri2_dpy->fence->create_fence_fd() directly, if the available (i.e. dri2_dpy->fence and dri2_dpy->fence->create_fence_fd are non-NULL) or falling back to -1 if not. Best regards, Tomasz > >> >> Best regards, >> Tomasz >> >> >> > >> >> > [1] >> >> > https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/drivers/dri2/eg >> >> > l_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 > _______________________________________________ > 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
