Commit 0587152b authored by Chris Wilson's avatar Chris Wilson

drm/i915: Drop assertion that ce->pin_mutex guards state updates

The actual conditions are that we know the GPU is not accessing the
context, and we hold a pin on the context image to allow CPU access. We
used a fake lock on ce->pin_mutex so that we could try and use lockdep
to assert that access is serialised, but the various different
hardirq/softirq contexts where we need to *fake* holding the pin_mutex
are causing more trouble.

Still it would be nice if we did have a way to reassure ourselves that
the direct update to the context image is serialised with GPU execution.
In the meantime, stop lockdep complaining about false irq inversions.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111923Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Acked-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022122845.25038-1-chris@chris-wilson.co.uk
parent e948761f
...@@ -176,9 +176,7 @@ int intel_gt_resume(struct intel_gt *gt) ...@@ -176,9 +176,7 @@ int intel_gt_resume(struct intel_gt *gt)
ce = engine->kernel_context; ce = engine->kernel_context;
if (ce) { if (ce) {
GEM_BUG_ON(!intel_context_is_pinned(ce)); GEM_BUG_ON(!intel_context_is_pinned(ce));
mutex_acquire(&ce->pin_mutex.dep_map, 0, 0, _THIS_IP_);
ce->ops->reset(ce); ce->ops->reset(ce);
mutex_release(&ce->pin_mutex.dep_map, 0, _THIS_IP_);
} }
engine->serial++; /* kernel context lost */ engine->serial++; /* kernel context lost */
......
...@@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state, ...@@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state,
const struct intel_ring *ring, const struct intel_ring *ring,
bool close); bool close);
static void __context_pin_acquire(struct intel_context *ce)
{
mutex_acquire(&ce->pin_mutex.dep_map, 2, 0, _RET_IP_);
}
static void __context_pin_release(struct intel_context *ce)
{
mutex_release(&ce->pin_mutex.dep_map, 0, _RET_IP_);
}
static void mark_eio(struct i915_request *rq) static void mark_eio(struct i915_request *rq)
{ {
if (i915_request_completed(rq)) if (i915_request_completed(rq))
...@@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) ...@@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
ce = rq->hw_context; ce = rq->hw_context;
GEM_BUG_ON(!i915_vma_is_pinned(ce->state)); GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
/* Proclaim we have exclusive access to the context image! */
__context_pin_acquire(ce);
rq = active_request(rq); rq = active_request(rq);
if (!rq) { if (!rq) {
/* Idle context; tidy up the ring so we can restart afresh */ /* Idle context; tidy up the ring so we can restart afresh */
...@@ -2860,7 +2847,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) ...@@ -2860,7 +2847,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
__execlists_reset_reg_state(ce, engine); __execlists_reset_reg_state(ce, engine);
__execlists_update_reg_state(ce, engine); __execlists_update_reg_state(ce, engine);
ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */ ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */
__context_pin_release(ce);
unwind: unwind:
/* Push back any incomplete requests for replay after the reset. */ /* Push back any incomplete requests for replay after the reset. */
...@@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine, ...@@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
bool scrub) bool scrub)
{ {
GEM_BUG_ON(!intel_context_is_pinned(ce)); GEM_BUG_ON(!intel_context_is_pinned(ce));
__context_pin_acquire(ce);
/* /*
* We want a simple context + ring to execute the breadcrumb update. * We want a simple context + ring to execute the breadcrumb update.
...@@ -4528,7 +4513,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine, ...@@ -4528,7 +4513,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine,
intel_ring_update_space(ce->ring); intel_ring_update_space(ce->ring);
__execlists_update_reg_state(ce, engine); __execlists_update_reg_state(ce, engine);
__context_pin_release(ce);
} }
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
......
...@@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio) ...@@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio)
} }
GEM_BUG_ON(!ce[1]->ring->size); GEM_BUG_ON(!ce[1]->ring->size);
intel_ring_reset(ce[1]->ring, ce[1]->ring->size / 2); intel_ring_reset(ce[1]->ring, ce[1]->ring->size / 2);
local_irq_disable(); /* appease lockdep */
__context_pin_acquire(ce[1]);
__execlists_update_reg_state(ce[1], engine); __execlists_update_reg_state(ce[1], engine);
__context_pin_release(ce[1]);
local_irq_enable();
rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK); rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK);
if (IS_ERR(rq[0])) { if (IS_ERR(rq[0])) {
......
...@@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce, ...@@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
struct i915_perf_stream *stream; struct i915_perf_stream *stream;
/* perf.exclusive_stream serialised by gen8_configure_all_contexts() */ /* perf.exclusive_stream serialised by gen8_configure_all_contexts() */
lockdep_assert_held(&ce->pin_mutex);
if (engine->class != RENDER_CLASS) if (engine->class != RENDER_CLASS)
return; return;
......
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