[igt-dev] [PATCH i-g-t] lib/i915/gem_mman.c: add cpu coherency mapping wrapper

[Dixit, Ashutosh]

On Mon, 13 Jan 2020 21:27:28 -0800, Zbigniew Kempczyński wrote:
>
> For reduce code redundancy adding a wrapper for cpu memory mapping.
>
> +void *__gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset,
> +			       uint64_t size, unsigned prot)
> +{
> +	void *ptr = __gem_mmap_offset__cpu(fd, handle, offset, size, prot);
> +
> +	if (!ptr)
> +		ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> +
> +	return ptr;
> +}

We need similar wrappers for WC and GTT too. So why don't we put this code
in __gem_mmap__cpu() itself and we can do the same for __gem_mmap__wc() and
__gem_mmap__gtt() too? Otherwise what are we going to call those functions?
Something like:

void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
{
	if (gem_has_mmap_offset(fd))
		return __gem_mmap_offset(fd, handle, offset, size, prot, I915_MMAP_OFFSET_WB);
	else
		return __gem_mmap(fd, handle, offset, size, prot, 0);
}

So I am not sure of the point of introducing new wrappers, this code could
just be put in the old existing wrappers.

> +
> +/**
> + * gem_mmap__cpu_coherent:
> + * @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()
> + *
> + * Call __gem_mmap__cpu__coherent(), asserts on fail.
> + * Offset argument passed in function call must be 0. In the future
> + * when driver will allow slice mapping of buffer object this restriction
> + * will be removed.
> + *
> + * Returns: A pointer to the created memory mapping.
> + */
> +void *gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset,

This can just be gem_mmap__cpu()?

> +			     uint64_t size, unsigned prot)
> +{
> +	void *ptr;
> +
> +	igt_assert(offset == 0);

Not needed?