From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri, 6 Apr 2018 23:03:53 +0100
Subject: drm/i915: Treat i915_reset_engine() as guilty until proven innocent
Git-commit: bba0869b18e44ff2f713c98575ddad8c7c5e9b10
Patch-mainline: v4.18-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166
If we are resetting just one engine, we know it has stalled. So we can
pass the stalled parameter directly to i915_gem_reset_engine(), which
alleviates the necessity to poke at the generic engine->hangcheck.stalled
magic variable, leaving that under control of hangcheck as its name
implies. Other than simplifying by removing the indirect parameter along
this path, this allows us to introduce new reset mechanisms that run
independently of hangcheck.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180406220354.18911-1-chris@chris-wilson.co.uk
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
drivers/gpu/drm/i915/i915_drv.c | 2 -
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_gem.c | 36 ++++++++++-------------
drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 9 -----
4 files changed, 20 insertions(+), 30 deletions(-)
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2056,7 +2056,7 @@ int i915_reset_engine(struct intel_engin
* active request and can drop it, adjust head to skip the offending
* request to resume executing remaining requests in the queue.
*/
- i915_gem_reset_engine(engine, active_request);
+ i915_gem_reset_engine(engine, active_request, true);
/*
* The engine and its registers (and workarounds in case of render)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3132,7 +3132,8 @@ void i915_gem_reset_finish(struct drm_i9
void i915_gem_set_wedged(struct drm_i915_private *dev_priv);
bool i915_gem_unset_wedged(struct drm_i915_private *dev_priv);
void i915_gem_reset_engine(struct intel_engine_cs *engine,
- struct i915_request *request);
+ struct i915_request *request,
+ bool stalled);
void i915_gem_init_mmio(struct drm_i915_private *i915);
int __must_check i915_gem_init(struct drm_i915_private *dev_priv);
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2990,20 +2990,6 @@ i915_gem_find_active_request(struct inte
return active;
}
-static bool engine_stalled(struct intel_engine_cs *engine)
-{
- if (!engine->hangcheck.stalled)
- return false;
-
- /* Check for possible seqno movement after hang declaration */
- if (engine->hangcheck.seqno != intel_engine_get_seqno(engine)) {
- DRM_DEBUG_DRIVER("%s pardoned\n", engine->name);
- return false;
- }
-
- return true;
-}
-
/*
* Ensure irq handler finishes, and not run again.
* Also return the active request so that we only search for it once.
@@ -3142,7 +3128,8 @@ static void engine_skip_context(struct i
/* Returns the request if it was guilty of the hang */
static struct i915_request *
i915_gem_reset_request(struct intel_engine_cs *engine,
- struct i915_request *request)
+ struct i915_request *request,
+ bool stalled)
{
/* The guilty request will get skipped on a hung engine.
*
@@ -3165,7 +3152,15 @@ i915_gem_reset_request(struct intel_engi
* subsequent hangs.
*/
- if (engine_stalled(engine)) {
+ if (i915_request_completed(request)) {
+ GEM_TRACE("%s pardoned global=%d (fence %llx:%d), current %d\n",
+ engine->name, request->global_seqno,
+ request->fence.context, request->fence.seqno,
+ intel_engine_get_seqno(engine));
+ stalled = false;
+ }
+
+ if (stalled) {
i915_gem_context_mark_guilty(request->ctx);
skip_request(request);
@@ -3196,7 +3191,8 @@ i915_gem_reset_request(struct intel_engi
}
void i915_gem_reset_engine(struct intel_engine_cs *engine,
- struct i915_request *request)
+ struct i915_request *request,
+ bool stalled)
{
/*
* Make sure this write is visible before we re-enable the interrupt
@@ -3206,7 +3202,7 @@ void i915_gem_reset_engine(struct intel_
smp_store_mb(engine->irq_posted, 0);
if (request)
- request = i915_gem_reset_request(engine, request);
+ request = i915_gem_reset_request(engine, request, stalled);
if (request) {
DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n",
@@ -3229,7 +3225,9 @@ void i915_gem_reset(struct drm_i915_priv
for_each_engine(engine, dev_priv, id) {
struct i915_gem_context *ctx;
- i915_gem_reset_engine(engine, engine->hangcheck.active_request);
+ i915_gem_reset_engine(engine,
+ engine->hangcheck.active_request,
+ engine->hangcheck.stalled);
ctx = fetch_and_zero(&engine->last_retired_context);
if (ctx)
engine->context_unpin(engine, ctx);
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -522,9 +522,6 @@ static int __igt_reset_engine(struct drm
i915_request_put(rq);
}
- engine->hangcheck.stalled = true;
- engine->hangcheck.seqno = seqno;
-
err = i915_reset_engine(engine, NULL);
if (err) {
pr_err("i915_reset_engine failed\n");
@@ -545,8 +542,6 @@ static int __igt_reset_engine(struct drm
err = -EINVAL;
break;
}
-
- engine->hangcheck.stalled = false;
} while (time_before(jiffies, end_time));
clear_bit(I915_RESET_ENGINE + id, &i915->gpu_error.flags);
@@ -764,9 +759,6 @@ static int __igt_reset_engines(struct dr
seqno = rq->global_seqno - 1;
}
- engine->hangcheck.stalled = true;
- engine->hangcheck.seqno = seqno;
-
err = i915_reset_engine(engine, NULL);
if (err) {
pr_err("i915_reset_engine(%s:%s): failed, err=%d\n",
@@ -774,7 +766,6 @@ static int __igt_reset_engines(struct dr
break;
}
- engine->hangcheck.stalled = false;
count++;
if (rq) {