[PATCH v2 1/3] drm/i915: Invalidate media caches on gen7

[PATCH v2 1/3] drm/i915: Invalidate media caches on gen7

[Chris Wilson]

In the gen7 pipe control there is an extra bit to flush the media
caches, so let's set it during cache invalidation flushes.

v2: Rename to MEDIA_STATE_CLEAR to be more inline with spec.

Cc: Simon Farnsworth <simon@farnz.org.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d5be7cd13f23..faa5f66e82dd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -401,6 +401,7 @@
 #define   PIPE_CONTROL_STORE_DATA_INDEX			(1<<21)
 #define   PIPE_CONTROL_CS_STALL				(1<<20)
 #define   PIPE_CONTROL_TLB_INVALIDATE			(1<<18)
+#define   PIPE_CONTROL_MEDIA_STATE_CLEAR		(1<<16)
 #define   PIPE_CONTROL_QW_WRITE				(1<<14)
 #define   PIPE_CONTROL_POST_SYNC_OP_MASK                (3<<14)
 #define   PIPE_CONTROL_DEPTH_STALL			(1<<13)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 72b5cfbabe0d..ea0d22852baa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -373,6 +373,7 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
 		flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
+		flags |= PIPE_CONTROL_MEDIA_STATE_CLEAR;
 		/*
 		 * TLB invalidate requires a post-sync write.
 		 */
-- 
2.1.3

[PATCH v2 2/3] drm/i915: Force the CS stall for invalidate flushes

[Chris Wilson]

In order to act as a full command barrier by itself, we need to tell the
pipecontrol to actually stall the command streamer while the flush runs.
We require the full command barrier before operations like
MI_SET_CONTEXT, which currently rely on a prior invalidate flush.

References: https://bugs.freedesktop.org/show_bug.cgi?id=83677
Cc: Simon Farnsworth <simon@farnz.org.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ea0d22852baa..12a36f0ca53d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -380,6 +380,8 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
 		flags |= PIPE_CONTROL_QW_WRITE;
 		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
 
+		flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD;
+
 		/* Workaround: we must issue a pipe_control with CS-stall bit
 		 * set before a pipe_control command that has the state cache
 		 * invalidate bit set. */
-- 
2.1.3

[PATCH v2 3/3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[Chris Wilson]

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.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80660
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83677
Cc: Simon Farnsworth <simon@farnz.org.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_gem_context.c | 41 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/i915_reg.h         |  2 ++
 2 files changed, 35 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..1e0b51e945c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -484,7 +484,9 @@ mi_set_context(struct intel_engine_cs *ring,
 	       u32 hw_flags)
 {
 	u32 flags = hw_flags | MI_MM_SPACE_GTT;
-	int ret;
+	struct intel_engine_cs *engine;
+	const int num_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 +503,29 @@ 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) {
+			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings));
+			for_each_ring(engine, to_i915(ring->dev), i) {
+				if (i == RCS)
+					continue;
+
+				intel_ring_emit(ring, RING_PSMI_CTL(engine->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 +537,19 @@ 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) {
+			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings));
+			for_each_ring(engine, to_i915(ring->dev), i) {
+				if (i == RCS)
+					continue;
+
+				intel_ring_emit(ring, RING_PSMI_CTL(engine->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

Re: [PATCH v2 3/3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[Chris Wilson]

On Tue, Dec 16, 2014 at 08:44:33AM +0000, Chris Wilson wrote:
>  	/* 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) {
> +			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings));
> +			for_each_ring(engine, to_i915(ring->dev), i) {
> +				if (i == RCS)
> +					continue;

I would have preferred to have written

	if (engine == ring)
		continue;

here instead. And s/engine/signaller/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2 3/3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[Daniel Vetter]

On Tue, Dec 16, 2014 at 08:44:33AM +0000, Chris Wilson 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.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80660
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83677
> Cc: Simon Farnsworth <simon@farnz.org.uk>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 41 ++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>  2 files changed, 35 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..1e0b51e945c9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -484,7 +484,9 @@ mi_set_context(struct intel_engine_cs *ring,
>  	       u32 hw_flags)
>  {
>  	u32 flags = hw_flags | MI_MM_SPACE_GTT;
> -	int ret;
> +	struct intel_engine_cs *engine;
> +	const int num_rings = i915_semaphore_is_enabled(ring->dev) ? hweight32(INTEL_INFO(ring->dev)->ring_mask) - 1: 0;

Maybe a linebreak after ? to make sure the -1 doesnt drop off the left
edge. I guess Jani could polish that while applying.

On the series (patch 2 lost my r-b it seems):

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	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 +503,29 @@ 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) {
> +			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings));
> +			for_each_ring(engine, to_i915(ring->dev), i) {
> +				if (i == RCS)
> +					continue;
> +
> +				intel_ring_emit(ring, RING_PSMI_CTL(engine->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 +537,19 @@ 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) {
> +			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_rings));
> +			for_each_ring(engine, to_i915(ring->dev), i) {
> +				if (i == RCS)
> +					continue;
> +
> +				intel_ring_emit(ring, RING_PSMI_CTL(engine->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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[Chris Wilson]

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@farnz.org.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Peter Frühberger <fritsch@xbmc.org>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: stable@vger.kernel.org
---
 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

Re: [PATCH v2 3/3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +5                 360/366              365/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_drv_suspend_fence-restore-untiled      DMESG_WARN(1, M26)PASS(8, M37M26)      PASS(1, M37)
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(8, M37M26)      PASS(1, M37)
 ILK  igt_kms_flip_busy-flip-interruptible      DMESG_WARN(2, M26)PASS(7, M37M26)      PASS(1, M37)
 ILK  igt_kms_flip_flip-vs-rmfb-interruptible      DMESG_WARN(1, M26)PASS(8, M37M26)      PASS(1, M37)
 ILK  igt_kms_flip_rcs-flip-vs-dpms      DMESG_WARN(1, M26)PASS(7, M37M26)      PASS(1, M37)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[Jani Nikula]

On Tue, 16 Dec 2014, Chris Wilson <chris@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@farnz.org.uk>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Tested-by: Peter Frühberger <fritsch@xbmc.org>
> Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
> Cc: stable@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Disable PSMI sleep messages on all rings around context switches

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +4-1              360/366              363/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt_kms_flip_nonexisting-fb      PASS(3, M26)      DMESG_WARN(1, M26)
 ILK  igt_drv_suspend_fence-restore-untiled      DMESG_WARN(1, M26)PASS(9, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(9, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_flip-vs-rmfb-interruptible      DMESG_WARN(1, M26)PASS(9, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_rcs-flip-vs-dpms      DMESG_WARN(1, M26)PASS(8, M37M26)      PASS(1, M26)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx