diff mbox series

[v3,26/37] drm/i915: error capture with no ggtt slot

Message ID 20190809222643.23142-27-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 Aug. 9, 2019, 10:26 p.m. UTC
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

If the aperture is not available in HW we can't use a ggtt slot and wc
copy, so fall back to regular kmap.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c   | 19 ++++----
 drivers/gpu/drm/i915/i915_gpu_error.c | 64 ++++++++++++++++++++++-----
 2 files changed, 63 insertions(+), 20 deletions(-)

Comments

Chris Wilson Aug. 10, 2019, 11:11 a.m. UTC | #1
Quoting Matthew Auld (2019-08-09 23:26:32)
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> If the aperture is not available in HW we can't use a ggtt slot and wc
> copy, so fall back to regular kmap.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c   | 19 ++++----
>  drivers/gpu/drm/i915/i915_gpu_error.c | 64 ++++++++++++++++++++++-----
>  2 files changed, 63 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index dd28c54527e3..0819ac9837dc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2630,7 +2630,8 @@ static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
>  static void cleanup_init_ggtt(struct i915_ggtt *ggtt)
>  {
>         ggtt_release_guc_top(ggtt);
> -       drm_mm_remove_node(&ggtt->error_capture);
> +       if (drm_mm_node_allocated(&ggtt->error_capture))
> +               drm_mm_remove_node(&ggtt->error_capture);
>  }
>  
>  static int init_ggtt(struct i915_ggtt *ggtt)
> @@ -2661,13 +2662,15 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>         if (ret)
>                 return ret;
>  
> -       /* Reserve a mappable slot for our lockless error capture */
> -       ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
> -                                         PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
> -                                         0, ggtt->mappable_end,
> -                                         DRM_MM_INSERT_LOW);
> -       if (ret)
> -               return ret;
> +       if (HAS_MAPPABLE_APERTURE(ggtt->vm.i915)) {
> +               /* Reserve a mappable slot for our lockless error capture */
> +               ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
> +                                                 PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
> +                                                 0, ggtt->mappable_end,
> +                                                 DRM_MM_INSERT_LOW);
> +               if (ret)
> +                       return ret;
> +       }
>  
>         /*
>          * The upper portion of the GuC address space has a sizeable hole
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 92986d3f6995..19eb5ccba387 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -40,6 +40,7 @@
>  #include "display/intel_overlay.h"
>  
>  #include "gem/i915_gem_context.h"
> +#include "gem/i915_gem_lmem.h"
>  
>  #include "i915_drv.h"
>  #include "i915_gpu_error.h"
> @@ -235,6 +236,7 @@ struct compress {
>         struct pagevec pool;
>         struct z_stream_s zstream;
>         void *tmp;
> +       bool wc;
>  };
>  
>  static bool compress_init(struct compress *c)
> @@ -292,7 +294,7 @@ static int compress_page(struct compress *c,
>         struct z_stream_s *zstream = &c->zstream;
>  
>         zstream->next_in = src;
> -       if (c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
> +       if (c->wc && c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
>                 zstream->next_in = c->tmp;
>         zstream->avail_in = PAGE_SIZE;
>  
> @@ -367,6 +369,7 @@ static void err_compression_marker(struct drm_i915_error_state_buf *m)
>  
>  struct compress {
>         struct pagevec pool;
> +       bool wc;
>  };
>  
>  static bool compress_init(struct compress *c)
> @@ -389,7 +392,7 @@ static int compress_page(struct compress *c,
>         if (!ptr)
>                 return -ENOMEM;
>  
> -       if (!i915_memcpy_from_wc(ptr, src, PAGE_SIZE))
> +       if (!(c->wc && i915_memcpy_from_wc(ptr, src, PAGE_SIZE)))
>                 memcpy(ptr, src, PAGE_SIZE);
>         dst->pages[dst->page_count++] = ptr;
>  
> @@ -963,7 +966,6 @@ i915_error_object_create(struct drm_i915_private *i915,
>         struct drm_i915_error_object *dst;
>         unsigned long num_pages;
>         struct sgt_iter iter;
> -       dma_addr_t dma;
>         int ret;
>  
>         might_sleep();
> @@ -988,17 +990,53 @@ i915_error_object_create(struct drm_i915_private *i915,
>         dst->page_count = 0;
>         dst->unused = 0;
>  
> +       compress->wc = i915_gem_object_is_lmem(vma->obj) ||

We need to talk about this fixation you appear to have on vma->obj!

> +                      drm_mm_node_allocated(&ggtt->error_capture);
> +
>         ret = -EINVAL;
> -       for_each_sgt_dma(dma, iter, vma->pages) {
> +       if (drm_mm_node_allocated(&ggtt->error_capture)) {
>                 void __iomem *s;
> +               dma_addr_t dma;
>  
> -               ggtt->vm.insert_page(&ggtt->vm, dma, slot, I915_CACHE_NONE, 0);
> +               for_each_sgt_dma(dma, iter, vma->pages) {
> +                       ggtt->vm.insert_page(&ggtt->vm, dma, slot,
> +                                            I915_CACHE_NONE, 0);
>  
> -               s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
> -               ret = compress_page(compress, (void  __force *)s, dst);
> -               io_mapping_unmap(s);
> -               if (ret)
> -                       break;
> +                       s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
> +                       ret = compress_page(compress, (void  __force *)s, dst);
> +                       io_mapping_unmap(s);
> +                       if (ret)
> +                               break;
> +               }
> +       } else if (i915_gem_object_is_lmem(vma->obj)) {
> +               void *s;
> +               dma_addr_t dma;
> +               struct intel_memory_region *mem = vma->obj->mm.region;

You did that on purpose!

> +
> +               for_each_sgt_dma(dma, iter, vma->pages) {
> +                       s = io_mapping_map_atomic_wc(&mem->iomap, dma);
> +                       ret = compress_page(compress, s, dst);
> +                       io_mapping_unmap_atomic(s);
> +
> +                       if (ret)
> +                               break;
> +               }
> +       } else {
> +               void *s;
> +               struct page *page;
> +
> +               for_each_sgt_page(page, iter, vma->pages) {
void *s;

Then I wouldn't even complain about the ordering.

> +                       drm_clflush_pages(&page, 1);
> +
> +                       s = kmap_atomic(page);
> +                       ret = compress_page(compress, s, dst);
> +                       kunmap_atomic(s);
> +
> +                       if (ret)
> +                               break;
> +
> +                       drm_clflush_pages(&page, 1);

clflush before the ret

I have to say the circle is complete.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index dd28c54527e3..0819ac9837dc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2630,7 +2630,8 @@  static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
 static void cleanup_init_ggtt(struct i915_ggtt *ggtt)
 {
 	ggtt_release_guc_top(ggtt);
-	drm_mm_remove_node(&ggtt->error_capture);
+	if (drm_mm_node_allocated(&ggtt->error_capture))
+		drm_mm_remove_node(&ggtt->error_capture);
 }
 
 static int init_ggtt(struct i915_ggtt *ggtt)
@@ -2661,13 +2662,15 @@  static int init_ggtt(struct i915_ggtt *ggtt)
 	if (ret)
 		return ret;
 
-	/* Reserve a mappable slot for our lockless error capture */
-	ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
-					  PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
-					  0, ggtt->mappable_end,
-					  DRM_MM_INSERT_LOW);
-	if (ret)
-		return ret;
+	if (HAS_MAPPABLE_APERTURE(ggtt->vm.i915)) {
+		/* Reserve a mappable slot for our lockless error capture */
+		ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->error_capture,
+						  PAGE_SIZE, 0, I915_COLOR_UNEVICTABLE,
+						  0, ggtt->mappable_end,
+						  DRM_MM_INSERT_LOW);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * The upper portion of the GuC address space has a sizeable hole
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 92986d3f6995..19eb5ccba387 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -40,6 +40,7 @@ 
 #include "display/intel_overlay.h"
 
 #include "gem/i915_gem_context.h"
+#include "gem/i915_gem_lmem.h"
 
 #include "i915_drv.h"
 #include "i915_gpu_error.h"
@@ -235,6 +236,7 @@  struct compress {
 	struct pagevec pool;
 	struct z_stream_s zstream;
 	void *tmp;
+	bool wc;
 };
 
 static bool compress_init(struct compress *c)
@@ -292,7 +294,7 @@  static int compress_page(struct compress *c,
 	struct z_stream_s *zstream = &c->zstream;
 
 	zstream->next_in = src;
-	if (c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
+	if (c->wc && c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
 		zstream->next_in = c->tmp;
 	zstream->avail_in = PAGE_SIZE;
 
@@ -367,6 +369,7 @@  static void err_compression_marker(struct drm_i915_error_state_buf *m)
 
 struct compress {
 	struct pagevec pool;
+	bool wc;
 };
 
 static bool compress_init(struct compress *c)
@@ -389,7 +392,7 @@  static int compress_page(struct compress *c,
 	if (!ptr)
 		return -ENOMEM;
 
-	if (!i915_memcpy_from_wc(ptr, src, PAGE_SIZE))
+	if (!(c->wc && i915_memcpy_from_wc(ptr, src, PAGE_SIZE)))
 		memcpy(ptr, src, PAGE_SIZE);
 	dst->pages[dst->page_count++] = ptr;
 
@@ -963,7 +966,6 @@  i915_error_object_create(struct drm_i915_private *i915,
 	struct drm_i915_error_object *dst;
 	unsigned long num_pages;
 	struct sgt_iter iter;
-	dma_addr_t dma;
 	int ret;
 
 	might_sleep();
@@ -988,17 +990,53 @@  i915_error_object_create(struct drm_i915_private *i915,
 	dst->page_count = 0;
 	dst->unused = 0;
 
+	compress->wc = i915_gem_object_is_lmem(vma->obj) ||
+		       drm_mm_node_allocated(&ggtt->error_capture);
+
 	ret = -EINVAL;
-	for_each_sgt_dma(dma, iter, vma->pages) {
+	if (drm_mm_node_allocated(&ggtt->error_capture)) {
 		void __iomem *s;
+		dma_addr_t dma;
 
-		ggtt->vm.insert_page(&ggtt->vm, dma, slot, I915_CACHE_NONE, 0);
+		for_each_sgt_dma(dma, iter, vma->pages) {
+			ggtt->vm.insert_page(&ggtt->vm, dma, slot,
+					     I915_CACHE_NONE, 0);
 
-		s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
-		ret = compress_page(compress, (void  __force *)s, dst);
-		io_mapping_unmap(s);
-		if (ret)
-			break;
+			s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
+			ret = compress_page(compress, (void  __force *)s, dst);
+			io_mapping_unmap(s);
+			if (ret)
+				break;
+		}
+	} else if (i915_gem_object_is_lmem(vma->obj)) {
+		void *s;
+		dma_addr_t dma;
+		struct intel_memory_region *mem = vma->obj->mm.region;
+
+		for_each_sgt_dma(dma, iter, vma->pages) {
+			s = io_mapping_map_atomic_wc(&mem->iomap, dma);
+			ret = compress_page(compress, s, dst);
+			io_mapping_unmap_atomic(s);
+
+			if (ret)
+				break;
+		}
+	} else {
+		void *s;
+		struct page *page;
+
+		for_each_sgt_page(page, iter, vma->pages) {
+			drm_clflush_pages(&page, 1);
+
+			s = kmap_atomic(page);
+			ret = compress_page(compress, s, dst);
+			kunmap_atomic(s);
+
+			if (ret)
+				break;
+
+			drm_clflush_pages(&page, 1);
+		}
 	}
 
 	if (ret || compress_flush(compress, dst)) {
@@ -1664,9 +1702,11 @@  static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
 static void capture_finish(struct i915_gpu_state *error)
 {
 	struct i915_ggtt *ggtt = &error->i915->ggtt;
-	const u64 slot = ggtt->error_capture.start;
 
-	ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
+	if (drm_mm_node_allocated(&ggtt->error_capture)) {
+		const u64 slot = ggtt->error_capture.start;
+		ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
+	}
 }
 
 #define DAY_AS_SECONDS(x) (24 * 60 * 60 * (x))