On 13/06/2019 16:36, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-06-13 16:19:00)
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Continuing on the theme of better logical organization of our code, make
the first step towards making the ggtt code better isolated from wider
struct drm_i915_private.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 54 +++++++++++++++++------------
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d09a4d9b71da..285a7a02c015 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2832,6 +2832,8 @@ static void fini_aliasing_ppgtt(struct drm_i915_private *i915)
struct i915_ggtt *ggtt = &i915->ggtt;
struct i915_ppgtt *ppgtt;
+ mutex_lock(&i915->drm.struct_mutex);
Do we still need to appease lockdep here? Hopefully not.
Maybe not in your tree :) but in drm-tip AFAICS:
fini_aliasing_ppgtt
-> i915_vm_put
-> i915_vm_release
-> ppgtt_destroy_vma
-> i915_vma_destroy
+
ppgtt = fetch_and_zero(&i915->mm.aliasing_ppgtt);
if (!ppgtt)
return;
@@ -2840,6 +2842,8 @@ static void fini_aliasing_ppgtt(struct drm_i915_private *i915)
ggtt->vm.vma_ops.bind_vma = ggtt_bind_vma;
ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
+
+ mutex_unlock(&i915->drm.struct_mutex);
}
static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
@@ -2941,21 +2945,15 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
return ret;
}
-/**
- * i915_ggtt_cleanup_hw - Clean up GGTT hardware initialization
- * @dev_priv: i915 device
- */
-void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
+static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
{
- struct i915_ggtt *ggtt = &dev_priv->ggtt;
struct i915_address_space *vm = &ggtt->vm;
+ struct drm_i915_private *i915 = vm->i915;
struct i915_vma *vma, *vn;
- struct pagevec *pvec;
vm->closed = true;
- mutex_lock(&dev_priv->drm.struct_mutex);
- fini_aliasing_ppgtt(dev_priv);
+ mutex_lock(&i915->drm.struct_mutex);
As the lock here is just for the i915_vma_unbind and not
fini_aliasing_ppgtt.
list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link)
WARN_ON(i915_vma_unbind(vma));
@@ -2972,18 +2970,33 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
vm->cleanup(vm);
- pvec = &dev_priv->mm.wc_stash.pvec;
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ arch_phys_wc_del(ggtt->mtrr);
+ io_mapping_fini(&ggtt->iomap);
+}
+
+/**
+ * i915_ggtt_cleanup_hw - Clean up GGTT hardware initialization
+ * @dev_priv: i915 device
+ */
+void i915_ggtt_cleanup_hw(struct drm_i915_private *i915)
+{
+ struct pagevec *pvec;
+
+ fini_aliasing_ppgtt(i915);
+
+ ggtt_cleanup_hw(&i915->ggtt);
+
+ mutex_lock(&i915->drm.struct_mutex);
+ pvec = &i915->mm.wc_stash.pvec;
if (pvec->nr) {
set_pages_array_wb(pvec->pages, pvec->nr);
__pagevec_release(pvec);
}
+ mutex_unlock(&i915->drm.struct_mutex);
The wc_stash doesn't use struct_mutex, so no lockdep appeasing required
here.
True, removed.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
