diff mbox series

[09/10] drm/i915: Replace hangcheck by heartbeats

Message ID 20191010071434.31195-9-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [01/10] drm/i915: Note the addition of timeslicing to the pretend scheduler | expand

Commit Message

Chris Wilson Oct. 10, 2019, 7:14 a.m. UTC
Replace sampling the engine state every so often with a periodic
heartbeat request to measure the health of an engine. This is coupled
with the forced-preemption to allow long running requests to survive so
long as they do not block other users.

The heartbeat interval can be adjusted per-engine using,

	/sys/class/drm/card?/engine/*/heartbeat_interval_ms

v2: Couple in sysfs controls

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
 drivers/gpu/drm/i915/Kconfig.profile          |  14 +
 drivers/gpu/drm/i915/Makefile                 |   1 -
 drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |   1 -
 drivers/gpu/drm/i915/gem/i915_gem_pm.c        |   2 -
 drivers/gpu/drm/i915/gt/intel_engine.h        |  32 --
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  11 +-
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 115 ++++++
 .../gpu/drm/i915/gt/intel_engine_heartbeat.h  |   5 +
 drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   5 +-
 drivers/gpu/drm/i915/gt/intel_engine_sysfs.c  |  29 ++
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  17 +-
 drivers/gpu/drm/i915/gt/intel_gt.c            |   1 -
 drivers/gpu/drm/i915/gt/intel_gt.h            |   4 -
 drivers/gpu/drm/i915/gt/intel_gt_pm.c         |   1 -
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |   9 -
 drivers/gpu/drm/i915/gt/intel_hangcheck.c     | 361 ------------------
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   4 -
 drivers/gpu/drm/i915/i915_debugfs.c           |  87 -----
 drivers/gpu/drm/i915/i915_drv.c               |   3 -
 drivers/gpu/drm/i915/i915_drv.h               |   1 -
 drivers/gpu/drm/i915/i915_gpu_error.c         |  33 +-
 drivers/gpu/drm/i915/i915_gpu_error.h         |   2 -
 drivers/gpu/drm/i915/i915_priolist_types.h    |   6 +
 25 files changed, 194 insertions(+), 555 deletions(-)
 delete mode 100644 drivers/gpu/drm/i915/gt/intel_hangcheck.c

Comments

Tvrtko Ursulin Oct. 11, 2019, 2:24 p.m. UTC | #1
On 10/10/2019 08:14, Chris Wilson wrote:
> Replace sampling the engine state every so often with a periodic
> heartbeat request to measure the health of an engine. This is coupled
> with the forced-preemption to allow long running requests to survive so
> long as they do not block other users.
> 
> The heartbeat interval can be adjusted per-engine using,
> 
> 	/sys/class/drm/card?/engine/*/heartbeat_interval_ms
> 
> v2: Couple in sysfs controls
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
> ---
>   drivers/gpu/drm/i915/Kconfig.profile          |  14 +
>   drivers/gpu/drm/i915/Makefile                 |   1 -
>   drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
>   drivers/gpu/drm/i915/gem/i915_gem_object.h    |   1 -
>   drivers/gpu/drm/i915/gem/i915_gem_pm.c        |   2 -
>   drivers/gpu/drm/i915/gt/intel_engine.h        |  32 --
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  11 +-
>   .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 115 ++++++
>   .../gpu/drm/i915/gt/intel_engine_heartbeat.h  |   5 +
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   5 +-
>   drivers/gpu/drm/i915/gt/intel_engine_sysfs.c  |  29 ++
>   drivers/gpu/drm/i915/gt/intel_engine_types.h  |  17 +-
>   drivers/gpu/drm/i915/gt/intel_gt.c            |   1 -
>   drivers/gpu/drm/i915/gt/intel_gt.h            |   4 -
>   drivers/gpu/drm/i915/gt/intel_gt_pm.c         |   1 -
>   drivers/gpu/drm/i915/gt/intel_gt_types.h      |   9 -
>   drivers/gpu/drm/i915/gt/intel_hangcheck.c     | 361 ------------------
>   drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
>   drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   4 -
>   drivers/gpu/drm/i915/i915_debugfs.c           |  87 -----
>   drivers/gpu/drm/i915/i915_drv.c               |   3 -
>   drivers/gpu/drm/i915/i915_drv.h               |   1 -
>   drivers/gpu/drm/i915/i915_gpu_error.c         |  33 +-
>   drivers/gpu/drm/i915/i915_gpu_error.h         |   2 -
>   drivers/gpu/drm/i915/i915_priolist_types.h    |   6 +
>   25 files changed, 194 insertions(+), 555 deletions(-)
>   delete mode 100644 drivers/gpu/drm/i915/gt/intel_hangcheck.c
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
> index 8fceea85937b..d3950aabb497 100644
> --- a/drivers/gpu/drm/i915/Kconfig.profile
> +++ b/drivers/gpu/drm/i915/Kconfig.profile
> @@ -40,3 +40,17 @@ config DRM_I915_PREEMPT_TIMEOUT
>   	  /sys/class/drm/card?/engine/*/preempt_timeout_ms
>   
>   	  May be 0 to disable the timeout.
> +
> +config DRM_I915_HEARTBEAT_INTERVAL
> +	int "Interval between heartbeat pulses (ms)"
> +	default 2500 # milliseconds
> +	help
> +	  While active the driver uses a periodic request, a heartbeat, to
> +	  check the wellness of the GPU and to regularly flush state changes
> +	  (idle barriers).

Should this be somehow reworded to be more end user friendly? My idea, 
may need to be corrected for bad English:

The driver sends a periodic heartbeat down all active GT engines to 
check the health of the GPU and undertake regular house-keeping of 
internal driver state.

Main points from the user perspective: "request" - whaat? "idle 
barriers" - ditto. "Wellness" - a bit unusual in this context, no?

> +
> +	  This is adjustable via
> +	  /sys/class/drm/card?/engine/*/heartbeat_interval_ms
> +
> +	  May be 0 to disable heartbeats and therefore disable automatic GPU
> +	  hang detection.
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index cfab7c8585b3..59d356cc406c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -88,7 +88,6 @@ gt-y += \
>   	gt/intel_gt_pm.o \
>   	gt/intel_gt_pm_irq.o \
>   	gt/intel_gt_requests.o \
> -	gt/intel_hangcheck.o \
>   	gt/intel_lrc.o \
>   	gt/intel_rc6.o \
>   	gt/intel_renderstate.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1a533ccdb54f..5e5de3081f48 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14338,7 +14338,7 @@ static void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
>   static void fb_obj_bump_render_priority(struct drm_i915_gem_object *obj)
>   {
>   	struct i915_sched_attr attr = {
> -		.priority = I915_PRIORITY_DISPLAY,
> +		.priority = I915_USER_PRIORITY(I915_PRIORITY_DISPLAY),
>   	};
>   
>   	i915_gem_object_wait_priority(obj, 0, &attr);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index c5e14c9c805c..5bd51e397371 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -460,6 +460,5 @@ int i915_gem_object_wait(struct drm_i915_gem_object *obj,
>   int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
>   				  unsigned int flags,
>   				  const struct i915_sched_attr *attr);
> -#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
>   
>   #endif
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> index 7987b54fb1f5..0e97520cb1bb 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> @@ -100,8 +100,6 @@ void i915_gem_suspend(struct drm_i915_private *i915)
>   	intel_gt_suspend(&i915->gt);
>   	intel_uc_suspend(&i915->gt.uc);
>   
> -	cancel_delayed_work_sync(&i915->gt.hangcheck.work);
> -
>   	i915_gem_drain_freed_objects(i915);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 93ea367fe624..8ad57eace351 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -89,38 +89,6 @@ struct drm_printer;
>   /* seqno size is actually only a uint32, but since we plan to use MI_FLUSH_DW to
>    * do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
>    */
> -enum intel_engine_hangcheck_action {
> -	ENGINE_IDLE = 0,
> -	ENGINE_WAIT,
> -	ENGINE_ACTIVE_SEQNO,
> -	ENGINE_ACTIVE_HEAD,
> -	ENGINE_ACTIVE_SUBUNITS,
> -	ENGINE_WAIT_KICK,
> -	ENGINE_DEAD,
> -};
> -
> -static inline const char *
> -hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
> -{
> -	switch (a) {
> -	case ENGINE_IDLE:
> -		return "idle";
> -	case ENGINE_WAIT:
> -		return "wait";
> -	case ENGINE_ACTIVE_SEQNO:
> -		return "active seqno";
> -	case ENGINE_ACTIVE_HEAD:
> -		return "active head";
> -	case ENGINE_ACTIVE_SUBUNITS:
> -		return "active subunits";
> -	case ENGINE_WAIT_KICK:
> -		return "wait kick";
> -	case ENGINE_DEAD:
> -		return "dead";
> -	}
> -
> -	return "unknown";
> -}
>   
>   static inline unsigned int
>   execlists_num_ports(const struct intel_engine_execlists * const execlists)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 1eb51147839a..d829ad340ca0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -305,6 +305,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	__sprint_engine_name(engine);
>   
>   	engine->props.preempt_timeout = CONFIG_DRM_I915_PREEMPT_TIMEOUT;
> +	engine->props.heartbeat_interval = CONFIG_DRM_I915_HEARTBEAT_INTERVAL;
>   
>   	/*
>   	 * To be overridden by the backend on setup. However to facilitate
> @@ -599,7 +600,6 @@ static int intel_engine_setup_common(struct intel_engine_cs *engine)
>   	intel_engine_init_active(engine, ENGINE_PHYSICAL);
>   	intel_engine_init_breadcrumbs(engine);
>   	intel_engine_init_execlists(engine);
> -	intel_engine_init_hangcheck(engine);
>   	intel_engine_init_cmd_parser(engine);
>   	intel_engine_init__pm(engine);
>   
> @@ -1432,8 +1432,13 @@ void intel_engine_dump(struct intel_engine_cs *engine,
>   		drm_printf(m, "*** WEDGED ***\n");
>   
>   	drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
> -	drm_printf(m, "\tHangcheck: %d ms ago\n",
> -		   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
> +
> +	rcu_read_lock();
> +	rq = READ_ONCE(engine->heartbeat.systole);
> +	if (rq)
> +		drm_printf(m, "\tHeartbeat: %d ms ago\n",
> +			   jiffies_to_msecs(jiffies - rq->emitted_jiffies));
> +	rcu_read_unlock();
>   	drm_printf(m, "\tReset count: %d (global %d)\n",
>   		   i915_reset_engine_count(error, engine),
>   		   i915_reset_count(error));
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> index 2fc413f9d506..f68acf9118f3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> @@ -11,6 +11,27 @@
>   #include "intel_engine_pm.h"
>   #include "intel_engine.h"
>   #include "intel_gt.h"
> +#include "intel_reset.h"
> +
> +/*
> + * While the engine is active, we send a periodic pulse along the engine
> + * to check on its health and to flush any idle-barriers. If that request
> + * is stuck, and we fail to preempt it, we declare the engine hung and
> + * issue a reset -- in the hope that restores progress.
> + */
> +
> +static void next_heartbeat(struct intel_engine_cs *engine)
> +{
> +	long delay;
> +
> +	delay = READ_ONCE(engine->props.heartbeat_interval);
> +	if (!delay)
> +		return;
> +
> +	delay = msecs_to_jiffies_timeout(delay);
> +	schedule_delayed_work(&engine->heartbeat.work,
> +			      round_jiffies_up_relative(delay));
> +}
>   
>   static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
>   {
> @@ -18,6 +39,100 @@ static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
>   	i915_request_add_active_barriers(rq);
>   }
>   
> +static void heartbeat(struct work_struct *wrk)
> +{
> +	struct i915_sched_attr attr = {
> +		.priority = I915_USER_PRIORITY(I915_PRIORITY_MIN),

You were saying it's better to start from zero, right?

> +	};
> +	struct intel_engine_cs *engine =
> +		container_of(wrk, typeof(*engine), heartbeat.work.work);
> +	struct intel_context *ce = engine->kernel_context;
> +	struct i915_request *rq;
> +
> +	if (!intel_engine_pm_get_if_awake(engine))
> +		return;
> +
> +	rq = engine->heartbeat.systole;
> +	if (rq && i915_request_completed(rq)) {
> +		i915_request_put(rq);
> +		engine->heartbeat.systole = NULL;
> +	}
> +
> +	if (intel_gt_is_wedged(engine->gt))
> +		goto out;
> +
> +	if (engine->heartbeat.systole) {
> +		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {
> +			struct drm_printer p = drm_debug_printer(__func__);
> +
> +			intel_engine_dump(engine, &p,
> +					  "%s heartbeat not ticking\n",
> +					  engine->name);

This could perhaps be better only when we have reached a higher priority 
attempt. Okay it's under DEBUG_GEM but still, not sure there is value in 
being so panicky if for any reason preemption does not work. Heartbeat 
does not depend on preemption as far as I could spot, right?

> +		}
> +
> +		if (engine->schedule &&
> +		    rq->sched.attr.priority < I915_PRIORITY_BARRIER) {
> +			attr.priority =
> +				I915_USER_PRIORITY(I915_PRIORITY_HEARTBEAT);
> +			if (rq->sched.attr.priority >= attr.priority)
> +				attr.priority = I915_PRIORITY_BARRIER;
> +
> +			local_bh_disable();
> +			engine->schedule(rq, &attr);
> +			local_bh_enable();
> +		} else {
> +			intel_gt_handle_error(engine->gt, engine->mask,
> +					      I915_ERROR_CAPTURE,
> +					      "stopped heartbeat on %s",
> +					      engine->name);
> +		}
> +		goto out;
> +	}
> +
> +	if (engine->wakeref_serial == engine->serial)
> +		goto out;
> +
> +	mutex_lock(&ce->timeline->mutex);
> +
> +	intel_context_enter(ce);
> +	rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
> +	intel_context_exit(ce);
> +	if (IS_ERR(rq))
> +		goto unlock;
> +
> +	idle_pulse(engine, rq);
> +	if (i915_modparams.enable_hangcheck)
> +		engine->heartbeat.systole = i915_request_get(rq);
> +
> +	__i915_request_commit(rq);
> +	__i915_request_queue(rq, &attr);
> +
> +unlock:
> +	mutex_unlock(&ce->timeline->mutex);
> +out:
> +	next_heartbeat(engine);
> +	intel_engine_pm_put(engine);
> +}
> +
> +void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine)
> +{
> +	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
> +		return;
> +
> +	next_heartbeat(engine);
> +}
> +
> +void intel_engine_park_heartbeat(struct intel_engine_cs *engine)
> +{
> +	cancel_delayed_work(&engine->heartbeat.work);
> +	i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
> +}
> +
> +void intel_engine_init_heartbeat(struct intel_engine_cs *engine)
> +{
> +	INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat);
> +}
> +
>   int intel_engine_pulse(struct intel_engine_cs *engine)
>   {
>   	struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
> index b950451b5998..39391004554d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
> @@ -9,6 +9,11 @@
>   
>   struct intel_engine_cs;
>   
> +void intel_engine_init_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);
> +
>   int intel_engine_pulse(struct intel_engine_cs *engine);
>   
>   #endif /* INTEL_ENGINE_HEARTBEAT_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index 7d76611d9df1..6fbfa2162e54 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -7,6 +7,7 @@
>   #include "i915_drv.h"
>   
>   #include "intel_engine.h"
> +#include "intel_engine_heartbeat.h"
>   #include "intel_engine_pm.h"
>   #include "intel_engine_pool.h"
>   #include "intel_gt.h"
> @@ -34,7 +35,7 @@ static int __engine_unpark(struct intel_wakeref *wf)
>   	if (engine->unpark)
>   		engine->unpark(engine);
>   
> -	intel_engine_init_hangcheck(engine);
> +	intel_engine_unpark_heartbeat(engine);
>   	return 0;
>   }
>   
> @@ -158,6 +159,7 @@ static int __engine_park(struct intel_wakeref *wf)
>   
>   	call_idle_barriers(engine); /* cleanup after wedging */
>   
> +	intel_engine_park_heartbeat(engine);
>   	intel_engine_disarm_breadcrumbs(engine);
>   	intel_engine_pool_park(&engine->pool);
>   
> @@ -188,6 +190,7 @@ void intel_engine_init__pm(struct intel_engine_cs *engine)
>   	struct intel_runtime_pm *rpm = engine->uncore->rpm;
>   
>   	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
> +	intel_engine_init_heartbeat(engine);
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
> index aac26097c916..8532f9cdc885 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
> @@ -70,12 +70,38 @@ preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
>   	return count;
>   }
>   
> +static ssize_t
> +heartbeat_interval_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	struct intel_engine_cs *engine = kobj_to_engine(kobj);
> +
> +	return sprintf(buf, "%lu\n", engine->props.heartbeat_interval);
> +}
> +
> +static ssize_t
> +heartbeat_interval_store(struct kobject *kobj, struct kobj_attribute *attr,
> +			 const char *buf, size_t count)
> +{
> +	struct intel_engine_cs *engine = kobj_to_engine(kobj);
> +	unsigned long delay;
> +	int err;
> +
> +	err = kstrtoul(buf, 0, &delay);
> +	if (err)
> +		return err;
> +
> +	engine->props.heartbeat_interval = delay;
> +	return count;
> +}
> +
>   static struct kobj_attribute name_attr = __ATTR(name, 0444, name_show, NULL);
>   static struct kobj_attribute class_attr = __ATTR(class, 0444, class_show, NULL);
>   static struct kobj_attribute inst_attr = __ATTR(instance, 0444, inst_show, NULL);
>   static struct kobj_attribute mmio_attr = __ATTR(mmio_base, 0444, mmio_show, NULL);
>   static struct kobj_attribute preempt_timeout_attr =
>   __ATTR(preempt_timeout_ms, 0600, preempt_timeout_show, preempt_timeout_store);
> +static struct kobj_attribute heartbeat_interval_attr =
> +__ATTR(heartbeat_interval_ms, 0600, heartbeat_interval_show, heartbeat_interval_store);
>   
>   static void kobj_engine_release(struct kobject *kobj)
>   {
> @@ -115,6 +141,9 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
>   		&class_attr.attr,
>   		&inst_attr.attr,
>   		&mmio_attr.attr,
> +#if CONFIG_DRM_I915_HEARTBEAT_INTERVAL
> +		&heartbeat_interval_attr.attr,
> +#endif

Presumably compiler is happy (or the linker) with only this part getting 
the #ifdef treatment? (The show/store functions above don't have it.)

>   		NULL
>   	};
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 6af9b0096975..ad3be2fbd71a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -15,6 +15,7 @@
>   #include <linux/rbtree.h>
>   #include <linux/timer.h>
>   #include <linux/types.h>
> +#include <linux/workqueue.h>
>   
>   #include "i915_gem.h"
>   #include "i915_pmu.h"
> @@ -76,14 +77,6 @@ struct intel_instdone {
>   	u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
>   };
>   
> -struct intel_engine_hangcheck {
> -	u64 acthd;
> -	u32 last_ring;
> -	u32 last_head;
> -	unsigned long action_timestamp;
> -	struct intel_instdone instdone;
> -};
> -
>   struct intel_ring {
>   	struct kref ref;
>   	struct i915_vma *vma;
> @@ -330,6 +323,11 @@ struct intel_engine_cs {
>   
>   	intel_engine_mask_t saturated; /* submitting semaphores too late? */
>   
> +	struct {
> +		struct delayed_work work;
> +		struct i915_request *systole;
> +	} heartbeat;
> +
>   	unsigned long serial;
>   
>   	unsigned long wakeref_serial;
> @@ -480,8 +478,6 @@ struct intel_engine_cs {
>   	/* status_notifier: list of callbacks for context-switch changes */
>   	struct atomic_notifier_head context_status_notifier;
>   
> -	struct intel_engine_hangcheck hangcheck;
> -
>   #define I915_ENGINE_NEEDS_CMD_PARSER BIT(0)
>   #define I915_ENGINE_SUPPORTS_STATS   BIT(1)
>   #define I915_ENGINE_HAS_PREEMPTION   BIT(2)
> @@ -549,6 +545,7 @@ struct intel_engine_cs {
>   
>   	struct {
>   		unsigned long preempt_timeout;
> +		unsigned long heartbeat_interval;
>   	} props;
>   };
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index b3619a2a5d0e..f3e1925987e1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -22,7 +22,6 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
>   	INIT_LIST_HEAD(&gt->closed_vma);
>   	spin_lock_init(&gt->closed_lock);
>   
> -	intel_gt_init_hangcheck(gt);
>   	intel_gt_init_reset(gt);
>   	intel_gt_init_requests(gt);
>   	intel_gt_pm_init_early(gt);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
> index e6ab0bff0efb..5b6effed3713 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -46,8 +46,6 @@ void intel_gt_clear_error_registers(struct intel_gt *gt,
>   void intel_gt_flush_ggtt_writes(struct intel_gt *gt);
>   void intel_gt_chipset_flush(struct intel_gt *gt);
>   
> -void intel_gt_init_hangcheck(struct intel_gt *gt);
> -
>   static inline u32 intel_gt_scratch_offset(const struct intel_gt *gt,
>   					  enum intel_gt_scratch_field field)
>   {
> @@ -59,6 +57,4 @@ static inline bool intel_gt_is_wedged(struct intel_gt *gt)
>   	return __intel_reset_failed(&gt->reset);
>   }
>   
> -void intel_gt_queue_hangcheck(struct intel_gt *gt);
> -
>   #endif /* __INTEL_GT_H__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 87e34e0b6427..85af0d16f869 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -52,7 +52,6 @@ static int __gt_unpark(struct intel_wakeref *wf)
>   
>   	i915_pmu_gt_unparked(i915);
>   
> -	intel_gt_queue_hangcheck(gt);
>   	intel_gt_unpark_requests(gt);
>   
>   	pm_notify(gt, INTEL_GT_UNPARK);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index 802f516a3430..59f8ee0aa151 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -26,14 +26,6 @@ struct i915_ggtt;
>   struct intel_engine_cs;
>   struct intel_uncore;
>   
> -struct intel_hangcheck {
> -	/* For hangcheck timer */
> -#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
> -#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
> -
> -	struct delayed_work work;
> -};
> -
>   struct intel_gt {
>   	struct drm_i915_private *i915;
>   	struct intel_uncore *uncore;
> @@ -67,7 +59,6 @@ struct intel_gt {
>   	struct list_head closed_vma;
>   	spinlock_t closed_lock; /* guards the list of closed_vma */
>   
> -	struct intel_hangcheck hangcheck;
>   	struct intel_reset reset;
>   
>   	/**
> diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> deleted file mode 100644
> index c14dbeb3ccc3..000000000000
> --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> +++ /dev/null
> @@ -1,361 +0,0 @@
> -/*
> - * Copyright © 2016 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> - * IN THE SOFTWARE.
> - *
> - */
> -
> -#include "i915_drv.h"
> -#include "intel_engine.h"
> -#include "intel_gt.h"
> -#include "intel_reset.h"
> -
> -struct hangcheck {
> -	u64 acthd;
> -	u32 ring;
> -	u32 head;
> -	enum intel_engine_hangcheck_action action;
> -	unsigned long action_timestamp;
> -	int deadlock;
> -	struct intel_instdone instdone;
> -	bool wedged:1;
> -	bool stalled:1;
> -};
> -
> -static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone)
> -{
> -	u32 tmp = current_instdone | *old_instdone;
> -	bool unchanged;
> -
> -	unchanged = tmp == *old_instdone;
> -	*old_instdone |= tmp;
> -
> -	return unchanged;
> -}
> -
> -static bool subunits_stuck(struct intel_engine_cs *engine)
> -{
> -	struct drm_i915_private *dev_priv = engine->i915;
> -	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
> -	struct intel_instdone instdone;
> -	struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
> -	bool stuck;
> -	int slice;
> -	int subslice;
> -
> -	intel_engine_get_instdone(engine, &instdone);
> -
> -	/* There might be unstable subunit states even when
> -	 * actual head is not moving. Filter out the unstable ones by
> -	 * accumulating the undone -> done transitions and only
> -	 * consider those as progress.
> -	 */
> -	stuck = instdone_unchanged(instdone.instdone,
> -				   &accu_instdone->instdone);
> -	stuck &= instdone_unchanged(instdone.slice_common,
> -				    &accu_instdone->slice_common);
> -
> -	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) {
> -		stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
> -					    &accu_instdone->sampler[slice][subslice]);
> -		stuck &= instdone_unchanged(instdone.row[slice][subslice],
> -					    &accu_instdone->row[slice][subslice]);
> -	}
> -
> -	return stuck;
> -}
> -
> -static enum intel_engine_hangcheck_action
> -head_stuck(struct intel_engine_cs *engine, u64 acthd)
> -{
> -	if (acthd != engine->hangcheck.acthd) {
> -
> -		/* Clear subunit states on head movement */
> -		memset(&engine->hangcheck.instdone, 0,
> -		       sizeof(engine->hangcheck.instdone));
> -
> -		return ENGINE_ACTIVE_HEAD;
> -	}
> -
> -	if (!subunits_stuck(engine))
> -		return ENGINE_ACTIVE_SUBUNITS;
> -
> -	return ENGINE_DEAD;
> -}
> -
> -static enum intel_engine_hangcheck_action
> -engine_stuck(struct intel_engine_cs *engine, u64 acthd)
> -{
> -	enum intel_engine_hangcheck_action ha;
> -	u32 tmp;
> -
> -	ha = head_stuck(engine, acthd);
> -	if (ha != ENGINE_DEAD)
> -		return ha;
> -
> -	if (IS_GEN(engine->i915, 2))
> -		return ENGINE_DEAD;
> -
> -	/* Is the chip hanging on a WAIT_FOR_EVENT?
> -	 * If so we can simply poke the RB_WAIT bit
> -	 * and break the hang. This should work on
> -	 * all but the second generation chipsets.
> -	 */
> -	tmp = ENGINE_READ(engine, RING_CTL);
> -	if (tmp & RING_WAIT) {
> -		intel_gt_handle_error(engine->gt, engine->mask, 0,
> -				      "stuck wait on %s", engine->name);
> -		ENGINE_WRITE(engine, RING_CTL, tmp);
> -		return ENGINE_WAIT_KICK;
> -	}
> -
> -	return ENGINE_DEAD;
> -}
> -
> -static void hangcheck_load_sample(struct intel_engine_cs *engine,
> -				  struct hangcheck *hc)
> -{
> -	hc->acthd = intel_engine_get_active_head(engine);
> -	hc->ring = ENGINE_READ(engine, RING_START);
> -	hc->head = ENGINE_READ(engine, RING_HEAD);
> -}
> -
> -static void hangcheck_store_sample(struct intel_engine_cs *engine,
> -				   const struct hangcheck *hc)
> -{
> -	engine->hangcheck.acthd = hc->acthd;
> -	engine->hangcheck.last_ring = hc->ring;
> -	engine->hangcheck.last_head = hc->head;
> -}
> -
> -static enum intel_engine_hangcheck_action
> -hangcheck_get_action(struct intel_engine_cs *engine,
> -		     const struct hangcheck *hc)
> -{
> -	if (intel_engine_is_idle(engine))
> -		return ENGINE_IDLE;
> -
> -	if (engine->hangcheck.last_ring != hc->ring)
> -		return ENGINE_ACTIVE_SEQNO;
> -
> -	if (engine->hangcheck.last_head != hc->head)
> -		return ENGINE_ACTIVE_SEQNO;
> -
> -	return engine_stuck(engine, hc->acthd);
> -}
> -
> -static void hangcheck_accumulate_sample(struct intel_engine_cs *engine,
> -					struct hangcheck *hc)
> -{
> -	unsigned long timeout = I915_ENGINE_DEAD_TIMEOUT;
> -
> -	hc->action = hangcheck_get_action(engine, hc);
> -
> -	/* We always increment the progress
> -	 * if the engine is busy and still processing
> -	 * the same request, so that no single request
> -	 * can run indefinitely (such as a chain of
> -	 * batches). The only time we do not increment
> -	 * the hangcheck score on this ring, if this
> -	 * engine is in a legitimate wait for another
> -	 * engine. In that case the waiting engine is a
> -	 * victim and we want to be sure we catch the
> -	 * right culprit. Then every time we do kick
> -	 * the ring, make it as a progress as the seqno
> -	 * advancement might ensure and if not, it
> -	 * will catch the hanging engine.
> -	 */
> -
> -	switch (hc->action) {
> -	case ENGINE_IDLE:
> -	case ENGINE_ACTIVE_SEQNO:
> -		/* Clear head and subunit states on seqno movement */
> -		hc->acthd = 0;
> -
> -		memset(&engine->hangcheck.instdone, 0,
> -		       sizeof(engine->hangcheck.instdone));
> -
> -		/* Intentional fall through */
> -	case ENGINE_WAIT_KICK:
> -	case ENGINE_WAIT:
> -		engine->hangcheck.action_timestamp = jiffies;
> -		break;
> -
> -	case ENGINE_ACTIVE_HEAD:
> -	case ENGINE_ACTIVE_SUBUNITS:
> -		/*
> -		 * Seqno stuck with still active engine gets leeway,
> -		 * in hopes that it is just a long shader.
> -		 */
> -		timeout = I915_SEQNO_DEAD_TIMEOUT;
> -		break;
> -
> -	case ENGINE_DEAD:
> -		break;
> -
> -	default:
> -		MISSING_CASE(hc->action);
> -	}
> -
> -	hc->stalled = time_after(jiffies,
> -				 engine->hangcheck.action_timestamp + timeout);
> -	hc->wedged = time_after(jiffies,
> -				 engine->hangcheck.action_timestamp +
> -				 I915_ENGINE_WEDGED_TIMEOUT);
> -}
> -
> -static void hangcheck_declare_hang(struct intel_gt *gt,
> -				   intel_engine_mask_t hung,
> -				   intel_engine_mask_t stuck)
> -{
> -	struct intel_engine_cs *engine;
> -	intel_engine_mask_t tmp;
> -	char msg[80];
> -	int len;
> -
> -	/* If some rings hung but others were still busy, only
> -	 * blame the hanging rings in the synopsis.
> -	 */
> -	if (stuck != hung)
> -		hung &= ~stuck;
> -	len = scnprintf(msg, sizeof(msg),
> -			"%s on ", stuck == hung ? "no progress" : "hang");
> -	for_each_engine_masked(engine, gt->i915, hung, tmp)
> -		len += scnprintf(msg + len, sizeof(msg) - len,
> -				 "%s, ", engine->name);
> -	msg[len-2] = '\0';
> -
> -	return intel_gt_handle_error(gt, hung, I915_ERROR_CAPTURE, "%s", msg);
> -}
> -
> -/*
> - * This is called when the chip hasn't reported back with completed
> - * batchbuffers in a long time. We keep track per ring seqno progress and
> - * if there are no progress, hangcheck score for that ring is increased.
> - * Further, acthd is inspected to see if the ring is stuck. On stuck case
> - * we kick the ring. If we see no progress on three subsequent calls
> - * we assume chip is wedged and try to fix it by resetting the chip.
> - */
> -static void hangcheck_elapsed(struct work_struct *work)
> -{
> -	struct intel_gt *gt =
> -		container_of(work, typeof(*gt), hangcheck.work.work);
> -	intel_engine_mask_t hung = 0, stuck = 0, wedged = 0;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	intel_wakeref_t wakeref;
> -
> -	if (!i915_modparams.enable_hangcheck)
> -		return;
> -
> -	if (!READ_ONCE(gt->awake))
> -		return;
> -
> -	if (intel_gt_is_wedged(gt))
> -		return;
> -
> -	wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
> -	if (!wakeref)
> -		return;
> -
> -	/* As enabling the GPU requires fairly extensive mmio access,
> -	 * periodically arm the mmio checker to see if we are triggering
> -	 * any invalid access.
> -	 */
> -	intel_uncore_arm_unclaimed_mmio_detection(gt->uncore);
> -
> -	for_each_engine(engine, gt->i915, id) {
> -		struct hangcheck hc;
> -
> -		intel_engine_breadcrumbs_irq(engine);
> -
> -		hangcheck_load_sample(engine, &hc);
> -		hangcheck_accumulate_sample(engine, &hc);
> -		hangcheck_store_sample(engine, &hc);
> -
> -		if (hc.stalled) {
> -			hung |= engine->mask;
> -			if (hc.action != ENGINE_DEAD)
> -				stuck |= engine->mask;
> -		}
> -
> -		if (hc.wedged)
> -			wedged |= engine->mask;
> -	}
> -
> -	if (GEM_SHOW_DEBUG() && (hung | stuck)) {
> -		struct drm_printer p = drm_debug_printer("hangcheck");
> -
> -		for_each_engine(engine, gt->i915, id) {
> -			if (intel_engine_is_idle(engine))
> -				continue;
> -
> -			intel_engine_dump(engine, &p, "%s\n", engine->name);
> -		}
> -	}
> -
> -	if (wedged) {
> -		dev_err(gt->i915->drm.dev,
> -			"GPU recovery timed out,"
> -			" cancelling all in-flight rendering.\n");
> -		GEM_TRACE_DUMP();
> -		intel_gt_set_wedged(gt);
> -	}
> -
> -	if (hung)
> -		hangcheck_declare_hang(gt, hung, stuck);
> -
> -	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
> -
> -	/* Reset timer in case GPU hangs without another request being added */
> -	intel_gt_queue_hangcheck(gt);
> -}
> -
> -void intel_gt_queue_hangcheck(struct intel_gt *gt)
> -{
> -	unsigned long delay;
> -
> -	if (unlikely(!i915_modparams.enable_hangcheck))
> -		return;
> -
> -	/*
> -	 * Don't continually defer the hangcheck so that it is always run at
> -	 * least once after work has been scheduled on any ring. Otherwise,
> -	 * we will ignore a hung ring if a second ring is kept busy.
> -	 */
> -
> -	delay = round_jiffies_up_relative(DRM_I915_HANGCHECK_JIFFIES);
> -	queue_delayed_work(system_long_wq, &gt->hangcheck.work, delay);
> -}
> -
> -void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
> -{
> -	memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
> -	engine->hangcheck.action_timestamp = jiffies;
> -}
> -
> -void intel_gt_init_hangcheck(struct intel_gt *gt)
> -{
> -	INIT_DELAYED_WORK(&gt->hangcheck.work, hangcheck_elapsed);
> -}
> -
> -#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> -#include "selftest_hangcheck.c"
> -#endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 34791fc79dea..9ed2cf91a46d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1018,8 +1018,6 @@ void intel_gt_reset(struct intel_gt *gt,
>   	if (ret)
>   		goto taint;
>   
> -	intel_gt_queue_hangcheck(gt);
> -
>   finish:
>   	reset_finish(gt, awake);
>   unlock:
> @@ -1347,4 +1345,5 @@ void __intel_fini_wedge(struct intel_wedge_me *w)
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>   #include "selftest_reset.c"
> +#include "selftest_hangcheck.c"
>   #endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 569a4105d49e..570546eda5e8 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -1686,7 +1686,6 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
>   	};
>   	struct intel_gt *gt = &i915->gt;
>   	intel_wakeref_t wakeref;
> -	bool saved_hangcheck;
>   	int err;
>   
>   	if (!intel_has_gpu_reset(gt))
> @@ -1696,12 +1695,9 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
>   		return -EIO; /* we're long past hope of a successful reset */
>   
>   	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
> -	saved_hangcheck = fetch_and_zero(&i915_modparams.enable_hangcheck);
> -	drain_delayed_work(&gt->hangcheck.work); /* flush param */
>   
>   	err = intel_gt_live_subtests(tests, gt);
>   
> -	i915_modparams.enable_hangcheck = saved_hangcheck;
>   	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
>   
>   	return err;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 277f31297f29..55852e045c3a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1011,92 +1011,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>   	return ret;
>   }
>   
> -static void i915_instdone_info(struct drm_i915_private *dev_priv,
> -			       struct seq_file *m,
> -			       struct intel_instdone *instdone)
> -{
> -	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
> -	int slice;
> -	int subslice;
> -
> -	seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
> -		   instdone->instdone);
> -
> -	if (INTEL_GEN(dev_priv) <= 3)
> -		return;
> -
> -	seq_printf(m, "\t\tSC_INSTDONE: 0x%08x\n",
> -		   instdone->slice_common);
> -
> -	if (INTEL_GEN(dev_priv) <= 6)
> -		return;
> -
> -	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
> -		seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> -			   slice, subslice, instdone->sampler[slice][subslice]);
> -
> -	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
> -		seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
> -			   slice, subslice, instdone->row[slice][subslice]);
> -}
> -
> -static int i915_hangcheck_info(struct seq_file *m, void *unused)
> -{
> -	struct drm_i915_private *i915 = node_to_i915(m->private);
> -	struct intel_gt *gt = &i915->gt;
> -	struct intel_engine_cs *engine;
> -	intel_wakeref_t wakeref;
> -	enum intel_engine_id id;
> -
> -	seq_printf(m, "Reset flags: %lx\n", gt->reset.flags);
> -	if (test_bit(I915_WEDGED, &gt->reset.flags))
> -		seq_puts(m, "\tWedged\n");
> -	if (test_bit(I915_RESET_BACKOFF, &gt->reset.flags))
> -		seq_puts(m, "\tDevice (global) reset in progress\n");
> -
> -	if (!i915_modparams.enable_hangcheck) {
> -		seq_puts(m, "Hangcheck disabled\n");
> -		return 0;
> -	}
> -
> -	if (timer_pending(&gt->hangcheck.work.timer))
> -		seq_printf(m, "Hangcheck active, timer fires in %dms\n",
> -			   jiffies_to_msecs(gt->hangcheck.work.timer.expires -
> -					    jiffies));
> -	else if (delayed_work_pending(&gt->hangcheck.work))
> -		seq_puts(m, "Hangcheck active, work pending\n");
> -	else
> -		seq_puts(m, "Hangcheck inactive\n");
> -
> -	seq_printf(m, "GT active? %s\n", yesno(gt->awake));
> -
> -	with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> -		for_each_engine(engine, i915, id) {
> -			struct intel_instdone instdone;
> -
> -			seq_printf(m, "%s: %d ms ago\n",
> -				   engine->name,
> -				   jiffies_to_msecs(jiffies -
> -						    engine->hangcheck.action_timestamp));
> -
> -			seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
> -				   (long long)engine->hangcheck.acthd,
> -				   intel_engine_get_active_head(engine));
> -
> -			intel_engine_get_instdone(engine, &instdone);
> -
> -			seq_puts(m, "\tinstdone read =\n");
> -			i915_instdone_info(i915, m, &instdone);
> -
> -			seq_puts(m, "\tinstdone accu =\n");
> -			i915_instdone_info(i915, m,
> -					   &engine->hangcheck.instdone);
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>   static int ironlake_drpc_info(struct seq_file *m)
>   {
>   	struct drm_i915_private *i915 = node_to_i915(m->private);
> @@ -4303,7 +4217,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_guc_stage_pool", i915_guc_stage_pool, 0},
>   	{"i915_huc_load_status", i915_huc_load_status_info, 0},
>   	{"i915_frequency_info", i915_frequency_info, 0},
> -	{"i915_hangcheck_info", i915_hangcheck_info, 0},
>   	{"i915_drpc_info", i915_drpc_info, 0},
>   	{"i915_ring_freq_table", i915_ring_freq_table, 0},
>   	{"i915_frontbuffer_tracking", i915_frontbuffer_tracking, 0},
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f02a34722217..1dae43ed4c48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1546,10 +1546,7 @@ void i915_driver_remove(struct drm_i915_private *i915)
>   
>   	i915_driver_modeset_remove(i915);
>   
> -	/* Free error state after interrupts are fully disabled. */
> -	cancel_delayed_work_sync(&i915->gt.hangcheck.work);
>   	i915_reset_error_state(i915);
> -
>   	i915_gem_driver_remove(i915);
>   
>   	intel_power_domains_driver_remove(i915);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d284b04c492b..58340c99af02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1845,7 +1845,6 @@ void i915_driver_remove(struct drm_i915_private *i915);
>   int i915_resume_switcheroo(struct drm_i915_private *i915);
>   int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
>   
> -void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
>   int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
>   
>   static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 5cf4eed5add8..47239df653f2 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -534,10 +534,6 @@ static void error_print_engine(struct drm_i915_error_state_buf *m,
>   	}
>   	err_printf(m, "  ring->head: 0x%08x\n", ee->cpu_ring_head);
>   	err_printf(m, "  ring->tail: 0x%08x\n", ee->cpu_ring_tail);
> -	err_printf(m, "  hangcheck timestamp: %dms (%lu%s)\n",
> -		   jiffies_to_msecs(ee->hangcheck_timestamp - epoch),
> -		   ee->hangcheck_timestamp,
> -		   ee->hangcheck_timestamp == epoch ? "; epoch" : "");
>   	err_printf(m, "  engine reset count: %u\n", ee->reset_count);
>   
>   	for (n = 0; n < ee->num_ports; n++) {
> @@ -679,11 +675,8 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>   	ts = ktime_to_timespec64(error->uptime);
>   	err_printf(m, "Uptime: %lld s %ld us\n",
>   		   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
> -	err_printf(m, "Epoch: %lu jiffies (%u HZ)\n", error->epoch, HZ);
> -	err_printf(m, "Capture: %lu jiffies; %d ms ago, %d ms after epoch\n",
> -		   error->capture,
> -		   jiffies_to_msecs(jiffies - error->capture),
> -		   jiffies_to_msecs(error->capture - error->epoch));
> +	err_printf(m, "Capture: %lu jiffies; %d ms ago\n",
> +		   error->capture, jiffies_to_msecs(jiffies - error->capture));
>   
>   	for (ee = error->engine; ee; ee = ee->next)
>   		err_printf(m, "Active process (on ring %s): %s [%d]\n",
> @@ -742,7 +735,7 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>   		err_printf(m, "GTT_CACHE_EN: 0x%08x\n", error->gtt_cache);
>   
>   	for (ee = error->engine; ee; ee = ee->next)
> -		error_print_engine(m, ee, error->epoch);
> +		error_print_engine(m, ee, error->capture);
>   
>   	for (ee = error->engine; ee; ee = ee->next) {
>   		const struct drm_i915_error_object *obj;
> @@ -770,7 +763,7 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>   			for (j = 0; j < ee->num_requests; j++)
>   				error_print_request(m, " ",
>   						    &ee->requests[j],
> -						    error->epoch);
> +						    error->capture);
>   		}
>   
>   		print_error_obj(m, ee->engine, "ringbuffer", ee->ringbuffer);
> @@ -1144,8 +1137,6 @@ static void error_record_engine_registers(struct i915_gpu_state *error,
>   	}
>   
>   	ee->idle = intel_engine_is_idle(engine);
> -	if (!ee->idle)
> -		ee->hangcheck_timestamp = engine->hangcheck.action_timestamp;
>   	ee->reset_count = i915_reset_engine_count(&dev_priv->gpu_error,
>   						  engine);
>   
> @@ -1657,20 +1648,6 @@ static void capture_params(struct i915_gpu_state *error)
>   	i915_params_copy(&error->params, &i915_modparams);
>   }
>   
> -static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
> -{
> -	const struct drm_i915_error_engine *ee;
> -	unsigned long epoch = error->capture;
> -
> -	for (ee = error->engine; ee; ee = ee->next) {
> -		if (ee->hangcheck_timestamp &&
> -		    time_before(ee->hangcheck_timestamp, epoch))
> -			epoch = ee->hangcheck_timestamp;
> -	}
> -
> -	return epoch;
> -}
> -
>   static void capture_finish(struct i915_gpu_state *error)
>   {
>   	struct i915_ggtt *ggtt = &error->i915->ggtt;
> @@ -1722,8 +1699,6 @@ i915_capture_gpu_state(struct drm_i915_private *i915)
>   	error->overlay = intel_overlay_capture_error_state(i915);
>   	error->display = intel_display_capture_error_state(i915);
>   
> -	error->epoch = capture_find_epoch(error);
> -
>   	capture_finish(error);
>   	compress_fini(&compress);
>   
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
> index 7f1cd0b1fef7..4dc36d6ee3a2 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> @@ -34,7 +34,6 @@ struct i915_gpu_state {
>   	ktime_t boottime;
>   	ktime_t uptime;
>   	unsigned long capture;
> -	unsigned long epoch;
>   
>   	struct drm_i915_private *i915;
>   
> @@ -86,7 +85,6 @@ struct i915_gpu_state {
>   
>   		/* Software tracked state */
>   		bool idle;
> -		unsigned long hangcheck_timestamp;
>   		int num_requests;
>   		u32 reset_count;
>   
> diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h b/drivers/gpu/drm/i915/i915_priolist_types.h
> index ae8bb3cb627e..732aad148881 100644
> --- a/drivers/gpu/drm/i915/i915_priolist_types.h
> +++ b/drivers/gpu/drm/i915/i915_priolist_types.h
> @@ -16,6 +16,12 @@ enum {
>   	I915_PRIORITY_MIN = I915_CONTEXT_MIN_USER_PRIORITY - 1,
>   	I915_PRIORITY_NORMAL = I915_CONTEXT_DEFAULT_PRIORITY,
>   	I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
> +
> +	/* A preemptive pulse used to monitor the health of each engine */
> +	I915_PRIORITY_HEARTBEAT,
> +
> +	/* Interactive workload, scheduled for immediate pageflipping */
> +	I915_PRIORITY_DISPLAY,
>   };
>   
>   #define I915_USER_PRIORITY_SHIFT 2
> 

Regards,

Tvrtko
Chris Wilson Oct. 11, 2019, 3:06 p.m. UTC | #2
Quoting Tvrtko Ursulin (2019-10-11 15:24:21)
> 
> On 10/10/2019 08:14, Chris Wilson wrote:
> > +config DRM_I915_HEARTBEAT_INTERVAL
> > +     int "Interval between heartbeat pulses (ms)"
> > +     default 2500 # milliseconds
> > +     help
> > +       While active the driver uses a periodic request, a heartbeat, to
> > +       check the wellness of the GPU and to regularly flush state changes
> > +       (idle barriers).
> 
> Should this be somehow reworded to be more end user friendly? My idea, 
> may need to be corrected for bad English:

End user friendly. Sure, but that means I didn't hide this well enough
;)
 
> The driver sends a periodic heartbeat down all active GT engines to 
> check the health of the GPU and undertake regular house-keeping of 
> internal driver state.
> 
> Main points from the user perspective: "request" - whaat? "idle 
> barriers" - ditto. "Wellness" - a bit unusual in this context, no?

> > +static void heartbeat(struct work_struct *wrk)
> > +{
> > +     struct i915_sched_attr attr = {
> > +             .priority = I915_USER_PRIORITY(I915_PRIORITY_MIN),
> 
> You were saying it's better to start from zero, right?

The first bump. Starting at lowest, means run when first idle. Then we
jump to 0 and be scheduled like any other normal user.

> > +     };
> > +     struct intel_engine_cs *engine =
> > +             container_of(wrk, typeof(*engine), heartbeat.work.work);
> > +     struct intel_context *ce = engine->kernel_context;
> > +     struct i915_request *rq;
> > +
> > +     if (!intel_engine_pm_get_if_awake(engine))
> > +             return;
> > +
> > +     rq = engine->heartbeat.systole;
> > +     if (rq && i915_request_completed(rq)) {
> > +             i915_request_put(rq);
> > +             engine->heartbeat.systole = NULL;
> > +     }
> > +
> > +     if (intel_gt_is_wedged(engine->gt))
> > +             goto out;
> > +
> > +     if (engine->heartbeat.systole) {
> > +             if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {
> > +                     struct drm_printer p = drm_debug_printer(__func__);
> > +
> > +                     intel_engine_dump(engine, &p,
> > +                                       "%s heartbeat not ticking\n",
> > +                                       engine->name);
> 
> This could perhaps be better only when we have reached a higher priority 
> attempt. Okay it's under DEBUG_GEM but still, not sure there is value in 
> being so panicky if for any reason preemption does not work. Heartbeat 
> does not depend on preemption as far as I could spot, right?

The challenge is evident by the else path where we immediately reset.
If we cause a preemption event from the heartbeat (even strictly at min
prio we could cause a timeslice to expire) it is useful to have the
debug in dmesg (as in CI we don't get error-state very often).

Yes, I've tried trimming it to only on the vital paths, but so far
haven't found a satisfactory means.

To make me happy I think I need to push it down into the reset routines
themselves. Hmm. Except those we definitely don't want dmesg spam as
they get runs 10s of thousands times during CI.

It'll do for now. I'm sure we'll get tired of it and find it a new home.

> > +static struct kobj_attribute heartbeat_interval_attr =
> > +__ATTR(heartbeat_interval_ms, 0600, heartbeat_interval_show, heartbeat_interval_store);
> >   
> >   static void kobj_engine_release(struct kobject *kobj)
> >   {
> > @@ -115,6 +141,9 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
> >               &class_attr.attr,
> >               &inst_attr.attr,
> >               &mmio_attr.attr,
> > +#if CONFIG_DRM_I915_HEARTBEAT_INTERVAL
> > +             &heartbeat_interval_attr.attr,
> > +#endif
> 
> Presumably compiler is happy (or the linker) with only this part getting 
> the #ifdef treatment? (The show/store functions above don't have it.)

Yup, it's not annoying enough to complain about the dead globals. Although
it should be more than smart enough to remove them.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
index 8fceea85937b..d3950aabb497 100644
--- a/drivers/gpu/drm/i915/Kconfig.profile
+++ b/drivers/gpu/drm/i915/Kconfig.profile
@@ -40,3 +40,17 @@  config DRM_I915_PREEMPT_TIMEOUT
 	  /sys/class/drm/card?/engine/*/preempt_timeout_ms
 
 	  May be 0 to disable the timeout.
+
+config DRM_I915_HEARTBEAT_INTERVAL
+	int "Interval between heartbeat pulses (ms)"
+	default 2500 # milliseconds
+	help
+	  While active the driver uses a periodic request, a heartbeat, to
+	  check the wellness of the GPU and to regularly flush state changes
+	  (idle barriers).
+
+	  This is adjustable via
+	  /sys/class/drm/card?/engine/*/heartbeat_interval_ms
+
+	  May be 0 to disable heartbeats and therefore disable automatic GPU
+	  hang detection.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index cfab7c8585b3..59d356cc406c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -88,7 +88,6 @@  gt-y += \
 	gt/intel_gt_pm.o \
 	gt/intel_gt_pm_irq.o \
 	gt/intel_gt_requests.o \
-	gt/intel_hangcheck.o \
 	gt/intel_lrc.o \
 	gt/intel_rc6.o \
 	gt/intel_renderstate.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1a533ccdb54f..5e5de3081f48 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14338,7 +14338,7 @@  static void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
 static void fb_obj_bump_render_priority(struct drm_i915_gem_object *obj)
 {
 	struct i915_sched_attr attr = {
-		.priority = I915_PRIORITY_DISPLAY,
+		.priority = I915_USER_PRIORITY(I915_PRIORITY_DISPLAY),
 	};
 
 	i915_gem_object_wait_priority(obj, 0, &attr);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index c5e14c9c805c..5bd51e397371 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -460,6 +460,5 @@  int i915_gem_object_wait(struct drm_i915_gem_object *obj,
 int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
 				  unsigned int flags,
 				  const struct i915_sched_attr *attr);
-#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
 
 #endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 7987b54fb1f5..0e97520cb1bb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -100,8 +100,6 @@  void i915_gem_suspend(struct drm_i915_private *i915)
 	intel_gt_suspend(&i915->gt);
 	intel_uc_suspend(&i915->gt.uc);
 
-	cancel_delayed_work_sync(&i915->gt.hangcheck.work);
-
 	i915_gem_drain_freed_objects(i915);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 93ea367fe624..8ad57eace351 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -89,38 +89,6 @@  struct drm_printer;
 /* seqno size is actually only a uint32, but since we plan to use MI_FLUSH_DW to
  * do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
  */
-enum intel_engine_hangcheck_action {
-	ENGINE_IDLE = 0,
-	ENGINE_WAIT,
-	ENGINE_ACTIVE_SEQNO,
-	ENGINE_ACTIVE_HEAD,
-	ENGINE_ACTIVE_SUBUNITS,
-	ENGINE_WAIT_KICK,
-	ENGINE_DEAD,
-};
-
-static inline const char *
-hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
-{
-	switch (a) {
-	case ENGINE_IDLE:
-		return "idle";
-	case ENGINE_WAIT:
-		return "wait";
-	case ENGINE_ACTIVE_SEQNO:
-		return "active seqno";
-	case ENGINE_ACTIVE_HEAD:
-		return "active head";
-	case ENGINE_ACTIVE_SUBUNITS:
-		return "active subunits";
-	case ENGINE_WAIT_KICK:
-		return "wait kick";
-	case ENGINE_DEAD:
-		return "dead";
-	}
-
-	return "unknown";
-}
 
 static inline unsigned int
 execlists_num_ports(const struct intel_engine_execlists * const execlists)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1eb51147839a..d829ad340ca0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -305,6 +305,7 @@  static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	__sprint_engine_name(engine);
 
 	engine->props.preempt_timeout = CONFIG_DRM_I915_PREEMPT_TIMEOUT;
+	engine->props.heartbeat_interval = CONFIG_DRM_I915_HEARTBEAT_INTERVAL;
 
 	/*
 	 * To be overridden by the backend on setup. However to facilitate
@@ -599,7 +600,6 @@  static int intel_engine_setup_common(struct intel_engine_cs *engine)
 	intel_engine_init_active(engine, ENGINE_PHYSICAL);
 	intel_engine_init_breadcrumbs(engine);
 	intel_engine_init_execlists(engine);
-	intel_engine_init_hangcheck(engine);
 	intel_engine_init_cmd_parser(engine);
 	intel_engine_init__pm(engine);
 
@@ -1432,8 +1432,13 @@  void intel_engine_dump(struct intel_engine_cs *engine,
 		drm_printf(m, "*** WEDGED ***\n");
 
 	drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
-	drm_printf(m, "\tHangcheck: %d ms ago\n",
-		   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
+
+	rcu_read_lock();
+	rq = READ_ONCE(engine->heartbeat.systole);
+	if (rq)
+		drm_printf(m, "\tHeartbeat: %d ms ago\n",
+			   jiffies_to_msecs(jiffies - rq->emitted_jiffies));
+	rcu_read_unlock();
 	drm_printf(m, "\tReset count: %d (global %d)\n",
 		   i915_reset_engine_count(error, engine),
 		   i915_reset_count(error));
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 2fc413f9d506..f68acf9118f3 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -11,6 +11,27 @@ 
 #include "intel_engine_pm.h"
 #include "intel_engine.h"
 #include "intel_gt.h"
+#include "intel_reset.h"
+
+/*
+ * While the engine is active, we send a periodic pulse along the engine
+ * to check on its health and to flush any idle-barriers. If that request
+ * is stuck, and we fail to preempt it, we declare the engine hung and
+ * issue a reset -- in the hope that restores progress.
+ */
+
+static void next_heartbeat(struct intel_engine_cs *engine)
+{
+	long delay;
+
+	delay = READ_ONCE(engine->props.heartbeat_interval);
+	if (!delay)
+		return;
+
+	delay = msecs_to_jiffies_timeout(delay);
+	schedule_delayed_work(&engine->heartbeat.work,
+			      round_jiffies_up_relative(delay));
+}
 
 static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
 {
@@ -18,6 +39,100 @@  static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
 	i915_request_add_active_barriers(rq);
 }
 
+static void heartbeat(struct work_struct *wrk)
+{
+	struct i915_sched_attr attr = {
+		.priority = I915_USER_PRIORITY(I915_PRIORITY_MIN),
+	};
+	struct intel_engine_cs *engine =
+		container_of(wrk, typeof(*engine), heartbeat.work.work);
+	struct intel_context *ce = engine->kernel_context;
+	struct i915_request *rq;
+
+	if (!intel_engine_pm_get_if_awake(engine))
+		return;
+
+	rq = engine->heartbeat.systole;
+	if (rq && i915_request_completed(rq)) {
+		i915_request_put(rq);
+		engine->heartbeat.systole = NULL;
+	}
+
+	if (intel_gt_is_wedged(engine->gt))
+		goto out;
+
+	if (engine->heartbeat.systole) {
+		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) {
+			struct drm_printer p = drm_debug_printer(__func__);
+
+			intel_engine_dump(engine, &p,
+					  "%s heartbeat not ticking\n",
+					  engine->name);
+		}
+
+		if (engine->schedule &&
+		    rq->sched.attr.priority < I915_PRIORITY_BARRIER) {
+			attr.priority =
+				I915_USER_PRIORITY(I915_PRIORITY_HEARTBEAT);
+			if (rq->sched.attr.priority >= attr.priority)
+				attr.priority = I915_PRIORITY_BARRIER;
+
+			local_bh_disable();
+			engine->schedule(rq, &attr);
+			local_bh_enable();
+		} else {
+			intel_gt_handle_error(engine->gt, engine->mask,
+					      I915_ERROR_CAPTURE,
+					      "stopped heartbeat on %s",
+					      engine->name);
+		}
+		goto out;
+	}
+
+	if (engine->wakeref_serial == engine->serial)
+		goto out;
+
+	mutex_lock(&ce->timeline->mutex);
+
+	intel_context_enter(ce);
+	rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
+	intel_context_exit(ce);
+	if (IS_ERR(rq))
+		goto unlock;
+
+	idle_pulse(engine, rq);
+	if (i915_modparams.enable_hangcheck)
+		engine->heartbeat.systole = i915_request_get(rq);
+
+	__i915_request_commit(rq);
+	__i915_request_queue(rq, &attr);
+
+unlock:
+	mutex_unlock(&ce->timeline->mutex);
+out:
+	next_heartbeat(engine);
+	intel_engine_pm_put(engine);
+}
+
+void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine)
+{
+	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
+		return;
+
+	next_heartbeat(engine);
+}
+
+void intel_engine_park_heartbeat(struct intel_engine_cs *engine)
+{
+	cancel_delayed_work(&engine->heartbeat.work);
+	i915_request_put(fetch_and_zero(&engine->heartbeat.systole));
+}
+
+void intel_engine_init_heartbeat(struct intel_engine_cs *engine)
+{
+	INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat);
+}
+
 int intel_engine_pulse(struct intel_engine_cs *engine)
 {
 	struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
index b950451b5998..39391004554d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
@@ -9,6 +9,11 @@ 
 
 struct intel_engine_cs;
 
+void intel_engine_init_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);
+
 int intel_engine_pulse(struct intel_engine_cs *engine);
 
 #endif /* INTEL_ENGINE_HEARTBEAT_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 7d76611d9df1..6fbfa2162e54 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -7,6 +7,7 @@ 
 #include "i915_drv.h"
 
 #include "intel_engine.h"
+#include "intel_engine_heartbeat.h"
 #include "intel_engine_pm.h"
 #include "intel_engine_pool.h"
 #include "intel_gt.h"
@@ -34,7 +35,7 @@  static int __engine_unpark(struct intel_wakeref *wf)
 	if (engine->unpark)
 		engine->unpark(engine);
 
-	intel_engine_init_hangcheck(engine);
+	intel_engine_unpark_heartbeat(engine);
 	return 0;
 }
 
@@ -158,6 +159,7 @@  static int __engine_park(struct intel_wakeref *wf)
 
 	call_idle_barriers(engine); /* cleanup after wedging */
 
+	intel_engine_park_heartbeat(engine);
 	intel_engine_disarm_breadcrumbs(engine);
 	intel_engine_pool_park(&engine->pool);
 
@@ -188,6 +190,7 @@  void intel_engine_init__pm(struct intel_engine_cs *engine)
 	struct intel_runtime_pm *rpm = engine->uncore->rpm;
 
 	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
+	intel_engine_init_heartbeat(engine);
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
index aac26097c916..8532f9cdc885 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_sysfs.c
@@ -70,12 +70,38 @@  preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
 	return count;
 }
 
+static ssize_t
+heartbeat_interval_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	struct intel_engine_cs *engine = kobj_to_engine(kobj);
+
+	return sprintf(buf, "%lu\n", engine->props.heartbeat_interval);
+}
+
+static ssize_t
+heartbeat_interval_store(struct kobject *kobj, struct kobj_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct intel_engine_cs *engine = kobj_to_engine(kobj);
+	unsigned long delay;
+	int err;
+
+	err = kstrtoul(buf, 0, &delay);
+	if (err)
+		return err;
+
+	engine->props.heartbeat_interval = delay;
+	return count;
+}
+
 static struct kobj_attribute name_attr = __ATTR(name, 0444, name_show, NULL);
 static struct kobj_attribute class_attr = __ATTR(class, 0444, class_show, NULL);
 static struct kobj_attribute inst_attr = __ATTR(instance, 0444, inst_show, NULL);
 static struct kobj_attribute mmio_attr = __ATTR(mmio_base, 0444, mmio_show, NULL);
 static struct kobj_attribute preempt_timeout_attr =
 __ATTR(preempt_timeout_ms, 0600, preempt_timeout_show, preempt_timeout_store);
+static struct kobj_attribute heartbeat_interval_attr =
+__ATTR(heartbeat_interval_ms, 0600, heartbeat_interval_show, heartbeat_interval_store);
 
 static void kobj_engine_release(struct kobject *kobj)
 {
@@ -115,6 +141,9 @@  void intel_engines_add_sysfs(struct drm_i915_private *i915)
 		&class_attr.attr,
 		&inst_attr.attr,
 		&mmio_attr.attr,
+#if CONFIG_DRM_I915_HEARTBEAT_INTERVAL
+		&heartbeat_interval_attr.attr,
+#endif
 		NULL
 	};
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 6af9b0096975..ad3be2fbd71a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -15,6 +15,7 @@ 
 #include <linux/rbtree.h>
 #include <linux/timer.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 
 #include "i915_gem.h"
 #include "i915_pmu.h"
@@ -76,14 +77,6 @@  struct intel_instdone {
 	u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
 };
 
-struct intel_engine_hangcheck {
-	u64 acthd;
-	u32 last_ring;
-	u32 last_head;
-	unsigned long action_timestamp;
-	struct intel_instdone instdone;
-};
-
 struct intel_ring {
 	struct kref ref;
 	struct i915_vma *vma;
@@ -330,6 +323,11 @@  struct intel_engine_cs {
 
 	intel_engine_mask_t saturated; /* submitting semaphores too late? */
 
+	struct {
+		struct delayed_work work;
+		struct i915_request *systole;
+	} heartbeat;
+
 	unsigned long serial;
 
 	unsigned long wakeref_serial;
@@ -480,8 +478,6 @@  struct intel_engine_cs {
 	/* status_notifier: list of callbacks for context-switch changes */
 	struct atomic_notifier_head context_status_notifier;
 
-	struct intel_engine_hangcheck hangcheck;
-
 #define I915_ENGINE_NEEDS_CMD_PARSER BIT(0)
 #define I915_ENGINE_SUPPORTS_STATS   BIT(1)
 #define I915_ENGINE_HAS_PREEMPTION   BIT(2)
@@ -549,6 +545,7 @@  struct intel_engine_cs {
 
 	struct {
 		unsigned long preempt_timeout;
+		unsigned long heartbeat_interval;
 	} props;
 };
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index b3619a2a5d0e..f3e1925987e1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -22,7 +22,6 @@  void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
 	INIT_LIST_HEAD(&gt->closed_vma);
 	spin_lock_init(&gt->closed_lock);
 
-	intel_gt_init_hangcheck(gt);
 	intel_gt_init_reset(gt);
 	intel_gt_init_requests(gt);
 	intel_gt_pm_init_early(gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index e6ab0bff0efb..5b6effed3713 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -46,8 +46,6 @@  void intel_gt_clear_error_registers(struct intel_gt *gt,
 void intel_gt_flush_ggtt_writes(struct intel_gt *gt);
 void intel_gt_chipset_flush(struct intel_gt *gt);
 
-void intel_gt_init_hangcheck(struct intel_gt *gt);
-
 static inline u32 intel_gt_scratch_offset(const struct intel_gt *gt,
 					  enum intel_gt_scratch_field field)
 {
@@ -59,6 +57,4 @@  static inline bool intel_gt_is_wedged(struct intel_gt *gt)
 	return __intel_reset_failed(&gt->reset);
 }
 
-void intel_gt_queue_hangcheck(struct intel_gt *gt);
-
 #endif /* __INTEL_GT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 87e34e0b6427..85af0d16f869 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -52,7 +52,6 @@  static int __gt_unpark(struct intel_wakeref *wf)
 
 	i915_pmu_gt_unparked(i915);
 
-	intel_gt_queue_hangcheck(gt);
 	intel_gt_unpark_requests(gt);
 
 	pm_notify(gt, INTEL_GT_UNPARK);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 802f516a3430..59f8ee0aa151 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -26,14 +26,6 @@  struct i915_ggtt;
 struct intel_engine_cs;
 struct intel_uncore;
 
-struct intel_hangcheck {
-	/* For hangcheck timer */
-#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
-#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
-
-	struct delayed_work work;
-};
-
 struct intel_gt {
 	struct drm_i915_private *i915;
 	struct intel_uncore *uncore;
@@ -67,7 +59,6 @@  struct intel_gt {
 	struct list_head closed_vma;
 	spinlock_t closed_lock; /* guards the list of closed_vma */
 
-	struct intel_hangcheck hangcheck;
 	struct intel_reset reset;
 
 	/**
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
deleted file mode 100644
index c14dbeb3ccc3..000000000000
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ /dev/null
@@ -1,361 +0,0 @@ 
-/*
- * Copyright © 2016 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- */
-
-#include "i915_drv.h"
-#include "intel_engine.h"
-#include "intel_gt.h"
-#include "intel_reset.h"
-
-struct hangcheck {
-	u64 acthd;
-	u32 ring;
-	u32 head;
-	enum intel_engine_hangcheck_action action;
-	unsigned long action_timestamp;
-	int deadlock;
-	struct intel_instdone instdone;
-	bool wedged:1;
-	bool stalled:1;
-};
-
-static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone)
-{
-	u32 tmp = current_instdone | *old_instdone;
-	bool unchanged;
-
-	unchanged = tmp == *old_instdone;
-	*old_instdone |= tmp;
-
-	return unchanged;
-}
-
-static bool subunits_stuck(struct intel_engine_cs *engine)
-{
-	struct drm_i915_private *dev_priv = engine->i915;
-	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
-	struct intel_instdone instdone;
-	struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
-	bool stuck;
-	int slice;
-	int subslice;
-
-	intel_engine_get_instdone(engine, &instdone);
-
-	/* There might be unstable subunit states even when
-	 * actual head is not moving. Filter out the unstable ones by
-	 * accumulating the undone -> done transitions and only
-	 * consider those as progress.
-	 */
-	stuck = instdone_unchanged(instdone.instdone,
-				   &accu_instdone->instdone);
-	stuck &= instdone_unchanged(instdone.slice_common,
-				    &accu_instdone->slice_common);
-
-	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) {
-		stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
-					    &accu_instdone->sampler[slice][subslice]);
-		stuck &= instdone_unchanged(instdone.row[slice][subslice],
-					    &accu_instdone->row[slice][subslice]);
-	}
-
-	return stuck;
-}
-
-static enum intel_engine_hangcheck_action
-head_stuck(struct intel_engine_cs *engine, u64 acthd)
-{
-	if (acthd != engine->hangcheck.acthd) {
-
-		/* Clear subunit states on head movement */
-		memset(&engine->hangcheck.instdone, 0,
-		       sizeof(engine->hangcheck.instdone));
-
-		return ENGINE_ACTIVE_HEAD;
-	}
-
-	if (!subunits_stuck(engine))
-		return ENGINE_ACTIVE_SUBUNITS;
-
-	return ENGINE_DEAD;
-}
-
-static enum intel_engine_hangcheck_action
-engine_stuck(struct intel_engine_cs *engine, u64 acthd)
-{
-	enum intel_engine_hangcheck_action ha;
-	u32 tmp;
-
-	ha = head_stuck(engine, acthd);
-	if (ha != ENGINE_DEAD)
-		return ha;
-
-	if (IS_GEN(engine->i915, 2))
-		return ENGINE_DEAD;
-
-	/* Is the chip hanging on a WAIT_FOR_EVENT?
-	 * If so we can simply poke the RB_WAIT bit
-	 * and break the hang. This should work on
-	 * all but the second generation chipsets.
-	 */
-	tmp = ENGINE_READ(engine, RING_CTL);
-	if (tmp & RING_WAIT) {
-		intel_gt_handle_error(engine->gt, engine->mask, 0,
-				      "stuck wait on %s", engine->name);
-		ENGINE_WRITE(engine, RING_CTL, tmp);
-		return ENGINE_WAIT_KICK;
-	}
-
-	return ENGINE_DEAD;
-}
-
-static void hangcheck_load_sample(struct intel_engine_cs *engine,
-				  struct hangcheck *hc)
-{
-	hc->acthd = intel_engine_get_active_head(engine);
-	hc->ring = ENGINE_READ(engine, RING_START);
-	hc->head = ENGINE_READ(engine, RING_HEAD);
-}
-
-static void hangcheck_store_sample(struct intel_engine_cs *engine,
-				   const struct hangcheck *hc)
-{
-	engine->hangcheck.acthd = hc->acthd;
-	engine->hangcheck.last_ring = hc->ring;
-	engine->hangcheck.last_head = hc->head;
-}
-
-static enum intel_engine_hangcheck_action
-hangcheck_get_action(struct intel_engine_cs *engine,
-		     const struct hangcheck *hc)
-{
-	if (intel_engine_is_idle(engine))
-		return ENGINE_IDLE;
-
-	if (engine->hangcheck.last_ring != hc->ring)
-		return ENGINE_ACTIVE_SEQNO;
-
-	if (engine->hangcheck.last_head != hc->head)
-		return ENGINE_ACTIVE_SEQNO;
-
-	return engine_stuck(engine, hc->acthd);
-}
-
-static void hangcheck_accumulate_sample(struct intel_engine_cs *engine,
-					struct hangcheck *hc)
-{
-	unsigned long timeout = I915_ENGINE_DEAD_TIMEOUT;
-
-	hc->action = hangcheck_get_action(engine, hc);
-
-	/* We always increment the progress
-	 * if the engine is busy and still processing
-	 * the same request, so that no single request
-	 * can run indefinitely (such as a chain of
-	 * batches). The only time we do not increment
-	 * the hangcheck score on this ring, if this
-	 * engine is in a legitimate wait for another
-	 * engine. In that case the waiting engine is a
-	 * victim and we want to be sure we catch the
-	 * right culprit. Then every time we do kick
-	 * the ring, make it as a progress as the seqno
-	 * advancement might ensure and if not, it
-	 * will catch the hanging engine.
-	 */
-
-	switch (hc->action) {
-	case ENGINE_IDLE:
-	case ENGINE_ACTIVE_SEQNO:
-		/* Clear head and subunit states on seqno movement */
-		hc->acthd = 0;
-
-		memset(&engine->hangcheck.instdone, 0,
-		       sizeof(engine->hangcheck.instdone));
-
-		/* Intentional fall through */
-	case ENGINE_WAIT_KICK:
-	case ENGINE_WAIT:
-		engine->hangcheck.action_timestamp = jiffies;
-		break;
-
-	case ENGINE_ACTIVE_HEAD:
-	case ENGINE_ACTIVE_SUBUNITS:
-		/*
-		 * Seqno stuck with still active engine gets leeway,
-		 * in hopes that it is just a long shader.
-		 */
-		timeout = I915_SEQNO_DEAD_TIMEOUT;
-		break;
-
-	case ENGINE_DEAD:
-		break;
-
-	default:
-		MISSING_CASE(hc->action);
-	}
-
-	hc->stalled = time_after(jiffies,
-				 engine->hangcheck.action_timestamp + timeout);
-	hc->wedged = time_after(jiffies,
-				 engine->hangcheck.action_timestamp +
-				 I915_ENGINE_WEDGED_TIMEOUT);
-}
-
-static void hangcheck_declare_hang(struct intel_gt *gt,
-				   intel_engine_mask_t hung,
-				   intel_engine_mask_t stuck)
-{
-	struct intel_engine_cs *engine;
-	intel_engine_mask_t tmp;
-	char msg[80];
-	int len;
-
-	/* If some rings hung but others were still busy, only
-	 * blame the hanging rings in the synopsis.
-	 */
-	if (stuck != hung)
-		hung &= ~stuck;
-	len = scnprintf(msg, sizeof(msg),
-			"%s on ", stuck == hung ? "no progress" : "hang");
-	for_each_engine_masked(engine, gt->i915, hung, tmp)
-		len += scnprintf(msg + len, sizeof(msg) - len,
-				 "%s, ", engine->name);
-	msg[len-2] = '\0';
-
-	return intel_gt_handle_error(gt, hung, I915_ERROR_CAPTURE, "%s", msg);
-}
-
-/*
- * This is called when the chip hasn't reported back with completed
- * batchbuffers in a long time. We keep track per ring seqno progress and
- * if there are no progress, hangcheck score for that ring is increased.
- * Further, acthd is inspected to see if the ring is stuck. On stuck case
- * we kick the ring. If we see no progress on three subsequent calls
- * we assume chip is wedged and try to fix it by resetting the chip.
- */
-static void hangcheck_elapsed(struct work_struct *work)
-{
-	struct intel_gt *gt =
-		container_of(work, typeof(*gt), hangcheck.work.work);
-	intel_engine_mask_t hung = 0, stuck = 0, wedged = 0;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	intel_wakeref_t wakeref;
-
-	if (!i915_modparams.enable_hangcheck)
-		return;
-
-	if (!READ_ONCE(gt->awake))
-		return;
-
-	if (intel_gt_is_wedged(gt))
-		return;
-
-	wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
-	if (!wakeref)
-		return;
-
-	/* As enabling the GPU requires fairly extensive mmio access,
-	 * periodically arm the mmio checker to see if we are triggering
-	 * any invalid access.
-	 */
-	intel_uncore_arm_unclaimed_mmio_detection(gt->uncore);
-
-	for_each_engine(engine, gt->i915, id) {
-		struct hangcheck hc;
-
-		intel_engine_breadcrumbs_irq(engine);
-
-		hangcheck_load_sample(engine, &hc);
-		hangcheck_accumulate_sample(engine, &hc);
-		hangcheck_store_sample(engine, &hc);
-
-		if (hc.stalled) {
-			hung |= engine->mask;
-			if (hc.action != ENGINE_DEAD)
-				stuck |= engine->mask;
-		}
-
-		if (hc.wedged)
-			wedged |= engine->mask;
-	}
-
-	if (GEM_SHOW_DEBUG() && (hung | stuck)) {
-		struct drm_printer p = drm_debug_printer("hangcheck");
-
-		for_each_engine(engine, gt->i915, id) {
-			if (intel_engine_is_idle(engine))
-				continue;
-
-			intel_engine_dump(engine, &p, "%s\n", engine->name);
-		}
-	}
-
-	if (wedged) {
-		dev_err(gt->i915->drm.dev,
-			"GPU recovery timed out,"
-			" cancelling all in-flight rendering.\n");
-		GEM_TRACE_DUMP();
-		intel_gt_set_wedged(gt);
-	}
-
-	if (hung)
-		hangcheck_declare_hang(gt, hung, stuck);
-
-	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
-
-	/* Reset timer in case GPU hangs without another request being added */
-	intel_gt_queue_hangcheck(gt);
-}
-
-void intel_gt_queue_hangcheck(struct intel_gt *gt)
-{
-	unsigned long delay;
-
-	if (unlikely(!i915_modparams.enable_hangcheck))
-		return;
-
-	/*
-	 * Don't continually defer the hangcheck so that it is always run at
-	 * least once after work has been scheduled on any ring. Otherwise,
-	 * we will ignore a hung ring if a second ring is kept busy.
-	 */
-
-	delay = round_jiffies_up_relative(DRM_I915_HANGCHECK_JIFFIES);
-	queue_delayed_work(system_long_wq, &gt->hangcheck.work, delay);
-}
-
-void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
-{
-	memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
-	engine->hangcheck.action_timestamp = jiffies;
-}
-
-void intel_gt_init_hangcheck(struct intel_gt *gt)
-{
-	INIT_DELAYED_WORK(&gt->hangcheck.work, hangcheck_elapsed);
-}
-
-#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-#include "selftest_hangcheck.c"
-#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 34791fc79dea..9ed2cf91a46d 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1018,8 +1018,6 @@  void intel_gt_reset(struct intel_gt *gt,
 	if (ret)
 		goto taint;
 
-	intel_gt_queue_hangcheck(gt);
-
 finish:
 	reset_finish(gt, awake);
 unlock:
@@ -1347,4 +1345,5 @@  void __intel_fini_wedge(struct intel_wedge_me *w)
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftest_reset.c"
+#include "selftest_hangcheck.c"
 #endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 569a4105d49e..570546eda5e8 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -1686,7 +1686,6 @@  int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
 	};
 	struct intel_gt *gt = &i915->gt;
 	intel_wakeref_t wakeref;
-	bool saved_hangcheck;
 	int err;
 
 	if (!intel_has_gpu_reset(gt))
@@ -1696,12 +1695,9 @@  int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
 		return -EIO; /* we're long past hope of a successful reset */
 
 	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-	saved_hangcheck = fetch_and_zero(&i915_modparams.enable_hangcheck);
-	drain_delayed_work(&gt->hangcheck.work); /* flush param */
 
 	err = intel_gt_live_subtests(tests, gt);
 
-	i915_modparams.enable_hangcheck = saved_hangcheck;
 	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
 
 	return err;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 277f31297f29..55852e045c3a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1011,92 +1011,6 @@  static int i915_frequency_info(struct seq_file *m, void *unused)
 	return ret;
 }
 
-static void i915_instdone_info(struct drm_i915_private *dev_priv,
-			       struct seq_file *m,
-			       struct intel_instdone *instdone)
-{
-	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
-	int slice;
-	int subslice;
-
-	seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
-		   instdone->instdone);
-
-	if (INTEL_GEN(dev_priv) <= 3)
-		return;
-
-	seq_printf(m, "\t\tSC_INSTDONE: 0x%08x\n",
-		   instdone->slice_common);
-
-	if (INTEL_GEN(dev_priv) <= 6)
-		return;
-
-	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
-		seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
-			   slice, subslice, instdone->sampler[slice][subslice]);
-
-	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
-		seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
-			   slice, subslice, instdone->row[slice][subslice]);
-}
-
-static int i915_hangcheck_info(struct seq_file *m, void *unused)
-{
-	struct drm_i915_private *i915 = node_to_i915(m->private);
-	struct intel_gt *gt = &i915->gt;
-	struct intel_engine_cs *engine;
-	intel_wakeref_t wakeref;
-	enum intel_engine_id id;
-
-	seq_printf(m, "Reset flags: %lx\n", gt->reset.flags);
-	if (test_bit(I915_WEDGED, &gt->reset.flags))
-		seq_puts(m, "\tWedged\n");
-	if (test_bit(I915_RESET_BACKOFF, &gt->reset.flags))
-		seq_puts(m, "\tDevice (global) reset in progress\n");
-
-	if (!i915_modparams.enable_hangcheck) {
-		seq_puts(m, "Hangcheck disabled\n");
-		return 0;
-	}
-
-	if (timer_pending(&gt->hangcheck.work.timer))
-		seq_printf(m, "Hangcheck active, timer fires in %dms\n",
-			   jiffies_to_msecs(gt->hangcheck.work.timer.expires -
-					    jiffies));
-	else if (delayed_work_pending(&gt->hangcheck.work))
-		seq_puts(m, "Hangcheck active, work pending\n");
-	else
-		seq_puts(m, "Hangcheck inactive\n");
-
-	seq_printf(m, "GT active? %s\n", yesno(gt->awake));
-
-	with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
-		for_each_engine(engine, i915, id) {
-			struct intel_instdone instdone;
-
-			seq_printf(m, "%s: %d ms ago\n",
-				   engine->name,
-				   jiffies_to_msecs(jiffies -
-						    engine->hangcheck.action_timestamp));
-
-			seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
-				   (long long)engine->hangcheck.acthd,
-				   intel_engine_get_active_head(engine));
-
-			intel_engine_get_instdone(engine, &instdone);
-
-			seq_puts(m, "\tinstdone read =\n");
-			i915_instdone_info(i915, m, &instdone);
-
-			seq_puts(m, "\tinstdone accu =\n");
-			i915_instdone_info(i915, m,
-					   &engine->hangcheck.instdone);
-		}
-	}
-
-	return 0;
-}
-
 static int ironlake_drpc_info(struct seq_file *m)
 {
 	struct drm_i915_private *i915 = node_to_i915(m->private);
@@ -4303,7 +4217,6 @@  static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_guc_stage_pool", i915_guc_stage_pool, 0},
 	{"i915_huc_load_status", i915_huc_load_status_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
-	{"i915_hangcheck_info", i915_hangcheck_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
 	{"i915_frontbuffer_tracking", i915_frontbuffer_tracking, 0},
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f02a34722217..1dae43ed4c48 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1546,10 +1546,7 @@  void i915_driver_remove(struct drm_i915_private *i915)
 
 	i915_driver_modeset_remove(i915);
 
-	/* Free error state after interrupts are fully disabled. */
-	cancel_delayed_work_sync(&i915->gt.hangcheck.work);
 	i915_reset_error_state(i915);
-
 	i915_gem_driver_remove(i915);
 
 	intel_power_domains_driver_remove(i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d284b04c492b..58340c99af02 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1845,7 +1845,6 @@  void i915_driver_remove(struct drm_i915_private *i915);
 int i915_resume_switcheroo(struct drm_i915_private *i915);
 int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
 
-void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
 
 static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 5cf4eed5add8..47239df653f2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -534,10 +534,6 @@  static void error_print_engine(struct drm_i915_error_state_buf *m,
 	}
 	err_printf(m, "  ring->head: 0x%08x\n", ee->cpu_ring_head);
 	err_printf(m, "  ring->tail: 0x%08x\n", ee->cpu_ring_tail);
-	err_printf(m, "  hangcheck timestamp: %dms (%lu%s)\n",
-		   jiffies_to_msecs(ee->hangcheck_timestamp - epoch),
-		   ee->hangcheck_timestamp,
-		   ee->hangcheck_timestamp == epoch ? "; epoch" : "");
 	err_printf(m, "  engine reset count: %u\n", ee->reset_count);
 
 	for (n = 0; n < ee->num_ports; n++) {
@@ -679,11 +675,8 @@  static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
 	ts = ktime_to_timespec64(error->uptime);
 	err_printf(m, "Uptime: %lld s %ld us\n",
 		   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
-	err_printf(m, "Epoch: %lu jiffies (%u HZ)\n", error->epoch, HZ);
-	err_printf(m, "Capture: %lu jiffies; %d ms ago, %d ms after epoch\n",
-		   error->capture,
-		   jiffies_to_msecs(jiffies - error->capture),
-		   jiffies_to_msecs(error->capture - error->epoch));
+	err_printf(m, "Capture: %lu jiffies; %d ms ago\n",
+		   error->capture, jiffies_to_msecs(jiffies - error->capture));
 
 	for (ee = error->engine; ee; ee = ee->next)
 		err_printf(m, "Active process (on ring %s): %s [%d]\n",
@@ -742,7 +735,7 @@  static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
 		err_printf(m, "GTT_CACHE_EN: 0x%08x\n", error->gtt_cache);
 
 	for (ee = error->engine; ee; ee = ee->next)
-		error_print_engine(m, ee, error->epoch);
+		error_print_engine(m, ee, error->capture);
 
 	for (ee = error->engine; ee; ee = ee->next) {
 		const struct drm_i915_error_object *obj;
@@ -770,7 +763,7 @@  static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
 			for (j = 0; j < ee->num_requests; j++)
 				error_print_request(m, " ",
 						    &ee->requests[j],
-						    error->epoch);
+						    error->capture);
 		}
 
 		print_error_obj(m, ee->engine, "ringbuffer", ee->ringbuffer);
@@ -1144,8 +1137,6 @@  static void error_record_engine_registers(struct i915_gpu_state *error,
 	}
 
 	ee->idle = intel_engine_is_idle(engine);
-	if (!ee->idle)
-		ee->hangcheck_timestamp = engine->hangcheck.action_timestamp;
 	ee->reset_count = i915_reset_engine_count(&dev_priv->gpu_error,
 						  engine);
 
@@ -1657,20 +1648,6 @@  static void capture_params(struct i915_gpu_state *error)
 	i915_params_copy(&error->params, &i915_modparams);
 }
 
-static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
-{
-	const struct drm_i915_error_engine *ee;
-	unsigned long epoch = error->capture;
-
-	for (ee = error->engine; ee; ee = ee->next) {
-		if (ee->hangcheck_timestamp &&
-		    time_before(ee->hangcheck_timestamp, epoch))
-			epoch = ee->hangcheck_timestamp;
-	}
-
-	return epoch;
-}
-
 static void capture_finish(struct i915_gpu_state *error)
 {
 	struct i915_ggtt *ggtt = &error->i915->ggtt;
@@ -1722,8 +1699,6 @@  i915_capture_gpu_state(struct drm_i915_private *i915)
 	error->overlay = intel_overlay_capture_error_state(i915);
 	error->display = intel_display_capture_error_state(i915);
 
-	error->epoch = capture_find_epoch(error);
-
 	capture_finish(error);
 	compress_fini(&compress);
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index 7f1cd0b1fef7..4dc36d6ee3a2 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -34,7 +34,6 @@  struct i915_gpu_state {
 	ktime_t boottime;
 	ktime_t uptime;
 	unsigned long capture;
-	unsigned long epoch;
 
 	struct drm_i915_private *i915;
 
@@ -86,7 +85,6 @@  struct i915_gpu_state {
 
 		/* Software tracked state */
 		bool idle;
-		unsigned long hangcheck_timestamp;
 		int num_requests;
 		u32 reset_count;
 
diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h b/drivers/gpu/drm/i915/i915_priolist_types.h
index ae8bb3cb627e..732aad148881 100644
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -16,6 +16,12 @@  enum {
 	I915_PRIORITY_MIN = I915_CONTEXT_MIN_USER_PRIORITY - 1,
 	I915_PRIORITY_NORMAL = I915_CONTEXT_DEFAULT_PRIORITY,
 	I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+	/* A preemptive pulse used to monitor the health of each engine */
+	I915_PRIORITY_HEARTBEAT,
+
+	/* Interactive workload, scheduled for immediate pageflipping */
+	I915_PRIORITY_DISPLAY,
 };
 
 #define I915_USER_PRIORITY_SHIFT 2