Re: [igt-dev] [PATCH i-g-t v3 3/4] lib/ioctl_wrapper: Implement __gem_mmap

Daniele Ceraolo Spurio
2019-01-11T17:39:18Z

On 01/11/2019 08:15 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.
> v2: Reordered and splited this patch into two separete patches

I think it make more sense for this to be a single patch together with
the next one. Easier to review as well.

> v3: Dropped unnecessary check
>
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> lib/ioctl_wrappers.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index f71f0e32..e9225da0 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -736,6 +736,42 @@ 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()
> + * @flags: flags used to determine caching
> + *
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu (valid for wc == false). For wc == true

This is a bit unclear IMO, as it sounds like the bypassing the GPU is
only valid for wc == false, which isn't true since WC also bypasses the
gpu caches.

Also, instead of using wc == true/false I would say "wc mmappings" and
"!wc mmappings" or something like that.

Daniele

> + * 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.
> + */
> +static void
> +*__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned int prot, uint64_t flags)
> +{
> + struct drm_i915_gem_mmap arg;
> +
> + memset(&arg, 0, sizeof(arg));
> + arg.handle = handle;
> + arg.offset = offset;
> + arg.size = size;
> + arg.flags = flags;
> +
> + 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