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

On 12/17/2015 04:19 PM, Alex Deucher wrote:

On Wed, Dec 16, 2015 at 5:25 PM, Tiago Vignatti tiago.vignatti@intel.com wrote:

...

From: Daniel Vetter daniel.vetter@ffwll.ch

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@linaro.org Signed-off-by: Daniel Vetter daniel.vetter@intel.com Signed-off-by: Tiago Vignatti tiago.vignatti@intel.com

---

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