On Mon, Mar 15, 2021 at 10:11 PM Jason Ekstrand <jason@xxxxxxxxxxxxxx> wrote: > > On Wed, Sep 30, 2020 at 4:55 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > On Wed, Sep 30, 2020 at 11:39:06AM +0200, Michel Dänzer wrote: > > > On 2020-03-17 10:21 p.m., Jason Ekstrand wrote: > > > > Explicit synchronization is the future. At least, that seems to be what > > > > most userspace APIs are agreeing on at this point. However, most of our > > > > Linux APIs (both userspace and kernel UAPI) are currently built around > > > > implicit synchronization with dma-buf. While work is ongoing to change > > > > many of the userspace APIs and protocols to an explicit synchronization > > > > model, switching over piecemeal is difficult due to the number of > > > > potential components involved. On the kernel side, many drivers use > > > > dma-buf including GPU (3D/compute), display, v4l, and others. In > > > > userspace, we have X11, several Wayland compositors, 3D drivers, compute > > > > drivers (OpenCL etc.), media encode/decode, and the list goes on. > > > > > > > > This patch provides a path forward by allowing userspace to manually > > > > manage the fences attached to a dma-buf. Alternatively, one can think > > > > of this as making dma-buf's implicit synchronization simply a carrier > > > > for an explicit fence. This is accomplished by adding two IOCTLs to > > > > dma-buf for importing and exporting a sync file to/from the dma-buf. > > > > This way a userspace component which is uses explicit synchronization, > > > > such as a Vulkan driver, can manually set the write fence on a buffer > > > > before handing it off to an implicitly synchronized component such as a > > > > Wayland compositor or video encoder. In this way, each of the different > > > > components can be upgraded to an explicit synchronization model one at a > > > > time as long as the userspace pieces connecting them are aware of it and > > > > import/export fences at the right times. > > > > > > > > There is a potential race condition with this API if userspace is not > > > > careful. A typical use case for implicit synchronization is to wait for > > > > the dma-buf to be ready, use it, and then signal it for some other > > > > component. Because a sync_file cannot be created until it is guaranteed > > > > to complete in finite time, userspace can only signal the dma-buf after > > > > it has already submitted the work which uses it to the kernel and has > > > > received a sync_file back. There is no way to atomically submit a > > > > wait-use-signal operation. This is not, however, really a problem with > > > > this API so much as it is a problem with explicit synchronization > > > > itself. The way this is typically handled is to have very explicit > > > > ownership transfer points in the API or protocol which ensure that only > > > > one component is using it at any given time. Both X11 (via the PRESENT > > > > extension) and Wayland provide such ownership transfer points via > > > > explicit present and idle messages. > > > > > > > > The decision was intentionally made in this patch to make the import and > > > > export operations IOCTLs on the dma-buf itself rather than as a DRM > > > > IOCTL. This makes it the import/export operation universal across all > > > > components which use dma-buf including GPU, display, v4l, and others. > > > > It also means that a userspace component can do the import/export > > > > without access to the DRM fd which may be tricky to get in cases where > > > > the client communicates with DRM via a userspace API such as OpenGL or > > > > Vulkan. At a future date we may choose to add direct import/export APIs > > > > to components such as drm_syncobj to avoid allocating a file descriptor > > > > and going through two ioctls. However, that seems to be something of a > > > > micro-optimization as import/export operations are likely to happen at a > > > > rate of a few per frame of rendered or decoded video. > > > > > > > > v2 (Jason Ekstrand): > > > > - Use a wrapper dma_fence_array of all fences including the new one > > > > when importing an exclusive fence. > > > > > > > > v3 (Jason Ekstrand): > > > > - Lock around setting shared fences as well as exclusive > > > > - Mark SIGNAL_SYNC_FILE as a read-write ioctl. > > > > - Initialize ret to 0 in dma_buf_wait_sync_file > > > > > > > > v4 (Jason Ekstrand): > > > > - Use the new dma_resv_get_singleton helper > > > > > > > > v5 (Jason Ekstrand): > > > > - Rename the IOCTLs to import/export rather than wait/signal > > > > - Drop the WRITE flag and always get/set the exclusive fence > > > > > > > > Signed-off-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx> > > > > > > What's the status of this? DMA_BUF_IOCTL_EXPORT_SYNC_FILE would be useful > > > for Wayland compositors to wait for client buffers to become ready without > > > being prone to getting delayed by later HW access to them, so it would be > > > nice to merge that at least (if DMA_BUF_IOCTL_IMPORT_SYNC_FILE is still > > > controversial). > > > > I think the missing bits are just the usual stuff > > - igt testcases > > - userspace using the new ioctls > > - review of the entire pile > > > > I don't think there's any fundamental objections aside from "no one ever > > pushed this over the finish line". > > I just re-upped the patch series and you should have been on the CC > for the cover letter. The Mesa MR is here: > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 I'm > going to try and knock out an IGT test quick but I don't know how much > is really practical to test in that environment besides a basic fuzz > for "does the IOCTL return a sync file". With vgem you should be able to control the interface fully, since that allows you to control dma_fences we attach to stuff. Or at least it has some of the ingredients, and a bunch of igts test it. E.g. we have one that creates a fence with vgem and then checks (using crc) that your display driver works correctly with the page flip. > I've dropped all the sync_file import stuff in the latest version. It > would have been useful for testing but it's also where all the scary > stuff lives and I'm no longer convinced it solves any real problems > for userspace. Yeah vgem should allow you to get at least some of that sorted. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch