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@xxxxxxxxxxxxxxxxxx>
---
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(®->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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
