Full ppgtt for hsw, closer

Full ppgtt for hsw, closer

[Chris Wilson]

In today's edition, IGT should be happy; meaning we should have most of
the resource allocation issues licked, and that the gpu is indeed
switching mm between each context. Running piglit through it though, the
picture is a little more murky, as there are a few GPU hangs...
-Chris


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

[Chris Wilson]

When we update the gen6 ppgtt page directories, we do so by writing the
new address into a reserved slot in the GGTT. It appears that when the
GPU reads that entry from the gsm, it uses its small cache and that we
need to invalidate that cache after writing. We don't see an issue
currently as we prefill the ppgtt page directories on creation; and only
create the single aliasing_ppgtt long before we start using the GGTT
(and so before the cache mayhave a conflicting entry).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 163c71784014..b232452ff5b9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1687,8 +1687,8 @@ static inline void gen6_write_pde(const struct i915_hw_ppgtt *ppgtt,
 				  const struct i915_page_table *pt)
 {
 	/* Caller needs to make sure the write completes if necessary */
-	writel_relaxed(GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID,
-		       ppgtt->pd_addr + pde);
+	iowrite32(GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID,
+		  ppgtt->pd_addr + pde);
 }
 
 /* Write all the page tables found in the ppgtt structure to incrementing page
@@ -1703,7 +1703,7 @@ static void gen6_write_page_range(struct i915_hw_ppgtt *ppgtt,
 		gen6_write_pde(ppgtt, pde, pt);
 
 	mark_tlbs_dirty(ppgtt);
-	wmb();
+	gen6_ggtt_invalidate(ppgtt->vm.i915);
 }
 
 static inline u32 get_pd_offset(struct i915_hw_ppgtt *ppgtt)
@@ -1919,7 +1919,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 	if (flush) {
 		mark_tlbs_dirty(ppgtt);
-		wmb();
+		gen6_ggtt_invalidate(ppgtt->vm.i915);
 	}
 
 	return 0;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/17] drm/i915: Prepare for non-object vma

[Chris Wilson]

In order to allow ourselves to use VMA to wrap other entities other than
GEM objects, we need to allow for the vma->obj backpointer to be NULL.
In most cases, we know we are operating on a GEM object and its vma, but
we need the core code (such as i915_vma_pin/insert/bind/unbind) to work
regardless of the innards.

The remaining eyesore here is vma->obj->cache_level and related (but
less of an issue) vma->obj->gt_ro. With a bit of care we should mirror
those on the vma itself.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c   |  7 +-
 drivers/gpu/drm/i915/i915_gpu_error.c |  3 +
 drivers/gpu/drm/i915/i915_vma.c       | 96 ++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_vma.h       |  2 +-
 4 files changed, 65 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b232452ff5b9..62b7f88a820d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3588,8 +3588,11 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		if (!i915_vma_unbind(vma))
 			continue;
 
-		WARN_ON(i915_vma_bind(vma, obj->cache_level, PIN_UPDATE));
-		WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
+		WARN_ON(i915_vma_bind(vma,
+				      obj ? obj->cache_level : 0,
+				      PIN_UPDATE));
+		if (obj)
+			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
 	ggtt->vm.closed = false;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 758234d20f4e..df524c9cad40 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1050,6 +1050,9 @@ static u32 capture_error_bo(struct drm_i915_error_buffer *err,
 	int i = 0;
 
 	list_for_each_entry(vma, head, vm_link) {
+		if (!vma->obj)
+			continue;
+
 		if (pinned_only && !i915_vma_is_pinned(vma))
 			continue;
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 912f16ffe7ee..b71265066cd1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -345,7 +345,7 @@ void i915_vma_flush_writes(struct i915_vma *vma)
 
 void i915_vma_unpin_iomap(struct i915_vma *vma)
 {
-	lockdep_assert_held(&vma->obj->base.dev->struct_mutex);
+	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
 
 	GEM_BUG_ON(vma->iomap == NULL);
 
@@ -365,6 +365,7 @@ void i915_vma_unpin_and_release(struct i915_vma **p_vma)
 		return;
 
 	obj = vma->obj;
+	GEM_BUG_ON(!obj);
 
 	i915_vma_unpin(vma);
 	i915_vma_close(vma);
@@ -489,7 +490,7 @@ static int
 i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 {
 	struct drm_i915_private *dev_priv = vma->vm->i915;
-	struct drm_i915_gem_object *obj = vma->obj;
+	unsigned int cache_level;
 	u64 start, end;
 	int ret;
 
@@ -524,16 +525,21 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	 * attempt to find space.
 	 */
 	if (size > end) {
-		DRM_DEBUG("Attempting to bind an object larger than the aperture: request=%llu [object=%zd] > %s aperture=%llu\n",
-			  size, obj->base.size,
-			  flags & PIN_MAPPABLE ? "mappable" : "total",
+		DRM_DEBUG("Attempting to bind an object larger than the aperture: request=%llu > %s aperture=%llu\n",
+			  size, flags & PIN_MAPPABLE ? "mappable" : "total",
 			  end);
 		return -ENOSPC;
 	}
 
-	ret = i915_gem_object_pin_pages(obj);
-	if (ret)
-		return ret;
+	if (vma->obj) {
+		ret = i915_gem_object_pin_pages(vma->obj);
+		if (ret)
+			return ret;
+
+		cache_level = vma->obj->cache_level;
+	} else {
+		cache_level = 0;
+	}
 
 	GEM_BUG_ON(vma->pages);
 
@@ -550,7 +556,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 		}
 
 		ret = i915_gem_gtt_reserve(vma->vm, &vma->node,
-					   size, offset, obj->cache_level,
+					   size, offset, cache_level,
 					   flags);
 		if (ret)
 			goto err_clear;
@@ -589,7 +595,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 		}
 
 		ret = i915_gem_gtt_insert(vma->vm, &vma->node,
-					  size, alignment, obj->cache_level,
+					  size, alignment, cache_level,
 					  start, end, flags);
 		if (ret)
 			goto err_clear;
@@ -598,23 +604,28 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 		GEM_BUG_ON(vma->node.start + vma->node.size > end);
 	}
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
-	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
+	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, cache_level));
 
 	list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
 
-	spin_lock(&dev_priv->mm.obj_lock);
-	list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
-	obj->bind_count++;
-	spin_unlock(&dev_priv->mm.obj_lock);
+	if (vma->obj) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		spin_lock(&dev_priv->mm.obj_lock);
+		list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
+		obj->bind_count++;
+		spin_unlock(&dev_priv->mm.obj_lock);
 
-	assert_bind_count(obj);
+		assert_bind_count(obj);
+	}
 
 	return 0;
 
 err_clear:
 	vma->vm->clear_pages(vma);
 err_unpin:
-	i915_gem_object_unpin_pages(obj);
+	if (vma->obj)
+		i915_gem_object_unpin_pages(vma->obj);
 	return ret;
 }
 
@@ -622,7 +633,6 @@ static void
 i915_vma_remove(struct i915_vma *vma)
 {
 	struct drm_i915_private *i915 = vma->vm->i915;
-	struct drm_i915_gem_object *obj = vma->obj;
 
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));
@@ -632,20 +642,26 @@ i915_vma_remove(struct i915_vma *vma)
 	drm_mm_remove_node(&vma->node);
 	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
 
-	/* Since the unbound list is global, only move to that list if
+	/*
+	 * Since the unbound list is global, only move to that list if
 	 * no more VMAs exist.
 	 */
-	spin_lock(&i915->mm.obj_lock);
-	if (--obj->bind_count == 0)
-		list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
-	spin_unlock(&i915->mm.obj_lock);
-
-	/* And finally now the object is completely decoupled from this vma,
-	 * we can drop its hold on the backing storage and allow it to be
-	 * reaped by the shrinker.
-	 */
-	i915_gem_object_unpin_pages(obj);
-	assert_bind_count(obj);
+	if (vma->obj) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		spin_lock(&i915->mm.obj_lock);
+		if (--obj->bind_count == 0)
+			list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+		spin_unlock(&i915->mm.obj_lock);
+
+		/*
+		 * And finally now the object is completely decoupled from this
+		 * vma, we can drop its hold on the backing storage and allow
+		 * it to be reaped by the shrinker.
+		 */
+		i915_gem_object_unpin_pages(obj);
+		assert_bind_count(obj);
+	}
 }
 
 int __i915_vma_do_pin(struct i915_vma *vma,
@@ -670,7 +686,7 @@ int __i915_vma_do_pin(struct i915_vma *vma,
 	}
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 
-	ret = i915_vma_bind(vma, vma->obj->cache_level, flags);
+	ret = i915_vma_bind(vma, vma->obj ? vma->obj->cache_level : 0, flags);
 	if (ret)
 		goto err_remove;
 
@@ -727,6 +743,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 
 static void __i915_vma_destroy(struct i915_vma *vma)
 {
+	struct drm_i915_private *i915 = vma->vm->i915;
 	int i;
 
 	GEM_BUG_ON(vma->node.allocated);
@@ -738,12 +755,13 @@ static void __i915_vma_destroy(struct i915_vma *vma)
 
 	list_del(&vma->obj_link);
 	list_del(&vma->vm_link);
-	rb_erase(&vma->obj_node, &vma->obj->vma_tree);
+	if (vma->obj)
+		rb_erase(&vma->obj_node, &vma->obj->vma_tree);
 
 	if (!i915_vma_is_ggtt(vma))
 		i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm));
 
-	kmem_cache_free(to_i915(vma->obj->base.dev)->vmas, vma);
+	kmem_cache_free(i915->vmas, vma);
 }
 
 void i915_vma_destroy(struct i915_vma *vma)
@@ -809,13 +827,13 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
 
 int i915_vma_unbind(struct i915_vma *vma)
 {
-	struct drm_i915_gem_object *obj = vma->obj;
 	unsigned long active;
 	int ret;
 
-	lockdep_assert_held(&obj->base.dev->struct_mutex);
+	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
 
-	/* First wait upon any activity as retiring the request may
+	/*
+	 * First wait upon any activity as retiring the request may
 	 * have side-effects such as unpinning or even unbinding this vma.
 	 */
 	might_sleep();
@@ -823,7 +841,8 @@ int i915_vma_unbind(struct i915_vma *vma)
 	if (active) {
 		int idx;
 
-		/* When a closed VMA is retired, it is unbound - eek.
+		/*
+		 * When a closed VMA is retired, it is unbound - eek.
 		 * In order to prevent it from being recursively closed,
 		 * take a pin on the vma so that the second unbind is
 		 * aborted.
@@ -861,9 +880,6 @@ int i915_vma_unbind(struct i915_vma *vma)
 	if (!drm_mm_node_allocated(&vma->node))
 		return 0;
 
-	GEM_BUG_ON(obj->bind_count == 0);
-	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
-
 	if (i915_vma_is_map_and_fenceable(vma)) {
 		/*
 		 * Check that we have flushed all writes through the GGTT
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index fc4294cfaa91..f0532f1a4953 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -407,7 +407,7 @@ static inline void __i915_vma_unpin_fence(struct i915_vma *vma)
 static inline void
 i915_vma_unpin_fence(struct i915_vma *vma)
 {
-	lockdep_assert_held(&vma->obj->base.dev->struct_mutex);
+	/* lockdep_assert_held(&vma->vm->i915->drm.struct_mutex); */
 	if (vma->fence)
 		__i915_vma_unpin_fence(vma);
 }
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/17] drm/i915: Decouple vma vfuncs from vm

[Chris Wilson]

To allow for future non-object backed vma, we need to be able to
specialise the callbacks for binding, et al, the vma. For example,
instead of calling vma->vm->bind_vma(), we now call
vma->ops->bind_vma(). This gives us the opportunity to later override the
operation for a custom vma.

v2: flip order of unbind/bind

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 57 ++++++++++++-----------
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 27 +++++++----
 drivers/gpu/drm/i915/i915_vma.c           | 11 +++--
 drivers/gpu/drm/i915/i915_vma.h           |  1 +
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 18 +++----
 5 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 62b7f88a820d..a54977106681 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1616,12 +1616,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
 	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
-	ppgtt->vm.bind_vma = gen8_ppgtt_bind_vma;
-	ppgtt->vm.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->vm.set_pages = ppgtt_set_pages;
-	ppgtt->vm.clear_pages = clear_pages;
 	ppgtt->debug_dump = gen8_dump_ppgtt;
 
+	ppgtt->vm.vma_ops.bind_vma    = gen8_ppgtt_bind_vma;
+	ppgtt->vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
+	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
+	ppgtt->vm.vma_ops.clear_pages = clear_pages;
+
 	return 0;
 
 free_scratch:
@@ -2059,13 +2060,14 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 	ppgtt->vm.clear_range = gen6_ppgtt_clear_range;
 	ppgtt->vm.insert_entries = gen6_ppgtt_insert_entries;
-	ppgtt->vm.bind_vma = gen6_ppgtt_bind_vma;
-	ppgtt->vm.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->vm.set_pages = ppgtt_set_pages;
-	ppgtt->vm.clear_pages = clear_pages;
 	ppgtt->vm.cleanup = gen6_ppgtt_cleanup;
 	ppgtt->debug_dump = gen6_dump_ppgtt;
 
+	ppgtt->vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
+	ppgtt->vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
+	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
+	ppgtt->vm.vma_ops.clear_pages = clear_pages;
+
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 			 ppgtt->node.size >> 20,
 			 ppgtt->node.start / PAGE_SIZE);
@@ -2793,11 +2795,11 @@ int i915_gem_init_aliasing_ppgtt(struct drm_i915_private *i915)
 
 	i915->mm.aliasing_ppgtt = ppgtt;
 
-	GEM_BUG_ON(ggtt->vm.bind_vma != ggtt_bind_vma);
-	ggtt->vm.bind_vma = aliasing_gtt_bind_vma;
+	GEM_BUG_ON(ggtt->vm.vma_ops.bind_vma != ggtt_bind_vma);
+	ggtt->vm.vma_ops.bind_vma = aliasing_gtt_bind_vma;
 
-	GEM_BUG_ON(ggtt->vm.unbind_vma != ggtt_unbind_vma);
-	ggtt->vm.unbind_vma = aliasing_gtt_unbind_vma;
+	GEM_BUG_ON(ggtt->vm.vma_ops.unbind_vma != ggtt_unbind_vma);
+	ggtt->vm.vma_ops.unbind_vma = aliasing_gtt_unbind_vma;
 
 	return 0;
 
@@ -2817,8 +2819,8 @@ void i915_gem_fini_aliasing_ppgtt(struct drm_i915_private *i915)
 
 	i915_ppgtt_put(ppgtt);
 
-	ggtt->vm.bind_vma = ggtt_bind_vma;
-	ggtt->vm.unbind_vma = ggtt_unbind_vma;
+	ggtt->vm.vma_ops.bind_vma   = ggtt_bind_vma;
+	ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
 }
 
 int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
@@ -3310,10 +3312,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ggtt->vm.total = (size / sizeof(gen8_pte_t)) << PAGE_SHIFT;
 	ggtt->vm.cleanup = gen6_gmch_remove;
-	ggtt->vm.bind_vma = ggtt_bind_vma;
-	ggtt->vm.unbind_vma = ggtt_unbind_vma;
-	ggtt->vm.set_pages = ggtt_set_pages;
-	ggtt->vm.clear_pages = clear_pages;
 	ggtt->vm.insert_page = gen8_ggtt_insert_page;
 	ggtt->vm.clear_range = nop_clear_range;
 	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
@@ -3331,6 +3329,11 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ggtt->invalidate = gen6_ggtt_invalidate;
 
+	ggtt->vm.vma_ops.bind_vma    = ggtt_bind_vma;
+	ggtt->vm.vma_ops.unbind_vma  = ggtt_unbind_vma;
+	ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
+	ggtt->vm.vma_ops.clear_pages = clear_pages;
+
 	setup_private_pat(dev_priv);
 
 	return ggtt_probe_common(ggtt, size);
@@ -3370,10 +3373,6 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->vm.clear_range = gen6_ggtt_clear_range;
 	ggtt->vm.insert_page = gen6_ggtt_insert_page;
 	ggtt->vm.insert_entries = gen6_ggtt_insert_entries;
-	ggtt->vm.bind_vma = ggtt_bind_vma;
-	ggtt->vm.unbind_vma = ggtt_unbind_vma;
-	ggtt->vm.set_pages = ggtt_set_pages;
-	ggtt->vm.clear_pages = clear_pages;
 	ggtt->vm.cleanup = gen6_gmch_remove;
 
 	ggtt->invalidate = gen6_ggtt_invalidate;
@@ -3389,6 +3388,11 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	else
 		ggtt->vm.pte_encode = snb_pte_encode;
 
+	ggtt->vm.vma_ops.bind_vma    = ggtt_bind_vma;
+	ggtt->vm.vma_ops.unbind_vma  = ggtt_unbind_vma;
+	ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
+	ggtt->vm.vma_ops.clear_pages = clear_pages;
+
 	return ggtt_probe_common(ggtt, size);
 }
 
@@ -3419,14 +3423,15 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->vm.insert_page = i915_ggtt_insert_page;
 	ggtt->vm.insert_entries = i915_ggtt_insert_entries;
 	ggtt->vm.clear_range = i915_ggtt_clear_range;
-	ggtt->vm.bind_vma = ggtt_bind_vma;
-	ggtt->vm.unbind_vma = ggtt_unbind_vma;
-	ggtt->vm.set_pages = ggtt_set_pages;
-	ggtt->vm.clear_pages = clear_pages;
 	ggtt->vm.cleanup = i915_gmch_remove;
 
 	ggtt->invalidate = gmch_ggtt_invalidate;
 
+	ggtt->vm.vma_ops.bind_vma    = ggtt_bind_vma;
+	ggtt->vm.vma_ops.unbind_vma  = ggtt_unbind_vma;
+	ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
+	ggtt->vm.vma_ops.clear_pages = clear_pages;
+
 	if (unlikely(ggtt->do_idle_maps))
 		DRM_INFO("applying Ironlake quirks for intel_iommu\n");
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 197c2c06ecb7..16307ba7e303 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -58,6 +58,7 @@
 
 struct drm_i915_file_private;
 struct drm_i915_fence_reg;
+struct i915_vma;
 
 typedef u32 gen6_pte_t;
 typedef u64 gen8_pte_t;
@@ -254,6 +255,21 @@ struct i915_pml4 {
 	struct i915_page_directory_pointer *pdps[GEN8_PML4ES_PER_PML4];
 };
 
+struct i915_vma_ops {
+	/* Map an object into an address space with the given cache flags. */
+	int (*bind_vma)(struct i915_vma *vma,
+			enum i915_cache_level cache_level,
+			u32 flags);
+	/*
+	 * Unmap an object from an address space. This usually consists of
+	 * setting the valid PTE entries to a reserved scratch page.
+	 */
+	void (*unbind_vma)(struct i915_vma *vma);
+
+	int (*set_pages)(struct i915_vma *vma);
+	void (*clear_pages)(struct i915_vma *vma);
+};
+
 struct i915_address_space {
 	struct drm_mm mm;
 	struct drm_i915_private *i915;
@@ -331,15 +347,8 @@ struct i915_address_space {
 			       enum i915_cache_level cache_level,
 			       u32 flags);
 	void (*cleanup)(struct i915_address_space *vm);
-	/** Unmap an object from an address space. This usually consists of
-	 * setting the valid PTE entries to a reserved scratch page. */
-	void (*unbind_vma)(struct i915_vma *vma);
-	/* Map an object into an address space with the given cache flags. */
-	int (*bind_vma)(struct i915_vma *vma,
-			enum i915_cache_level cache_level,
-			u32 flags);
-	int (*set_pages)(struct i915_vma *vma);
-	void (*clear_pages)(struct i915_vma *vma);
+
+	struct i915_vma_ops vma_ops;
 
 	I915_SELFTEST_DECLARE(struct fault_attr fault_attr);
 	I915_SELFTEST_DECLARE(bool scrub_64K);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index b71265066cd1..e82aa804cdba 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -95,6 +95,7 @@ vma_create(struct drm_i915_gem_object *obj,
 		init_request_active(&vma->last_read[i], i915_vma_retire);
 	init_request_active(&vma->last_fence, NULL);
 	vma->vm = vm;
+	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
 	vma->resv = obj->resv;
 	vma->size = obj->base.size;
@@ -280,7 +281,7 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 	GEM_BUG_ON(!vma->pages);
 
 	trace_i915_vma_bind(vma, bind_flags);
-	ret = vma->vm->bind_vma(vma, cache_level, bind_flags);
+	ret = vma->ops->bind_vma(vma, cache_level, bind_flags);
 	if (ret)
 		return ret;
 
@@ -543,7 +544,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 
 	GEM_BUG_ON(vma->pages);
 
-	ret = vma->vm->set_pages(vma);
+	ret = vma->ops->set_pages(vma);
 	if (ret)
 		goto err_unpin;
 
@@ -622,7 +623,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	return 0;
 
 err_clear:
-	vma->vm->clear_pages(vma);
+	vma->ops->clear_pages(vma);
 err_unpin:
 	if (vma->obj)
 		i915_gem_object_unpin_pages(vma->obj);
@@ -637,7 +638,7 @@ i915_vma_remove(struct i915_vma *vma)
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));
 
-	vma->vm->clear_pages(vma);
+	vma->ops->clear_pages(vma);
 
 	drm_mm_remove_node(&vma->node);
 	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
@@ -906,7 +907,7 @@ int i915_vma_unbind(struct i915_vma *vma)
 
 	if (likely(!vma->vm->closed)) {
 		trace_i915_vma_unbind(vma);
-		vma->vm->unbind_vma(vma);
+		vma->ops->unbind_vma(vma);
 	}
 	vma->flags &= ~(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
 
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f0532f1a4953..4321476a6a32 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -49,6 +49,7 @@ struct i915_vma {
 	struct drm_mm_node node;
 	struct drm_i915_gem_object *obj;
 	struct i915_address_space *vm;
+	const struct i915_vma_ops *ops;
 	struct drm_i915_fence_reg *fence;
 	struct reservation_object *resv; /** Alias of obj->resv */
 	struct sg_table *pages;
diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
index 556c546f2715..6a7f4da7b523 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
@@ -80,12 +80,13 @@ mock_ppgtt(struct drm_i915_private *i915,
 	ppgtt->vm.clear_range = nop_clear_range;
 	ppgtt->vm.insert_page = mock_insert_page;
 	ppgtt->vm.insert_entries = mock_insert_entries;
-	ppgtt->vm.bind_vma = mock_bind_ppgtt;
-	ppgtt->vm.unbind_vma = mock_unbind_ppgtt;
-	ppgtt->vm.set_pages = ppgtt_set_pages;
-	ppgtt->vm.clear_pages = clear_pages;
 	ppgtt->vm.cleanup = mock_cleanup;
 
+	ppgtt->vm.vma_ops.bind_vma    = mock_bind_ppgtt;
+	ppgtt->vm.vma_ops.unbind_vma  = mock_unbind_ppgtt;
+	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
+	ppgtt->vm.vma_ops.clear_pages = clear_pages;
+
 	return ppgtt;
 }
 
@@ -116,12 +117,13 @@ void mock_init_ggtt(struct drm_i915_private *i915)
 	ggtt->vm.clear_range = nop_clear_range;
 	ggtt->vm.insert_page = mock_insert_page;
 	ggtt->vm.insert_entries = mock_insert_entries;
-	ggtt->vm.bind_vma = mock_bind_ggtt;
-	ggtt->vm.unbind_vma = mock_unbind_ggtt;
-	ggtt->vm.set_pages = ggtt_set_pages;
-	ggtt->vm.clear_pages = clear_pages;
 	ggtt->vm.cleanup = mock_cleanup;
 
+	ggtt->vm.vma_ops.bind_vma    = mock_bind_ggtt;
+	ggtt->vm.vma_ops.unbind_vma  = mock_unbind_ggtt;
+	ggtt->vm.vma_ops.set_pages   = ggtt_set_pages;
+	ggtt->vm.vma_ops.clear_pages = clear_pages;
+
 	i915_address_space_init(&ggtt->vm, i915, "global");
 }
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/17] drm/i915/gtt: Push allocation to hw ppgtt constructor

[Chris Wilson]

In the next patch, we will subclass the gen6 hw_ppgtt. In order, for the
two different generations of hw ppgtt stucts to be of different size,
push the allocation down to the constructor.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c           | 140 +++++++++---------
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  16 +-
 2 files changed, 83 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a54977106681..f1443ba948a5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1562,32 +1562,36 @@ static int gen8_preallocate_top_level_pdp(struct i915_hw_ppgtt *ppgtt)
  * space.
  *
  */
-static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
+static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 {
-	struct i915_address_space *vm = &ppgtt->vm;
-	struct drm_i915_private *dev_priv = vm->i915;
-	int ret;
+	struct i915_hw_ppgtt *ppgtt;
+	int err;
+
+	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
+	if (!ppgtt)
+		return ERR_PTR(-ENOMEM);
+
+	ppgtt->vm.i915 = i915;
+	ppgtt->vm.dma = &i915->drm.pdev->dev;
 
-	ppgtt->vm.total = USES_FULL_48BIT_PPGTT(dev_priv) ?
+	ppgtt->vm.total = USES_FULL_48BIT_PPGTT(i915) ?
 		1ULL << 48 :
 		1ULL << 32;
 
 	/* There are only few exceptions for gen >=6. chv and bxt.
 	 * And we are not sure about the latter so play safe for now.
 	 */
-	if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv))
+	if (IS_CHERRYVIEW(i915) || IS_BROXTON(i915))
 		ppgtt->vm.pt_kmap_wc = true;
 
-	ret = gen8_init_scratch(&ppgtt->vm);
-	if (ret) {
-		ppgtt->vm.total = 0;
-		return ret;
-	}
+	err = gen8_init_scratch(&ppgtt->vm);
+	if (err)
+		goto err_free;
 
-	if (use_4lvl(vm)) {
-		ret = setup_px(&ppgtt->vm, &ppgtt->pml4);
-		if (ret)
-			goto free_scratch;
+	if (use_4lvl(&ppgtt->vm)) {
+		err = setup_px(&ppgtt->vm, &ppgtt->pml4);
+		if (err)
+			goto err_scratch;
 
 		gen8_initialize_pml4(&ppgtt->vm, &ppgtt->pml4);
 
@@ -1595,15 +1599,15 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		ppgtt->vm.insert_entries = gen8_ppgtt_insert_4lvl;
 		ppgtt->vm.clear_range = gen8_ppgtt_clear_4lvl;
 	} else {
-		ret = __pdp_init(&ppgtt->vm, &ppgtt->pdp);
-		if (ret)
-			goto free_scratch;
+		err = __pdp_init(&ppgtt->vm, &ppgtt->pdp);
+		if (err)
+			goto err_scratch;
 
-		if (intel_vgpu_active(dev_priv)) {
-			ret = gen8_preallocate_top_level_pdp(ppgtt);
-			if (ret) {
+		if (intel_vgpu_active(i915)) {
+			err = gen8_preallocate_top_level_pdp(ppgtt);
+			if (err) {
 				__pdp_fini(&ppgtt->pdp);
-				goto free_scratch;
+				goto err_scratch;
 			}
 		}
 
@@ -1612,7 +1616,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		ppgtt->vm.clear_range = gen8_ppgtt_clear_3lvl;
 	}
 
-	if (intel_vgpu_active(dev_priv))
+	if (intel_vgpu_active(i915))
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
 	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
@@ -1623,11 +1627,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
 	ppgtt->vm.vma_ops.clear_pages = clear_pages;
 
-	return 0;
+	return ppgtt;
 
-free_scratch:
+err_scratch:
 	gen8_free_scratch(&ppgtt->vm);
-	return ret;
+err_free:
+	kfree(ppgtt);
+	return ERR_PTR(err);
 }
 
 static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
@@ -1638,8 +1644,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	u32 pd_entry, pte, pde;
 	u32 start = 0, length = ppgtt->vm.total;
 
-	scratch_pte = vm->pte_encode(vm->scratch_page.daddr,
-				     I915_CACHE_LLC, 0);
+	scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
 
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, pde) {
 		u32 expected;
@@ -2027,36 +2032,41 @@ static void gen6_scratch_va_range(struct i915_hw_ppgtt *ppgtt,
 		ppgtt->pd.page_table[pde] = ppgtt->vm.scratch_pt;
 }
 
-static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
+static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 {
-	struct drm_i915_private *dev_priv = ppgtt->vm.i915;
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	int ret;
+	struct i915_ggtt * const ggtt = &i915->ggtt;
+	struct i915_hw_ppgtt *ppgtt;
+	int err;
+
+	ppgtt = kmalloc(sizeof(*ppgtt), GFP_KERNEL);
+	if (!ppgtt)
+		return ERR_PTR(-ENOMEM);
+
+	ppgtt->vm.i915 = i915;
+	ppgtt->vm.dma = &i915->drm.pdev->dev;
 
 	ppgtt->vm.pte_encode = ggtt->vm.pte_encode;
-	if (intel_vgpu_active(dev_priv) || IS_GEN6(dev_priv))
+	if (intel_vgpu_active(i915) || IS_GEN6(i915))
 		ppgtt->switch_mm = gen6_mm_switch;
-	else if (IS_HASWELL(dev_priv))
+	else if (IS_HASWELL(i915))
 		ppgtt->switch_mm = hsw_mm_switch;
-	else if (IS_GEN7(dev_priv))
+	else if (IS_GEN7(i915))
 		ppgtt->switch_mm = gen7_mm_switch;
 	else
 		BUG();
 
-	ret = gen6_ppgtt_alloc(ppgtt);
-	if (ret)
-		return ret;
+	err = gen6_ppgtt_alloc(ppgtt);
+	if (err)
+		goto err_free;
 
 	ppgtt->vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
 
 	gen6_scratch_va_range(ppgtt, 0, ppgtt->vm.total);
 	gen6_write_page_range(ppgtt, 0, ppgtt->vm.total);
 
-	ret = gen6_alloc_va_range(&ppgtt->vm, 0, ppgtt->vm.total);
-	if (ret) {
-		gen6_ppgtt_cleanup(&ppgtt->vm);
-		return ret;
-	}
+	err = gen6_alloc_va_range(&ppgtt->vm, 0, ppgtt->vm.total);
+	if (err)
+		goto err_cleanup;
 
 	ppgtt->vm.clear_range = gen6_ppgtt_clear_range;
 	ppgtt->vm.insert_entries = gen6_ppgtt_insert_entries;
@@ -2075,19 +2085,13 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
 			 ppgtt->pd.base.ggtt_offset << 10);
 
-	return 0;
-}
-
-static int __hw_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
-			   struct drm_i915_private *dev_priv)
-{
-	ppgtt->vm.i915 = dev_priv;
-	ppgtt->vm.dma = &dev_priv->drm.pdev->dev;
+	return ppgtt;
 
-	if (INTEL_GEN(dev_priv) < 8)
-		return gen6_ppgtt_init(ppgtt);
-	else
-		return gen8_ppgtt_init(ppgtt);
+err_cleanup:
+	gen6_ppgtt_cleanup(&ppgtt->vm);
+err_free:
+	kfree(ppgtt);
+	return ERR_PTR(err);
 }
 
 static void i915_address_space_init(struct i915_address_space *vm,
@@ -2173,26 +2177,28 @@ int i915_ppgtt_init_hw(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
+static struct i915_hw_ppgtt *
+__hw_ppgtt_create(struct drm_i915_private *i915)
+{
+	if (INTEL_GEN(i915) < 8)
+		return gen6_ppgtt_create(i915);
+	else
+		return gen8_ppgtt_create(i915);
+}
+
 struct i915_hw_ppgtt *
-i915_ppgtt_create(struct drm_i915_private *dev_priv,
+i915_ppgtt_create(struct drm_i915_private *i915,
 		  struct drm_i915_file_private *fpriv,
 		  const char *name)
 {
 	struct i915_hw_ppgtt *ppgtt;
-	int ret;
 
-	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
-	if (!ppgtt)
-		return ERR_PTR(-ENOMEM);
-
-	ret = __hw_ppgtt_init(ppgtt, dev_priv);
-	if (ret) {
-		kfree(ppgtt);
-		return ERR_PTR(ret);
-	}
+	ppgtt = __hw_ppgtt_create(i915);
+	if (IS_ERR(ppgtt))
+		return ppgtt;
 
 	kref_init(&ppgtt->ref);
-	i915_address_space_init(&ppgtt->vm, dev_priv, name);
+	i915_address_space_init(&ppgtt->vm, i915, name);
 	ppgtt->vm.file = fpriv;
 
 	trace_i915_ppgtt_create(&ppgtt->vm);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 58ab5e84ceb7..f80cf7ce3fa9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -147,12 +147,16 @@ static int igt_ppgtt_alloc(void *arg)
 		return -ENOMEM;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	err = __hw_ppgtt_init(ppgtt, dev_priv);
-	if (err)
-		goto err_ppgtt;
+	ppgtt = __hw_ppgtt_create(dev_priv);
+	if (IS_ERR(ppgtt)) {
+		err = PTR_ERR(ppgtt);
+		goto err_unlock;
+	}
 
-	if (!ppgtt->vm.allocate_va_range)
+	if (!ppgtt->vm.allocate_va_range) {
+		err = 0;
 		goto err_ppgtt_cleanup;
+	}
 
 	/* Check we can allocate the entire range */
 	for (size = 4096;
@@ -189,9 +193,9 @@ static int igt_ppgtt_alloc(void *arg)
 
 err_ppgtt_cleanup:
 	ppgtt->vm.cleanup(&ppgtt->vm);
-err_ppgtt:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
 	kfree(ppgtt);
+err_unlock:
+	mutex_unlock(&dev_priv->drm.struct_mutex);
 	return err;
 }
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/17] drm/i915/gtt: Subclass gen6_hw_ppgtt

[Chris Wilson]

The legacy gen6 ppgtt needs a little more hand holding than gen8+, and
so requires a larger structure. As I intend to make this slightly more
complicated in the future, separate the gen6 from the core gen8 hw
struct by subclassing. This patch moves the gen6 only features out to
gen6_hw_ppgtt and pipes the new type everywhere that needs it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c           | 108 +++++++++---------
 drivers/gpu/drm/i915/i915_gem_gtt.h           |  21 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c       |  10 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h       |   2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |   4 -
 5 files changed, 77 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f1443ba948a5..cf1b13190957 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1636,20 +1636,20 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	return ERR_PTR(err);
 }
 
-static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
+static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
 {
-	struct i915_address_space *vm = &ppgtt->vm;
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
+	struct i915_address_space *vm = &base->vm;
 	struct i915_page_table *unused;
 	gen6_pte_t scratch_pte;
 	u32 pd_entry, pte, pde;
-	u32 start = 0, length = ppgtt->vm.total;
 
 	scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
 
-	gen6_for_each_pde(unused, &ppgtt->pd, start, length, pde) {
+	gen6_for_all_pdes(unused, &base->pd, pde) {
 		u32 expected;
 		gen6_pte_t *pt_vaddr;
-		const dma_addr_t pt_addr = px_dma(ppgtt->pd.page_table[pde]);
+		const dma_addr_t pt_addr = px_dma(base->pd.page_table[pde]);
 		pd_entry = readl(ppgtt->pd_addr + pde);
 		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
 
@@ -1660,7 +1660,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 				   expected);
 		seq_printf(m, "\tPDE: %x\n", pd_entry);
 
-		pt_vaddr = kmap_atomic_px(ppgtt->pd.page_table[pde]);
+		pt_vaddr = kmap_atomic_px(base->pd.page_table[pde]);
 
 		for (pte = 0; pte < GEN6_PTES; pte+=4) {
 			unsigned long va =
@@ -1688,7 +1688,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 }
 
 /* Write pde (index) from the page directory @pd to the page table @pt */
-static inline void gen6_write_pde(const struct i915_hw_ppgtt *ppgtt,
+static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
 				  const unsigned int pde,
 				  const struct i915_page_table *pt)
 {
@@ -1699,26 +1699,27 @@ static inline void gen6_write_pde(const struct i915_hw_ppgtt *ppgtt,
 
 /* Write all the page tables found in the ppgtt structure to incrementing page
  * directories. */
-static void gen6_write_page_range(struct i915_hw_ppgtt *ppgtt,
+static void gen6_write_page_range(struct i915_hw_ppgtt *base,
 				  u32 start, u32 length)
 {
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	struct i915_page_table *pt;
 	unsigned int pde;
 
-	gen6_for_each_pde(pt, &ppgtt->pd, start, length, pde)
+	gen6_for_each_pde(pt, &base->pd, start, length, pde)
 		gen6_write_pde(ppgtt, pde, pt);
 
-	mark_tlbs_dirty(ppgtt);
-	gen6_ggtt_invalidate(ppgtt->vm.i915);
+	mark_tlbs_dirty(base);
+	gen6_ggtt_invalidate(base->vm.i915);
 }
 
-static inline u32 get_pd_offset(struct i915_hw_ppgtt *ppgtt)
+static inline u32 get_pd_offset(struct gen6_hw_ppgtt *ppgtt)
 {
-	GEM_BUG_ON(ppgtt->pd.base.ggtt_offset & 0x3f);
-	return ppgtt->pd.base.ggtt_offset << 10;
+	GEM_BUG_ON(ppgtt->base.pd.base.ggtt_offset & 0x3f);
+	return ppgtt->base.pd.base.ggtt_offset << 10;
 }
 
-static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
+static int hsw_mm_switch(struct gen6_hw_ppgtt *ppgtt,
 			 struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
@@ -1740,7 +1741,7 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
 	return 0;
 }
 
-static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
+static int gen7_mm_switch(struct gen6_hw_ppgtt *ppgtt,
 			  struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
@@ -1762,7 +1763,7 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
 	return 0;
 }
 
-static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
+static int gen6_mm_switch(struct gen6_hw_ppgtt *ppgtt,
 			  struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
@@ -1904,28 +1905,28 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 static int gen6_alloc_va_range(struct i915_address_space *vm,
 			       u64 start, u64 length)
 {
-	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 	struct i915_page_table *pt;
 	u64 from = start;
 	unsigned int pde;
 	bool flush = false;
 
-	gen6_for_each_pde(pt, &ppgtt->pd, start, length, pde) {
+	gen6_for_each_pde(pt, &ppgtt->base.pd, start, length, pde) {
 		if (pt == vm->scratch_pt) {
 			pt = alloc_pt(vm);
 			if (IS_ERR(pt))
 				goto unwind_out;
 
 			gen6_initialize_pt(vm, pt);
-			ppgtt->pd.page_table[pde] = pt;
+			ppgtt->base.pd.page_table[pde] = pt;
 			gen6_write_pde(ppgtt, pde, pt);
 			flush = true;
 		}
 	}
 
 	if (flush) {
-		mark_tlbs_dirty(ppgtt);
-		gen6_ggtt_invalidate(ppgtt->vm.i915);
+		mark_tlbs_dirty(&ppgtt->base);
+		gen6_ggtt_invalidate(ppgtt->base.vm.i915);
 	}
 
 	return 0;
@@ -1962,24 +1963,23 @@ static void gen6_free_scratch(struct i915_address_space *vm)
 
 static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
-	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
-	struct i915_page_directory *pd = &ppgtt->pd;
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 	struct i915_page_table *pt;
 	u32 pde;
 
 	drm_mm_remove_node(&ppgtt->node);
 
-	gen6_for_all_pdes(pt, pd, pde)
+	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
 		if (pt != vm->scratch_pt)
 			free_pt(vm, pt);
 
 	gen6_free_scratch(vm);
 }
 
-static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
+static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
 {
-	struct i915_address_space *vm = &ppgtt->vm;
-	struct drm_i915_private *dev_priv = ppgtt->vm.i915;
+	struct i915_address_space *vm = &ppgtt->base.vm;
+	struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	int ret;
 
@@ -2004,11 +2004,11 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
 	if (ppgtt->node.start < ggtt->mappable_end)
 		DRM_DEBUG("Forced to use aperture for PDEs\n");
 
-	ppgtt->pd.base.ggtt_offset =
+	ppgtt->base.pd.base.ggtt_offset =
 		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
 
 	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm +
-		ppgtt->pd.base.ggtt_offset / sizeof(gen6_pte_t);
+		ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
 
 	return 0;
 
@@ -2017,35 +2017,35 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
 	return ret;
 }
 
-static int gen6_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt)
+static int gen6_ppgtt_alloc(struct gen6_hw_ppgtt *ppgtt)
 {
 	return gen6_ppgtt_allocate_page_directories(ppgtt);
 }
 
-static void gen6_scratch_va_range(struct i915_hw_ppgtt *ppgtt,
+static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
 				  u64 start, u64 length)
 {
 	struct i915_page_table *unused;
 	u32 pde;
 
-	gen6_for_each_pde(unused, &ppgtt->pd, start, length, pde)
-		ppgtt->pd.page_table[pde] = ppgtt->vm.scratch_pt;
+	gen6_for_each_pde(unused, &ppgtt->base.pd, start, length, pde)
+		ppgtt->base.pd.page_table[pde] = ppgtt->base.vm.scratch_pt;
 }
 
 static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 {
 	struct i915_ggtt * const ggtt = &i915->ggtt;
-	struct i915_hw_ppgtt *ppgtt;
+	struct gen6_hw_ppgtt *ppgtt;
 	int err;
 
-	ppgtt = kmalloc(sizeof(*ppgtt), GFP_KERNEL);
+	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
 	if (!ppgtt)
 		return ERR_PTR(-ENOMEM);
 
-	ppgtt->vm.i915 = i915;
-	ppgtt->vm.dma = &i915->drm.pdev->dev;
+	ppgtt->base.vm.i915 = i915;
+	ppgtt->base.vm.dma = &i915->drm.pdev->dev;
 
-	ppgtt->vm.pte_encode = ggtt->vm.pte_encode;
+	ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode;
 	if (intel_vgpu_active(i915) || IS_GEN6(i915))
 		ppgtt->switch_mm = gen6_mm_switch;
 	else if (IS_HASWELL(i915))
@@ -2059,36 +2059,36 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	if (err)
 		goto err_free;
 
-	ppgtt->vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
+	ppgtt->base.vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
 
-	gen6_scratch_va_range(ppgtt, 0, ppgtt->vm.total);
-	gen6_write_page_range(ppgtt, 0, ppgtt->vm.total);
+	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.vm.total);
+	gen6_write_page_range(&ppgtt->base, 0, ppgtt->base.vm.total);
 
-	err = gen6_alloc_va_range(&ppgtt->vm, 0, ppgtt->vm.total);
+	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
 	if (err)
 		goto err_cleanup;
 
-	ppgtt->vm.clear_range = gen6_ppgtt_clear_range;
-	ppgtt->vm.insert_entries = gen6_ppgtt_insert_entries;
-	ppgtt->vm.cleanup = gen6_ppgtt_cleanup;
-	ppgtt->debug_dump = gen6_dump_ppgtt;
+	ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range;
+	ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries;
+	ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
+	ppgtt->base.debug_dump = gen6_dump_ppgtt;
 
-	ppgtt->vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
-	ppgtt->vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
-	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
-	ppgtt->vm.vma_ops.clear_pages = clear_pages;
+	ppgtt->base.vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
+	ppgtt->base.vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
+	ppgtt->base.vm.vma_ops.set_pages   = ppgtt_set_pages;
+	ppgtt->base.vm.vma_ops.clear_pages = clear_pages;
 
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 			 ppgtt->node.size >> 20,
 			 ppgtt->node.start / PAGE_SIZE);
 
 	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
-			 ppgtt->pd.base.ggtt_offset << 10);
+			 ppgtt->base.pd.base.ggtt_offset << 10);
 
-	return ppgtt;
+	return &ppgtt->base;
 
 err_cleanup:
-	gen6_ppgtt_cleanup(&ppgtt->vm);
+	gen6_ppgtt_cleanup(&ppgtt->base.vm);
 err_free:
 	kfree(ppgtt);
 	return ERR_PTR(err);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 16307ba7e303..199d6f47a557 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -396,7 +396,7 @@ struct i915_ggtt {
 struct i915_hw_ppgtt {
 	struct i915_address_space vm;
 	struct kref ref;
-	struct drm_mm_node node;
+
 	unsigned long pd_dirty_rings;
 	union {
 		struct i915_pml4 pml4;		/* GEN8+ & 48b PPGTT */
@@ -404,13 +404,26 @@ struct i915_hw_ppgtt {
 		struct i915_page_directory pd;		/* GEN6-7 */
 	};
 
+	void (*debug_dump)(struct i915_hw_ppgtt *ppgtt, struct seq_file *m);
+};
+
+struct gen6_hw_ppgtt {
+	struct i915_hw_ppgtt base;
+
+	struct drm_mm_node node;
 	gen6_pte_t __iomem *pd_addr;
 
-	int (*switch_mm)(struct i915_hw_ppgtt *ppgtt,
-			 struct i915_request *rq);
-	void (*debug_dump)(struct i915_hw_ppgtt *ppgtt, struct seq_file *m);
+	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
 };
 
+#define __to_gen6_ppgtt(base) container_of(base, struct gen6_hw_ppgtt, base)
+
+static inline struct gen6_hw_ppgtt *to_gen6_ppgtt(struct i915_hw_ppgtt *base)
+{
+	BUILD_BUG_ON(offsetof(struct gen6_hw_ppgtt, base));
+	return __to_gen6_ppgtt(base);
+}
+
 /*
  * gen6_for_each_pde() iterates over every pde from start until start+length.
  * If start and start+length are not perfectly divisible, the macro will round
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 65811e2fa7da..7ea4660f13e1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1566,10 +1566,10 @@ static int switch_context(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct i915_gem_context *to_ctx = rq->gem_context;
-	struct i915_hw_ppgtt *to_mm =
-		to_ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
+	struct gen6_hw_ppgtt *to_mm =
+		to_gen6_ppgtt(to_ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt);
 	struct i915_gem_context *from_ctx = engine->legacy_active_context;
-	struct i915_hw_ppgtt *from_mm = engine->legacy_active_ppgtt;
+	struct gen6_hw_ppgtt *from_mm = engine->legacy_active_ppgtt;
 	u32 hw_flags = 0;
 	int ret, i;
 
@@ -1577,13 +1577,13 @@ static int switch_context(struct i915_request *rq)
 	GEM_BUG_ON(HAS_EXECLISTS(rq->i915));
 
 	if (to_mm != from_mm ||
-	    (to_mm && intel_engine_flag(engine) & to_mm->pd_dirty_rings)) {
+	    (to_mm && intel_engine_flag(engine) & to_mm->base.pd_dirty_rings)) {
 		trace_switch_mm(engine, to_ctx);
 		ret = to_mm->switch_mm(to_mm, rq);
 		if (ret)
 			goto err;
 
-		to_mm->pd_dirty_rings &= ~intel_engine_flag(engine);
+		to_mm->base.pd_dirty_rings &= ~intel_engine_flag(engine);
 		engine->legacy_active_ppgtt = to_mm;
 		hw_flags = MI_FORCE_RESTORE;
 	}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index acef385c4c80..83cf55e44c08 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -564,7 +564,7 @@ struct intel_engine_cs {
 	 * stream (ring).
 	 */
 	struct i915_gem_context *legacy_active_context;
-	struct i915_hw_ppgtt *legacy_active_ppgtt;
+	struct gen6_hw_ppgtt *legacy_active_ppgtt;
 
 	/* status_notifier: list of callbacks for context-switch changes */
 	struct atomic_notifier_head context_status_notifier;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index f80cf7ce3fa9..538e658252f7 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -142,10 +142,6 @@ static int igt_ppgtt_alloc(void *arg)
 	if (!USES_PPGTT(dev_priv))
 		return 0;
 
-	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
-	if (!ppgtt)
-		return -ENOMEM;
-
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	ppgtt = __hw_ppgtt_create(dev_priv);
 	if (IS_ERR(ppgtt)) {
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/17] drm/i915/gtt Onionify error handling for gen6_ppgtt_create

[Chris Wilson]

Pull the empty stubs together into the top level gen6_ppgtt_create, and
tear each one down on error in proper onion order (rather than use
Joonas' pet hate of calling the cleanup function in indeterminable
state).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 81 ++++++++++++++---------------
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index cf1b13190957..ff8eebd5c714 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1936,7 +1936,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	return -ENOMEM;
 }
 
-static int gen6_init_scratch(struct i915_address_space *vm)
+static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
 {
 	int ret;
 
@@ -1955,33 +1955,37 @@ static int gen6_init_scratch(struct i915_address_space *vm)
 	return 0;
 }
 
-static void gen6_free_scratch(struct i915_address_space *vm)
+static void gen6_ppgtt_free_scratch(struct i915_address_space *vm)
 {
 	free_pt(vm, vm->scratch_pt);
 	cleanup_scratch_page(vm);
 }
 
-static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
+static void gen6_ppgtt_free_pd(struct gen6_hw_ppgtt *ppgtt)
 {
-	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 	struct i915_page_table *pt;
 	u32 pde;
 
-	drm_mm_remove_node(&ppgtt->node);
-
 	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
-		if (pt != vm->scratch_pt)
-			free_pt(vm, pt);
+		if (pt != ppgtt->base.vm.scratch_pt)
+			free_pt(&ppgtt->base.vm, pt);
+}
 
-	gen6_free_scratch(vm);
+static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
+{
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
+
+	drm_mm_remove_node(&ppgtt->node);
+
+	gen6_ppgtt_free_pd(ppgtt);
+	gen6_ppgtt_free_scratch(vm);
 }
 
 static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
 {
-	struct i915_address_space *vm = &ppgtt->base.vm;
 	struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	int ret;
+	int err;
 
 	/* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
 	 * allocator works in address space sizes, so it's multiplied by page
@@ -1989,17 +1993,13 @@ static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
 	 */
 	BUG_ON(!drm_mm_initialized(&ggtt->vm.mm));
 
-	ret = gen6_init_scratch(vm);
-	if (ret)
-		return ret;
-
-	ret = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
+	err = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
 				  GEN6_PD_SIZE, GEN6_PD_ALIGN,
 				  I915_COLOR_UNEVICTABLE,
 				  0, ggtt->vm.total,
 				  PIN_HIGH);
-	if (ret)
-		goto err_out;
+	if (err)
+		return err;
 
 	if (ppgtt->node.start < ggtt->mappable_end)
 		DRM_DEBUG("Forced to use aperture for PDEs\n");
@@ -2011,15 +2011,6 @@ static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
 		ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
 
 	return 0;
-
-err_out:
-	gen6_free_scratch(vm);
-	return ret;
-}
-
-static int gen6_ppgtt_alloc(struct gen6_hw_ppgtt *ppgtt)
-{
-	return gen6_ppgtt_allocate_page_directories(ppgtt);
 }
 
 static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
@@ -2045,6 +2036,18 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	ppgtt->base.vm.i915 = i915;
 	ppgtt->base.vm.dma = &i915->drm.pdev->dev;
 
+	ppgtt->base.vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
+
+	ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range;
+	ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries;
+	ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
+	ppgtt->base.debug_dump = gen6_dump_ppgtt;
+
+	ppgtt->base.vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
+	ppgtt->base.vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
+	ppgtt->base.vm.vma_ops.set_pages   = ppgtt_set_pages;
+	ppgtt->base.vm.vma_ops.clear_pages = clear_pages;
+
 	ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode;
 	if (intel_vgpu_active(i915) || IS_GEN6(i915))
 		ppgtt->switch_mm = gen6_mm_switch;
@@ -2055,28 +2058,20 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	else
 		BUG();
 
-	err = gen6_ppgtt_alloc(ppgtt);
+	err = gen6_ppgtt_init_scratch(&ppgtt->base.vm);
 	if (err)
 		goto err_free;
 
-	ppgtt->base.vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
+	err = gen6_ppgtt_allocate_page_directories(ppgtt);
+	if (err)
+		goto err_scratch;
 
 	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.vm.total);
 	gen6_write_page_range(&ppgtt->base, 0, ppgtt->base.vm.total);
 
 	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
 	if (err)
-		goto err_cleanup;
-
-	ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range;
-	ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries;
-	ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
-	ppgtt->base.debug_dump = gen6_dump_ppgtt;
-
-	ppgtt->base.vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
-	ppgtt->base.vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
-	ppgtt->base.vm.vma_ops.set_pages   = ppgtt_set_pages;
-	ppgtt->base.vm.vma_ops.clear_pages = clear_pages;
+		goto err_pd;
 
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 			 ppgtt->node.size >> 20,
@@ -2087,8 +2082,10 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 	return &ppgtt->base;
 
-err_cleanup:
-	gen6_ppgtt_cleanup(&ppgtt->base.vm);
+err_pd:
+	gen6_ppgtt_free_pd(ppgtt);
+err_scratch:
+	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
 err_free:
 	kfree(ppgtt);
 	return ERR_PTR(err);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/17] drm/i915/gtt: Reorder aliasing_ppgtt fini

[Chris Wilson]

To allow ourselves to use a first class vma for the aliasing_ppgtt page
directory, we have to reorder the shutdown on module unload to remove
and unpin the aliasing_ppgtt before complaining about any objects left
in the GGTT.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ff8eebd5c714..ea27f799101f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2891,15 +2891,11 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 	ggtt->vm.closed = true;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
+	i915_gem_fini_aliasing_ppgtt(dev_priv);
+
 	GEM_BUG_ON(!list_empty(&ggtt->vm.active_list));
 	list_for_each_entry_safe(vma, vn, &ggtt->vm.inactive_list, vm_link)
 		WARN_ON(i915_vma_unbind(vma));
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-
-	i915_gem_cleanup_stolen(&dev_priv->drm);
-
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	i915_gem_fini_aliasing_ppgtt(dev_priv);
 
 	if (drm_mm_node_allocated(&ggtt->error_capture))
 		drm_mm_remove_node(&ggtt->error_capture);
@@ -2921,6 +2917,8 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 
 	arch_phys_wc_del(ggtt->mtrr);
 	io_mapping_fini(&ggtt->iomap);
+
+	i915_gem_cleanup_stolen(&dev_priv->drm);
 }
 
 static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/17] drm/i915/gtt: Make gen6 page directories evictable

[Chris Wilson]

Currently all page directories are bound at creation using an
unevictable node in the GGTT. This severely limits us as we cannot
remove any inactive ppgtt for new contexts, or under aperture pressure.
To fix this we need to make the page directory into a first class and
unbindable vma. Hence, the creation of a custom vma to wrap the page
directory as opposed to a GEM object.

In this patch, we leave the page directories pinned upon creation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 276 ++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   2 +-
 drivers/gpu/drm/i915/i915_vma.h     |   7 +
 3 files changed, 164 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ea27f799101f..28ab76684f99 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1640,79 +1640,58 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	struct i915_address_space *vm = &base->vm;
-	struct i915_page_table *unused;
-	gen6_pte_t scratch_pte;
-	u32 pd_entry, pte, pde;
-
-	scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
-
-	gen6_for_all_pdes(unused, &base->pd, pde) {
-		u32 expected;
-		gen6_pte_t *pt_vaddr;
-		const dma_addr_t pt_addr = px_dma(base->pd.page_table[pde]);
-		pd_entry = readl(ppgtt->pd_addr + pde);
-		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
-
-		if (pd_entry != expected)
-			seq_printf(m, "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
-				   pde,
-				   pd_entry,
-				   expected);
-		seq_printf(m, "\tPDE: %x\n", pd_entry);
-
-		pt_vaddr = kmap_atomic_px(base->pd.page_table[pde]);
-
-		for (pte = 0; pte < GEN6_PTES; pte+=4) {
-			unsigned long va =
-				(pde * PAGE_SIZE * GEN6_PTES) +
-				(pte * PAGE_SIZE);
+	const gen6_pte_t scratch_pte =
+		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
+	struct i915_page_table *pt;
+	u32 pte, pde;
+
+	gen6_for_all_pdes(pt, &base->pd, pde) {
+		gen6_pte_t *vaddr;
+
+		if (pt == base->vm.scratch_pt)
+			continue;
+
+		if (i915_vma_is_bound(ppgtt->vma, I915_VMA_GLOBAL_BIND)) {
+			u32 expected =
+				GEN6_PDE_ADDR_ENCODE(px_dma(pt)) |
+				GEN6_PDE_VALID;
+			u32 pd_entry = readl(ppgtt->pd_addr + pde);
+
+			if (pd_entry != expected)
+				seq_printf(m,
+					   "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
+					   pde,
+					   pd_entry,
+					   expected);
+
+			seq_printf(m, "\tPDE: %x\n", pd_entry);
+		}
+
+		vaddr = kmap_atomic_px(base->pd.page_table[pde]);
+		for (pte = 0; pte < GEN6_PTES; pte += 4) {
 			int i;
-			bool found = false;
+
 			for (i = 0; i < 4; i++)
-				if (pt_vaddr[pte + i] != scratch_pte)
-					found = true;
-			if (!found)
+				if (vaddr[pte + i] != scratch_pte)
+					break;
+			if (i == 4)
 				continue;
 
-			seq_printf(m, "\t\t0x%lx [%03d,%04d]: =", va, pde, pte);
+			seq_printf(m, "\t\t(%03d, %04d) %08lx: ",
+				   pde, pte,
+				   (pde * GEN6_PTES + pte) * PAGE_SIZE);
 			for (i = 0; i < 4; i++) {
-				if (pt_vaddr[pte + i] != scratch_pte)
-					seq_printf(m, " %08x", pt_vaddr[pte + i]);
+				if (vaddr[pte + i] != scratch_pte)
+					seq_printf(m, " %08x", vaddr[pte + i]);
 				else
-					seq_puts(m, "  SCRATCH ");
+					seq_puts(m, "  SCRATCH");
 			}
 			seq_puts(m, "\n");
 		}
-		kunmap_atomic(pt_vaddr);
+		kunmap_atomic(vaddr);
 	}
 }
 
-/* Write pde (index) from the page directory @pd to the page table @pt */
-static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
-				  const unsigned int pde,
-				  const struct i915_page_table *pt)
-{
-	/* Caller needs to make sure the write completes if necessary */
-	iowrite32(GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID,
-		  ppgtt->pd_addr + pde);
-}
-
-/* Write all the page tables found in the ppgtt structure to incrementing page
- * directories. */
-static void gen6_write_page_range(struct i915_hw_ppgtt *base,
-				  u32 start, u32 length)
-{
-	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
-	struct i915_page_table *pt;
-	unsigned int pde;
-
-	gen6_for_each_pde(pt, &base->pd, start, length, pde)
-		gen6_write_pde(ppgtt, pde, pt);
-
-	mark_tlbs_dirty(base);
-	gen6_ggtt_invalidate(base->vm.i915);
-}
-
 static inline u32 get_pd_offset(struct gen6_hw_ppgtt *ppgtt)
 {
 	GEM_BUG_ON(ppgtt->base.pd.base.ggtt_offset & 0x3f);
@@ -1919,8 +1898,15 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 			gen6_initialize_pt(vm, pt);
 			ppgtt->base.pd.page_table[pde] = pt;
-			gen6_write_pde(ppgtt, pde, pt);
-			flush = true;
+
+			if (i915_vma_is_bound(ppgtt->vma,
+					      I915_VMA_GLOBAL_BIND)) {
+				u32 addr = GEN6_PDE_ADDR_ENCODE(px_dma(pt));
+
+				iowrite32(addr | GEN6_PDE_VALID,
+					  ppgtt->pd_addr + pde);
+				flush = true;
+			}
 		}
 	}
 
@@ -1936,8 +1922,11 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	return -ENOMEM;
 }
 
-static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
+static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
 {
+	struct i915_address_space * const vm = &ppgtt->base.vm;
+	struct i915_page_table *unused;
+	u32 pde;
 	int ret;
 
 	ret = setup_scratch_page(vm, __GFP_HIGHMEM);
@@ -1951,6 +1940,8 @@ static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen6_initialize_pt(vm, vm->scratch_pt);
+	gen6_for_all_pdes(unused, &ppgtt->base.pd, pde)
+		ppgtt->base.pd.page_table[pde] = vm->scratch_pt;
 
 	return 0;
 }
@@ -1975,52 +1966,112 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	drm_mm_remove_node(&ppgtt->node);
+	i915_vma_unpin(ppgtt->vma);
+	i915_vma_destroy(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
 	gen6_ppgtt_free_scratch(vm);
 }
 
-static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
+static int pd_vma_set_pages(struct i915_vma *vma)
 {
-	struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	int err;
+	vma->pages = ERR_PTR(-ENODEV);
+	return 0;
+}
 
-	/* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
-	 * allocator works in address space sizes, so it's multiplied by page
-	 * size. We allocate at the top of the GTT to avoid fragmentation.
-	 */
-	BUG_ON(!drm_mm_initialized(&ggtt->vm.mm));
+static void pd_vma_clear_pages(struct i915_vma *vma)
+{
+	GEM_BUG_ON(!vma->pages);
 
-	err = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
-				  GEN6_PD_SIZE, GEN6_PD_ALIGN,
-				  I915_COLOR_UNEVICTABLE,
-				  0, ggtt->vm.total,
-				  PIN_HIGH);
-	if (err)
-		return err;
+	vma->pages = NULL;
+}
 
-	if (ppgtt->node.start < ggtt->mappable_end)
-		DRM_DEBUG("Forced to use aperture for PDEs\n");
+static int pd_vma_bind(struct i915_vma *vma,
+		       enum i915_cache_level cache_level,
+		       u32 unused)
+{
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
+	struct gen6_hw_ppgtt *ppgtt = vma->private;
+	u32 ggtt_offset = i915_ggtt_offset(vma) / PAGE_SIZE;
+	struct i915_page_table *pt;
+	unsigned int pde;
 
-	ppgtt->base.pd.base.ggtt_offset =
-		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
+	ppgtt->base.pd.base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
+	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm + ggtt_offset;
 
-	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm +
-		ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
+	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde) {
+		u32 val = GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID;
+
+		writel_relaxed(val, ppgtt->pd_addr + pde);
+	}
+	mark_tlbs_dirty(&ppgtt->base);
+	wmb();
 
 	return 0;
 }
 
-static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
-				  u64 start, u64 length)
+static void pd_vma_unbind(struct i915_vma *vma)
 {
-	struct i915_page_table *unused;
-	u32 pde;
+}
+
+static const struct i915_vma_ops pd_vma_ops = {
+	.set_pages = pd_vma_set_pages,
+	.clear_pages = pd_vma_clear_pages,
+	.bind_vma = pd_vma_bind,
+	.unbind_vma = pd_vma_unbind,
+};
+
+static struct i915_vma *pd_vma_create(struct gen6_hw_ppgtt *ppgtt, int size)
+{
+	struct drm_i915_private *i915 = ppgtt->base.vm.i915;
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct i915_vma *vma;
+	int i;
+
+	GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE));
+	GEM_BUG_ON(size > ggtt->vm.total);
+
+	vma = kmem_cache_zalloc(i915->vmas, GFP_KERNEL);
+	if (!vma)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
+		init_request_active(&vma->last_read[i], NULL);
+	init_request_active(&vma->last_fence, NULL);
+
+	vma->vm = &ggtt->vm;
+	vma->ops = &pd_vma_ops;
+	vma->private = ppgtt;
+
+	vma->obj = NULL;
+	vma->resv = NULL;
+	vma->size = size;
+	vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->fence_size = size;
+	vma->fence_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->flags |= I915_VMA_GGTT;
+	vma->ggtt_view.type = I915_GGTT_VIEW_ROTATED; /* prevent fencing */
+
+	INIT_LIST_HEAD(&vma->obj_link);
+	list_add(&vma->vm_link, &vma->vm->unbound_list);
 
-	gen6_for_each_pde(unused, &ppgtt->base.pd, start, length, pde)
-		ppgtt->base.pd.page_table[pde] = ppgtt->base.vm.scratch_pt;
+	return vma;
+}
+
+static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
+{
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
+
+	/*
+	 * PPGTT PDEs reside in the GGTT and consists of 512 entries. The
+	 * allocator works in address space sizes, so it's multiplied by page
+	 * size. We allocate at the top of the GTT to avoid fragmentation.
+	 */
+	return i915_vma_pin(ppgtt->vma,
+			    0, GEN6_PD_ALIGN,
+			    PIN_GLOBAL | PIN_HIGH);
 }
 
 static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
@@ -2058,24 +2109,25 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	else
 		BUG();
 
-	err = gen6_ppgtt_init_scratch(&ppgtt->base.vm);
+	err = gen6_ppgtt_init_scratch(ppgtt);
 	if (err)
 		goto err_free;
 
-	err = gen6_ppgtt_allocate_page_directories(ppgtt);
-	if (err)
+	ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
+	if (IS_ERR(ppgtt->vma))
 		goto err_scratch;
 
-	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.vm.total);
-	gen6_write_page_range(&ppgtt->base, 0, ppgtt->base.vm.total);
-
 	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
+	if (err)
+		goto err_vma;
+
+	err = gen6_ppgtt_pin(&ppgtt->base);
 	if (err)
 		goto err_pd;
 
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
-			 ppgtt->node.size >> 20,
-			 ppgtt->node.start / PAGE_SIZE);
+			 ppgtt->vma->node.size >> 20,
+			 ppgtt->vma->node.start / PAGE_SIZE);
 
 	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
 			 ppgtt->base.pd.base.ggtt_offset << 10);
@@ -2084,6 +2136,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 err_pd:
 	gen6_ppgtt_free_pd(ppgtt);
+err_vma:
+	i915_vma_destroy(ppgtt->vma);
 err_scratch:
 	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
 err_free:
@@ -3602,6 +3656,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 	}
 
 	ggtt->vm.closed = false;
+	i915_ggtt_invalidate(dev_priv);
 
 	if (INTEL_GEN(dev_priv) >= 8) {
 		struct intel_ppat *ppat = &dev_priv->ppat;
@@ -3610,25 +3665,6 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		dev_priv->ppat.update_hw(dev_priv);
 		return;
 	}
-
-	if (USES_PPGTT(dev_priv)) {
-		struct i915_address_space *vm;
-
-		list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
-			struct i915_hw_ppgtt *ppgtt;
-
-			if (i915_is_ggtt(vm))
-				ppgtt = dev_priv->mm.aliasing_ppgtt;
-			else
-				ppgtt = i915_vm_to_ppgtt(vm);
-			if (!ppgtt)
-				continue;
-
-			gen6_write_page_range(ppgtt, 0, ppgtt->vm.total);
-		}
-	}
-
-	i915_ggtt_invalidate(dev_priv);
 }
 
 static struct scatterlist *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 199d6f47a557..c2f270c90bea 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -410,7 +410,7 @@ struct i915_hw_ppgtt {
 struct gen6_hw_ppgtt {
 	struct i915_hw_ppgtt base;
 
-	struct drm_mm_node node;
+	struct i915_vma *vma;
 	gen6_pte_t __iomem *pd_addr;
 
 	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4321476a6a32..66a228931517 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -54,6 +54,7 @@ struct i915_vma {
 	struct reservation_object *resv; /** Alias of obj->resv */
 	struct sg_table *pages;
 	void __iomem *iomap;
+	void *private; /* owned by creator */
 	u64 size;
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
@@ -340,6 +341,12 @@ static inline void i915_vma_unpin(struct i915_vma *vma)
 	__i915_vma_unpin(vma);
 }
 
+static inline bool i915_vma_is_bound(const struct i915_vma *vma,
+				     unsigned int where)
+{
+	return vma->flags & where;
+}
+
 /**
  * i915_vma_pin_iomap - calls ioremap_wc to map the GGTT VMA via the aperture
  * @vma: VMA to iomap
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/17] drm/i915/gtt: Only keep gen6 page directories pinned while active

[Chris Wilson]

In order to be able to evict the gen6 ppgtt, we have to unpin it at some
point. We can simply use our context activity tracking to know when the
ppgtt is no longer in use by hardware, and so only keep it pinned while
being used a request.

For the kernel_context (and thus aliasing_ppgtt), it remains pinned at
all times, as the kernel_context itself is pinned at all times.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c     | 36 ++++++++++++++-----------
 drivers/gpu/drm/i915/i915_gem_gtt.h     |  5 ++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 28 +++++++++++++++++++
 3 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 28ab76684f99..6aea9caf2f24 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1966,7 +1966,6 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	i915_vma_unpin(ppgtt->vma);
 	i915_vma_destroy(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
@@ -2060,10 +2059,19 @@ static struct i915_vma *pd_vma_create(struct gen6_hw_ppgtt *ppgtt, int size)
 	return vma;
 }
 
-static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
+int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
 
+	/*
+	 * Workaround the limited maximum vma->pin_count and the aliasing_ppgtt
+	 * which will be pinned into every active context.
+	 * (When vma->pin_count becomes atomic, I expect we will naturally
+	 * need a larger, unpacked, type and kill this redundancy.)
+	 */
+	if (ppgtt->pin_count++)
+		return 0;
+
 	/*
 	 * PPGTT PDEs reside in the GGTT and consists of 512 entries. The
 	 * allocator works in address space sizes, so it's multiplied by page
@@ -2074,6 +2082,17 @@ static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
 			    PIN_GLOBAL | PIN_HIGH);
 }
 
+void gen6_ppgtt_unpin(struct i915_hw_ppgtt *base)
+{
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
+
+	GEM_BUG_ON(!ppgtt->pin_count);
+	if (--ppgtt->pin_count)
+		return;
+
+	i915_vma_unpin(ppgtt->vma);
+}
+
 static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 {
 	struct i915_ggtt * const ggtt = &i915->ggtt;
@@ -2121,21 +2140,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	if (err)
 		goto err_vma;
 
-	err = gen6_ppgtt_pin(&ppgtt->base);
-	if (err)
-		goto err_pd;
-
-	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
-			 ppgtt->vma->node.size >> 20,
-			 ppgtt->vma->node.start / PAGE_SIZE);
-
-	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
-			 ppgtt->base.pd.base.ggtt_offset << 10);
-
 	return &ppgtt->base;
 
-err_pd:
-	gen6_ppgtt_free_pd(ppgtt);
 err_vma:
 	i915_vma_destroy(ppgtt->vma);
 err_scratch:
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c2f270c90bea..c20a4f06db37 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -413,6 +413,8 @@ struct gen6_hw_ppgtt {
 	struct i915_vma *vma;
 	gen6_pte_t __iomem *pd_addr;
 
+	unsigned int pin_count;
+
 	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
 };
 
@@ -627,6 +629,9 @@ static inline void i915_ppgtt_put(struct i915_hw_ppgtt *ppgtt)
 		kref_put(&ppgtt->ref, i915_ppgtt_release);
 }
 
+int gen6_ppgtt_pin(struct i915_hw_ppgtt *base);
+void gen6_ppgtt_unpin(struct i915_hw_ppgtt *base);
+
 void i915_check_and_clear_faults(struct drm_i915_private *dev_priv);
 void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv);
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 7ea4660f13e1..fb8d6217b03c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1195,6 +1195,27 @@ static void intel_ring_context_destroy(struct intel_context *ce)
 		__i915_gem_object_release_unless_active(ce->state->obj);
 }
 
+static int __context_pin_ppgtt(struct i915_gem_context *ctx)
+{
+	struct i915_hw_ppgtt *ppgtt;
+	int err = 0;
+
+	ppgtt = ctx->ppgtt ?: ctx->i915->mm.aliasing_ppgtt;
+	if (ppgtt)
+		err = gen6_ppgtt_pin(ppgtt);
+
+	return err;
+}
+
+static void __context_unpin_ppgtt(struct i915_gem_context *ctx)
+{
+	struct i915_hw_ppgtt *ppgtt;
+
+	ppgtt = ctx->ppgtt ?: ctx->i915->mm.aliasing_ppgtt;
+	if (ppgtt)
+		gen6_ppgtt_unpin(ppgtt);
+}
+
 static int __context_pin(struct intel_context *ce)
 {
 	struct i915_vma *vma;
@@ -1243,6 +1264,7 @@ static void __context_unpin(struct intel_context *ce)
 
 static void intel_ring_context_unpin(struct intel_context *ce)
 {
+	__context_unpin_ppgtt(ce->gem_context);
 	__context_unpin(ce);
 
 	i915_gem_context_put(ce->gem_context);
@@ -1340,6 +1362,10 @@ __ring_context_pin(struct intel_engine_cs *engine,
 	if (err)
 		goto err;
 
+	err = __context_pin_ppgtt(ce->gem_context);
+	if (err)
+		goto err_unpin;
+
 	i915_gem_context_get(ctx);
 
 	/* One ringbuffer to rule them all */
@@ -1348,6 +1374,8 @@ __ring_context_pin(struct intel_engine_cs *engine,
 
 	return ce;
 
+err_unpin:
+	__context_unpin(ce);
 err:
 	ce->pin_count = 0;
 	return ERR_PTR(err);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/17] drm/i915/gtt: Lazily allocate page directories for gen7

[Chris Wilson]

As we were only supporting aliasing_ppgtt on gen7 for some time, we
saved a few checks by preallocating the page directories on creation.
However, since we need 2MiB of page directories for each ppgtt, to
support arbitrary numbers of user contexts, we need to be more prudent
in our allocations, and defer the page allocation until it is used. We
don't recover unused pages yet as we found that doing so on the fly
(i.e. altering TLB entries) would confuse the GPU.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 67 +++++++++++------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6aea9caf2f24..d29e49226f33 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -190,11 +190,19 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 	return 1;
 }
 
-static int gen6_ppgtt_bind_vma(struct i915_vma *vma,
-			       enum i915_cache_level cache_level,
-			       u32 unused)
+static int ppgtt_bind_vma(struct i915_vma *vma,
+			  enum i915_cache_level cache_level,
+			  u32 unused)
 {
 	u32 pte_flags;
+	int err;
+
+	if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
+		err = vma->vm->allocate_va_range(vma->vm,
+						 vma->node.start, vma->size);
+		if (err)
+			return err;
+	}
 
 	/* Currently applicable only to VLV */
 	pte_flags = 0;
@@ -206,22 +214,6 @@ static int gen6_ppgtt_bind_vma(struct i915_vma *vma,
 	return 0;
 }
 
-static int gen8_ppgtt_bind_vma(struct i915_vma *vma,
-			       enum i915_cache_level cache_level,
-			       u32 unused)
-{
-	int ret;
-
-	if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
-		ret = vma->vm->allocate_va_range(vma->vm,
-						 vma->node.start, vma->size);
-		if (ret)
-			return ret;
-	}
-
-	return gen6_ppgtt_bind_vma(vma, cache_level, unused);
-}
-
 static void ppgtt_unbind_vma(struct i915_vma *vma)
 {
 	vma->vm->clear_range(vma->vm, vma->node.start, vma->size);
@@ -1622,7 +1614,7 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
 	ppgtt->debug_dump = gen8_dump_ppgtt;
 
-	ppgtt->vm.vma_ops.bind_vma    = gen8_ppgtt_bind_vma;
+	ppgtt->vm.vma_ops.bind_vma    = ppgtt_bind_vma;
 	ppgtt->vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
 	ppgtt->vm.vma_ops.set_pages   = ppgtt_set_pages;
 	ppgtt->vm.vma_ops.clear_pages = clear_pages;
@@ -1827,7 +1819,8 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 
 		num_entries -= end - pte;
 
-		/* Note that the hw doesn't support removing PDE on the fly
+		/*
+		 * Note that the hw doesn't support removing PDE on the fly
 		 * (they are cached inside the context with no means to
 		 * invalidate the cache), so we can only reset the PTE
 		 * entries back to scratch.
@@ -2108,12 +2101,13 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 	ppgtt->base.vm.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
 
+	ppgtt->base.vm.allocate_va_range = gen6_alloc_va_range;
 	ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range;
 	ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries;
 	ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
 	ppgtt->base.debug_dump = gen6_dump_ppgtt;
 
-	ppgtt->base.vm.vma_ops.bind_vma    = gen6_ppgtt_bind_vma;
+	ppgtt->base.vm.vma_ops.bind_vma    = ppgtt_bind_vma;
 	ppgtt->base.vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
 	ppgtt->base.vm.vma_ops.set_pages   = ppgtt_set_pages;
 	ppgtt->base.vm.vma_ops.clear_pages = clear_pages;
@@ -2136,14 +2130,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	if (IS_ERR(ppgtt->vma))
 		goto err_scratch;
 
-	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
-	if (err)
-		goto err_vma;
-
 	return &ppgtt->base;
 
-err_vma:
-	i915_vma_destroy(ppgtt->vma);
 err_scratch:
 	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
 err_free:
@@ -2739,8 +2727,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 	if (flags & I915_VMA_LOCAL_BIND) {
 		struct i915_hw_ppgtt *appgtt = i915->mm.aliasing_ppgtt;
 
-		if (!(vma->flags & I915_VMA_LOCAL_BIND) &&
-		    appgtt->vm.allocate_va_range) {
+		if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
 			ret = appgtt->vm.allocate_va_range(&appgtt->vm,
 							   vma->node.start,
 							   vma->size);
@@ -2844,17 +2831,15 @@ int i915_gem_init_aliasing_ppgtt(struct drm_i915_private *i915)
 		goto err_ppgtt;
 	}
 
-	if (ppgtt->vm.allocate_va_range) {
-		/* Note we only pre-allocate as far as the end of the global
-		 * GTT. On 48b / 4-level page-tables, the difference is very,
-		 * very significant! We have to preallocate as GVT/vgpu does
-		 * not like the page directory disappearing.
-		 */
-		err = ppgtt->vm.allocate_va_range(&ppgtt->vm,
-						  0, ggtt->vm.total);
-		if (err)
-			goto err_ppgtt;
-	}
+	/*
+	 * Note we only pre-allocate as far as the end of the global
+	 * GTT. On 48b / 4-level page-tables, the difference is very,
+	 * very significant! We have to preallocate as GVT/vgpu does
+	 * not like the page directory disappearing.
+	 */
+	err = ppgtt->vm.allocate_va_range(&ppgtt->vm, 0, ggtt->vm.total);
+	if (err)
+		goto err_ppgtt;
 
 	i915->mm.aliasing_ppgtt = ppgtt;
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/17] drm/i915/gtt: Free unused page tables on unbind the context

[Chris Wilson]

As we cannot reliably change used page tables while the context is
active, the earliest opportunity we have to recover excess pages is when
the context becomes idle. So whenever we unbind the context (it must be
idle, and indeed being evicted) free the unused ptes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 44 +++++++++++++++++++++++++----
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d29e49226f33..bb1b26030d81 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1804,20 +1804,28 @@ static void gen6_ppgtt_enable(struct drm_i915_private *dev_priv)
 static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 				   u64 start, u64 length)
 {
-	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 	unsigned int first_entry = start >> PAGE_SHIFT;
 	unsigned int pde = first_entry / GEN6_PTES;
 	unsigned int pte = first_entry % GEN6_PTES;
 	unsigned int num_entries = length >> PAGE_SHIFT;
-	gen6_pte_t scratch_pte =
+	const gen6_pte_t scratch_pte =
 		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
 
 	while (num_entries) {
-		struct i915_page_table *pt = ppgtt->pd.page_table[pde++];
-		unsigned int end = min(pte + num_entries, GEN6_PTES);
+		struct i915_page_table *pt = ppgtt->base.pd.page_table[pde++];
+		const unsigned int end = min(pte + num_entries, GEN6_PTES);
+		const unsigned int count = end - pte;
 		gen6_pte_t *vaddr;
 
-		num_entries -= end - pte;
+		GEM_BUG_ON(pt == vm->scratch_pt);
+
+		num_entries -= count;
+
+		GEM_BUG_ON(count > pt->used_ptes);
+		pt->used_ptes -= count;
+		if (!pt->used_ptes)
+			ppgtt->scan_for_unused_pt = true;
 
 		/*
 		 * Note that the hw doesn't support removing PDE on the fly
@@ -1849,6 +1857,8 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 	struct sgt_dma iter = sgt_dma(vma);
 	gen6_pte_t *vaddr;
 
+	GEM_BUG_ON(ppgtt->pd.page_table[act_pt] == vm->scratch_pt);
+
 	vaddr = kmap_atomic_px(ppgtt->pd.page_table[act_pt]);
 	do {
 		vaddr[act_pte] = pte_encode | GEN6_PTE_ADDR_ENCODE(iter.dma);
@@ -1884,6 +1894,8 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	bool flush = false;
 
 	gen6_for_each_pde(pt, &ppgtt->base.pd, start, length, pde) {
+		const unsigned int count = gen6_pte_count(start, length);
+
 		if (pt == vm->scratch_pt) {
 			pt = alloc_pt(vm);
 			if (IS_ERR(pt))
@@ -1900,7 +1912,11 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 					  ppgtt->pd_addr + pde);
 				flush = true;
 			}
+
+			GEM_BUG_ON(pt->used_ptes);
 		}
+
+		pt->used_ptes += count;
 	}
 
 	if (flush) {
@@ -2004,6 +2020,24 @@ static int pd_vma_bind(struct i915_vma *vma,
 
 static void pd_vma_unbind(struct i915_vma *vma)
 {
+	struct gen6_hw_ppgtt *ppgtt = vma->private;
+	struct i915_page_table * const scratch_pt = ppgtt->base.vm.scratch_pt;
+	struct i915_page_table *pt;
+	unsigned int pde;
+
+	if (!ppgtt->scan_for_unused_pt)
+		return;
+
+	/* Free all no longer used page tables */
+	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde) {
+		if (pt->used_ptes || pt == scratch_pt)
+			continue;
+
+		free_pt(&ppgtt->base.vm, pt);
+		ppgtt->base.pd.page_table[pde] = scratch_pt;
+	}
+
+	ppgtt->scan_for_unused_pt = false;
 }
 
 static const struct i915_vma_ops pd_vma_ops = {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c20a4f06db37..dc98830fae69 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -414,6 +414,7 @@ struct gen6_hw_ppgtt {
 	gen6_pte_t __iomem *pd_addr;
 
 	unsigned int pin_count;
+	bool scan_for_unused_pt;
 
 	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
 };
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/17] drm/i915/gtt: Skip initializing PT with scratch if full

[Chris Wilson]

If we will completely overwrite the PT with PTEs for the object, we can
forgo filling it with scratch entries.

References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bb1b26030d81..fd2e3974e688 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1901,7 +1901,8 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 			if (IS_ERR(pt))
 				goto unwind_out;
 
-			gen6_initialize_pt(vm, pt);
+			if (count < GEN6_PTES)
+				gen6_initialize_pt(vm, pt);
 			ppgtt->base.pd.page_table[pde] = pt;
 
 			if (i915_vma_is_bound(ppgtt->vma,
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 13/17] drm/i915/gtt: Cache the PTE encoding of the scratch page

[Chris Wilson]

As the most frequent PTE encoding is for the scratch page, cache it upon
creation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++-----
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index fd2e3974e688..35fe2d1a55a2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1631,9 +1631,7 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
-	struct i915_address_space *vm = &base->vm;
-	const gen6_pte_t scratch_pte =
-		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
+	const gen6_pte_t scratch_pte = ppgtt->scratch_pte;
 	struct i915_page_table *pt;
 	u32 pte, pde;
 
@@ -1809,8 +1807,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 	unsigned int pde = first_entry / GEN6_PTES;
 	unsigned int pte = first_entry % GEN6_PTES;
 	unsigned int num_entries = length >> PAGE_SHIFT;
-	const gen6_pte_t scratch_pte =
-		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
+	const gen6_pte_t scratch_pte = ppgtt->scratch_pte;
 
 	while (num_entries) {
 		struct i915_page_table *pt = ppgtt->base.pd.page_table[pde++];
@@ -1943,6 +1940,10 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
 	if (ret)
 		return ret;
 
+	ppgtt->scratch_pte =
+		vm->pte_encode(vm->scratch_page.daddr,
+			       I915_CACHE_NONE, PTE_READ_ONLY);
+
 	vm->scratch_pt = alloc_pt(vm);
 	if (IS_ERR(vm->scratch_pt)) {
 		cleanup_scratch_page(vm);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index dc98830fae69..c50bbde007f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -412,6 +412,7 @@ struct gen6_hw_ppgtt {
 
 	struct i915_vma *vma;
 	gen6_pte_t __iomem *pd_addr;
+	gen6_pte_t scratch_pte;
 
 	unsigned int pin_count;
 	bool scan_for_unused_pt;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 14/17] drm/i915/gtt: Reduce a pair of runtime asserts

[Chris Wilson]

We can stop asserting using WARN_ON as given sufficient CI coverage, we
can rely on using GEM_BUG_ON() to catch problems before merging.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 35fe2d1a55a2..43fb36a99922 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2862,7 +2862,7 @@ int i915_gem_init_aliasing_ppgtt(struct drm_i915_private *i915)
 	if (IS_ERR(ppgtt))
 		return PTR_ERR(ppgtt);
 
-	if (WARN_ON(ppgtt->vm.total < ggtt->vm.total)) {
+	if (GEM_WARN_ON(ppgtt->vm.total < ggtt->vm.total)) {
 		err = -ENODEV;
 		goto err_ppgtt;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c50bbde007f8..37f565a38d3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -466,8 +466,8 @@ static inline u32 i915_pte_count(u64 addr, u64 length, unsigned int pde_shift)
 	const u64 mask = ~((1ULL << pde_shift) - 1);
 	u64 end;
 
-	WARN_ON(length == 0);
-	WARN_ON(offset_in_page(addr|length));
+	GEM_BUG_ON(length == 0);
+	GEM_BUG_ON(offset_in_page(addr|length));
 
 	end = addr + length;
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 15/17] drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt

[Chris Wilson]

If we know that the user cannot access the GGTT, by virtue of having a
segregated memory area, we can skip clearing the unused entries as they
cannot be accessed.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 43fb36a99922..81e846664fdf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3452,7 +3452,9 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	size = gen6_get_total_gtt_size(snb_gmch_ctl);
 	ggtt->vm.total = (size / sizeof(gen6_pte_t)) << PAGE_SHIFT;
 
-	ggtt->vm.clear_range = gen6_ggtt_clear_range;
+	ggtt->vm.clear_range = nop_clear_range;
+	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
+		ggtt->vm.clear_range = gen6_ggtt_clear_range;
 	ggtt->vm.insert_page = gen6_ggtt_insert_page;
 	ggtt->vm.insert_entries = gen6_ggtt_insert_entries;
 	ggtt->vm.cleanup = gen6_gmch_remove;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 16/17] drm/i915/ringbuffer: Force restore of mm after failed context switch

[Chris Wilson]

If we interrupt the context switch and unwind, we leave the to_mm
believing that we have cleared the dirty bit for this engine (but the
LRI will never take place). Just in case we immediately reload the same
context, mark this engine as dirty so that we force the LRI to reload
the PD.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index fb8d6217b03c..991187e1864a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1598,6 +1598,7 @@ static int switch_context(struct i915_request *rq)
 		to_gen6_ppgtt(to_ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt);
 	struct i915_gem_context *from_ctx = engine->legacy_active_context;
 	struct gen6_hw_ppgtt *from_mm = engine->legacy_active_ppgtt;
+	unsigned int unwind_mm = 0;
 	u32 hw_flags = 0;
 	int ret, i;
 
@@ -1611,8 +1612,13 @@ static int switch_context(struct i915_request *rq)
 		if (ret)
 			goto err;
 
-		to_mm->base.pd_dirty_rings &= ~intel_engine_flag(engine);
+		if (to_mm->base.pd_dirty_rings & intel_engine_flag(engine)) {
+			unwind_mm = intel_engine_flag(engine);
+			to_mm->base.pd_dirty_rings &= ~unwind_mm;
+		}
+
 		engine->legacy_active_ppgtt = to_mm;
+
 		hw_flags = MI_FORCE_RESTORE;
 	}
 
@@ -1655,6 +1661,8 @@ static int switch_context(struct i915_request *rq)
 err_ctx:
 	engine->legacy_active_context = from_ctx;
 err_mm:
+	if (unwind_mm)
+		to_mm->base.pd_dirty_rings |= unwind_mm;
 	engine->legacy_active_ppgtt = from_mm;
 err:
 	return ret;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 17/17] RFT drm/i915/gtt: Enable full-ppgtt by default everywhere

[Chris Wilson]

Let's see if we have all the kinks worked out and full-ppgtt now works
reliably on gen7 (Ivybridge, Valleyview/Baytrail and Haswell). If we can
let userspace have full control over their own ppgtt, it makes softpinning
far more effective, in turn making GPU dispatch far more efficient and
more secure (due to better mm segregation).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 81e846664fdf..314b7b29a5a3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -179,13 +179,11 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 		return 0;
 	}
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
-		if (has_full_48bit_ppgtt)
-			return 3;
+	if (has_full_48bit_ppgtt)
+		return 3;
 
-		if (has_full_ppgtt)
-			return 2;
-	}
+	if (has_full_ppgtt)
+		return 2;
 
 	return 1;
 }
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
URL   : https://patchwork.freedesktop.org/series/44328/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a380c2fdd48d drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
cf74192a8d7e drm/i915: Prepare for non-object vma
a484b45ce969 drm/i915: Decouple vma vfuncs from vm
522560532597 drm/i915/gtt: Push allocation to hw ppgtt constructor
6f87adae8a48 drm/i915/gtt: Subclass gen6_hw_ppgtt
-:327: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'base' - possible side-effects?
#327: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:419:
+#define __to_gen6_ppgtt(base) container_of(base, struct gen6_hw_ppgtt, base)

total: 0 errors, 0 warnings, 1 checks, 345 lines checked
c52753fa3ab8 drm/i915/gtt Onionify error handling for gen6_ppgtt_create
1eb3007c16bd drm/i915/gtt: Reorder aliasing_ppgtt fini
630e6ffe9fcc drm/i915/gtt: Make gen6 page directories evictable
-:250: WARNING:MEMORY_BARRIER: memory barrier without comment
#250: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:2008:
+	wmb();

total: 0 errors, 1 warnings, 0 checks, 397 lines checked
a4c245d93682 drm/i915/gtt: Only keep gen6 page directories pinned while active
38f48cf74350 drm/i915/gtt: Lazily allocate page directories for gen7
160d54d3f3e8 drm/i915/gtt: Free unused page tables on unbind the context
b71fa26f802a drm/i915/gtt: Skip initializing PT with scratch if full
-:9: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")

-:9: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 14826673247e ("drm/i915: Only initialize partially filled pagetables")'
#9: 
References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")

total: 1 errors, 1 warnings, 0 checks, 9 lines checked
b1af94ec4084 drm/i915/gtt: Cache the PTE encoding of the scratch page
41d291274bef drm/i915/gtt: Reduce a pair of runtime asserts
-:38: CHECK:SPACING: spaces preferred around that '|' (ctx:VxV)
#38: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:470:
+	GEM_BUG_ON(offset_in_page(addr|length));
 	                              ^

total: 0 errors, 0 warnings, 1 checks, 18 lines checked
4b2339b6feae drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
ca8c607168df drm/i915/ringbuffer: Force restore of mm after failed context switch
0c48e668d3df RFT drm/i915/gtt: Enable full-ppgtt by default everywhere

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/gtt: Make gen6 page directories evictable

[Chris Wilson]

Currently all page directories are bound at creation using an
unevictable node in the GGTT. This severely limits us as we cannot
remove any inactive ppgtt for new contexts, or under aperture pressure.
To fix this we need to make the page directory into a first class and
unbindable vma. Hence, the creation of a custom vma to wrap the page
directory as opposed to a GEM object.

In this patch, we leave the page directories pinned upon creation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 276 ++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   2 +-
 drivers/gpu/drm/i915/i915_vma.h     |   7 +
 3 files changed, 164 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ea27f799101f..1487d99ce406 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1640,79 +1640,58 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	struct i915_address_space *vm = &base->vm;
-	struct i915_page_table *unused;
-	gen6_pte_t scratch_pte;
-	u32 pd_entry, pte, pde;
-
-	scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
-
-	gen6_for_all_pdes(unused, &base->pd, pde) {
-		u32 expected;
-		gen6_pte_t *pt_vaddr;
-		const dma_addr_t pt_addr = px_dma(base->pd.page_table[pde]);
-		pd_entry = readl(ppgtt->pd_addr + pde);
-		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
-
-		if (pd_entry != expected)
-			seq_printf(m, "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
-				   pde,
-				   pd_entry,
-				   expected);
-		seq_printf(m, "\tPDE: %x\n", pd_entry);
-
-		pt_vaddr = kmap_atomic_px(base->pd.page_table[pde]);
-
-		for (pte = 0; pte < GEN6_PTES; pte+=4) {
-			unsigned long va =
-				(pde * PAGE_SIZE * GEN6_PTES) +
-				(pte * PAGE_SIZE);
+	const gen6_pte_t scratch_pte =
+		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
+	struct i915_page_table *pt;
+	u32 pte, pde;
+
+	gen6_for_all_pdes(pt, &base->pd, pde) {
+		gen6_pte_t *vaddr;
+
+		if (pt == base->vm.scratch_pt)
+			continue;
+
+		if (i915_vma_is_bound(ppgtt->vma, I915_VMA_GLOBAL_BIND)) {
+			u32 expected =
+				GEN6_PDE_ADDR_ENCODE(px_dma(pt)) |
+				GEN6_PDE_VALID;
+			u32 pd_entry = readl(ppgtt->pd_addr + pde);
+
+			if (pd_entry != expected)
+				seq_printf(m,
+					   "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
+					   pde,
+					   pd_entry,
+					   expected);
+
+			seq_printf(m, "\tPDE: %x\n", pd_entry);
+		}
+
+		vaddr = kmap_atomic_px(base->pd.page_table[pde]);
+		for (pte = 0; pte < GEN6_PTES; pte += 4) {
 			int i;
-			bool found = false;
+
 			for (i = 0; i < 4; i++)
-				if (pt_vaddr[pte + i] != scratch_pte)
-					found = true;
-			if (!found)
+				if (vaddr[pte + i] != scratch_pte)
+					break;
+			if (i == 4)
 				continue;
 
-			seq_printf(m, "\t\t0x%lx [%03d,%04d]: =", va, pde, pte);
+			seq_printf(m, "\t\t(%03d, %04d) %08lx: ",
+				   pde, pte,
+				   (pde * GEN6_PTES + pte) * PAGE_SIZE);
 			for (i = 0; i < 4; i++) {
-				if (pt_vaddr[pte + i] != scratch_pte)
-					seq_printf(m, " %08x", pt_vaddr[pte + i]);
+				if (vaddr[pte + i] != scratch_pte)
+					seq_printf(m, " %08x", vaddr[pte + i]);
 				else
-					seq_puts(m, "  SCRATCH ");
+					seq_puts(m, "  SCRATCH");
 			}
 			seq_puts(m, "\n");
 		}
-		kunmap_atomic(pt_vaddr);
+		kunmap_atomic(vaddr);
 	}
 }
 
-/* Write pde (index) from the page directory @pd to the page table @pt */
-static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
-				  const unsigned int pde,
-				  const struct i915_page_table *pt)
-{
-	/* Caller needs to make sure the write completes if necessary */
-	iowrite32(GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID,
-		  ppgtt->pd_addr + pde);
-}
-
-/* Write all the page tables found in the ppgtt structure to incrementing page
- * directories. */
-static void gen6_write_page_range(struct i915_hw_ppgtt *base,
-				  u32 start, u32 length)
-{
-	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
-	struct i915_page_table *pt;
-	unsigned int pde;
-
-	gen6_for_each_pde(pt, &base->pd, start, length, pde)
-		gen6_write_pde(ppgtt, pde, pt);
-
-	mark_tlbs_dirty(base);
-	gen6_ggtt_invalidate(base->vm.i915);
-}
-
 static inline u32 get_pd_offset(struct gen6_hw_ppgtt *ppgtt)
 {
 	GEM_BUG_ON(ppgtt->base.pd.base.ggtt_offset & 0x3f);
@@ -1919,8 +1898,15 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 			gen6_initialize_pt(vm, pt);
 			ppgtt->base.pd.page_table[pde] = pt;
-			gen6_write_pde(ppgtt, pde, pt);
-			flush = true;
+
+			if (i915_vma_is_bound(ppgtt->vma,
+					      I915_VMA_GLOBAL_BIND)) {
+				u32 addr = GEN6_PDE_ADDR_ENCODE(px_dma(pt));
+
+				iowrite32(addr | GEN6_PDE_VALID,
+					  ppgtt->pd_addr + pde);
+				flush = true;
+			}
 		}
 	}
 
@@ -1936,8 +1922,11 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	return -ENOMEM;
 }
 
-static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
+static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
 {
+	struct i915_address_space * const vm = &ppgtt->base.vm;
+	struct i915_page_table *unused;
+	u32 pde;
 	int ret;
 
 	ret = setup_scratch_page(vm, __GFP_HIGHMEM);
@@ -1951,6 +1940,8 @@ static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen6_initialize_pt(vm, vm->scratch_pt);
+	gen6_for_all_pdes(unused, &ppgtt->base.pd, pde)
+		ppgtt->base.pd.page_table[pde] = vm->scratch_pt;
 
 	return 0;
 }
@@ -1975,52 +1966,112 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	drm_mm_remove_node(&ppgtt->node);
+	i915_vma_unpin(ppgtt->vma);
+	i915_vma_destroy(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
 	gen6_ppgtt_free_scratch(vm);
 }
 
-static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
+static int pd_vma_set_pages(struct i915_vma *vma)
 {
-	struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	int err;
+	vma->pages = ERR_PTR(-ENODEV);
+	return 0;
+}
 
-	/* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
-	 * allocator works in address space sizes, so it's multiplied by page
-	 * size. We allocate at the top of the GTT to avoid fragmentation.
-	 */
-	BUG_ON(!drm_mm_initialized(&ggtt->vm.mm));
+static void pd_vma_clear_pages(struct i915_vma *vma)
+{
+	GEM_BUG_ON(!vma->pages);
 
-	err = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
-				  GEN6_PD_SIZE, GEN6_PD_ALIGN,
-				  I915_COLOR_UNEVICTABLE,
-				  0, ggtt->vm.total,
-				  PIN_HIGH);
-	if (err)
-		return err;
+	vma->pages = NULL;
+}
 
-	if (ppgtt->node.start < ggtt->mappable_end)
-		DRM_DEBUG("Forced to use aperture for PDEs\n");
+static int pd_vma_bind(struct i915_vma *vma,
+		       enum i915_cache_level cache_level,
+		       u32 unused)
+{
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
+	struct gen6_hw_ppgtt *ppgtt = vma->private;
+	u32 ggtt_offset = i915_ggtt_offset(vma) / PAGE_SIZE;
+	struct i915_page_table *pt;
+	unsigned int pde;
 
-	ppgtt->base.pd.base.ggtt_offset =
-		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
+	ppgtt->base.pd.base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
+	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm + ggtt_offset;
 
-	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm +
-		ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
+	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde) {
+		u32 val = GEN6_PDE_ADDR_ENCODE(px_dma(pt)) | GEN6_PDE_VALID;
+
+		iowrite32(val, ppgtt->pd_addr + pde);
+	}
+	mark_tlbs_dirty(&ppgtt->base);
+	gen6_ggtt_invalidate(ppgtt->base.vm.i915);
 
 	return 0;
 }
 
-static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
-				  u64 start, u64 length)
+static void pd_vma_unbind(struct i915_vma *vma)
 {
-	struct i915_page_table *unused;
-	u32 pde;
+}
+
+static const struct i915_vma_ops pd_vma_ops = {
+	.set_pages = pd_vma_set_pages,
+	.clear_pages = pd_vma_clear_pages,
+	.bind_vma = pd_vma_bind,
+	.unbind_vma = pd_vma_unbind,
+};
+
+static struct i915_vma *pd_vma_create(struct gen6_hw_ppgtt *ppgtt, int size)
+{
+	struct drm_i915_private *i915 = ppgtt->base.vm.i915;
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct i915_vma *vma;
+	int i;
+
+	GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE));
+	GEM_BUG_ON(size > ggtt->vm.total);
+
+	vma = kmem_cache_zalloc(i915->vmas, GFP_KERNEL);
+	if (!vma)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
+		init_request_active(&vma->last_read[i], NULL);
+	init_request_active(&vma->last_fence, NULL);
+
+	vma->vm = &ggtt->vm;
+	vma->ops = &pd_vma_ops;
+	vma->private = ppgtt;
+
+	vma->obj = NULL;
+	vma->resv = NULL;
+	vma->size = size;
+	vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->fence_size = size;
+	vma->fence_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->flags |= I915_VMA_GGTT;
+	vma->ggtt_view.type = I915_GGTT_VIEW_ROTATED; /* prevent fencing */
+
+	INIT_LIST_HEAD(&vma->obj_link);
+	list_add(&vma->vm_link, &vma->vm->unbound_list);
 
-	gen6_for_each_pde(unused, &ppgtt->base.pd, start, length, pde)
-		ppgtt->base.pd.page_table[pde] = ppgtt->base.vm.scratch_pt;
+	return vma;
+}
+
+static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
+{
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
+
+	/*
+	 * PPGTT PDEs reside in the GGTT and consists of 512 entries. The
+	 * allocator works in address space sizes, so it's multiplied by page
+	 * size. We allocate at the top of the GTT to avoid fragmentation.
+	 */
+	return i915_vma_pin(ppgtt->vma,
+			    0, GEN6_PD_ALIGN,
+			    PIN_GLOBAL | PIN_HIGH);
 }
 
 static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
@@ -2058,24 +2109,25 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	else
 		BUG();
 
-	err = gen6_ppgtt_init_scratch(&ppgtt->base.vm);
+	err = gen6_ppgtt_init_scratch(ppgtt);
 	if (err)
 		goto err_free;
 
-	err = gen6_ppgtt_allocate_page_directories(ppgtt);
-	if (err)
+	ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
+	if (IS_ERR(ppgtt->vma))
 		goto err_scratch;
 
-	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.vm.total);
-	gen6_write_page_range(&ppgtt->base, 0, ppgtt->base.vm.total);
-
 	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
+	if (err)
+		goto err_vma;
+
+	err = gen6_ppgtt_pin(&ppgtt->base);
 	if (err)
 		goto err_pd;
 
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
-			 ppgtt->node.size >> 20,
-			 ppgtt->node.start / PAGE_SIZE);
+			 ppgtt->vma->node.size >> 20,
+			 ppgtt->vma->node.start / PAGE_SIZE);
 
 	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
 			 ppgtt->base.pd.base.ggtt_offset << 10);
@@ -2084,6 +2136,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 err_pd:
 	gen6_ppgtt_free_pd(ppgtt);
+err_vma:
+	i915_vma_destroy(ppgtt->vma);
 err_scratch:
 	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
 err_free:
@@ -3602,6 +3656,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 	}
 
 	ggtt->vm.closed = false;
+	i915_ggtt_invalidate(dev_priv);
 
 	if (INTEL_GEN(dev_priv) >= 8) {
 		struct intel_ppat *ppat = &dev_priv->ppat;
@@ -3610,25 +3665,6 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		dev_priv->ppat.update_hw(dev_priv);
 		return;
 	}
-
-	if (USES_PPGTT(dev_priv)) {
-		struct i915_address_space *vm;
-
-		list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
-			struct i915_hw_ppgtt *ppgtt;
-
-			if (i915_is_ggtt(vm))
-				ppgtt = dev_priv->mm.aliasing_ppgtt;
-			else
-				ppgtt = i915_vm_to_ppgtt(vm);
-			if (!ppgtt)
-				continue;
-
-			gen6_write_page_range(ppgtt, 0, ppgtt->vm.total);
-		}
-	}
-
-	i915_ggtt_invalidate(dev_priv);
 }
 
 static struct scatterlist *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 199d6f47a557..c2f270c90bea 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -410,7 +410,7 @@ struct i915_hw_ppgtt {
 struct gen6_hw_ppgtt {
 	struct i915_hw_ppgtt base;
 
-	struct drm_mm_node node;
+	struct i915_vma *vma;
 	gen6_pte_t __iomem *pd_addr;
 
 	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4321476a6a32..66a228931517 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -54,6 +54,7 @@ struct i915_vma {
 	struct reservation_object *resv; /** Alias of obj->resv */
 	struct sg_table *pages;
 	void __iomem *iomap;
+	void *private; /* owned by creator */
 	u64 size;
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
@@ -340,6 +341,12 @@ static inline void i915_vma_unpin(struct i915_vma *vma)
 	__i915_vma_unpin(vma);
 }
 
+static inline bool i915_vma_is_bound(const struct i915_vma *vma,
+				     unsigned int where)
+{
+	return vma->flags & where;
+}
+
 /**
  * i915_vma_pin_iomap - calls ioremap_wc to map the GGTT VMA via the aperture
  * @vma: VMA to iomap
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
URL   : https://patchwork.freedesktop.org/series/44328/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
Okay!

Commit: drm/i915: Prepare for non-object vma
Okay!

Commit: drm/i915: Decouple vma vfuncs from vm
Okay!

Commit: drm/i915/gtt: Push allocation to hw ppgtt constructor
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1644:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1644:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Subclass gen6_hw_ppgtt
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1708:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1708:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1913:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1913:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1914:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1914:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt Onionify error handling for gen6_ppgtt_create
Okay!

Commit: drm/i915/gtt: Reorder aliasing_ppgtt fini
Okay!

Commit: drm/i915/gtt: Make gen6 page directories evictable
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2022:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2022:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Only keep gen6 page directories pinned while active
Okay!

Commit: drm/i915/gtt: Lazily allocate page directories for gen7
Okay!

Commit: drm/i915/gtt: Free unused page tables on unbind the context
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1817:36: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1886:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1886:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1817:42: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1896:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1896:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Skip initializing PT with scratch if full
Okay!

Commit: drm/i915/gtt: Cache the PTE encoding of the scratch page
Okay!

Commit: drm/i915/gtt: Reduce a pair of runtime asserts
Okay!

Commit: drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
Okay!

Commit: drm/i915/ringbuffer: Force restore of mm after failed context switch
Okay!

Commit: RFT drm/i915/gtt: Enable full-ppgtt by default everywhere
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/gtt: Make gen6 page directories evictable

[Chris Wilson]

Currently all page directories are bound at creation using an
unevictable node in the GGTT. This severely limits us as we cannot
remove any inactive ppgtt for new contexts, or under aperture pressure.
To fix this we need to make the page directory into a first class and
unbindable vma. Hence, the creation of a custom vma to wrap the page
directory as opposed to a GEM object.

In this patch, we leave the page directories pinned upon creation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 261 ++++++++++++++++------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   2 +-
 drivers/gpu/drm/i915/i915_vma.h     |   7 +
 3 files changed, 159 insertions(+), 111 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ea27f799101f..60b5966360b6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1640,50 +1640,55 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	struct i915_address_space *vm = &base->vm;
-	struct i915_page_table *unused;
-	gen6_pte_t scratch_pte;
-	u32 pd_entry, pte, pde;
-
-	scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
-
-	gen6_for_all_pdes(unused, &base->pd, pde) {
-		u32 expected;
-		gen6_pte_t *pt_vaddr;
-		const dma_addr_t pt_addr = px_dma(base->pd.page_table[pde]);
-		pd_entry = readl(ppgtt->pd_addr + pde);
-		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
-
-		if (pd_entry != expected)
-			seq_printf(m, "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
-				   pde,
-				   pd_entry,
-				   expected);
-		seq_printf(m, "\tPDE: %x\n", pd_entry);
-
-		pt_vaddr = kmap_atomic_px(base->pd.page_table[pde]);
-
-		for (pte = 0; pte < GEN6_PTES; pte+=4) {
-			unsigned long va =
-				(pde * PAGE_SIZE * GEN6_PTES) +
-				(pte * PAGE_SIZE);
+	const gen6_pte_t scratch_pte =
+		vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
+	struct i915_page_table *pt;
+	u32 pte, pde;
+
+	gen6_for_all_pdes(pt, &base->pd, pde) {
+		gen6_pte_t *vaddr;
+
+		if (pt == base->vm.scratch_pt)
+			continue;
+
+		if (i915_vma_is_bound(ppgtt->vma, I915_VMA_GLOBAL_BIND)) {
+			u32 expected =
+				GEN6_PDE_ADDR_ENCODE(px_dma(pt)) |
+				GEN6_PDE_VALID;
+			u32 pd_entry = readl(ppgtt->pd_addr + pde);
+
+			if (pd_entry != expected)
+				seq_printf(m,
+					   "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
+					   pde,
+					   pd_entry,
+					   expected);
+
+			seq_printf(m, "\tPDE: %x\n", pd_entry);
+		}
+
+		vaddr = kmap_atomic_px(base->pd.page_table[pde]);
+		for (pte = 0; pte < GEN6_PTES; pte += 4) {
 			int i;
-			bool found = false;
+
 			for (i = 0; i < 4; i++)
-				if (pt_vaddr[pte + i] != scratch_pte)
-					found = true;
-			if (!found)
+				if (vaddr[pte + i] != scratch_pte)
+					break;
+			if (i == 4)
 				continue;
 
-			seq_printf(m, "\t\t0x%lx [%03d,%04d]: =", va, pde, pte);
+			seq_printf(m, "\t\t(%03d, %04d) %08lx: ",
+				   pde, pte,
+				   (pde * GEN6_PTES + pte) * PAGE_SIZE);
 			for (i = 0; i < 4; i++) {
-				if (pt_vaddr[pte + i] != scratch_pte)
-					seq_printf(m, " %08x", pt_vaddr[pte + i]);
+				if (vaddr[pte + i] != scratch_pte)
+					seq_printf(m, " %08x", vaddr[pte + i]);
 				else
-					seq_puts(m, "  SCRATCH ");
+					seq_puts(m, "  SCRATCH");
 			}
 			seq_puts(m, "\n");
 		}
-		kunmap_atomic(pt_vaddr);
+		kunmap_atomic(vaddr);
 	}
 }
 
@@ -1697,22 +1702,6 @@ static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
 		  ppgtt->pd_addr + pde);
 }
 
-/* Write all the page tables found in the ppgtt structure to incrementing page
- * directories. */
-static void gen6_write_page_range(struct i915_hw_ppgtt *base,
-				  u32 start, u32 length)
-{
-	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
-	struct i915_page_table *pt;
-	unsigned int pde;
-
-	gen6_for_each_pde(pt, &base->pd, start, length, pde)
-		gen6_write_pde(ppgtt, pde, pt);
-
-	mark_tlbs_dirty(base);
-	gen6_ggtt_invalidate(base->vm.i915);
-}
-
 static inline u32 get_pd_offset(struct gen6_hw_ppgtt *ppgtt)
 {
 	GEM_BUG_ON(ppgtt->base.pd.base.ggtt_offset & 0x3f);
@@ -1919,8 +1908,12 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 			gen6_initialize_pt(vm, pt);
 			ppgtt->base.pd.page_table[pde] = pt;
-			gen6_write_pde(ppgtt, pde, pt);
-			flush = true;
+
+			if (i915_vma_is_bound(ppgtt->vma,
+					      I915_VMA_GLOBAL_BIND)) {
+				gen6_write_pde(ppgtt, pde, pt);
+				flush = true;
+			}
 		}
 	}
 
@@ -1936,8 +1929,11 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	return -ENOMEM;
 }
 
-static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
+static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
 {
+	struct i915_address_space * const vm = &ppgtt->base.vm;
+	struct i915_page_table *unused;
+	u32 pde;
 	int ret;
 
 	ret = setup_scratch_page(vm, __GFP_HIGHMEM);
@@ -1951,6 +1947,8 @@ static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen6_initialize_pt(vm, vm->scratch_pt);
+	gen6_for_all_pdes(unused, &ppgtt->base.pd, pde)
+		ppgtt->base.pd.page_table[pde] = vm->scratch_pt;
 
 	return 0;
 }
@@ -1975,52 +1973,110 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	drm_mm_remove_node(&ppgtt->node);
+	i915_vma_unpin(ppgtt->vma);
+	i915_vma_destroy(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
 	gen6_ppgtt_free_scratch(vm);
 }
 
-static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
+static int pd_vma_set_pages(struct i915_vma *vma)
 {
-	struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	int err;
+	vma->pages = ERR_PTR(-ENODEV);
+	return 0;
+}
 
-	/* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
-	 * allocator works in address space sizes, so it's multiplied by page
-	 * size. We allocate at the top of the GTT to avoid fragmentation.
-	 */
-	BUG_ON(!drm_mm_initialized(&ggtt->vm.mm));
+static void pd_vma_clear_pages(struct i915_vma *vma)
+{
+	GEM_BUG_ON(!vma->pages);
 
-	err = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
-				  GEN6_PD_SIZE, GEN6_PD_ALIGN,
-				  I915_COLOR_UNEVICTABLE,
-				  0, ggtt->vm.total,
-				  PIN_HIGH);
-	if (err)
-		return err;
+	vma->pages = NULL;
+}
 
-	if (ppgtt->node.start < ggtt->mappable_end)
-		DRM_DEBUG("Forced to use aperture for PDEs\n");
+static int pd_vma_bind(struct i915_vma *vma,
+		       enum i915_cache_level cache_level,
+		       u32 unused)
+{
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
+	struct gen6_hw_ppgtt *ppgtt = vma->private;
+	u32 ggtt_offset = i915_ggtt_offset(vma) / PAGE_SIZE;
+	struct i915_page_table *pt;
+	unsigned int pde;
 
-	ppgtt->base.pd.base.ggtt_offset =
-		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
+	ppgtt->base.pd.base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
+	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm + ggtt_offset;
 
-	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm +
-		ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
+	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
+		gen6_write_pde(ppgtt, pde, pt);
+
+	mark_tlbs_dirty(&ppgtt->base);
+	gen6_ggtt_invalidate(ppgtt->base.vm.i915);
 
 	return 0;
 }
 
-static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
-				  u64 start, u64 length)
+static void pd_vma_unbind(struct i915_vma *vma)
 {
-	struct i915_page_table *unused;
-	u32 pde;
+}
+
+static const struct i915_vma_ops pd_vma_ops = {
+	.set_pages = pd_vma_set_pages,
+	.clear_pages = pd_vma_clear_pages,
+	.bind_vma = pd_vma_bind,
+	.unbind_vma = pd_vma_unbind,
+};
+
+static struct i915_vma *pd_vma_create(struct gen6_hw_ppgtt *ppgtt, int size)
+{
+	struct drm_i915_private *i915 = ppgtt->base.vm.i915;
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct i915_vma *vma;
+	int i;
 
-	gen6_for_each_pde(unused, &ppgtt->base.pd, start, length, pde)
-		ppgtt->base.pd.page_table[pde] = ppgtt->base.vm.scratch_pt;
+	GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE));
+	GEM_BUG_ON(size > ggtt->vm.total);
+
+	vma = kmem_cache_zalloc(i915->vmas, GFP_KERNEL);
+	if (!vma)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
+		init_request_active(&vma->last_read[i], NULL);
+	init_request_active(&vma->last_fence, NULL);
+
+	vma->vm = &ggtt->vm;
+	vma->ops = &pd_vma_ops;
+	vma->private = ppgtt;
+
+	vma->obj = NULL;
+	vma->resv = NULL;
+	vma->size = size;
+	vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->fence_size = size;
+	vma->fence_alignment = I915_GTT_MIN_ALIGNMENT;
+
+	vma->flags |= I915_VMA_GGTT;
+	vma->ggtt_view.type = I915_GGTT_VIEW_ROTATED; /* prevent fencing */
+
+	INIT_LIST_HEAD(&vma->obj_link);
+	list_add(&vma->vm_link, &vma->vm->unbound_list);
+
+	return vma;
+}
+
+static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
+{
+	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
+
+	/*
+	 * PPGTT PDEs reside in the GGTT and consists of 512 entries. The
+	 * allocator works in address space sizes, so it's multiplied by page
+	 * size. We allocate at the top of the GTT to avoid fragmentation.
+	 */
+	return i915_vma_pin(ppgtt->vma,
+			    0, GEN6_PD_ALIGN,
+			    PIN_GLOBAL | PIN_HIGH);
 }
 
 static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
@@ -2058,24 +2114,25 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 	else
 		BUG();
 
-	err = gen6_ppgtt_init_scratch(&ppgtt->base.vm);
+	err = gen6_ppgtt_init_scratch(ppgtt);
 	if (err)
 		goto err_free;
 
-	err = gen6_ppgtt_allocate_page_directories(ppgtt);
-	if (err)
+	ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
+	if (IS_ERR(ppgtt->vma))
 		goto err_scratch;
 
-	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.vm.total);
-	gen6_write_page_range(&ppgtt->base, 0, ppgtt->base.vm.total);
-
 	err = gen6_alloc_va_range(&ppgtt->base.vm, 0, ppgtt->base.vm.total);
+	if (err)
+		goto err_vma;
+
+	err = gen6_ppgtt_pin(&ppgtt->base);
 	if (err)
 		goto err_pd;
 
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
-			 ppgtt->node.size >> 20,
-			 ppgtt->node.start / PAGE_SIZE);
+			 ppgtt->vma->node.size >> 20,
+			 ppgtt->vma->node.start / PAGE_SIZE);
 
 	DRM_DEBUG_DRIVER("Adding PPGTT at offset %x\n",
 			 ppgtt->base.pd.base.ggtt_offset << 10);
@@ -2084,6 +2141,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 err_pd:
 	gen6_ppgtt_free_pd(ppgtt);
+err_vma:
+	i915_vma_destroy(ppgtt->vma);
 err_scratch:
 	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
 err_free:
@@ -3602,6 +3661,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 	}
 
 	ggtt->vm.closed = false;
+	i915_ggtt_invalidate(dev_priv);
 
 	if (INTEL_GEN(dev_priv) >= 8) {
 		struct intel_ppat *ppat = &dev_priv->ppat;
@@ -3610,25 +3670,6 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		dev_priv->ppat.update_hw(dev_priv);
 		return;
 	}
-
-	if (USES_PPGTT(dev_priv)) {
-		struct i915_address_space *vm;
-
-		list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
-			struct i915_hw_ppgtt *ppgtt;
-
-			if (i915_is_ggtt(vm))
-				ppgtt = dev_priv->mm.aliasing_ppgtt;
-			else
-				ppgtt = i915_vm_to_ppgtt(vm);
-			if (!ppgtt)
-				continue;
-
-			gen6_write_page_range(ppgtt, 0, ppgtt->vm.total);
-		}
-	}
-
-	i915_ggtt_invalidate(dev_priv);
 }
 
 static struct scatterlist *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 199d6f47a557..c2f270c90bea 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -410,7 +410,7 @@ struct i915_hw_ppgtt {
 struct gen6_hw_ppgtt {
 	struct i915_hw_ppgtt base;
 
-	struct drm_mm_node node;
+	struct i915_vma *vma;
 	gen6_pte_t __iomem *pd_addr;
 
 	int (*switch_mm)(struct gen6_hw_ppgtt *ppgtt, struct i915_request *rq);
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4321476a6a32..66a228931517 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -54,6 +54,7 @@ struct i915_vma {
 	struct reservation_object *resv; /** Alias of obj->resv */
 	struct sg_table *pages;
 	void __iomem *iomap;
+	void *private; /* owned by creator */
 	u64 size;
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
@@ -340,6 +341,12 @@ static inline void i915_vma_unpin(struct i915_vma *vma)
 	__i915_vma_unpin(vma);
 }
 
+static inline bool i915_vma_is_bound(const struct i915_vma *vma,
+				     unsigned int where)
+{
+	return vma->flags & where;
+}
+
 /**
  * i915_vma_pin_iomap - calls ioremap_wc to map the GGTT VMA via the aperture
  * @vma: VMA to iomap
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
URL   : https://patchwork.freedesktop.org/series/44328/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4282 -> Patchwork_9212 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44328/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9212 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@gem_sync@basic-many-each:
      fi-cnl-y3:          NOTRUN -> INCOMPLETE (fdo#105086)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097, fdo#106000)

    
    ==== Possible fixes ====

    igt@gem_sync@basic-each:
      fi-cnl-y3:          INCOMPLETE (fdo#105086) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097


== Participating hosts (41 -> 36) ==

  Missing    (5): fi-byt-j1900 fi-byt-squawks fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4282 -> Patchwork_9212

  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9212: 0c48e668d3df05e66629785ff1d6a03e435d3829 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0c48e668d3df RFT drm/i915/gtt: Enable full-ppgtt by default everywhere
ca8c607168df drm/i915/ringbuffer: Force restore of mm after failed context switch
4b2339b6feae drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
41d291274bef drm/i915/gtt: Reduce a pair of runtime asserts
b1af94ec4084 drm/i915/gtt: Cache the PTE encoding of the scratch page
b71fa26f802a drm/i915/gtt: Skip initializing PT with scratch if full
160d54d3f3e8 drm/i915/gtt: Free unused page tables on unbind the context
38f48cf74350 drm/i915/gtt: Lazily allocate page directories for gen7
a4c245d93682 drm/i915/gtt: Only keep gen6 page directories pinned while active
630e6ffe9fcc drm/i915/gtt: Make gen6 page directories evictable
1eb3007c16bd drm/i915/gtt: Reorder aliasing_ppgtt fini
c52753fa3ab8 drm/i915/gtt Onionify error handling for gen6_ppgtt_create
6f87adae8a48 drm/i915/gtt: Subclass gen6_hw_ppgtt
522560532597 drm/i915/gtt: Push allocation to hw ppgtt constructor
a484b45ce969 drm/i915: Decouple vma vfuncs from vm
cf74192a8d7e drm/i915: Prepare for non-object vma
a380c2fdd48d drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9212/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)
URL   : https://patchwork.freedesktop.org/series/44328/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b4c3a32216a8 drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
70d6ec96d7bf drm/i915: Prepare for non-object vma
bb92e563bcba drm/i915: Decouple vma vfuncs from vm
1b47b1859905 drm/i915/gtt: Push allocation to hw ppgtt constructor
30575afa671e drm/i915/gtt: Subclass gen6_hw_ppgtt
-:327: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'base' - possible side-effects?
#327: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:419:
+#define __to_gen6_ppgtt(base) container_of(base, struct gen6_hw_ppgtt, base)

total: 0 errors, 0 warnings, 1 checks, 345 lines checked
0fcca377a4c9 drm/i915/gtt Onionify error handling for gen6_ppgtt_create
51d64fa11e1b drm/i915/gtt: Reorder aliasing_ppgtt fini
2aec149417e8 drm/i915/gtt: Make gen6 page directories evictable
37c40c9d4250 drm/i915/gtt: Only keep gen6 page directories pinned while active
946348137797 drm/i915/gtt: Lazily allocate page directories for gen7
1b94847f49c7 drm/i915/gtt: Free unused page tables on unbind the context
151e03fef4c0 drm/i915/gtt: Skip initializing PT with scratch if full
-:9: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")

-:9: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 14826673247e ("drm/i915: Only initialize partially filled pagetables")'
#9: 
References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")

total: 1 errors, 1 warnings, 0 checks, 9 lines checked
7a457a0a7c9b drm/i915/gtt: Cache the PTE encoding of the scratch page
10db8c23ef65 drm/i915/gtt: Reduce a pair of runtime asserts
-:38: CHECK:SPACING: spaces preferred around that '|' (ctx:VxV)
#38: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:470:
+	GEM_BUG_ON(offset_in_page(addr|length));
 	                              ^

total: 0 errors, 0 warnings, 1 checks, 18 lines checked
5f8f4ee7b867 drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
90dad5f7947a drm/i915/ringbuffer: Force restore of mm after failed context switch
bcdf55b7b6b7 RFT drm/i915/gtt: Enable full-ppgtt by default everywhere

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)
URL   : https://patchwork.freedesktop.org/series/44328/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories
Okay!

Commit: drm/i915: Prepare for non-object vma
Okay!

Commit: drm/i915: Decouple vma vfuncs from vm
Okay!

Commit: drm/i915/gtt: Push allocation to hw ppgtt constructor
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1644:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1644:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Subclass gen6_hw_ppgtt
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1649:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1708:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1708:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1913:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1913:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1914:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1914:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:2031:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt Onionify error handling for gen6_ppgtt_create
Okay!

Commit: drm/i915/gtt: Reorder aliasing_ppgtt fini
Okay!

Commit: drm/i915/gtt: Make gen6 page directories evictable
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1709:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2022:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:2022:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Only keep gen6 page directories pinned while active
Okay!

Commit: drm/i915/gtt: Lazily allocate page directories for gen7
Okay!

Commit: drm/i915/gtt: Free unused page tables on unbind the context
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1827:36: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1896:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1896:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1827:42: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1906:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1906:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Skip initializing PT with scratch if full
Okay!

Commit: drm/i915/gtt: Cache the PTE encoding of the scratch page
Okay!

Commit: drm/i915/gtt: Reduce a pair of runtime asserts
Okay!

Commit: drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
Okay!

Commit: drm/i915/ringbuffer: Force restore of mm after failed context switch
Okay!

Commit: RFT drm/i915/gtt: Enable full-ppgtt by default everywhere
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)
URL   : https://patchwork.freedesktop.org/series/44328/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4282 -> Patchwork_9213 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9213 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9213, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44328/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9213:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9213 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_flush@basic-uc-prw-default:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000, fdo#106097)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
      fi-skl-6700k2:      PASS -> FAIL (fdo#103191, fdo#104724)

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097


== Participating hosts (41 -> 37) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4282 -> Patchwork_9213

  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9213: bcdf55b7b6b742fc63634cef1ebbadcd0e937a79 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bcdf55b7b6b7 RFT drm/i915/gtt: Enable full-ppgtt by default everywhere
90dad5f7947a drm/i915/ringbuffer: Force restore of mm after failed context switch
5f8f4ee7b867 drm/i915/gtt: Skip clearing the GGTT under gen6+ full-ppgtt
10db8c23ef65 drm/i915/gtt: Reduce a pair of runtime asserts
7a457a0a7c9b drm/i915/gtt: Cache the PTE encoding of the scratch page
151e03fef4c0 drm/i915/gtt: Skip initializing PT with scratch if full
1b94847f49c7 drm/i915/gtt: Free unused page tables on unbind the context
946348137797 drm/i915/gtt: Lazily allocate page directories for gen7
37c40c9d4250 drm/i915/gtt: Only keep gen6 page directories pinned while active
2aec149417e8 drm/i915/gtt: Make gen6 page directories evictable
51d64fa11e1b drm/i915/gtt: Reorder aliasing_ppgtt fini
0fcca377a4c9 drm/i915/gtt Onionify error handling for gen6_ppgtt_create
30575afa671e drm/i915/gtt: Subclass gen6_hw_ppgtt
1b47b1859905 drm/i915/gtt: Push allocation to hw ppgtt constructor
bb92e563bcba drm/i915: Decouple vma vfuncs from vm
70d6ec96d7bf drm/i915: Prepare for non-object vma
b4c3a32216a8 drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9213/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [01/17] drm/i915/gtt: Invalidate GGTT caches after writing the gen6 page directories (rev3)
URL   : https://patchwork.freedesktop.org/series/44328/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4282_full -> Patchwork_9213_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9213_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9213_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44328/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9213_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      shard-hsw:          PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_ppgtt@flink-and-close-vma-leak:
      shard-hsw:          SKIP -> PASS +2

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9213_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      shard-apl:          PASS -> DMESG-FAIL (fdo#106560)

    igt@gem_exec_reloc@basic-cpu-wc-active:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@wf_vblank-ts-check:
      shard-glk:          PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@gem_eio@hibernate:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_exec_basic@gtt-vebox:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@gem_exec_big:
      shard-hsw:          INCOMPLETE (fdo#103540) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#105189) -> PASS

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4282 -> Patchwork_9213

  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9213: bcdf55b7b6b742fc63634cef1ebbadcd0e937a79 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9213/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/17] drm/i915/gtt: Lazily allocate page directories for gen7

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As we were only supporting aliasing_ppgtt on gen7 for some time, we
> saved a few checks by preallocating the page directories on creation.
> However, since we need 2MiB of page directories for each ppgtt, to
> support arbitrary numbers of user contexts, we need to be more prudent
> in our allocations, and defer the page allocation until it is used. We
> don't recover unused pages yet as we found that doing so on the fly
> (i.e. altering TLB entries) would confuse the GPU.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/17] drm/i915/gtt: Free unused page tables on unbind the context

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As we cannot reliably change used page tables while the context is
> active, the earliest opportunity we have to recover excess pages is when
> the context becomes idle. So whenever we unbind the context (it must be
> idle, and indeed being evicted) free the unused ptes.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/17] drm/i915/gtt: Skip initializing PT with scratch if full

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If we will completely overwrite the PT with PTEs for the object, we can
> forgo filling it with scratch entries.
>
> References: 14826673247e ("drm/i915: Only initialize partially filled pagetables")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/17] drm/i915/gtt: Cache the PTE encoding of the scratch page

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As the most frequent PTE encoding is for the scratch page, cache it upon
> creation.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 14/17] drm/i915/gtt: Reduce a pair of runtime asserts

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> We can stop asserting using WARN_ON as given sufficient CI coverage, we
> can rely on using GEM_BUG_ON() to catch problems before merging.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gtt: Make gen6 page directories evictable

[Matthew Auld]

On 6 June 2018 at 07:51, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Currently all page directories are bound at creation using an
> unevictable node in the GGTT. This severely limits us as we cannot
> remove any inactive ppgtt for new contexts, or under aperture pressure.
> To fix this we need to make the page directory into a first class and
> unbindable vma. Hence, the creation of a custom vma to wrap the page
> directory as opposed to a GEM object.
>
> In this patch, we leave the page directories pinned upon creation.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 261 ++++++++++++++++------------
>  drivers/gpu/drm/i915/i915_gem_gtt.h |   2 +-
>  drivers/gpu/drm/i915/i915_vma.h     |   7 +
>  3 files changed, 159 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ea27f799101f..60b5966360b6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1640,50 +1640,55 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
>  {
>         struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
>         struct i915_address_space *vm = &base->vm;
> -       struct i915_page_table *unused;
> -       gen6_pte_t scratch_pte;
> -       u32 pd_entry, pte, pde;
> -
> -       scratch_pte = vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
> -
> -       gen6_for_all_pdes(unused, &base->pd, pde) {
> -               u32 expected;
> -               gen6_pte_t *pt_vaddr;
> -               const dma_addr_t pt_addr = px_dma(base->pd.page_table[pde]);
> -               pd_entry = readl(ppgtt->pd_addr + pde);
> -               expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
> -
> -               if (pd_entry != expected)
> -                       seq_printf(m, "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
> -                                  pde,
> -                                  pd_entry,
> -                                  expected);
> -               seq_printf(m, "\tPDE: %x\n", pd_entry);
> -
> -               pt_vaddr = kmap_atomic_px(base->pd.page_table[pde]);
> -
> -               for (pte = 0; pte < GEN6_PTES; pte+=4) {
> -                       unsigned long va =
> -                               (pde * PAGE_SIZE * GEN6_PTES) +
> -                               (pte * PAGE_SIZE);
> +       const gen6_pte_t scratch_pte =
> +               vm->pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
> +       struct i915_page_table *pt;
> +       u32 pte, pde;
> +
> +       gen6_for_all_pdes(pt, &base->pd, pde) {
> +               gen6_pte_t *vaddr;
> +
> +               if (pt == base->vm.scratch_pt)
> +                       continue;
> +
> +               if (i915_vma_is_bound(ppgtt->vma, I915_VMA_GLOBAL_BIND)) {
> +                       u32 expected =
> +                               GEN6_PDE_ADDR_ENCODE(px_dma(pt)) |
> +                               GEN6_PDE_VALID;
> +                       u32 pd_entry = readl(ppgtt->pd_addr + pde);
> +
> +                       if (pd_entry != expected)
> +                               seq_printf(m,
> +                                          "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
> +                                          pde,
> +                                          pd_entry,
> +                                          expected);
> +
> +                       seq_printf(m, "\tPDE: %x\n", pd_entry);
> +               }
> +
> +               vaddr = kmap_atomic_px(base->pd.page_table[pde]);
> +               for (pte = 0; pte < GEN6_PTES; pte += 4) {
>                         int i;
> -                       bool found = false;
> +
>                         for (i = 0; i < 4; i++)
> -                               if (pt_vaddr[pte + i] != scratch_pte)
> -                                       found = true;
> -                       if (!found)
> +                               if (vaddr[pte + i] != scratch_pte)
> +                                       break;
> +                       if (i == 4)
>                                 continue;
>
> -                       seq_printf(m, "\t\t0x%lx [%03d,%04d]: =", va, pde, pte);
> +                       seq_printf(m, "\t\t(%03d, %04d) %08lx: ",
> +                                  pde, pte,
> +                                  (pde * GEN6_PTES + pte) * PAGE_SIZE);
>                         for (i = 0; i < 4; i++) {
> -                               if (pt_vaddr[pte + i] != scratch_pte)
> -                                       seq_printf(m, " %08x", pt_vaddr[pte + i]);
> +                               if (vaddr[pte + i] != scratch_pte)
> +                                       seq_printf(m, " %08x", vaddr[pte + i]);
>                                 else
> -                                       seq_puts(m, "  SCRATCH ");
> +                                       seq_puts(m, "  SCRATCH");
>                         }
>                         seq_puts(m, "\n");
>                 }
> -               kunmap_atomic(pt_vaddr);
> +               kunmap_atomic(vaddr);
>         }
>  }
>
> @@ -1697,22 +1702,6 @@ static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
>                   ppgtt->pd_addr + pde);
>  }
>
> -/* Write all the page tables found in the ppgtt structure to incrementing page
> - * directories. */
> -static void gen6_write_page_range(struct i915_hw_ppgtt *base,
> -                                 u32 start, u32 length)
> -{
> -       struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
> -       struct i915_page_table *pt;
> -       unsigned int pde;
> -
> -       gen6_for_each_pde(pt, &base->pd, start, length, pde)
> -               gen6_write_pde(ppgtt, pde, pt);
> -
> -       mark_tlbs_dirty(base);
> -       gen6_ggtt_invalidate(base->vm.i915);
> -}
> -
>  static inline u32 get_pd_offset(struct gen6_hw_ppgtt *ppgtt)
>  {
>         GEM_BUG_ON(ppgtt->base.pd.base.ggtt_offset & 0x3f);
> @@ -1919,8 +1908,12 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>
>                         gen6_initialize_pt(vm, pt);
>                         ppgtt->base.pd.page_table[pde] = pt;
> -                       gen6_write_pde(ppgtt, pde, pt);
> -                       flush = true;
> +
> +                       if (i915_vma_is_bound(ppgtt->vma,
> +                                             I915_VMA_GLOBAL_BIND)) {
> +                               gen6_write_pde(ppgtt, pde, pt);
> +                               flush = true;
> +                       }
>                 }
>         }
>
> @@ -1936,8 +1929,11 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>         return -ENOMEM;
>  }
>
> -static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
> +static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
>  {
> +       struct i915_address_space * const vm = &ppgtt->base.vm;
> +       struct i915_page_table *unused;
> +       u32 pde;
>         int ret;
>
>         ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> @@ -1951,6 +1947,8 @@ static int gen6_ppgtt_init_scratch(struct i915_address_space *vm)
>         }
>
>         gen6_initialize_pt(vm, vm->scratch_pt);
> +       gen6_for_all_pdes(unused, &ppgtt->base.pd, pde)
> +               ppgtt->base.pd.page_table[pde] = vm->scratch_pt;
>
>         return 0;
>  }
> @@ -1975,52 +1973,110 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>  {
>         struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>
> -       drm_mm_remove_node(&ppgtt->node);
> +       i915_vma_unpin(ppgtt->vma);
> +       i915_vma_destroy(ppgtt->vma);
>
>         gen6_ppgtt_free_pd(ppgtt);
>         gen6_ppgtt_free_scratch(vm);
>  }
>
> -static int gen6_ppgtt_allocate_page_directories(struct gen6_hw_ppgtt *ppgtt)
> +static int pd_vma_set_pages(struct i915_vma *vma)
>  {
> -       struct drm_i915_private *dev_priv = ppgtt->base.vm.i915;
> -       struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -       int err;
> +       vma->pages = ERR_PTR(-ENODEV);
> +       return 0;
> +}
>
> -       /* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
> -        * allocator works in address space sizes, so it's multiplied by page
> -        * size. We allocate at the top of the GTT to avoid fragmentation.
> -        */
> -       BUG_ON(!drm_mm_initialized(&ggtt->vm.mm));
> +static void pd_vma_clear_pages(struct i915_vma *vma)
> +{
> +       GEM_BUG_ON(!vma->pages);
>
> -       err = i915_gem_gtt_insert(&ggtt->vm, &ppgtt->node,
> -                                 GEN6_PD_SIZE, GEN6_PD_ALIGN,
> -                                 I915_COLOR_UNEVICTABLE,
> -                                 0, ggtt->vm.total,
> -                                 PIN_HIGH);
> -       if (err)
> -               return err;
> +       vma->pages = NULL;
> +}
>
> -       if (ppgtt->node.start < ggtt->mappable_end)
> -               DRM_DEBUG("Forced to use aperture for PDEs\n");
> +static int pd_vma_bind(struct i915_vma *vma,
> +                      enum i915_cache_level cache_level,
> +                      u32 unused)
> +{
> +       struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
> +       struct gen6_hw_ppgtt *ppgtt = vma->private;
> +       u32 ggtt_offset = i915_ggtt_offset(vma) / PAGE_SIZE;
> +       struct i915_page_table *pt;
> +       unsigned int pde;
>
> -       ppgtt->base.pd.base.ggtt_offset =
> -               ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
> +       ppgtt->base.pd.base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
> +       ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm + ggtt_offset;
>
> -       ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm +
> -               ppgtt->base.pd.base.ggtt_offset / sizeof(gen6_pte_t);
> +       gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
> +               gen6_write_pde(ppgtt, pde, pt);
> +
> +       mark_tlbs_dirty(&ppgtt->base);
> +       gen6_ggtt_invalidate(ppgtt->base.vm.i915);
>
>         return 0;
>  }
>
> -static void gen6_scratch_va_range(struct gen6_hw_ppgtt *ppgtt,
> -                                 u64 start, u64 length)
> +static void pd_vma_unbind(struct i915_vma *vma)
>  {
> -       struct i915_page_table *unused;
> -       u32 pde;
> +}
> +
> +static const struct i915_vma_ops pd_vma_ops = {
> +       .set_pages = pd_vma_set_pages,
> +       .clear_pages = pd_vma_clear_pages,
> +       .bind_vma = pd_vma_bind,
> +       .unbind_vma = pd_vma_unbind,
> +};
> +
> +static struct i915_vma *pd_vma_create(struct gen6_hw_ppgtt *ppgtt, int size)
> +{
> +       struct drm_i915_private *i915 = ppgtt->base.vm.i915;
> +       struct i915_ggtt *ggtt = &i915->ggtt;
> +       struct i915_vma *vma;
> +       int i;
>
> -       gen6_for_each_pde(unused, &ppgtt->base.pd, start, length, pde)
> -               ppgtt->base.pd.page_table[pde] = ppgtt->base.vm.scratch_pt;
> +       GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE));
> +       GEM_BUG_ON(size > ggtt->vm.total);
> +
> +       vma = kmem_cache_zalloc(i915->vmas, GFP_KERNEL);
> +       if (!vma)
> +               return ERR_PTR(-ENOMEM);
> +
> +       for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
> +               init_request_active(&vma->last_read[i], NULL);
> +       init_request_active(&vma->last_fence, NULL);
> +
> +       vma->vm = &ggtt->vm;
> +       vma->ops = &pd_vma_ops;
> +       vma->private = ppgtt;
> +
> +       vma->obj = NULL;
> +       vma->resv = NULL;
> +       vma->size = size;
> +       vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
> +
> +       vma->fence_size = size;
> +       vma->fence_alignment = I915_GTT_MIN_ALIGNMENT;
> +
> +       vma->flags |= I915_VMA_GGTT;
> +       vma->ggtt_view.type = I915_GGTT_VIEW_ROTATED; /* prevent fencing */
> +
> +       INIT_LIST_HEAD(&vma->obj_link);
> +       list_add(&vma->vm_link, &vma->vm->unbound_list);
> +
> +       return vma;
> +}
> +
> +static int gen6_ppgtt_pin(struct i915_hw_ppgtt *base)
> +{
> +       struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
> +
> +       /*
> +        * PPGTT PDEs reside in the GGTT and consists of 512 entries. The
> +        * allocator works in address space sizes, so it's multiplied by page
> +        * size. We allocate at the top of the GTT to avoid fragmentation.
> +        */
> +       return i915_vma_pin(ppgtt->vma,
> +                           0, GEN6_PD_ALIGN,
> +                           PIN_GLOBAL | PIN_HIGH);
>  }
>
>  static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
> @@ -2058,24 +2114,25 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
>         else
>                 BUG();
>
> -       err = gen6_ppgtt_init_scratch(&ppgtt->base.vm);
> +       err = gen6_ppgtt_init_scratch(ppgtt);
>         if (err)
>                 goto err_free;
>
> -       err = gen6_ppgtt_allocate_page_directories(ppgtt);
> -       if (err)
> +       ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
> +       if (IS_ERR(ppgtt->vma))

err = PTR_ERR()

Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/17] drm/i915: Prepare for non-object vma

[Matthew Auld]

On 6 June 2018 at 07:26, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> In order to allow ourselves to use VMA to wrap other entities other than
> GEM objects, we need to allow for the vma->obj backpointer to be NULL.
> In most cases, we know we are operating on a GEM object and its vma, but
> we need the core code (such as i915_vma_pin/insert/bind/unbind) to work
> regardless of the innards.
>
> The remaining eyesore here is vma->obj->cache_level and related (but
> less of an issue) vma->obj->gt_ro. With a bit of care we should mirror
> those on the vma itself.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/17] drm/i915/gtt Onionify error handling for gen6_ppgtt_create

[Matthew Auld]

On 6 June 2018 at 07:26, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Pull the empty stubs together into the top level gen6_ppgtt_create, and
> tear each one down on error in proper onion order (rather than use
> Joonas' pet hate of calling the cleanup function in indeterminable
> state).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/17] drm/i915/gtt Onionify error handling for gen6_ppgtt_create

[Matthew Auld]

On 7 June 2018 at 00:24, Matthew Auld <matthew.william.auld@gmail.com> wrote:
> On 6 June 2018 at 07:26, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Pull the empty stubs together into the top level gen6_ppgtt_create, and
>> tear each one down on error in proper onion order (rather than use
>> Joonas' pet hate of calling the cleanup function in indeterminable
>> state).
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>

s/gtt Onionify/gtt: Onionify/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/17] drm/i915/gtt: Reorder aliasing_ppgtt fini

[Matthew Auld]

On 6 June 2018 at 07:27, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> To allow ourselves to use a first class vma for the aliasing_ppgtt page
> directory, we have to reorder the shutdown on module unload to remove
> and unpin the aliasing_ppgtt before complaining about any objects left
> in the GGTT.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx