Skip to content

Commit

Permalink
drm/i915: Track ggtt fence reservations under its own mutex
Browse files Browse the repository at this point in the history
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>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190822060914.2671-1-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Aug 22, 2019
1 parent 8e458fe commit e2ccc50
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 49 deletions.
7 changes: 7 additions & 0 deletions drivers/gpu/drm/i915/gt/selftest_hangcheck.c
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,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;
Expand Down
10 changes: 5 additions & 5 deletions drivers/gpu/drm/i915/gvt/aperture_gm.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,14 @@ static void free_vgpu_fence(struct intel_vgpu *vgpu)

intel_runtime_pm_get(&dev_priv->runtime_pm);

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->runtime_pm);
}
Expand All @@ -195,7 +195,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
intel_runtime_pm_get(rpm);

/* 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);
Expand All @@ -207,7 +207,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(rpm);
return 0;
out_free_fence:
Expand All @@ -220,7 +220,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(rpm);
return -ENOSPC;
}
Expand Down
5 changes: 3 additions & 2 deletions drivers/gpu/drm/i915/i915_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,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
Expand Down
108 changes: 69 additions & 39 deletions drivers/gpu/drm/i915/i915_gem_fence_reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,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)
Expand All @@ -317,7 +326,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;
Expand All @@ -330,6 +339,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
Expand All @@ -350,48 +401,23 @@ 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;

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

Expand All @@ -404,16 +430,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);

Expand All @@ -429,6 +456,7 @@ struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
}

list_del(&fence->link);

return fence;
}

Expand All @@ -440,9 +468,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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/i915_gem_fence_reg.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/i915_vma.h
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,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);
}

/**
Expand Down

0 comments on commit e2ccc50

Please sign in to comment.