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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html