On Wed, Dec 30, 2015 at 10:31:17PM +0000, Ben Hutchings wrote: > This fix from 3.19 (commit 2c550183476d) has so far only been applied > to 3.18.y, but the bug appears to have been introduced in 3.14 or > earlier. There is a positive result of testing on 3.16-ckt here: > https://bugs.debian.org/777231 > > For 3.16-ckt I only made context changes (see attached patch). I gave > up trying to backport to 3.14. > > Ben. >
Thanks Ben, I'll queue for the next 3.16 release. Cheers, -- Luís > -- > Ben Hutchings > This sentence contradicts itself - no actually it doesn't. > From d2317cde33256e6e0f302af8ed8a888e35921255 Mon Sep 17 00:00:00 2001 > From: Chris Wilson <[email protected]> > Date: Tue, 16 Dec 2014 10:02:27 +0000 > Subject: [PATCH] drm/i915: Disable PSMI sleep messages on all rings around > context switches > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > commit 2c550183476dfa25641309ae9a28d30feed14379 upstream. > > There exists a current workaround to prevent a hang on context switch > should the ring go to sleep in the middle of the restore, > WaProgramMiArbOnOffAroundMiSetContext (applicable to all gen7+). In > spite of disabling arbitration (which prevents the ring from powering > down during the critical section) we were still hitting hangs that had > the hallmarks of the known erratum. That is we are still seeing hangs > "on the last instruction in the context restore". By comparing -nightly > (broken) with requests (working), we were able to deduce that it was the > semaphore LRI cross-talk that reproduced the original failure. The key > was that requests implemented deferred semaphore signalling, and > disabling that, i.e. emitting the semaphore signal to every other ring > after every batch restored the frequent hang. Explicitly disabling PSMI > sleep on the RCS ring was insufficient, all the rings had to be awake to > prevent the hangs. Fortunately, we can reduce the wakelock to the > MI_SET_CONTEXT operation itself, and so should be able to limit the extra > power implications. > > Since the MI_ARB_ON_OFF workaround is listed for all gen7 and above > products, we should apply this extra hammer for all of the same > platforms despite so far that we have only been able to reproduce the > hang on certain ivb and hsw models. The last question is whether we want > to always use the extra hammer or only when we know semaphores are in > operation. At the moment, we only use LRI on non-RCS rings for > semaphores, but that may change in the future with the possibility of > reintroducing this bug under subtle conditions. > > v2: Make it explicit that the PSMI LRI are an extension to the original > workaround for the other rings. > v3: Bikeshedding variable names and whitespacing > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80660 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83677 > Cc: Simon Farnsworth <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > Tested-by: Peter Frühberger <[email protected]> > Reviewed-by: Daniel Vetter <[email protected]> > Cc: [email protected] > Signed-off-by: Jani Nikula <[email protected]> > [bwh: Backported to 3.16: adjust context] > Signed-off-by: Ben Hutchings <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 48 > +++++++++++++++++++++++++++------ > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index a5ddf3b..14f9264 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -545,7 +545,12 @@ mi_set_context(struct intel_engine_cs *ring, > struct intel_context *new_context, > u32 hw_flags) > { > - int ret; > + const int num_rings = > + /* Use an extended w/a on ivb+ if signalling from other rings */ > + i915_semaphore_is_enabled(ring->dev) ? > + hweight32(INTEL_INFO(ring->dev)->ring_mask) - 1 : > + 0; > + int len, i, ret; > > /* w/a: If Flush TLB Invalidation Mode is enabled, driver must do a TLB > * invalidation prior to MI_SET_CONTEXT. On GEN6 we don't set the value > @@ -558,15 +563,31 @@ mi_set_context(struct intel_engine_cs *ring, > return ret; > } > > - ret = intel_ring_begin(ring, 6); > + > + len = 4; > + if (INTEL_INFO(ring->dev)->gen >= 7) > + len += 2 + (num_rings ? 4*num_rings + 2 : 0); > + > + ret = intel_ring_begin(ring, len); > if (ret) > return ret; > > /* WaProgramMiArbOnOffAroundMiSetContext:ivb,vlv,hsw,bdw,chv */ > - if (INTEL_INFO(ring->dev)->gen >= 7) > + if (INTEL_INFO(ring->dev)->gen >= 7) { > intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_DISABLE); > - else > - intel_ring_emit(ring, MI_NOOP); > + if (num_rings) { > + struct intel_engine_cs *signaller; > + > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings)); > + for_each_ring(signaller, to_i915(ring->dev), i) { > + if (signaller == ring) > + continue; > + > + intel_ring_emit(ring, > RING_PSMI_CTL(signaller->mmio_base)); > + intel_ring_emit(ring, > _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE)); > + } > + } > + } > > intel_ring_emit(ring, MI_NOOP); > intel_ring_emit(ring, MI_SET_CONTEXT); > @@ -581,10 +602,21 @@ mi_set_context(struct intel_engine_cs *ring, > */ > intel_ring_emit(ring, MI_NOOP); > > - if (INTEL_INFO(ring->dev)->gen >= 7) > + if (INTEL_INFO(ring->dev)->gen >= 7) { > + if (num_rings) { > + struct intel_engine_cs *signaller; > + > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings)); > + for_each_ring(signaller, to_i915(ring->dev), i) { > + if (signaller == ring) > + continue; > + > + intel_ring_emit(ring, > RING_PSMI_CTL(signaller->mmio_base)); > + intel_ring_emit(ring, > _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE)); > + } > + } > intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_ENABLE); > - else > - intel_ring_emit(ring, MI_NOOP); > + } > > intel_ring_advance(ring); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index fa0ec5a..8196408 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -978,6 +978,7 @@ enum punit_power_well { > #define GEN6_VERSYNC (RING_SYNC_1(VEBOX_RING_BASE)) > #define GEN6_VEVSYNC (RING_SYNC_2(VEBOX_RING_BASE)) > #define GEN6_NOSYNC 0 > +#define RING_PSMI_CTL(base) ((base)+0x50) > #define RING_MAX_IDLE(base) ((base)+0x54) > #define RING_HWS_PGA(base) ((base)+0x80) > #define RING_HWS_PGA_GEN6(base) ((base)+0x2080) > @@ -1301,6 +1302,7 @@ enum punit_power_well { > #define GEN6_BLITTER_FBC_NOTIFY (1<<3) > > #define GEN6_RC_SLEEP_PSMI_CONTROL 0x2050 > +#define GEN6_PSMI_SLEEP_MSG_DISABLE (1 << 0) > #define GEN8_RC_SEMA_IDLE_MSG_DISABLE (1 << 12) > #define GEN8_FF_DOP_CLOCK_GATE_DISABLE (1<<10) > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
