Commit 3dd66a94 authored by Chris Wilson's avatar Chris Wilson

drm/i915/gt: Always send a pulse down the engine after disabling heartbeat

Currently, we check we can send a pulse prior to disabling the
heartbeat to verify that we can change the heartbeat, but since we may
re-evaluate execution upon changing the heartbeat interval we need another
pulse afterwards to refresh execution.

v2: Tvrtko asked if we could reduce the double pulse to a single, which
opened up a discussion of how we should handle the pulse-error after
attempting to change the property, and the desire to serialise
adjustment of the property with its validating pulse, and unwind upon
failure.

Fixes: 9a40bddd ("drm/i915/gt: Expose heartbeat interval via sysfs")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.7+
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200928221510.26044-2-chris@chris-wilson.co.uk
parent 7a991cd3
...@@ -177,36 +177,82 @@ void intel_engine_init_heartbeat(struct intel_engine_cs *engine) ...@@ -177,36 +177,82 @@ void intel_engine_init_heartbeat(struct intel_engine_cs *engine)
INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat); INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat);
} }
int intel_engine_set_heartbeat(struct intel_engine_cs *engine, static int __intel_engine_pulse(struct intel_engine_cs *engine)
unsigned long delay)
{ {
int err; struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
struct intel_context *ce = engine->kernel_context;
struct i915_request *rq;
/* Send one last pulse before to cleanup persistent hogs */ lockdep_assert_held(&ce->timeline->mutex);
if (!delay && IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT)) { GEM_BUG_ON(!intel_engine_has_preemption(engine));
err = intel_engine_pulse(engine); GEM_BUG_ON(!intel_engine_pm_is_awake(engine));
if (err)
return err; intel_context_enter(ce);
} rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
intel_context_exit(ce);
if (IS_ERR(rq))
return PTR_ERR(rq);
__set_bit(I915_FENCE_FLAG_SENTINEL, &rq->fence.flags);
idle_pulse(engine, rq);
__i915_request_commit(rq);
__i915_request_queue(rq, &attr);
GEM_BUG_ON(rq->sched.attr.priority < I915_PRIORITY_BARRIER);
WRITE_ONCE(engine->props.heartbeat_interval_ms, delay); return 0;
}
if (intel_engine_pm_get_if_awake(engine)) { static unsigned long set_heartbeat(struct intel_engine_cs *engine,
unsigned long delay)
{
unsigned long old;
old = xchg(&engine->props.heartbeat_interval_ms, delay);
if (delay) if (delay)
intel_engine_unpark_heartbeat(engine); intel_engine_unpark_heartbeat(engine);
else else
intel_engine_park_heartbeat(engine); intel_engine_park_heartbeat(engine);
intel_engine_pm_put(engine);
return old;
}
int intel_engine_set_heartbeat(struct intel_engine_cs *engine,
unsigned long delay)
{
struct intel_context *ce = engine->kernel_context;
int err = 0;
if (!delay && !intel_engine_has_preempt_reset(engine))
return -ENODEV;
intel_engine_pm_get(engine);
err = mutex_lock_interruptible(&ce->timeline->mutex);
if (err)
goto out_rpm;
if (delay != engine->props.heartbeat_interval_ms) {
unsigned long saved = set_heartbeat(engine, delay);
/* recheck current execution */
if (intel_engine_has_preemption(engine)) {
err = __intel_engine_pulse(engine);
if (err)
set_heartbeat(engine, saved);
}
} }
return 0; mutex_unlock(&ce->timeline->mutex);
out_rpm:
intel_engine_pm_put(engine);
return err;
} }
int intel_engine_pulse(struct intel_engine_cs *engine) int intel_engine_pulse(struct intel_engine_cs *engine)
{ {
struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER };
struct intel_context *ce = engine->kernel_context; struct intel_context *ce = engine->kernel_context;
struct i915_request *rq;
int err; int err;
if (!intel_engine_has_preemption(engine)) if (!intel_engine_has_preemption(engine))
...@@ -215,30 +261,12 @@ int intel_engine_pulse(struct intel_engine_cs *engine) ...@@ -215,30 +261,12 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
if (!intel_engine_pm_get_if_awake(engine)) if (!intel_engine_pm_get_if_awake(engine))
return 0; return 0;
if (mutex_lock_interruptible(&ce->timeline->mutex)) {
err = -EINTR; err = -EINTR;
goto out_rpm; if (!mutex_lock_interruptible(&ce->timeline->mutex)) {
} err = __intel_engine_pulse(engine);
mutex_unlock(&ce->timeline->mutex);
intel_context_enter(ce);
rq = __i915_request_create(ce, GFP_NOWAIT | __GFP_NOWARN);
intel_context_exit(ce);
if (IS_ERR(rq)) {
err = PTR_ERR(rq);
goto out_unlock;
} }
__set_bit(I915_FENCE_FLAG_SENTINEL, &rq->fence.flags);
idle_pulse(engine, rq);
__i915_request_commit(rq);
__i915_request_queue(rq, &attr);
GEM_BUG_ON(rq->sched.attr.priority < I915_PRIORITY_BARRIER);
err = 0;
out_unlock:
mutex_unlock(&ce->timeline->mutex);
out_rpm:
intel_engine_pm_put(engine); intel_engine_pm_put(engine);
return err; return err;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment