Re: [Intel-gfx] [RFC PATCH 65/97] drm/i915: Reset GPU immediately if submission is disabled

On 06/05/2021 20:14, Matthew Brost wrote:

If submission is disabled by the backend for any reason, reset the GPU immediately in the heartbeat code.

Okay that's what, but why is also often good to have in commit messages.

Regards,

Tvrtko

Signed-off-by: Matthew Brost matthew.brost@intel.com

.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 63 +++++++++++++++---- .../gpu/drm/i915/gt/intel_engine_heartbeat.h | 4 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 9 +++ drivers/gpu/drm/i915/i915_scheduler.c | 6 ++ drivers/gpu/drm/i915/i915_scheduler.h | 6 ++ drivers/gpu/drm/i915/i915_scheduler_types.h | 3 + 6 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c index b6a305e6a974..a8495364d906 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c @@ -70,12 +70,30 @@ static void show_heartbeat(const struct i915_request *rq, { struct drm_printer p = drm_debug_printer("heartbeat");

• intel_engine_dump(engine, &p,

• 	  "%s heartbeat {seqno:%llx:%lld, prio:%d} not ticking\n",
  

• 	  engine->name,
  

• 	  rq->fence.context,
  

• 	  rq->fence.seqno,
  

• 	  rq->sched.attr.priority);
  

• if (!rq) {

• intel_engine_dump(engine, &p,
  

• 		  "%s heartbeat not ticking\n",
  

• 		  engine->name);
  

• } else {

• intel_engine_dump(engine, &p,
  

• 		  "%s heartbeat {seqno:%llx:%lld, prio:%d} not ticking\n",
  

• 		  engine->name,
  

• 		  rq->fence.context,
  

• 		  rq->fence.seqno,
  

• 		  rq->sched.attr.priority);
  

• }

+}

+static void +reset_engine(struct intel_engine_cs *engine, struct i915_request *rq) +{

• if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))

• show_heartbeat(rq, engine);
  

• intel_gt_handle_error(engine->gt, engine->mask,

• 	      I915_ERROR_CAPTURE,
  

• 	      "stopped heartbeat on %s",
  

• 	      engine->name);
  

  }

  static void heartbeat(struct work_struct *wrk)

@@ -102,6 +120,11 @@ static void heartbeat(struct work_struct *wrk) if (intel_gt_is_wedged(engine->gt)) goto out;

• if (i915_sched_engine_disabled(engine->sched_engine)) {

• reset_engine(engine, engine->heartbeat.systole);
  

• goto out;
  

• }
• if (engine->heartbeat.systole) { long delay = READ_ONCE(engine->props.heartbeat_interval_ms);

@@ -139,13 +162,7 @@ static void heartbeat(struct work_struct *wrk) engine->sched_engine->schedule(rq, &attr); local_bh_enable(); } else {

• 	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
  

• 		show_heartbeat(rq, engine);
  

• 	intel_gt_handle_error(engine->gt, engine->mask,
  

• 			      I915_ERROR_CAPTURE,
  

• 			      "stopped heartbeat on %s",
  

• 			      engine->name);
  

• 	reset_engine(engine, rq);
  

  }

  rq->emitted_jiffies = jiffies;

@@ -194,6 +211,26 @@ void intel_engine_park_heartbeat(struct intel_engine_cs *engine) i915_request_put(fetch_and_zero(&engine->heartbeat.systole)); }

+void intel_gt_unpark_heartbeats(struct intel_gt *gt) +{

• struct intel_engine_cs *engine;
• enum intel_engine_id id;
• for_each_engine(engine, gt, id)

• if (intel_engine_pm_is_awake(engine))
  

• 	intel_engine_unpark_heartbeat(engine);
  

+}

+void intel_gt_park_heartbeats(struct intel_gt *gt) +{

• struct intel_engine_cs *engine;
• enum intel_engine_id id;
• for_each_engine(engine, gt, id)

• intel_engine_park_heartbeat(engine);
  

+}

• void intel_engine_init_heartbeat(struct intel_engine_cs *engine) { INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat);

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h index a488ea3e84a3..5da6d809a87a 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h @@ -7,6 +7,7 @@ #define INTEL_ENGINE_HEARTBEAT_H

struct intel_engine_cs; +struct intel_gt;

void intel_engine_init_heartbeat(struct intel_engine_cs *engine);

@@ -16,6 +17,9 @@ int intel_engine_set_heartbeat(struct intel_engine_cs *engine, void intel_engine_park_heartbeat(struct intel_engine_cs *engine); void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine);

+void intel_gt_park_heartbeats(struct intel_gt *gt); +void intel_gt_unpark_heartbeats(struct intel_gt *gt);

• int intel_engine_pulse(struct intel_engine_cs *engine); int intel_engine_flush_barriers(struct intel_engine_cs *engine);

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 8c093bc2d3a4..a5997d6b4aa4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -10,6 +10,7 @@ #include "gt/intel_breadcrumbs.h" #include "gt/intel_context.h" #include "gt/intel_engine_pm.h" +#include "gt/intel_engine_heartbeat.h" #include "gt/intel_gt.h" #include "gt/intel_gt_irq.h" #include "gt/intel_gt_pm.h" @@ -604,6 +605,7 @@ void intel_guc_submission_reset_prepare(struct intel_guc *guc) /* Reset called during driver load? GuC not yet initialised! */ return;

• intel_gt_park_heartbeats(guc_to_gt(guc)); disable_submission(guc); guc->interrupts.disable(guc);

@@ -889,6 +891,7 @@ void intel_guc_submission_reset_finish(struct intel_guc *guc) atomic_set(&guc->outstanding_submission_g2h, 0);

enable_submission(guc);

• intel_gt_unpark_heartbeats(guc_to_gt(guc)); }

  /*

@@ -1856,6 +1859,11 @@ static int guc_resume(struct intel_engine_cs *engine) return 0; }

+static bool guc_sched_engine_disabled(struct i915_sched_engine *sched_engine) +{

• return !sched_engine->tasklet.callback;

+}

• static void guc_set_default_submission(struct intel_engine_cs *engine) { engine->submit_request = guc_submit_request;

@@ -2006,6 +2014,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine) return -ENOMEM;

guc->sched_engine->schedule = i915_schedule;

• guc->sched_engine->disabled = guc_sched_engine_disabled;
  

  guc->sched_engine->engine = engine; tasklet_setup(&guc->sched_engine->tasklet, guc_submission_tasklet);

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index 28d403a8d7d2..72a9bee3026f 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -440,6 +440,11 @@ void i915_sched_engine_free(struct kref *kref) kfree(sched_engine); }

+static bool default_disabled(struct i915_sched_engine *sched_engine) +{

• return false;

+}

• struct i915_sched_engine * i915_sched_engine_create(unsigned int subclass) {

@@ -453,6 +458,7 @@ i915_sched_engine_create(unsigned int subclass)

sched_engine->queue = RB_ROOT_CACHED; sched_engine->queue_priority_hint = INT_MIN;

• sched_engine->disabled = default_disabled;

  INIT_LIST_HEAD(&sched_engine->requests); INIT_LIST_HEAD(&sched_engine->hold);

diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index a78b1f50ecb4..ec8dfa87cbb6 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -116,4 +116,10 @@ sched_engine_active_unlock_bh(struct i915_sched_engine *sched_engine) local_bh_enable(); /* restore softirq, and kick ksoftirqd! */ }

+static inline bool +i915_sched_engine_disabled(struct i915_sched_engine *sched_engine) +{

• return sched_engine->disabled(sched_engine);

+}

• #endif /* _I915_SCHEDULER_H_ */

diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 90b389ba661b..a7183792d110 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -141,6 +141,9 @@ struct i915_sched_engine { /* Back pointer to engine */ struct intel_engine_cs *engine;

• /* Schedule engine is disabled by backend */
• bool (*disabled)(struct i915_sched_engine *sched_engine);
• /* Kick backend */ void (*kick_backend)(const struct i915_request *rq, int prio);