EGL-Registry icon indicating copy to clipboard operation
EGL-Registry copied to clipboard

EGL_ANDROID_native_fence_sync: Clarification of what a "fence command" is

Open ascent12 opened this issue 5 years ago • 9 comments

This arises from a mesa issue found here: https://gitlab.freedesktop.org/mesa/mesa/issues/2080

In the situation where the OpenGL command stream contains

eglCreateSyncKHR(EGL_SYNC_NATIVE_FENCE_ANDROID);
glClear(); // i.e. do some work
eglCreateSyncKHR(EGL_SYNC_NATIVE_FENCE_ANDROID);

and is flushed, when the first native fence is extracted, the radeonsi mesa driver returns a native fence from the previous submission rather than creating a new one. This leads to surprising results when we extract the timestamp from this fence (using Linux-specific APIs).

From the spec:

When a fence sync object is created or when an EGL native fence sync object is created with the EGL_SYNC_NATIVE_FENCE_FD_ANDROID attribute set to EGL_NO_NATIVE_FENCE_FD_ANDROID, eglCreateSyncKHR also inserts a fence command into the command stream of the bound client API's current context (i.e., the context returned by eglGetCurrentContext), and associates it with the newly created sync object.

To me, this would imply that a new native fence is created and is signalled when the GPU reaches that point in the OpenGL command stream, assuming that what what a "fence command" means.

But it was argued that this is not what a fence command is, and it is really just telling the driver to fetch the fence from the last batch of work that was completed: https://gitlab.freedesktop.org/mesa/mesa/issues/2080#note_289275

Is radeonsi's behaviour conformant to this specification and my understanding is wrong? Is the spec not worded precisely enough, and should it be changed to clarify it?

ascent12 avatar Nov 13 '19 12:11 ascent12

Fences signal when all work submitted before them is completed. Their primary use case is synchronization, not timing. And in general for synchronization you want to unblock the dependent work as early as possible.

So that leads to designs where if you already have a synchronization object marking the end of previous work, and you get a request to create a new EGLSync, you just make it refer to that existing synchronization object. When you do this, sometimes you naturally end up with fences that have a signal timestamp from before the EGLSync was created. For synchronization, that's perfectly fine and in fact usually what you want.

critsec avatar Nov 13 '19 17:11 critsec

Right, but the problem is the wording in the spec.

The spec says, as quoted above, that creating a fence will insert a fence command into the command stream. That means that new work is pending, created by creating the fence. The new work (fence command) must then be submitted to the GPU to be executed. The fence signals when this work has been completed.

I really cannot see any other way to interpret the spec wording.

ppaalanen avatar Nov 14 '19 08:11 ppaalanen

What critsec described is reality. Unfortunately you can't infer Android specific behavior from this interpretation. Sorry can't give you a more useful answer. Closing.

stonesthrow avatar Jan 06 '20 18:01 stonesthrow

Why not fix the spec then?

Remove the word "command" from the phrase "fence command", and add a Q&A that explains that fences are not executable commands but something else. Also, remove the wording that says "inserts a command into the command stream" because implementations obviously do not do that, and apparently cannot do that.

The point here is to figure out what the spec means and clarify the situation.

Please, re-open.

ppaalanen avatar Jan 10 '20 08:01 ppaalanen

As it apparently has been established that the current implementations are correct with respect to the spirit of the specification, then the wording of the specification needs to be corrected to not mislead application developers.

ppaalanen avatar Jan 16 '20 08:01 ppaalanen

I agree the language could be a little better. Re-opening and assigning myself, though I'm pretty busy at the moment and might not get to it for a bit.

critsec avatar Jan 16 '20 15:01 critsec

Thank you very much. Looking forward to seeing a new wording while not holding my breath. ;-)

ppaalanen avatar Jan 17 '20 07:01 ppaalanen

Hi, any news about this @critsec? Just got hit by the same misunderstanding.

emersion avatar Feb 17 '21 08:02 emersion

Ping

emersion avatar Sep 06 '21 08:09 emersion