On Tue, Apr 12, 2016 at 09:03:05PM +0100, Chris Wilson wrote: > Two concurrent writes into the same register cacheline has the chance of > killing the machine on Ivybridge and other gen7. This includes LRI > emitted from the command parser. The MI_SET_CONTEXT itself serves as > serialising barrier and prevents the pair of register writes in the first > packet from triggering the fault. However, if a second switch-context > immediately occurs then we may have two adjacent blocks of LRI to the > same registers which may then trigger the hang. To counteract this we > need to insert a delay after the second register write using SRM. > > This is easiest to reproduce with something like > igt/gem_ctx_switch/interruptible that triggers back-to-back context > switches (with no operations in between them in the command stream, > which requires the execbuf operation to be interrupted after the > MI_SET_CONTEXT) but can be observed sporadically elsewhere when running > interruptible igt. No reports from the wild though, so it must be of low > enough frequency that no one has correlated the random machine freezes > with i915.ko > > The issue was introduced with > commit 2c550183476dfa25641309ae9a28d30feed14379 [v3.19] > Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Date: Tue Dec 16 10:02:27 2014 +0000 > > drm/i915: Disable PSMI sleep messages on all rings around context switches > > Testcase: igt/gem_ctx_switch/render-interruptible #ivb > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index fe580cb9501a..e5ad7b21e356 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -539,7 +539,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) > > len = 4; > if (INTEL_INFO(engine->dev)->gen >= 7) > - len += 2 + (num_rings ? 4*num_rings + 2 : 0); > + len += 2 + (num_rings ? 4*num_rings + 6 : 0); > > ret = intel_ring_begin(req, len); > if (ret) > @@ -579,6 +579,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) > if (INTEL_INFO(engine->dev)->gen >= 7) { > if (num_rings) { > struct intel_engine_cs *signaller; > + i915_reg_t last_reg = {}; /* keep gcc quiet */ > > intel_ring_emit(engine, > MI_LOAD_REGISTER_IMM(num_rings)); > @@ -586,11 +587,19 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags) > if (signaller == engine) > continue; > > - intel_ring_emit_reg(engine, > - RING_PSMI_CTL(signaller->mmio_base)); > + last_reg = RING_PSMI_CTL(signaller->mmio_base); > + intel_ring_emit_reg(engine, last_reg); > intel_ring_emit(engine, > _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE)); > } > + > + /* Insert a delay before the next switch! */ > + intel_ring_emit(engine, > + MI_STORE_REGISTER_MEM | > + MI_SRM_LRM_GLOBAL_GTT); > + intel_ring_emit_reg(engine, last_reg); > + intel_ring_emit(engine, engine->scratch.gtt_offset); > + intel_ring_emit(engine, MI_NOOP); > } > intel_ring_emit(engine, MI_ARB_ON_OFF | MI_ARB_ENABLE); > } > -- > 2.8.0.rc3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx