From: Matthew Brost <matthew.brost@intel.com>
To: <intel-gfx@lists.freedesktop.org>, <dri-devel@lists.freedesktop.org>
Cc: <daniel.vetter@ffwll.ch>
Subject: [Intel-gfx] [PATCH 20/22] drm/i915/guc: Drop pin count check trick between sched_disable and re-pin
Date: Mon, 16 Aug 2021 06:51:37 -0700 [thread overview]
Message-ID: <20210816135139.10060-21-matthew.brost@intel.com> (raw)
In-Reply-To: <20210816135139.10060-1-matthew.brost@intel.com>
Drop pin count check trick between a sched_disable and re-pin, now rely
on the lock and counter of the number of committed requests to determine
if scheduling should be disabled on the context.
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 2 +
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 49 ++++++++++++-------
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index d5d643b04d54..524a35a78bf4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -169,6 +169,8 @@ struct intel_context {
struct list_head fences;
/* GuC context blocked fence */
struct i915_sw_fence blocked_fence;
+ /* GuC committed requests */
+ int number_committed_requests;
} guc_state;
struct {
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 2ecb2f002bed..c6ae6b4417c2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -247,6 +247,25 @@ static inline void decr_context_blocked(struct intel_context *ce)
ce->guc_state.sched_state -= SCHED_STATE_BLOCKED;
}
+static inline bool context_has_committed_requests(struct intel_context *ce)
+{
+ return !!ce->guc_state.number_committed_requests;
+}
+
+static inline void incr_context_committed_requests(struct intel_context *ce)
+{
+ lockdep_assert_held(&ce->guc_state.lock);
+ ++ce->guc_state.number_committed_requests;
+ GEM_BUG_ON(ce->guc_state.number_committed_requests < 0);
+}
+
+static inline void decr_context_committed_requests(struct intel_context *ce)
+{
+ lockdep_assert_held(&ce->guc_state.lock);
+ --ce->guc_state.number_committed_requests;
+ GEM_BUG_ON(ce->guc_state.number_committed_requests < 0);
+}
+
static inline bool context_guc_id_invalid(struct intel_context *ce)
{
return ce->guc_id == GUC_INVALID_LRC_ID;
@@ -1736,14 +1755,11 @@ static void guc_context_sched_disable(struct intel_context *ce)
spin_lock_irqsave(&ce->guc_state.lock, flags);
/*
- * We have to check if the context has been disabled by another thread.
- * We also have to check if the context has been pinned again as another
- * pin operation is allowed to pass this function. Checking the pin
- * count, within ce->guc_state.lock, synchronizes this function with
- * guc_request_alloc ensuring a request doesn't slip through the
- * 'context_pending_disable' fence. Checking within the spin lock (can't
- * sleep) ensures another process doesn't pin this context and generate
- * a request before we set the 'context_pending_disable' flag here.
+ * We have to check if the context has been disabled by another thread,
+ * check if submssion has been disabled to seal a race with reset and
+ * finally check if any more requests have been committed to the
+ * context ensursing that a request doesn't slip through the
+ * 'context_pending_disable' fence.
*/
enabled = context_enabled(ce);
if (unlikely(!enabled || submission_disabled(guc))) {
@@ -1752,7 +1768,8 @@ static void guc_context_sched_disable(struct intel_context *ce)
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
goto unpin;
}
- if (unlikely(atomic_add_unless(&ce->pin_count, -2, 2))) {
+ if (unlikely(context_has_committed_requests(ce))) {
+ intel_context_sched_disable_unpin(ce);
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
return;
}
@@ -1785,6 +1802,7 @@ static void __guc_context_destroy(struct intel_context *ce)
ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+ GEM_BUG_ON(ce->guc_state.number_committed_requests);
lrc_fini(ce);
intel_context_fini(ce);
@@ -2015,6 +2033,10 @@ static void remove_from_context(struct i915_request *rq)
spin_unlock_irq(&ce->guc_active.lock);
+ spin_lock_irq(&ce->guc_state.lock);
+ decr_context_committed_requests(ce);
+ spin_unlock_irq(&ce->guc_state.lock);
+
atomic_dec(&ce->guc_id_ref);
i915_request_notify_execute_cb_imm(rq);
}
@@ -2162,15 +2184,7 @@ static int guc_request_alloc(struct i915_request *rq)
* schedule enable or context registration if either G2H is pending
* respectfully. Once a G2H returns, the fence is released that is
* blocking these requests (see guc_signal_context_fence).
- *
- * We can safely check the below fields outside of the lock as it isn't
- * possible for these fields to transition from being clear to set but
- * converse is possible, hence the need for the check within the lock.
*/
- if (likely(!context_wait_for_deregister_to_register(ce) &&
- !context_pending_disable(ce)))
- return 0;
-
spin_lock_irqsave(&ce->guc_state.lock, flags);
if (context_wait_for_deregister_to_register(ce) ||
context_pending_disable(ce)) {
@@ -2179,6 +2193,7 @@ static int guc_request_alloc(struct i915_request *rq)
list_add_tail(&rq->guc_fence_link, &ce->guc_state.fences);
}
+ incr_context_committed_requests(ce);
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
return 0;
--
2.32.0
next prev parent reply other threads:[~2021-08-16 13:58 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-16 13:51 [Intel-gfx] [PATCH 00/22] Clean up GuC CI failures, simplify locking, and kernel DOC Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 01/22] drm/i915/guc: Fix blocked context accounting Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 02/22] drm/i915/guc: Fix outstanding G2H accounting Matthew Brost
2021-08-17 9:39 ` Daniel Vetter
2021-08-17 18:17 ` Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 03/22] drm/i915/guc: Unwind context requests in reverse order Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 04/22] drm/i915/guc: Don't drop ce->guc_active.lock when unwinding context Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 05/22] drm/i915/guc: Workaround reset G2H is received after schedule done G2H Matthew Brost
2021-08-17 9:32 ` Daniel Vetter
2021-08-17 15:03 ` Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 06/22] drm/i915/execlists: Do not propagate errors to dependent fences Matthew Brost
2021-08-17 9:21 ` Daniel Vetter
2021-08-17 15:08 ` Matthew Brost
2021-08-17 15:49 ` Daniel Vetter
2021-08-16 13:51 ` [Intel-gfx] [PATCH 07/22] drm/i915/selftests: Add a cancel request selftest that triggers a reset Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 08/22] drm/i915/guc: Don't enable scheduling on a banned context, guc_id invalid, not registered Matthew Brost
2021-08-17 9:47 ` Daniel Vetter
2021-08-17 9:57 ` Daniel Vetter
2021-08-17 16:44 ` Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 09/22] drm/i915/selftests: Fix memory corruption in live_lrc_isolation Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 10/22] drm/i915/selftests: Add initial GuC selftest for scrubbing lost G2H Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 11/22] drm/i915/guc: Take context ref when cancelling request Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 12/22] drm/i915/guc: Don't touch guc_state.sched_state without a lock Matthew Brost
2021-08-17 7:21 ` kernel test robot
2021-08-16 13:51 ` [Intel-gfx] [PATCH 13/22] drm/i915/guc: Reset LRC descriptor if register returns -ENODEV Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 14/22] drm/i915: Allocate error capture in atomic context Matthew Brost
2021-08-17 10:06 ` Daniel Vetter
2021-08-17 16:12 ` Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 15/22] drm/i915/guc: Flush G2H work queue during reset Matthew Brost
2021-08-17 10:06 ` Daniel Vetter
2021-08-16 13:51 ` [Intel-gfx] [PATCH 16/22] drm/i915/guc: Release submit fence from an IRQ Matthew Brost
2021-08-17 10:08 ` Daniel Vetter
2021-08-16 13:51 ` [Intel-gfx] [PATCH 17/22] drm/i915/guc: Move guc_blocked fence to struct guc_state Matthew Brost
2021-08-17 10:10 ` Daniel Vetter
2021-08-16 13:51 ` [Intel-gfx] [PATCH 18/22] drm/i915/guc: Rework and simplify locking Matthew Brost
2021-08-17 10:15 ` Daniel Vetter
2021-08-17 15:30 ` Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 19/22] drm/i915/guc: Proper xarray usage for contexts_lookup Matthew Brost
2021-08-17 10:27 ` Daniel Vetter
2021-08-17 15:26 ` Matthew Brost
2021-08-17 17:13 ` Daniel Vetter
2021-08-17 17:13 ` Matthew Brost
2021-08-16 13:51 ` Matthew Brost [this message]
2021-08-16 13:51 ` [Intel-gfx] [PATCH 21/22] drm/i915/guc: Move GuC priority fields in context under guc_active Matthew Brost
2021-08-16 13:51 ` [Intel-gfx] [PATCH 22/22] drm/i915/guc: Add GuC kernel doc Matthew Brost
2021-08-17 11:11 ` Daniel Vetter
2021-08-17 16:36 ` Matthew Brost
2021-08-17 17:20 ` Daniel Vetter
2021-08-17 17:27 ` Michal Wajdeczko
2021-08-17 17:34 ` Daniel Vetter
2021-08-17 20:41 ` Michal Wajdeczko
2021-08-17 21:49 ` Daniel Vetter
2021-08-17 12:49 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Clean up GuC CI failures, simplify locking, and kernel DOC (rev2) Patchwork
2021-08-17 12:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-08-17 13:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-08-17 14:39 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210816135139.10060-21-matthew.brost@intel.com \
--to=matthew.brost@intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).