[Intel-gfx] [PATCH v2 09/37] drm/i915/lmem: support kernel mapping

[Chris Wilson]

Quoting Matthew Auld (2019-06-27 21:56:05)
> From: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> 
> We can create LMEM objects, but we also need to support mapping them
> into kernel space for internal use.
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 18 ++++-
>  drivers/gpu/drm/i915/intel_region_lmem.c      | 24 ++++++
>  drivers/gpu/drm/i915/intel_region_lmem.h      |  6 ++
>  .../drm/i915/selftests/intel_memory_region.c  | 77 +++++++++++++++++++
>  4 files changed, 121 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> index b36ad269f4ea..15eaaedffc46 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> @@ -176,7 +176,9 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
>                 void *ptr;
>  
>                 ptr = page_mask_bits(obj->mm.mapping);
> -               if (is_vmalloc_addr(ptr))
> +               if (i915_gem_object_is_lmem(obj))
> +                       io_mapping_unmap(ptr);
> +               else if (is_vmalloc_addr(ptr))
>                         vunmap(ptr);
>                 else
>                         kunmap(kmap_to_page(ptr));
> @@ -235,7 +237,7 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
>  }
>  
>  /* The 'mapping' part of i915_gem_object_pin_map() below */
> -static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
> +static void *i915_gem_object_map(struct drm_i915_gem_object *obj,
>                                  enum i915_map_type type)
>  {
>         unsigned long n_pages = obj->base.size >> PAGE_SHIFT;
> @@ -248,6 +250,11 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
>         pgprot_t pgprot;
>         void *addr;
>  
> +       if (i915_gem_object_is_lmem(obj)) {
> +               /* XXX: we are ignoring the type here -- this is simply wc */

Yeah, don't. The callers certainly do not expect that.

> +               return i915_gem_object_lmem_io_map(obj, 0, obj->base.size);
> +       }
> +
>         /* A single page can always be kmapped */
>         if (n_pages == 1 && type == I915_MAP_WB)
>                 return kmap(sg_page(sgt->sgl));
> @@ -293,7 +300,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
>         void *ptr;
>         int err;
>  
> -       if (unlikely(!i915_gem_object_has_struct_page(obj)))
> +       if (unlikely(!i915_gem_object_has_struct_page(obj) &&
> +                    !i915_gem_object_is_lmem(obj)))

Redefine the feature bit in the obj->ops->flags.

>                 return ERR_PTR(-ENXIO);
>  
>         err = mutex_lock_interruptible(&obj->mm.lock);
> @@ -325,7 +333,9 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
>                         goto err_unpin;
>                 }
>  
> -               if (is_vmalloc_addr(ptr))
> +               if (i915_gem_object_is_lmem(obj))
> +                       io_mapping_unmap(ptr);
> +               else if (is_vmalloc_addr(ptr))
>                         vunmap(ptr);
>                 else
>                         kunmap(kmap_to_page(ptr));
> diff --git a/drivers/gpu/drm/i915/intel_region_lmem.c b/drivers/gpu/drm/i915/intel_region_lmem.c
> index 15655cc5013f..701bcac3479e 100644
> --- a/drivers/gpu/drm/i915/intel_region_lmem.c
> +++ b/drivers/gpu/drm/i915/intel_region_lmem.c
> @@ -73,6 +73,30 @@ static const struct intel_memory_region_ops region_lmem_ops = {
>         .create_object = lmem_create_object,
>  };
>  
> +/* XXX: Time to vfunc your life up? */
> +void __iomem *i915_gem_object_lmem_io_map_page(struct drm_i915_gem_object *obj,
> +                                              unsigned long n)
> +{
> +       resource_size_t offset;
> +
> +       offset = i915_gem_object_get_dma_address(obj, n);

That seems dubious. So dubious that I again say do not mix terms.

> +       return io_mapping_map_atomic_wc(&obj->memory_region->iomap, offset);

Ahem. The caller may not be ready to abide by the terms of the atomic
contract.

> +}