[PATCH 4/4] drm/i915: Track ggtt fence reservations under its own mutex

Chris Wilson
2019-06-13T06:20:28Z

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@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 7 ++
drivers/gpu/drm/i915/gvt/aperture_gm.c | 10 +-
drivers/gpu/drm/i915/i915_debugfs.c | 5 +-
drivers/gpu/drm/i915/i915_gem_fence_reg.c | 108 ++++++++++++-------
drivers/gpu/drm/i915/i915_gem_fence_reg.h | 2 +-
drivers/gpu/drm/i915/i915_vma.h | 4 +-
6 files changed, 87 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 45379a63e013..3752d3172543 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -1165,7 +1165,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/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c
index 4098902bfaeb..2bd9dcebd48c 100644
--- a/drivers/gpu/drm/i915/gvt/aperture_gm.c
+++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c
@@ -172,14 +172,14 @@ static void free_vgpu_fence(struct intel_vgpu *vgpu)

intel_runtime_pm_get(dev_priv);

- mutex_lock(&dev_priv->drm.struct_mutex);
+ mutex_lock(&dev_priv->ggtt.vm.mutex);
_clear_vgpu_fence(vgpu);
for (i = 0; i < vgpu_fence_sz(vgpu); i++) {
reg = vgpu->fence.regs[i];
i915_unreserve_fence(reg);
vgpu->fence.regs[i] = NULL;
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ mutex_unlock(&dev_priv->ggtt.vm.mutex);

intel_runtime_pm_put_unchecked(dev_priv);
}
@@ -194,7 +194,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
intel_runtime_pm_get(dev_priv);

/* Request fences from host */
- mutex_lock(&dev_priv->drm.struct_mutex);
+ mutex_lock(&dev_priv->ggtt.vm.mutex);

for (i = 0; i < vgpu_fence_sz(vgpu); i++) {
reg = i915_reserve_fence(dev_priv);
@@ -206,7 +206,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)

_clear_vgpu_fence(vgpu);

- mutex_unlock(&dev_priv->drm.struct_mutex);
+ mutex_unlock(&dev_priv->ggtt.vm.mutex);
intel_runtime_pm_put_unchecked(dev_priv);
return 0;
out_free_fence:
@@ -219,7 +219,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
i915_unreserve_fence(reg);
vgpu->fence.regs[i] = NULL;
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ mutex_unlock(&dev_priv->ggtt.vm.mutex);
intel_runtime_pm_put_unchecked(dev_priv);
return -ENOSPC;
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 323863504111..7a54f2d432af 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -710,10 +710,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 1c9466676caf..24bdffac6380 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;
@@ -332,6 +341,48 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
return ERR_PTR(-EDEADLK);
}

+static 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;
+
+ /* 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);
+ atomic_inc(&fence->pin_count);
+ if (!fence->dirty) {
+ list_move_tail(&fence->link, &ggtt->fence_list);
+ return 0;
+ }
+ } else if (set) {
+ fence = fence_find(vma->vm->i915);
+ if (IS_ERR(fence))
+ return PTR_ERR(fence);
+
+ GEM_BUG_ON(atomic_read(&fence->pin_count));
+ atomic_inc(&fence->pin_count);
+ } else {
+ return 0;
+ }
+
+ err = fence_update(fence, set);
+ if (err)
+ goto out_unpin;
+
+ GEM_BUG_ON(fence->vma != set);
+ GEM_BUG_ON(vma->fence != (set ? fence : NULL));
+
+ if (set)
+ return 0;
+
+out_unpin:
+ atomic_dec(&fence->pin_count);
+ return err;
+}
+
/**
* i915_vma_pin_fence - set up fencing for a vma
* @vma: vma to map through a fence reg
@@ -352,8 +403,6 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
*/
int i915_vma_pin_fence(struct i915_vma *vma)
{
- struct i915_fence_reg *fence;
- struct i915_vma *set = i915_gem_object_is_tiled(vma->obj) ? vma : NULL;
int err;

/*
@@ -361,39 +410,16 @@ int i915_vma_pin_fence(struct i915_vma *vma)
* must keep the device awake whilst using the fence.
*/
assert_rpm_wakelock_held(vma->vm->i915);
+ GEM_BUG_ON(!i915_vma_is_pinned(vma));
+ GEM_BUG_ON(!i915_vma_is_ggtt(vma));

- /* 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++;
- if (!fence->dirty) {
- list_move_tail(&fence->link,
- &fence->i915->ggtt.fence_list);
- return 0;
- }
- } else if (set) {
- fence = fence_find(vma->vm->i915);
- if (IS_ERR(fence))
- return PTR_ERR(fence);
-
- GEM_BUG_ON(fence->pin_count);
- fence->pin_count++;
- } else
- return 0;
-
- err = fence_update(fence, set);
+ err = mutex_lock_interruptible(&vma->vm->mutex);
if (err)
- goto out_unpin;
+ return err;

- GEM_BUG_ON(fence->vma != set);
- GEM_BUG_ON(vma->fence != (set ? fence : NULL));
-
- if (set)
- return 0;
+ err = __i915_vma_pin_fence(vma);
+ mutex_unlock(&vma->vm->mutex);

-out_unpin:
- fence->pin_count--;
return err;
}

@@ -406,16 +432,17 @@ int i915_vma_pin_fence(struct i915_vma *vma)
*/
struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
{
+ struct i915_ggtt *ggtt = &i915->ggtt;
struct i915_fence_reg *fence;
int count;
int ret;

- lockdep_assert_held(&i915->drm.struct_mutex);
+ lockdep_assert_held(&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;
+ list_for_each_entry(fence, &ggtt->fence_list, link)
+ count += !atomic_read(&fence->pin_count);
if (count <= 1)
return ERR_PTR(-ENOSPC);

@@ -431,6 +458,7 @@ struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
}

list_del(&fence->link);
+
return fence;
}

@@ -442,9 +470,11 @@ struct i915_fence_reg *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;
+
+ lockdep_assert_held(&ggtt->vm.mutex);

- list_add(&fence->link, &fence->i915->ggtt.fence_list);
+ list_add(&fence->link, &ggtt->fence_list);
}

/**
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_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);
}

/**