[PATCH 2/2] try5, cleaner

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Tue Jun 15 10:59:32 UTC 2021


Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.c       | 37 +++++++----
 drivers/gpu/drm/i915/gt/intel_context.h       | 11 +++-
 drivers/gpu/drm/i915/gt/intel_context_param.c | 26 +++++---
 drivers/gpu/drm/i915/gt/intel_context_sseu.c  |  2 +-
 drivers/gpu/drm/i915/gt/intel_context_types.h |  5 +-
 .../drm/i915/gt/intel_execlists_submission.c  | 12 ++--
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 63 +++++++------------
 drivers/gpu/drm/i915/gt/intel_lrc.h           |  4 +-
 .../gpu/drm/i915/gt/intel_ring_submission.c   | 27 ++------
 drivers/gpu/drm/i915/gt/mock_engine.c         |  4 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 +-
 11 files changed, 94 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 705c781f4375..e6dab37c4266 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -51,18 +51,30 @@ intel_context_create(struct intel_engine_cs *engine)
 
 int intel_context_alloc_state(struct intel_context *ce)
 {
-	int err;
+	int err = 0;
 
 	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags))
 		return 0;
 
-	if (intel_context_is_banned(ce))
-		return -EIO;
+	if (mutex_lock_interruptible(&ce->alloc_mutex))
+		return -EINTR;
 
-	err = ce->ops->alloc_state(ce);
-	if (!err)
-		GEM_WARN_ON(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags));
+	if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
+		if (intel_context_is_banned(ce)) {
+			err = -EIO;
+			goto unlock;
+		}
+
+		err = ce->ops->alloc(ce);
+		if (unlikely(err))
+			goto unlock;
+
+		smp_mb__before_atomic();
+		set_bit(CONTEXT_ALLOC_BIT, &ce->flags);
+	}
 
+unlock:
+	mutex_unlock(&ce->alloc_mutex);
 	return err;
 }
 
@@ -197,13 +209,11 @@ int __intel_context_do_pin_ww(struct intel_context *ce,
 {
 	bool handoff = false;
 	void *vaddr;
-	int err = 0;
+	int err;
 
-	if (unlikely(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags))) {
-		err = intel_context_alloc_state(ce);
-		if (err)
-			return err;
-	}
+	err = intel_context_alloc_state(ce);
+	if (err)
+		return err;
 
 	err = i915_gem_object_lock(ce->timeline->hwsp_ggtt->obj, ww);
 	if (!err && ce->ring->vma->obj)
@@ -361,6 +371,8 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine)
 	spin_lock_init(&ce->signal_lock);
 	INIT_LIST_HEAD(&ce->signals);
 
+	mutex_init(&ce->alloc_mutex);
+
 	i915_active_init(&ce->active,
 			 __intel_context_active, __intel_context_retire, 0);
 }
@@ -371,6 +383,7 @@ void intel_context_fini(struct intel_context *ce)
 		intel_timeline_put(ce->timeline);
 	i915_vm_put(ce->vm);
 
+	mutex_destroy(&ce->alloc_mutex);
 	i915_active_fini(&ce->active);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 0de56c50e828..27865f8bcecb 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -49,14 +49,16 @@ int intel_context_reconfigure_sseu(struct intel_context *ce,
  * intel_context_is_pinned() remains stable.
  */
 static inline int intel_context_lock_pinned(struct intel_context *ce)
-	__acquires(&ce->state->obj->base.resv->lock)
 {
 	int ret = intel_context_alloc_state(ce);
 
 	if (ret)
 		return ret;
 
-	return i915_gem_object_lock_interruptible(ce->state->obj, NULL);
+	if (ce->state)
+		return i915_gem_object_lock_interruptible(ce->state->obj, NULL);
+	else
+		return i915_gem_object_lock_interruptible(ce->timeline->hwsp_ggtt->obj, NULL);
 }
 
 /**
@@ -83,7 +85,10 @@ intel_context_is_pinned(struct intel_context *ce)
 static inline void intel_context_unlock_pinned(struct intel_context *ce)
 	__releases(&ce->state->obj->base.resv->lock)
 {
-	i915_gem_object_unlock(ce->state->obj);
+	if (ce->state)
+		return i915_gem_object_unlock(ce->state->obj);
+	else
+		return i915_gem_object_unlock(ce->timeline->hwsp_ggtt->obj);
 }
 
 int __intel_context_do_pin(struct intel_context *ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.c b/drivers/gpu/drm/i915/gt/intel_context_param.c
index 36819baa3377..65dcd090245d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_param.c
+++ b/drivers/gpu/drm/i915/gt/intel_context_param.c
@@ -10,7 +10,6 @@
 
 int intel_context_set_ring_size(struct intel_context *ce, long sz)
 {
-	struct intel_ring *ring;
 	int err;
 
 	if (intel_context_lock_pinned(ce))
@@ -25,16 +24,23 @@ int intel_context_set_ring_size(struct intel_context *ce, long sz)
 		goto unlock;
 	}
 
-	/* Replace the existing ringbuffer */
-	ring = intel_engine_create_ring(ce->engine, sz);
-	if (IS_ERR(ring)) {
-		err = PTR_ERR(ring);
-		goto unlock;
-	}
+	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
+		struct intel_ring *ring;
 
-	intel_ring_put(ce->ring);
-	ce->ring = ring;
-	/* Context image will be updated on next pin */
+		/* Replace the existing ringbuffer */
+		ring = intel_engine_create_ring(ce->engine, sz);
+		if (IS_ERR(ring)) {
+			err = PTR_ERR(ring);
+			goto unlock;
+		}
+
+		intel_ring_put(ce->ring);
+		ce->ring = ring;
+
+		/* Context image will be updated on next pin */
+	} else {
+		ce->ring = __intel_context_ring_size(sz);
+	}
 
 unlock:
 	intel_context_unlock_pinned(ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_sseu.c b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
index e86d8255feec..393ddf57983f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
@@ -42,7 +42,7 @@ gen8_modify_rpcs(struct intel_context *ce, const struct intel_sseu sseu)
 	struct i915_request *rq;
 	int ret;
 
-	lockdep_assert_held(&ce->pin_mutex);
+	assert_object_held(ce->state->obj);;
 
 	/*
 	 * If the context is not idle, we have to submit an ordered request to
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index e6e57566adf0..f5d45c8c96ee 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -33,8 +33,7 @@ struct intel_context_ops {
 #define COPS_HAS_INFLIGHT_BIT 0
 #define COPS_HAS_INFLIGHT BIT(COPS_HAS_INFLIGHT_BIT)
 
-	/* alloc_state() may be called multiple times without lock held, so have to careful here */
-	int (*alloc_state)(struct intel_context *ce);
+	int (*alloc)(struct intel_context *ce);
 
 	int (*pre_pin)(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void **vaddr);
 	int (*pin)(struct intel_context *ce, void *vaddr);
@@ -123,7 +122,7 @@ struct intel_context {
 	unsigned int active_count; /* protected by timeline->mutex */
 
 	atomic_t pin_count;
-	struct mutex pin_mutex; /* guards pinning and associated on-gpuing */
+	struct mutex alloc_mutex; /* guards allocation-gpuing */
 
 	/**
 	 * active: Active tracker for the rq activity (inc. external) on this
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 4515985e6474..fc77592d88a9 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -2528,15 +2528,15 @@ static int execlists_context_pin(struct intel_context *ce, void *vaddr)
 	return lrc_pin(ce, ce->engine, vaddr);
 }
 
-static int execlists_context_alloc_state(struct intel_context *ce)
+static int execlists_context_alloc(struct intel_context *ce)
 {
-	return lrc_alloc_state(ce, ce->engine);
+	return lrc_alloc(ce, ce->engine);
 }
 
 static const struct intel_context_ops execlists_context_ops = {
 	.flags = COPS_HAS_INFLIGHT,
 
-	.alloc_state = execlists_context_alloc_state,
+	.alloc = execlists_context_alloc,
 
 	.pre_pin = execlists_context_pre_pin,
 	.pin = execlists_context_pin,
@@ -3394,11 +3394,11 @@ static void virtual_engine_initial_hint(struct virtual_engine *ve)
 		swap(ve->siblings[swp], ve->siblings[0]);
 }
 
-static int virtual_context_alloc_state(struct intel_context *ce)
+static int virtual_context_alloc(struct intel_context *ce)
 {
 	struct virtual_engine *ve = container_of(ce, typeof(*ve), context);
 
-	return lrc_alloc_state(ce, ve->siblings[0]);
+	return lrc_alloc(ce, ve->siblings[0]);
 }
 
 static int virtual_context_pre_pin(struct intel_context *ce,
@@ -3443,7 +3443,7 @@ static void virtual_context_exit(struct intel_context *ce)
 static const struct intel_context_ops virtual_context_ops = {
 	.flags = COPS_HAS_INFLIGHT,
 
-	.alloc_state = virtual_context_alloc_state,
+	.alloc = virtual_context_alloc,
 
 	.pre_pin = virtual_context_pre_pin,
 	.pin = virtual_context_pin,
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c94a193bc412..a27bac0a4bfb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -806,8 +806,7 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine)
 		context_size += I915_GTT_PAGE_SIZE; /* for redzone */
 
 	if (GRAPHICS_VER(engine->i915) == 12) {
-		/* harmless to repeatedly write, since this is always the same */
-		WRITE_ONCE(ce->wa_bb_page, context_size / PAGE_SIZE);
+		ce->wa_bb_page = context_size / PAGE_SIZE;
 		context_size += PAGE_SIZE;
 	}
 
@@ -834,35 +833,23 @@ pinned_timeline(struct intel_context *ce, struct intel_engine_cs *engine)
 	return intel_timeline_create_from_engine(engine, page_unmask_bits(tl));
 }
 
-int lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine)
+int lrc_alloc(struct intel_context *ce, struct intel_engine_cs *engine)
 {
 	struct intel_ring *ring;
-	struct i915_vma *vma = NULL, *old_vma;
+	struct i915_vma *vma;
 	int err;
 
-	vma = READ_ONCE(ce->state);
-	if (likely(!vma)) {
-		vma = __lrc_alloc_state(ce, engine);
-		if (IS_ERR(vma))
-			return PTR_ERR(vma);
-
-		old_vma = cmpxchg(&ce->state, NULL, vma);
-		if (old_vma) {
-			i915_vma_put(vma);
-			vma = old_vma;
-		}
-	}
-
-	err = i915_gem_object_lock_interruptible(vma->obj, NULL);
-	if (err)
-		return err;
+	GEM_BUG_ON(ce->state);
 
-	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags))
-		goto err_unlock;
+	vma = __lrc_alloc_state(ce, engine);
+	if (IS_ERR(vma))
+		return PTR_ERR(vma);
 
 	ring = intel_engine_create_ring(engine, (unsigned long)ce->ring);
-	if (IS_ERR(ring))
-		return PTR_ERR(ring);
+	if (IS_ERR(ring)) {
+		err = PTR_ERR(ring);
+		goto err_vma;
+	}
 
 	if (!page_mask_bits(ce->timeline)) {
 		struct intel_timeline *tl;
@@ -875,23 +862,23 @@ int lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine)
 			tl = pinned_timeline(ce, engine);
 		else
 			tl = intel_timeline_create(engine->gt);
-
-		if (IS_ERR(tl))
+		if (IS_ERR(tl)) {
 			err = PTR_ERR(tl);
-		else
-			ce->timeline = tl;
+			goto err_ring;
+		}
+
+		ce->timeline = tl;
 	}
 
-	if (!err)
-		ce->ring = ring;
-	else
-		intel_ring_put(ring);
+	ce->ring = ring;
+	ce->state = vma;
 
-	smp_mb__before_atomic();
-	set_bit(CONTEXT_ALLOC_BIT, &ce->flags);
+	return 0;
 
-err_unlock:
-	i915_gem_object_unlock(vma->obj);
+err_ring:
+	intel_ring_put(ring);
+err_vma:
+	i915_vma_put(vma);
 	return err;
 }
 
@@ -954,10 +941,8 @@ void lrc_fini(struct intel_context *ce)
 	if (!ce->state)
 		return;
 
+	intel_ring_put(fetch_and_zero(&ce->ring));
 	i915_vma_put(fetch_and_zero(&ce->state));
-
-	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags))
-		intel_ring_put(fetch_and_zero(&ce->ring));
 }
 
 void lrc_destroy(struct kref *kref)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/intel_lrc.h
index 60d5e1113987..7f697845c4cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
@@ -29,8 +29,8 @@ struct intel_ring;
 void lrc_init_wa_ctx(struct intel_engine_cs *engine);
 void lrc_fini_wa_ctx(struct intel_engine_cs *engine);
 
-int lrc_alloc_state(struct intel_context *ce,
-		    struct intel_engine_cs *engine);
+int lrc_alloc(struct intel_context *ce,
+	      struct intel_engine_cs *engine);
 void lrc_reset(struct intel_context *ce);
 void lrc_fini(struct intel_context *ce);
 void lrc_destroy(struct kref *kref);
diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
index 0a7e52bf7169..0c423f096e2b 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
@@ -549,22 +549,12 @@ alloc_context_vma(struct intel_engine_cs *engine)
 	return ERR_PTR(err);
 }
 
-static int ring_context_alloc_state(struct intel_context *ce)
+static int ring_context_alloc(struct intel_context *ce)
 {
 	struct intel_engine_cs *engine = ce->engine;
-	int err;
-
-	err = i915_gem_object_lock(engine->legacy.timeline->hwsp_ggtt->obj, NULL);
-	if (err)
-		return err;
 
 	/* One ringbuffer to rule them all */
 	GEM_BUG_ON(!engine->legacy.ring);
-
-	/* only run once */
-	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags))
-		goto err_unlock;
-
 	ce->ring = engine->legacy.ring;
 	ce->timeline = intel_timeline_get(engine->legacy.timeline);
 
@@ -573,20 +563,13 @@ static int ring_context_alloc_state(struct intel_context *ce)
 		struct i915_vma *vma;
 
 		vma = alloc_context_vma(engine);
-		if (IS_ERR(vma)) {
-			err = PTR_ERR(vma);
-			goto err_unlock;
-		}
+		if (IS_ERR(vma))
+			return PTR_ERR(vma);
 
 		ce->state = vma;
 	}
 
-	smp_mb__before_atomic();
-	set_bit(CONTEXT_ALLOC_BIT, &ce->flags);
-
-err_unlock:
-	i915_gem_object_unlock(engine->legacy.timeline->hwsp_ggtt->obj);
-	return err;
+	return 0;
 }
 
 static int ring_context_pin(struct intel_context *ce, void *unused)
@@ -601,7 +584,7 @@ static void ring_context_reset(struct intel_context *ce)
 }
 
 static const struct intel_context_ops ring_context_ops = {
-	.alloc_state = ring_context_alloc_state,
+	.alloc = ring_context_alloc,
 
 	.pre_pin = ring_context_pre_pin,
 	.pin = ring_context_pin,
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index cf77c3197bb3..32589c6625e1 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -142,7 +142,7 @@ static void mock_context_destroy(struct kref *ref)
 	intel_context_free(ce);
 }
 
-static int mock_context_alloc_state(struct intel_context *ce)
+static int mock_context_alloc(struct intel_context *ce)
 {
 	int err;
 
@@ -182,7 +182,7 @@ static void mock_context_reset(struct intel_context *ce)
 }
 
 static const struct intel_context_ops mock_context_ops = {
-	.alloc_state = mock_context_alloc_state,
+	.alloc = mock_context_alloc,
 
 	.pre_pin = mock_context_pre_pin,
 	.pin = mock_context_pin,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index e585d108140a..7c8ff9792f7b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -432,9 +432,9 @@ void intel_guc_submission_fini(struct intel_guc *guc)
 	}
 }
 
-static int guc_context_alloc_state(struct intel_context *ce)
+static int guc_context_alloc(struct intel_context *ce)
 {
-	return lrc_alloc_state(ce, ce->engine);
+	return lrc_alloc(ce, ce->engine);
 }
 
 static int guc_context_pre_pin(struct intel_context *ce,
@@ -450,7 +450,7 @@ static int guc_context_pin(struct intel_context *ce, void *vaddr)
 }
 
 static const struct intel_context_ops guc_context_ops = {
-	.alloc_state = guc_context_alloc_state,
+	.alloc = guc_context_alloc,
 
 	.pre_pin = guc_context_pre_pin,
 	.pin = guc_context_pin,
-- 
2.31.0



More information about the Intel-gfx-trybot mailing list