[PATCH v2 0/3] vm- and vma cleanups

[PATCH v2 0/3] vm- and vma cleanups

[Thomas Hellström]

The first patch of the series addresses a vm open count bug by
removing the vm open count.

The second patch removes the vma refcount that is no longer needed;
the vma is kept a live by taking the vm refcount and object lock.

Finally the last patch removes some unnecessary code. There should be
no functional changes.

Thomas Hellström (3):
  drm/i915: Remove the vm open count
  drm/i915: Remove the vma refcount
  drm/i915/gem: Remove some unnecessary code

 drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
 drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
 drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
 drivers/gpu/drm/i915/i915_gem.c               | 55 ++++++------
 drivers/gpu/drm/i915/i915_vma.c               | 69 +++++++++------
 drivers/gpu/drm/i915/i915_vma.h               | 14 ---
 drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
 drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
 drivers/gpu/drm/i915/i915_vma_types.h         |  8 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 16 files changed, 216 insertions(+), 212 deletions(-)

-- 
2.34.1

[Intel-gfx] [PATCH v2 0/3] vm- and vma cleanups

[Thomas Hellström]

The first patch of the series addresses a vm open count bug by
removing the vm open count.

The second patch removes the vma refcount that is no longer needed;
the vma is kept a live by taking the vm refcount and object lock.

Finally the last patch removes some unnecessary code. There should be
no functional changes.

Thomas Hellström (3):
  drm/i915: Remove the vm open count
  drm/i915: Remove the vma refcount
  drm/i915/gem: Remove some unnecessary code

 drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
 drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
 drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
 drivers/gpu/drm/i915/i915_gem.c               | 55 ++++++------
 drivers/gpu/drm/i915/i915_vma.c               | 69 +++++++++------
 drivers/gpu/drm/i915/i915_vma.h               | 14 ---
 drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
 drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
 drivers/gpu/drm/i915/i915_vma_types.h         |  8 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 16 files changed, 216 insertions(+), 212 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] drm/i915: Remove the vm open count

[Thomas Hellström]

vms are not getting properly closed. Rather than fixing that,
Remove the vm open count and instead rely on the vm refcount.

The vm open count existed solely to break the strong references the
vmas had on the vms. Now instead make those references weak and
ensure vmas are destroyed when the vm is destroyed.

Unfortunately if the vm destructor and the object destructor both
wants to destroy a vma, that may lead to a race in that the vm
destructor just unbinds the vma and leaves the actual vma destruction
to the object destructor. However in order for the object destructor
to ensure the vma is unbound it needs to grab the vm mutex. In order
to keep the vm mutex alive until the object destructor is done with
it, somewhat hackishly grab a vm_resv refcount that is released late
in the vma destruction process, when the vm mutex is no longer needed.

v2: Address review-comments from Niranjana
- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and
  should ideally be replaced in an upcoming patch.
- Remove an unneeded continue in clear_vm_list and update comment.

Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
 drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
 drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
 drivers/gpu/drm/i915/i915_gem.c               |  6 +-
 drivers/gpu/drm/i915/i915_vma.c               | 55 ++++++++----
 drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
 drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
 drivers/gpu/drm/i915/i915_vma_types.h         |  7 ++
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 15 files changed, 186 insertions(+), 164 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
index 05dd7dba3a5c..3af4930c1095 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -300,5 +300,5 @@ void intel_dpt_destroy(struct i915_address_space *vm)
 {
 	struct i915_dpt *dpt = i915_vm_to_dpt(vm);
 
-	i915_vm_close(&dpt->vm);
+	i915_vm_put(&dpt->vm);
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index bc6d59df064d..fe872e02b395 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1456,8 +1456,6 @@ static void set_closed_name(struct i915_gem_context *ctx)
 
 static void context_close(struct i915_gem_context *ctx)
 {
-	struct i915_address_space *vm;
-
 	/* Flush any concurrent set_engines() */
 	mutex_lock(&ctx->engines_mutex);
 	unpin_engines(__context_engines_static(ctx));
@@ -1469,19 +1467,6 @@ static void context_close(struct i915_gem_context *ctx)
 
 	set_closed_name(ctx);
 
-	vm = ctx->vm;
-	if (vm) {
-		/* i915_vm_close drops the final reference, which is a bit too
-		 * early and could result in surprises with concurrent
-		 * operations racing with thist ctx close. Keep a full reference
-		 * until the end.
-		 */
-		i915_vm_get(vm);
-		i915_vm_close(vm);
-	}
-
-	ctx->file_priv = ERR_PTR(-EBADF);
-
 	/*
 	 * The LUT uses the VMA as a backpointer to unref the object,
 	 * so we need to clear the LUT before we close all the VMA (inside
@@ -1489,6 +1474,8 @@ static void context_close(struct i915_gem_context *ctx)
 	 */
 	lut_close(ctx);
 
+	ctx->file_priv = ERR_PTR(-EBADF);
+
 	spin_lock(&ctx->i915->gem.contexts.lock);
 	list_del(&ctx->link);
 	spin_unlock(&ctx->i915->gem.contexts.lock);
@@ -1587,12 +1574,8 @@ i915_gem_create_context(struct drm_i915_private *i915,
 		}
 		vm = &ppgtt->vm;
 	}
-	if (vm) {
-		ctx->vm = i915_vm_open(vm);
-
-		/* i915_vm_open() takes a reference */
-		i915_vm_put(vm);
-	}
+	if (vm)
+		ctx->vm = vm;
 
 	mutex_init(&ctx->engines_mutex);
 	if (pc->num_user_engines >= 0) {
@@ -1642,7 +1625,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 	free_engines(e);
 err_vm:
 	if (ctx->vm)
-		i915_vm_close(ctx->vm);
+		i915_vm_put(ctx->vm);
 err_ctx:
 	kfree(ctx);
 	return ERR_PTR(err);
@@ -1826,7 +1809,7 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	if (err)
 		return err;
 
-	i915_vm_open(vm);
+	i915_vm_get(vm);
 
 	GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
 	args->value = id;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 89aa0557ade1..2b87ec76db9a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2702,6 +2702,11 @@ eb_select_engine(struct i915_execbuffer *eb)
 	if (err)
 		goto err;
 
+	if (!i915_vm_tryget(ce->vm)) {
+		err = -ENOENT;
+		goto err;
+	}
+
 	eb->context = ce;
 	eb->gt = ce->engine->gt;
 
@@ -2725,6 +2730,7 @@ eb_put_engine(struct i915_execbuffer *eb)
 {
 	struct intel_context *child;
 
+	i915_vm_put(eb->context->vm);
 	intel_gt_pm_put(eb->gt);
 	for_each_child(eb->context, child)
 		intel_context_put(child);
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 6d6082b5f31f..8ac6726ec16b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -42,8 +42,7 @@ mock_context(struct drm_i915_private *i915,
 		if (!ppgtt)
 			goto err_free;
 
-		ctx->vm = i915_vm_open(&ppgtt->vm);
-		i915_vm_put(&ppgtt->vm);
+		ctx->vm = &ppgtt->vm;
 	}
 
 	mutex_init(&ctx->engines_mutex);
@@ -59,7 +58,7 @@ mock_context(struct drm_i915_private *i915,
 
 err_vm:
 	if (ctx->vm)
-		i915_vm_close(ctx->vm);
+		i915_vm_put(ctx->vm);
 err_free:
 	kfree(ctx);
 	return NULL;
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 871fe7bda0e0..1bb766c79dcb 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -322,7 +322,7 @@ int gen6_ppgtt_pin(struct i915_ppgtt *base, struct i915_gem_ww_ctx *ww)
 	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	int err;
 
-	GEM_BUG_ON(!atomic_read(&ppgtt->base.vm.open));
+	GEM_BUG_ON(!kref_read(&ppgtt->base.vm.ref));
 
 	/*
 	 * Workaround the limited maximum vma->pin_count and the aliasing_ppgtt
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 8850d4e0f9cc..04191fe2ee34 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -126,7 +126,7 @@ static bool needs_idle_maps(struct drm_i915_private *i915)
 void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 {
 	struct i915_vma *vma, *vn;
-	int open;
+	int save_skip_rewrite;
 
 	drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
 
@@ -135,8 +135,12 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 	mutex_lock(&vm->mutex);
 
-	/* Skip rewriting PTE on VMA unbind. */
-	open = atomic_xchg(&vm->open, 0);
+	/*
+	 * Skip rewriting PTE on VMA unbind.
+	 * FIXME: Use an argument to i915_vma_unbind() instead?
+	 */
+	save_skip_rewrite = vm->skip_pte_rewrite;
+	vm->skip_pte_rewrite = true;
 
 	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
@@ -154,16 +158,14 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 			 */
 			i915_gem_object_get(obj);
 
-			atomic_set(&vm->open, open);
 			mutex_unlock(&vm->mutex);
 
 			i915_gem_object_lock(obj, NULL);
-			open = i915_vma_unbind(vma);
+			GEM_WARN_ON(i915_vma_unbind(vma));
 			i915_gem_object_unlock(obj);
-
-			GEM_WARN_ON(open);
-
 			i915_gem_object_put(obj);
+
+			vm->skip_pte_rewrite = save_skip_rewrite;
 			goto retry;
 		}
 
@@ -179,7 +181,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 	vm->clear_range(vm, 0, vm->total);
 
-	atomic_set(&vm->open, open);
+	vm->skip_pte_rewrite = save_skip_rewrite;
 
 	mutex_unlock(&vm->mutex);
 }
@@ -773,13 +775,13 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
 {
 	struct i915_vma *vma, *vn;
 
-	atomic_set(&ggtt->vm.open, 0);
-
 	flush_workqueue(ggtt->vm.i915->wq);
 	i915_gem_drain_freed_objects(ggtt->vm.i915);
 
 	mutex_lock(&ggtt->vm.mutex);
 
+	ggtt->vm.skip_pte_rewrite = true;
+
 	list_for_each_entry_safe(vma, vn, &ggtt->vm.bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
 		bool trylock;
@@ -1307,16 +1309,12 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 {
 	struct i915_vma *vma;
 	bool write_domain_objs = false;
-	int open;
 
 	drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
 
 	/* First fill our portion of the GTT with scratch pages */
 	vm->clear_range(vm, 0, vm->total);
 
-	/* Skip rewriting PTE on VMA unbind. */
-	open = atomic_xchg(&vm->open, 0);
-
 	/* clflush objects bound into the GGTT and rebind them. */
 	list_for_each_entry(vma, &vm->bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
@@ -1333,8 +1331,6 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 		}
 	}
 
-	atomic_set(&vm->open, open);
-
 	return write_domain_objs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 4bcdfcab3642..4f70d512a000 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -97,32 +97,52 @@ int map_pt_dma_locked(struct i915_address_space *vm, struct drm_i915_gem_object
 	return 0;
 }
 
-void __i915_vm_close(struct i915_address_space *vm)
+static void clear_vm_list(struct list_head *list)
 {
 	struct i915_vma *vma, *vn;
 
-	if (!atomic_dec_and_mutex_lock(&vm->open, &vm->mutex))
-		return;
-
-	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
+	list_for_each_entry_safe(vma, vn, list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
 
-		if (!kref_get_unless_zero(&obj->base.refcount)) {
+		if (!i915_gem_object_get_rcu(obj)) {
 			/*
-			 * Unbind the dying vma to ensure the bound_list
+			 * Object is dying, but has not yet cleared its
+			 * vma list.
+			 * Unbind the dying vma to ensure our list
 			 * is completely drained. We leave the destruction to
-			 * the object destructor.
+			 * the object destructor to avoid the vma
+			 * disappearing under it.
 			 */
 			atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
 			WARN_ON(__i915_vma_unbind(vma));
-			continue;
+
+			/* Remove from the unbound list */
+			list_del_init(&vma->vm_link);
+
+			/*
+			 * Delay the vm and vm mutex freeing until the
+			 * object is done with destruction.
+			 */
+			i915_vm_resv_get(vma->vm);
+			vma->vm_ddestroy = true;
+		} else {
+			i915_vma_destroy_locked(vma);
+			i915_gem_object_put(obj);
 		}
 
-		/* Keep the obj (and hence the vma) alive as _we_ destroy it */
-		i915_vma_destroy_locked(vma);
-		i915_gem_object_put(obj);
 	}
+}
+
+static void __i915_vm_close(struct i915_address_space *vm)
+{
+	mutex_lock(&vm->mutex);
+
+	clear_vm_list(&vm->bound_list);
+	clear_vm_list(&vm->unbound_list);
+
+	/* Check for must-fix unanticipated side-effects */
 	GEM_BUG_ON(!list_empty(&vm->bound_list));
+	GEM_BUG_ON(!list_empty(&vm->unbound_list));
 
 	mutex_unlock(&vm->mutex);
 }
@@ -144,7 +164,6 @@ int i915_vm_lock_objects(struct i915_address_space *vm,
 void i915_address_space_fini(struct i915_address_space *vm)
 {
 	drm_mm_takedown(&vm->mm);
-	mutex_destroy(&vm->mutex);
 }
 
 /**
@@ -152,7 +171,8 @@ void i915_address_space_fini(struct i915_address_space *vm)
  * @kref: Pointer to the &i915_address_space.resv_ref member.
  *
  * This function is called when the last lock sharer no longer shares the
- * &i915_address_space._resv lock.
+ * &i915_address_space._resv lock, and also if we raced when
+ * destroying a vma by the vma destruction
  */
 void i915_vm_resv_release(struct kref *kref)
 {
@@ -160,6 +180,8 @@ void i915_vm_resv_release(struct kref *kref)
 		container_of(kref, typeof(*vm), resv_ref);
 
 	dma_resv_fini(&vm->_resv);
+	mutex_destroy(&vm->mutex);
+
 	kfree(vm);
 }
 
@@ -168,6 +190,8 @@ static void __i915_vm_release(struct work_struct *work)
 	struct i915_address_space *vm =
 		container_of(work, struct i915_address_space, release_work);
 
+	__i915_vm_close(vm);
+
 	/* Synchronize async unbinds. */
 	i915_vma_resource_bind_dep_sync_all(vm);
 
@@ -201,7 +225,6 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 
 	vm->pending_unbind = RB_ROOT_CACHED;
 	INIT_WORK(&vm->release_work, __i915_vm_release);
-	atomic_set(&vm->open, 1);
 
 	/*
 	 * The vm->mutex must be reclaim safe (for use in the shrinker).
@@ -245,6 +268,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 	vm->mm.head_node.color = I915_COLOR_UNEVICTABLE;
 
 	INIT_LIST_HEAD(&vm->bound_list);
+	INIT_LIST_HEAD(&vm->unbound_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 9d83c2d3959c..4529b5e9f6e6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -240,15 +240,6 @@ struct i915_address_space {
 
 	unsigned int bind_async_flags;
 
-	/*
-	 * Each active user context has its own address space (in full-ppgtt).
-	 * Since the vm may be shared between multiple contexts, we count how
-	 * many contexts keep us "open". Once open hits zero, we are closed
-	 * and do not allow any new attachments, and proceed to shutdown our
-	 * vma and page directories.
-	 */
-	atomic_t open;
-
 	struct mutex mutex; /* protects vma and our lists */
 
 	struct kref resv_ref; /* kref to keep the reservation lock alive. */
@@ -263,6 +254,11 @@ struct i915_address_space {
 	 */
 	struct list_head bound_list;
 
+	/**
+	 * List of vmas not yet bound or evicted.
+	 */
+	struct list_head unbound_list;
+
 	/* Global GTT */
 	bool is_ggtt:1;
 
@@ -272,6 +268,9 @@ struct i915_address_space {
 	/* Some systems support read-only mappings for GGTT and/or PPGTT */
 	bool has_read_only:1;
 
+	/* Skip pte rewrite on unbind for suspend. Protected by @mutex */
+	bool skip_pte_rewrite:1;
+
 	u8 top;
 	u8 pd_shift;
 	u8 scratch_order;
@@ -446,6 +445,17 @@ i915_vm_get(struct i915_address_space *vm)
 	return vm;
 }
 
+static inline struct i915_address_space *
+i915_vm_tryget(struct i915_address_space *vm)
+{
+	return kref_get_unless_zero(&vm->ref) ? vm : NULL;
+}
+
+static inline void assert_vm_alive(struct i915_address_space *vm)
+{
+	GEM_BUG_ON(!kref_read(&vm->ref));
+}
+
 /**
  * i915_vm_resv_get - Obtain a reference on the vm's reservation lock
  * @vm: The vm whose reservation lock we want to share.
@@ -476,34 +486,6 @@ static inline void i915_vm_resv_put(struct i915_address_space *vm)
 	kref_put(&vm->resv_ref, i915_vm_resv_release);
 }
 
-static inline struct i915_address_space *
-i915_vm_open(struct i915_address_space *vm)
-{
-	GEM_BUG_ON(!atomic_read(&vm->open));
-	atomic_inc(&vm->open);
-	return i915_vm_get(vm);
-}
-
-static inline bool
-i915_vm_tryopen(struct i915_address_space *vm)
-{
-	if (atomic_add_unless(&vm->open, 1, 0))
-		return i915_vm_get(vm);
-
-	return false;
-}
-
-void __i915_vm_close(struct i915_address_space *vm);
-
-static inline void
-i915_vm_close(struct i915_address_space *vm)
-{
-	GEM_BUG_ON(!atomic_read(&vm->open));
-	__i915_vm_close(vm);
-
-	i915_vm_put(vm);
-}
-
 void i915_address_space_init(struct i915_address_space *vm, int subclass);
 void i915_address_space_fini(struct i915_address_space *vm);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index 72d5faab8f9a..09f8cd2d0e2c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -1736,15 +1736,9 @@ static int live_preempt(void *arg)
 	enum intel_engine_id id;
 	int err = -ENOMEM;
 
-	if (igt_spinner_init(&spin_hi, gt))
-		return -ENOMEM;
-
-	if (igt_spinner_init(&spin_lo, gt))
-		goto err_spin_hi;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
@@ -1752,6 +1746,12 @@ static int live_preempt(void *arg)
 		goto err_ctx_hi;
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
+	if (igt_spinner_init(&spin_hi, gt))
+		goto err_ctx_lo;
+
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_spin_hi;
+
 	for_each_engine(engine, gt, id) {
 		struct igt_live_test t;
 		struct i915_request *rq;
@@ -1761,14 +1761,14 @@ static int live_preempt(void *arg)
 
 		if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_lo, ctx_lo, engine,
 					    MI_ARB_CHECK);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1777,7 +1777,7 @@ static int live_preempt(void *arg)
 			GEM_TRACE_DUMP();
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_hi, ctx_hi, engine,
@@ -1785,7 +1785,7 @@ static int live_preempt(void *arg)
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1794,7 +1794,7 @@ static int live_preempt(void *arg)
 			GEM_TRACE_DUMP();
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		igt_spinner_end(&spin_hi);
@@ -1802,19 +1802,19 @@ static int live_preempt(void *arg)
 
 		if (igt_live_test_end(&t)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 	}
 
 	err = 0;
-err_ctx_lo:
-	kernel_context_close(ctx_lo);
-err_ctx_hi:
-	kernel_context_close(ctx_hi);
 err_spin_lo:
 	igt_spinner_fini(&spin_lo);
 err_spin_hi:
 	igt_spinner_fini(&spin_hi);
+err_ctx_lo:
+	kernel_context_close(ctx_lo);
+err_ctx_hi:
+	kernel_context_close(ctx_hi);
 	return err;
 }
 
@@ -1828,20 +1828,20 @@ static int live_late_preempt(void *arg)
 	enum intel_engine_id id;
 	int err = -ENOMEM;
 
-	if (igt_spinner_init(&spin_hi, gt))
-		return -ENOMEM;
-
-	if (igt_spinner_init(&spin_lo, gt))
-		goto err_spin_hi;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
 	if (!ctx_lo)
 		goto err_ctx_hi;
 
+	if (igt_spinner_init(&spin_hi, gt))
+		goto err_ctx_lo;
+
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_spin_hi;
+
 	/* Make sure ctx_lo stays before ctx_hi until we trigger preemption. */
 	ctx_lo->sched.priority = 1;
 
@@ -1854,14 +1854,14 @@ static int live_late_preempt(void *arg)
 
 		if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_lo, ctx_lo, engine,
 					    MI_ARB_CHECK);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1875,7 +1875,7 @@ static int live_late_preempt(void *arg)
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1898,19 +1898,19 @@ static int live_late_preempt(void *arg)
 
 		if (igt_live_test_end(&t)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 	}
 
 	err = 0;
-err_ctx_lo:
-	kernel_context_close(ctx_lo);
-err_ctx_hi:
-	kernel_context_close(ctx_hi);
 err_spin_lo:
 	igt_spinner_fini(&spin_lo);
 err_spin_hi:
 	igt_spinner_fini(&spin_hi);
+err_ctx_lo:
+	kernel_context_close(ctx_lo);
+err_ctx_hi:
+	kernel_context_close(ctx_hi);
 	return err;
 
 err_wedged:
@@ -1918,7 +1918,7 @@ static int live_late_preempt(void *arg)
 	igt_spinner_end(&spin_lo);
 	intel_gt_set_wedged(gt);
 	err = -EIO;
-	goto err_ctx_lo;
+	goto err_spin_lo;
 }
 
 struct preempt_client {
@@ -3382,12 +3382,9 @@ static int live_preempt_timeout(void *arg)
 	if (!intel_has_reset_engine(gt))
 		return 0;
 
-	if (igt_spinner_init(&spin_lo, gt))
-		return -ENOMEM;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
@@ -3395,6 +3392,9 @@ static int live_preempt_timeout(void *arg)
 		goto err_ctx_hi;
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_ctx_lo;
+
 	for_each_engine(engine, gt, id) {
 		unsigned long saved_timeout;
 		struct i915_request *rq;
@@ -3406,21 +3406,21 @@ static int live_preempt_timeout(void *arg)
 					    MI_NOOP); /* preemption disabled */
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
 		if (!igt_wait_for_spinner(&spin_lo, rq)) {
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = igt_request_alloc(ctx_hi, engine);
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		/* Flush the previous CS ack before changing timeouts */
@@ -3440,7 +3440,7 @@ static int live_preempt_timeout(void *arg)
 			intel_gt_set_wedged(gt);
 			i915_request_put(rq);
 			err = -ETIME;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		igt_spinner_end(&spin_lo);
@@ -3448,12 +3448,12 @@ static int live_preempt_timeout(void *arg)
 	}
 
 	err = 0;
+err_spin_lo:
+	igt_spinner_fini(&spin_lo);
 err_ctx_lo:
 	kernel_context_close(ctx_lo);
 err_ctx_hi:
 	kernel_context_close(ctx_hi);
-err_spin_lo:
-	igt_spinner_fini(&spin_lo);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e10187cd0a0..dd84ebabb50f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -142,8 +142,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 	while (!ret && (vma = list_first_entry_or_null(&obj->vma.list,
 						       struct i915_vma,
 						       obj_link))) {
-		struct i915_address_space *vm = vma->vm;
-
 		list_move_tail(&vma->obj_link, &still_in_list);
 		if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK))
 			continue;
@@ -154,7 +152,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		}
 
 		ret = -EAGAIN;
-		if (!i915_vm_tryopen(vm))
+		if (!i915_vm_tryget(vma->vm))
 			break;
 
 		/* Prevent vma being freed by i915_vma_parked as we unbind */
@@ -186,7 +184,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			__i915_vma_put(vma);
 		}
 
-		i915_vm_close(vm);
+		i915_vm_put(vma->vm);
 		spin_lock(&obj->vma.lock);
 	}
 	list_splice_init(&still_in_list, &obj->vma.list);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 94fcdb7bd21d..91538bc38110 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -47,7 +47,7 @@ static inline void assert_vma_held_evict(const struct i915_vma *vma)
 	 * This is the only exception to the requirement of the object lock
 	 * being held.
 	 */
-	if (atomic_read(&vma->vm->open))
+	if (kref_read(&vma->vm->ref))
 		assert_object_held_shared(vma->obj);
 }
 
@@ -113,6 +113,7 @@ vma_create(struct drm_i915_gem_object *obj,
 	struct i915_vma *pos = ERR_PTR(-E2BIG);
 	struct i915_vma *vma;
 	struct rb_node *rb, **p;
+	int err;
 
 	/* The aliasing_ppgtt should never be used directly! */
 	GEM_BUG_ON(vm == &vm->gt->ggtt->alias->vm);
@@ -122,7 +123,6 @@ vma_create(struct drm_i915_gem_object *obj,
 		return ERR_PTR(-ENOMEM);
 
 	kref_init(&vma->ref);
-	vma->vm = i915_vm_get(vm);
 	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
 	vma->size = obj->base.size;
@@ -138,6 +138,8 @@ vma_create(struct drm_i915_gem_object *obj,
 	}
 
 	INIT_LIST_HEAD(&vma->closed_link);
+	INIT_LIST_HEAD(&vma->obj_link);
+	RB_CLEAR_NODE(&vma->obj_node);
 
 	if (view && view->type != I915_GGTT_VIEW_NORMAL) {
 		vma->ggtt_view = *view;
@@ -163,8 +165,16 @@ vma_create(struct drm_i915_gem_object *obj,
 
 	GEM_BUG_ON(!IS_ALIGNED(vma->size, I915_GTT_PAGE_SIZE));
 
-	spin_lock(&obj->vma.lock);
+	err = mutex_lock_interruptible(&vm->mutex);
+	if (err) {
+		pos = ERR_PTR(err);
+		goto err_vma;
+	}
 
+	vma->vm = vm;
+	list_add_tail(&vma->vm_link, &vm->unbound_list);
+
+	spin_lock(&obj->vma.lock);
 	if (i915_is_ggtt(vm)) {
 		if (unlikely(overflows_type(vma->size, u32)))
 			goto err_unlock;
@@ -222,13 +232,15 @@ vma_create(struct drm_i915_gem_object *obj,
 		list_add_tail(&vma->obj_link, &obj->vma.list);
 
 	spin_unlock(&obj->vma.lock);
+	mutex_unlock(&vm->mutex);
 
 	return vma;
 
 err_unlock:
 	spin_unlock(&obj->vma.lock);
+	list_del_init(&vma->vm_link);
+	mutex_unlock(&vm->mutex);
 err_vma:
-	i915_vm_put(vm);
 	i915_vma_free(vma);
 	return pos;
 }
@@ -279,7 +291,7 @@ i915_vma_instance(struct drm_i915_gem_object *obj,
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm));
-	GEM_BUG_ON(!atomic_read(&vm->open));
+	GEM_BUG_ON(!kref_read(&vm->ref));
 
 	spin_lock(&obj->vma.lock);
 	vma = i915_vma_lookup(obj, vm, view);
@@ -322,7 +334,6 @@ static void __vma_release(struct dma_fence_work *work)
 		i915_gem_object_put(vw->pinned);
 
 	i915_vm_free_pt_stash(vw->vm, &vw->stash);
-	i915_vm_put(vw->vm);
 	if (vw->vma_res)
 		i915_vma_resource_put(vw->vma_res);
 }
@@ -841,7 +852,7 @@ i915_vma_insert(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, color));
 
-	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
 
 	return 0;
 }
@@ -857,7 +868,7 @@ i915_vma_detach(struct i915_vma *vma)
 	 * vma, we can drop its hold on the backing storage and allow
 	 * it to be reaped by the shrinker.
 	 */
-	list_del(&vma->vm_link);
+	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
 }
 
 static bool try_qad_pin(struct i915_vma *vma, unsigned int flags)
@@ -1373,8 +1384,7 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
 			goto err_rpm;
 		}
 
-		work->vm = i915_vm_get(vma->vm);
-
+		work->vm = vma->vm;
 		dma_fence_work_chain(&work->base, moving);
 
 		/* Allocate enough page directories to used PTE */
@@ -1622,7 +1632,6 @@ void i915_vma_release(struct kref *ref)
 {
 	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
 
-	i915_vm_put(vma->vm);
 	i915_active_fini(&vma->active);
 	GEM_WARN_ON(vma->resource);
 	i915_vma_free(vma);
@@ -1638,7 +1647,7 @@ static void force_unbind(struct i915_vma *vma)
 	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 }
 
-static void release_references(struct i915_vma *vma)
+static void release_references(struct i915_vma *vma, bool vm_ddestroy)
 {
 	struct drm_i915_gem_object *obj = vma->obj;
 
@@ -1648,10 +1657,14 @@ static void release_references(struct i915_vma *vma)
 	list_del(&vma->obj_link);
 	if (!RB_EMPTY_NODE(&vma->obj_node))
 		rb_erase(&vma->obj_node, &obj->vma.tree);
+
 	spin_unlock(&obj->vma.lock);
 
 	__i915_vma_remove_closed(vma);
 
+	if (vm_ddestroy)
+		i915_vm_resv_put(vma->vm);
+
 	__i915_vma_put(vma);
 }
 
@@ -1685,15 +1698,21 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
 	lockdep_assert_held(&vma->vm->mutex);
 
 	force_unbind(vma);
-	release_references(vma);
+	list_del_init(&vma->vm_link);
+	release_references(vma, false);
 }
 
 void i915_vma_destroy(struct i915_vma *vma)
 {
+	bool vm_ddestroy;
+
 	mutex_lock(&vma->vm->mutex);
 	force_unbind(vma);
+	list_del_init(&vma->vm_link);
+	vm_ddestroy = vma->vm_ddestroy;
+	vma->vm_ddestroy = false;
 	mutex_unlock(&vma->vm->mutex);
-	release_references(vma);
+	release_references(vma, vm_ddestroy);
 }
 
 void i915_vma_parked(struct intel_gt *gt)
@@ -1711,7 +1730,7 @@ void i915_vma_parked(struct intel_gt *gt)
 		if (!kref_get_unless_zero(&obj->base.refcount))
 			continue;
 
-		if (!i915_vm_tryopen(vm)) {
+		if (!i915_vm_tryget(vm)) {
 			i915_gem_object_put(obj);
 			continue;
 		}
@@ -1737,7 +1756,7 @@ void i915_vma_parked(struct intel_gt *gt)
 		}
 
 		i915_gem_object_put(obj);
-		i915_vm_close(vm);
+		i915_vm_put(vm);
 	}
 }
 
@@ -1888,7 +1907,9 @@ struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async)
 
 	/* If vm is not open, unbind is a nop. */
 	vma_res->needs_wakeref = i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND) &&
-		atomic_read(&vma->vm->open);
+		kref_read(&vma->vm->ref);
+	vma_res->skip_pte_rewrite = !kref_read(&vma->vm->ref) ||
+		vma->vm->skip_pte_rewrite;
 	trace_i915_vma_unbind(vma);
 
 	unbind_fence = i915_vma_resource_unbind(vma_res);
diff --git a/drivers/gpu/drm/i915/i915_vma_resource.c b/drivers/gpu/drm/i915/i915_vma_resource.c
index 57ae92ba8af1..27c55027387a 100644
--- a/drivers/gpu/drm/i915/i915_vma_resource.c
+++ b/drivers/gpu/drm/i915/i915_vma_resource.c
@@ -178,7 +178,7 @@ static void i915_vma_resource_unbind_work(struct work_struct *work)
 	bool lockdep_cookie;
 
 	lockdep_cookie = dma_fence_begin_signalling();
-	if (likely(atomic_read(&vm->open)))
+	if (likely(!vma_res->skip_pte_rewrite))
 		vma_res->ops->unbind_vma(vm, vma_res);
 
 	dma_fence_end_signalling(lockdep_cookie);
diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h
index 25913913baa6..5d8427caa2ba 100644
--- a/drivers/gpu/drm/i915/i915_vma_resource.h
+++ b/drivers/gpu/drm/i915/i915_vma_resource.h
@@ -62,6 +62,11 @@ struct i915_page_sizes {
  * deferred to a work item awaiting unsignaled fences. This is a hack.
  * (dma_fence_work uses a fence flag for this, but this seems slightly
  * cleaner).
+ * @needs_wakeref: Whether a wakeref is needed during unbind. Since we can't
+ * take a wakeref in the dma-fence signalling critical path, it needs to be
+ * taken when the unbind is scheduled.
+ * @skip_pte_rewrite: During ggtt suspend and vm takedown pte rewriting
+ * needs to be skipped for unbind.
  *
  * The lifetime of a struct i915_vma_resource is from a binding request to
  * the actual possible asynchronous unbind has completed.
@@ -113,6 +118,7 @@ struct i915_vma_resource {
 	bool allocated:1;
 	bool immediate_unbind:1;
 	bool needs_wakeref:1;
+	bool skip_pte_rewrite:1;
 };
 
 bool i915_vma_resource_hold(struct i915_vma_resource *vma_res,
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index 88370dadca82..eac36be184e5 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -271,6 +271,13 @@ struct i915_vma {
 #define I915_VMA_PAGES_ACTIVE (BIT(24) | 1)
 	atomic_t pages_count; /* number of active binds to the pages */
 
+	/**
+	 * Whether we hold a reference on the vm dma_resv lock to temporarily
+	 * block vm freeing until the vma is destroyed.
+	 * Protected by the vm mutex.
+	 */
+	bool vm_ddestroy;
+
 	/**
 	 * Support different GGTT views into the same object.
 	 * This means there can be multiple VMA mappings per object and per VM.
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index ab751192eb3b..5c9bfa409ff5 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1205,7 +1205,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
 		goto out_free;
 	}
 	GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
-	GEM_BUG_ON(!atomic_read(&ppgtt->vm.open));
+	assert_vm_alive(&ppgtt->vm);
 
 	err = func(&ppgtt->vm, 0, ppgtt->vm.total, end_time);
 
@@ -1438,7 +1438,7 @@ static void track_vma_bind(struct i915_vma *vma)
 	vma->resource->bi.pages = vma->pages;
 
 	mutex_lock(&vma->vm->mutex);
-	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
 	mutex_unlock(&vma->vm->mutex);
 }
 
-- 
2.34.1

[Intel-gfx] [PATCH v2 1/3] drm/i915: Remove the vm open count

[Thomas Hellström]

vms are not getting properly closed. Rather than fixing that,
Remove the vm open count and instead rely on the vm refcount.

The vm open count existed solely to break the strong references the
vmas had on the vms. Now instead make those references weak and
ensure vmas are destroyed when the vm is destroyed.

Unfortunately if the vm destructor and the object destructor both
wants to destroy a vma, that may lead to a race in that the vm
destructor just unbinds the vma and leaves the actual vma destruction
to the object destructor. However in order for the object destructor
to ensure the vma is unbound it needs to grab the vm mutex. In order
to keep the vm mutex alive until the object destructor is done with
it, somewhat hackishly grab a vm_resv refcount that is released late
in the vma destruction process, when the vm mutex is no longer needed.

v2: Address review-comments from Niranjana
- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and
  should ideally be replaced in an upcoming patch.
- Remove an unneeded continue in clear_vm_list and update comment.

Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
 drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
 drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
 drivers/gpu/drm/i915/i915_gem.c               |  6 +-
 drivers/gpu/drm/i915/i915_vma.c               | 55 ++++++++----
 drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
 drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
 drivers/gpu/drm/i915/i915_vma_types.h         |  7 ++
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 15 files changed, 186 insertions(+), 164 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
index 05dd7dba3a5c..3af4930c1095 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -300,5 +300,5 @@ void intel_dpt_destroy(struct i915_address_space *vm)
 {
 	struct i915_dpt *dpt = i915_vm_to_dpt(vm);
 
-	i915_vm_close(&dpt->vm);
+	i915_vm_put(&dpt->vm);
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index bc6d59df064d..fe872e02b395 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1456,8 +1456,6 @@ static void set_closed_name(struct i915_gem_context *ctx)
 
 static void context_close(struct i915_gem_context *ctx)
 {
-	struct i915_address_space *vm;
-
 	/* Flush any concurrent set_engines() */
 	mutex_lock(&ctx->engines_mutex);
 	unpin_engines(__context_engines_static(ctx));
@@ -1469,19 +1467,6 @@ static void context_close(struct i915_gem_context *ctx)
 
 	set_closed_name(ctx);
 
-	vm = ctx->vm;
-	if (vm) {
-		/* i915_vm_close drops the final reference, which is a bit too
-		 * early and could result in surprises with concurrent
-		 * operations racing with thist ctx close. Keep a full reference
-		 * until the end.
-		 */
-		i915_vm_get(vm);
-		i915_vm_close(vm);
-	}
-
-	ctx->file_priv = ERR_PTR(-EBADF);
-
 	/*
 	 * The LUT uses the VMA as a backpointer to unref the object,
 	 * so we need to clear the LUT before we close all the VMA (inside
@@ -1489,6 +1474,8 @@ static void context_close(struct i915_gem_context *ctx)
 	 */
 	lut_close(ctx);
 
+	ctx->file_priv = ERR_PTR(-EBADF);
+
 	spin_lock(&ctx->i915->gem.contexts.lock);
 	list_del(&ctx->link);
 	spin_unlock(&ctx->i915->gem.contexts.lock);
@@ -1587,12 +1574,8 @@ i915_gem_create_context(struct drm_i915_private *i915,
 		}
 		vm = &ppgtt->vm;
 	}
-	if (vm) {
-		ctx->vm = i915_vm_open(vm);
-
-		/* i915_vm_open() takes a reference */
-		i915_vm_put(vm);
-	}
+	if (vm)
+		ctx->vm = vm;
 
 	mutex_init(&ctx->engines_mutex);
 	if (pc->num_user_engines >= 0) {
@@ -1642,7 +1625,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 	free_engines(e);
 err_vm:
 	if (ctx->vm)
-		i915_vm_close(ctx->vm);
+		i915_vm_put(ctx->vm);
 err_ctx:
 	kfree(ctx);
 	return ERR_PTR(err);
@@ -1826,7 +1809,7 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	if (err)
 		return err;
 
-	i915_vm_open(vm);
+	i915_vm_get(vm);
 
 	GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
 	args->value = id;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 89aa0557ade1..2b87ec76db9a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2702,6 +2702,11 @@ eb_select_engine(struct i915_execbuffer *eb)
 	if (err)
 		goto err;
 
+	if (!i915_vm_tryget(ce->vm)) {
+		err = -ENOENT;
+		goto err;
+	}
+
 	eb->context = ce;
 	eb->gt = ce->engine->gt;
 
@@ -2725,6 +2730,7 @@ eb_put_engine(struct i915_execbuffer *eb)
 {
 	struct intel_context *child;
 
+	i915_vm_put(eb->context->vm);
 	intel_gt_pm_put(eb->gt);
 	for_each_child(eb->context, child)
 		intel_context_put(child);
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 6d6082b5f31f..8ac6726ec16b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -42,8 +42,7 @@ mock_context(struct drm_i915_private *i915,
 		if (!ppgtt)
 			goto err_free;
 
-		ctx->vm = i915_vm_open(&ppgtt->vm);
-		i915_vm_put(&ppgtt->vm);
+		ctx->vm = &ppgtt->vm;
 	}
 
 	mutex_init(&ctx->engines_mutex);
@@ -59,7 +58,7 @@ mock_context(struct drm_i915_private *i915,
 
 err_vm:
 	if (ctx->vm)
-		i915_vm_close(ctx->vm);
+		i915_vm_put(ctx->vm);
 err_free:
 	kfree(ctx);
 	return NULL;
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 871fe7bda0e0..1bb766c79dcb 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -322,7 +322,7 @@ int gen6_ppgtt_pin(struct i915_ppgtt *base, struct i915_gem_ww_ctx *ww)
 	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(base);
 	int err;
 
-	GEM_BUG_ON(!atomic_read(&ppgtt->base.vm.open));
+	GEM_BUG_ON(!kref_read(&ppgtt->base.vm.ref));
 
 	/*
 	 * Workaround the limited maximum vma->pin_count and the aliasing_ppgtt
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 8850d4e0f9cc..04191fe2ee34 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -126,7 +126,7 @@ static bool needs_idle_maps(struct drm_i915_private *i915)
 void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 {
 	struct i915_vma *vma, *vn;
-	int open;
+	int save_skip_rewrite;
 
 	drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
 
@@ -135,8 +135,12 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 	mutex_lock(&vm->mutex);
 
-	/* Skip rewriting PTE on VMA unbind. */
-	open = atomic_xchg(&vm->open, 0);
+	/*
+	 * Skip rewriting PTE on VMA unbind.
+	 * FIXME: Use an argument to i915_vma_unbind() instead?
+	 */
+	save_skip_rewrite = vm->skip_pte_rewrite;
+	vm->skip_pte_rewrite = true;
 
 	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
@@ -154,16 +158,14 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 			 */
 			i915_gem_object_get(obj);
 
-			atomic_set(&vm->open, open);
 			mutex_unlock(&vm->mutex);
 
 			i915_gem_object_lock(obj, NULL);
-			open = i915_vma_unbind(vma);
+			GEM_WARN_ON(i915_vma_unbind(vma));
 			i915_gem_object_unlock(obj);
-
-			GEM_WARN_ON(open);
-
 			i915_gem_object_put(obj);
+
+			vm->skip_pte_rewrite = save_skip_rewrite;
 			goto retry;
 		}
 
@@ -179,7 +181,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 	vm->clear_range(vm, 0, vm->total);
 
-	atomic_set(&vm->open, open);
+	vm->skip_pte_rewrite = save_skip_rewrite;
 
 	mutex_unlock(&vm->mutex);
 }
@@ -773,13 +775,13 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
 {
 	struct i915_vma *vma, *vn;
 
-	atomic_set(&ggtt->vm.open, 0);
-
 	flush_workqueue(ggtt->vm.i915->wq);
 	i915_gem_drain_freed_objects(ggtt->vm.i915);
 
 	mutex_lock(&ggtt->vm.mutex);
 
+	ggtt->vm.skip_pte_rewrite = true;
+
 	list_for_each_entry_safe(vma, vn, &ggtt->vm.bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
 		bool trylock;
@@ -1307,16 +1309,12 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 {
 	struct i915_vma *vma;
 	bool write_domain_objs = false;
-	int open;
 
 	drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
 
 	/* First fill our portion of the GTT with scratch pages */
 	vm->clear_range(vm, 0, vm->total);
 
-	/* Skip rewriting PTE on VMA unbind. */
-	open = atomic_xchg(&vm->open, 0);
-
 	/* clflush objects bound into the GGTT and rebind them. */
 	list_for_each_entry(vma, &vm->bound_list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
@@ -1333,8 +1331,6 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 		}
 	}
 
-	atomic_set(&vm->open, open);
-
 	return write_domain_objs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 4bcdfcab3642..4f70d512a000 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -97,32 +97,52 @@ int map_pt_dma_locked(struct i915_address_space *vm, struct drm_i915_gem_object
 	return 0;
 }
 
-void __i915_vm_close(struct i915_address_space *vm)
+static void clear_vm_list(struct list_head *list)
 {
 	struct i915_vma *vma, *vn;
 
-	if (!atomic_dec_and_mutex_lock(&vm->open, &vm->mutex))
-		return;
-
-	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
+	list_for_each_entry_safe(vma, vn, list, vm_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
 
-		if (!kref_get_unless_zero(&obj->base.refcount)) {
+		if (!i915_gem_object_get_rcu(obj)) {
 			/*
-			 * Unbind the dying vma to ensure the bound_list
+			 * Object is dying, but has not yet cleared its
+			 * vma list.
+			 * Unbind the dying vma to ensure our list
 			 * is completely drained. We leave the destruction to
-			 * the object destructor.
+			 * the object destructor to avoid the vma
+			 * disappearing under it.
 			 */
 			atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
 			WARN_ON(__i915_vma_unbind(vma));
-			continue;
+
+			/* Remove from the unbound list */
+			list_del_init(&vma->vm_link);
+
+			/*
+			 * Delay the vm and vm mutex freeing until the
+			 * object is done with destruction.
+			 */
+			i915_vm_resv_get(vma->vm);
+			vma->vm_ddestroy = true;
+		} else {
+			i915_vma_destroy_locked(vma);
+			i915_gem_object_put(obj);
 		}
 
-		/* Keep the obj (and hence the vma) alive as _we_ destroy it */
-		i915_vma_destroy_locked(vma);
-		i915_gem_object_put(obj);
 	}
+}
+
+static void __i915_vm_close(struct i915_address_space *vm)
+{
+	mutex_lock(&vm->mutex);
+
+	clear_vm_list(&vm->bound_list);
+	clear_vm_list(&vm->unbound_list);
+
+	/* Check for must-fix unanticipated side-effects */
 	GEM_BUG_ON(!list_empty(&vm->bound_list));
+	GEM_BUG_ON(!list_empty(&vm->unbound_list));
 
 	mutex_unlock(&vm->mutex);
 }
@@ -144,7 +164,6 @@ int i915_vm_lock_objects(struct i915_address_space *vm,
 void i915_address_space_fini(struct i915_address_space *vm)
 {
 	drm_mm_takedown(&vm->mm);
-	mutex_destroy(&vm->mutex);
 }
 
 /**
@@ -152,7 +171,8 @@ void i915_address_space_fini(struct i915_address_space *vm)
  * @kref: Pointer to the &i915_address_space.resv_ref member.
  *
  * This function is called when the last lock sharer no longer shares the
- * &i915_address_space._resv lock.
+ * &i915_address_space._resv lock, and also if we raced when
+ * destroying a vma by the vma destruction
  */
 void i915_vm_resv_release(struct kref *kref)
 {
@@ -160,6 +180,8 @@ void i915_vm_resv_release(struct kref *kref)
 		container_of(kref, typeof(*vm), resv_ref);
 
 	dma_resv_fini(&vm->_resv);
+	mutex_destroy(&vm->mutex);
+
 	kfree(vm);
 }
 
@@ -168,6 +190,8 @@ static void __i915_vm_release(struct work_struct *work)
 	struct i915_address_space *vm =
 		container_of(work, struct i915_address_space, release_work);
 
+	__i915_vm_close(vm);
+
 	/* Synchronize async unbinds. */
 	i915_vma_resource_bind_dep_sync_all(vm);
 
@@ -201,7 +225,6 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 
 	vm->pending_unbind = RB_ROOT_CACHED;
 	INIT_WORK(&vm->release_work, __i915_vm_release);
-	atomic_set(&vm->open, 1);
 
 	/*
 	 * The vm->mutex must be reclaim safe (for use in the shrinker).
@@ -245,6 +268,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 	vm->mm.head_node.color = I915_COLOR_UNEVICTABLE;
 
 	INIT_LIST_HEAD(&vm->bound_list);
+	INIT_LIST_HEAD(&vm->unbound_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 9d83c2d3959c..4529b5e9f6e6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -240,15 +240,6 @@ struct i915_address_space {
 
 	unsigned int bind_async_flags;
 
-	/*
-	 * Each active user context has its own address space (in full-ppgtt).
-	 * Since the vm may be shared between multiple contexts, we count how
-	 * many contexts keep us "open". Once open hits zero, we are closed
-	 * and do not allow any new attachments, and proceed to shutdown our
-	 * vma and page directories.
-	 */
-	atomic_t open;
-
 	struct mutex mutex; /* protects vma and our lists */
 
 	struct kref resv_ref; /* kref to keep the reservation lock alive. */
@@ -263,6 +254,11 @@ struct i915_address_space {
 	 */
 	struct list_head bound_list;
 
+	/**
+	 * List of vmas not yet bound or evicted.
+	 */
+	struct list_head unbound_list;
+
 	/* Global GTT */
 	bool is_ggtt:1;
 
@@ -272,6 +268,9 @@ struct i915_address_space {
 	/* Some systems support read-only mappings for GGTT and/or PPGTT */
 	bool has_read_only:1;
 
+	/* Skip pte rewrite on unbind for suspend. Protected by @mutex */
+	bool skip_pte_rewrite:1;
+
 	u8 top;
 	u8 pd_shift;
 	u8 scratch_order;
@@ -446,6 +445,17 @@ i915_vm_get(struct i915_address_space *vm)
 	return vm;
 }
 
+static inline struct i915_address_space *
+i915_vm_tryget(struct i915_address_space *vm)
+{
+	return kref_get_unless_zero(&vm->ref) ? vm : NULL;
+}
+
+static inline void assert_vm_alive(struct i915_address_space *vm)
+{
+	GEM_BUG_ON(!kref_read(&vm->ref));
+}
+
 /**
  * i915_vm_resv_get - Obtain a reference on the vm's reservation lock
  * @vm: The vm whose reservation lock we want to share.
@@ -476,34 +486,6 @@ static inline void i915_vm_resv_put(struct i915_address_space *vm)
 	kref_put(&vm->resv_ref, i915_vm_resv_release);
 }
 
-static inline struct i915_address_space *
-i915_vm_open(struct i915_address_space *vm)
-{
-	GEM_BUG_ON(!atomic_read(&vm->open));
-	atomic_inc(&vm->open);
-	return i915_vm_get(vm);
-}
-
-static inline bool
-i915_vm_tryopen(struct i915_address_space *vm)
-{
-	if (atomic_add_unless(&vm->open, 1, 0))
-		return i915_vm_get(vm);
-
-	return false;
-}
-
-void __i915_vm_close(struct i915_address_space *vm);
-
-static inline void
-i915_vm_close(struct i915_address_space *vm)
-{
-	GEM_BUG_ON(!atomic_read(&vm->open));
-	__i915_vm_close(vm);
-
-	i915_vm_put(vm);
-}
-
 void i915_address_space_init(struct i915_address_space *vm, int subclass);
 void i915_address_space_fini(struct i915_address_space *vm);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index 72d5faab8f9a..09f8cd2d0e2c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -1736,15 +1736,9 @@ static int live_preempt(void *arg)
 	enum intel_engine_id id;
 	int err = -ENOMEM;
 
-	if (igt_spinner_init(&spin_hi, gt))
-		return -ENOMEM;
-
-	if (igt_spinner_init(&spin_lo, gt))
-		goto err_spin_hi;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
@@ -1752,6 +1746,12 @@ static int live_preempt(void *arg)
 		goto err_ctx_hi;
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
+	if (igt_spinner_init(&spin_hi, gt))
+		goto err_ctx_lo;
+
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_spin_hi;
+
 	for_each_engine(engine, gt, id) {
 		struct igt_live_test t;
 		struct i915_request *rq;
@@ -1761,14 +1761,14 @@ static int live_preempt(void *arg)
 
 		if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_lo, ctx_lo, engine,
 					    MI_ARB_CHECK);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1777,7 +1777,7 @@ static int live_preempt(void *arg)
 			GEM_TRACE_DUMP();
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_hi, ctx_hi, engine,
@@ -1785,7 +1785,7 @@ static int live_preempt(void *arg)
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1794,7 +1794,7 @@ static int live_preempt(void *arg)
 			GEM_TRACE_DUMP();
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		igt_spinner_end(&spin_hi);
@@ -1802,19 +1802,19 @@ static int live_preempt(void *arg)
 
 		if (igt_live_test_end(&t)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 	}
 
 	err = 0;
-err_ctx_lo:
-	kernel_context_close(ctx_lo);
-err_ctx_hi:
-	kernel_context_close(ctx_hi);
 err_spin_lo:
 	igt_spinner_fini(&spin_lo);
 err_spin_hi:
 	igt_spinner_fini(&spin_hi);
+err_ctx_lo:
+	kernel_context_close(ctx_lo);
+err_ctx_hi:
+	kernel_context_close(ctx_hi);
 	return err;
 }
 
@@ -1828,20 +1828,20 @@ static int live_late_preempt(void *arg)
 	enum intel_engine_id id;
 	int err = -ENOMEM;
 
-	if (igt_spinner_init(&spin_hi, gt))
-		return -ENOMEM;
-
-	if (igt_spinner_init(&spin_lo, gt))
-		goto err_spin_hi;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
 	if (!ctx_lo)
 		goto err_ctx_hi;
 
+	if (igt_spinner_init(&spin_hi, gt))
+		goto err_ctx_lo;
+
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_spin_hi;
+
 	/* Make sure ctx_lo stays before ctx_hi until we trigger preemption. */
 	ctx_lo->sched.priority = 1;
 
@@ -1854,14 +1854,14 @@ static int live_late_preempt(void *arg)
 
 		if (igt_live_test_begin(&t, gt->i915, __func__, engine->name)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = spinner_create_request(&spin_lo, ctx_lo, engine,
 					    MI_ARB_CHECK);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1875,7 +1875,7 @@ static int live_late_preempt(void *arg)
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
@@ -1898,19 +1898,19 @@ static int live_late_preempt(void *arg)
 
 		if (igt_live_test_end(&t)) {
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 	}
 
 	err = 0;
-err_ctx_lo:
-	kernel_context_close(ctx_lo);
-err_ctx_hi:
-	kernel_context_close(ctx_hi);
 err_spin_lo:
 	igt_spinner_fini(&spin_lo);
 err_spin_hi:
 	igt_spinner_fini(&spin_hi);
+err_ctx_lo:
+	kernel_context_close(ctx_lo);
+err_ctx_hi:
+	kernel_context_close(ctx_hi);
 	return err;
 
 err_wedged:
@@ -1918,7 +1918,7 @@ static int live_late_preempt(void *arg)
 	igt_spinner_end(&spin_lo);
 	intel_gt_set_wedged(gt);
 	err = -EIO;
-	goto err_ctx_lo;
+	goto err_spin_lo;
 }
 
 struct preempt_client {
@@ -3382,12 +3382,9 @@ static int live_preempt_timeout(void *arg)
 	if (!intel_has_reset_engine(gt))
 		return 0;
 
-	if (igt_spinner_init(&spin_lo, gt))
-		return -ENOMEM;
-
 	ctx_hi = kernel_context(gt->i915, NULL);
 	if (!ctx_hi)
-		goto err_spin_lo;
+		return -ENOMEM;
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
@@ -3395,6 +3392,9 @@ static int live_preempt_timeout(void *arg)
 		goto err_ctx_hi;
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
+	if (igt_spinner_init(&spin_lo, gt))
+		goto err_ctx_lo;
+
 	for_each_engine(engine, gt, id) {
 		unsigned long saved_timeout;
 		struct i915_request *rq;
@@ -3406,21 +3406,21 @@ static int live_preempt_timeout(void *arg)
 					    MI_NOOP); /* preemption disabled */
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		i915_request_add(rq);
 		if (!igt_wait_for_spinner(&spin_lo, rq)) {
 			intel_gt_set_wedged(gt);
 			err = -EIO;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		rq = igt_request_alloc(ctx_hi, engine);
 		if (IS_ERR(rq)) {
 			igt_spinner_end(&spin_lo);
 			err = PTR_ERR(rq);
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		/* Flush the previous CS ack before changing timeouts */
@@ -3440,7 +3440,7 @@ static int live_preempt_timeout(void *arg)
 			intel_gt_set_wedged(gt);
 			i915_request_put(rq);
 			err = -ETIME;
-			goto err_ctx_lo;
+			goto err_spin_lo;
 		}
 
 		igt_spinner_end(&spin_lo);
@@ -3448,12 +3448,12 @@ static int live_preempt_timeout(void *arg)
 	}
 
 	err = 0;
+err_spin_lo:
+	igt_spinner_fini(&spin_lo);
 err_ctx_lo:
 	kernel_context_close(ctx_lo);
 err_ctx_hi:
 	kernel_context_close(ctx_hi);
-err_spin_lo:
-	igt_spinner_fini(&spin_lo);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e10187cd0a0..dd84ebabb50f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -142,8 +142,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 	while (!ret && (vma = list_first_entry_or_null(&obj->vma.list,
 						       struct i915_vma,
 						       obj_link))) {
-		struct i915_address_space *vm = vma->vm;
-
 		list_move_tail(&vma->obj_link, &still_in_list);
 		if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK))
 			continue;
@@ -154,7 +152,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		}
 
 		ret = -EAGAIN;
-		if (!i915_vm_tryopen(vm))
+		if (!i915_vm_tryget(vma->vm))
 			break;
 
 		/* Prevent vma being freed by i915_vma_parked as we unbind */
@@ -186,7 +184,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			__i915_vma_put(vma);
 		}
 
-		i915_vm_close(vm);
+		i915_vm_put(vma->vm);
 		spin_lock(&obj->vma.lock);
 	}
 	list_splice_init(&still_in_list, &obj->vma.list);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 94fcdb7bd21d..91538bc38110 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -47,7 +47,7 @@ static inline void assert_vma_held_evict(const struct i915_vma *vma)
 	 * This is the only exception to the requirement of the object lock
 	 * being held.
 	 */
-	if (atomic_read(&vma->vm->open))
+	if (kref_read(&vma->vm->ref))
 		assert_object_held_shared(vma->obj);
 }
 
@@ -113,6 +113,7 @@ vma_create(struct drm_i915_gem_object *obj,
 	struct i915_vma *pos = ERR_PTR(-E2BIG);
 	struct i915_vma *vma;
 	struct rb_node *rb, **p;
+	int err;
 
 	/* The aliasing_ppgtt should never be used directly! */
 	GEM_BUG_ON(vm == &vm->gt->ggtt->alias->vm);
@@ -122,7 +123,6 @@ vma_create(struct drm_i915_gem_object *obj,
 		return ERR_PTR(-ENOMEM);
 
 	kref_init(&vma->ref);
-	vma->vm = i915_vm_get(vm);
 	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
 	vma->size = obj->base.size;
@@ -138,6 +138,8 @@ vma_create(struct drm_i915_gem_object *obj,
 	}
 
 	INIT_LIST_HEAD(&vma->closed_link);
+	INIT_LIST_HEAD(&vma->obj_link);
+	RB_CLEAR_NODE(&vma->obj_node);
 
 	if (view && view->type != I915_GGTT_VIEW_NORMAL) {
 		vma->ggtt_view = *view;
@@ -163,8 +165,16 @@ vma_create(struct drm_i915_gem_object *obj,
 
 	GEM_BUG_ON(!IS_ALIGNED(vma->size, I915_GTT_PAGE_SIZE));
 
-	spin_lock(&obj->vma.lock);
+	err = mutex_lock_interruptible(&vm->mutex);
+	if (err) {
+		pos = ERR_PTR(err);
+		goto err_vma;
+	}
 
+	vma->vm = vm;
+	list_add_tail(&vma->vm_link, &vm->unbound_list);
+
+	spin_lock(&obj->vma.lock);
 	if (i915_is_ggtt(vm)) {
 		if (unlikely(overflows_type(vma->size, u32)))
 			goto err_unlock;
@@ -222,13 +232,15 @@ vma_create(struct drm_i915_gem_object *obj,
 		list_add_tail(&vma->obj_link, &obj->vma.list);
 
 	spin_unlock(&obj->vma.lock);
+	mutex_unlock(&vm->mutex);
 
 	return vma;
 
 err_unlock:
 	spin_unlock(&obj->vma.lock);
+	list_del_init(&vma->vm_link);
+	mutex_unlock(&vm->mutex);
 err_vma:
-	i915_vm_put(vm);
 	i915_vma_free(vma);
 	return pos;
 }
@@ -279,7 +291,7 @@ i915_vma_instance(struct drm_i915_gem_object *obj,
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm));
-	GEM_BUG_ON(!atomic_read(&vm->open));
+	GEM_BUG_ON(!kref_read(&vm->ref));
 
 	spin_lock(&obj->vma.lock);
 	vma = i915_vma_lookup(obj, vm, view);
@@ -322,7 +334,6 @@ static void __vma_release(struct dma_fence_work *work)
 		i915_gem_object_put(vw->pinned);
 
 	i915_vm_free_pt_stash(vw->vm, &vw->stash);
-	i915_vm_put(vw->vm);
 	if (vw->vma_res)
 		i915_vma_resource_put(vw->vma_res);
 }
@@ -841,7 +852,7 @@ i915_vma_insert(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, color));
 
-	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
 
 	return 0;
 }
@@ -857,7 +868,7 @@ i915_vma_detach(struct i915_vma *vma)
 	 * vma, we can drop its hold on the backing storage and allow
 	 * it to be reaped by the shrinker.
 	 */
-	list_del(&vma->vm_link);
+	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
 }
 
 static bool try_qad_pin(struct i915_vma *vma, unsigned int flags)
@@ -1373,8 +1384,7 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
 			goto err_rpm;
 		}
 
-		work->vm = i915_vm_get(vma->vm);
-
+		work->vm = vma->vm;
 		dma_fence_work_chain(&work->base, moving);
 
 		/* Allocate enough page directories to used PTE */
@@ -1622,7 +1632,6 @@ void i915_vma_release(struct kref *ref)
 {
 	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
 
-	i915_vm_put(vma->vm);
 	i915_active_fini(&vma->active);
 	GEM_WARN_ON(vma->resource);
 	i915_vma_free(vma);
@@ -1638,7 +1647,7 @@ static void force_unbind(struct i915_vma *vma)
 	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 }
 
-static void release_references(struct i915_vma *vma)
+static void release_references(struct i915_vma *vma, bool vm_ddestroy)
 {
 	struct drm_i915_gem_object *obj = vma->obj;
 
@@ -1648,10 +1657,14 @@ static void release_references(struct i915_vma *vma)
 	list_del(&vma->obj_link);
 	if (!RB_EMPTY_NODE(&vma->obj_node))
 		rb_erase(&vma->obj_node, &obj->vma.tree);
+
 	spin_unlock(&obj->vma.lock);
 
 	__i915_vma_remove_closed(vma);
 
+	if (vm_ddestroy)
+		i915_vm_resv_put(vma->vm);
+
 	__i915_vma_put(vma);
 }
 
@@ -1685,15 +1698,21 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
 	lockdep_assert_held(&vma->vm->mutex);
 
 	force_unbind(vma);
-	release_references(vma);
+	list_del_init(&vma->vm_link);
+	release_references(vma, false);
 }
 
 void i915_vma_destroy(struct i915_vma *vma)
 {
+	bool vm_ddestroy;
+
 	mutex_lock(&vma->vm->mutex);
 	force_unbind(vma);
+	list_del_init(&vma->vm_link);
+	vm_ddestroy = vma->vm_ddestroy;
+	vma->vm_ddestroy = false;
 	mutex_unlock(&vma->vm->mutex);
-	release_references(vma);
+	release_references(vma, vm_ddestroy);
 }
 
 void i915_vma_parked(struct intel_gt *gt)
@@ -1711,7 +1730,7 @@ void i915_vma_parked(struct intel_gt *gt)
 		if (!kref_get_unless_zero(&obj->base.refcount))
 			continue;
 
-		if (!i915_vm_tryopen(vm)) {
+		if (!i915_vm_tryget(vm)) {
 			i915_gem_object_put(obj);
 			continue;
 		}
@@ -1737,7 +1756,7 @@ void i915_vma_parked(struct intel_gt *gt)
 		}
 
 		i915_gem_object_put(obj);
-		i915_vm_close(vm);
+		i915_vm_put(vm);
 	}
 }
 
@@ -1888,7 +1907,9 @@ struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async)
 
 	/* If vm is not open, unbind is a nop. */
 	vma_res->needs_wakeref = i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND) &&
-		atomic_read(&vma->vm->open);
+		kref_read(&vma->vm->ref);
+	vma_res->skip_pte_rewrite = !kref_read(&vma->vm->ref) ||
+		vma->vm->skip_pte_rewrite;
 	trace_i915_vma_unbind(vma);
 
 	unbind_fence = i915_vma_resource_unbind(vma_res);
diff --git a/drivers/gpu/drm/i915/i915_vma_resource.c b/drivers/gpu/drm/i915/i915_vma_resource.c
index 57ae92ba8af1..27c55027387a 100644
--- a/drivers/gpu/drm/i915/i915_vma_resource.c
+++ b/drivers/gpu/drm/i915/i915_vma_resource.c
@@ -178,7 +178,7 @@ static void i915_vma_resource_unbind_work(struct work_struct *work)
 	bool lockdep_cookie;
 
 	lockdep_cookie = dma_fence_begin_signalling();
-	if (likely(atomic_read(&vm->open)))
+	if (likely(!vma_res->skip_pte_rewrite))
 		vma_res->ops->unbind_vma(vm, vma_res);
 
 	dma_fence_end_signalling(lockdep_cookie);
diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h
index 25913913baa6..5d8427caa2ba 100644
--- a/drivers/gpu/drm/i915/i915_vma_resource.h
+++ b/drivers/gpu/drm/i915/i915_vma_resource.h
@@ -62,6 +62,11 @@ struct i915_page_sizes {
  * deferred to a work item awaiting unsignaled fences. This is a hack.
  * (dma_fence_work uses a fence flag for this, but this seems slightly
  * cleaner).
+ * @needs_wakeref: Whether a wakeref is needed during unbind. Since we can't
+ * take a wakeref in the dma-fence signalling critical path, it needs to be
+ * taken when the unbind is scheduled.
+ * @skip_pte_rewrite: During ggtt suspend and vm takedown pte rewriting
+ * needs to be skipped for unbind.
  *
  * The lifetime of a struct i915_vma_resource is from a binding request to
  * the actual possible asynchronous unbind has completed.
@@ -113,6 +118,7 @@ struct i915_vma_resource {
 	bool allocated:1;
 	bool immediate_unbind:1;
 	bool needs_wakeref:1;
+	bool skip_pte_rewrite:1;
 };
 
 bool i915_vma_resource_hold(struct i915_vma_resource *vma_res,
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index 88370dadca82..eac36be184e5 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -271,6 +271,13 @@ struct i915_vma {
 #define I915_VMA_PAGES_ACTIVE (BIT(24) | 1)
 	atomic_t pages_count; /* number of active binds to the pages */
 
+	/**
+	 * Whether we hold a reference on the vm dma_resv lock to temporarily
+	 * block vm freeing until the vma is destroyed.
+	 * Protected by the vm mutex.
+	 */
+	bool vm_ddestroy;
+
 	/**
 	 * Support different GGTT views into the same object.
 	 * This means there can be multiple VMA mappings per object and per VM.
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index ab751192eb3b..5c9bfa409ff5 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1205,7 +1205,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
 		goto out_free;
 	}
 	GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
-	GEM_BUG_ON(!atomic_read(&ppgtt->vm.open));
+	assert_vm_alive(&ppgtt->vm);
 
 	err = func(&ppgtt->vm, 0, ppgtt->vm.total, end_time);
 
@@ -1438,7 +1438,7 @@ static void track_vma_bind(struct i915_vma *vma)
 	vma->resource->bi.pages = vma->pages;
 
 	mutex_lock(&vma->vm->mutex);
-	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
 	mutex_unlock(&vma->vm->mutex);
 }
 
-- 
2.34.1

Re: [PATCH v2 1/3] drm/i915: Remove the vm open count

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:21:58AM +0100, Thomas Hellström wrote:
>vms are not getting properly closed. Rather than fixing that,
>Remove the vm open count and instead rely on the vm refcount.
>
>The vm open count existed solely to break the strong references the
>vmas had on the vms. Now instead make those references weak and
>ensure vmas are destroyed when the vm is destroyed.
>
>Unfortunately if the vm destructor and the object destructor both
>wants to destroy a vma, that may lead to a race in that the vm
>destructor just unbinds the vma and leaves the actual vma destruction
>to the object destructor. However in order for the object destructor
>to ensure the vma is unbound it needs to grab the vm mutex. In order
>to keep the vm mutex alive until the object destructor is done with
>it, somewhat hackishly grab a vm_resv refcount that is released late
>in the vma destruction process, when the vm mutex is no longer needed.
>
>v2: Address review-comments from Niranjana
>- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and
>  should ideally be replaced in an upcoming patch.
>- Remove an unneeded continue in clear_vm_list and update comment.
>
>Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
> .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
> drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
> drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
> drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
> drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
> drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
> drivers/gpu/drm/i915/i915_gem.c               |  6 +-
> drivers/gpu/drm/i915/i915_vma.c               | 55 ++++++++----
> drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
> drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
> drivers/gpu/drm/i915/i915_vma_types.h         |  7 ++
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
> 15 files changed, 186 insertions(+), 164 deletions(-)
>

Looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Remove the vm open count

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:21:58AM +0100, Thomas Hellström wrote:
>vms are not getting properly closed. Rather than fixing that,
>Remove the vm open count and instead rely on the vm refcount.
>
>The vm open count existed solely to break the strong references the
>vmas had on the vms. Now instead make those references weak and
>ensure vmas are destroyed when the vm is destroyed.
>
>Unfortunately if the vm destructor and the object destructor both
>wants to destroy a vma, that may lead to a race in that the vm
>destructor just unbinds the vma and leaves the actual vma destruction
>to the object destructor. However in order for the object destructor
>to ensure the vma is unbound it needs to grab the vm mutex. In order
>to keep the vm mutex alive until the object destructor is done with
>it, somewhat hackishly grab a vm_resv refcount that is released late
>in the vma destruction process, when the vm mutex is no longer needed.
>
>v2: Address review-comments from Niranjana
>- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and
>  should ideally be replaced in an upcoming patch.
>- Remove an unneeded continue in clear_vm_list and update comment.
>
>Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dpt.c      |  2 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.c   | 29 ++-----
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++
> .../gpu/drm/i915/gem/selftests/mock_context.c |  5 +-
> drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
> drivers/gpu/drm/i915/gt/intel_ggtt.c          | 30 +++----
> drivers/gpu/drm/i915/gt/intel_gtt.c           | 54 ++++++++----
> drivers/gpu/drm/i915/gt/intel_gtt.h           | 56 ++++--------
> drivers/gpu/drm/i915/gt/selftest_execlists.c  | 86 +++++++++----------
> drivers/gpu/drm/i915/i915_gem.c               |  6 +-
> drivers/gpu/drm/i915/i915_vma.c               | 55 ++++++++----
> drivers/gpu/drm/i915/i915_vma_resource.c      |  2 +-
> drivers/gpu/drm/i915/i915_vma_resource.h      |  6 ++
> drivers/gpu/drm/i915/i915_vma_types.h         |  7 ++
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
> 15 files changed, 186 insertions(+), 164 deletions(-)
>

Looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Remove the vm open count

[Matthew Auld]

On Wed, 2 Mar 2022 at 10:22, Thomas Hellström
<thomas.hellstrom@linux.intel.com> wrote:
>
> vms are not getting properly closed. Rather than fixing that,
> Remove the vm open count and instead rely on the vm refcount.
>
> The vm open count existed solely to break the strong references the
> vmas had on the vms. Now instead make those references weak and
> ensure vmas are destroyed when the vm is destroyed.
>
> Unfortunately if the vm destructor and the object destructor both
> wants to destroy a vma, that may lead to a race in that the vm
> destructor just unbinds the vma and leaves the actual vma destruction
> to the object destructor. However in order for the object destructor
> to ensure the vma is unbound it needs to grab the vm mutex. In order
> to keep the vm mutex alive until the object destructor is done with
> it, somewhat hackishly grab a vm_resv refcount that is released late
> in the vma destruction process, when the vm mutex is no longer needed.
>
> v2: Address review-comments from Niranjana
> - Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and
>   should ideally be replaced in an upcoming patch.
> - Remove an unneeded continue in clear_vm_list and update comment.
>
> Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

[PATCH v2 2/3] drm/i915: Remove the vma refcount

[Thomas Hellström]

Now that i915_vma_parked() is taking the object lock on vma destruction,
and the only user of the vma refcount, i915_gem_object_unbind()
also takes the object lock, remove the vma refcount.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
 drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
 drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
 drivers/gpu/drm/i915/i915_vma_types.h |  1 -
 4 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dd84ebabb50f..c26110abcc0b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			break;
 		}
 
+		/*
+		 * Requiring the vm destructor to take the object lock
+		 * before destroying a vma would help us eliminate the
+		 * i915_vm_tryget() here, AND thus also the barrier stuff
+		 * at the end. That's an easy fix, but sleeping locks in
+		 * a kthread should generally be avoided.
+		 */
 		ret = -EAGAIN;
 		if (!i915_vm_tryget(vma->vm))
 			break;
 
-		/* Prevent vma being freed by i915_vma_parked as we unbind */
-		vma = __i915_vma_get(vma);
 		spin_unlock(&obj->vma.lock);
 
+		/*
+		 * Since i915_vma_parked() takes the object lock
+		 * before vma destruction, it won't race us here,
+		 * and destroy the vma from under us.
+		 */
+
 		if (vma) {
 			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
 			ret = -EBUSY;
@@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 					ret = i915_vma_unbind(vma);
 				}
 			}
-
-			__i915_vma_put(vma);
 		}
 
 		i915_vm_put(vma->vm);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 91538bc38110..6fd25b39748f 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
 	if (vma == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	kref_init(&vma->ref);
 	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
 	vma->size = obj->base.size;
@@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
 		__i915_vma_remove_closed(vma);
 }
 
-void i915_vma_release(struct kref *ref)
-{
-	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
-
-	i915_active_fini(&vma->active);
-	GEM_WARN_ON(vma->resource);
-	i915_vma_free(vma);
-}
-
 static void force_unbind(struct i915_vma *vma)
 {
 	if (!drm_mm_node_allocated(&vma->node))
@@ -1665,7 +1655,9 @@ static void release_references(struct i915_vma *vma, bool vm_ddestroy)
 	if (vm_ddestroy)
 		i915_vm_resv_put(vma->vm);
 
-	__i915_vma_put(vma);
+	i915_active_fini(&vma->active);
+	GEM_WARN_ON(vma->resource);
+	i915_vma_free(vma);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 67ae7341c7e0..6034991d89fe 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma *vma);
 void i915_vma_close(struct i915_vma *vma);
 void i915_vma_reopen(struct i915_vma *vma);
 
-static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
-{
-	if (kref_get_unless_zero(&vma->ref))
-		return vma;
-
-	return NULL;
-}
-
-void i915_vma_release(struct kref *ref);
-static inline void __i915_vma_put(struct i915_vma *vma)
-{
-	kref_put(&vma->ref, i915_vma_release);
-}
-
 void i915_vma_destroy_locked(struct i915_vma *vma);
 void i915_vma_destroy(struct i915_vma *vma);
 
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index eac36be184e5..be6e028c3b57 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -211,7 +211,6 @@ struct i915_vma {
 	 * handles (but same file) for execbuf, i.e. the number of aliases
 	 * that exist in the ctx->handle_vmas LUT for this vma.
 	 */
-	struct kref ref;
 	atomic_t open_count;
 	atomic_t flags;
 	/**
-- 
2.34.1

[Intel-gfx] [PATCH v2 2/3] drm/i915: Remove the vma refcount

[Thomas Hellström]

Now that i915_vma_parked() is taking the object lock on vma destruction,
and the only user of the vma refcount, i915_gem_object_unbind()
also takes the object lock, remove the vma refcount.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
 drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
 drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
 drivers/gpu/drm/i915/i915_vma_types.h |  1 -
 4 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dd84ebabb50f..c26110abcc0b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			break;
 		}
 
+		/*
+		 * Requiring the vm destructor to take the object lock
+		 * before destroying a vma would help us eliminate the
+		 * i915_vm_tryget() here, AND thus also the barrier stuff
+		 * at the end. That's an easy fix, but sleeping locks in
+		 * a kthread should generally be avoided.
+		 */
 		ret = -EAGAIN;
 		if (!i915_vm_tryget(vma->vm))
 			break;
 
-		/* Prevent vma being freed by i915_vma_parked as we unbind */
-		vma = __i915_vma_get(vma);
 		spin_unlock(&obj->vma.lock);
 
+		/*
+		 * Since i915_vma_parked() takes the object lock
+		 * before vma destruction, it won't race us here,
+		 * and destroy the vma from under us.
+		 */
+
 		if (vma) {
 			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
 			ret = -EBUSY;
@@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 					ret = i915_vma_unbind(vma);
 				}
 			}
-
-			__i915_vma_put(vma);
 		}
 
 		i915_vm_put(vma->vm);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 91538bc38110..6fd25b39748f 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
 	if (vma == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	kref_init(&vma->ref);
 	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
 	vma->size = obj->base.size;
@@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
 		__i915_vma_remove_closed(vma);
 }
 
-void i915_vma_release(struct kref *ref)
-{
-	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
-
-	i915_active_fini(&vma->active);
-	GEM_WARN_ON(vma->resource);
-	i915_vma_free(vma);
-}
-
 static void force_unbind(struct i915_vma *vma)
 {
 	if (!drm_mm_node_allocated(&vma->node))
@@ -1665,7 +1655,9 @@ static void release_references(struct i915_vma *vma, bool vm_ddestroy)
 	if (vm_ddestroy)
 		i915_vm_resv_put(vma->vm);
 
-	__i915_vma_put(vma);
+	i915_active_fini(&vma->active);
+	GEM_WARN_ON(vma->resource);
+	i915_vma_free(vma);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 67ae7341c7e0..6034991d89fe 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma *vma);
 void i915_vma_close(struct i915_vma *vma);
 void i915_vma_reopen(struct i915_vma *vma);
 
-static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
-{
-	if (kref_get_unless_zero(&vma->ref))
-		return vma;
-
-	return NULL;
-}
-
-void i915_vma_release(struct kref *ref);
-static inline void __i915_vma_put(struct i915_vma *vma)
-{
-	kref_put(&vma->ref, i915_vma_release);
-}
-
 void i915_vma_destroy_locked(struct i915_vma *vma);
 void i915_vma_destroy(struct i915_vma *vma);
 
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index eac36be184e5..be6e028c3b57 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -211,7 +211,6 @@ struct i915_vma {
 	 * handles (but same file) for execbuf, i.e. the number of aliases
 	 * that exist in the ctx->handle_vmas LUT for this vma.
 	 */
-	struct kref ref;
 	atomic_t open_count;
 	atomic_t flags;
 	/**
-- 
2.34.1

Re: [PATCH v2 2/3] drm/i915: Remove the vma refcount

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:21:59AM +0100, Thomas Hellström wrote:
>Now that i915_vma_parked() is taking the object lock on vma destruction,
>and the only user of the vma refcount, i915_gem_object_unbind()
>also takes the object lock, remove the vma refcount.
>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
> drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
> drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
> drivers/gpu/drm/i915/i915_vma_types.h |  1 -
> 4 files changed, 16 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>index dd84ebabb50f..c26110abcc0b 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 			break;
> 		}
>
>+		/*
>+		 * Requiring the vm destructor to take the object lock
>+		 * before destroying a vma would help us eliminate the
>+		 * i915_vm_tryget() here, AND thus also the barrier stuff
>+		 * at the end. That's an easy fix, but sleeping locks in
>+		 * a kthread should generally be avoided.
>+		 */
> 		ret = -EAGAIN;
> 		if (!i915_vm_tryget(vma->vm))
> 			break;
>
>-		/* Prevent vma being freed by i915_vma_parked as we unbind */
>-		vma = __i915_vma_get(vma);
> 		spin_unlock(&obj->vma.lock);
>
>+		/*
>+		 * Since i915_vma_parked() takes the object lock
>+		 * before vma destruction, it won't race us here,
>+		 * and destroy the vma from under us.
>+		 */
>+
> 		if (vma) {
> 			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> 			ret = -EBUSY;
>@@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 					ret = i915_vma_unbind(vma);
> 				}
> 			}
>-
>-			__i915_vma_put(vma);
> 		}
>
> 		i915_vm_put(vma->vm);

One issue still left in i915_gem_object_unbind is that it temporarily removes
vmas from the obj->vma.list and adds back later as vma needs to be unbind outside
the obj->vma.lock spinlock. This is an issue for other places where we iterate
over the obj->vma.list. i915_debugfs_describe_obj is one such case (upcoming
vm_bind will be another) that iterates over this list.
What is the plan here? Do we need to take object lock while iterating over the
list?

But this just something I noticed and not related to this patch.
This patch looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
 

>diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>index 91538bc38110..6fd25b39748f 100644
>--- a/drivers/gpu/drm/i915/i915_vma.c
>+++ b/drivers/gpu/drm/i915/i915_vma.c
>@@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
> 	if (vma == NULL)
> 		return ERR_PTR(-ENOMEM);
>
>-	kref_init(&vma->ref);
> 	vma->ops = &vm->vma_ops;
> 	vma->obj = obj;
> 	vma->size = obj->base.size;
>@@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
> 		__i915_vma_remove_closed(vma);
> }
>
>-void i915_vma_release(struct kref *ref)
>-{
>-	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
>-
>-	i915_active_fini(&vma->active);
>-	GEM_WARN_ON(vma->resource);
>-	i915_vma_free(vma);
>-}
>-
> static void force_unbind(struct i915_vma *vma)
> {
> 	if (!drm_mm_node_allocated(&vma->node))
>@@ -1665,7 +1655,9 @@ static void release_references(struct i915_vma *vma, bool vm_ddestroy)
> 	if (vm_ddestroy)
> 		i915_vm_resv_put(vma->vm);
>
>-	__i915_vma_put(vma);
>+	i915_active_fini(&vma->active);
>+	GEM_WARN_ON(vma->resource);
>+	i915_vma_free(vma);
> }
>
> /**
>diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
>index 67ae7341c7e0..6034991d89fe 100644
>--- a/drivers/gpu/drm/i915/i915_vma.h
>+++ b/drivers/gpu/drm/i915/i915_vma.h
>@@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma *vma);
> void i915_vma_close(struct i915_vma *vma);
> void i915_vma_reopen(struct i915_vma *vma);
>
>-static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
>-{
>-	if (kref_get_unless_zero(&vma->ref))
>-		return vma;
>-
>-	return NULL;
>-}
>-
>-void i915_vma_release(struct kref *ref);
>-static inline void __i915_vma_put(struct i915_vma *vma)
>-{
>-	kref_put(&vma->ref, i915_vma_release);
>-}
>-
> void i915_vma_destroy_locked(struct i915_vma *vma);
> void i915_vma_destroy(struct i915_vma *vma);
>
>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
>index eac36be184e5..be6e028c3b57 100644
>--- a/drivers/gpu/drm/i915/i915_vma_types.h
>+++ b/drivers/gpu/drm/i915/i915_vma_types.h
>@@ -211,7 +211,6 @@ struct i915_vma {
> 	 * handles (but same file) for execbuf, i.e. the number of aliases
> 	 * that exist in the ctx->handle_vmas LUT for this vma.
> 	 */
>-	struct kref ref;
> 	atomic_t open_count;
> 	atomic_t flags;
> 	/**
>-- 
>2.34.1
>

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Remove the vma refcount

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:21:59AM +0100, Thomas Hellström wrote:
>Now that i915_vma_parked() is taking the object lock on vma destruction,
>and the only user of the vma refcount, i915_gem_object_unbind()
>also takes the object lock, remove the vma refcount.
>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
> drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
> drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
> drivers/gpu/drm/i915/i915_vma_types.h |  1 -
> 4 files changed, 16 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>index dd84ebabb50f..c26110abcc0b 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 			break;
> 		}
>
>+		/*
>+		 * Requiring the vm destructor to take the object lock
>+		 * before destroying a vma would help us eliminate the
>+		 * i915_vm_tryget() here, AND thus also the barrier stuff
>+		 * at the end. That's an easy fix, but sleeping locks in
>+		 * a kthread should generally be avoided.
>+		 */
> 		ret = -EAGAIN;
> 		if (!i915_vm_tryget(vma->vm))
> 			break;
>
>-		/* Prevent vma being freed by i915_vma_parked as we unbind */
>-		vma = __i915_vma_get(vma);
> 		spin_unlock(&obj->vma.lock);
>
>+		/*
>+		 * Since i915_vma_parked() takes the object lock
>+		 * before vma destruction, it won't race us here,
>+		 * and destroy the vma from under us.
>+		 */
>+
> 		if (vma) {
> 			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> 			ret = -EBUSY;
>@@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 					ret = i915_vma_unbind(vma);
> 				}
> 			}
>-
>-			__i915_vma_put(vma);
> 		}
>
> 		i915_vm_put(vma->vm);

One issue still left in i915_gem_object_unbind is that it temporarily removes
vmas from the obj->vma.list and adds back later as vma needs to be unbind outside
the obj->vma.lock spinlock. This is an issue for other places where we iterate
over the obj->vma.list. i915_debugfs_describe_obj is one such case (upcoming
vm_bind will be another) that iterates over this list.
What is the plan here? Do we need to take object lock while iterating over the
list?

But this just something I noticed and not related to this patch.
This patch looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
 

>diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>index 91538bc38110..6fd25b39748f 100644
>--- a/drivers/gpu/drm/i915/i915_vma.c
>+++ b/drivers/gpu/drm/i915/i915_vma.c
>@@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
> 	if (vma == NULL)
> 		return ERR_PTR(-ENOMEM);
>
>-	kref_init(&vma->ref);
> 	vma->ops = &vm->vma_ops;
> 	vma->obj = obj;
> 	vma->size = obj->base.size;
>@@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
> 		__i915_vma_remove_closed(vma);
> }
>
>-void i915_vma_release(struct kref *ref)
>-{
>-	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
>-
>-	i915_active_fini(&vma->active);
>-	GEM_WARN_ON(vma->resource);
>-	i915_vma_free(vma);
>-}
>-
> static void force_unbind(struct i915_vma *vma)
> {
> 	if (!drm_mm_node_allocated(&vma->node))
>@@ -1665,7 +1655,9 @@ static void release_references(struct i915_vma *vma, bool vm_ddestroy)
> 	if (vm_ddestroy)
> 		i915_vm_resv_put(vma->vm);
>
>-	__i915_vma_put(vma);
>+	i915_active_fini(&vma->active);
>+	GEM_WARN_ON(vma->resource);
>+	i915_vma_free(vma);
> }
>
> /**
>diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
>index 67ae7341c7e0..6034991d89fe 100644
>--- a/drivers/gpu/drm/i915/i915_vma.h
>+++ b/drivers/gpu/drm/i915/i915_vma.h
>@@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma *vma);
> void i915_vma_close(struct i915_vma *vma);
> void i915_vma_reopen(struct i915_vma *vma);
>
>-static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
>-{
>-	if (kref_get_unless_zero(&vma->ref))
>-		return vma;
>-
>-	return NULL;
>-}
>-
>-void i915_vma_release(struct kref *ref);
>-static inline void __i915_vma_put(struct i915_vma *vma)
>-{
>-	kref_put(&vma->ref, i915_vma_release);
>-}
>-
> void i915_vma_destroy_locked(struct i915_vma *vma);
> void i915_vma_destroy(struct i915_vma *vma);
>
>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
>index eac36be184e5..be6e028c3b57 100644
>--- a/drivers/gpu/drm/i915/i915_vma_types.h
>+++ b/drivers/gpu/drm/i915/i915_vma_types.h
>@@ -211,7 +211,6 @@ struct i915_vma {
> 	 * handles (but same file) for execbuf, i.e. the number of aliases
> 	 * that exist in the ctx->handle_vmas LUT for this vma.
> 	 */
>-	struct kref ref;
> 	atomic_t open_count;
> 	atomic_t flags;
> 	/**
>-- 
>2.34.1
>

Re: [PATCH v2 2/3] drm/i915: Remove the vma refcount

[Thomas Hellström]

On Wed, 2022-03-02 at 14:01 -0800, Niranjana Vishwanathapura wrote:
> On Wed, Mar 02, 2022 at 11:21:59AM +0100, Thomas Hellström wrote:
> > Now that i915_vma_parked() is taking the object lock on vma
> > destruction,
> > and the only user of the vma refcount, i915_gem_object_unbind()
> > also takes the object lock, remove the vma refcount.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
> > drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
> > drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
> > drivers/gpu/drm/i915/i915_vma_types.h |  1 -
> > 4 files changed, 16 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index dd84ebabb50f..c26110abcc0b 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct
> > drm_i915_gem_object *obj,
> >                         break;
> >                 }
> > 
> > +               /*
> > +                * Requiring the vm destructor to take the object
> > lock
> > +                * before destroying a vma would help us eliminate
> > the
> > +                * i915_vm_tryget() here, AND thus also the barrier
> > stuff
> > +                * at the end. That's an easy fix, but sleeping
> > locks in
> > +                * a kthread should generally be avoided.
> > +                */
> >                 ret = -EAGAIN;
> >                 if (!i915_vm_tryget(vma->vm))
> >                         break;
> > 
> > -               /* Prevent vma being freed by i915_vma_parked as we
> > unbind */
> > -               vma = __i915_vma_get(vma);
> >                 spin_unlock(&obj->vma.lock);
> > 
> > +               /*
> > +                * Since i915_vma_parked() takes the object lock
> > +                * before vma destruction, it won't race us here,
> > +                * and destroy the vma from under us.
> > +                */
> > +
> >                 if (vma) {
> >                         bool vm_trylock = !!(flags &
> > I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> >                         ret = -EBUSY;
> > @@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct
> > drm_i915_gem_object *obj,
> >                                         ret = i915_vma_unbind(vma);
> >                                 }
> >                         }
> > -
> > -                       __i915_vma_put(vma);
> >                 }
> > 
> >                 i915_vm_put(vma->vm);
> 
> One issue still left in i915_gem_object_unbind is that it temporarily
> removes
> vmas from the obj->vma.list and adds back later as vma needs to be
> unbind outside
> the obj->vma.lock spinlock. This is an issue for other places where
> we iterate
> over the obj->vma.list. i915_debugfs_describe_obj is one such case
> (upcoming
> vm_bind will be another) that iterates over this list.
> What is the plan here? Do we need to take object lock while iterating
> over the
> list?

Yeah, I guess that's an option if that's at all possible (we might need
to iterate over the list in the mmu notifier, for example).

The other option is to
*) get rid of the GGTT / PPGTT sorting of vmas in the list,
*) being able to determine per vma *before we unlock* if we need to
unlock the list spinlock to take action,
*) re-add all vmas we've previously iterated over at the *tail* of the
list before unlocking the list lock.

Then a termination criterion for iterating would be that we reached the
end of the list without unlocking. Otherwise we need to restart
iteration after unlocking.

This would typically give us O(2N) complexity for the iteration. If we
re-add at the *head* of the list, we'd see O(N²), but to be able to re-
add previous vmas to the tail of the list requires us to get rid of the
sorting.

> 
> But this just something I noticed and not related to this patch.
> This patch looks good to me.
> Reviewed-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura@intel.com>
>  

Thanks for reviewing. I noticed there is some documentation needing
updating as well, so I'll send out a v3 without functional changes.

/Thomas


> 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c
> > b/drivers/gpu/drm/i915/i915_vma.c
> > index 91538bc38110..6fd25b39748f 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
> >         if (vma == NULL)
> >                 return ERR_PTR(-ENOMEM);
> > 
> > -       kref_init(&vma->ref);
> >         vma->ops = &vm->vma_ops;
> >         vma->obj = obj;
> >         vma->size = obj->base.size;
> > @@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
> >                 __i915_vma_remove_closed(vma);
> > }
> > 
> > -void i915_vma_release(struct kref *ref)
> > -{
> > -       struct i915_vma *vma = container_of(ref, typeof(*vma),
> > ref);
> > -
> > -       i915_active_fini(&vma->active);
> > -       GEM_WARN_ON(vma->resource);
> > -       i915_vma_free(vma);
> > -}
> > -
> > static void force_unbind(struct i915_vma *vma)
> > {
> >         if (!drm_mm_node_allocated(&vma->node))
> > @@ -1665,7 +1655,9 @@ static void release_references(struct
> > i915_vma *vma, bool vm_ddestroy)
> >         if (vm_ddestroy)
> >                 i915_vm_resv_put(vma->vm);
> > 
> > -       __i915_vma_put(vma);
> > +       i915_active_fini(&vma->active);
> > +       GEM_WARN_ON(vma->resource);
> > +       i915_vma_free(vma);
> > }
> > 
> > /**
> > diff --git a/drivers/gpu/drm/i915/i915_vma.h
> > b/drivers/gpu/drm/i915/i915_vma.h
> > index 67ae7341c7e0..6034991d89fe 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.h
> > +++ b/drivers/gpu/drm/i915/i915_vma.h
> > @@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma
> > *vma);
> > void i915_vma_close(struct i915_vma *vma);
> > void i915_vma_reopen(struct i915_vma *vma);
> > 
> > -static inline struct i915_vma *__i915_vma_get(struct i915_vma
> > *vma)
> > -{
> > -       if (kref_get_unless_zero(&vma->ref))
> > -               return vma;
> > -
> > -       return NULL;
> > -}
> > -
> > -void i915_vma_release(struct kref *ref);
> > -static inline void __i915_vma_put(struct i915_vma *vma)
> > -{
> > -       kref_put(&vma->ref, i915_vma_release);
> > -}
> > -
> > void i915_vma_destroy_locked(struct i915_vma *vma);
> > void i915_vma_destroy(struct i915_vma *vma);
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma_types.h
> > b/drivers/gpu/drm/i915/i915_vma_types.h
> > index eac36be184e5..be6e028c3b57 100644
> > --- a/drivers/gpu/drm/i915/i915_vma_types.h
> > +++ b/drivers/gpu/drm/i915/i915_vma_types.h
> > @@ -211,7 +211,6 @@ struct i915_vma {
> >          * handles (but same file) for execbuf, i.e. the number of
> > aliases
> >          * that exist in the ctx->handle_vmas LUT for this vma.
> >          */
> > -       struct kref ref;
> >         atomic_t open_count;
> >         atomic_t flags;
> >         /**
> > -- 
> > 2.34.1
> > 

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Remove the vma refcount

[Thomas Hellström]

On Wed, 2022-03-02 at 14:01 -0800, Niranjana Vishwanathapura wrote:
> On Wed, Mar 02, 2022 at 11:21:59AM +0100, Thomas Hellström wrote:
> > Now that i915_vma_parked() is taking the object lock on vma
> > destruction,
> > and the only user of the vma refcount, i915_gem_object_unbind()
> > also takes the object lock, remove the vma refcount.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c       | 17 +++++++++++++----
> > drivers/gpu/drm/i915/i915_vma.c       | 14 +++-----------
> > drivers/gpu/drm/i915/i915_vma.h       | 14 --------------
> > drivers/gpu/drm/i915/i915_vma_types.h |  1 -
> > 4 files changed, 16 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index dd84ebabb50f..c26110abcc0b 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -151,14 +151,25 @@ int i915_gem_object_unbind(struct
> > drm_i915_gem_object *obj,
> >                         break;
> >                 }
> > 
> > +               /*
> > +                * Requiring the vm destructor to take the object
> > lock
> > +                * before destroying a vma would help us eliminate
> > the
> > +                * i915_vm_tryget() here, AND thus also the barrier
> > stuff
> > +                * at the end. That's an easy fix, but sleeping
> > locks in
> > +                * a kthread should generally be avoided.
> > +                */
> >                 ret = -EAGAIN;
> >                 if (!i915_vm_tryget(vma->vm))
> >                         break;
> > 
> > -               /* Prevent vma being freed by i915_vma_parked as we
> > unbind */
> > -               vma = __i915_vma_get(vma);
> >                 spin_unlock(&obj->vma.lock);
> > 
> > +               /*
> > +                * Since i915_vma_parked() takes the object lock
> > +                * before vma destruction, it won't race us here,
> > +                * and destroy the vma from under us.
> > +                */
> > +
> >                 if (vma) {
> >                         bool vm_trylock = !!(flags &
> > I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> >                         ret = -EBUSY;
> > @@ -180,8 +191,6 @@ int i915_gem_object_unbind(struct
> > drm_i915_gem_object *obj,
> >                                         ret = i915_vma_unbind(vma);
> >                                 }
> >                         }
> > -
> > -                       __i915_vma_put(vma);
> >                 }
> > 
> >                 i915_vm_put(vma->vm);
> 
> One issue still left in i915_gem_object_unbind is that it temporarily
> removes
> vmas from the obj->vma.list and adds back later as vma needs to be
> unbind outside
> the obj->vma.lock spinlock. This is an issue for other places where
> we iterate
> over the obj->vma.list. i915_debugfs_describe_obj is one such case
> (upcoming
> vm_bind will be another) that iterates over this list.
> What is the plan here? Do we need to take object lock while iterating
> over the
> list?

Yeah, I guess that's an option if that's at all possible (we might need
to iterate over the list in the mmu notifier, for example).

The other option is to
*) get rid of the GGTT / PPGTT sorting of vmas in the list,
*) being able to determine per vma *before we unlock* if we need to
unlock the list spinlock to take action,
*) re-add all vmas we've previously iterated over at the *tail* of the
list before unlocking the list lock.

Then a termination criterion for iterating would be that we reached the
end of the list without unlocking. Otherwise we need to restart
iteration after unlocking.

This would typically give us O(2N) complexity for the iteration. If we
re-add at the *head* of the list, we'd see O(N²), but to be able to re-
add previous vmas to the tail of the list requires us to get rid of the
sorting.

> 
> But this just something I noticed and not related to this patch.
> This patch looks good to me.
> Reviewed-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura@intel.com>
>  

Thanks for reviewing. I noticed there is some documentation needing
updating as well, so I'll send out a v3 without functional changes.

/Thomas


> 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c
> > b/drivers/gpu/drm/i915/i915_vma.c
> > index 91538bc38110..6fd25b39748f 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -122,7 +122,6 @@ vma_create(struct drm_i915_gem_object *obj,
> >         if (vma == NULL)
> >                 return ERR_PTR(-ENOMEM);
> > 
> > -       kref_init(&vma->ref);
> >         vma->ops = &vm->vma_ops;
> >         vma->obj = obj;
> >         vma->size = obj->base.size;
> > @@ -1628,15 +1627,6 @@ void i915_vma_reopen(struct i915_vma *vma)
> >                 __i915_vma_remove_closed(vma);
> > }
> > 
> > -void i915_vma_release(struct kref *ref)
> > -{
> > -       struct i915_vma *vma = container_of(ref, typeof(*vma),
> > ref);
> > -
> > -       i915_active_fini(&vma->active);
> > -       GEM_WARN_ON(vma->resource);
> > -       i915_vma_free(vma);
> > -}
> > -
> > static void force_unbind(struct i915_vma *vma)
> > {
> >         if (!drm_mm_node_allocated(&vma->node))
> > @@ -1665,7 +1655,9 @@ static void release_references(struct
> > i915_vma *vma, bool vm_ddestroy)
> >         if (vm_ddestroy)
> >                 i915_vm_resv_put(vma->vm);
> > 
> > -       __i915_vma_put(vma);
> > +       i915_active_fini(&vma->active);
> > +       GEM_WARN_ON(vma->resource);
> > +       i915_vma_free(vma);
> > }
> > 
> > /**
> > diff --git a/drivers/gpu/drm/i915/i915_vma.h
> > b/drivers/gpu/drm/i915/i915_vma.h
> > index 67ae7341c7e0..6034991d89fe 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.h
> > +++ b/drivers/gpu/drm/i915/i915_vma.h
> > @@ -222,20 +222,6 @@ void i915_vma_unlink_ctx(struct i915_vma
> > *vma);
> > void i915_vma_close(struct i915_vma *vma);
> > void i915_vma_reopen(struct i915_vma *vma);
> > 
> > -static inline struct i915_vma *__i915_vma_get(struct i915_vma
> > *vma)
> > -{
> > -       if (kref_get_unless_zero(&vma->ref))
> > -               return vma;
> > -
> > -       return NULL;
> > -}
> > -
> > -void i915_vma_release(struct kref *ref);
> > -static inline void __i915_vma_put(struct i915_vma *vma)
> > -{
> > -       kref_put(&vma->ref, i915_vma_release);
> > -}
> > -
> > void i915_vma_destroy_locked(struct i915_vma *vma);
> > void i915_vma_destroy(struct i915_vma *vma);
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma_types.h
> > b/drivers/gpu/drm/i915/i915_vma_types.h
> > index eac36be184e5..be6e028c3b57 100644
> > --- a/drivers/gpu/drm/i915/i915_vma_types.h
> > +++ b/drivers/gpu/drm/i915/i915_vma_types.h
> > @@ -211,7 +211,6 @@ struct i915_vma {
> >          * handles (but same file) for execbuf, i.e. the number of
> > aliases
> >          * that exist in the ctx->handle_vmas LUT for this vma.
> >          */
> > -       struct kref ref;
> >         atomic_t open_count;
> >         atomic_t flags;
> >         /**
> > -- 
> > 2.34.1
> > 

[PATCH v2 3/3] drm/i915/gem: Remove some unnecessary code

[Thomas Hellström]

The test for vma should always return true, and when assigning -EBUSY
to ret, the variable should already have that value.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c26110abcc0b..9747924cc57b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -118,6 +118,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			   unsigned long flags)
 {
 	struct intel_runtime_pm *rpm = &to_i915(obj->base.dev)->runtime_pm;
+	bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
 	LIST_HEAD(still_in_list);
 	intel_wakeref_t wakeref;
 	struct i915_vma *vma;
@@ -170,26 +171,21 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		 * and destroy the vma from under us.
 		 */
 
-		if (vma) {
-			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
-			ret = -EBUSY;
-			if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
-				assert_object_held(vma->obj);
-				ret = i915_vma_unbind_async(vma, vm_trylock);
-			}
+		ret = -EBUSY;
+		if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
+			assert_object_held(vma->obj);
+			ret = i915_vma_unbind_async(vma, vm_trylock);
+		}
 
-			if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
-					      !i915_vma_is_active(vma))) {
-				if (vm_trylock) {
-					if (mutex_trylock(&vma->vm->mutex)) {
-						ret = __i915_vma_unbind(vma);
-						mutex_unlock(&vma->vm->mutex);
-					} else {
-						ret = -EBUSY;
-					}
-				} else {
-					ret = i915_vma_unbind(vma);
+		if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
+				      !i915_vma_is_active(vma))) {
+			if (vm_trylock) {
+				if (mutex_trylock(&vma->vm->mutex)) {
+					ret = __i915_vma_unbind(vma);
+					mutex_unlock(&vma->vm->mutex);
 				}
+			} else {
+				ret = i915_vma_unbind(vma);
 			}
 		}
 
-- 
2.34.1

[Intel-gfx] [PATCH v2 3/3] drm/i915/gem: Remove some unnecessary code

[Thomas Hellström]

The test for vma should always return true, and when assigning -EBUSY
to ret, the variable should already have that value.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c26110abcc0b..9747924cc57b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -118,6 +118,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			   unsigned long flags)
 {
 	struct intel_runtime_pm *rpm = &to_i915(obj->base.dev)->runtime_pm;
+	bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
 	LIST_HEAD(still_in_list);
 	intel_wakeref_t wakeref;
 	struct i915_vma *vma;
@@ -170,26 +171,21 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		 * and destroy the vma from under us.
 		 */
 
-		if (vma) {
-			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
-			ret = -EBUSY;
-			if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
-				assert_object_held(vma->obj);
-				ret = i915_vma_unbind_async(vma, vm_trylock);
-			}
+		ret = -EBUSY;
+		if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
+			assert_object_held(vma->obj);
+			ret = i915_vma_unbind_async(vma, vm_trylock);
+		}
 
-			if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
-					      !i915_vma_is_active(vma))) {
-				if (vm_trylock) {
-					if (mutex_trylock(&vma->vm->mutex)) {
-						ret = __i915_vma_unbind(vma);
-						mutex_unlock(&vma->vm->mutex);
-					} else {
-						ret = -EBUSY;
-					}
-				} else {
-					ret = i915_vma_unbind(vma);
+		if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
+				      !i915_vma_is_active(vma))) {
+			if (vm_trylock) {
+				if (mutex_trylock(&vma->vm->mutex)) {
+					ret = __i915_vma_unbind(vma);
+					mutex_unlock(&vma->vm->mutex);
 				}
+			} else {
+				ret = i915_vma_unbind(vma);
 			}
 		}
 
-- 
2.34.1

Re: [PATCH v2 3/3] drm/i915/gem: Remove some unnecessary code

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:22:00AM +0100, Thomas Hellström wrote:
>The test for vma should always return true, and when assigning -EBUSY
>to ret, the variable should already have that value.
>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>index c26110abcc0b..9747924cc57b 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -118,6 +118,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 			   unsigned long flags)
> {
> 	struct intel_runtime_pm *rpm = &to_i915(obj->base.dev)->runtime_pm;
>+	bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> 	LIST_HEAD(still_in_list);
> 	intel_wakeref_t wakeref;
> 	struct i915_vma *vma;
>@@ -170,26 +171,21 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 		 * and destroy the vma from under us.
> 		 */
>
>-		if (vma) {
>-			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
>-			ret = -EBUSY;
>-			if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
>-				assert_object_held(vma->obj);
>-				ret = i915_vma_unbind_async(vma, vm_trylock);
>-			}
>+		ret = -EBUSY;
>+		if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
>+			assert_object_held(vma->obj);
>+			ret = i915_vma_unbind_async(vma, vm_trylock);
>+		}
>
>-			if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
>-					      !i915_vma_is_active(vma))) {
>-				if (vm_trylock) {
>-					if (mutex_trylock(&vma->vm->mutex)) {
>-						ret = __i915_vma_unbind(vma);
>-						mutex_unlock(&vma->vm->mutex);
>-					} else {
>-						ret = -EBUSY;
>-					}
>-				} else {
>-					ret = i915_vma_unbind(vma);
>+		if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
>+				      !i915_vma_is_active(vma))) {
>+			if (vm_trylock) {
>+				if (mutex_trylock(&vma->vm->mutex)) {
>+					ret = __i915_vma_unbind(vma);
>+					mutex_unlock(&vma->vm->mutex);
> 				}
>+			} else {
>+				ret = i915_vma_unbind(vma);
> 			}
> 		}

Looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

>
>-- 
>2.34.1
>

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/gem: Remove some unnecessary code

[Niranjana Vishwanathapura]

On Wed, Mar 02, 2022 at 11:22:00AM +0100, Thomas Hellström wrote:
>The test for vma should always return true, and when assigning -EBUSY
>to ret, the variable should already have that value.
>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>index c26110abcc0b..9747924cc57b 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -118,6 +118,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 			   unsigned long flags)
> {
> 	struct intel_runtime_pm *rpm = &to_i915(obj->base.dev)->runtime_pm;
>+	bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> 	LIST_HEAD(still_in_list);
> 	intel_wakeref_t wakeref;
> 	struct i915_vma *vma;
>@@ -170,26 +171,21 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> 		 * and destroy the vma from under us.
> 		 */
>
>-		if (vma) {
>-			bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
>-			ret = -EBUSY;
>-			if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
>-				assert_object_held(vma->obj);
>-				ret = i915_vma_unbind_async(vma, vm_trylock);
>-			}
>+		ret = -EBUSY;
>+		if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
>+			assert_object_held(vma->obj);
>+			ret = i915_vma_unbind_async(vma, vm_trylock);
>+		}
>
>-			if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
>-					      !i915_vma_is_active(vma))) {
>-				if (vm_trylock) {
>-					if (mutex_trylock(&vma->vm->mutex)) {
>-						ret = __i915_vma_unbind(vma);
>-						mutex_unlock(&vma->vm->mutex);
>-					} else {
>-						ret = -EBUSY;
>-					}
>-				} else {
>-					ret = i915_vma_unbind(vma);
>+		if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
>+				      !i915_vma_is_active(vma))) {
>+			if (vm_trylock) {
>+				if (mutex_trylock(&vma->vm->mutex)) {
>+					ret = __i915_vma_unbind(vma);
>+					mutex_unlock(&vma->vm->mutex);
> 				}
>+			} else {
>+				ret = i915_vma_unbind(vma);
> 			}
> 		}

Looks good to me.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

>
>-- 
>2.34.1
>

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for vm- and vma cleanups

[Patchwork]

== Series Details ==

Series: vm- and vma cleanups
URL   : https://patchwork.freedesktop.org/series/100945/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
67a198829865 drm/i915: Remove the vm open count
-:26: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#26: 
- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack and

total: 0 errors, 1 warnings, 0 checks, 901 lines checked
0a77c40b2518 drm/i915: Remove the vma refcount
dc594b31f5d2 drm/i915/gem: Remove some unnecessary code

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for vm- and vma cleanups

[Patchwork]

== Series Details ==

Series: vm- and vma cleanups
URL   : https://patchwork.freedesktop.org/series/100945/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for vm- and vma cleanups

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6770 bytes --]

== Series Details ==

Series: vm- and vma cleanups
URL   : https://patchwork.freedesktop.org/series/100945/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11308 -> Patchwork_22462
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/index.html

Participating hosts (51 -> 43)
------------------------------

  Additional (1): bat-adlp-4 
  Missing    (9): fi-cml-u2 fi-bdw-samus shard-tglu fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 shard-rkl shard-dg1 bat-jsl-1 

Known issues
------------

  Here are the changes found in Patchwork_22462 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@bad-flink:
    - fi-skl-6600u:       [PASS][1] -> [INCOMPLETE][2] ([i915#4547])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html

  * igt@gem_lmem_swapping@basic:
    - bat-adlp-4:         NOTRUN -> [SKIP][3] ([i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-adlp-4:         NOTRUN -> [SKIP][4] ([i915#3282])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@gem_tiled_pread_basic.html

  * igt@i915_selftest@live@gem:
    - fi-blb-e6850:       [PASS][5] -> [DMESG-FAIL][6] ([i915#4528])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/fi-blb-e6850/igt@i915_selftest@live@gem.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-blb-e6850/igt@i915_selftest@live@gem.html

  * igt@i915_selftest@live@hangcheck:
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][7] ([i915#3921])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][8] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-bdw-5557u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_chamelium@vga-hpd-fast:
    - bat-adlp-4:         NOTRUN -> [SKIP][9] ([fdo#111827]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@kms_chamelium@vga-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-adlp-4:         NOTRUN -> [SKIP][10] ([i915#4103]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-adlp-4:         NOTRUN -> [SKIP][11] ([fdo#109285])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@cursor_plane_move:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][12] ([fdo#109271]) +13 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-bdw-5557u/igt@kms_psr@cursor_plane_move.html

  * igt@prime_vgem@basic-fence-read:
    - bat-adlp-4:         NOTRUN -> [SKIP][13] ([i915#3291] / [i915#3708]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-userptr:
    - bat-adlp-4:         NOTRUN -> [SKIP][14] ([i915#3301] / [i915#3708])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-4/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-blb-e6850:       NOTRUN -> [FAIL][15] ([fdo#109271] / [i915#2403] / [i915#4312])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-blb-e6850/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - {bat-adlp-6}:       [DMESG-WARN][16] ([i915#3576]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-skl-6600u:       [FAIL][18] ([i915#4312]) -> [FAIL][19] ([i915#2722] / [i915#4312])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/fi-skl-6600u/igt@runner@aborted.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/fi-skl-6600u/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5068]: https://gitlab.freedesktop.org/drm/intel/issues/5068
  [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127


Build changes
-------------

  * Linux: CI_DRM_11308 -> Patchwork_22462

  CI-20190529: 20190529
  CI_DRM_11308: e1345b708d76052ef668aaba362ef8ad81ed0b9b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6361: 2372a4beb6a33c5f0799a4a8ccbb93794f52dbca @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22462: dc594b31f5d255a9fb14fb082a5573de4c52cd61 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

dc594b31f5d2 drm/i915/gem: Remove some unnecessary code
0a77c40b2518 drm/i915: Remove the vma refcount
67a198829865 drm/i915: Remove the vm open count

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/index.html

[-- Attachment #2: Type: text/html, Size: 8015 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for vm- and vma cleanups

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 30241 bytes --]

== Series Details ==

Series: vm- and vma cleanups
URL   : https://patchwork.freedesktop.org/series/100945/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11308_full -> Patchwork_22462_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (13 -> 13)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22462_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_cursor_legacy@pipe-a-single-move:
    - {shard-rkl}:        ([PASS][1], [PASS][2]) -> [INCOMPLETE][3]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-rkl-4/igt@kms_cursor_legacy@pipe-a-single-move.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-rkl-2/igt@kms_cursor_legacy@pipe-a-single-move.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-rkl-5/igt@kms_cursor_legacy@pipe-a-single-move.html

  * {igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-a-edp-1-downscale-with-pixel-format}:
    - shard-iclb:         NOTRUN -> [SKIP][4] +2 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-a-edp-1-downscale-with-pixel-format.html

  * {igt@kms_plane_scaling@downscale-with-rotation-factor-0-75@pipe-b-hdmi-a-3-downscale-with-rotation}:
    - {shard-dg1}:        NOTRUN -> [SKIP][5] +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-dg1-18/igt@kms_plane_scaling@downscale-with-rotation-factor-0-75@pipe-b-hdmi-a-3-downscale-with-rotation.html

  * {igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-a-edp-1-planes-upscale-downscale}:
    - shard-iclb:         [PASS][6] -> [SKIP][7] +5 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-iclb6/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-a-edp-1-planes-upscale-downscale.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb2/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-a-edp-1-planes-upscale-downscale.html

  * {igt@kms_plane_scaling@scaler-with-rotation-unity-scaling}:
    - {shard-rkl}:        NOTRUN -> [SKIP][8] +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-rkl-1/igt@kms_plane_scaling@scaler-with-rotation-unity-scaling.html

  
New tests
---------

  New tests have been introduced between CI_DRM_11308_full and Patchwork_22462_full:

### New IGT tests (1) ###

  * igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-75@pipe-d-edp-1-planes-upscale-downscale:
    - Statuses : 1 pass(s)
    - Exec time: [1.28] s

  

Known issues
------------

  Here are the changes found in Patchwork_22462_full that come from known issues:

### CI changes ###

#### Issues hit ####

  * boot:
    - shard-glk:          ([PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33]) -> ([PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [FAIL][50], [PASS][51], [PASS][52], [PASS][53], [PASS][54], [PASS][55], [PASS][56], [PASS][57], [PASS][58]) ([i915#4392])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk9/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk9/boot.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk9/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk8/boot.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk8/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk8/boot.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk7/boot.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk7/boot.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk7/boot.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk6/boot.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk6/boot.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk5/boot.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk5/boot.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk5/boot.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk4/boot.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk4/boot.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk4/boot.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk3/boot.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk3/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk2/boot.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk2/boot.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk2/boot.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk1/boot.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk1/boot.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk1/boot.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk5/boot.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk5/boot.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk7/boot.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk5/boot.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk4/boot.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk4/boot.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk7/boot.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk4/boot.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk3/boot.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk3/boot.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk3/boot.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk8/boot.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk2/boot.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk8/boot.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk2/boot.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk8/boot.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk9/boot.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk2/boot.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk9/boot.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk1/boot.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk6/boot.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk1/boot.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk9/boot.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk6/boot.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk6/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@preservation-s3@bcs0:
    - shard-apl:          [PASS][59] -> [DMESG-WARN][60] ([i915#180]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-apl3/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl4/igt@gem_ctx_isolation@preservation-s3@bcs0.html

  * igt@gem_ctx_param@set-priority-not-supported:
    - shard-iclb:         NOTRUN -> [SKIP][61] ([fdo#109314])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@gem_ctx_param@set-priority-not-supported.html

  * igt@gem_exec_balancer@parallel-bb-first:
    - shard-tglb:         NOTRUN -> [DMESG-WARN][62] ([i915#5076])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@gem_exec_balancer@parallel-bb-first.html

  * igt@gem_exec_capture@pi@bcs0:
    - shard-iclb:         [PASS][63] -> [INCOMPLETE][64] ([i915#3371])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-iclb7/igt@gem_exec_capture@pi@bcs0.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb1/igt@gem_exec_capture@pi@bcs0.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-skl:          NOTRUN -> [FAIL][65] ([i915#2846])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [PASS][66] -> [FAIL][67] ([i915#2842])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-iclb4/igt@gem_exec_fair@basic-none-share@rcs0.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb2/igt@gem_exec_fair@basic-none-share@rcs0.html
    - shard-glk:          [PASS][68] -> [FAIL][69] ([i915#2842])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk5/igt@gem_exec_fair@basic-none-share@rcs0.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk7/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-iclb:         NOTRUN -> [FAIL][70] ([i915#2842]) +3 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-kbl:          [PASS][71] -> [FAIL][72] ([i915#2842])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-kbl7/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl1/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_lmem_swapping@basic:
    - shard-iclb:         NOTRUN -> [SKIP][73] ([i915#4613]) +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@heavy-verify-random:
    - shard-skl:          NOTRUN -> [SKIP][74] ([fdo#109271] / [i915#4613]) +2 similar issues
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@gem_lmem_swapping@heavy-verify-random.html

  * igt@gem_lmem_swapping@parallel-random-verify:
    - shard-glk:          NOTRUN -> [SKIP][75] ([fdo#109271] / [i915#4613])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk9/igt@gem_lmem_swapping@parallel-random-verify.html

  * igt@gem_pread@exhaustion:
    - shard-skl:          NOTRUN -> [WARN][76] ([i915#2658])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@gem_pread@exhaustion.html

  * igt@gem_pxp@create-regular-context-1:
    - shard-iclb:         NOTRUN -> [SKIP][77] ([i915#4270]) +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@gem_pxp@create-regular-context-1.html

  * igt@gem_pxp@create-regular-context-2:
    - shard-tglb:         NOTRUN -> [SKIP][78] ([i915#4270])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@gem_pxp@create-regular-context-2.html

  * igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs:
    - shard-iclb:         NOTRUN -> [SKIP][79] ([i915#768]) +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-skl:          NOTRUN -> [SKIP][80] ([fdo#109271] / [i915#3323])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@input-checking:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][81] ([i915#4991])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl7/igt@gem_userptr_blits@input-checking.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-iclb:         NOTRUN -> [SKIP][82] ([i915#3297]) +1 similar issue
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@gen7_exec_parse@batch-without-end:
    - shard-iclb:         NOTRUN -> [SKIP][83] ([fdo#109289]) +2 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@gen7_exec_parse@batch-without-end.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-glk:          [PASS][84] -> [DMESG-WARN][85] ([i915#1436] / [i915#716])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-glk9/igt@gen9_exec_parse@allowed-single.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk6/igt@gen9_exec_parse@allowed-single.html

  * igt@gen9_exec_parse@batch-zero-length:
    - shard-iclb:         NOTRUN -> [SKIP][86] ([i915#2856])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@gen9_exec_parse@batch-zero-length.html

  * igt@gen9_exec_parse@shadow-peek:
    - shard-tglb:         NOTRUN -> [SKIP][87] ([i915#2527] / [i915#2856]) +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb8/igt@gen9_exec_parse@shadow-peek.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          NOTRUN -> [FAIL][88] ([i915#454])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-iclb:         NOTRUN -> [SKIP][89] ([fdo#109293] / [fdo#109506])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html

  * igt@i915_pm_sseu@full-enable:
    - shard-iclb:         NOTRUN -> [SKIP][90] ([i915#4387])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@i915_pm_sseu@full-enable.html

  * igt@i915_suspend@debugfs-reader:
    - shard-skl:          [PASS][91] -> [INCOMPLETE][92] ([i915#4939]) +1 similar issue
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-skl10/igt@i915_suspend@debugfs-reader.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl4/igt@i915_suspend@debugfs-reader.html

  * igt@kms_async_flips@crc:
    - shard-skl:          NOTRUN -> [FAIL][93] ([i915#4272])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@kms_async_flips@crc.html

  * igt@kms_atomic@plane-primary-overlay-mutable-zpos:
    - shard-tglb:         NOTRUN -> [SKIP][94] ([i915#404])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_atomic@plane-primary-overlay-mutable-zpos.html

  * igt@kms_big_fb@linear-8bpp-rotate-270:
    - shard-iclb:         NOTRUN -> [SKIP][95] ([fdo#110725] / [fdo#111614]) +1 similar issue
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_big_fb@linear-8bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][96] ([i915#3743])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-async-flip.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
    - shard-apl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#3777]) +2 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
    - shard-skl:          NOTRUN -> [SKIP][98] ([fdo#109271] / [i915#3777]) +2 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html

  * igt@kms_big_fb@yf-tiled-64bpp-rotate-0:
    - shard-tglb:         NOTRUN -> [SKIP][99] ([fdo#111615])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_big_fb@yf-tiled-64bpp-rotate-0.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180:
    - shard-iclb:         NOTRUN -> [SKIP][100] ([fdo#110723]) +1 similar issue
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-kbl:          NOTRUN -> [SKIP][101] ([fdo#109271] / [i915#3886]) +1 similar issue
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl4/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][102] ([i915#3689])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_ccs.html

  * igt@kms_ccs@pipe-b-crc-primary-rotation-180-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][103] ([fdo#111615] / [i915#3689]) +1 similar issue
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_ccs@pipe-b-crc-primary-rotation-180-yf_tiled_ccs.html

  * igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
    - shard-iclb:         NOTRUN -> [SKIP][104] ([fdo#109278] / [i915#3886]) +1 similar issue
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][105] ([fdo#109271] / [i915#3886]) +3 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl1/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][106] ([fdo#109271] / [i915#3886]) +3 similar issues
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@hdmi-hpd-enable-disable-mode:
    - shard-kbl:          NOTRUN -> [SKIP][107] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl7/igt@kms_chamelium@hdmi-hpd-enable-disable-mode.html

  * igt@kms_chamelium@vga-hpd-after-suspend:
    - shard-apl:          NOTRUN -> [SKIP][108] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_chamelium@vga-hpd-after-suspend.html

  * igt@kms_color_chamelium@pipe-a-ctm-blue-to-red:
    - shard-iclb:         NOTRUN -> [SKIP][109] ([fdo#109284] / [fdo#111827]) +7 similar issues
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html

  * igt@kms_color_chamelium@pipe-b-ctm-limited-range:
    - shard-skl:          NOTRUN -> [SKIP][110] ([fdo#109271] / [fdo#111827]) +10 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@kms_color_chamelium@pipe-b-ctm-limited-range.html

  * igt@kms_color_chamelium@pipe-b-ctm-red-to-blue:
    - shard-glk:          NOTRUN -> [SKIP][111] ([fdo#109271] / [fdo#111827])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-glk9/igt@kms_color_chamelium@pipe-b-ctm-red-to-blue.html

  * igt@kms_content_protection@lic:
    - shard-apl:          NOTRUN -> [TIMEOUT][112] ([i915#1319]) +1 similar issue
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_content_protection@lic.html

  * igt@kms_content_protection@type1:
    - shard-iclb:         NOTRUN -> [SKIP][113] ([fdo#109300] / [fdo#111066])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_content_protection@type1.html

  * igt@kms_cursor_crc@pipe-a-cursor-512x170-offscreen:
    - shard-tglb:         NOTRUN -> [SKIP][114] ([fdo#109279] / [i915#3359]) +1 similar issue
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-512x170-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-max-size-onscreen:
    - shard-kbl:          NOTRUN -> [SKIP][115] ([fdo#109271]) +48 similar issues
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-max-size-onscreen.html

  * igt@kms_cursor_crc@pipe-b-cursor-512x170-sliding:
    - shard-iclb:         NOTRUN -> [SKIP][116] ([fdo#109278] / [fdo#109279]) +1 similar issue
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_cursor_crc@pipe-b-cursor-512x170-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][117] ([i915#180])
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-d-cursor-256x85-rapid-movement:
    - shard-iclb:         NOTRUN -> [SKIP][118] ([fdo#109278]) +26 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_cursor_crc@pipe-d-cursor-256x85-rapid-movement.html

  * igt@kms_cursor_legacy@cursora-vs-flipb-atomic:
    - shard-tglb:         NOTRUN -> [SKIP][119] ([fdo#109274] / [fdo#111825]) +2 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_cursor_legacy@cursora-vs-flipb-atomic.html

  * igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
    - shard-iclb:         NOTRUN -> [SKIP][120] ([fdo#109274] / [fdo#109278]) +2 similar issues
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@kms_cursor_legacy@cursorb-vs-flipb-toggle.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          NOTRUN -> [FAIL][121] ([i915#2346])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][122] -> [FAIL][123] ([i915#2346] / [i915#533])
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@pipe-d-torture-move:
    - shard-skl:          NOTRUN -> [SKIP][124] ([fdo#109271]) +107 similar issues
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@kms_cursor_legacy@pipe-d-torture-move.html

  * igt@kms_flip@2x-flip-vs-panning-vs-hang:
    - shard-iclb:         NOTRUN -> [SKIP][125] ([fdo#109274]) +4 similar issues
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@kms_flip@2x-flip-vs-panning-vs-hang.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [PASS][126] -> [FAIL][127] ([i915#79])
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-apl:          NOTRUN -> [DMESG-WARN][128] ([i915#180]) +1 similar issue
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-kbl:          [PASS][129] -> [DMESG-WARN][130] ([i915#180]) +1 similar issue
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-kbl1/igt@kms_flip@flip-vs-suspend@a-dp1.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl6/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling:
    - shard-skl:          NOTRUN -> [INCOMPLETE][131] ([i915#3701])
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
    - shard-iclb:         [PASS][132] -> [SKIP][133] ([i915#3701])
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-iclb4/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-blt:
    - shard-tglb:         NOTRUN -> [SKIP][134] ([fdo#109280] / [fdo#111825]) +3 similar issues
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-shrfb-fliptrack-mmap-gtt:
    - shard-iclb:         NOTRUN -> [SKIP][135] ([fdo#109280]) +22 similar issues
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-2p-shrfb-fliptrack-mmap-gtt.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][136] -> [FAIL][137] ([i915#1188])
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl10/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-apl:          NOTRUN -> [SKIP][138] ([fdo#109271] / [i915#533])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-apl:          NOTRUN -> [FAIL][139] ([fdo#108145] / [i915#265])
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> [FAIL][140] ([fdo#108145] / [i915#265])
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-kbl4/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> [FAIL][141] ([fdo#108145] / [i915#265]) +2 similar issues
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][142] -> [FAIL][143] ([fdo#108145] / [i915#265]) +1 similar issue
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11308/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         NOTRUN -> [SKIP][144] ([i915#3536]) +1 similar issue
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_plane_lowres@pipe-b-tiling-yf:
    - shard-tglb:         NOTRUN -> [SKIP][145] ([fdo#111615] / [fdo#112054])
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_plane_lowres@pipe-b-tiling-yf.html

  * igt@kms_plane_lowres@pipe-c-tiling-none:
    - shard-tglb:         NOTRUN -> [SKIP][146] ([i915#3536]) +1 similar issue
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-tglb5/igt@kms_plane_lowres@pipe-c-tiling-none.html

  * igt@kms_psr2_sf@cursor-plane-update-sf:
    - shard-skl:          NOTRUN -> [SKIP][147] ([fdo#109271] / [i915#658])
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-skl9/igt@kms_psr2_sf@cursor-plane-update-sf.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area:
    - shard-apl:          NOTRUN -> [SKIP][148] ([fdo#109271] / [i915#658])
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area:
    - shard-iclb:         NOTRUN -> [SKIP][149] ([fdo#111068] / [i915#658])
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-iclb5/igt@kms_psr2_sf@plane-move-sf-dmg-area.html

  * igt@kms_psr@basic:
    - shard-apl:          NOTRUN -> [SKIP][150] ([fdo#109271]) +47 similar issues
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/shard-apl1/igt@kms_psr@basic.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         [PASS][151] -> [SKIP][152] ([fdo#109441])

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22462/index.html

[-- Attachment #2: Type: text/html, Size: 33715 bytes --]