[Intel-gfx] [PATCH v3] drm/i915: Disable PSMI sleep messages on all rings around context switches
Jani Nikula
jani.nikula at linux.intel.com
Tue Dec 16 05:12:14 PST 2014
On Tue, 16 Dec 2014, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> 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 <simon at farnz.org.uk>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Tested-by: Peter Frühberger <fritsch at xbmc.org>
> Reviewed-by: Daniel Vetter <daniel at ffwll.ch>
> Cc: stable at vger.kernel.org
Pushed all three to drm-intel-next-fixes, thanks for the patch, review
and testing.
BR,
Jani.
> ---
> 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 b9deade53f2e..9b23fb1f5bf6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -484,7 +484,12 @@ mi_set_context(struct intel_engine_cs *ring,
> u32 hw_flags)
> {
> u32 flags = hw_flags | MI_MM_SPACE_GTT;
> - 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
> @@ -501,15 +506,31 @@ mi_set_context(struct intel_engine_cs *ring,
> if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
> flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
>
> - 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);
> @@ -521,10 +542,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 faa5f66e82dd..40ca873a05ad 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1136,6 +1136,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)
> @@ -1466,6 +1467,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)
>
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list