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

[Wu, Zhongmin]

Add Chris Wilson


-----Original Message-----
From: Wu, Zhongmin 
Sent: Thursday, July 13, 2017 16:31 
To: 'Tomasz Figa' <tfiga at chromium.org>; 'Michel Dänzer' <michel at daenzer.net>
Cc: Liu, Zhiquan <zhiquan.liu at intel.com>; Kondapally, Kalyan <kalyan.kondapally at intel.com>; Marathe, Yogesh <yogesh.marathe at intel.com>; 'Chad Versace' <chadversary at chromium.org>; 'Eric Engestrom' <eric at engestrom.ch>; 'Emil Velikov' <emil.l.velikov at gmail.com>; 'Rob Clark' <robclark at freedesktop.org>; 'Kenneth Graunke' <kenneth at whitecape.org>; Widawsky, Benjamin <benjamin.widawsky at intel.com>; 'ML mesa-dev' <mesa-dev at lists.freedesktop.org>; '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

By the way,  we can't use create_fence_fd in cancelBuffer even if we implemented the last fence feature as radeonsi driver , since the context is reset before cancelBuffer, and we cannot depend on the last fence in context object either...

So things may be tricky on Android ....

We will try to modify the previous patch and save the last fence in the drawable object (contained in EGLSurface) rather than context object to support the cancelBuffer...

As for the using of last fence when the batch buffer is empty for create_fence_fd,  I suggest it can be another story and we will try to optimize it in the future...



-----Original Message-----
From: Wu, Zhongmin
Sent: Thursday, July 13, 2017 12:23
To: Tomasz Figa <tfiga at chromium.org>; Michel Dänzer <michel at daenzer.net>
Cc: Liu, Zhiquan <zhiquan.liu at intel.com>; Kondapally, Kalyan <kalyan.kondapally at intel.com>; Marathe, Yogesh <yogesh.marathe at intel.com>; Chad Versace <chadversary at chromium.org>; Eric Engestrom <eric at engestrom.ch>; Emil Velikov <emil.l.velikov at gmail.com>; Rob Clark <robclark at freedesktop.org>; Kenneth Graunke <kenneth at whitecape.org>; Widawsky, Benjamin <benjamin.widawsky at intel.com>; ML mesa-dev <mesa-dev at lists.freedesktop.org>; 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

Hi Tomasz:

That is right,  what we needed is just the "last fence", and then where to record it and how to get it.  

We will check the solutions on radeonsi by Michel, if create_fence_fd can also get the last fence, of course we can use such DRI2 fence extension API in queuebuffer and canclebuffer.

-----Original Message-----
From: Tomasz Figa [mailto:tfiga at chromium.org]
Sent: Thursday, July 13, 2017 12:08
To: Michel Dänzer <michel at daenzer.net>; Wu, Zhongmin <zhongmin.wu at intel.com>
Cc: Liu, Zhiquan <zhiquan.liu at intel.com>; Kondapally, Kalyan <kalyan.kondapally at intel.com>; Marathe, Yogesh <yogesh.marathe at intel.com>; Chad Versace <chadversary at chromium.org>; Eric Engestrom <eric at engestrom.ch>; Emil Velikov <emil.l.velikov at gmail.com>; Rob Clark <robclark at freedesktop.org>; Kenneth Graunke <kenneth at whitecape.org>; Widawsky, Benjamin <benjamin.widawsky at intel.com>; ML mesa-dev <mesa-dev at lists.freedesktop.org>; 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

Hi Zhongmin, Michel,

On Thu, Jul 13, 2017 at 12:21 PM, Michel Dänzer <michel at daenzer.net> wrote:
> On 13/07/17 09:28 AM, Wu, Zhongmin wrote:
>> Hi Tomasz
>>
>> Thanks,  but I am afraid we have to use the last fence from the last buffer flushing , According to my understanding:
>>
>> If the glFlush was called before swapbuffer ,    there would be no new fence after that since the batch buffer may be flushed out, no batch flushing, no fence generated.
>>
>> ====
>> You can check the function "_intel_batchbuffer_flush_fence",  at the very beginning, it will check the content of the buffer, if it is empty, it will return and do nothing.
>> =====
>>
>> So, as the same reason,  it may not work if we use dri2_dpy->fence->create_fence_fd,   the process is below.
>>
>> Swapbuffer (==> flush batch buffer) ==>  create_fence_fd(===> flush 
>> batch buffer and get fd)
>>
>> You can see, the second buffer flushing may get nothing (not to 
>> mention  the first buffer flushing may get nothing either)

First of all, the sequence of calls is like this:

- brw_dri_create_fence_fd()
  - brw_fence_insert_locked()
    - brw_emit_mi_flush() <--- this emits commands to the command buffer...
    - intel_batchbuffer_flush_fence() <--- this should flush the buffer and get a fence

but...

>
> Apologies for jumping into this discussion, and possibly missing some 
> context.
>
> FWIW, it should be possible to re-use the previous fence when there is 
> no new work to flush. See e.g. commits
>
> f1be3d8cdde1 ("radeonsi: don't flush an empty IB if the only thing we
>                need is a fence")
> 800efb0690e9 ("radeonsi: Flush when we're asked to return a fence but
>                don't have one yet")
>
> for how the radeonsi driver handles this.

I agree with Michel that it would make more sense if Intel driver kept the last fence and returned it using when DRI2 fence extension is called.

Best regards,
Tomasz