Re: [PATCH v6 3/5] dma-buf: Add ioctls to allow userspace to flush

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/17/2015 04:19 PM, Alex Deucher wrote:
On Wed, Dec 16, 2015 at 5:25 PM, Tiago Vignatti
<tiago.vignatti@xxxxxxxxx> wrote:
From: Daniel Vetter <daniel.vetter@xxxxxxxx>

The userspace might need some sort of cache coherency management e.g. when CPU
and GPU domains are being accessed through dma-buf at the same time. To
circumvent this problem there are begin/end coherency markers, that forward
directly to existing dma-buf device drivers vfunc hooks. Userspace can make use
of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The sequence would be
used like following:
      - mmap dma-buf fd
      - for each drawing/upload cycle in CPU 1. SYNC_START ioctl, 2. read/write
        to mmap area 3. SYNC_END ioctl. This can be repeated as often as you
        want (with the new data being consumed by the GPU or say scanout device)
      - munmap once you don't need the buffer any more

v2 (Tiago): Fix header file type names (u64 -> __u64)
v3 (Tiago): Add documentation. Use enum dma_buf_sync_flags to the begin/end
dma-buf functions. Check for overflows in start/length.
v4 (Tiago): use 2d regions for sync.
v5 (Tiago): forget about 2d regions (v4); use _IOW in DMA_BUF_IOCTL_SYNC and
remove range information from struct dma_buf_sync.
v6 (Tiago): use __u64 structured padded flags instead enum. Adjust
documentation about the recommendation on using sync ioctls.

Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Signed-off-by: Tiago Vignatti <tiago.vignatti@xxxxxxxxx>
---
  Documentation/dma-buf-sharing.txt | 22 +++++++++++++++++++-
  drivers/dma-buf/dma-buf.c         | 43 +++++++++++++++++++++++++++++++++++++++
  include/uapi/linux/dma-buf.h      | 38 ++++++++++++++++++++++++++++++++++
  3 files changed, 102 insertions(+), 1 deletion(-)
  create mode 100644 include/uapi/linux/dma-buf.h

diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt
index 4f4a84b..2ddd4b2 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -350,7 +350,27 @@ Being able to mmap an export dma-buf buffer object has 2 main use-cases:
     handles, too). So it's beneficial to support this in a similar fashion on
     dma-buf to have a good transition path for existing Android userspace.

-   No special interfaces, userspace simply calls mmap on the dma-buf fd.
+   No special interfaces, userspace simply calls mmap on the dma-buf fd. Very
+   important to note though is that, even if it is not mandatory, the userspace
+   is strongly recommended to always use the cache synchronization ioctl
+   (DMA_BUF_IOCTL_SYNC) discussed next.
+
+   Some systems might need some sort of cache coherency management e.g. when
+   CPU and GPU domains are being accessed through dma-buf at the same time. To
+   circumvent this problem there are begin/end coherency markers, that forward
+   directly to existing dma-buf device drivers vfunc hooks. Userspace can make
+   use of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The sequence
+   would be used like following:
+     - mmap dma-buf fd
+     - for each drawing/upload cycle in CPU 1. SYNC_START ioctl, 2. read/write
+       to mmap area 3. SYNC_END ioctl. This can be repeated as often as you
+       want (with the new data being consumed by the GPU or say scanout device)
+     - munmap once you don't need the buffer any more
+
+    In principle systems with the memory cache shared by the GPU and CPU may
+    not need SYNC_START and SYNC_END but still, userspace is always encouraged
+    to use these ioctls before and after, respectively, when accessing the
+    mapped address.

  2. Supporting existing mmap interfaces in importers

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index b2ac13b..9a298bd 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -34,6 +34,8 @@
  #include <linux/poll.h>
  #include <linux/reservation.h>

+#include <uapi/linux/dma-buf.h>
+
  static inline int is_dma_buf_file(struct file *);

  struct dma_buf_list {
@@ -251,11 +253,52 @@ out:
         return events;
  }

+static long dma_buf_ioctl(struct file *file,
+                         unsigned int cmd, unsigned long arg)
+{
+       struct dma_buf *dmabuf;
+       struct dma_buf_sync sync;
+       enum dma_data_direction direction;
+
+       dmabuf = file->private_data;
+
+       if (!is_dma_buf_file(file))
+               return -EINVAL;
+
+       switch (cmd) {
+       case DMA_BUF_IOCTL_SYNC:
+               if (copy_from_user(&sync, (void __user *) arg, sizeof(sync)))
+                       return -EFAULT;
+
+               if (sync.flags & DMA_BUF_SYNC_RW)
+                       direction = DMA_BIDIRECTIONAL;
+               else if (sync.flags & DMA_BUF_SYNC_READ)
+                       direction = DMA_FROM_DEVICE;
+               else if (sync.flags & DMA_BUF_SYNC_WRITE)
+                       direction = DMA_TO_DEVICE;
+               else
+                       return -EINVAL;
+
+               if (sync.flags & ~DMA_BUF_SYNC_VALID_FLAGS_MASK)
+                       return -EINVAL;
+
+               if (sync.flags & DMA_BUF_SYNC_END)
+                       dma_buf_end_cpu_access(dmabuf, direction);
+               else
+                       dma_buf_begin_cpu_access(dmabuf, direction);
+
+               return 0;
+       default:
+               return -ENOTTY;
+       }
+}
+
  static const struct file_operations dma_buf_fops = {
         .release        = dma_buf_release,
         .mmap           = dma_buf_mmap_internal,
         .llseek         = dma_buf_llseek,
         .poll           = dma_buf_poll,
+       .unlocked_ioctl = dma_buf_ioctl,
  };

  /*
diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
new file mode 100644
index 0000000..bd195f2
--- /dev/null
+++ b/include/uapi/linux/dma-buf.h
@@ -0,0 +1,38 @@
+/*
+ * Framework for buffer objects that can be shared across devices/subsystems.
+ *
+ * Copyright(C) 2015 Intel Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _DMA_BUF_UAPI_H_
+#define _DMA_BUF_UAPI_H_
+
+/* begin/end dma-buf functions used for userspace mmap. */
+struct dma_buf_sync {
+       __u64 flags;
+};
+
+#define DMA_BUF_SYNC_READ      (1 << 0)
+#define DMA_BUF_SYNC_WRITE     (2 << 0)
+#define DMA_BUF_SYNC_RW        (3 << 0)

Maybe make this:

#define DMA_BUF_SYNC_RW        (DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE)

or maybe:

#define DMA_BUF_SYNC_RW_MASK        (3 << 0)

Thanks Alex. I think I like your first option for being a bit more suggestive than the other -- I'm changing for it now.

Tiago

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux