On Thu, May 27, 2021 at 4:49 AM Christian König christian.koenig@amd.com wrote:

Am 26.05.21 um 19:42 schrieb Jason Ekstrand:

...

On Wed, May 26, 2021 at 6:02 AM Christian König christian.koenig@amd.com wrote:

...

Regarding that, why do we actually use a syncfile and not a drm_syncobj here?

A sync file is a userspace handle to a dma_fence which is exactly what we're grabbing from the dma-buf so it's a better match. A syncobj, on the other hand, is a container for fences. If we really want it in a syncobj (which we may), we can use another ioctl to stuff it in there. The only thing making this work in terms of syncobj would save us is a little ioctl overhead. In my Mesa patches, we do stuff it in a syncobj but, instead of acting on the syncobj directly, we go through vkSemaphoreImportFd() which is way more convenient for generic WSI code.

Yeah, that is a really good argument.

...

It also works nicer because a sync_file is a pretty generic construct but a syncobj is a DRM construct. This lets us do the export in an entirely driver-generic way without even having access to a DRM fd. It all happens as an ioctl on the dma-buf.

Well that's an even better argument and I think the killer argument here.

Cool.

We should probably mention that on the commit message as a single sentence somewhere.

Happy to do so. How does this sound:

By making this an ioctl on the dma-buf itself, it allows this new functionality to be used in an entirely driver-agnostic way without having access to a DRM fd. This makes it ideal for use in driver-generic code in Mesa or in a client such as a compositor where the DRM fd may be hard to reach.

BTW: You replied exclusively to me. Was that intentionally? I don't think so :)

Oops... I've re-added the whole lot in this reply.

--Jason

Regards, Christian.

...

On Wed, May 26, 2021 at 8:23 AM Christian König christian.koenig@amd.com wrote:

...

Am 26.05.21 um 15:12 schrieb Daniel Stone:

...

On Wed, 26 May 2021 at 13:46, Christian König christian.koenig@amd.com wrote:

...

Am 26.05.21 um 13:31 schrieb Daniel Stone:

...

How would we insert a syncobj+val into a resv though? Like, if we pass an unmaterialised syncobj+val here to insert into the resv, then an implicit-only media user (or KMS) goes to sync against the resv, what happens?

Well this is for exporting, not importing. So we don't need to worry about that.

It's just my thinking because the drm_syncobj is the backing object on VkSemaphore implementations these days, isn't it?

Yeah, I can see that to an extent. But then binary vs. timeline syncobjs are very different in use (which is unfortunate tbh), and then we have an asymmetry between syncobj export & sync_file import.

You're right that we do want a syncobj though.

I'm not convinced. Some people seem to think that we have a direct progression of superiority: sync_file < syncobj < timeline syncobj. I don't think this is true. They each serve different purposes. A sync_file is a handle to a single immutable fence operation (a dma_fence*). A syncobj is a container for a fence and is, by definition, mutable (a dma_fence**). A timeline syncobj is a construct that lets us implement VK_KHR_timeline_semaphore with only immutable finite-time (not future) fences.

From a WSI protocol PoV, it's sometimes nicer to pass a container object once and then pass a serial or a simple "I just updated it" signal every time instead of a new FD. It certainly makes all the "who closes this, again?" semantics easier. But we shouldn't think of syncobj as being better than sync_file. With binary syncobj, it really is the difference between passing a dma_fence* vs. a dma_fence**. Sometimes you want one and sometimes you want the other. The real pity, IMO, is that the uAPI is scattered everywhere.

...

...

This is probably not practical due to smashing uAPI to bits, but if we could wind the clock back a couple of years, I suspect the interface we want is that export can either export a sync_file or a binary syncobj, and further that binary syncobjs could transparently act as timeline semaphores by mapping any value (either wait or signal) to the binary signal. In hindsight, we should probably just never have had binary syncobj. Oh well.

Well the later is the case IIRC. Don't ask me for the detail semantics, but in general the drm_syncobj in timeline mode is compatible to the binary mode.

The sync_file is also import/exportable to a certain drm_syncobj timeline point (or as binary signal). So no big deal, we are all compatible here :)

I just thought that it might be more appropriate to return a drm_syncobj directly instead of a sync_file.

Maybe. I'm not convinced that's better. In the current Mesa WSI code, it'd actually be quite a pain.

--Jason