From bf0840cdb3043ebfa40ac28e19be2886efcd5886 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 25 May 2020 08:53:36 +0100 Subject: drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt In order to keep userptr distinct from ggtt mmaps in the eyes of lockdep, we need to avoid marking those userptr vma as PIN_GLOBAL. (So long as we comply with only using them as local PIN_USER!) References: https://gitlab.freedesktop.org/drm/intel/-/issues/1880 Signed-off-by: Chris Wilson Acked-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20200525075347.582-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/i915/gt/intel_ggtt.c') diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 66165b10256e..8c275f8588c3 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -424,22 +424,17 @@ static int ggtt_bind_vma(struct i915_vma *vma, struct drm_i915_gem_object *obj = vma->obj; u32 pte_flags; + if (i915_vma_is_bound(vma, ~flags & I915_VMA_BIND_MASK)) + return 0; + /* Applicable to VLV (gen8+ do not support RO in the GGTT) */ pte_flags = 0; if (i915_gem_object_is_readonly(obj)) pte_flags |= PTE_READ_ONLY; vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags); - vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; - /* - * Without aliasing PPGTT there's no difference between - * GLOBAL/LOCAL_BIND, it's all the same ptes. Hence unconditionally - * upgrade to both bound if we bind either to avoid double-binding. - */ - atomic_or(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND, &vma->flags); - return 0; } -- cgit From 0109a16ef391b2ebfbfdf08250c1dfb5dbf83d1e Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 26 May 2020 16:07:39 +0100 Subject: drm/i915/gt: Clear LOCAL_BIND from shared GGTT on resume We only restore GLOBAL binds upon resume as we expect these to be pinned for use by HW, whereas the LOCAL binds can be recreated on demand once userspace is resumed. For the LOCAL bind to be recreated in the global GTT (for old systems without ppgtt), we need to clear its presence flag on deciding not to restore the mapping upon resume. Fixes: bf0840cdb304 ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20200526150739.26147-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/gt/intel_ggtt.c') diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8c275f8588c3..317172ad5ef3 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -1161,6 +1161,11 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt) ggtt->invalidate(ggtt); } +static unsigned int clear_bind(struct i915_vma *vma) +{ + return atomic_fetch_and(~I915_VMA_BIND_MASK, &vma->flags); +} + void i915_ggtt_resume(struct i915_ggtt *ggtt) { struct i915_vma *vma; @@ -1179,10 +1184,9 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) list_for_each_entry(vma, &ggtt->vm.bound_list, vm_link) { struct drm_i915_gem_object *obj = vma->obj; - if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) + if (!(clear_bind(vma) & I915_VMA_GLOBAL_BIND)) continue; - clear_bit(I915_VMA_GLOBAL_BIND_BIT, __i915_vma_flags(vma)); WARN_ON(i915_vma_bind(vma, obj ? obj->cache_level : 0, PIN_GLOBAL, NULL)); -- cgit From dc6cd912c7cd83ec9859429c552b2986c0386b90 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 28 May 2020 16:04:52 +0100 Subject: drm/i915/gt: Restore both GGTT bindings on resume We should be able to skip restoring LOCAL (user) binds within the GGTT on resume and let them be restored upon demand. However, our consistency checks demand that the bind flags match the node state, and we cannot simply clear the flags, we need to evict as well. For now, make sure we restore the bind flags exactly upon resume. Fixes: 0109a16ef391 ("drm/i915/gt: Clear LOCAL_BIND from shared GGTT on resume") Fixes: bf0840cdb304 ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20200528150452.7880-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/gt/intel_ggtt.c') diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 317172ad5ef3..ffe285b0b3bd 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -1183,13 +1183,11 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) /* clflush objects bound into the GGTT and rebind them. */ list_for_each_entry(vma, &ggtt->vm.bound_list, vm_link) { struct drm_i915_gem_object *obj = vma->obj; - - if (!(clear_bind(vma) & I915_VMA_GLOBAL_BIND)) - continue; + unsigned int was_bound = clear_bind(vma); WARN_ON(i915_vma_bind(vma, obj ? obj->cache_level : 0, - PIN_GLOBAL, NULL)); + was_bound, NULL)); if (obj) { /* only used during resume => exclusive access */ flush |= fetch_and_zero(&obj->write_domain); obj->read_domains |= I915_GEM_DOMAIN_GTT; -- cgit From bffa18dd0bca90112746bafd333386c71fe55efe Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 28 May 2020 09:24:27 +0100 Subject: drm/i915/gt: Remove local entries from GGTT on suspend Across suspend/resume, we clear the entire GGTT and rebuild from scratch. In particular, we want to only preserve the global entries for use by the HW, and delay reinstating the local binds until required by the user. This means that we can evict any local binds in the global GTT, saving any time in preserving their state, as they will be rebound on demand. References: https://gitlab.freedesktop.org/drm/intel/-/issues/1947 Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20200528082427.21402-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/gt/intel_ggtt.c') diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index ffe285b0b3bd..323c328d444a 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -108,13 +108,32 @@ static bool needs_idle_maps(struct drm_i915_private *i915) void i915_ggtt_suspend(struct i915_ggtt *ggtt) { - struct i915_vma *vma; + struct i915_vma *vma, *vn; + int open; + + mutex_lock(&ggtt->vm.mutex); + + /* Skip rewriting PTE on VMA unbind. */ + open = atomic_xchg(&ggtt->vm.open, 0); - list_for_each_entry(vma, &ggtt->vm.bound_list, vm_link) + list_for_each_entry_safe(vma, vn, &ggtt->vm.bound_list, vm_link) { + GEM_BUG_ON(!drm_mm_node_allocated(&vma->node)); i915_vma_wait_for_bind(vma); + if (i915_vma_is_pinned(vma)) + continue; + + if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) { + __i915_vma_evict(vma); + drm_mm_remove_node(&vma->node); + } + } + ggtt->vm.clear_range(&ggtt->vm, 0, ggtt->vm.total); ggtt->invalidate(ggtt); + atomic_set(&ggtt->vm.open, open); + + mutex_unlock(&ggtt->vm.mutex); intel_gt_check_and_clear_faults(ggtt->vm.gt); } -- cgit From 12b07256c22399e10590c994f3e93970b7600053 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 3 Jul 2020 11:25:19 +0100 Subject: drm/i915: Export ppgtt_bind_vma Reuse the ppgtt_bind_vma() for aliasing_ppgtt_bind_vma() so we can reduce some code near-duplication. The catch is that we need to then pass along the i915_address_space and not rely on vma->vm, as they differ with the aliasing-ppgtt. Signed-off-by: Chris Wilson Reviewed-by: Andi Shyti Link: https://patchwork.freedesktop.org/patch/msgid/20200703102519.26539-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 49 +++++++++++++----------------------- 1 file changed, 17 insertions(+), 32 deletions(-) (limited to 'drivers/gpu/drm/i915/gt/intel_ggtt.c') diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 323c328d444a..62979ea591f0 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -436,7 +436,8 @@ static void i915_ggtt_clear_range(struct i915_address_space *vm, intel_gtt_clear_range(start >> PAGE_SHIFT, length >> PAGE_SHIFT); } -static int ggtt_bind_vma(struct i915_vma *vma, +static int ggtt_bind_vma(struct i915_address_space *vm, + struct i915_vma *vma, enum i915_cache_level cache_level, u32 flags) { @@ -451,15 +452,15 @@ static int ggtt_bind_vma(struct i915_vma *vma, if (i915_gem_object_is_readonly(obj)) pte_flags |= PTE_READ_ONLY; - vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags); + vm->insert_entries(vm, vma, cache_level, pte_flags); vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; return 0; } -static void ggtt_unbind_vma(struct i915_vma *vma) +static void ggtt_unbind_vma(struct i915_address_space *vm, struct i915_vma *vma) { - vma->vm->clear_range(vma->vm, vma->node.start, vma->size); + vm->clear_range(vm, vma->node.start, vma->size); } static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt) @@ -567,7 +568,8 @@ err: return ret; } -static int aliasing_gtt_bind_vma(struct i915_vma *vma, +static int aliasing_gtt_bind_vma(struct i915_address_space *vm, + struct i915_vma *vma, enum i915_cache_level cache_level, u32 flags) { @@ -580,44 +582,27 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma, pte_flags |= PTE_READ_ONLY; if (flags & I915_VMA_LOCAL_BIND) { - struct i915_ppgtt *alias = i915_vm_to_ggtt(vma->vm)->alias; + struct i915_ppgtt *alias = i915_vm_to_ggtt(vm)->alias; - if (flags & I915_VMA_ALLOC) { - ret = alias->vm.allocate_va_range(&alias->vm, - vma->node.start, - vma->size); - if (ret) - return ret; - - set_bit(I915_VMA_ALLOC_BIT, __i915_vma_flags(vma)); - } - - GEM_BUG_ON(!test_bit(I915_VMA_ALLOC_BIT, - __i915_vma_flags(vma))); - alias->vm.insert_entries(&alias->vm, vma, - cache_level, pte_flags); + ret = ppgtt_bind_vma(&alias->vm, vma, cache_level, flags); + if (ret) + return ret; } if (flags & I915_VMA_GLOBAL_BIND) - vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags); + vm->insert_entries(vm, vma, cache_level, pte_flags); return 0; } -static void aliasing_gtt_unbind_vma(struct i915_vma *vma) +static void aliasing_gtt_unbind_vma(struct i915_address_space *vm, + struct i915_vma *vma) { - if (i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) { - struct i915_address_space *vm = vma->vm; - + if (i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) vm->clear_range(vm, vma->node.start, vma->size); - } - - if (test_and_clear_bit(I915_VMA_ALLOC_BIT, __i915_vma_flags(vma))) { - struct i915_address_space *vm = - &i915_vm_to_ggtt(vma->vm)->alias->vm; - vm->clear_range(vm, vma->node.start, vma->size); - } + if (i915_vma_is_bound(vma, I915_VMA_LOCAL_BIND)) + ppgtt_unbind_vma(&i915_vm_to_ggtt(vm)->alias->vm, vma); } static int init_aliasing_ppgtt(struct i915_ggtt *ggtt) -- cgit