[PATCH v4] mmap on the dma-buf directly

[PATCH v4] mmap on the dma-buf directly

[Tiago Vignatti]

Hi,

The idea is to create a GEM bo in one process and pass the prime handle of the
it to another process, which in turn uses the handle only to map and write.
This could be useful for Chrome OS architecture, where the Web content
("unpriviledged process") maps and CPU-draws a buffer, which was previously
allocated in the GPU process ("priviledged process").

In v2, I've added a patch that Daniel kindly drafted to allow the
unpriviledged process flush through a prime fd. In v3, I've fixed a few
concerns and then added end_cpu_access to i915. In v4, I fixed Sumit Semwal's
concerns about dma-duf documentation and the FIXME missing in that same patch,
and also removed WARN in i915 dma-buf mmap (pointed by Chris). PTAL.

Best Regards,

Tiago


Daniel Thompson (1):
  drm: prime: Honour O_RDWR during prime-handle-to-fd

Daniel Vetter (1):
  dma-buf: Add ioctls to allow userspace to flush

Tiago Vignatti (2):
  drm/i915: Implement end_cpu_access
  drm/i915: Use CPU mapping for userspace dma-buf mmap()

 Documentation/dma-buf-sharing.txt      | 12 ++++++++
 drivers/dma-buf/dma-buf.c              | 50 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_prime.c            | 10 ++-----
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 28 ++++++++++++++++++-
 include/uapi/drm/drm.h                 |  1 +
 include/uapi/linux/dma-buf.h           | 43 +++++++++++++++++++++++++++++
 6 files changed, 136 insertions(+), 8 deletions(-)
 create mode 100644 include/uapi/linux/dma-buf.h

-- 
2.1.0

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

[PATCH 1/4] drm: prime: Honour O_RDWR during prime-handle-to-fd

[Tiago Vignatti]

From: Daniel Thompson <daniel.thompson@linaro.org>

Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
(DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
to mmap() the resulting dma-buf even when this is supported by the
DRM driver.

It is trivial to relax the restriction and permit read/write access.
This is safe because the flags are seldom touched by drm; mostly they
are passed verbatim to dma_buf calls.

v3 (Tiago): removed unused flags variable from drm_prime_handle_to_fd_ioctl.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 drivers/gpu/drm/drm_prime.c | 10 +++-------
 include/uapi/drm/drm.h      |  1 +
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 27aa718..df6cdc7 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -329,7 +329,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
  * drm_gem_prime_export - helper library implementation of the export callback
  * @dev: drm_device to export from
  * @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
  *
  * This is the implementation of the gem_prime_export functions for GEM drivers
  * using the PRIME helpers.
@@ -628,7 +628,6 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
 	struct drm_prime_handle *args = data;
-	uint32_t flags;
 
 	if (!drm_core_check_feature(dev, DRIVER_PRIME))
 		return -EINVAL;
@@ -637,14 +636,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 		return -ENOSYS;
 
 	/* check flags are valid */
-	if (args->flags & ~DRM_CLOEXEC)
+	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
 		return -EINVAL;
 
-	/* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
-	flags = args->flags & DRM_CLOEXEC;
-
 	return dev->driver->prime_handle_to_fd(dev, file_priv,
-			args->handle, flags, &args->fd);
+			args->handle, args->flags, &args->fd);
 }
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..ad8223e 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -668,6 +668,7 @@ struct drm_set_client_cap {
 	__u64 value;
 };
 
+#define DRM_RDWR O_RDWR
 #define DRM_CLOEXEC O_CLOEXEC
 struct drm_prime_handle {
 	__u32 handle;
-- 
2.1.0

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

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

[Tiago Vignatti]

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)
     - 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 <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 | 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 <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,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 0000000..902c9f6
--- /dev/null
+++ b/include/uapi/linux/dma-buf.h
@@ -0,0 +1,43 @@
+/*
+ * 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_
+
+enum dma_buf_sync_flags {
+	DMA_BUF_SYNC_READ = (1 << 0),
+	DMA_BUF_SYNC_WRITE = (2 << 0),
+	DMA_BUF_SYNC_RW = (3 << 0),
+	DMA_BUF_SYNC_START = (0 << 2),
+	DMA_BUF_SYNC_END = (1 << 2),
+
+	DMA_BUF_SYNC_VALID_FLAGS_MASK = DMA_BUF_SYNC_RW |
+		DMA_BUF_SYNC_END
+};
+
+/* begin/end dma-buf functions used for userspace mmap. */
+struct dma_buf_sync {
+	enum dma_buf_sync_flags flags;
+	__u64 start;
+	__u64 length;
+};
+
+#define DMA_BUF_BASE		'b'
+#define DMA_BUF_IOCTL_SYNC	_IOWR(DMA_BUF_BASE, 0, struct dma_buf_sync)
+
+#endif
-- 
2.1.0

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

[PATCH 3/4] drm/i915: Implement end_cpu_access

[Tiago Vignatti]

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index e9c2bfd..8447ba4 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -212,6 +212,15 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
 	return ret;
 }
 
+static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction)
+{
+	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+	bool write = (direction == DMA_BIDIRECTIONAL || direction == DMA_TO_DEVICE);
+
+	if (i915_gem_object_set_to_gtt_domain(obj, write))
+		DRM_ERROR("failed to set bo into the GTT\n");
+}
+
 static const struct dma_buf_ops i915_dmabuf_ops =  {
 	.map_dma_buf = i915_gem_map_dma_buf,
 	.unmap_dma_buf = i915_gem_unmap_dma_buf,
@@ -224,6 +233,7 @@ static const struct dma_buf_ops i915_dmabuf_ops =  {
 	.vmap = i915_gem_dmabuf_vmap,
 	.vunmap = i915_gem_dmabuf_vunmap,
 	.begin_cpu_access = i915_gem_begin_cpu_access,
+	.end_cpu_access = i915_gem_end_cpu_access,
 };
 
 struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
-- 
2.1.0

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

Re: [PATCH 3/4] drm/i915: Implement end_cpu_access

[Chris Wilson]

On Wed, Aug 12, 2015 at 08:29:12PM -0300, Tiago Vignatti wrote:
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e9c2bfd..8447ba4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -212,6 +212,15 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
>  	return ret;
>  }
>  
> +static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction)
> +{
> +	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> +	bool write = (direction == DMA_BIDIRECTIONAL || direction == DMA_TO_DEVICE);
> +
> +	if (i915_gem_object_set_to_gtt_domain(obj, write))
> +		DRM_ERROR("failed to set bo into the GTT\n");

Ok, the error here should indeed be rare (would require that the buffer
become active during the sync period and for the end_cpu_access to be
interrupted). To be completely sure that the error is worth reporting,
we should

mutex_lock(&dev->struct_mutex);
was_interruptible = dev_priv->mm.interruptible;
dev_priv->mm.interruptible = false;

ret = i915_gem_object_set_to_gtt_domain(obj, write);

dev_priv->mm.interruptible = was_interruptible;
mutex_unlock(&dev->struct_mutex);

if (unlikely(ret)) DRM_ERROR("unable to flush buffer following CPU access; rendering may be corrupt\n");
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Use CPU mapping for userspace dma-buf mmap()

[Tiago Vignatti]

Userspace is the one in charge of flush CPU by wrapping mmap with
begin{,end}_cpu_access.

v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return
before transferring ownership when mmap fails.
v3: Fix return values.
v4: !obj->base.filp is user triggerable, so removed the WARN_ON.

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 8447ba4..ecd00d6 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -193,7 +193,23 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf *dma_buf, unsigned long page_n
 
 static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
 {
-	return -EINVAL;
+	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+	int ret;
+
+	if (obj->base.size < vma->vm_end - vma->vm_start)
+		return -EINVAL;
+
+	if (!obj->base.filp)
+		return -ENODEV;
+
+	ret = obj->base.filp->f_op->mmap(obj->base.filp, vma);
+	if (ret)
+		return ret;
+
+	fput(vma->vm_file);
+	vma->vm_file = get_file(obj->base.filp);
+
+	return 0;
 }
 
 static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction)
-- 
2.1.0

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

Re: [PATCH 4/4] drm/i915: Use CPU mapping for userspace dma-buf mmap()

[Chris Wilson]

On Wed, Aug 12, 2015 at 08:29:13PM -0300, Tiago Vignatti wrote:
> Userspace is the one in charge of flush CPU by wrapping mmap with
> begin{,end}_cpu_access.
> 
> v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return
> before transferring ownership when mmap fails.
> v3: Fix return values.
> v4: !obj->base.filp is user triggerable, so removed the WARN_ON.
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>

I have some reservations with CPU mmapping on !llc platforms, somehow we
need to encourage creation of snooped bo (and proper usage thereof).

However, the patches look good.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] prime_mmap: Add new test for calling mmap() on dma-buf fds

[Tiago Vignatti]

From: Rob Bradford <rob@linux.intel.com>

This test has the following subtests:
 - test_correct for correctness of the data
 - test_map_unmap checks for mapping idempotency
 - test_reprime checks for dma-buf creation idempotency
 - test_forked checks for multiprocess access
 - test_refcounting checks for buffer reference counting
 - test_dup chats that dup()ing the fd works
 - test_errors checks the error return values for failures
 - test_aperture_limit tests multiple buffer creation at the gtt aperture
   limit

Signed-off-by: Rob Bradford <rob@linux.intel.com>
Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 tests/Makefile.sources |   1 +
 tests/prime_mmap.c     | 383 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 384 insertions(+)
 create mode 100644 tests/prime_mmap.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c94714b..5b2072e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs_M = \
 	pm_rps \
 	pm_rc6_residency \
 	pm_sseu \
+	prime_mmap \
 	prime_self_import \
 	template \
 	$(NULL)
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
new file mode 100644
index 0000000..4dc2055
--- /dev/null
+++ b/tests/prime_mmap.c
@@ -0,0 +1,383 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Rob Bradford <rob at linux.intel.com>
+ *
+ */
+
+/*
+ * Testcase: Check whether mmap()ing dma-buf works
+ */
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <pthread.h>
+
+#include "drm.h"
+#include "i915_drm.h"
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "ioctl_wrappers.h"
+
+#define BO_SIZE (16*1024)
+
+static int fd;
+
+char pattern[] = {0xff, 0x00, 0x00, 0x00,
+	0x00, 0xff, 0x00, 0x00,
+	0x00, 0x00, 0xff, 0x00,
+	0x00, 0x00, 0x00, 0xff};
+
+static void
+fill_bo(uint32_t handle, size_t size)
+{
+	off_t i;
+	for (i = 0; i < size; i+=sizeof(pattern))
+	{
+		gem_write(fd, handle, i, pattern, sizeof(pattern));
+	}
+}
+
+static int
+pattern_check(char *ptr, size_t size)
+{
+	off_t i;
+	for (i = 0; i < size; i+=sizeof(pattern))
+	{
+		if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
+			return 1;
+	}
+
+	return 0;
+}
+
+static void
+test_correct(void)
+{
+	int dma_buf_fd;
+	char *ptr1, *ptr2;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	/* Check correctness vs GEM_MMAP_GTT */
+	ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
+	ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr1 != MAP_FAILED);
+	igt_assert(ptr2 != MAP_FAILED);
+	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
+
+	/* Check pattern correctness */
+	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+
+	munmap(ptr1, BO_SIZE);
+	munmap(ptr2, BO_SIZE);
+	close(dma_buf_fd);
+	gem_close(fd, handle);
+}
+
+static void
+test_map_unmap(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+	/* Unmap and remap */
+	munmap(ptr, BO_SIZE);
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+	munmap(ptr, BO_SIZE);
+	close(dma_buf_fd);
+	gem_close(fd, handle);
+}
+
+/* prime and then unprime and then prime again the same handle */
+static void
+test_reprime(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+	close (dma_buf_fd);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	munmap(ptr, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+	munmap(ptr, BO_SIZE);
+	close(dma_buf_fd);
+	gem_close(fd, handle);
+}
+
+/* map from another process */
+static void
+test_forked(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	igt_fork(childno, 1) {
+		ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+		igt_assert(ptr != MAP_FAILED);
+		igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+		munmap(ptr, BO_SIZE);
+		close(dma_buf_fd);
+	}
+	close(dma_buf_fd);
+	igt_waitchildren();
+	gem_close(fd, handle);
+}
+
+static void
+test_refcounting(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+	/* Close gem object before mapping */
+	gem_close(fd, handle);
+
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	munmap(ptr, BO_SIZE);
+	close (dma_buf_fd);
+}
+
+/* dup before mmap */
+static void
+test_dup(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+
+	dma_buf_fd = dup(prime_handle_to_fd(fd, handle));
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	munmap(ptr, BO_SIZE);
+	gem_close(fd, handle);
+	close (dma_buf_fd);
+}
+
+
+/* Used for error case testing to avoid wrapper */
+static int prime_handle_to_fd_no_assert(uint32_t handle, int *fd_out)
+{
+	struct drm_prime_handle args;
+	int ret;
+
+	args.handle = handle;
+	args.flags = DRM_CLOEXEC;
+	args.fd = -1;
+
+	ret = drmIoctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
+
+	*fd_out = args.fd;
+
+	return ret;
+}
+
+static void
+test_errors(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	/* Close gem object before priming */
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+	gem_close(fd, handle);
+	prime_handle_to_fd_no_assert(handle, &dma_buf_fd);
+	igt_assert(dma_buf_fd == -1 && errno == ENOENT);
+	errno = 0;
+
+	/* close fd before mapping */
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+	close(dma_buf_fd);
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr == MAP_FAILED && errno == EBADF);
+	errno = 0;
+	gem_close(fd, handle);
+
+	/* Map too big */
+	handle = gem_create(fd, BO_SIZE);
+	fill_bo(handle, BO_SIZE);
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+	ptr = mmap(NULL, BO_SIZE * 2, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr == MAP_FAILED && errno == EINVAL);
+	errno = 0;
+	close(dma_buf_fd);
+	gem_close(fd, handle);
+}
+
+static void
+test_aperture_limit(void)
+{
+	int dma_buf_fd1, dma_buf_fd2;
+	char *ptr1, *ptr2;
+	uint32_t handle1, handle2;
+	/* Two buffers the sum of which > mappable aperture */
+	uint64_t size1 = (gem_mappable_aperture_size() * 7) / 8;
+	uint64_t size2 = (gem_mappable_aperture_size() * 3) / 8;
+
+	handle1 = gem_create(fd, size1);
+	fill_bo(handle1, BO_SIZE);
+
+	dma_buf_fd1 = prime_handle_to_fd(fd, handle1);
+	igt_assert(errno == 0);
+	ptr1 = mmap(NULL, size1, PROT_READ, MAP_SHARED, dma_buf_fd1, 0);
+	igt_assert(ptr1 != MAP_FAILED);
+	igt_assert(pattern_check(ptr1, BO_SIZE) == 0);
+
+	handle2 = gem_create(fd, size1);
+	fill_bo(handle2, BO_SIZE);
+	dma_buf_fd2 = prime_handle_to_fd(fd, handle2);
+	igt_assert(errno == 0);
+	ptr2 = mmap(NULL, size2, PROT_READ, MAP_SHARED, dma_buf_fd2, 0);
+	igt_assert(ptr2 != MAP_FAILED);
+	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+
+	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
+
+	munmap(ptr1, size1);
+	munmap(ptr2, size2);
+	close(dma_buf_fd1);
+	close(dma_buf_fd2);
+	gem_close(fd, handle1);
+	gem_close(fd, handle2);
+}
+
+static int
+check_for_dma_buf_mmap(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+	int ret = 1;
+
+	handle = gem_create(fd, BO_SIZE);
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	if (ptr != MAP_FAILED)
+		ret = 0;
+	munmap(ptr, BO_SIZE);
+	gem_close(fd, handle);
+	close (dma_buf_fd);
+	return ret;
+}
+
+igt_main
+{
+	struct {
+		const char *name;
+		void (*fn)(void);
+	} tests[] = {
+		{ "test_correct", test_correct },
+		{ "test_map_unmap", test_map_unmap },
+		{ "test_reprime", test_reprime },
+		{ "test_forked", test_forked },
+		{ "test_refcounting", test_refcounting },
+		{ "test_dup", test_dup },
+		{ "test_errors", test_errors },
+		{ "test_aperture_limit", test_aperture_limit },
+	};
+	int i;
+
+	igt_fixture {
+		fd = drm_open_any();
+		errno = 0;
+	}
+
+	igt_skip_on((check_for_dma_buf_mmap() != 0));
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		igt_subtest(tests[i].name)
+			tests[i].fn();
+	}
+
+	igt_fixture
+		close(fd);
+}
-- 
2.1.0

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

Re: [PATCH 1/7] prime_mmap: Add new test for calling mmap() on dma-buf fds

[Daniel Vetter]

On Wed, Aug 12, 2015 at 08:29:14PM -0300, Tiago Vignatti wrote:
> From: Rob Bradford <rob@linux.intel.com>
> 
> This test has the following subtests:
>  - test_correct for correctness of the data
>  - test_map_unmap checks for mapping idempotency
>  - test_reprime checks for dma-buf creation idempotency
>  - test_forked checks for multiprocess access
>  - test_refcounting checks for buffer reference counting
>  - test_dup chats that dup()ing the fd works
>  - test_errors checks the error return values for failures
>  - test_aperture_limit tests multiple buffer creation at the gtt aperture
>    limit
> 
> Signed-off-by: Rob Bradford <rob@linux.intel.com>
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/prime_mmap.c     | 383 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 384 insertions(+)
>  create mode 100644 tests/prime_mmap.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c94714b..5b2072e 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -90,6 +90,7 @@ TESTS_progs_M = \
>  	pm_rps \
>  	pm_rc6_residency \
>  	pm_sseu \
> +	prime_mmap \
>  	prime_self_import \
>  	template \
>  	$(NULL)
> diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
> new file mode 100644
> index 0000000..4dc2055
> --- /dev/null
> +++ b/tests/prime_mmap.c
> @@ -0,0 +1,383 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Rob Bradford <rob at linux.intel.com>
> + *
> + */
> +
> +/*
> + * Testcase: Check whether mmap()ing dma-buf works
> + */
> +#define _GNU_SOURCE
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <pthread.h>
> +
> +#include "drm.h"
> +#include "i915_drm.h"
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "ioctl_wrappers.h"
> +
> +#define BO_SIZE (16*1024)
> +
> +static int fd;
> +
> +char pattern[] = {0xff, 0x00, 0x00, 0x00,
> +	0x00, 0xff, 0x00, 0x00,
> +	0x00, 0x00, 0xff, 0x00,
> +	0x00, 0x00, 0x00, 0xff};
> +
> +static void
> +fill_bo(uint32_t handle, size_t size)
> +{
> +	off_t i;
> +	for (i = 0; i < size; i+=sizeof(pattern))
> +	{
> +		gem_write(fd, handle, i, pattern, sizeof(pattern));
> +	}
> +}
> +
> +static int
> +pattern_check(char *ptr, size_t size)
> +{
> +	off_t i;
> +	for (i = 0; i < size; i+=sizeof(pattern))
> +	{
> +		if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +test_correct(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr1, *ptr2;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	/* Check correctness vs GEM_MMAP_GTT */
> +	ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
> +	ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr1 != MAP_FAILED);
> +	igt_assert(ptr2 != MAP_FAILED);
> +	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
> +
> +	/* Check pattern correctness */
> +	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
> +
> +	munmap(ptr1, BO_SIZE);
> +	munmap(ptr2, BO_SIZE);
> +	close(dma_buf_fd);
> +	gem_close(fd, handle);
> +}
> +
> +static void
> +test_map_unmap(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +
> +	/* Unmap and remap */
> +	munmap(ptr, BO_SIZE);
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +
> +	munmap(ptr, BO_SIZE);
> +	close(dma_buf_fd);
> +	gem_close(fd, handle);
> +}
> +
> +/* prime and then unprime and then prime again the same handle */
> +static void
> +test_reprime(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +
> +	close (dma_buf_fd);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +	munmap(ptr, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +
> +	munmap(ptr, BO_SIZE);
> +	close(dma_buf_fd);
> +	gem_close(fd, handle);
> +}
> +
> +/* map from another process */
> +static void
> +test_forked(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	igt_fork(childno, 1) {
> +		ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +		igt_assert(ptr != MAP_FAILED);
> +		igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +		munmap(ptr, BO_SIZE);
> +		close(dma_buf_fd);
> +	}
> +	close(dma_buf_fd);
> +	igt_waitchildren();
> +	gem_close(fd, handle);
> +}
> +
> +static void
> +test_refcounting(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +	/* Close gem object before mapping */
> +	gem_close(fd, handle);
> +
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +	munmap(ptr, BO_SIZE);
> +	close (dma_buf_fd);
> +}
> +
> +/* dup before mmap */
> +static void
> +test_dup(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +
> +	dma_buf_fd = dup(prime_handle_to_fd(fd, handle));
> +	igt_assert(errno == 0);
> +
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
> +	munmap(ptr, BO_SIZE);
> +	gem_close(fd, handle);
> +	close (dma_buf_fd);
> +}
> +
> +
> +/* Used for error case testing to avoid wrapper */
> +static int prime_handle_to_fd_no_assert(uint32_t handle, int *fd_out)
> +{
> +	struct drm_prime_handle args;
> +	int ret;
> +
> +	args.handle = handle;
> +	args.flags = DRM_CLOEXEC;
> +	args.fd = -1;
> +
> +	ret = drmIoctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
> +
> +	*fd_out = args.fd;
> +
> +	return ret;
> +}
> +
> +static void
> +test_errors(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	/* Close gem object before priming */
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +	gem_close(fd, handle);
> +	prime_handle_to_fd_no_assert(handle, &dma_buf_fd);
> +	igt_assert(dma_buf_fd == -1 && errno == ENOENT);
> +	errno = 0;
> +
> +	/* close fd before mapping */
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +	close(dma_buf_fd);
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr == MAP_FAILED && errno == EBADF);
> +	errno = 0;
> +	gem_close(fd, handle);
> +
> +	/* Map too big */
> +	handle = gem_create(fd, BO_SIZE);
> +	fill_bo(handle, BO_SIZE);
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +	ptr = mmap(NULL, BO_SIZE * 2, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr == MAP_FAILED && errno == EINVAL);
> +	errno = 0;
> +	close(dma_buf_fd);
> +	gem_close(fd, handle);

That only checks for one of the conditions, trying to map something
offset/overlapping the end of the buffer, but small enough needs to be
tested separately.

Also I think a testcase for invalid buffer2fd flags would be good, just
for completeness - we seem to be missing that one.
-Daniel

> +}
> +
> +static void
> +test_aperture_limit(void)
> +{
> +	int dma_buf_fd1, dma_buf_fd2;
> +	char *ptr1, *ptr2;
> +	uint32_t handle1, handle2;
> +	/* Two buffers the sum of which > mappable aperture */
> +	uint64_t size1 = (gem_mappable_aperture_size() * 7) / 8;
> +	uint64_t size2 = (gem_mappable_aperture_size() * 3) / 8;
> +
> +	handle1 = gem_create(fd, size1);
> +	fill_bo(handle1, BO_SIZE);
> +
> +	dma_buf_fd1 = prime_handle_to_fd(fd, handle1);
> +	igt_assert(errno == 0);
> +	ptr1 = mmap(NULL, size1, PROT_READ, MAP_SHARED, dma_buf_fd1, 0);
> +	igt_assert(ptr1 != MAP_FAILED);
> +	igt_assert(pattern_check(ptr1, BO_SIZE) == 0);
> +
> +	handle2 = gem_create(fd, size1);
> +	fill_bo(handle2, BO_SIZE);
> +	dma_buf_fd2 = prime_handle_to_fd(fd, handle2);
> +	igt_assert(errno == 0);
> +	ptr2 = mmap(NULL, size2, PROT_READ, MAP_SHARED, dma_buf_fd2, 0);
> +	igt_assert(ptr2 != MAP_FAILED);
> +	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
> +
> +	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
> +
> +	munmap(ptr1, size1);
> +	munmap(ptr2, size2);
> +	close(dma_buf_fd1);
> +	close(dma_buf_fd2);
> +	gem_close(fd, handle1);
> +	gem_close(fd, handle2);
> +}
> +
> +static int
> +check_for_dma_buf_mmap(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +	int ret = 1;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> +	if (ptr != MAP_FAILED)
> +		ret = 0;
> +	munmap(ptr, BO_SIZE);
> +	gem_close(fd, handle);
> +	close (dma_buf_fd);
> +	return ret;
> +}
> +
> +igt_main
> +{
> +	struct {
> +		const char *name;
> +		void (*fn)(void);
> +	} tests[] = {
> +		{ "test_correct", test_correct },
> +		{ "test_map_unmap", test_map_unmap },
> +		{ "test_reprime", test_reprime },
> +		{ "test_forked", test_forked },
> +		{ "test_refcounting", test_refcounting },
> +		{ "test_dup", test_dup },
> +		{ "test_errors", test_errors },
> +		{ "test_aperture_limit", test_aperture_limit },
> +	};
> +	int i;
> +
> +	igt_fixture {
> +		fd = drm_open_any();
> +		errno = 0;
> +	}
> +
> +	igt_skip_on((check_for_dma_buf_mmap() != 0));
> +
> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +		igt_subtest(tests[i].name)
> +			tests[i].fn();
> +	}
> +
> +	igt_fixture
> +		close(fd);
> +}
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] prime_mmap: Add new test for calling mmap() on dma-buf fds

[Tiago Vignatti]

Hi Daniel,

On 08/13/2015 04:04 AM, Daniel Vetter wrote:
> On Wed, Aug 12, 2015 at 08:29:14PM -0300, Tiago Vignatti wrote:
>> +	/* Map too big */
>> +	handle = gem_create(fd, BO_SIZE);
>> +	fill_bo(handle, BO_SIZE);
>> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
>> +	igt_assert(errno == 0);
>> +	ptr = mmap(NULL, BO_SIZE * 2, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
>> +	igt_assert(ptr == MAP_FAILED && errno == EINVAL);
>> +	errno = 0;
>> +	close(dma_buf_fd);
>> +	gem_close(fd, handle);
>
> That only checks for one of the conditions, trying to map something
> offset/overlapping the end of the buffer, but small enough needs to be
> tested separately.

you mean test for smaller length with a non-zero offset? I don't think I 
get what you wanted to say here maybe.

> Also I think a testcase for invalid buffer2fd flags would be good, just
> for completeness - we seem to be missing that one.

you mean test for different flags than the ones supported by 
DRM_IOCTL_PRIME_HANDLE_TO_FD?

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

Re: [PATCH 1/7] prime_mmap: Add new test for calling mmap() on dma-buf fds

[Daniel Vetter]

On Fri, Aug 14, 2015 at 07:17:05PM -0300, Tiago Vignatti wrote:
> Hi Daniel,
> 
> On 08/13/2015 04:04 AM, Daniel Vetter wrote:
> >On Wed, Aug 12, 2015 at 08:29:14PM -0300, Tiago Vignatti wrote:
> >>+	/* Map too big */
> >>+	handle = gem_create(fd, BO_SIZE);
> >>+	fill_bo(handle, BO_SIZE);
> >>+	dma_buf_fd = prime_handle_to_fd(fd, handle);
> >>+	igt_assert(errno == 0);
> >>+	ptr = mmap(NULL, BO_SIZE * 2, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
> >>+	igt_assert(ptr == MAP_FAILED && errno == EINVAL);
> >>+	errno = 0;
> >>+	close(dma_buf_fd);
> >>+	gem_close(fd, handle);
> >
> >That only checks for one of the conditions, trying to map something
> >offset/overlapping the end of the buffer, but small enough needs to be
> >tested separately.
> 
> you mean test for smaller length with a non-zero offset? I don't think I get
> what you wanted to say here maybe.

Atm you test size=2*bo_size, offset=0. I also want to test size=bo_size,
offset=bo_size/2 since there seems to be a bug for that case in your code.

> >Also I think a testcase for invalid buffer2fd flags would be good, just
> >for completeness - we seem to be missing that one.
> 
> you mean test for different flags than the ones supported by
> DRM_IOCTL_PRIME_HANDLE_TO_FD?

Yup, just the normal "fill in missing coverage" we do when extending an
existing interface.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/7] prime_mmap: Fix a few misc stuff

[Tiago Vignatti]

- Remove pattern_check(), which was walking through a useless iterator
- Remove superfluous PROT_WRITE from gem_mmap, in test_correct()
- Add binary file to .gitignore

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 tests/.gitignore   |  1 +
 tests/prime_mmap.c | 37 ++++++++++++-------------------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/tests/.gitignore b/tests/.gitignore
index 0af0899..5bc4a58 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -163,6 +163,7 @@ pm_sseu
 prime_nv_api
 prime_nv_pcopy
 prime_nv_test
+prime_mmap
 prime_self_import
 prime_udl
 template
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index 4dc2055..dc59e8f 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -65,19 +65,6 @@ fill_bo(uint32_t handle, size_t size)
 	}
 }
 
-static int
-pattern_check(char *ptr, size_t size)
-{
-	off_t i;
-	for (i = 0; i < size; i+=sizeof(pattern))
-	{
-		if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
-			return 1;
-	}
-
-	return 0;
-}
-
 static void
 test_correct(void)
 {
@@ -92,14 +79,14 @@ test_correct(void)
 	igt_assert(errno == 0);
 
 	/* Check correctness vs GEM_MMAP_GTT */
-	ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
+	ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ);
 	ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr1 != MAP_FAILED);
 	igt_assert(ptr2 != MAP_FAILED);
 	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
 
 	/* Check pattern correctness */
-	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr2, pattern, sizeof(pattern)) == 0);
 
 	munmap(ptr1, BO_SIZE);
 	munmap(ptr2, BO_SIZE);
@@ -122,13 +109,13 @@ test_map_unmap(void)
 
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 
 	/* Unmap and remap */
 	munmap(ptr, BO_SIZE);
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 
 	munmap(ptr, BO_SIZE);
 	close(dma_buf_fd);
@@ -151,16 +138,16 @@ test_reprime(void)
 
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 
 	close (dma_buf_fd);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 	munmap(ptr, BO_SIZE);
 
 	dma_buf_fd = prime_handle_to_fd(fd, handle);
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 
 	munmap(ptr, BO_SIZE);
 	close(dma_buf_fd);
@@ -184,7 +171,7 @@ test_forked(void)
 	igt_fork(childno, 1) {
 		ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 		igt_assert(ptr != MAP_FAILED);
-		igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+		igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 		munmap(ptr, BO_SIZE);
 		close(dma_buf_fd);
 	}
@@ -210,7 +197,7 @@ test_refcounting(void)
 
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 	munmap(ptr, BO_SIZE);
 	close (dma_buf_fd);
 }
@@ -231,7 +218,7 @@ test_dup(void)
 
 	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
 	igt_assert(ptr != MAP_FAILED);
-	igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
 	munmap(ptr, BO_SIZE);
 	gem_close(fd, handle);
 	close (dma_buf_fd);
@@ -310,7 +297,7 @@ test_aperture_limit(void)
 	igt_assert(errno == 0);
 	ptr1 = mmap(NULL, size1, PROT_READ, MAP_SHARED, dma_buf_fd1, 0);
 	igt_assert(ptr1 != MAP_FAILED);
-	igt_assert(pattern_check(ptr1, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr1, pattern, sizeof(pattern)) == 0);
 
 	handle2 = gem_create(fd, size1);
 	fill_bo(handle2, BO_SIZE);
@@ -318,7 +305,7 @@ test_aperture_limit(void)
 	igt_assert(errno == 0);
 	ptr2 = mmap(NULL, size2, PROT_READ, MAP_SHARED, dma_buf_fd2, 0);
 	igt_assert(ptr2 != MAP_FAILED);
-	igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+	igt_assert(memcmp(ptr2, pattern, sizeof(pattern)) == 0);
 
 	igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
 
-- 
2.1.0

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

[PATCH 3/7] prime_mmap: Add basic tests to write in a bo using CPU

[Tiago Vignatti]

This patch adds test_correct_cpu_write, which maps the texture buffer through a
prime fd and then writes directly to it using the CPU. It stresses the driver
to guarantee cache synchronization among the different domains.

This test also adds test_forked_cpu_write, which creates the GEM bo in one
process and pass the prime handle of the it to another process, which in turn
uses the handle only to map and write. Grossly speaking this test simulates
Chrome OS  architecture, where the Web content ("unpriviledged process") maps
and CPU-draws a buffer, which was previously allocated in the GPU process
("priviledged process").

This requires kernel modifications (Daniel Thompson's "drm: prime: Honour
O_RDWR during prime-handle-to-fd").

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 lib/ioctl_wrappers.c |  5 +++-
 tests/prime_mmap.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 53bd635..941fa66 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1125,6 +1125,9 @@ void gem_require_ring(int fd, int ring_id)
 
 /* prime */
 
+#ifndef DRM_RDWR
+#define DRM_RDWR O_RDWR
+#endif
 /**
  * prime_handle_to_fd:
  * @fd: open i915 drm file descriptor
@@ -1142,7 +1145,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
 
 	memset(&args, 0, sizeof(args));
 	args.handle = handle;
-	args.flags = DRM_CLOEXEC;
+	args.flags = DRM_CLOEXEC | DRM_RDWR;
 	args.fd = -1;
 
 	do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index dc59e8f..ad91371 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -22,6 +22,7 @@
  *
  * Authors:
  *    Rob Bradford <rob at linux.intel.com>
+ *    Tiago Vignatti <tiago.vignatti at intel.com>
  *
  */
 
@@ -66,6 +67,12 @@ fill_bo(uint32_t handle, size_t size)
 }
 
 static void
+fill_bo_cpu(char *ptr)
+{
+	memcpy(ptr, pattern, sizeof(pattern));
+}
+
+static void
 test_correct(void)
 {
 	int dma_buf_fd;
@@ -180,6 +187,62 @@ test_forked(void)
 	gem_close(fd, handle);
 }
 
+/* test CPU write. This has a rather big implication for the driver which must
+ * guarantee cache synchronization when writing the bo using CPU. */
+static void
+test_correct_cpu_write(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	/* Check correctness of map using write protection (PROT_WRITE) */
+	ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+
+	/* Fill bo using CPU */
+	fill_bo_cpu(ptr);
+
+	/* Check pattern correctness */
+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
+
+	munmap(ptr, BO_SIZE);
+	close(dma_buf_fd);
+	gem_close(fd, handle);
+}
+
+/* map from another process and then write using CPU */
+static void
+test_forked_cpu_write(void)
+{
+	int dma_buf_fd;
+	char *ptr;
+	uint32_t handle;
+
+	handle = gem_create(fd, BO_SIZE);
+
+	dma_buf_fd = prime_handle_to_fd(fd, handle);
+	igt_assert(errno == 0);
+
+	igt_fork(childno, 1) {
+		ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE , MAP_SHARED, dma_buf_fd, 0);
+		igt_assert(ptr != MAP_FAILED);
+		fill_bo_cpu(ptr);
+
+		igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
+		munmap(ptr, BO_SIZE);
+		close(dma_buf_fd);
+	}
+	close(dma_buf_fd);
+	igt_waitchildren();
+	gem_close(fd, handle);
+}
+
 static void
 test_refcounting(void)
 {
@@ -346,6 +409,8 @@ igt_main
 		{ "test_map_unmap", test_map_unmap },
 		{ "test_reprime", test_reprime },
 		{ "test_forked", test_forked },
+		{ "test_correct_cpu_write", test_correct_cpu_write },
+		{ "test_forked_cpu_write", test_forked_cpu_write },
 		{ "test_refcounting", test_refcounting },
 		{ "test_dup", test_dup },
 		{ "test_errors", test_errors },
-- 
2.1.0

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

Re: [PATCH 3/7] prime_mmap: Add basic tests to write in a bo using CPU

[Daniel Vetter]

On Wed, Aug 12, 2015 at 08:29:16PM -0300, Tiago Vignatti wrote:
> This patch adds test_correct_cpu_write, which maps the texture buffer through a
> prime fd and then writes directly to it using the CPU. It stresses the driver
> to guarantee cache synchronization among the different domains.
> 
> This test also adds test_forked_cpu_write, which creates the GEM bo in one
> process and pass the prime handle of the it to another process, which in turn
> uses the handle only to map and write. Grossly speaking this test simulates
> Chrome OS  architecture, where the Web content ("unpriviledged process") maps
> and CPU-draws a buffer, which was previously allocated in the GPU process
> ("priviledged process").
> 
> This requires kernel modifications (Daniel Thompson's "drm: prime: Honour
> O_RDWR during prime-handle-to-fd").
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>

Squash with previous patch?
> ---
>  lib/ioctl_wrappers.c |  5 +++-
>  tests/prime_mmap.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 53bd635..941fa66 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1125,6 +1125,9 @@ void gem_require_ring(int fd, int ring_id)
>  
>  /* prime */
>  
> +#ifndef DRM_RDWR
> +#define DRM_RDWR O_RDWR
> +#endif
>  /**
>   * prime_handle_to_fd:
>   * @fd: open i915 drm file descriptor
> @@ -1142,7 +1145,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
>  
>  	memset(&args, 0, sizeof(args));
>  	args.handle = handle;
> -	args.flags = DRM_CLOEXEC;
> +	args.flags = DRM_CLOEXEC | DRM_RDWR;

This needs to be optional otherwise all the existing prime tests start
falling over on older kernels. Probably need a
prime_handle_to_fd_with_mmap, which doesn an igt_skip if it fails.
-Daniel

>  	args.fd = -1;
>  
>  	do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
> diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
> index dc59e8f..ad91371 100644
> --- a/tests/prime_mmap.c
> +++ b/tests/prime_mmap.c
> @@ -22,6 +22,7 @@
>   *
>   * Authors:
>   *    Rob Bradford <rob at linux.intel.com>
> + *    Tiago Vignatti <tiago.vignatti at intel.com>
>   *
>   */
>  
> @@ -66,6 +67,12 @@ fill_bo(uint32_t handle, size_t size)
>  }
>  
>  static void
> +fill_bo_cpu(char *ptr)
> +{
> +	memcpy(ptr, pattern, sizeof(pattern));
> +}
> +
> +static void
>  test_correct(void)
>  {
>  	int dma_buf_fd;
> @@ -180,6 +187,62 @@ test_forked(void)
>  	gem_close(fd, handle);
>  }
>  
> +/* test CPU write. This has a rather big implication for the driver which must
> + * guarantee cache synchronization when writing the bo using CPU. */
> +static void
> +test_correct_cpu_write(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	/* Check correctness of map using write protection (PROT_WRITE) */
> +	ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd, 0);
> +	igt_assert(ptr != MAP_FAILED);
> +
> +	/* Fill bo using CPU */
> +	fill_bo_cpu(ptr);
> +
> +	/* Check pattern correctness */
> +	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
> +
> +	munmap(ptr, BO_SIZE);
> +	close(dma_buf_fd);
> +	gem_close(fd, handle);
> +}
> +
> +/* map from another process and then write using CPU */
> +static void
> +test_forked_cpu_write(void)
> +{
> +	int dma_buf_fd;
> +	char *ptr;
> +	uint32_t handle;
> +
> +	handle = gem_create(fd, BO_SIZE);
> +
> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
> +	igt_assert(errno == 0);
> +
> +	igt_fork(childno, 1) {
> +		ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE , MAP_SHARED, dma_buf_fd, 0);
> +		igt_assert(ptr != MAP_FAILED);
> +		fill_bo_cpu(ptr);
> +
> +		igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
> +		munmap(ptr, BO_SIZE);
> +		close(dma_buf_fd);
> +	}
> +	close(dma_buf_fd);
> +	igt_waitchildren();
> +	gem_close(fd, handle);
> +}
> +
>  static void
>  test_refcounting(void)
>  {
> @@ -346,6 +409,8 @@ igt_main
>  		{ "test_map_unmap", test_map_unmap },
>  		{ "test_reprime", test_reprime },
>  		{ "test_forked", test_forked },
> +		{ "test_correct_cpu_write", test_correct_cpu_write },
> +		{ "test_forked_cpu_write", test_forked_cpu_write },
>  		{ "test_refcounting", test_refcounting },
>  		{ "test_dup", test_dup },
>  		{ "test_errors", test_errors },
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] prime_mmap: Add basic tests to write in a bo using CPU

[Tiago Vignatti]

On 08/13/2015 04:01 AM, Daniel Vetter wrote:
> On Wed, Aug 12, 2015 at 08:29:16PM -0300, Tiago Vignatti wrote:
>> This patch adds test_correct_cpu_write, which maps the texture buffer through a
>> prime fd and then writes directly to it using the CPU. It stresses the driver
>> to guarantee cache synchronization among the different domains.
>>
>> This test also adds test_forked_cpu_write, which creates the GEM bo in one
>> process and pass the prime handle of the it to another process, which in turn
>> uses the handle only to map and write. Grossly speaking this test simulates
>> Chrome OS  architecture, where the Web content ("unpriviledged process") maps
>> and CPU-draws a buffer, which was previously allocated in the GPU process
>> ("priviledged process").
>>
>> This requires kernel modifications (Daniel Thompson's "drm: prime: Honour
>> O_RDWR during prime-handle-to-fd").
>>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
>
> Squash with previous patch?

why? if the whole point is to decrease the amount of patches, then I 
prefer to squash 2/7 with the 1/7 (although they're from different 
authors and would be nice to keep separately the changes from each). 
This patch here introduces this writing to mmap'ed dma-buf fd, a concept 
that is still in debate, requiring a kernel counter-part so that's why I 
preferred to keep it away.


>> ---
>>   lib/ioctl_wrappers.c |  5 +++-
>>   tests/prime_mmap.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>> index 53bd635..941fa66 100644
>> --- a/lib/ioctl_wrappers.c
>> +++ b/lib/ioctl_wrappers.c
>> @@ -1125,6 +1125,9 @@ void gem_require_ring(int fd, int ring_id)
>>
>>   /* prime */
>>
>> +#ifndef DRM_RDWR
>> +#define DRM_RDWR O_RDWR
>> +#endif
>>   /**
>>    * prime_handle_to_fd:
>>    * @fd: open i915 drm file descriptor
>> @@ -1142,7 +1145,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
>>
>>   	memset(&args, 0, sizeof(args));
>>   	args.handle = handle;
>> -	args.flags = DRM_CLOEXEC;
>> +	args.flags = DRM_CLOEXEC | DRM_RDWR;
>
> This needs to be optional otherwise all the existing prime tests start
> falling over on older kernels. Probably need a
> prime_handle_to_fd_with_mmap, which doesn an igt_skip if it fails.

true. Thank you.


> -Daniel
>
>>   	args.fd = -1;
>>
>>   	do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
>> diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
>> index dc59e8f..ad91371 100644
>> --- a/tests/prime_mmap.c
>> +++ b/tests/prime_mmap.c
>> @@ -22,6 +22,7 @@
>>    *
>>    * Authors:
>>    *    Rob Bradford <rob at linux.intel.com>
>> + *    Tiago Vignatti <tiago.vignatti at intel.com>
>>    *
>>    */
>>
>> @@ -66,6 +67,12 @@ fill_bo(uint32_t handle, size_t size)
>>   }
>>
>>   static void
>> +fill_bo_cpu(char *ptr)
>> +{
>> +	memcpy(ptr, pattern, sizeof(pattern));
>> +}
>> +
>> +static void
>>   test_correct(void)
>>   {
>>   	int dma_buf_fd;
>> @@ -180,6 +187,62 @@ test_forked(void)
>>   	gem_close(fd, handle);
>>   }
>>
>> +/* test CPU write. This has a rather big implication for the driver which must
>> + * guarantee cache synchronization when writing the bo using CPU. */
>> +static void
>> +test_correct_cpu_write(void)
>> +{
>> +	int dma_buf_fd;
>> +	char *ptr;
>> +	uint32_t handle;
>> +
>> +	handle = gem_create(fd, BO_SIZE);
>> +
>> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
>> +	igt_assert(errno == 0);
>> +
>> +	/* Check correctness of map using write protection (PROT_WRITE) */
>> +	ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd, 0);
>> +	igt_assert(ptr != MAP_FAILED);
>> +
>> +	/* Fill bo using CPU */
>> +	fill_bo_cpu(ptr);
>> +
>> +	/* Check pattern correctness */
>> +	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
>> +
>> +	munmap(ptr, BO_SIZE);
>> +	close(dma_buf_fd);
>> +	gem_close(fd, handle);
>> +}
>> +
>> +/* map from another process and then write using CPU */
>> +static void
>> +test_forked_cpu_write(void)
>> +{
>> +	int dma_buf_fd;
>> +	char *ptr;
>> +	uint32_t handle;
>> +
>> +	handle = gem_create(fd, BO_SIZE);
>> +
>> +	dma_buf_fd = prime_handle_to_fd(fd, handle);
>> +	igt_assert(errno == 0);
>> +
>> +	igt_fork(childno, 1) {
>> +		ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE , MAP_SHARED, dma_buf_fd, 0);
>> +		igt_assert(ptr != MAP_FAILED);
>> +		fill_bo_cpu(ptr);
>> +
>> +		igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
>> +		munmap(ptr, BO_SIZE);
>> +		close(dma_buf_fd);
>> +	}
>> +	close(dma_buf_fd);
>> +	igt_waitchildren();
>> +	gem_close(fd, handle);
>> +}
>> +
>>   static void
>>   test_refcounting(void)
>>   {
>> @@ -346,6 +409,8 @@ igt_main
>>   		{ "test_map_unmap", test_map_unmap },
>>   		{ "test_reprime", test_reprime },
>>   		{ "test_forked", test_forked },
>> +		{ "test_correct_cpu_write", test_correct_cpu_write },
>> +		{ "test_forked_cpu_write", test_forked_cpu_write },
>>   		{ "test_refcounting", test_refcounting },
>>   		{ "test_dup", test_dup },
>>   		{ "test_errors", test_errors },
>> --
>> 2.1.0
>>
>

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

Re: [PATCH 3/7] prime_mmap: Add basic tests to write in a bo using CPU

[Daniel Vetter]

On Thu, Aug 13, 2015 at 11:26:57AM -0300, Tiago Vignatti wrote:
> On 08/13/2015 04:01 AM, Daniel Vetter wrote:
> >On Wed, Aug 12, 2015 at 08:29:16PM -0300, Tiago Vignatti wrote:
> >>This patch adds test_correct_cpu_write, which maps the texture buffer through a
> >>prime fd and then writes directly to it using the CPU. It stresses the driver
> >>to guarantee cache synchronization among the different domains.
> >>
> >>This test also adds test_forked_cpu_write, which creates the GEM bo in one
> >>process and pass the prime handle of the it to another process, which in turn
> >>uses the handle only to map and write. Grossly speaking this test simulates
> >>Chrome OS  architecture, where the Web content ("unpriviledged process") maps
> >>and CPU-draws a buffer, which was previously allocated in the GPU process
> >>("priviledged process").
> >>
> >>This requires kernel modifications (Daniel Thompson's "drm: prime: Honour
> >>O_RDWR during prime-handle-to-fd").
> >>
> >>Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
> >
> >Squash with previous patch?
> 
> why? if the whole point is to decrease the amount of patches, then I prefer
> to squash 2/7 with the 1/7 (although they're from different authors and
> would be nice to keep separately the changes from each). This patch here
> introduces this writing to mmap'ed dma-buf fd, a concept that is still in
> debate, requiring a kernel counter-part so that's why I preferred to keep it
> away.

Replied to the wrong patch, I meant merging patch 1&2 ofc ;-)
-Daniel

> 
> 
> >>---
> >>  lib/ioctl_wrappers.c |  5 +++-
> >>  tests/prime_mmap.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 69 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >>index 53bd635..941fa66 100644
> >>--- a/lib/ioctl_wrappers.c
> >>+++ b/lib/ioctl_wrappers.c
> >>@@ -1125,6 +1125,9 @@ void gem_require_ring(int fd, int ring_id)
> >>
> >>  /* prime */
> >>
> >>+#ifndef DRM_RDWR
> >>+#define DRM_RDWR O_RDWR
> >>+#endif
> >>  /**
> >>   * prime_handle_to_fd:
> >>   * @fd: open i915 drm file descriptor
> >>@@ -1142,7 +1145,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
> >>
> >>  	memset(&args, 0, sizeof(args));
> >>  	args.handle = handle;
> >>-	args.flags = DRM_CLOEXEC;
> >>+	args.flags = DRM_CLOEXEC | DRM_RDWR;
> >
> >This needs to be optional otherwise all the existing prime tests start
> >falling over on older kernels. Probably need a
> >prime_handle_to_fd_with_mmap, which doesn an igt_skip if it fails.
> 
> true. Thank you.
> 
> 
> >-Daniel
> >
> >>  	args.fd = -1;
> >>
> >>  	do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
> >>diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
> >>index dc59e8f..ad91371 100644
> >>--- a/tests/prime_mmap.c
> >>+++ b/tests/prime_mmap.c
> >>@@ -22,6 +22,7 @@
> >>   *
> >>   * Authors:
> >>   *    Rob Bradford <rob at linux.intel.com>
> >>+ *    Tiago Vignatti <tiago.vignatti at intel.com>
> >>   *
> >>   */
> >>
> >>@@ -66,6 +67,12 @@ fill_bo(uint32_t handle, size_t size)
> >>  }
> >>
> >>  static void
> >>+fill_bo_cpu(char *ptr)
> >>+{
> >>+	memcpy(ptr, pattern, sizeof(pattern));
> >>+}
> >>+
> >>+static void
> >>  test_correct(void)
> >>  {
> >>  	int dma_buf_fd;
> >>@@ -180,6 +187,62 @@ test_forked(void)
> >>  	gem_close(fd, handle);
> >>  }
> >>
> >>+/* test CPU write. This has a rather big implication for the driver which must
> >>+ * guarantee cache synchronization when writing the bo using CPU. */
> >>+static void
> >>+test_correct_cpu_write(void)
> >>+{
> >>+	int dma_buf_fd;
> >>+	char *ptr;
> >>+	uint32_t handle;
> >>+
> >>+	handle = gem_create(fd, BO_SIZE);
> >>+
> >>+	dma_buf_fd = prime_handle_to_fd(fd, handle);
> >>+	igt_assert(errno == 0);
> >>+
> >>+	/* Check correctness of map using write protection (PROT_WRITE) */
> >>+	ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd, 0);
> >>+	igt_assert(ptr != MAP_FAILED);
> >>+
> >>+	/* Fill bo using CPU */
> >>+	fill_bo_cpu(ptr);
> >>+
> >>+	/* Check pattern correctness */
> >>+	igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
> >>+
> >>+	munmap(ptr, BO_SIZE);
> >>+	close(dma_buf_fd);
> >>+	gem_close(fd, handle);
> >>+}
> >>+
> >>+/* map from another process and then write using CPU */
> >>+static void
> >>+test_forked_cpu_write(void)
> >>+{
> >>+	int dma_buf_fd;
> >>+	char *ptr;
> >>+	uint32_t handle;
> >>+
> >>+	handle = gem_create(fd, BO_SIZE);
> >>+
> >>+	dma_buf_fd = prime_handle_to_fd(fd, handle);
> >>+	igt_assert(errno == 0);
> >>+
> >>+	igt_fork(childno, 1) {
> >>+		ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE , MAP_SHARED, dma_buf_fd, 0);
> >>+		igt_assert(ptr != MAP_FAILED);
> >>+		fill_bo_cpu(ptr);
> >>+
> >>+		igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
> >>+		munmap(ptr, BO_SIZE);
> >>+		close(dma_buf_fd);
> >>+	}
> >>+	close(dma_buf_fd);
> >>+	igt_waitchildren();
> >>+	gem_close(fd, handle);
> >>+}
> >>+
> >>  static void
> >>  test_refcounting(void)
> >>  {
> >>@@ -346,6 +409,8 @@ igt_main
> >>  		{ "test_map_unmap", test_map_unmap },
> >>  		{ "test_reprime", test_reprime },
> >>  		{ "test_forked", test_forked },
> >>+		{ "test_correct_cpu_write", test_correct_cpu_write },
> >>+		{ "test_forked_cpu_write", test_forked_cpu_write },
> >>  		{ "test_refcounting", test_refcounting },
> >>  		{ "test_dup", test_dup },
> >>  		{ "test_errors", test_errors },
> >>--
> >>2.1.0
> >>
> >
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] lib: Add gem_userptr helpers

[Tiago Vignatti]

This patch moves userptr definitions and helpers implementation that were
locally in gem_userptr_benchmark and gem_userptr_blits to the library, so other
tests can make use of them as well. There's no functional changes.

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 benchmarks/gem_userptr_benchmark.c | 45 +++----------------
 lib/ioctl_wrappers.c               | 30 +++++++++++++
 lib/ioctl_wrappers.h               | 13 ++++++
 tests/gem_userptr_blits.c          | 92 +++++++++++---------------------------
 4 files changed, 73 insertions(+), 107 deletions(-)

diff --git a/benchmarks/gem_userptr_benchmark.c b/benchmarks/gem_userptr_benchmark.c
index b804fdd..e0797dc 100644
--- a/benchmarks/gem_userptr_benchmark.c
+++ b/benchmarks/gem_userptr_benchmark.c
@@ -58,17 +58,6 @@
   #define PAGE_SIZE 4096
 #endif
 
-#define LOCAL_I915_GEM_USERPTR       0x33
-#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
-struct local_i915_gem_userptr {
-	uint64_t user_ptr;
-	uint64_t user_size;
-	uint32_t flags;
-#define LOCAL_I915_USERPTR_READ_ONLY (1<<0)
-#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (1<<31)
-	uint32_t handle;
-};
-
 static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
 
 #define BO_SIZE (65536)
@@ -83,30 +72,6 @@ static void gem_userptr_test_synchronized(void)
 	userptr_flags = 0;
 }
 
-static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t *handle)
-{
-	struct local_i915_gem_userptr userptr;
-	int ret;
-
-	userptr.user_ptr = (uintptr_t)ptr;
-	userptr.user_size = size;
-	userptr.flags = userptr_flags;
-	if (read_only)
-		userptr.flags |= LOCAL_I915_USERPTR_READ_ONLY;
-
-	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, &userptr);
-	if (ret)
-		ret = errno;
-	igt_skip_on_f(ret == ENODEV &&
-		      (userptr_flags & LOCAL_I915_USERPTR_UNSYNCHRONIZED) == 0 &&
-		      !read_only,
-		      "Skipping, synchronized mappings with no kernel CONFIG_MMU_NOTIFIER?");
-	if (ret == 0)
-		*handle = userptr.handle;
-
-	return ret;
-}
-
 static void **handle_ptr_map;
 static unsigned int num_handle_ptr_map;
 
@@ -144,7 +109,7 @@ static uint32_t create_userptr_bo(int fd, int size)
 	ret = posix_memalign(&ptr, PAGE_SIZE, size);
 	igt_assert(ret == 0);
 
-	ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, &handle);
+	ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, userptr_flags, &handle);
 	igt_assert(ret == 0);
 	add_handle_ptr(handle, ptr);
 
@@ -167,7 +132,7 @@ static int has_userptr(int fd)
 	assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
 	oldflags = userptr_flags;
 	gem_userptr_test_unsynchronized();
-	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle);
 	userptr_flags = oldflags;
 	if (ret != 0) {
 		free(ptr);
@@ -379,7 +344,7 @@ static void test_impact_overlap(int fd, const char *prefix)
 
 			for (i = 0, p = block; i < nr_bos[subtest];
 			     i++, p += PAGE_SIZE)
-				ret = gem_userptr(fd, (uint32_t *)p, BO_SIZE, 0,
+				ret = gem_userptr(fd, (uint32_t *)p, BO_SIZE, 0, userptr_flags,
 						  &handles[i]);
 				igt_assert(ret == 0);
 		}
@@ -439,7 +404,7 @@ static void test_single(int fd)
 	start_test(test_duration_sec);
 
 	while (run_test) {
-		ret = gem_userptr(fd, bo_ptr, BO_SIZE, 0, &handle);
+		ret = gem_userptr(fd, bo_ptr, BO_SIZE, 0, userptr_flags, &handle);
 		assert(ret == 0);
 		gem_close(fd, handle);
 		iter++;
@@ -480,7 +445,7 @@ static void test_multiple(int fd, unsigned int batch, int random)
 		for (i = 0; i < batch; i++) {
 			ret = gem_userptr(fd, bo_ptr + map[i] * BO_SIZE,
 						BO_SIZE,
-						0, &handles[i]);
+						0, userptr_flags, &handles[i]);
 			assert(ret == 0);
 		}
 		if (random)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 941fa66..5152647 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -742,6 +742,36 @@ void gem_context_require_ban_period(int fd)
 	igt_require(has_ban_period);
 }
 
+int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle)
+{
+	struct local_i915_gem_userptr userptr;
+	int ret;
+
+	memset(&userptr, 0, sizeof(userptr));
+	userptr.user_ptr = (uintptr_t)ptr;
+	userptr.user_size = size;
+	userptr.flags = flags;
+	if (read_only)
+		userptr.flags |= LOCAL_I915_USERPTR_READ_ONLY;
+
+	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, &userptr);
+	if (ret)
+		ret = errno;
+	igt_skip_on_f(ret == ENODEV &&
+			(flags & LOCAL_I915_USERPTR_UNSYNCHRONIZED) == 0 &&
+			!read_only,
+			"Skipping, synchronized mappings with no kernel CONFIG_MMU_NOTIFIER?");
+	if (ret == 0)
+		*handle = userptr.handle;
+
+	return ret;
+}
+
+void gem_userptr_sync(int fd, uint32_t handle)
+{
+	gem_set_domain(fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+}
+
 /**
  * gem_sw_finish:
  * @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index bc5d4bd..030f785 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -109,6 +109,19 @@ void gem_context_require_param(int fd, uint64_t param);
 void gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 void gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 
+#define LOCAL_I915_GEM_USERPTR       0x33
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+  uint64_t user_ptr;
+  uint64_t user_size;
+  uint32_t flags;
+#define LOCAL_I915_USERPTR_READ_ONLY (1<<0)
+#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (1<<31)
+  uint32_t handle;
+};
+int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t flags, uint32_t *handle);
+void gem_userptr_sync(int fd, uint32_t handle);
+
 void gem_sw_finish(int fd, uint32_t handle);
 
 bool gem_bo_busy(int fd, uint32_t handle);
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index 1f2cc96..ecad90e 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -64,17 +64,6 @@
 #define PAGE_SIZE 4096
 #endif
 
-#define LOCAL_I915_GEM_USERPTR       0x33
-#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
-struct local_i915_gem_userptr {
-	uint64_t user_ptr;
-	uint64_t user_size;
-	uint32_t flags;
-#define LOCAL_I915_USERPTR_READ_ONLY (1<<0)
-#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (1<<31)
-	uint32_t handle;
-};
-
 static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
 
 #define WIDTH 512
@@ -92,37 +81,6 @@ static void gem_userptr_test_synchronized(void)
 	userptr_flags = 0;
 }
 
-static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t *handle)
-{
-	struct local_i915_gem_userptr userptr;
-	int ret;
-
-	memset(&userptr, 0, sizeof(userptr));
-	userptr.user_ptr = (uintptr_t)ptr;
-	userptr.user_size = size;
-	userptr.flags = userptr_flags;
-	if (read_only)
-		userptr.flags |= LOCAL_I915_USERPTR_READ_ONLY;
-
-	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, &userptr);
-	if (ret)
-		ret = errno;
-	igt_skip_on_f(ret == ENODEV &&
-		      (userptr_flags & LOCAL_I915_USERPTR_UNSYNCHRONIZED) == 0 &&
-		      !read_only,
-		      "Skipping, synchronized mappings with no kernel CONFIG_MMU_NOTIFIER?");
-	if (ret == 0)
-		*handle = userptr.handle;
-
-	return ret;
-}
-
-
-static void gem_userptr_sync(int fd, uint32_t handle)
-{
-	gem_set_domain(fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-}
-
 static void
 copy(int fd, uint32_t dst, uint32_t src, unsigned int error)
 {
@@ -294,7 +252,7 @@ create_userptr(int fd, uint32_t val, uint32_t *ptr)
 	uint32_t handle;
 	int i, ret;
 
-	ret = gem_userptr(fd, ptr, sizeof(linear), 0, &handle);
+	ret = gem_userptr(fd, ptr, sizeof(linear), 0, userptr_flags, &handle);
 	igt_assert_eq(ret, 0);
 	igt_assert(handle != 0);
 
@@ -374,7 +332,7 @@ static uint32_t create_userptr_bo(int fd, uint64_t size)
 		   -1, 0);
 	igt_assert(ptr != MAP_FAILED);
 
-	ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, &handle);
+	ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, userptr_flags, &handle);
 	igt_assert_eq(ret, 0);
 	add_handle_ptr(handle, ptr, size);
 
@@ -453,7 +411,7 @@ static int has_userptr(int fd)
 	igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
 	oldflags = userptr_flags;
 	gem_userptr_test_unsynchronized();
-	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle);
 	userptr_flags = oldflags;
 	if (ret != 0) {
 		free(ptr);
@@ -512,7 +470,7 @@ static int test_access_control(int fd)
 
 		igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
 
-		ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
+		ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle);
 		if (ret == 0)
 			gem_close(fd, handle);
 		free(ptr);
@@ -530,7 +488,7 @@ static int test_invalid_null_pointer(int fd)
 	int ret;
 
 	/* NULL pointer. */
-	ret = gem_userptr(fd, NULL, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, NULL, PAGE_SIZE, 0, userptr_flags, &handle);
 	igt_assert_eq(ret, 0);
 
 	copy(fd, handle, handle, ~0); /* QQQ Precise errno? */
@@ -553,7 +511,7 @@ static int test_invalid_gtt_mapping(int fd)
 	igt_assert(((unsigned long)ptr & (PAGE_SIZE - 1)) == 0);
 	igt_assert((sizeof(linear) & (PAGE_SIZE - 1)) == 0);
 
-	ret = gem_userptr(fd, ptr, sizeof(linear), 0, &handle2);
+	ret = gem_userptr(fd, ptr, sizeof(linear), 0, userptr_flags, &handle2);
 	igt_assert_eq(ret, 0);
 	copy(fd, handle2, handle2, ~0); /* QQQ Precise errno? */
 	gem_close(fd, handle2);
@@ -597,7 +555,7 @@ static void test_forked_access(int fd)
 #ifdef MADV_DONTFORK
 	ret |= madvise(ptr1, sizeof(linear), MADV_DONTFORK);
 #endif
-	ret |= gem_userptr(fd, ptr1, sizeof(linear), 0, &handle1);
+	ret |= gem_userptr(fd, ptr1, sizeof(linear), 0, userptr_flags, &handle1);
 	igt_assert_eq(ret, 0);
 	igt_assert(ptr1);
 	igt_assert(handle1);
@@ -606,7 +564,7 @@ static void test_forked_access(int fd)
 #ifdef MADV_DONTFORK
 	ret |= madvise(ptr2, sizeof(linear), MADV_DONTFORK);
 #endif
-	ret |= gem_userptr(fd, ptr2, sizeof(linear), 0, &handle2);
+	ret |= gem_userptr(fd, ptr2, sizeof(linear), 0, userptr_flags, &handle2);
 	igt_assert_eq(ret, 0);
 	igt_assert(ptr2);
 	igt_assert(handle2);
@@ -654,7 +612,7 @@ static int test_forbidden_ops(int fd)
 
 	igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
 
-	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle);
 	igt_assert_eq(ret, 0);
 
 	/* pread/pwrite are not always forbidden, but when they
@@ -841,19 +799,19 @@ static int test_usage_restrictions(int fd)
 	igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE * 2) == 0);
 
 	/* Address not aligned. */
-	ret = gem_userptr(fd, (char *)ptr + 1, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, (char *)ptr + 1, PAGE_SIZE, 0, userptr_flags, &handle);
 	igt_assert_neq(ret, 0);
 
 	/* Size not rounded to page size. */
-	ret = gem_userptr(fd, ptr, PAGE_SIZE - 1, 0, &handle);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE - 1, 0, userptr_flags, &handle);
 	igt_assert_neq(ret, 0);
 
 	/* Both wrong. */
-	ret = gem_userptr(fd, (char *)ptr + 1, PAGE_SIZE - 1, 0, &handle);
+	ret = gem_userptr(fd, (char *)ptr + 1, PAGE_SIZE - 1, 0, userptr_flags, &handle);
 	igt_assert_neq(ret, 0);
 
 	/* Read-only not supported. */
-	ret = gem_userptr(fd, (char *)ptr, PAGE_SIZE, 1, &handle);
+	ret = gem_userptr(fd, (char *)ptr, PAGE_SIZE, 1, userptr_flags, &handle);
 	igt_assert_neq(ret, 0);
 
 	free(ptr);
@@ -875,7 +833,7 @@ static int test_create_destroy(int fd, int time)
 		for (n = 0; n < 1000; n++) {
 			igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
 
-			do_or_die(gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle));
+			do_or_die(gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle));
 
 			gem_close(fd, handle);
 			free(ptr);
@@ -1067,41 +1025,41 @@ static void test_overlap(int fd, int expected)
 
 	igt_assert(posix_memalign((void *)&ptr, PAGE_SIZE, PAGE_SIZE * 3) == 0);
 
-	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE, 0, &handle);
+	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE, 0, userptr_flags, &handle);
 	igt_assert_eq(ret, 0);
 
 	/* before, no overlap */
-	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle2);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert_eq(ret, 0);
 
 	/* after, no overlap */
-	ret = gem_userptr(fd, ptr + PAGE_SIZE * 2, PAGE_SIZE, 0, &handle2);
+	ret = gem_userptr(fd, ptr + PAGE_SIZE * 2, PAGE_SIZE, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert_eq(ret, 0);
 
 	/* exactly overlapping */
-	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE, 0, &handle2);
+	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert(ret == 0 || ret == expected);
 
 	/* start overlaps */
-	ret = gem_userptr(fd, ptr, PAGE_SIZE * 2, 0, &handle2);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE * 2, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert(ret == 0 || ret == expected);
 
 	/* end overlaps */
-	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE * 2, 0, &handle2);
+	ret = gem_userptr(fd, ptr + PAGE_SIZE, PAGE_SIZE * 2, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert(ret == 0 || ret == expected);
 
 	/* subsumes */
-	ret = gem_userptr(fd, ptr, PAGE_SIZE * 3, 0, &handle2);
+	ret = gem_userptr(fd, ptr, PAGE_SIZE * 3, 0, userptr_flags, &handle2);
 	if (ret == 0)
 		gem_close(fd, handle2);
 	igt_assert(ret == 0 || ret == expected);
@@ -1126,7 +1084,7 @@ static void test_unmap(int fd, int expected)
 	bo_ptr = (char *)ALIGN((unsigned long)ptr, PAGE_SIZE);
 
 	for (i = 0; i < num_obj; i++, bo_ptr += sizeof(linear)) {
-		ret = gem_userptr(fd, bo_ptr, sizeof(linear), 0, &bo[i]);
+		ret = gem_userptr(fd, bo_ptr, sizeof(linear), 0, userptr_flags, &bo[i]);
 		igt_assert_eq(ret, 0);
 	}
 
@@ -1161,7 +1119,7 @@ static void test_unmap_after_close(int fd)
 	bo_ptr = (char *)ALIGN((unsigned long)ptr, PAGE_SIZE);
 
 	for (i = 0; i < num_obj; i++, bo_ptr += sizeof(linear)) {
-		ret = gem_userptr(fd, bo_ptr, sizeof(linear), 0, &bo[i]);
+		ret = gem_userptr(fd, bo_ptr, sizeof(linear), 0, userptr_flags, &bo[i]);
 		igt_assert_eq(ret, 0);
 	}
 
@@ -1232,7 +1190,7 @@ static void test_stress_mm(int fd)
 	igt_assert_eq(ret, 0);
 
 	while (loops--) {
-		ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
+		ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags, &handle);
 		igt_assert_eq(ret, 0);
 
 		gem_close(fd, handle);
@@ -1267,7 +1225,7 @@ static void *mm_userptr_close_thread(void *data)
 	while (!t->stop) {
 		pthread_mutex_unlock(&t->mutex);
 		for (int i = 0; i < num_handles; i++)
-			igt_assert_eq(gem_userptr(t->fd, t->ptr, PAGE_SIZE, 0, &handle[i]),
+			igt_assert_eq(gem_userptr(t->fd, t->ptr, PAGE_SIZE, 0, userptr_flags, &handle[i]),
 				      0);
 		for (int i = 0; i < num_handles; i++)
 			gem_close(t->fd, handle[i]);
-- 
2.1.0

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

Re: [PATCH 4/7] lib: Add gem_userptr helpers

[Chris Wilson]

On Wed, Aug 12, 2015 at 08:29:17PM -0300, Tiago Vignatti wrote:
> This patch moves userptr definitions and helpers implementation that were
> locally in gem_userptr_benchmark and gem_userptr_blits to the library, so other
> tests can make use of them as well. There's no functional changes.

We have a pattern in the helpers to use
gem_userptr() as a no-fail (failures are detected and asserted upon
inside the handler) and __gem_userptr() where the errno is passed back
to the caller. Commonly when writing tests it is simpler to just get the
handle back (and have the test fail appropriately otherwise) and doing
negative parameter testing is rarer and so delegated to the
__gem_ variant.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] lib: Add gem_userptr helpers

[Daniel Vetter]

On Sun, Aug 23, 2015 at 01:10:42PM +0100, Chris Wilson wrote:
> On Wed, Aug 12, 2015 at 08:29:17PM -0300, Tiago Vignatti wrote:
> > This patch moves userptr definitions and helpers implementation that were
> > locally in gem_userptr_benchmark and gem_userptr_blits to the library, so other
> > tests can make use of them as well. There's no functional changes.
> 
> We have a pattern in the helpers to use
> gem_userptr() as a no-fail (failures are detected and asserted upon
> inside the handler) and __gem_userptr() where the errno is passed back
> to the caller. Commonly when writing tests it is simpler to just get the
> handle back (and have the test fail appropriately otherwise) and doing
> negative parameter testing is rarer and so delegated to the
> __gem_ variant.

Also please add gtkdoc when adding new wrappers (and generally those doc
comments mention the distinction Chris raised).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] prime_mmap: Test for userptr mmap

[Tiago Vignatti]

A userptr doesn't have the obj->base.filp, but can be exported via dma-buf, so
make sure it fails when mmaping.

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
In machine, export the handle to fd is actually returning error and falling
before the actual test happens. Same issue happens in gem_userptr_blits's
test_dmabuf(). This patch needs to be tested properly therefore.

 tests/prime_mmap.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index ad91371..fd6d13b 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -299,12 +299,47 @@ static int prime_handle_to_fd_no_assert(uint32_t handle, int *fd_out)
 	args.fd = -1;
 
 	ret = drmIoctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
-
+	if (ret)
+		ret = errno;
 	*fd_out = args.fd;
 
 	return ret;
 }
 
+/* test for mmap(dma_buf_export(userptr)) */
+static void
+test_userptr(void)
+{
+	int ret, dma_buf_fd;
+	void *ptr;
+	uint32_t handle;
+
+	/* create userptr bo */
+	ret = posix_memalign(&ptr, 4096, BO_SIZE);
+	igt_assert_eq(ret, 0);
+
+	ret = gem_userptr(fd, (uint32_t *)ptr, BO_SIZE, 0, LOCAL_I915_USERPTR_UNSYNCHRONIZED, &handle);
+	igt_assert_eq(ret, 0);
+
+	/* export userptr */
+	ret = prime_handle_to_fd_no_assert(handle, &dma_buf_fd);
+	if (ret) {
+		igt_assert(ret == EINVAL || ret == ENODEV);
+		goto free_userptr;
+	} else {
+		igt_assert_eq(ret, 0);
+		igt_assert_lte(0, dma_buf_fd);
+	}
+
+	/* a userptr doesn't have the obj->base.filp, but can be exported via
+	 * dma-buf, so make sure it fails here */
+	ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr == MAP_FAILED && errno == ENODEV);
+free_userptr:
+	gem_close(fd, handle);
+	close(dma_buf_fd);
+}
+
 static void
 test_errors(void)
 {
@@ -413,6 +448,7 @@ igt_main
 		{ "test_forked_cpu_write", test_forked_cpu_write },
 		{ "test_refcounting", test_refcounting },
 		{ "test_dup", test_dup },
+		{ "test_userptr", test_userptr },
 		{ "test_errors", test_errors },
 		{ "test_aperture_limit", test_aperture_limit },
 	};
-- 
2.1.0

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

Re: [PATCH 5/7] prime_mmap: Test for userptr mmap

[Chris Wilson]

On Wed, Aug 12, 2015 at 08:29:18PM -0300, Tiago Vignatti wrote:
> A userptr doesn't have the obj->base.filp, but can be exported via dma-buf, so
> make sure it fails when mmaping.
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
> ---
> In machine, export the handle to fd is actually returning error and falling
> before the actual test happens. Same issue happens in gem_userptr_blits's
> test_dmabuf(). This patch needs to be tested properly therefore.

Yes, you are not allowed to export unsynchronized userptr. Just create a
normal one.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] tests: Add kms_mmap_write_crc for cache coherency tests

[Tiago Vignatti]

This program can be used to detect when the writes don't land in scanout due
cache incoherency. Although this seems a problem inherently of non-LCC machines
("Atom"), this particular test catches a cache dirt on scanout on LLC machines
as well. It's inspired in Ville's kms_pwrite_crc.c and can be used also to test
the correctness of the driver's begin_cpu_access (TODO end_cpu_access).

To see the need for flush, one has to run this same binary a few times cause
it's not 100% reproducible (in my Core machine it's always possible to catch
the problem by running it at most 5 times).

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 tests/.gitignore           |   1 +
 tests/Makefile.sources     |   1 +
 tests/kms_mmap_write_crc.c | 236 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 tests/kms_mmap_write_crc.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 5bc4a58..9ba1e48 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -140,6 +140,7 @@ kms_force_connector
 kms_frontbuffer_tracking
 kms_legacy_colorkey
 kms_mmio_vs_cs_flip
+kms_mmap_write_crc
 kms_pipe_b_c_ivb
 kms_pipe_crc_basic
 kms_plane
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 5b2072e..31c5508 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -163,6 +163,7 @@ TESTS_progs = \
 	kms_3d \
 	kms_fence_pin_leak \
 	kms_force_connector \
+	kms_mmap_write_crc \
 	kms_pwrite_crc \
 	kms_sink_crc_basic \
 	prime_udl \
diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
new file mode 100644
index 0000000..e24d535
--- /dev/null
+++ b/tests/kms_mmap_write_crc.c
@@ -0,0 +1,236 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Tiago Vignatti <tiago.vignatti at intel.com>
+ */
+
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "ioctl_wrappers.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION(
+   "Use the display CRC support to validate mmap write to an already uncached future scanout buffer.");
+
+typedef struct {
+	int drm_fd;
+	igt_display_t display;
+	struct igt_fb fb[2];
+	igt_output_t *output;
+	igt_plane_t *primary;
+	enum pipe pipe;
+	igt_crc_t ref_crc;
+	igt_pipe_crc_t *pipe_crc;
+	uint32_t devid;
+} data_t;
+
+int dma_buf_fd;
+
+static char *dmabuf_mmap_framebuffer(int drm_fd, struct igt_fb *fb)
+{
+	char *ptr = NULL;
+
+	dma_buf_fd = prime_handle_to_fd(drm_fd, fb->gem_handle);
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, fb->size, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+
+	return ptr;
+}
+
+static void test_begin_access(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	struct igt_fb *fb = &data->fb[1];
+	drmModeModeInfo *mode;
+	cairo_t *cr;
+	char *ptr;
+	uint32_t caching;
+	void *buf;
+	igt_crc_t crc;
+
+	mode = igt_output_get_mode(output);
+
+	/* create a non-white fb where we can write later */
+	igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, fb);
+
+	ptr = dmabuf_mmap_framebuffer(data->drm_fd, fb);
+
+	cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	igt_paint_test_pattern(cr, fb->width, fb->height);
+	cairo_destroy(cr);
+
+	/* flip to it to make it UC/WC and fully flushed */
+	igt_plane_set_fb(data->primary, fb);
+	igt_display_commit(display);
+
+	/* flip back the original white buffer */
+	igt_plane_set_fb(data->primary, &data->fb[0]);
+	igt_display_commit(display);
+
+	/* make sure caching mode has become UC/WT */
+	caching = gem_get_caching(data->drm_fd, fb->gem_handle);
+	igt_assert(caching == I915_CACHING_NONE || caching == I915_CACHING_DISPLAY);
+
+	// Uncomment the following for flush and the crc check next passes. It
+	// requires the kernel counter-part of it implemented obviously.
+	// {
+	// struct dma_buf_sync sync_start;
+	// memset(&sync_start, 0, sizeof(sync_start));
+	// sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
+	// do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_start);
+	// }
+
+	/* use dmabuf pointer to make the other fb all white too */
+	buf = malloc(fb->size);
+	igt_assert(buf != NULL);
+	memset(buf, 0xff, fb->size);
+	memcpy(ptr, buf, fb->size);
+	free(buf);
+
+	/* and flip to it */
+	igt_plane_set_fb(data->primary, fb);
+	igt_display_commit(display);
+
+	/* check that the crc is as expected, which requires that caches got flushed */
+	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+	igt_assert_crc_equal(&crc, &data->ref_crc);
+}
+
+static bool prepare_crtc(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	drmModeModeInfo *mode;
+
+	/* select the pipe we want to use */
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	if (!output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	mode = igt_output_get_mode(output);
+
+	/* create a white reference fb and flip to it */
+	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+			    1.0, 1.0, 1.0, &data->fb[0]);
+
+	data->primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+
+	igt_plane_set_fb(data->primary, &data->fb[0]);
+	igt_display_commit(display);
+
+	if (data->pipe_crc)
+		igt_pipe_crc_free(data->pipe_crc);
+
+	data->pipe_crc = igt_pipe_crc_new(data->pipe,
+					  INTEL_PIPE_CRC_SOURCE_AUTO);
+
+	/* get reference crc for the white fb */
+	igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
+
+	return true;
+}
+
+static void cleanup_crtc(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+
+	igt_pipe_crc_free(data->pipe_crc);
+	data->pipe_crc = NULL;
+
+	igt_plane_set_fb(data->primary, NULL);
+
+	igt_output_set_pipe(output, PIPE_ANY);
+	igt_display_commit(display);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+	igt_remove_fb(data->drm_fd, &data->fb[1]);
+}
+
+static void run_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+
+	for_each_connected_output(display, output) {
+		data->output = output;
+		for_each_pipe(display, pipe) {
+			data->pipe = pipe;
+
+			if (!prepare_crtc(data))
+				continue;
+
+			test_begin_access(data);
+			cleanup_crtc(data);
+
+			/* once is enough */
+			return;
+		}
+	}
+
+	igt_skip("no valid crtc/connector combinations found\n");
+}
+
+static data_t data;
+
+igt_simple_main
+{
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		data.drm_fd = drm_open_any_master();
+
+		data.devid = intel_get_drm_devid(data.drm_fd);
+
+		kmstest_set_vt_graphics_mode();
+
+		igt_require_pipe_crc();
+
+		igt_display_init(&data.display, data.drm_fd);
+	}
+
+	run_test(&data);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+	}
+}
-- 
2.1.0

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

[PATCH 7/7] tests/kms_mmap_write_crc: Demonstrate the need for end_cpu_access

[Tiago Vignatti]

It requires i915 changes to add end_cpu_access().

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 tests/kms_mmap_write_crc.c | 63 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 8 deletions(-)

diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
index e24d535..59ac9e7 100644
--- a/tests/kms_mmap_write_crc.c
+++ b/tests/kms_mmap_write_crc.c
@@ -67,6 +67,24 @@ static char *dmabuf_mmap_framebuffer(int drm_fd, struct igt_fb *fb)
 	return ptr;
 }
 
+static void dmabuf_sync_start(void)
+{
+	struct dma_buf_sync sync_start;
+
+	memset(&sync_start, 0, sizeof(sync_start));
+	sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
+	do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_start);
+}
+
+static void dmabuf_sync_end(void)
+{
+	struct dma_buf_sync sync_end;
+
+	memset(&sync_end, 0, sizeof(sync_end));
+	sync_end.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_RW;
+	do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_end);
+}
+
 static void test_begin_access(data_t *data)
 {
 	igt_display_t *display = &data->display;
@@ -103,14 +121,11 @@ static void test_begin_access(data_t *data)
 	caching = gem_get_caching(data->drm_fd, fb->gem_handle);
 	igt_assert(caching == I915_CACHING_NONE || caching == I915_CACHING_DISPLAY);
 
-	// Uncomment the following for flush and the crc check next passes. It
-	// requires the kernel counter-part of it implemented obviously.
-	// {
-	// struct dma_buf_sync sync_start;
-	// memset(&sync_start, 0, sizeof(sync_start));
-	// sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
-	// do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_start);
-	// }
+	/*
+	 * firstly demonstrate the need for DMA_BUF_SYNC_START ("begin_cpu_access")
+	 */
+
+	dmabuf_sync_start();
 
 	/* use dmabuf pointer to make the other fb all white too */
 	buf = malloc(fb->size);
@@ -126,6 +141,38 @@ static void test_begin_access(data_t *data)
 	/* check that the crc is as expected, which requires that caches got flushed */
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
 	igt_assert_crc_equal(&crc, &data->ref_crc);
+
+	/*
+	 * now demonstrates the need for DMA_BUF_SYNC_END ("end_cpu_access")
+	 */
+
+	/* start over, writing non-white to the fb again and flip to it to make it
+	 * fully flushed */
+	cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	igt_paint_test_pattern(cr, fb->width, fb->height);
+	cairo_destroy(cr);
+
+	igt_plane_set_fb(data->primary, fb);
+	igt_display_commit(display);
+
+	/* sync start, to move to CPU domain */
+	dmabuf_sync_start();
+
+	/* use dmabuf pointer in the same fb to make it all white */
+	buf = malloc(fb->size);
+	igt_assert(buf != NULL);
+	memset(buf, 0xff, fb->size);
+	memcpy(ptr, buf, fb->size);
+	free(buf);
+
+	/* there's an implicit flush in set_fb() as well (to set to the GTT domain),
+	 * so if we don't do it and instead write directly into the fb as it is the
+	 * scanout, that should demonstrate the need for end_cpu_access */
+	dmabuf_sync_end();
+
+	/* check that the crc is as expected, which requires that caches got flushed */
+	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+	igt_assert_crc_equal(&crc, &data->ref_crc);
 }
 
 static bool prepare_crtc(data_t *data)
-- 
2.1.0

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

Re: [PATCH v4] mmap on the dma-buf directly

[Thomas Hellstrom]

On 08/13/2015 01:29 AM, Tiago Vignatti wrote:
> Hi,
>
> The idea is to create a GEM bo in one process and pass the prime handle of the
> it to another process, which in turn uses the handle only to map and write.
> This could be useful for Chrome OS architecture, where the Web content
> ("unpriviledged process") maps and CPU-draws a buffer, which was previously
> allocated in the GPU process ("priviledged process").
>
> In v2, I've added a patch that Daniel kindly drafted to allow the
> unpriviledged process flush through a prime fd. In v3, I've fixed a few
> concerns and then added end_cpu_access to i915. In v4, I fixed Sumit Semwal's
> concerns about dma-duf documentation and the FIXME missing in that same patch,
> and also removed WARN in i915 dma-buf mmap (pointed by Chris). PTAL.
>
> Best Regards,
>
> Tiago
Tiago,

I take it, this is intended to be a generic interface used mostly for 2D
rendering.

In that case, using SYNC is crucial for performance of incoherent
architectures and I'd rather see it mandatory than an option. It could
perhaps be made mandatory preferrably using an error or a one-time
kernel warning. If nobody uses the SYNC interface, it is of little use.

Also I think the syncing needs to be extended to two dimensions. A long
time ago when this was brought up people argued why we should limit it
to two dimensions, but I believe two dimensions addresses most
performance-problematic use-cases. A default implementation of
twodimensional sync can easily be made using the one-dimensional API.

Thanks,
Thomas






>
>
> Daniel Thompson (1):
>   drm: prime: Honour O_RDWR during prime-handle-to-fd
>
> Daniel Vetter (1):
>   dma-buf: Add ioctls to allow userspace to flush
>
> Tiago Vignatti (2):
>   drm/i915: Implement end_cpu_access
>   drm/i915: Use CPU mapping for userspace dma-buf mmap()
>
>  Documentation/dma-buf-sharing.txt      | 12 ++++++++
>  drivers/dma-buf/dma-buf.c              | 50 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_prime.c            | 10 ++-----
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 28 ++++++++++++++++++-
>  include/uapi/drm/drm.h                 |  1 +
>  include/uapi/linux/dma-buf.h           | 43 +++++++++++++++++++++++++++++
>  6 files changed, 136 insertions(+), 8 deletions(-)
>  create mode 100644 include/uapi/linux/dma-buf.h
>


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

Re: [PATCH v4] mmap on the dma-buf directly

[Tiago Vignatti]

On 08/13/2015 08:09 AM, Thomas Hellstrom wrote:
> Tiago,
>
> I take it, this is intended to be a generic interface used mostly for 2D
> rendering.

yup. "generic" is an important point that I've actually forgot to 
mention in the description, which is probably the whole motivation for 
bringing this up.

We want avoid link any vendor-specific library to the unpriviledged 
process for security reasons, so it's a requirement to it not have 
access to driver-specific ioctls when mapping the buffers. The use-case 
for it is texturing from CPU rendered buffer, like you said, with the 
intention of passing these buffers among processes without performing 
any copy in the user-space ("zero-copy").

> In that case, using SYNC is crucial for performance of incoherent
> architectures and I'd rather see it mandatory than an option. It could
> perhaps be made mandatory preferrably using an error or a one-time
> kernel warning. If nobody uses the SYNC interface, it is of little use.

hmm I'm not sure it is little use. Our hardware (the "LLC" capable) has 
this very specific case where the cache gets dirty wrt the GPU, which is 
when the same buffer is shared with the scanout device. This is not 
something will happen in Chrome OS for example, so we wouldn't need the 
SYNC markers there.

In any case I think that making it mandatory works for us, but I'll have 
to check with Daniel/Chris whether there are performance penalties when 
accessing it and so on.

> Also I think the syncing needs to be extended to two dimensions. A long
> time ago when this was brought up people argued why we should limit it
> to two dimensions, but I believe two dimensions addresses most
> performance-problematic use-cases. A default implementation of
> twodimensional sync can easily be made using the one-dimensional API.

two dimension sync? You're saying that there could be a GPU access API 
in dma-buf as well? I don't get it, what's the use-case? I'm sure I 
missed the discussions because I wasn't particularly interested in this 
whole thingy before :)

Thanks for reviewing, Thomas.

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

Re: [PATCH v4] mmap on the dma-buf directly

[Daniel Vetter]

On Thu, Aug 13, 2015 at 11:09:07AM -0300, Tiago Vignatti wrote:
> On 08/13/2015 08:09 AM, Thomas Hellstrom wrote:
> >Tiago,
> >
> >I take it, this is intended to be a generic interface used mostly for 2D
> >rendering.
> 
> yup. "generic" is an important point that I've actually forgot to mention in
> the description, which is probably the whole motivation for bringing this
> up.
> 
> We want avoid link any vendor-specific library to the unpriviledged process
> for security reasons, so it's a requirement to it not have access to
> driver-specific ioctls when mapping the buffers. The use-case for it is
> texturing from CPU rendered buffer, like you said, with the intention of
> passing these buffers among processes without performing any copy in the
> user-space ("zero-copy").
> 
> >In that case, using SYNC is crucial for performance of incoherent
> >architectures and I'd rather see it mandatory than an option. It could
> >perhaps be made mandatory preferrably using an error or a one-time
> >kernel warning. If nobody uses the SYNC interface, it is of little use.
> 
> hmm I'm not sure it is little use. Our hardware (the "LLC" capable) has this
> very specific case where the cache gets dirty wrt the GPU, which is when the
> same buffer is shared with the scanout device. This is not something will
> happen in Chrome OS for example, so we wouldn't need the SYNC markers there.
> 
> In any case I think that making it mandatory works for us, but I'll have to
> check with Daniel/Chris whether there are performance penalties when
> accessing it and so on.

2 more ioctls per upload should be bearable, imo we should make this
mandatory.

> >Also I think the syncing needs to be extended to two dimensions. A long
> >time ago when this was brought up people argued why we should limit it
> >to two dimensions, but I believe two dimensions addresses most
> >performance-problematic use-cases. A default implementation of
> >twodimensional sync can easily be made using the one-dimensional API.
> 
> two dimension sync? You're saying that there could be a GPU access API in
> dma-buf as well? I don't get it, what's the use-case? I'm sure I missed the
> discussions because I wasn't particularly interested in this whole thingy
> before :)

Most of the things I've seen that use subranges for up/download use linear
buffers where individual images are just packed in a queue, each with
their own stride. Adding a notion of 2d to dma-buf would be a big
departure from the "dma-buf doesn't track metadata" design. If there's a
real need I guess we can do it, but only after careful consideration, and
imo it shouldn't be in basic dma-buf mmap support.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx