[Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS

Tue Jul 11 16:29:20 UTC 2017

Hi Yogesh,

On Wed, Jul 12, 2017 at 1:09 AM, Marathe, Yogesh
<yogesh.marathe at intel.com> wrote:
> Hello Figa, Few caveats on that approach

(I'm Tomasz, by the way)

>
>> -----Original Message-----
>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On Behalf
>> Of Tomasz Figa
>> Sent: Tuesday, July 11, 2017 8:58 PM
>> To: Wu, Zhongmin <zhongmin.wu at intel.com>
>> Cc: Gao, Shuo <shuo.gao at intel.com>; Liu, Zhiquan <zhiquan.liu at intel.com>;
>> ML mesa-dev <mesa-dev at lists.freedesktop.org>; Emil Velikov
>> <emil.l.velikov at gmail.com>; Chad Versace <chadversary at chromium.org>; Eric
>> Engestrom <eric at engestrom.ch>; Marathe, Yogesh
>> <yogesh.marathe at intel.com>; Rob Clark <robclark at freedesktop.org>; Kenneth
>> Graunke <kenneth at whitecape.org>; Widawsky, Benjamin
>> <benjamin.widawsky at intel.com>; Kondapally, Kalyan
>> <kalyan.kondapally at intel.com>; Kristian H . Kristensen
>> <hoegsberg at chromium.org>; Timothy Arceri <tarceri at 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.weng at intel.com (bounces)
>> +"Gao, Shuo" <shuo.gao at intel.com> (forgot to add originally, sorry)
>>
>> On Wed, Jul 12, 2017 at 12:23 AM, Tomasz Figa <tfiga at chromium.org> wrote:
>> > Hi Zhongmin,
>> >
>> > On Tue, Jul 11, 2017 at 11:02 AM, Wu, Zhongmin <zhongmin.wu at 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.

Best regards,
Tomasz

>> >
>> > [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.velikov at gmail.com>; Marathe, Yogesh
>> >> <yogesh.marathe at intel.com>
>> >> Cc: Widawsky, Benjamin <benjamin.widawsky at intel.com>; Liu, Zhiquan
>> >> <zhiquan.liu at intel.com>; Eric Engestrom <eric at engestrom.ch>; Rob
>> >> Clark <robclark at freedesktop.org>; Tomasz Figa <tfiga at chromium.org>;
>> >> Kenneth Graunke <kenneth at whitecape.org>; Kondapally, Kalyan
>> >> <kalyan.kondapally at intel.com>; ML mesa-dev
>> >> <mesa-dev at lists.freedesktop.org>; Timothy Arceri
>> >> <tarceri at itsqueeze.com>; Chuanbo Weng <chuanbo.weng at 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.velikov at gmail.com]
>> >> Sent: Tuesday, July 11, 2017 1:18
>> >> To: Marathe, Yogesh <yogesh.marathe at intel.com>
>> >> Cc: Wu, Zhongmin <zhongmin.wu at intel.com>; Widawsky, Benjamin
>> >> <benjamin.widawsky at intel.com>; Liu, Zhiquan <zhiquan.liu at intel.com>;
>> >> Eric Engestrom <eric at engestrom.ch>; Rob Clark
>> >> <robclark at freedesktop.org>; Tomasz Figa <tfiga at chromium.org>; Kenneth
>> >> Graunke <kenneth at whitecape.org>; Kondapally, Kalyan
>> >> <kalyan.kondapally at intel.com>; ML mesa-dev
>> >> <mesa-dev at lists.freedesktop.org>; Timothy Arceri
>> >> <tarceri at itsqueeze.com>; Chuanbo Weng <chuanbo.weng at 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.marathe at intel.com>
>> wrote:
>> >>> Hello Emil, My two cents since I too spent some time on this.
>> >>>
>> >>>> -----Original Message-----
>> >>>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>> >>>> Behalf Of Emil Velikov
>> >>>> Sent: Monday, July 10, 2017 4:41 PM
>> >>>> To: Wu, Zhongmin <zhongmin.wu at intel.com>
>> >>>> Cc: Widawsky, Benjamin <benjamin.widawsky at intel.com>; Liu, Zhiquan
>> >>>> <zhiquan.liu at intel.com>; Eric Engestrom <eric at engestrom.ch>; Rob
>> >>>> Clark <robclark at freedesktop.org>; Tomasz Figa <tfiga at chromium.org>;
>> >>>> Kenneth Graunke <kenneth at whitecape.org>; Kondapally, Kalyan
>> >>>> <kalyan.kondapally at intel.com>; ML mesa-dev <mesa-
>> >>>> dev at lists.freedesktop.org>; Timothy Arceri <tarceri at itsqueeze.com>;
>> >>>> Chuanbo Weng <chuanbo.weng at 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.wu at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev