[Intel-gfx] [PATCH 2/4] dma-buf: Add ioctls to allow userspace to flush

2015-08-12 Thread Tiago Vignatti
From: Daniel Vetter 

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)
 - munamp 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.

Cc: Sumit Semwal 
Signed-off-by: Daniel Vetter 
Signed-off-by: Tiago Vignatti 
---
 Documentation/dma-buf-sharing.txt | 12 ++
 drivers/dma-buf/dma-buf.c | 50 +++
 include/uapi/linux/dma-buf.h  | 43 +
 3 files changed, 105 insertions(+)
 create mode 100644 include/uapi/linux/dma-buf.h

diff --git a/Documentation/dma-buf-sharing.txt 
b/Documentation/dma-buf-sharing.txt
index 480c8de..2d8ee3b 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -355,6 +355,18 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
 
No special interfaces, userspace simply calls mmap on the dma-buf fd.
 
+   Also, 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)
+ - munamp once you don't need the buffer any more
+
 2. Supporting existing mmap interfaces in importers
 
Similar to the motivation for kernel cpu access it is again important that
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 155c146..e628415 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -34,6 +34,8 @@
 #include 
 #include 
 
+#include 
+
 static inline int is_dma_buf_file(struct file *);
 
 struct dma_buf_list {
@@ -251,11 +253,59 @@ 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;
+
+   /* check for overflowing the buffer's size */
+   if (sync.start > dmabuf->size ||
+   sync.length > dmabuf->size - sync.start)
+   return -EINVAL;
+
+   if (sync.flags & DMA_BUF_SYNC_END)
+   dma_buf_end_cpu_access(dmabuf, sync.start,
+  sync.length, direction);
+   else
+   dma_buf_begin_cpu_access(dmabuf, sync.start,
+sync.length, 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 000..902c9f6
--- /dev/null
+++ b/include/uapi/linux

Re: [Intel-gfx] [PATCH 2/4] dma-buf: Add ioctls to allow userspace to flush

2015-08-12 Thread Sumit Semwal
Hi Tiago,

Thanks for the patch!

On 12 August 2015 at 02:29, Tiago Vignatti  wrote:
> From: Daniel Vetter 
>
> FIXME: Update kerneldoc for begin/end to make it clear that those are
> for mmap too.

I think if we're going to add this patch upstream, atleast the FIXMEs
should be fixed.
>
> Open: Do we need a special indication that the begin/end is from
> userspace mmap and not from kernel mmap?
>
> There's also the question already about kernel internal users - vmap
> and kmap interfaces are much different ... We might need to add a
> mapping enum to the begin/end dma-buf functions.

Also, this looks like another thing that could be added right now
while we're adding this change.
>
> v2 (Tiago): Fix header file type names (u64 -> __u64)
>
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Tiago Vignatti 
> ---
>  drivers/dma-buf/dma-buf.c| 47 
> 
>  include/uapi/linux/dma-buf.h | 39 
>  2 files changed, 86 insertions(+)
>  create mode 100644 include/uapi/linux/dma-buf.h
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 155c146..4820d61 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -34,6 +34,8 @@
>  #include 
>  #include 
>
> +#include 
> +
>  static inline int is_dma_buf_file(struct file *);
>
>  struct dma_buf_list {
> @@ -251,11 +253,56 @@ 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;
> +
> +   /* FIXME: Check for overflows in start/length. */
> +
Even this for fixing in this patch itself.

> +   if (sync.flags & DMA_BUF_SYNC_END)
> +   dma_buf_end_cpu_access(dmabuf, sync.start,
> +  sync.length, direction);
> +   else
> +   dma_buf_begin_cpu_access(dmabuf, sync.start,
> +sync.length, 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 000..e5327df
> --- /dev/null
> +++ b/include/uapi/linux/dma-buf.h
> @@ -0,0 +1,39 @@
> +/*
> + * 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 .
> + */
> +
> +#ifndef _DMA_BUF_UAPI_H_
> +#define _DMA_BUF_UAPI_H_
> +
> +struct dma_buf_sync {
> +   __u64 flags;
> +   __u64 start;
> +   __u64 length;
> +};
> +
> +#define DMA_BUF_SYNC_READ  (1 << 0)
> +#define DMA_BUF_SYNC_WRITE (2 << 0)
> +#define DMA_BUF_SYNC_RW(3 << 0)
> +#define DMA_BUF_SYNC_START (0 << 2)
> +#define DMA_BUF_SYNC_END   (1 << 2)
> +#define DMA_BUF_SYNC_VALID_FLAGS_MASK \
> +   (DMA_BUF_SYNC_RW | DMA_BUF_SYNC_END)
> +
> +#define DMA_BUF_BASE   'b'
> +#define DMA_BUF_IOCTL_SYNC _IOWR(DMA_BUF_BASE, 0, struct dma_buf_sync)
> +
> +#endif
> --
> 2.1.0
>
> _

[Intel-gfx] [PATCH 2/4] dma-buf: Add ioctls to allow userspace to flush

2015-08-11 Thread Tiago Vignatti
From: Daniel Vetter 

FIXME: Update kerneldoc for begin/end to make it clear that those are
for mmap too.

Open: Do we need a special indication that the begin/end is from
userspace mmap and not from kernel mmap?

There's also the question already about kernel internal users - vmap
and kmap interfaces are much different ... We might need to add a
mapping enum to the begin/end dma-buf functions.

v2 (Tiago): Fix header file type names (u64 -> __u64)

Signed-off-by: Daniel Vetter 
Signed-off-by: Tiago Vignatti 
---
 drivers/dma-buf/dma-buf.c| 47 
 include/uapi/linux/dma-buf.h | 39 
 2 files changed, 86 insertions(+)
 create mode 100644 include/uapi/linux/dma-buf.h

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 155c146..4820d61 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -34,6 +34,8 @@
 #include 
 #include 
 
+#include 
+
 static inline int is_dma_buf_file(struct file *);
 
 struct dma_buf_list {
@@ -251,11 +253,56 @@ 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;
+
+   /* FIXME: Check for overflows in start/length. */
+
+   if (sync.flags & DMA_BUF_SYNC_END)
+   dma_buf_end_cpu_access(dmabuf, sync.start,
+  sync.length, direction);
+   else
+   dma_buf_begin_cpu_access(dmabuf, sync.start,
+sync.length, 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 000..e5327df
--- /dev/null
+++ b/include/uapi/linux/dma-buf.h
@@ -0,0 +1,39 @@
+/*
+ * 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 .
+ */
+
+#ifndef _DMA_BUF_UAPI_H_
+#define _DMA_BUF_UAPI_H_
+
+struct dma_buf_sync {
+   __u64 flags;
+   __u64 start;
+   __u64 length;
+};
+
+#define DMA_BUF_SYNC_READ  (1 << 0)
+#define DMA_BUF_SYNC_WRITE (2 << 0)
+#define DMA_BUF_SYNC_RW(3 << 0)
+#define DMA_BUF_SYNC_START (0 << 2)
+#define DMA_BUF_SYNC_END   (1 << 2)
+#define DMA_BUF_SYNC_VALID_FLAGS_MASK \
+   (DMA_BUF_SYNC_RW | DMA_BUF_SYNC_END)
+
+#define DMA_BUF_BASE   'b'
+#define DMA_BUF_IOCTL_SYNC _IOWR(DMA_BUF_BASE, 0, struct dma_buf_sync)
+
+#endif
-- 
2.1.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx