diff mbox series

[v2,09/37] drm/i915/lmem: support kernel mapping

Message ID 20190627205633.1143-10-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series Introduce memory region concept (including device local memory) | expand

Commit Message

Matthew Auld June 27, 2019, 8:56 p.m. UTC
From: Abdiel Janulgue <abdiel.janulgue@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@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@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(-)

Comments

Chris Wilson June 27, 2019, 11:27 p.m. UTC | #1
Quoting Matthew Auld (2019-06-27 21:56:05)
> From: Abdiel Janulgue <abdiel.janulgue@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@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@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.

> +}
diff mbox series

Patch

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 */
+		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)))
 		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);
+
+	return io_mapping_map_atomic_wc(&obj->memory_region->iomap, offset);
+}
+
+void __iomem *i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
+					  unsigned long n,
+					  unsigned long size)
+{
+	resource_size_t offset;
+
+	GEM_BUG_ON(!(obj->flags & I915_BO_ALLOC_CONTIGUOUS));
+
+	offset = i915_gem_object_get_dma_address(obj, n);
+
+	return io_mapping_map_wc(&obj->memory_region->iomap, offset, size);
+}
+
 bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
 {
 	struct intel_memory_region *region = obj->memory_region;
diff --git a/drivers/gpu/drm/i915/intel_region_lmem.h b/drivers/gpu/drm/i915/intel_region_lmem.h
index 0f0a6249d5b9..20084f7b4bff 100644
--- a/drivers/gpu/drm/i915/intel_region_lmem.h
+++ b/drivers/gpu/drm/i915/intel_region_lmem.h
@@ -6,6 +6,12 @@ 
 #ifndef __INTEL_REGION_LMEM_H
 #define __INTEL_REGION_LMEM_H
 
+
+void __iomem *i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
+					  unsigned long n, unsigned long size);
+void __iomem *i915_gem_object_lmem_io_map_page(struct drm_i915_gem_object *obj,
+					       unsigned long n);
+
 bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
 
 struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index 3ac320b28ef1..85d118c10d15 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -9,8 +9,11 @@ 
 
 #include "mock_gem_device.h"
 #include "gem/selftests/mock_context.h"
+#include "selftests/igt_flush_test.h"
 #include "mock_drm.h"
 
+#include "gem/i915_gem_object_blt.h"
+
 static void close_objects(struct list_head *objects)
 {
 	struct drm_i915_gem_object *obj, *on;
@@ -365,6 +368,79 @@  static int igt_lmem_create(void *arg)
 	return err;
 }
 
+static int igt_lmem_write_cpu(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_context *ce = i915->engine[BCS0]->kernel_context;
+	struct drm_i915_gem_object *obj;
+	struct rnd_state prng;
+	u32 *vaddr;
+	u32 dword;
+	u32 val;
+	u32 sz;
+	int err;
+
+	if (!HAS_ENGINE(i915, BCS0))
+		return 0;
+
+	sz = round_up(prandom_u32_state(&prng) % SZ_32M, PAGE_SIZE);
+
+	obj = i915_gem_object_create_lmem(i915, sz, I915_BO_ALLOC_CONTIGUOUS);
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
+
+	vaddr = i915_gem_object_pin_map(obj, I915_MAP_WC);
+	if (IS_ERR(vaddr)) {
+		pr_err("Failed to iomap lmembar; err=%d\n", (int)PTR_ERR(vaddr));
+		err = PTR_ERR(vaddr);
+		goto out_put;
+	}
+
+	val = prandom_u32_state(&prng);
+
+	/* Write from gpu and then read from cpu */
+	err = i915_gem_object_fill_blt(obj, ce, val);
+	if (err)
+		goto out_unpin;
+
+	i915_gem_object_lock(obj);
+	err = i915_gem_object_set_to_wc_domain(obj, true);
+	i915_gem_object_unlock(obj);
+	if (err)
+		goto out_unpin;
+
+	for (dword = 0; dword < sz / sizeof(u32); ++dword) {
+		if (vaddr[dword] != val) {
+			pr_err("vaddr[%u]=%u, val=%u\n", dword, vaddr[dword],
+			        val);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	/* Write from the cpu and read again from the cpu */
+	memset32(vaddr, val ^ 0xdeadbeaf, sz / sizeof(u32));
+
+	for (dword = 0; dword < sz / sizeof(u32); ++dword) {
+		if (vaddr[dword] != (val ^ 0xdeadbeaf)) {
+			pr_err("vaddr[%u]=%u, val=%u\n", dword, vaddr[dword],
+			        val ^ 0xdeadbeaf);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+out_unpin:
+	i915_gem_object_unpin_map(obj);
+out_put:
+	i915_gem_object_put(obj);
+
+	if (igt_flush_test(i915, I915_WAIT_LOCKED))
+		err = -EIO;
+
+	return err;
+}
+
 int intel_memory_region_mock_selftests(void)
 {
 	static const struct i915_subtest tests[] = {
@@ -406,6 +482,7 @@  int intel_memory_region_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_lmem_create),
+		SUBTEST(igt_lmem_write_cpu),
 	};
 	int err;