[PATCH 11/17] drm/i915: Move aperture tracking under GGTT mutex

Chris Wilson chris at chris-wilson.co.uk
Mon Jul 23 20:09:49 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 8d78a0e89c76..afd79585146b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -80,7 +80,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 f7be29f1df94..a0381f81318e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -834,12 +834,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:
@@ -1981,12 +1983,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);
@@ -2039,6 +2043,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;
@@ -2077,8 +2082,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);
 	intel_runtime_pm_get(i915);
+	mutex_lock(&i915->ggtt.vm.mutex);
 
 	if (!obj->userfault_count)
 		goto out;
@@ -2095,6 +2100,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 	wmb();
 
 out:
+	mutex_unlock(&i915->ggtt.vm.mutex);
 	intel_runtime_pm_put(i915);
 }
 
@@ -2107,10 +2113,12 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
 	/*
 	 * 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(&i915->ggtt.vm.mutex);
+
 	list_for_each_entry_safe(obj, on,
 				 &i915->mm.userfault_list, userfault_link)
 		__i915_gem_object_release_mmap(obj);
@@ -2139,6 +2147,8 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
 		GEM_BUG_ON(i915_vma_has_userfault(reg->vma));
 		reg->dirty = true;
 	}
+
+	mutex_unlock(&i915->ggtt.vm.mutex);
 }
 
 static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
@@ -4767,7 +4777,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 c60f32bae8b7..e0d601f3f962 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -375,7 +375,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:
@@ -386,6 +388,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;
 
@@ -400,7 +404,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);
@@ -863,7 +869,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 e0bb71ba4194..08d73f7f6b24 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.18.0



More information about the Intel-gfx-trybot mailing list