drm/i915: replace in_atomic() with manually set flag

Commit Message

Krzysztof Karas Jan. 10, 2025, 3:46 p.m. UTC

GuC code uses in_atomic() function to determine if the current
context is atomic. As noted by the function's description it
should not be used in driver code, as it is not guaranteed to
determine atomicity correctly in every instance. This is also
pointed out by the FIXME note suggesting that the caller of
intel_guc_send_busy_loop() should be responsible for providing
information about whether the context is atomic or not to avoid
in_atomic() usage there.

Add a new flag to indicate active spin locks and pass it down to
intel_guc_send_busy_loop(), similarly to how "loop" flag is
handled.

Signed-off-by: Krzysztof Karas <[e-mail]>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |  12 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c    |   2 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 127 ++++++++++--------
 3 files changed, 74 insertions(+), 67 deletions(-)

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 57b903132776..0ccd97dc32f7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -358,18 +358,12 @@  static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
 					   const u32 *action,
 					   u32 len,
 					   u32 g2h_len_dw,
-					   bool loop)
+					   bool loop,
+					   bool locked)
 {
 	int err;
 	unsigned int sleep_period_ms = 1;
-	bool not_atomic = !in_atomic() && !irqs_disabled();
-
-	/*
-	 * FIXME: Have caller pass in if we are in an atomic context to avoid
-	 * using in_atomic(). It is likely safe here as we check for irqs
-	 * disabled which basically all the spin locks in the i915 do but
-	 * regardless this should be cleaned up.
-	 */
+	bool not_atomic = !locked && !irqs_disabled();
 
 	/* No sleeping with spin locks, just busy loop */
 	might_sleep_if(loop && not_atomic);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index 46fabbfc775e..6d7ec67e8f58 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -198,7 +198,7 @@  static int guc_action_policies_update(struct intel_guc *guc, u32 policy_offset)
 		policy_offset
 	};
 
-	return intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true);
+	return intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true, false);
 }
 
 int intel_guc_global_policies_update(struct intel_guc *guc)
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 12f1ba7ca9c1..2154718d278c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -618,7 +618,8 @@  static int guc_submission_send_busy_loop(struct intel_guc *guc,
 					 const u32 *action,
 					 u32 len,
 					 u32 g2h_len_dw,
-					 bool loop)
+					 bool loop,
+					 bool locked)
 {
 	int ret;
 
@@ -632,7 +633,7 @@  static int guc_submission_send_busy_loop(struct intel_guc *guc,
 	if (g2h_len_dw)
 		atomic_inc(&guc->outstanding_submission_g2h);
 
-	ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+	ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop, locked);
 	if (ret)
 		atomic_dec(&guc->outstanding_submission_g2h);
 
@@ -690,8 +691,8 @@  int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout)
 					      true, timeout);
 }
 
-static int guc_context_policy_init_v70(struct intel_context *ce, bool loop);
-static int try_context_registration(struct intel_context *ce, bool loop);
+static int guc_context_policy_init_v70(struct intel_context *ce, bool loop, bool locked);
+static int try_context_registration(struct intel_context *ce, bool loop, bool locked);
 
 static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq)
 {
@@ -718,7 +719,7 @@  static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq)
 	GEM_BUG_ON(context_guc_id_invalid(ce));
 
 	if (context_policy_required(ce)) {
-		err = guc_context_policy_init_v70(ce, false);
+		err = guc_context_policy_init_v70(ce, false, true);
 		if (err)
 			return err;
 	}
@@ -991,7 +992,7 @@  static int guc_dequeue_one_context(struct intel_guc *guc)
 
 		if (unlikely(!ctx_id_mapped(guc, ce->guc_id.id) &&
 			     intel_context_is_schedulable(ce))) {
-			ret = try_context_registration(ce, false);
+			ret = try_context_registration(ce, false, true);
 			if (unlikely(ret == -EPIPE)) {
 				goto deadlk;
 			} else if (ret == -EBUSY) {
@@ -2445,7 +2446,8 @@  static int __guc_action_register_multi_lrc_v69(struct intel_guc *guc,
 					       struct intel_context *ce,
 					       u32 guc_id,
 					       u32 offset,
-					       bool loop)
+					       bool loop,
+					       bool locked)
 {
 	struct intel_context *child;
 	u32 action[4 + MAX_ENGINE_INSTANCE];
@@ -2462,13 +2464,14 @@  static int __guc_action_register_multi_lrc_v69(struct intel_guc *guc,
 		action[len++] = offset;
 	}
 
-	return guc_submission_send_busy_loop(guc, action, len, 0, loop);
+	return guc_submission_send_busy_loop(guc, action, len, 0, loop, locked);
 }
 
 static int __guc_action_register_multi_lrc_v70(struct intel_guc *guc,
 					       struct intel_context *ce,
 					       struct guc_ctxt_registration_info *info,
-					       bool loop)
+					       bool loop,
+					       bool locked)
 {
 	struct intel_context *child;
 	u32 action[13 + (MAX_ENGINE_INSTANCE * 2)];
@@ -2505,13 +2508,14 @@  static int __guc_action_register_multi_lrc_v70(struct intel_guc *guc,
 
 	GEM_BUG_ON(len > ARRAY_SIZE(action));
 
-	return guc_submission_send_busy_loop(guc, action, len, 0, loop);
+	return guc_submission_send_busy_loop(guc, action, len, 0, loop, locked);
 }
 
 static int __guc_action_register_context_v69(struct intel_guc *guc,
 					     u32 guc_id,
 					     u32 offset,
-					     bool loop)
+					     bool loop,
+					     bool locked)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_REGISTER_CONTEXT,
@@ -2520,12 +2524,13 @@  static int __guc_action_register_context_v69(struct intel_guc *guc,
 	};
 
 	return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action),
-					     0, loop);
+					     0, loop, locked);
 }
 
 static int __guc_action_register_context_v70(struct intel_guc *guc,
 					     struct guc_ctxt_registration_info *info,
-					     bool loop)
+					     bool loop,
+					     bool locked)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_REGISTER_CONTEXT,
@@ -2543,7 +2548,7 @@  static int __guc_action_register_context_v70(struct intel_guc *guc,
 	};
 
 	return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action),
-					     0, loop);
+					     0, loop, locked);
 }
 
 static void prepare_context_registration_info_v69(struct intel_context *ce);
@@ -2551,7 +2556,7 @@  static void prepare_context_registration_info_v70(struct intel_context *ce,
 						  struct guc_ctxt_registration_info *info);
 
 static int
-register_context_v69(struct intel_guc *guc, struct intel_context *ce, bool loop)
+register_context_v69(struct intel_guc *guc, struct intel_context *ce, bool loop, bool locked)
 {
 	u32 offset = intel_guc_ggtt_offset(guc, guc->lrc_desc_pool_v69) +
 		ce->guc_id.id * sizeof(struct guc_lrc_desc_v69);
@@ -2560,26 +2565,26 @@  register_context_v69(struct intel_guc *guc, struct intel_context *ce, bool loop)
 
 	if (intel_context_is_parent(ce))
 		return __guc_action_register_multi_lrc_v69(guc, ce, ce->guc_id.id,
-							   offset, loop);
+							   offset, loop, locked);
 	else
 		return __guc_action_register_context_v69(guc, ce->guc_id.id,
-							 offset, loop);
+							 offset, loop, locked);
 }
 
 static int
-register_context_v70(struct intel_guc *guc, struct intel_context *ce, bool loop)
+register_context_v70(struct intel_guc *guc, struct intel_context *ce, bool loop, bool locked)
 {
 	struct guc_ctxt_registration_info info;
 
 	prepare_context_registration_info_v70(ce, &info);
 
 	if (intel_context_is_parent(ce))
-		return __guc_action_register_multi_lrc_v70(guc, ce, &info, loop);
+		return __guc_action_register_multi_lrc_v70(guc, ce, &info, loop, locked);
 	else
-		return __guc_action_register_context_v70(guc, &info, loop);
+		return __guc_action_register_context_v70(guc, &info, loop, locked);
 }
 
-static int register_context(struct intel_context *ce, bool loop)
+static int register_context(struct intel_context *ce, bool loop, bool locked)
 {
 	struct intel_guc *guc = ce_to_guc(ce);
 	int ret;
@@ -2588,9 +2593,9 @@  static int register_context(struct intel_context *ce, bool loop)
 	trace_intel_context_register(ce);
 
 	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
-		ret = register_context_v70(guc, ce, loop);
+		ret = register_context_v70(guc, ce, loop, locked);
 	else
-		ret = register_context_v69(guc, ce, loop);
+		ret = register_context_v69(guc, ce, loop, locked);
 
 	if (likely(!ret)) {
 		unsigned long flags;
@@ -2600,14 +2605,15 @@  static int register_context(struct intel_context *ce, bool loop)
 		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
 		if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
-			guc_context_policy_init_v70(ce, loop);
+			guc_context_policy_init_v70(ce, loop, locked);
 	}
 
 	return ret;
 }
 
 static int __guc_action_deregister_context(struct intel_guc *guc,
-					   u32 guc_id)
+					   u32 guc_id,
+					   bool locked)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_DEREGISTER_CONTEXT,
@@ -2616,17 +2622,17 @@  static int __guc_action_deregister_context(struct intel_guc *guc,
 
 	return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action),
 					     G2H_LEN_DW_DEREGISTER_CONTEXT,
-					     true);
+					     true, locked);
 }
 
-static int deregister_context(struct intel_context *ce, u32 guc_id)
+static int deregister_context(struct intel_context *ce, u32 guc_id, bool locked)
 {
 	struct intel_guc *guc = ce_to_guc(ce);
 
 	GEM_BUG_ON(intel_context_is_child(ce));
 	trace_intel_context_deregister(ce);
 
-	return __guc_action_deregister_context(guc, guc_id);
+	return __guc_action_deregister_context(guc, guc_id, locked);
 }
 
 static inline void clear_children_join_go_memory(struct intel_context *ce)
@@ -2691,14 +2697,15 @@  MAKE_CONTEXT_POLICY_ADD(slpc_ctx_freq_req, SLPM_GT_FREQUENCY)
 
 static int __guc_context_set_context_policies(struct intel_guc *guc,
 					      struct context_policy *policy,
-					      bool loop)
+					      bool loop,
+					      bool locked)
 {
 	return guc_submission_send_busy_loop(guc, (u32 *)&policy->h2g,
 					__guc_context_policy_action_size(policy),
-					0, loop);
+					0, loop, locked);
 }
 
-static int guc_context_policy_init_v70(struct intel_context *ce, bool loop)
+static int guc_context_policy_init_v70(struct intel_context *ce, bool loop, bool locked)
 {
 	struct intel_engine_cs *engine = ce->engine;
 	struct intel_guc *guc = gt_to_guc(engine->gt);
@@ -2730,7 +2737,7 @@  static int guc_context_policy_init_v70(struct intel_context *ce, bool loop)
 	if (engine->flags & I915_ENGINE_WANT_FORCED_PREEMPTION)
 		__guc_context_policy_add_preempt_to_idle(&policy, 1);
 
-	ret = __guc_context_set_context_policies(guc, &policy, loop);
+	ret = __guc_context_set_context_policies(guc, &policy, loop, locked);
 
 	spin_lock_irqsave(&ce->guc_state.lock, flags);
 	if (ret != 0)
@@ -2910,7 +2917,7 @@  static void prepare_context_registration_info_v70(struct intel_context *ce,
 	}
 }
 
-static int try_context_registration(struct intel_context *ce, bool loop)
+static int try_context_registration(struct intel_context *ce, bool loop, bool locked)
 {
 	struct intel_engine_cs *engine = ce->engine;
 	struct intel_runtime_pm *runtime_pm = engine->uncore->rpm;
@@ -2960,12 +2967,12 @@  static int try_context_registration(struct intel_context *ce, bool loop)
 		 * context whose guc_id was stolen.
 		 */
 		with_intel_runtime_pm(runtime_pm, wakeref)
-			ret = deregister_context(ce, ce->guc_id.id);
+			ret = deregister_context(ce, ce->guc_id.id, locked);
 		if (unlikely(ret == -ENODEV))
 			ret = 0;	/* Will get registered later */
 	} else {
 		with_intel_runtime_pm(runtime_pm, wakeref)
-			ret = register_context(ce, loop);
+			ret = register_context(ce, loop, locked);
 		if (unlikely(ret == -EBUSY)) {
 			clr_ctx_id_mapping(guc, ctx_id);
 		} else if (unlikely(ret == -ENODEV)) {
@@ -3036,7 +3043,8 @@  static void guc_context_post_unpin(struct intel_context *ce)
 }
 
 static void __guc_context_sched_enable(struct intel_guc *guc,
-				       struct intel_context *ce)
+				       struct intel_context *ce,
+				       bool locked)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET,
@@ -3047,12 +3055,13 @@  static void __guc_context_sched_enable(struct intel_guc *guc,
 	trace_intel_context_sched_enable(ce);
 
 	guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action),
-				      G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true);
+				      G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true, locked);
 }
 
 static void __guc_context_sched_disable(struct intel_guc *guc,
 					struct intel_context *ce,
-					u16 guc_id)
+					u16 guc_id,
+					bool locked)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET,
@@ -3066,7 +3075,7 @@  static void __guc_context_sched_disable(struct intel_guc *guc,
 	trace_intel_context_sched_disable(ce);
 
 	guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action),
-				      G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true);
+				      G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true, locked);
 }
 
 static void guc_blocked_fence_complete(struct intel_context *ce)
@@ -3139,7 +3148,7 @@  static struct i915_sw_fence *guc_context_block(struct intel_context *ce)
 	spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
 	with_intel_runtime_pm(runtime_pm, wakeref)
-		__guc_context_sched_disable(guc, ce, guc_id);
+		__guc_context_sched_disable(guc, ce, guc_id, false);
 
 	return &ce->guc_state.blocked;
 }
@@ -3190,7 +3199,7 @@  static void guc_context_unblock(struct intel_context *ce)
 
 	if (enable) {
 		with_intel_runtime_pm(runtime_pm, wakeref)
-			__guc_context_sched_enable(guc, ce);
+			__guc_context_sched_enable(guc, ce, false);
 	}
 }
 
@@ -3219,14 +3228,15 @@  static void guc_context_cancel_request(struct intel_context *ce,
 
 static void __guc_context_set_preemption_timeout(struct intel_guc *guc,
 						 u16 guc_id,
-						 u32 preemption_timeout)
+						 u32 preemption_timeout,
+						 bool locked)
 {
 	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
 		struct context_policy policy;
 
 		__guc_context_policy_start_klv(&policy, guc_id);
 		__guc_context_policy_add_preemption_timeout(&policy, preemption_timeout);
-		__guc_context_set_context_policies(guc, &policy, true);
+		__guc_context_set_context_policies(guc, &policy, true, locked);
 	} else {
 		u32 action[] = {
 			INTEL_GUC_ACTION_V69_SET_CONTEXT_PREEMPTION_TIMEOUT,
@@ -3234,7 +3244,7 @@  static void __guc_context_set_preemption_timeout(struct intel_guc *guc,
 			preemption_timeout
 		};
 
-		intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true);
+		intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true, locked);
 	}
 }
 
@@ -3280,15 +3290,17 @@  guc_context_revoke(struct intel_context *ce, struct i915_request *rq,
 		 */
 		with_intel_runtime_pm(runtime_pm, wakeref) {
 			__guc_context_set_preemption_timeout(guc, guc_id,
-							     preempt_timeout_ms);
-			__guc_context_sched_disable(guc, ce, guc_id);
+							     preempt_timeout_ms,
+							     false);
+			__guc_context_sched_disable(guc, ce, guc_id, false);
 		}
 	} else {
 		if (!context_guc_id_invalid(ce))
 			with_intel_runtime_pm(runtime_pm, wakeref)
 				__guc_context_set_preemption_timeout(guc,
 								     ce->guc_id.id,
-								     preempt_timeout_ms);
+								     preempt_timeout_ms,
+								     true);
 		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 	}
 }
@@ -3307,7 +3319,7 @@  static void do_sched_disable(struct intel_guc *guc, struct intel_context *ce,
 	spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
 	with_intel_runtime_pm(runtime_pm, wakeref)
-		__guc_context_sched_disable(guc, ce, guc_id);
+		__guc_context_sched_disable(guc, ce, guc_id, false);
 }
 
 static bool bypass_sched_disable(struct intel_guc *guc,
@@ -3431,7 +3443,7 @@  static inline int guc_lrc_desc_unpin(struct intel_context *ce)
 	 * with suspend, so we undo everything if the H2G fails in deregister_context so
 	 * that GuC reset will find this context during clean up.
 	 */
-	ret = deregister_context(ce, ce->guc_id.id);
+	ret = deregister_context(ce, ce->guc_id.id, false);
 	if (ret) {
 		spin_lock(&ce->guc_state.lock);
 		set_context_registered(ce);
@@ -3596,14 +3608,15 @@  static int guc_context_alloc(struct intel_context *ce)
 }
 
 static void __guc_context_set_prio(struct intel_guc *guc,
-				   struct intel_context *ce)
+				   struct intel_context *ce,
+				   bool locked)
 {
 	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
 		struct context_policy policy;
 
 		__guc_context_policy_start_klv(&policy, ce->guc_id.id);
 		__guc_context_policy_add_priority(&policy, ce->guc_state.prio);
-		__guc_context_set_context_policies(guc, &policy, true);
+		__guc_context_set_context_policies(guc, &policy, true, locked);
 	} else {
 		u32 action[] = {
 			INTEL_GUC_ACTION_V69_SET_CONTEXT_PRIORITY,
@@ -3611,7 +3624,7 @@  static void __guc_context_set_prio(struct intel_guc *guc,
 			ce->guc_state.prio,
 		};
 
-		guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true);
+		guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), 0, true, locked);
 	}
 }
 
@@ -3630,7 +3643,7 @@  static void guc_context_set_prio(struct intel_guc *guc,
 	}
 
 	ce->guc_state.prio = prio;
-	__guc_context_set_prio(guc, ce);
+	__guc_context_set_prio(guc, ce, true);
 
 	trace_intel_context_set_prio(ce);
 }
@@ -3927,7 +3940,7 @@  static int guc_request_alloc(struct i915_request *rq)
 	if (unlikely(ret < 0))
 		return ret;
 	if (context_needs_register(ce, !!ret)) {
-		ret = try_context_registration(ce, true);
+		ret = try_context_registration(ce, true, false);
 		if (unlikely(ret)) {	/* unwind */
 			if (ret == -EPIPE) {
 				disable_submission(guc);
@@ -4447,7 +4460,7 @@  static inline int guc_kernel_context_pin(struct intel_guc *guc,
 	if (!test_bit(CONTEXT_GUC_INIT, &ce->flags))
 		guc_context_init(ce);
 
-	ret = try_context_registration(ce, true);
+	ret = try_context_registration(ce, true, false);
 	if (ret)
 		unpin_guc_id(guc, ce);
 
@@ -4990,7 +5003,7 @@  static int guc_send_invalidate_tlb(struct intel_guc *guc,
 	add_wait_queue(&wq->wq, &wait);
 
 	/* This is a critical reclaim path and thus we must loop here. */
-	err = intel_guc_send_busy_loop(guc, action, size, G2H_LEN_DW_INVALIDATE_TLB, true);
+	err = intel_guc_send_busy_loop(guc, action, size, G2H_LEN_DW_INVALIDATE_TLB, true, false);
 	if (err)
 		goto out;
 
@@ -5062,7 +5075,7 @@  int intel_guc_deregister_done_process_msg(struct intel_guc *guc,
 		 * register this context.
 		 */
 		with_intel_runtime_pm(runtime_pm, wakeref)
-			register_context(ce, true);
+			register_context(ce, true, false);
 		guc_signal_context_fence(ce);
 		intel_context_put(ce);
 	} else if (context_destroyed(ce)) {