[igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Jan 9 18:04:39 UTC 2019
On 01/09/2019 09:40 AM, Lukasz Kalamarz wrote:
> Previous implementation of __gem_mmap__cpu and __gem_mmap_wc only
> differ with setting proper flag for caching. This patch implement __gem_mmap
> which merge those two functions into one wrapper.
> Small documentation modification are add to be in par with new implementation.
>
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> Cc: Katarzyna Dec <katarzyna.dec at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
> lib/ioctl_wrappers.c | 96 +++++++++++++++++++++++++++++++-------------
> lib/ioctl_wrappers.h | 1 +
> 2 files changed, 68 insertions(+), 29 deletions(-)
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index f71f0e32..87744edc 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -736,6 +736,45 @@ bool gem_mmap__has_wc(int fd)
> return has_wc > 0;
> }
>
> +/**
> + * __gem_mmap:
> + * @fd: open i915 drm file descriptor
> + * @handle: gem buffer object handle
> + * @offset: offset in the gem buffer of the mmap arena
> + * @size: size of the mmap arena
> + * @prot: memory protection bits as used by mmap()
> + * @wc: flag marking if we want to set up MMAP_WC flag
> + *
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu (valid for wc == false). For wc == true
> + * it also bypass cpu caches completely and GTT system agent (i.e. there is no
> + * automatic tiling of the mmapping through the fence registers).
> + *
> + * Returns: A pointer to the created memory mapping, NULL on failure.
> + */
> +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc)
This shouldn't be called from outside this file, so you can just make it
static.
> +{
> + struct drm_i915_gem_mmap arg;
> +
> + if (wc & !gem_mmap__has_wc(fd)) {
> + errno = ENOSYS;
> + return NULL;
> + }
> +
> + memset(&arg, 0, sizeof(arg));
> + arg.handle = handle;
> + arg.offset = offset;
> + arg.size = size;
> + arg.flags = wc ? I915_MMAP_WC : 0;
> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
> + return NULL;
> +
> + VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
> +
> + errno = 0;
> + return from_user_pointer(arg.addr_ptr);
> +}
> +
> /**
> * __gem_mmap__wc:
> * @fd: open i915 drm file descriptor
> @@ -753,25 +792,20 @@ bool gem_mmap__has_wc(int fd)
> */
> void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> {
> - struct drm_i915_gem_mmap arg;
> + struct drm_i915_gem_mmap mmap_arg;
>
Any reason not to just do:
void *__gem_mmap__wc(...)
{
return __gem_mmap(..., true);
}
?
> - if (!gem_mmap__has_wc(fd)) {
> - errno = ENOSYS;
> - return NULL;
> - }
> + memset(&mmap_arg, 0, sizeof(mmap_arg));
> + mmap_arg.handle = handle;
> + mmap_arg.offset = offset;
> + mmap_arg.size = size;
> + mmap_arg.flags = I915_MMAP_WC;
> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> + return NULL;
>
> - memset(&arg, 0, sizeof(arg));
> - arg.handle = handle;
> - arg.offset = offset;
> - arg.size = size;
> - arg.flags = I915_MMAP_WC;
> - if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
> - return NULL;
> + VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
>
> - VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
> -
> - errno = 0;
> - return from_user_pointer(arg.addr_ptr);
> + errno = 0;
> + return from_user_pointer(mmap_arg.addr_ptr);
Looks like you've changed all tabs to spaces here. Can just remove it
all if you follow the above suggestion.
> }
>
> /**
> @@ -782,13 +816,16 @@ void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, un
> * @size: size of the mmap arena
> * @prot: memory protection bits as used by mmap()
> *
> - * Like __gem_mmap__wc() except we assert on failure.
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu and cpu caches completely and also
> + * bypassing the GTT system agent (i.e. there is no automatic tiling of
> + * the mmapping through the fence registers).We assert on failure.
> *
> * Returns: A pointer to the created memory mapping
> */
> void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> {
> - void *ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
> + void *ptr = __gem_mmap(fd, handle, offset, size, prot, true);
If you follow the above suggestion for __gem_mmap__wc then there'll be
no need to update here.
> igt_assert(ptr);
> return ptr;
> }
> @@ -810,17 +847,17 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
The same suggestion I had for the __wc path apply for __cpu
Daniele
> {
> struct drm_i915_gem_mmap mmap_arg;
>
> - memset(&mmap_arg, 0, sizeof(mmap_arg));
> - mmap_arg.handle = handle;
> - mmap_arg.offset = offset;
> - mmap_arg.size = size;
> - if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> - return NULL;
> + memset(&mmap_arg, 0, sizeof(mmap_arg));
> + mmap_arg.handle = handle;
> + mmap_arg.offset = offset;
> + mmap_arg.size = size;
> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> + return NULL;
>
> - VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
> + VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
>
> - errno = 0;
> - return from_user_pointer(mmap_arg.addr_ptr);
> + errno = 0;
> + return from_user_pointer(mmap_arg.addr_ptr);
> }
>
> /**
> @@ -831,13 +868,14 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
> * @size: size of the mmap arena
> * @prot: memory protection bits as used by mmap()
> *
> - * Like __gem_mmap__cpu() except we assert on failure.
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu completely and we assert on failure.
> *
> * Returns: A pointer to the created memory mapping
> */
> void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> {
> - void *ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> + void *ptr = __gem_mmap(fd, handle, offset, size, prot, false);
> igt_assert(ptr);
> return ptr;
> }
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index b22b36b0..51e66c08 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -97,6 +97,7 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
> void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot);
> void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
> void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
> +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc);
>
> int gem_munmap(void *ptr, uint64_t size);
>
>
More information about the igt-dev
mailing list