| Message ID | 20191021080226.537-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/16] drm/i915: Don't set queue_priority_hint if we don't kick the submission | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > 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=111923 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> A tad sad that the price/payout didn't provide. Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/gt/intel_lrc.c | 16 ---------------- > drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 ----- > drivers/gpu/drm/i915/i915_perf.c | 1 - > 3 files changed, 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index d0088d020220..f9f3e985bb79 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state, > const struct intel_ring *ring, > 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) > { > if (i915_request_completed(rq)) > @@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) > ce = rq->hw_context; > 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); > if (!rq) { > /* 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) > __execlists_reset_reg_state(ce, engine); > __execlists_update_reg_state(ce, engine); > ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */ > - __context_pin_release(ce); > > unwind: > /* Push back any incomplete requests for replay after the reset. */ > @@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine, > bool scrub) > { > GEM_BUG_ON(!intel_context_is_pinned(ce)); > - __context_pin_acquire(ce); > > /* > * 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, > intel_ring_update_space(ce->ring); > > __execlists_update_reg_state(ce, engine); > - __context_pin_release(ce); > } > > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 5dc679781a08..7516d1c90925 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio) > } > GEM_BUG_ON(!ce[1]->ring->size); > 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); > - __context_pin_release(ce[1]); > - local_irq_enable(); > > rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK); > if (IS_ERR(rq[0])) { > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index d2ac51fe4f04..3130b0c7ed83 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce, > struct i915_perf_stream *stream; > > /* perf.exclusive_stream serialised by gen8_configure_all_contexts() */ > - lockdep_assert_held(&ce->pin_mutex); > > if (engine->class != RENDER_CLASS) > return; > -- > 2.24.0.rc0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index d0088d020220..f9f3e985bb79 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -235,16 +235,6 @@ static void execlists_init_reg_state(u32 *reg_state, const struct intel_ring *ring, 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) { if (i915_request_completed(rq)) @@ -2792,9 +2782,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) ce = rq->hw_context; 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); if (!rq) { /* 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) __execlists_reset_reg_state(ce, engine); __execlists_update_reg_state(ce, engine); ce->lrc_desc |= CTX_DESC_FORCE_RESTORE; /* paranoid: GPU was reset! */ - __context_pin_release(ce); unwind: /* Push back any incomplete requests for replay after the reset. */ @@ -4502,7 +4488,6 @@ void intel_lr_context_reset(struct intel_engine_cs *engine, bool scrub) { GEM_BUG_ON(!intel_context_is_pinned(ce)); - __context_pin_acquire(ce); /* * 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, intel_ring_update_space(ce->ring); __execlists_update_reg_state(ce, engine); - __context_pin_release(ce); } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index 5dc679781a08..7516d1c90925 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -168,12 +168,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio) } GEM_BUG_ON(!ce[1]->ring->size); 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); - __context_pin_release(ce[1]); - local_irq_enable(); rq[0] = igt_spinner_create_request(&spin, ce[0], MI_ARB_CHECK); if (IS_ERR(rq[0])) { diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index d2ac51fe4f04..3130b0c7ed83 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -2615,7 +2615,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce, struct i915_perf_stream *stream; /* perf.exclusive_stream serialised by gen8_configure_all_contexts() */ - lockdep_assert_held(&ce->pin_mutex); if (engine->class != RENDER_CLASS) return;
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=111923 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/intel_lrc.c | 16 ---------------- drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 ----- drivers/gpu/drm/i915/i915_perf.c | 1 - 3 files changed, 22 deletions(-)