[Intel-gfx] [PATCH 02/28] drm/i915: Track ggtt fence reservations under its own mutex

Chris Wilson chris at chris-wilson.co.uk
Mon Jun 10 07:21:00 UTC 2019


We can reduce the locking for fence registers from the dev->struct_mutex
to a local mutex. We could introduce a mutex for the sole purpose of
tracking the fence acquisition, except there is a little bit of overlap
with the fault tracking, so use the i915_ggtt.mutex as it covers both.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c |  7 ++
 drivers/gpu/drm/i915/i915_debugfs.c          |  5 +-
 drivers/gpu/drm/i915/i915_gem_fence_reg.c    | 81 ++++++++++++++------
 drivers/gpu/drm/i915/i915_gem_fence_reg.h    |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.h          |  1 +
 drivers/gpu/drm/i915/i915_vma.h              |  4 +-
 6 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 3be67e561c26..127faef8d8c2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -1166,7 +1166,14 @@ static int evict_fence(void *data)
 		goto out_unlock;
 	}
 
+	err = i915_vma_pin(arg->vma, 0, 0, PIN_GLOBAL | PIN_MAPPABLE);
+	if (err) {
+		pr_err("Unable to pin vma for Y-tiled fence; err:%d\n", err);
+		goto out_unlock;
+	}
+
 	err = i915_vma_pin_fence(arg->vma);
+	i915_vma_unpin(arg->vma);
 	if (err) {
 		pr_err("Unable to pin Y-tiled fence; err:%d\n", err);
 		goto out_unlock;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 331b2f478c48..b0f4c3638d21 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -882,10 +882,11 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data)
 
 	rcu_read_lock();
 	for (i = 0; i < i915->ggtt.num_fences; i++) {
-		struct i915_vma *vma = i915->ggtt.fence_regs[i].vma;
+		struct i915_fence_reg *reg = &i915->ggtt.fence_regs[i];
+		struct i915_vma *vma = reg->vma;
 
 		seq_printf(m, "Fence %d, pin count = %d, object = ",
-			   i, i915->ggtt.fence_regs[i].pin_count);
+			   i, atomic_read(&reg->pin_count));
 		if (!vma)
 			seq_puts(m, "unused");
 		else
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
index 4ba3726556a4..d13be3b0e91d 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
@@ -301,15 +301,24 @@ static int fence_update(struct i915_fence_reg *fence,
  */
 int i915_vma_put_fence(struct i915_vma *vma)
 {
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
 	struct i915_fence_reg *fence = vma->fence;
+	int err;
 
 	if (!fence)
 		return 0;
 
-	if (fence->pin_count)
+	if (atomic_read(&fence->pin_count))
 		return -EBUSY;
 
-	return fence_update(fence, NULL);
+	err = mutex_lock_interruptible(&ggtt->vm.mutex);
+	if (err)
+		return err;
+
+	err = fence_update(fence, NULL);
+	mutex_unlock(&ggtt->vm.mutex);
+
+	return err;
 }
 
 static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
@@ -319,7 +328,7 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
 	list_for_each_entry(fence, &i915->ggtt.fence_list, link) {
 		GEM_BUG_ON(fence->vma && fence->vma->fence != fence);
 
-		if (fence->pin_count)
+		if (atomic_read(&fence->pin_count))
 			continue;
 
 		return fence;
@@ -353,6 +362,7 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
 int
 i915_vma_pin_fence(struct i915_vma *vma)
 {
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
 	struct i915_fence_reg *fence;
 	struct i915_vma *set = i915_gem_object_is_tiled(vma->obj) ? vma : NULL;
 	int err;
@@ -361,27 +371,34 @@ i915_vma_pin_fence(struct i915_vma *vma)
 	 * Note that we revoke fences on runtime suspend. Therefore the user
 	 * must keep the device awake whilst using the fence.
 	 */
-	assert_rpm_wakelock_held(vma->vm->i915);
+	assert_rpm_wakelock_held(ggtt->vm.i915);
+	GEM_BUG_ON(!i915_vma_is_pinned(vma));
+
+	err = mutex_lock_interruptible(&ggtt->vm.mutex);
+	if (err)
+		return err;
 
 	/* Just update our place in the LRU if our fence is getting reused. */
 	if (vma->fence) {
 		fence = vma->fence;
 		GEM_BUG_ON(fence->vma != vma);
-		fence->pin_count++;
+		atomic_inc(&fence->pin_count);
 		if (!fence->dirty) {
-			list_move_tail(&fence->link,
-				       &fence->i915->ggtt.fence_list);
-			return 0;
+			list_move_tail(&fence->link, &ggtt->fence_list);
+			goto unlock;
 		}
 	} else if (set) {
 		fence = fence_find(vma->vm->i915);
-		if (IS_ERR(fence))
-			return PTR_ERR(fence);
+		if (IS_ERR(fence)) {
+			err = PTR_ERR(fence);
+			goto unlock;
+		}
 
-		GEM_BUG_ON(fence->pin_count);
-		fence->pin_count++;
-	} else
-		return 0;
+		GEM_BUG_ON(atomic_read(&fence->pin_count));
+		atomic_inc(&fence->pin_count);
+	} else {
+		goto unlock;
+	}
 
 	err = fence_update(fence, set);
 	if (err)
@@ -391,10 +408,12 @@ i915_vma_pin_fence(struct i915_vma *vma)
 	GEM_BUG_ON(vma->fence != (set ? fence : NULL));
 
 	if (set)
-		return 0;
+		goto unlock;
 
 out_unpin:
-	fence->pin_count--;
+	atomic_dec(&fence->pin_count);
+unlock:
+	mutex_unlock(&ggtt->vm.mutex);
 	return err;
 }
 
@@ -412,28 +431,38 @@ i915_reserve_fence(struct drm_i915_private *i915)
 	int count;
 	int ret;
 
-	lockdep_assert_held(&i915->drm.struct_mutex);
+	mutex_lock(&i915->ggtt.vm.mutex);
 
 	/* Keep at least one fence available for the display engine. */
 	count = 0;
 	list_for_each_entry(fence, &i915->ggtt.fence_list, link)
-		count += !fence->pin_count;
-	if (count <= 1)
-		return ERR_PTR(-ENOSPC);
+		count += !atomic_read(&fence->pin_count);
+	if (count <= 1) {
+		ret = -ENOSPC;
+		goto err;
+	}
 
 	fence = fence_find(i915);
-	if (IS_ERR(fence))
-		return fence;
+	if (IS_ERR(fence)) {
+		ret = PTR_ERR(fence);
+		goto err;
+	}
 
 	if (fence->vma) {
 		/* Force-remove fence from VMA */
 		ret = fence_update(fence, NULL);
 		if (ret)
-			return ERR_PTR(ret);
+			goto err;
 	}
 
 	list_del(&fence->link);
+	mutex_unlock(&i915->ggtt.vm.mutex);
+
 	return fence;
+
+err:
+	mutex_unlock(&i915->ggtt.vm.mutex);
+	return ERR_PTR(ret);
 }
 
 /**
@@ -444,9 +473,11 @@ i915_reserve_fence(struct drm_i915_private *i915)
  */
 void i915_unreserve_fence(struct i915_fence_reg *fence)
 {
-	lockdep_assert_held(&fence->i915->drm.struct_mutex);
+	struct i915_ggtt *ggtt = &fence->i915->ggtt;
 
-	list_add(&fence->link, &fence->i915->ggtt.fence_list);
+	mutex_lock(&ggtt->vm.mutex);
+	list_add(&fence->link, &ggtt->fence_list);
+	mutex_unlock(&ggtt->vm.mutex);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.h b/drivers/gpu/drm/i915/i915_gem_fence_reg.h
index d2da98828179..d7c6ebf789c1 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.h
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.h
@@ -40,7 +40,7 @@ struct i915_fence_reg {
 	struct list_head link;
 	struct drm_i915_private *i915;
 	struct i915_vma *vma;
-	int pin_count;
+	atomic_t pin_count;
 	int id;
 	/**
 	 * Whether the tiling parameters for the currently
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index ee396938de10..5f155bf183bb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -36,6 +36,7 @@
 
 #include <linux/io-mapping.h>
 #include <linux/mm.h>
+#include <linux/mutex.h>
 #include <linux/pagevec.h>
 
 #include "gt/intel_reset.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4b769db649bf..908118ade441 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -419,8 +419,8 @@ int __must_check i915_vma_put_fence(struct i915_vma *vma);
 
 static inline void __i915_vma_unpin_fence(struct i915_vma *vma)
 {
-	GEM_BUG_ON(vma->fence->pin_count <= 0);
-	vma->fence->pin_count--;
+	GEM_BUG_ON(atomic_read(&vma->fence->pin_count) <= 0);
+	atomic_dec(&vma->fence->pin_count);
 }
 
 /**
-- 
2.20.1



More information about the Intel-gfx mailing list