<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<body>
<div dir="auto">
<div dir="auto"><span style="font-size: 12pt;">On June 10, 2021 16:09:49 Jason Ekstrand <jason@jlekstrand.net> wrote:</span></div><div id="aqm-original" style="color: black;">
<div><br></div>
<blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;">
<div dir="auto">Modern userspace APIs like Vulkan are built on an explicit</div>
<div dir="auto">synchronization model. This doesn't always play nicely with the</div>
<div dir="auto">implicit synchronization used in the kernel and assumed by X11 and</div>
<div dir="auto">Wayland. The client -> compositor half of the synchronization isn't too</div>
<div dir="auto">bad, at least on intel, because we can control whether or not i915</div>
<div dir="auto">synchronizes on the buffer and whether or not it's considered written.</div>
<div dir="auto"><br></div>
<div dir="auto">The harder part is the compositor -> client synchronization when we get</div>
<div dir="auto">the buffer back from the compositor. We're required to be able to</div>
<div dir="auto">provide the client with a VkSemaphore and VkFence representing the point</div>
<div dir="auto">in time where the window system (compositor and/or display) finished</div>
<div dir="auto">using the buffer. With current APIs, it's very hard to do this in such</div>
<div dir="auto">a way that we don't get confused by the Vulkan driver's access of the</div>
<div dir="auto">buffer. In particular, once we tell the kernel that we're rendering to</div>
<div dir="auto">the buffer again, any CPU waits on the buffer or GPU dependencies will</div>
<div dir="auto">wait on some of the client rendering and not just the compositor.</div>
<div dir="auto"><br></div>
<div dir="auto">This new IOCTL solves this problem by allowing us to get a snapshot of</div>
<div dir="auto">the implicit synchronization state of a given dma-buf in the form of a</div>
<div dir="auto">sync file. It's effectively the same as a poll() or I915_GEM_WAIT only,</div>
<div dir="auto">instead of CPU waiting directly, it encapsulates the wait operation, at</div>
<div dir="auto">the current moment in time, in a sync_file so we can check/wait on it</div>
<div dir="auto">later. As long as the Vulkan driver does the sync_file export from the</div>
<div dir="auto">dma-buf before we re-introduce it for rendering, it will only contain</div>
<div dir="auto">fences from the compositor or display. This allows to accurately turn</div>
<div dir="auto">it into a VkFence or VkSemaphore without any over-synchronization.</div></blockquote></div><div dir="auto"><br></div><div dir="auto">FYI, the Mesa MR for using this ioctl (the export one) have now been reviewed. As far as I'm concerned, we're ready to land patches 1-4 from this series.</div><div dir="auto"><br></div><div dir="auto">--Jason</div><div dir="auto"><br></div><div dir="auto"><br></div><div id="aqm-original" style="color: black;" dir="auto"><blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;"><div dir="auto"></div>
<div dir="auto">By making this an ioctl on the dma-buf itself, it allows this new</div>
<div dir="auto">functionality to be used in an entirely driver-agnostic way without</div>
<div dir="auto">having access to a DRM fd. This makes it ideal for use in driver-generic</div>
<div dir="auto">code in Mesa or in a client such as a compositor where the DRM fd may be</div>
<div dir="auto">hard to reach.</div>
<div dir="auto"><br></div>
<div dir="auto">v2 (Jason Ekstrand):</div>
<div dir="auto"> - Use a wrapper dma_fence_array of all fences including the new one</div>
<div dir="auto"> when importing an exclusive fence.</div>
<div dir="auto"><br></div>
<div dir="auto">v3 (Jason Ekstrand):</div>
<div dir="auto"> - Lock around setting shared fences as well as exclusive</div>
<div dir="auto"> - Mark SIGNAL_SYNC_FILE as a read-write ioctl.</div>
<div dir="auto"> - Initialize ret to 0 in dma_buf_wait_sync_file</div>
<div dir="auto"><br></div>
<div dir="auto">v4 (Jason Ekstrand):</div>
<div dir="auto"> - Use the new dma_resv_get_singleton helper</div>
<div dir="auto"><br></div>
<div dir="auto">v5 (Jason Ekstrand):</div>
<div dir="auto"> - Rename the IOCTLs to import/export rather than wait/signal</div>
<div dir="auto"> - Drop the WRITE flag and always get/set the exclusive fence</div>
<div dir="auto"><br></div>
<div dir="auto">v6 (Jason Ekstrand):</div>
<div dir="auto"> - Drop the sync_file import as it was all-around sketchy and not nearly</div>
<div dir="auto"> as useful as import.</div>
<div dir="auto"> - Re-introduce READ/WRITE flag support for export</div>
<div dir="auto"> - Rework the commit message</div>
<div dir="auto"><br></div>
<div dir="auto">v7 (Jason Ekstrand):</div>
<div dir="auto"> - Require at least one sync flag</div>
<div dir="auto"> - Fix a refcounting bug: dma_resv_get_excl() doesn't take a reference</div>
<div dir="auto"> - Use _rcu helpers since we're accessing the dma_resv read-only</div>
<div dir="auto"><br></div>
<div dir="auto">v8 (Jason Ekstrand):</div>
<div dir="auto"> - Return -ENOMEM if the sync_file_create fails</div>
<div dir="auto"> - Predicate support on IS_ENABLED(CONFIG_SYNC_FILE)</div>
<div dir="auto"><br></div>
<div dir="auto">v9 (Jason Ekstrand):</div>
<div dir="auto"> - Add documentation for the new ioctl</div>
<div dir="auto"><br></div>
<div dir="auto">v10 (Jason Ekstrand):</div>
<div dir="auto"> - Go back to dma_buf_sync_file as the ioctl struct name</div>
<div dir="auto"><br></div>
<div dir="auto">v11 (Daniel Vetter):</div>
<div dir="auto"> - Go back to dma_buf_export_sync_file as the ioctl struct name</div>
<div dir="auto"> - Better kerneldoc describing what the read/write flags do</div>
<div dir="auto"><br></div>
<div dir="auto">v12 (Christian König):</div>
<div dir="auto"> - Document why we chose to make it an ioctl on dma-buf</div>
<div dir="auto"><br></div>
<div dir="auto">Signed-off-by: Jason Ekstrand <jason@jlekstrand.net></div>
<div dir="auto">Acked-by: Simon Ser <contact@emersion.fr></div>
<div dir="auto">Acked-by: Christian König <christian.koenig@amd.com></div>
<div dir="auto">Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch></div>
<div dir="auto">Cc: Sumit Semwal <sumit.semwal@linaro.org></div>
<div dir="auto">Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com></div>
<div dir="auto">---</div>
<div dir="auto"> drivers/dma-buf/dma-buf.c | 67 ++++++++++++++++++++++++++++++++++++</div>
<div dir="auto"> include/uapi/linux/dma-buf.h | 35 +++++++++++++++++++</div>
<div dir="auto"> 2 files changed, 102 insertions(+)</div>
<div dir="auto"><br></div>
<div dir="auto">diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c</div>
<div dir="auto">index 511fe0d217a08..41b14b53cdda3 100644</div>
<div dir="auto">--- a/drivers/dma-buf/dma-buf.c</div>
<div dir="auto">+++ b/drivers/dma-buf/dma-buf.c</div>
<div dir="auto">@@ -20,6 +20,7 @@</div>
<div dir="auto"> #include <linux/debugfs.h></div>
<div dir="auto"> #include <linux/module.h></div>
<div dir="auto"> #include <linux/seq_file.h></div>
<div dir="auto">+#include <linux/sync_file.h></div>
<div dir="auto"> #include <linux/poll.h></div>
<div dir="auto"> #include <linux/dma-resv.h></div>
<div dir="auto"> #include <linux/mm.h></div>
<div dir="auto">@@ -191,6 +192,9 @@ static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)</div>
<div dir="auto"> * Note that this only signals the completion of the respective fences, i.e. the</div>
<div dir="auto"> * DMA transfers are complete. Cache flushing and any other necessary</div>
<div dir="auto"> * preparations before CPU access can begin still need to happen.</div>
<div dir="auto">+ *</div>
<div dir="auto">+ * As an alternative to poll(), the set of fences on DMA buffer can be</div>
<div dir="auto">+ * exported as a &sync_file using &dma_buf_sync_file_export.</div>
<div dir="auto"> */</div>
<div dir="auto"> </div>
<div dir="auto"> static void dma_buf_poll_cb(struct dma_fence *fence, struct dma_fence_cb *cb)</div>
<div dir="auto">@@ -362,6 +366,64 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)</div>
<div dir="auto"> return ret;</div>
<div dir="auto"> }</div>
<div dir="auto"> </div>
<div dir="auto">+#if IS_ENABLED(CONFIG_SYNC_FILE)</div>
<div dir="auto">+static long dma_buf_export_sync_file(struct dma_buf *dmabuf,</div>
<div dir="auto">+ void __user *user_data)</div>
<div dir="auto">+{</div>
<div dir="auto">+ struct dma_buf_export_sync_file arg;</div>
<div dir="auto">+ struct dma_fence *fence = NULL;</div>
<div dir="auto">+ struct sync_file *sync_file;</div>
<div dir="auto">+ int fd, ret;</div>
<div dir="auto">+</div>
<div dir="auto">+ if (copy_from_user(&arg, user_data, sizeof(arg)))</div>
<div dir="auto">+ return -EFAULT;</div>
<div dir="auto">+</div>
<div dir="auto">+ if (arg.flags & ~DMA_BUF_SYNC_RW)</div>
<div dir="auto">+ return -EINVAL;</div>
<div dir="auto">+</div>
<div dir="auto">+ if ((arg.flags & DMA_BUF_SYNC_RW) == 0)</div>
<div dir="auto">+ return -EINVAL;</div>
<div dir="auto">+</div>
<div dir="auto">+ fd = get_unused_fd_flags(O_CLOEXEC);</div>
<div dir="auto">+ if (fd < 0)</div>
<div dir="auto">+ return fd;</div>
<div dir="auto">+</div>
<div dir="auto">+ if (arg.flags & DMA_BUF_SYNC_WRITE) {</div>
<div dir="auto">+ fence = dma_resv_get_singleton(dmabuf->resv);</div>
<div dir="auto">+ if (IS_ERR(fence)) {</div>
<div dir="auto">+ ret = PTR_ERR(fence);</div>
<div dir="auto">+ goto err_put_fd;</div>
<div dir="auto">+ }</div>
<div dir="auto">+ } else if (arg.flags & DMA_BUF_SYNC_READ) {</div>
<div dir="auto">+ fence = dma_resv_get_excl_unlocked(dmabuf->resv);</div>
<div dir="auto">+ }</div>
<div dir="auto">+</div>
<div dir="auto">+ if (!fence)</div>
<div dir="auto">+ fence = dma_fence_get_stub();</div>
<div dir="auto">+</div>
<div dir="auto">+ sync_file = sync_file_create(fence);</div>
<div dir="auto">+</div>
<div dir="auto">+ dma_fence_put(fence);</div>
<div dir="auto">+</div>
<div dir="auto">+ if (!sync_file) {</div>
<div dir="auto">+ ret = -ENOMEM;</div>
<div dir="auto">+ goto err_put_fd;</div>
<div dir="auto">+ }</div>
<div dir="auto">+</div>
<div dir="auto">+ fd_install(fd, sync_file->file);</div>
<div dir="auto">+</div>
<div dir="auto">+ arg.fd = fd;</div>
<div dir="auto">+ if (copy_to_user(user_data, &arg, sizeof(arg)))</div>
<div dir="auto">+ return -EFAULT;</div>
<div dir="auto">+</div>
<div dir="auto">+ return 0;</div>
<div dir="auto">+</div>
<div dir="auto">+err_put_fd:</div>
<div dir="auto">+ put_unused_fd(fd);</div>
<div dir="auto">+ return ret;</div>
<div dir="auto">+}</div>
<div dir="auto">+#endif</div>
<div dir="auto">+</div>
<div dir="auto"> static long dma_buf_ioctl(struct file *file,</div>
<div dir="auto"> unsigned int cmd, unsigned long arg)</div>
<div dir="auto"> {</div>
<div dir="auto">@@ -405,6 +467,11 @@ static long dma_buf_ioctl(struct file *file,</div>
<div dir="auto"> case DMA_BUF_SET_NAME_B:</div>
<div dir="auto"> return dma_buf_set_name(dmabuf, (const char __user *)arg);</div>
<div dir="auto"> </div>
<div dir="auto">+#if IS_ENABLED(CONFIG_SYNC_FILE)</div>
<div dir="auto">+ case DMA_BUF_IOCTL_EXPORT_SYNC_FILE:</div>
<div dir="auto">+ return dma_buf_export_sync_file(dmabuf, (void __user *)arg);</div>
<div dir="auto">+#endif</div>
<div dir="auto">+</div>
<div dir="auto"> default:</div>
<div dir="auto"> return -ENOTTY;</div>
<div dir="auto"> }</div>
<div dir="auto">diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h</div>
<div dir="auto">index 1c131002fe1ee..82f12a4640403 100644</div>
<div dir="auto">--- a/include/uapi/linux/dma-buf.h</div>
<div dir="auto">+++ b/include/uapi/linux/dma-buf.h</div>
<div dir="auto">@@ -81,6 +81,40 @@ struct dma_buf_sync {</div>
<div dir="auto"> </div>
<div dir="auto"> #define DMA_BUF_NAME_LEN 32</div>
<div dir="auto"> </div>
<div dir="auto">+/**</div>
<div dir="auto">+ * struct dma_buf_export_sync_file - Get a sync_file from a dma-buf</div>
<div dir="auto">+ *</div>
<div dir="auto">+ * Userspace can perform a DMA_BUF_IOCTL_EXPORT_SYNC_FILE to retrieve the</div>
<div dir="auto">+ * current set of fences on a dma-buf file descriptor as a sync_file. CPU</div>
<div dir="auto">+ * waits via poll() or other driver-specific mechanisms typically wait on</div>
<div dir="auto">+ * whatever fences are on the dma-buf at the time the wait begins. This</div>
<div dir="auto">+ * is similar except that it takes a snapshot of the current fences on the</div>
<div dir="auto">+ * dma-buf for waiting later instead of waiting immediately. This is</div>
<div dir="auto">+ * useful for modern graphics APIs such as Vulkan which assume an explicit</div>
<div dir="auto">+ * synchronization model but still need to inter-operate with dma-buf.</div>
<div dir="auto">+ */</div>
<div dir="auto">+struct dma_buf_export_sync_file {</div>
<div dir="auto">+ /**</div>
<div dir="auto">+ * @flags: Read/write flags</div>
<div dir="auto">+ *</div>
<div dir="auto">+ * Must be DMA_BUF_SYNC_READ, DMA_BUF_SYNC_WRITE, or both.</div>
<div dir="auto">+ *</div>
<div dir="auto">+ * If DMA_BUF_SYNC_READ is set and DMA_BUF_SYNC_WRITE is not set,</div>
<div dir="auto">+ * the returned sync file waits on any writers of the dma-buf to</div>
<div dir="auto">+ * complete. Waiting on the returned sync file is equivalent to</div>
<div dir="auto">+ * poll() with POLLIN.</div>
<div dir="auto">+ *</div>
<div dir="auto">+ * If DMA_BUF_SYNC_WRITE is set, the returned sync file waits on</div>
<div dir="auto">+ * any users of the dma-buf (read or write) to complete. Waiting</div>
<div dir="auto">+ * on the returned sync file is equivalent to poll() with POLLOUT.</div>
<div dir="auto">+ * If both DMA_BUF_SYNC_WRITE and DMA_BUF_SYNC_READ are set, this</div>
<div dir="auto">+ * is equivalent to just DMA_BUF_SYNC_WRITE.</div>
<div dir="auto">+ */</div>
<div dir="auto">+ __u32 flags;</div>
<div dir="auto">+ /** @fd: Returned sync file descriptor */</div>
<div dir="auto">+ __s32 fd;</div>
<div dir="auto">+};</div>
<div dir="auto">+</div>
<div dir="auto"> #define DMA_BUF_BASE 'b'</div>
<div dir="auto"> #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)</div>
<div dir="auto"> </div>
<div dir="auto">@@ -90,5 +124,6 @@ struct dma_buf_sync {</div>
<div dir="auto"> #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *)</div>
<div dir="auto"> #define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32)</div>
<div dir="auto"> #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64)</div>
<div dir="auto">+#define DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file)</div>
<div dir="auto"> </div>
<div dir="auto"> #endif</div>
<div dir="auto">-- </div>
<div dir="auto">2.31.1</div>
</blockquote>
</div><div dir="auto"><br></div>
</div></body>
</html>