Commit 70234728 authored by Tvrtko Ursulin's avatar Tvrtko Ursulin

drm/i915/guc: Fix revocation of non-persistent contexts

Patch which added graceful exit for non-persistent contexts missed the
fact it is not enough to set the exiting flag on a context and let the
backend handle it from there.

GuC backend cannot handle it because it runs independently in the
firmware and driver might not see the requests ever again. Patch also
missed the fact some usages of intel_context_is_banned in the GuC backend
needed replacing with newly introduced intel_context_is_schedulable.

Fix the first issue by calling into backend revoke when we know this is
the last chance to do it. Fix the second issue by replacing
intel_context_is_banned with intel_context_is_schedulable, which should
always be safe since latter is a superset of the former.

v2:
 * Just call ce->ops->revoke unconditionally. (Andrzej)
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 45c64ecf ("drm/i915: Improve user experience and driver robustness under SIGINT or similar")
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: <stable@vger.kernel.org> # v6.0+
Reviewed-by: default avatarAndrzej Hajda <andrzej.hajda@intel.com>
Acked-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221003121630.694249-1-tvrtko.ursulin@linux.intel.com
(cherry picked from commit 0add082c)
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
parent cdf6428d
...@@ -1383,14 +1383,8 @@ kill_engines(struct i915_gem_engines *engines, bool exit, bool persistent) ...@@ -1383,14 +1383,8 @@ kill_engines(struct i915_gem_engines *engines, bool exit, bool persistent)
*/ */
for_each_gem_engine(ce, engines, it) { for_each_gem_engine(ce, engines, it) {
struct intel_engine_cs *engine; struct intel_engine_cs *engine;
bool skip = false;
if (exit) if ((exit || !persistent) && intel_context_revoke(ce))
skip = intel_context_set_exiting(ce);
else if (!persistent)
skip = intel_context_exit_nonpersistent(ce, NULL);
if (skip)
continue; /* Already marked. */ continue; /* Already marked. */
/* /*
......
...@@ -614,13 +614,12 @@ bool intel_context_ban(struct intel_context *ce, struct i915_request *rq) ...@@ -614,13 +614,12 @@ bool intel_context_ban(struct intel_context *ce, struct i915_request *rq)
return ret; return ret;
} }
bool intel_context_exit_nonpersistent(struct intel_context *ce, bool intel_context_revoke(struct intel_context *ce)
struct i915_request *rq)
{ {
bool ret = intel_context_set_exiting(ce); bool ret = intel_context_set_exiting(ce);
if (ce->ops->revoke) if (ce->ops->revoke)
ce->ops->revoke(ce, rq, ce->engine->props.preempt_timeout_ms); ce->ops->revoke(ce, NULL, ce->engine->props.preempt_timeout_ms);
return ret; return ret;
} }
......
...@@ -329,8 +329,7 @@ static inline bool intel_context_set_exiting(struct intel_context *ce) ...@@ -329,8 +329,7 @@ static inline bool intel_context_set_exiting(struct intel_context *ce)
return test_and_set_bit(CONTEXT_EXITING, &ce->flags); return test_and_set_bit(CONTEXT_EXITING, &ce->flags);
} }
bool intel_context_exit_nonpersistent(struct intel_context *ce, bool intel_context_revoke(struct intel_context *ce);
struct i915_request *rq);
static inline bool static inline bool
intel_context_force_single_submission(const struct intel_context *ce) intel_context_force_single_submission(const struct intel_context *ce)
......
...@@ -684,7 +684,7 @@ static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq) ...@@ -684,7 +684,7 @@ static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq)
* Corner case where requests were sitting in the priority list or a * Corner case where requests were sitting in the priority list or a
* request resubmitted after the context was banned. * request resubmitted after the context was banned.
*/ */
if (unlikely(intel_context_is_banned(ce))) { if (unlikely(!intel_context_is_schedulable(ce))) {
i915_request_put(i915_request_mark_eio(rq)); i915_request_put(i915_request_mark_eio(rq));
intel_engine_signal_breadcrumbs(ce->engine); intel_engine_signal_breadcrumbs(ce->engine);
return 0; return 0;
...@@ -870,15 +870,15 @@ static int guc_wq_item_append(struct intel_guc *guc, ...@@ -870,15 +870,15 @@ static int guc_wq_item_append(struct intel_guc *guc,
struct i915_request *rq) struct i915_request *rq)
{ {
struct intel_context *ce = request_to_scheduling_context(rq); struct intel_context *ce = request_to_scheduling_context(rq);
int ret = 0; int ret;
if (likely(!intel_context_is_banned(ce))) { if (unlikely(!intel_context_is_schedulable(ce)))
ret = __guc_wq_item_append(rq); return 0;
if (unlikely(ret == -EBUSY)) { ret = __guc_wq_item_append(rq);
guc->stalled_request = rq; if (unlikely(ret == -EBUSY)) {
guc->submission_stall_reason = STALL_MOVE_LRC_TAIL; guc->stalled_request = rq;
} guc->submission_stall_reason = STALL_MOVE_LRC_TAIL;
} }
return ret; return ret;
...@@ -897,7 +897,7 @@ static bool multi_lrc_submit(struct i915_request *rq) ...@@ -897,7 +897,7 @@ static bool multi_lrc_submit(struct i915_request *rq)
* submitting all the requests generated in parallel. * submitting all the requests generated in parallel.
*/ */
return test_bit(I915_FENCE_FLAG_SUBMIT_PARALLEL, &rq->fence.flags) || return test_bit(I915_FENCE_FLAG_SUBMIT_PARALLEL, &rq->fence.flags) ||
intel_context_is_banned(ce); !intel_context_is_schedulable(ce);
} }
static int guc_dequeue_one_context(struct intel_guc *guc) static int guc_dequeue_one_context(struct intel_guc *guc)
...@@ -966,7 +966,7 @@ static int guc_dequeue_one_context(struct intel_guc *guc) ...@@ -966,7 +966,7 @@ static int guc_dequeue_one_context(struct intel_guc *guc)
struct intel_context *ce = request_to_scheduling_context(last); struct intel_context *ce = request_to_scheduling_context(last);
if (unlikely(!ctx_id_mapped(guc, ce->guc_id.id) && if (unlikely(!ctx_id_mapped(guc, ce->guc_id.id) &&
!intel_context_is_banned(ce))) { intel_context_is_schedulable(ce))) {
ret = try_context_registration(ce, false); ret = try_context_registration(ce, false);
if (unlikely(ret == -EPIPE)) { if (unlikely(ret == -EPIPE)) {
goto deadlk; goto deadlk;
...@@ -1576,7 +1576,7 @@ static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub) ...@@ -1576,7 +1576,7 @@ static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub)
{ {
struct intel_engine_cs *engine = __context_to_physical_engine(ce); struct intel_engine_cs *engine = __context_to_physical_engine(ce);
if (intel_context_is_banned(ce)) if (!intel_context_is_schedulable(ce))
return; return;
GEM_BUG_ON(!intel_context_is_pinned(ce)); GEM_BUG_ON(!intel_context_is_pinned(ce));
...@@ -4424,12 +4424,12 @@ static void guc_handle_context_reset(struct intel_guc *guc, ...@@ -4424,12 +4424,12 @@ static void guc_handle_context_reset(struct intel_guc *guc,
{ {
trace_intel_context_reset(ce); trace_intel_context_reset(ce);
if (likely(!intel_context_is_banned(ce))) { if (likely(intel_context_is_schedulable(ce))) {
capture_error_state(guc, ce); capture_error_state(guc, ce);
guc_context_replay(ce); guc_context_replay(ce);
} else { } else {
drm_info(&guc_to_gt(guc)->i915->drm, drm_info(&guc_to_gt(guc)->i915->drm,
"Ignoring context reset notification of banned context 0x%04X on %s", "Ignoring context reset notification of exiting context 0x%04X on %s",
ce->guc_id.id, ce->engine->name); ce->guc_id.id, ce->engine->name);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment