drm/i915: Always use the GTT for error capture

Since the GTT provides universal access to any GPU page, we can use it
to reduce our plethora of read methods to just one. It also has the
important characteristic of being exactly what the GPU sees - if there
are incoherency problems, seeing the batch as executed (rather than as
trapped inside the cpu cache) is important.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161012090522.367-4-chris@chris-wilson.co.uk

drivers/gpu/drm/i915/i915_gem_gtt.c

@@ -2717,6 +2717,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	 */
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	unsigned long hole_start, hole_end;
+	struct i915_hw_ppgtt *ppgtt;
 	struct drm_mm_node *entry;
 	int ret;
 
@@ -2724,6 +2725,15 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
+	/* Reserve a mappable slot for our lockless error capture */
+	ret = drm_mm_insert_node_in_range_generic(&ggtt->base.mm,
+						  &ggtt->error_capture,
+						  4096, 0, -1,
+						  0, ggtt->mappable_end,
+						  0, 0);
+	if (ret)
+		return ret;
+
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &ggtt->base.mm, hole_start, hole_end) {
 		DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
@@ -2738,25 +2748,21 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 			       true);
 
 	if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) {
-		struct i915_hw_ppgtt *ppgtt;
-
 		ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
-		if (!ppgtt)
+		if (!ppgtt) {
-			return -ENOMEM;
+			ret = -ENOMEM;
-
+			goto err;
-		ret = __hw_ppgtt_init(ppgtt, dev_priv);
-		if (ret) {
-			kfree(ppgtt);
-			return ret;
 		}
 
-		if (ppgtt->base.allocate_va_range)
+		ret = __hw_ppgtt_init(ppgtt, dev_priv);
+		if (ret)
+			goto err_ppgtt;
+
+		if (ppgtt->base.allocate_va_range) {
 			ret = ppgtt->base.allocate_va_range(&ppgtt->base, 0,
 							    ppgtt->base.total);
-		if (ret) {
+			if (ret)
-			ppgtt->base.cleanup(&ppgtt->base);
+				goto err_ppgtt_cleanup;
-			kfree(ppgtt);
-			return ret;
 		}
 
 		ppgtt->base.clear_range(&ppgtt->base,
@@ -2770,6 +2776,14 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	}
 
 	return 0;
+
+err_ppgtt_cleanup:
+	ppgtt->base.cleanup(&ppgtt->base);
+err_ppgtt:
+	kfree(ppgtt);
+err:
+	drm_mm_remove_node(&ggtt->error_capture);
+	return ret;
 }
 
 /**
@@ -2788,6 +2802,9 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 
 	i915_gem_cleanup_stolen(&dev_priv->drm);
 
+	if (drm_mm_node_allocated(&ggtt->error_capture))
+		drm_mm_remove_node(&ggtt->error_capture);
+
 	if (drm_mm_initialized(&ggtt->base.mm)) {
 		intel_vgt_deballoon(dev_priv);
 

drivers/gpu/drm/i915/i915_gem_gtt.h

@@ -450,6 +450,8 @@ struct i915_ggtt {
 	bool do_idle_maps;
 
 	int mtrr;
+
+	struct drm_mm_node error_capture;
 };
 
 struct i915_hw_ppgtt {

drivers/gpu/drm/i915/i915_gpu_error.c

@@ -656,7 +656,7 @@ static void i915_error_object_free(struct drm_i915_error_object *obj)
 		return;
 
 	for (page = 0; page < obj->page_count; page++)
-		kfree(obj->pages[page]);
+		free_page((unsigned long)obj->pages[page]);
 
 	kfree(obj);
 }
@@ -693,98 +693,69 @@ static void i915_error_state_free(struct kref *error_ref)
 	kfree(error);
 }
 
+static int compress_page(void *src, struct drm_i915_error_object *dst)
+{
+	unsigned long page;
+
+	page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
+	if (!page)
+		return -ENOMEM;
+
+	dst->pages[dst->page_count++] = (void *)page;
+
+	memcpy((void *)page, src, PAGE_SIZE);
+	return 0;
+}
+
 static struct drm_i915_error_object *
-i915_error_object_create(struct drm_i915_private *dev_priv,
+i915_error_object_create(struct drm_i915_private *i915,
 			 struct i915_vma *vma)
 {
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
+	struct i915_ggtt *ggtt = &i915->ggtt;
-	struct drm_i915_gem_object *src;
+	const u64 slot = ggtt->error_capture.start;
 	struct drm_i915_error_object *dst;
-	int num_pages;
+	unsigned long num_pages;
-	bool use_ggtt;
+	struct sgt_iter iter;
-	int i = 0;
+	dma_addr_t dma;
-	u64 reloc_offset;
 
 	if (!vma)
 		return NULL;
 
-	src = vma->obj;
+	num_pages = min_t(u64, vma->size, vma->obj->base.size) >> PAGE_SHIFT;
-	if (!src->pages)
+	dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *),
-		return NULL;
+		      GFP_ATOMIC | __GFP_NOWARN);
-
-	num_pages = src->base.size >> PAGE_SHIFT;
-
-	dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *), GFP_ATOMIC);
 	if (!dst)
 		return NULL;
 
 	dst->gtt_offset = vma->node.start;
 	dst->gtt_size = vma->node.size;
+	dst->page_count = 0;
 
-	reloc_offset = dst->gtt_offset;
+	for_each_sgt_dma(dma, iter, vma->pages) {
-	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
-		   (vma->flags & I915_VMA_GLOBAL_BIND) &&
-		   reloc_offset + num_pages * PAGE_SIZE <= ggtt->mappable_end);
-
-	/* Cannot access stolen address directly, try to use the aperture */
-	if (src->stolen) {
-		use_ggtt = true;
-
-		if (!(vma->flags & I915_VMA_GLOBAL_BIND))
-			goto unwind;
-
-		reloc_offset = vma->node.start;
-		if (reloc_offset + num_pages * PAGE_SIZE > ggtt->mappable_end)
-			goto unwind;
-	}
-
-	/* Cannot access snooped pages through the aperture */
-	if (use_ggtt && src->cache_level != I915_CACHE_NONE &&
-	    !HAS_LLC(dev_priv))
-		goto unwind;
-
-	dst->page_count = num_pages;
-	while (num_pages--) {
-		void *d;
-
-		d = kmalloc(PAGE_SIZE, GFP_ATOMIC);
-		if (d == NULL)
-			goto unwind;
-
-		if (use_ggtt) {
 		void __iomem *s;
+		int ret;
 
-			/* Simply ignore tiling or any overlapping fence.
+		ggtt->base.insert_page(&ggtt->base, dma, slot,
-			 * It's part of the error state, and this hopefully
+				       I915_CACHE_NONE, 0);
-			 * captures what the GPU read.
-			 */
 
-			s = io_mapping_map_atomic_wc(&ggtt->mappable,
+		s = io_mapping_map_atomic_wc(&ggtt->mappable, slot);
-						     reloc_offset);
+		ret = compress_page((void * __force)s, dst);
-			memcpy_fromio(d, s, PAGE_SIZE);
 		io_mapping_unmap_atomic(s);
-		} else {
-			struct page *page;
-			void *s;
 
-			page = i915_gem_object_get_page(src, i);
+		if (ret)
-
+			goto unwind;
-			s = kmap_atomic(page);
-			memcpy(d, s, PAGE_SIZE);
-			kunmap_atomic(s);
 	}
-
+	goto out;
-		dst->pages[i++] = d;
-		reloc_offset += PAGE_SIZE;
-	}
-
-	return dst;
 
 unwind:
-	while (i--)
+	while (dst->page_count--)
-		kfree(dst->pages[i]);
+		free_page((unsigned long)dst->pages[dst->page_count]);
 	kfree(dst);
-	return NULL;
+	dst = NULL;
+
+out:
+	ggtt->base.clear_range(&ggtt->base, slot, PAGE_SIZE, true);
+	return dst;
 }
 
 /* The error capture is special as tries to run underneath the normal
@@ -1445,9 +1416,6 @@ static int capture(void *data)
 {
 	struct drm_i915_error_state *error = data;
 
-	/* Ensure that what we readback from memory matches what the GPU sees */
-	wbinvd();
-
 	i915_capture_gen_state(error->i915, error);
 	i915_capture_reg_state(error->i915, error);
 	i915_gem_record_fences(error->i915, error);
@@ -1460,9 +1428,6 @@ static int capture(void *data)
 	error->overlay = intel_overlay_capture_error_state(error->i915);
 	error->display = intel_display_capture_error_state(error->i915);
 
-	/* And make sure we don't leave trash in the CPU cache */
-	wbinvd();
-
 	return 0;
 }
 
@@ -1539,7 +1504,6 @@ void i915_error_state_get(struct drm_device *dev,
 	if (error_priv->error)
 		kref_get(&error_priv->error->ref);
 	spin_unlock_irq(&dev_priv->gpu_error.lock);
-
 }
 
 void i915_error_state_put(struct i915_error_state_file_priv *error_priv)