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
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;
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
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,
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
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)
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
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); +}
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
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
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
- 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);
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 },
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 },-- 2.1.0
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 },-- 2.1.0
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 },-- 2.1.0
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]);
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
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
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 }, };
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
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); + } +}
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)
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
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
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
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter -
Thomas Hellstrom -
Tiago Vignatti