[PATCH 40/92] drm/i915: Move aperture tracking under GGTT mutex

Chris Wilson chris at chris-wilson.co.uk
Mon Sep 17 11:10:00 UTC 2018


Alongside tracking fence registers for aperture access, we have tracking
the aperture access itself, both for user faults and internal iomaps.
This move all the magic bits on the vma and global list management under
the GGTT mutex.

Similar to fence management, we require this mutex from inside runtime
suspend/resume management and so must be careful about its order wrt the
runtime wakeref (that is we must be holding the wakeref before taking
the mutex here).

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_gem.c     | 16 +++++++++++++---
 drivers/gpu/drm/i915/i915_vma.c     |  8 +++++++-
 drivers/gpu/drm/i915/i915_vma.h     |  4 ++++
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c6efb5ca7557..641ebb4425ef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -82,7 +82,7 @@ static char get_tiling_flag(struct drm_i915_gem_object *obj)
 
 static char get_global_flag(struct drm_i915_gem_object *obj)
 {
-	return obj->userfault_count ? 'g' : ' ';
+	return READ_ONCE(obj->userfault_count) ? 'g' : ' ';
 }
 
 static char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9f63036494d8..d48975d8a8db 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -837,12 +837,14 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
 
+		mutex_lock(&dev_priv->ggtt.vm.mutex);
 		for_each_ggtt_vma(vma, obj) {
 			if (vma->iomap)
 				continue;
 
 			i915_vma_unset_ggtt_write(vma);
 		}
+		mutex_unlock(&dev_priv->ggtt.vm.mutex);
 		break;
 
 	case I915_GEM_DOMAIN_WC:
@@ -1989,12 +1991,14 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 		goto err_fence;
 
 	/* Mark as being mmapped into userspace for later revocation */
+	mutex_lock(&vma->vm->mutex);
 	assert_rpm_wakelock_held(dev_priv);
 	if (!i915_vma_set_userfault(vma) && !obj->userfault_count++)
 		list_add(&obj->userfault_link, &dev_priv->mm.userfault_list);
 	GEM_BUG_ON(!obj->userfault_count);
 
 	i915_vma_set_ggtt_write(vma);
+	mutex_unlock(&vma->vm->mutex);
 
 err_fence:
 	i915_vma_unpin_fence(vma);
@@ -2047,6 +2051,7 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 {
 	struct i915_vma *vma;
 
+	lockdep_assert_held(&to_i915(obj->base.dev)->ggtt.vm.mutex);
 	GEM_BUG_ON(!obj->userfault_count);
 
 	obj->userfault_count = 0;
@@ -2086,8 +2091,8 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 	 * requirement that operations to the GGTT be made holding the RPM
 	 * wakeref.
 	 */
-	lockdep_assert_held(&i915->drm.struct_mutex);
 	wakeref = intel_runtime_pm_get(i915);
+	mutex_lock(&i915->ggtt.vm.mutex);
 
 	if (!obj->userfault_count)
 		goto out;
@@ -2104,6 +2109,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 	wmb();
 
 out:
+	mutex_unlock(&i915->ggtt.vm.mutex);
 	intel_runtime_pm_put(i915, wakeref);
 }
 
@@ -2115,10 +2121,12 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 	/*
 	 * Only called during RPM suspend. All users of the userfault_list
 	 * must be holding an RPM wakeref to ensure that this can not
-	 * run concurrently with themselves (and use the struct_mutex for
+	 * run concurrently with themselves (and use the ggtt->mutex for
 	 * protection between themselves).
 	 */
 
+	mutex_lock(&dev_priv->ggtt.vm.mutex);
+
 	list_for_each_entry_safe(obj, on,
 				 &dev_priv->mm.userfault_list, userfault_link)
 		__i915_gem_object_release_mmap(obj);
@@ -2147,6 +2155,8 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 		GEM_BUG_ON(i915_vma_has_userfault(reg->vma));
 		reg->dirty = true;
 	}
+
+	mutex_unlock(&dev_priv->ggtt.vm.mutex);
 }
 
 static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
@@ -4331,7 +4341,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 		mutex_unlock(&i915->drm.struct_mutex);
 
 		GEM_BUG_ON(obj->bind_count);
-		GEM_BUG_ON(obj->userfault_count);
+		GEM_BUG_ON(READ_ONCE(obj->userfault_count));
 		GEM_BUG_ON(atomic_read(&obj->frontbuffer_bits));
 		GEM_BUG_ON(!list_empty(&obj->lut_list));
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 31efc971a3a8..c42adedf6d40 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -374,7 +374,9 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	if (err)
 		goto err_unpin;
 
+	mutex_lock(&vma->vm->mutex);
 	i915_vma_set_ggtt_write(vma);
+	mutex_unlock(&vma->vm->mutex);
 	return ptr;
 
 err_unpin:
@@ -385,6 +387,8 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 
 void i915_vma_flush_writes(struct i915_vma *vma)
 {
+	lockdep_assert_held(&vma->vm->mutex);
+
 	if (!i915_vma_has_ggtt_write(vma))
 		return;
 
@@ -399,7 +403,9 @@ void i915_vma_unpin_iomap(struct i915_vma *vma)
 
 	GEM_BUG_ON(vma->iomap == NULL);
 
+	mutex_lock(&vma->vm->mutex);
 	i915_vma_flush_writes(vma);
+	mutex_unlock(&vma->vm->mutex);
 
 	i915_vma_unpin_fence(vma);
 	i915_vma_unpin(vma);
@@ -859,7 +865,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
 	struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
 	u64 vma_offset;
 
-	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
+	lockdep_assert_held(&vma->vm->mutex);
 
 	if (!i915_vma_has_userfault(vma))
 		return;
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4f7c1c7599f4..50508d93dbb9 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -162,12 +162,14 @@ static inline bool i915_vma_has_ggtt_write(const struct i915_vma *vma)
 
 static inline void i915_vma_set_ggtt_write(struct i915_vma *vma)
 {
+	lockdep_assert_held(&vma->vm->mutex);
 	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
 	vma->flags |= I915_VMA_GGTT_WRITE;
 }
 
 static inline void i915_vma_unset_ggtt_write(struct i915_vma *vma)
 {
+	lockdep_assert_held(&vma->vm->mutex);
 	vma->flags &= ~I915_VMA_GGTT_WRITE;
 }
 
@@ -185,12 +187,14 @@ static inline bool i915_vma_is_closed(const struct i915_vma *vma)
 
 static inline bool i915_vma_set_userfault(struct i915_vma *vma)
 {
+	lockdep_assert_held(&vma->vm->mutex);
 	GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
 	return __test_and_set_bit(I915_VMA_USERFAULT_BIT, &vma->flags);
 }
 
 static inline void i915_vma_unset_userfault(struct i915_vma *vma)
 {
+	lockdep_assert_held(&vma->vm->mutex);
 	return __clear_bit(I915_VMA_USERFAULT_BIT, &vma->flags);
 }
 
-- 
2.19.0



More information about the Intel-gfx-trybot mailing list