[Intel-gfx] [PATCH] drm/i915/gem/mman: only allow WC for lmem

[Thomas Hellström]

Hi,

On 6/2/21 11:36 AM, Matthew Auld wrote:
> For dgfx where we now have lmem and ttm, we can only support single mmap
> mode for the lifetime of the object, and for lmem objects this should be
> WC, so reject all other mapping modes for mmap_offset, including if the
> object can be placed in both smem and lmem.
>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_mman.c   |  4 ++++
>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 22 ++++++++++++++++++++++
>   drivers/gpu/drm/i915/gem/i915_gem_object.h |  4 ++++
>   3 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index fd1c9714f8d8..32f88f236771 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -689,6 +689,10 @@ __assign_mmap_offset(struct drm_file *file,
>   		goto out;
>   	}
>   
> +	if (mmap_type != I915_MMAP_TYPE_WC &&
> +	    i915_gem_object_placements_contain_type(obj, INTEL_MEMORY_LOCAL))
> +		return -ENODEV;
> +

I think we will also have the restriction that any other objects on DGFX 
can only be mapped cached? At least that's what the TTM code is 
implementing currently.


>   	mmo = mmap_offset_attach(obj, mmap_type, file);
>   	if (IS_ERR(mmo)) {
>   		err = PTR_ERR(mmo);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 2be6109d0093..d4b0da8ed969 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -403,6 +403,28 @@ int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64 offset,
>   	return 0;
>   }
>   
> +/**
> + * i915_gem_object_placements_contain_type - Check whether the object can be
> + * placed at certain memory type
> + * @obj: Pointer to the object
> + * @type: The memory type to check
> + *
> + * Return: True if the object can be placed in @type. False otherwise.
> + */
> +bool i915_gem_object_placements_contain_type(struct drm_i915_gem_object *obj,
> +					     enum intel_memory_type type)
> +{
> +	unsigned int i;
> +
> +	/* TODO: consider maybe storing as a mask when doing gem_create_ext */
> +	for (i = 0; i < obj->mm.n_placements; i++) {
> +		if (obj->mm.placements[i]->type == type)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +

Do we need something for the in-kernel mappings as well? Or just return 
a mapping with the only allowed caching mode?

/Thomas