[Mesa-dev] egl/android: fence_fd being forced to -1

[Marathe, Yogesh]

> -----Original Message-----
> On Thu 15 Jun 2017, Eric Engestrom wrote:
> > On Thursday, 2017-06-15 16:17:20 -0400, Rob Clark wrote:
> > > On Thu, Jun 15, 2017 at 1:17 PM, Tapani Pälli <tapani.palli at intel.com>
> wrote:
> > > > On 06/15/2017 07:57 PM, Rob Clark wrote:
> > > >
> > > > On Thu, Jun 15, 2017 at 12:04 PM, Tapani Pälli
> > > > <tapani.palli at intel.com>
> > > > wrote:
> > > >
> > > > On 06/15/2017 06:52 PM, Rob Clark wrote:
> > > >
> > > > On Thu, Jun 15, 2017 at 9:59 AM, Eric Engestrom
> > > > <eric.engestrom at imgtec.com> wrote:
> > > >
> > > > On Thursday, 2017-06-15 13:27:06 +0000, Marathe, Yogesh wrote:
> > > >
> > > > Hello,
> > > >
> > > > I'm tyring to run flatland native app on android. It apparantly
> > > > fails because of a fence issue.
> > > > while debuging further it is observed that
> > > > droid_window_enqueue_buffer() is forcing fence_fd =-1.
> > > >
> > > > Yogesh, can you describe a bit more what "fails" means?  What
> > > > sequence of gl calls, for example, is it making?
> > > >
> > > >
> > > > The problem described shortly: Flatland uses timestamps from Fence
> > > > objects for calculating time (using getSignalTime API) and in case
> > > > of having -1 from producer (Mesa) we will end up having same
> > > > timestamp for startFence and endFence (since both are
> > > > Fence::NO_FENCE) and thus flatland will keep running forever as it
> > > > thinks no time has been passed between 2 fences. It is stuck in a
> > > > loop where it tries to find how many frames are required so that driver will
> spend certain amount of time doing it.
> > > >
> > > > hmm, any idea what the getSignalTime() API sits on top of?  I
> > > > don't think we have such a capability with fence fd's..
> > > >
> > > >
> > > > I don't know much of libsync but it uses libsync functions 'sync_fence_info'
> > > > and 'sync_pt_info' to retrieve data from fence fd.
> > > >
> > >
> > > oh, yeah, upstream does look like it supports the SYNC_IOC_FILE_INFO
> > > ioctl.  So I guess someone did actually think this through ;-)
> >
> > If memory serves, Gustavo Padovan did most of that work :)
> 
> Hi, I wrote the comment :) The comment is ancient, and predates the existence
> of sync_file in the kernel and Mesa.

That comment seems to be coming from window.h :), thanks for sync_file input.

> 
> From the framework's perspective, at least from the comments in
> aosp://system/core/include/system/window.h [1], it's legal to call
> ANativeWindow::queueBuffer with fenceFd=-1 if implicit sync is enabled.
> And, as far as I know, implicity sync is still enabled on Intel unless you set a flag
> in execbuffer.
> 
> Yogesh and Tapani, are you disabling explicit sync for any execbuffer ioctl's on
> android-ia? If so, please point me to some code.  If not, then this definitely feels
> like a NOTOURBUG.

I think you meant implicit sync in the question, no we are not disabling it.

As I look at the code further, it appears that the native application depends upon
GLConsumer client's fence. This fence is expected to be coming from buffer producer
(ANativeWindow/Surface in this case) through a BufferQueue, buffers in this 
buffer queue have fence fd forced to -1, before they were enqueued. If producer finds
fence as -1 during enqueue it just sets mFenceFd = Fence::NO_FENCE. 
Isn't this a problem when buffer is acquired by client which depends upon fence? 

> 
> [1]: https://android.googlesource.com/platform/system/core/+/android-
> 7.1.1_r43/include/system/window.h#572
> 
> 
> 
>