[PATCH v4 16/17] drm/i915/vm_bind: userptr dma-resv changes
Niranjana Vishwanathapura
niranjana.vishwanathapura at intel.com
Thu Oct 20 16:39:35 UTC 2022
On Thu, Oct 20, 2022 at 05:29:45PM +0100, Matthew Auld wrote:
>On 18/10/2022 08:16, Niranjana Vishwanathapura wrote:
>>For persistent (vm_bind) vmas of userptr BOs, handle the user
>>page pinning by using the i915_gem_object_userptr_submit_init()
>>/done() functions
>>
>>v2: Do not double add vma to vm->userptr_invalidated_list
>>
>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>>Signed-off-by: Andi Shyti <andi.shyti at linux.intel.com>
>>---
>> .../gpu/drm/i915/gem/i915_gem_execbuffer3.c | 80 +++++++++++++++++++
>> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 19 +++++
>> .../drm/i915/gem/i915_gem_vm_bind_object.c | 15 ++++
>> drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +
>> drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +
>> drivers/gpu/drm/i915/i915_vma_types.h | 2 +
>> 6 files changed, 122 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
>>index 8120e4c6b7da..3f1157dd7fc2 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
>>@@ -20,6 +20,7 @@
>> #include "i915_gem_vm_bind.h"
>> #include "i915_trace.h"
>>+#define __EXEC3_USERPTR_USED BIT_ULL(34)
>> #define __EXEC3_HAS_PIN BIT_ULL(33)
>> #define __EXEC3_ENGINE_PINNED BIT_ULL(32)
>> #define __EXEC3_INTERNAL_FLAGS (~0ull << 32)
>>@@ -142,6 +143,21 @@ static void eb_scoop_unbound_vma_all(struct i915_address_space *vm)
>> {
>> struct i915_vma *vma, *vn;
>>+#ifdef CONFIG_MMU_NOTIFIER
>>+ /**
>
>Not proper kernel-doc AFAIK.
Ok, will use single asterisk above.
>
>>+ * Move all invalidated userptr vmas back into vm_bind_list so that
>>+ * they are looked up and revalidated.
>>+ */
>>+ spin_lock(&vm->userptr_invalidated_lock);
>>+ list_for_each_entry_safe(vma, vn, &vm->userptr_invalidated_list,
>>+ userptr_invalidated_link) {
>>+ list_del_init(&vma->userptr_invalidated_link);
>>+ if (!list_empty(&vma->vm_bind_link))
>>+ list_move_tail(&vma->vm_bind_link, &vm->vm_bind_list);
>>+ }
>>+ spin_unlock(&vm->userptr_invalidated_lock);
>>+#endif
>>+
>> /**
>> * Move all unbound vmas back into vm_bind_list so that they are
>> * revalidated.
>>@@ -155,10 +171,47 @@ static void eb_scoop_unbound_vma_all(struct i915_address_space *vm)
>> spin_unlock(&vm->vm_rebind_lock);
>> }
>>+static int eb_lookup_persistent_userptr_vmas(struct i915_execbuffer *eb)
>>+{
>>+ struct i915_address_space *vm = eb->context->vm;
>>+ struct i915_vma *last_vma = NULL;
>>+ struct i915_vma *vma;
>>+ int err;
>>+
>>+ lockdep_assert_held(&vm->vm_bind_lock);
>>+
>>+ list_for_each_entry(vma, &vm->vm_bind_list, vm_bind_link) {
>>+ if (!i915_gem_object_is_userptr(vma->obj))
>>+ continue;
>>+
>>+ err = i915_gem_object_userptr_submit_init(vma->obj);
>>+ if (err)
>>+ return err;
>>+
>>+ /**
>>+ * The above submit_init() call does the object unbind and
>>+ * hence adds vma into vm_rebind_list. Remove it from that
>>+ * list as it is already scooped for revalidation.
>>+ */
>
>Ditto.
ok
>
>>+ spin_lock(&vm->vm_rebind_lock);
>>+ if (!list_empty(&vma->vm_rebind_link))
>>+ list_del_init(&vma->vm_rebind_link);
>>+ spin_unlock(&vm->vm_rebind_lock);
>>+
>>+ last_vma = vma;
>>+ }
>>+
>>+ if (last_vma)
>>+ eb->args->flags |= __EXEC3_USERPTR_USED;
>>+
>>+ return 0;
>>+}
>>+
>> static int eb_lookup_vma_all(struct i915_execbuffer *eb)
>> {
>> unsigned int i, current_batch = 0;
>> struct i915_vma *vma;
>>+ int err = 0;
>> for (i = 0; i < eb->num_batches; i++) {
>> vma = eb_find_vma(eb->context->vm, eb->batch_addresses[i]);
>>@@ -171,6 +224,10 @@ static int eb_lookup_vma_all(struct i915_execbuffer *eb)
>> eb_scoop_unbound_vma_all(eb->context->vm);
>>+ err = eb_lookup_persistent_userptr_vmas(eb);
>>+ if (err)
>>+ return err;
>>+
>> return 0;
>> }
>>@@ -343,6 +400,29 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>> }
>> }
>>+#ifdef CONFIG_MMU_NOTIFIER
>>+ /* Check for further userptr invalidations */
>>+ spin_lock(&vm->userptr_invalidated_lock);
>>+ if (!list_empty(&vm->userptr_invalidated_list))
>>+ err = -EAGAIN;
>>+ spin_unlock(&vm->userptr_invalidated_lock);
>
>After dropping the lock here, the invalidated_list might no longer be
>empty? Is that not possible, or somehow not a concern?
>
It should be fine as we have already added the fence to dma-resv object above.
Any subsequent mmu invalidations will end up waiting for request to finish
(similar to case where mmu invalidation gets called after request is submitted).
>>+
>>+ if (!err && (eb->args->flags & __EXEC3_USERPTR_USED)) {
>>+ read_lock(&eb->i915->mm.notifier_lock);
>>+ list_for_each_entry(vma, &vm->vm_bind_list, vm_bind_link) {
>>+ if (!i915_gem_object_is_userptr(vma->obj))
>>+ continue;
>>+
>>+ err = i915_gem_object_userptr_submit_done(vma->obj);
>>+ if (err)
>>+ break;
>>+ }
>>+ read_unlock(&eb->i915->mm.notifier_lock);
>>+ }
>>+#endif
>>+ if (unlikely(err))
>>+ goto err_skip;
>>+
>> /* Unconditionally flush any chipset caches (for streaming writes). */
>> intel_gt_chipset_flush(eb->gt);
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>index b7e24476a0fd..3e88fd4d46b6 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>@@ -63,6 +63,7 @@ static bool i915_gem_userptr_invalidate(struct mmu_interval_notifier *mni,
>> {
>> struct drm_i915_gem_object *obj = container_of(mni, struct drm_i915_gem_object, userptr.notifier);
>> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>+ struct i915_vma *vma;
>> long r;
>> if (!mmu_notifier_range_blockable(range))
>>@@ -85,6 +86,24 @@ static bool i915_gem_userptr_invalidate(struct mmu_interval_notifier *mni,
>> if (current->flags & PF_EXITING)
>> return true;
>>+ /**
>>+ * Add persistent vmas into userptr_invalidated list for relookup
>>+ * and revalidation.
>>+ */
>
>Ditto.
Ok
Thanks,
Niranjana
>
>>+ spin_lock(&obj->vma.lock);
>>+ list_for_each_entry(vma, &obj->vma.list, obj_link) {
>>+ if (!i915_vma_is_persistent(vma))
>>+ continue;
>>+
>>+ spin_lock(&vma->vm->userptr_invalidated_lock);
>>+ if (list_empty(&vma->userptr_invalidated_link) &&
>>+ !i915_vma_is_purged(vma))
>>+ list_add_tail(&vma->userptr_invalidated_link,
>>+ &vma->vm->userptr_invalidated_list);
>>+ spin_unlock(&vma->vm->userptr_invalidated_lock);
>>+ }
>>+ spin_unlock(&obj->vma.lock);
>>+
>> /* we will unbind on next submission, still have userptr pins */
>> r = dma_resv_wait_timeout(obj->base.resv, DMA_RESV_USAGE_BOOKKEEP, false,
>> MAX_SCHEDULE_TIMEOUT);
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>index 63889ba00183..19071493355c 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>@@ -299,6 +299,12 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> goto put_obj;
>> }
>>+ if (i915_gem_object_is_userptr(obj)) {
>>+ ret = i915_gem_object_userptr_submit_init(obj);
>>+ if (ret)
>>+ goto put_obj;
>>+ }
>>+
>> ret = mutex_lock_interruptible(&vm->vm_bind_lock);
>> if (ret)
>> goto put_obj;
>>@@ -327,6 +333,15 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> if (ret)
>> continue;
>>+#ifdef CONFIG_MMU_NOTIFIER
>>+ if (i915_gem_object_is_userptr(obj)) {
>>+ read_lock(&vm->i915->mm.notifier_lock);
>>+ ret = i915_gem_object_userptr_submit_done(obj);
>>+ read_unlock(&vm->i915->mm.notifier_lock);
>>+ if (ret)
>>+ continue;
>>+ }
>>+#endif
>> list_add_tail(&vma->vm_bind_link, &vm->vm_bound_list);
>> i915_vm_bind_it_insert(vma, &vm->va);
>> if (!obj->priv_root)
>>diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>index ebf8fc3a4603..50648ab9214a 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>@@ -292,6 +292,8 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
>> INIT_LIST_HEAD(&vm->non_priv_vm_bind_list);
>> INIT_LIST_HEAD(&vm->vm_rebind_list);
>> spin_lock_init(&vm->vm_rebind_lock);
>>+ spin_lock_init(&vm->userptr_invalidated_lock);
>>+ INIT_LIST_HEAD(&vm->userptr_invalidated_list);
>> }
>> void *__px_vaddr(struct drm_i915_gem_object *p)
>>diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>index 384d1ee7c68d..1ade95b2a0fa 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>>+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>@@ -270,6 +270,10 @@ struct i915_address_space {
>> struct list_head vm_rebind_list;
>> /* @vm_rebind_lock: protects vm_rebound_list */
>> spinlock_t vm_rebind_lock;
>>+ /* @userptr_invalidated_list: list of invalidated userptr vmas */
>>+ struct list_head userptr_invalidated_list;
>>+ /* @userptr_invalidated_lock: protects userptr_invalidated_list */
>>+ spinlock_t userptr_invalidated_lock;
>> /* @va: tree of persistent vmas */
>> struct rb_root_cached va;
>> struct list_head non_priv_vm_bind_list;
>>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
>>index 2c740500ac1b..8e562e07d4a7 100644
>>--- a/drivers/gpu/drm/i915/i915_vma_types.h
>>+++ b/drivers/gpu/drm/i915/i915_vma_types.h
>>@@ -307,6 +307,8 @@ struct i915_vma {
>> struct list_head non_priv_vm_bind_link;
>> /* @vm_rebind_link: link to vm_rebind_list and protected by vm_rebind_lock */
>> struct list_head vm_rebind_link; /* Link in vm_rebind_list */
>>+ /*@userptr_invalidated_link: link to the vm->userptr_invalidated_list */
>>+ struct list_head userptr_invalidated_link;
>> /** Timeline fence for vm_bind completion notification */
>> struct {
More information about the dri-devel
mailing list