public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
  2019-02-27 16:58 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
@ 2019-02-27 16:58 ` Mika Kuoppala
  2019-02-27 17:14   ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-27 16:58 UTC (permalink / raw)
  To: intel-gfx

Hardware cannot be in a middle of idle flow messaging
when we pull the plug from ringbuffer. Disable idle
messaging before we do so to avoid potential deadlock
on engine initialization and reset.

v2: INVALID_MMIO_REG, unconditional enable (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 56 +++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a8e47cfa6e35..fe7fca392b63 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -725,7 +725,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
 
 /**
  * intel_engines_cleanup_common - cleans up the engine state created by
- *                                the common initiailizers.
+ *                                the common initializers.
  * @engine: Engine to cleanup.
  *
  * This cleans up everything created by the common helpers.
@@ -826,6 +826,55 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 		      _MASKED_BIT_DISABLE(STOP_RING));
 }
 
+static i915_reg_t get_idle_poll_reg(const struct intel_engine_cs *engine)
+{
+	if (engine->id != RCS)
+		return INVALID_MMIO_REG;
+
+	if (IS_GEN(engine->i915, 9))
+		return GEN9_RCS_FE_FSM2;
+
+	if (IS_GEN(engine->i915, 8))
+		return GEN6_RCS_PWR_FSM;
+
+	return INVALID_MMIO_REG;
+}
+
+static void disable_idle_messaging(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	i915_reg_t poll_reg;
+
+	poll_reg = get_idle_poll_reg(engine);
+	if (!i915_mmio_reg_valid(poll_reg))
+		return;
+
+	GEM_DEBUG_WARN_ON(I915_READ_FW(GEN6_RC_SLEEP_PSMI_CONTROL) &
+			  GEN6_PSMI_SLEEP_MSG_DISABLE);
+
+	I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
+		      _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
+
+	if (__intel_wait_for_register_fw(dev_priv, poll_reg,
+					 0x7f, 0x30,
+					 5000, 0,
+					 NULL))
+		DRM_DEBUG_DRIVER("psmi idle msg poll timeout\n");
+}
+
+static void enable_idle_messaging(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	i915_reg_t poll_reg;
+
+	poll_reg = get_idle_poll_reg(engine);
+	if (!i915_mmio_reg_valid(poll_reg))
+		return;
+
+	I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
+		      _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
+}
+
 int intel_engine_stop_ringbuffer(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -841,9 +890,14 @@ int intel_engine_stop_ringbuffer(struct intel_engine_cs *engine)
 	I915_WRITE_FW(RING_TAIL(base), 0);
 	POSTING_READ_FW(RING_TAIL(base));
 
+	/* Idle messaging needs to be off during ring disable */
+	disable_idle_messaging(engine);
+
 	/* The ring must be empty before it is disabled */
 	I915_WRITE_FW(RING_CTL(base), 0);
 
+	enable_idle_messaging(engine);
+
 	/* Check acts as a post */
 	if (I915_READ_FW(RING_HEAD(base))) {
 		GEM_TRACE("%s: ring head [%x] not parked\n",
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
  2019-02-27 16:58 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
@ 2019-02-27 17:14   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-02-27 17:14 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-27 16:58:50)
> Hardware cannot be in a middle of idle flow messaging
> when we pull the plug from ringbuffer. Disable idle
> messaging before we do so to avoid potential deadlock
> on engine initialization and reset.
> 
> v2: INVALID_MMIO_REG, unconditional enable (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 56 +++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index a8e47cfa6e35..fe7fca392b63 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -725,7 +725,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
>  
>  /**
>   * intel_engines_cleanup_common - cleans up the engine state created by
> - *                                the common initiailizers.
> + *                                the common initializers.
>   * @engine: Engine to cleanup.
>   *
>   * This cleans up everything created by the common helpers.
> @@ -826,6 +826,55 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>                       _MASKED_BIT_DISABLE(STOP_RING));
>  }
>  
> +static i915_reg_t get_idle_poll_reg(const struct intel_engine_cs *engine)
> +{
> +       if (engine->id != RCS)
> +               return INVALID_MMIO_REG;
> +
> +       if (IS_GEN(engine->i915, 9))
> +               return GEN9_RCS_FE_FSM2;
> +
> +       if (IS_GEN(engine->i915, 8))
> +               return GEN6_RCS_PWR_FSM;
> +
> +       return INVALID_MMIO_REG;
> +}
> +
> +static void disable_idle_messaging(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       i915_reg_t poll_reg;
> +
> +       poll_reg = get_idle_poll_reg(engine);
> +       if (!i915_mmio_reg_valid(poll_reg))
> +               return;
> +
> +       GEM_DEBUG_WARN_ON(I915_READ_FW(GEN6_RC_SLEEP_PSMI_CONTROL) &
> +                         GEN6_PSMI_SLEEP_MSG_DISABLE);
> +
> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
> +                     _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
> +

Needs some reference at least. Or a synposis of what exactly we are
waiting for in a comment.

> +       if (__intel_wait_for_register_fw(dev_priv, poll_reg,
> +                                        0x7f, 0x30,
> +                                        5000, 0,
> +                                        NULL))
> +               DRM_DEBUG_DRIVER("psmi idle msg poll timeout\n");
> +}
> +
> +static void enable_idle_messaging(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       i915_reg_t poll_reg;
> +
> +       poll_reg = get_idle_poll_reg(engine);
> +       if (!i915_mmio_reg_valid(poll_reg))
> +               return;
> +
> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
> +                     _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
> +}
> +
>  int intel_engine_stop_ringbuffer(struct intel_engine_cs *engine)
>  {
>         struct drm_i915_private *dev_priv = engine->i915;
> @@ -841,9 +890,14 @@ int intel_engine_stop_ringbuffer(struct intel_engine_cs *engine)
>         I915_WRITE_FW(RING_TAIL(base), 0);
>         POSTING_READ_FW(RING_TAIL(base));
>  
> +       /* Idle messaging needs to be off during ring disable */
> +       disable_idle_messaging(engine);
> +
>         /* The ring must be empty before it is disabled */
>         I915_WRITE_FW(RING_CTL(base), 0);
>  
> +       enable_idle_messaging(engine);
> +
>         /* Check acts as a post */
>         if (I915_READ_FW(RING_HEAD(base))) {
>                 GEM_TRACE("%s: ring head [%x] not parked\n",

Given the history SLEEP_MSG_DISABLE around RING_CTL is not surprising.

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
@ 2019-02-28 16:01 Mika Kuoppala
  2019-02-28 16:01 ` [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer Mika Kuoppala
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:01 UTC (permalink / raw)
  To: intel-gfx

We have an exported function for stopping the engine before
disabling a ringbuffer. Take it into use.

v2: use fw on empty check

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index df8f88142f1d..e35dc0386bf6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	if (INTEL_GEN(dev_priv) < 3)
+		return;
+
 	GEM_TRACE("%s\n", engine->name);
 
 	I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1b96b0960adc..d7486f9a29a9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
 	flush_cs_tlb(engine);
 }
 
+static bool ring_is_empty(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	const u32 base = engine->mmio_base;
+
+	return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
+		(I915_READ_FW(RING_HEAD(base)) & TAIL_ADDR);
+}
+
 static bool stop_ring(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	int ret;
 
-	if (INTEL_GEN(dev_priv) > 2) {
-		I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
-		if (intel_wait_for_register(dev_priv,
-					    RING_MI_MODE(engine->mmio_base),
-					    MODE_IDLE,
-					    MODE_IDLE,
-					    1000)) {
-			DRM_ERROR("%s : timed out trying to stop ring\n",
-				  engine->name);
-			/* Sometimes we observe that the idle flag is not
-			 * set even though the ring is empty. So double
-			 * check before giving up.
-			 */
-			if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
-				return false;
-		}
+	ret = intel_engine_stop_cs(engine);
+	if (ret == -ENODEV)
+		ret = 0;
+
+	if (ret) {
+		/*
+		 * Sometimes we observe that the idle flag is not
+		 * set even though the ring is empty. So double
+		 * check before giving up.
+		 */
+		if (!ring_is_empty(engine))
+			return false;
 	}
 
 	I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
@@ -718,8 +724,7 @@ static int init_ring_common(struct intel_engine_cs *engine)
 		goto out;
 	}
 
-	if (INTEL_GEN(dev_priv) > 2)
-		I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
+	intel_engine_cancel_stop_cs(engine);
 
 	/* Now awake, let it get started */
 	if (ring->tail != ring->head) {
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
@ 2019-02-28 16:01 ` Mika Kuoppala
  2019-02-28 16:51   ` Chris Wilson
  2019-02-28 16:01 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:01 UTC (permalink / raw)
  To: intel-gfx

We use identical sequence of stopping ringbuffer on reset
handing and on ring initialization. Make a function
to handle both cases.

v2: intel_engine_stop_ring, cleaner stop_ring (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reset.c       | 18 +---------------
 drivers/gpu/drm/i915/intel_engine_cs.c  | 28 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++--------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
 4 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index 55d6123dbba4..e7be0bf881c3 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -113,28 +113,12 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
 
 static void gen3_stop_engine(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
-	const u32 base = engine->mmio_base;
-
 	GEM_TRACE("%s\n", engine->name);
 
 	if (intel_engine_stop_cs(engine))
 		GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
 
-	I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
-	POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
-
-	I915_WRITE_FW(RING_HEAD(base), 0);
-	I915_WRITE_FW(RING_TAIL(base), 0);
-	POSTING_READ_FW(RING_TAIL(base));
-
-	/* The ring must be empty before it is disabled */
-	I915_WRITE_FW(RING_CTL(base), 0);
-
-	/* Check acts as a post */
-	if (I915_READ_FW(RING_HEAD(base)))
-		GEM_TRACE("%s: ring head [%x] not parked\n",
-			  engine->name, I915_READ_FW(RING_HEAD(base)));
+	intel_engine_stop_ring(engine);
 }
 
 static void i915_stop_engines(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index e35dc0386bf6..2a94b92cfcd3 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -865,6 +865,34 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 		      _MASKED_BIT_DISABLE(STOP_RING));
 }
 
+int intel_engine_stop_ring(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	const u32 base = engine->mmio_base;
+
+	assert_forcewakes_active(dev_priv, FORCEWAKE_ALL);
+	GEM_TRACE("%s\n", engine->name);
+
+	I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
+	POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
+
+	I915_WRITE_FW(RING_HEAD(base), 0);
+	I915_WRITE_FW(RING_TAIL(base), 0);
+	POSTING_READ_FW(RING_TAIL(base));
+
+	/* The ring must be empty before it is disabled */
+	I915_WRITE_FW(RING_CTL(base), 0);
+
+	/* Check acts as a post */
+	if (I915_READ_FW(RING_HEAD(base))) {
+		GEM_TRACE("%s: ring head [%x] not parked\n",
+			  engine->name, I915_READ_FW(RING_HEAD(base)));
+		return -EIO;
+	}
+
+	return 0;
+}
+
 const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
 {
 	switch (type) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d7486f9a29a9..e099ef088d1a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -615,7 +615,6 @@ static bool ring_is_empty(struct intel_engine_cs *engine)
 
 static bool stop_ring(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
 	int ret;
 
 	ret = intel_engine_stop_cs(engine);
@@ -632,15 +631,10 @@ static bool stop_ring(struct intel_engine_cs *engine)
 			return false;
 	}
 
-	I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
-
-	I915_WRITE_HEAD(engine, 0);
-	I915_WRITE_TAIL(engine, 0);
-
-	/* The ring must be empty before it is disabled */
-	I915_WRITE_CTL(engine, 0);
+	if (intel_engine_stop_ring(engine))
+		return false;
 
-	return (I915_READ_HEAD(engine) & HEAD_ADDR) == 0;
+	return true;
 }
 
 static int init_ring_common(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b8ec7e40a59b..5ed895c56a35 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -843,6 +843,8 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine);
 int intel_engine_stop_cs(struct intel_engine_cs *engine);
 void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine);
 
+int intel_engine_stop_ring(struct intel_engine_cs *engine);
+
 void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask);
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
  2019-02-28 16:01 ` [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer Mika Kuoppala
@ 2019-02-28 16:01 ` Mika Kuoppala
  2019-02-28 16:22   ` Chris Wilson
  2019-02-28 16:10 ` [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:01 UTC (permalink / raw)
  To: intel-gfx

Hardware cannot be in a middle of idle flow messaging
when we pull the plug from ringbuffer. Disable idle
messaging before we do so to avoid potential deadlock
on engine initialization and reset.

v2: INVALID_MMIO_REG, unconditional enable (Chris)
v3: comment (Chris), bspec reference

References: bspec/11751
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 64 +++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 2a94b92cfcd3..10cb6b22fe92 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -764,7 +764,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
 
 /**
  * intel_engines_cleanup_common - cleans up the engine state created by
- *                                the common initiailizers.
+ *                                the common initializers.
  * @engine: Engine to cleanup.
  *
  * This cleans up everything created by the common helpers.
@@ -865,6 +865,63 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 		      _MASKED_BIT_DISABLE(STOP_RING));
 }
 
+static i915_reg_t get_idle_poll_reg(const struct intel_engine_cs *engine)
+{
+	if (engine->id != RCS)
+		return INVALID_MMIO_REG;
+
+	if (IS_GEN(engine->i915, 9))
+		return GEN9_RCS_FE_FSM2;
+
+	if (IS_GEN(engine->i915, 8))
+		return GEN6_RCS_PWR_FSM;
+
+	return INVALID_MMIO_REG;
+}
+
+static void disable_idle_messaging(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	i915_reg_t poll_reg;
+
+	poll_reg = get_idle_poll_reg(engine);
+	if (!i915_mmio_reg_valid(poll_reg))
+		return;
+
+	GEM_DEBUG_WARN_ON(I915_READ_FW(GEN6_RC_SLEEP_PSMI_CONTROL) &
+			  GEN6_PSMI_SLEEP_MSG_DISABLE);
+	/*
+	 * Hardware must not be in middle of idle flow signalling
+	 * when the RING_CTL is zeroed. In order to prevent this
+	 * we disable the messaging temporarily.
+	 */
+	I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
+		      _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
+
+	/*
+	 * Poll the magic bits to ensure that engine state machine
+	 * is in valid state for stopping the ring.
+	 */
+	if (__intel_wait_for_register_fw(dev_priv, poll_reg,
+					 0x7f, 0x30,
+					 5000, 0,
+					 NULL))
+		DRM_DEBUG_DRIVER("psmi idle msg poll timeout\n");
+}
+
+static void enable_idle_messaging(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	i915_reg_t poll_reg;
+
+	poll_reg = get_idle_poll_reg(engine);
+	if (!i915_mmio_reg_valid(poll_reg))
+		return;
+
+	I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
+		      _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
+}
+
 int intel_engine_stop_ring(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -880,9 +937,14 @@ int intel_engine_stop_ring(struct intel_engine_cs *engine)
 	I915_WRITE_FW(RING_TAIL(base), 0);
 	POSTING_READ_FW(RING_TAIL(base));
 
+	/* Idle messaging needs to be off during ring disable */
+	disable_idle_messaging(engine);
+
 	/* The ring must be empty before it is disabled */
 	I915_WRITE_FW(RING_CTL(base), 0);
 
+	enable_idle_messaging(engine);
+
 	/* Check acts as a post */
 	if (I915_READ_FW(RING_HEAD(base))) {
 		GEM_TRACE("%s: ring head [%x] not parked\n",
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
  2019-02-28 16:01 ` [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer Mika Kuoppala
  2019-02-28 16:01 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
@ 2019-02-28 16:10 ` Mika Kuoppala
  2019-02-28 16:14 ` Mika Kuoppala
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:10 UTC (permalink / raw)
  To: intel-gfx

Mika Kuoppala <mika.kuoppala@linux.intel.com> writes:

> We have an exported function for stopping the engine before
> disabling a ringbuffer. Take it into use.
>
> v2: use fw on empty check
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
>  2 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index df8f88142f1d..e35dc0386bf6 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *dev_priv = engine->i915;
>  
> +	if (INTEL_GEN(dev_priv) < 3)
> +		return;
> +
>  	GEM_TRACE("%s\n", engine->name);
>  
>  	I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 1b96b0960adc..d7486f9a29a9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
>  	flush_cs_tlb(engine);
>  }
>  
> +static bool ring_is_empty(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *dev_priv = engine->i915;
> +	const u32 base = engine->mmio_base;
> +
> +	return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
> +		(I915_READ_FW(RING_HEAD(base)) & TAIL_ADDR);

ARgh!
-Mika

> +}
> +
>  static bool stop_ring(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *dev_priv = engine->i915;
> +	int ret;
>  
> -	if (INTEL_GEN(dev_priv) > 2) {
> -		I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
> -		if (intel_wait_for_register(dev_priv,
> -					    RING_MI_MODE(engine->mmio_base),
> -					    MODE_IDLE,
> -					    MODE_IDLE,
> -					    1000)) {
> -			DRM_ERROR("%s : timed out trying to stop ring\n",
> -				  engine->name);
> -			/* Sometimes we observe that the idle flag is not
> -			 * set even though the ring is empty. So double
> -			 * check before giving up.
> -			 */
> -			if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
> -				return false;
> -		}
> +	ret = intel_engine_stop_cs(engine);
> +	if (ret == -ENODEV)
> +		ret = 0;
> +
> +	if (ret) {
> +		/*
> +		 * Sometimes we observe that the idle flag is not
> +		 * set even though the ring is empty. So double
> +		 * check before giving up.
> +		 */
> +		if (!ring_is_empty(engine))
> +			return false;
>  	}
>  
>  	I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
> @@ -718,8 +724,7 @@ static int init_ring_common(struct intel_engine_cs *engine)
>  		goto out;
>  	}
>  
> -	if (INTEL_GEN(dev_priv) > 2)
> -		I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
> +	intel_engine_cancel_stop_cs(engine);
>  
>  	/* Now awake, let it get started */
>  	if (ring->tail != ring->head) {
> -- 
> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
                   ` (2 preceding siblings ...)
  2019-02-28 16:10 ` [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
@ 2019-02-28 16:14 ` Mika Kuoppala
  2019-02-28 16:34   ` Chris Wilson
  2019-02-28 17:01 ` ✓ Fi.CI.BAT: success for series starting with [1/3] " Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:14 UTC (permalink / raw)
  To: intel-gfx

We have an exported function for stopping the engine before
disabling a ringbuffer. Take it into use.

v2: use fw on empty check
v3: tail is tail

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index df8f88142f1d..e35dc0386bf6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	if (INTEL_GEN(dev_priv) < 3)
+		return;
+
 	GEM_TRACE("%s\n", engine->name);
 
 	I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1b96b0960adc..5fe28d9087b7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
 	flush_cs_tlb(engine);
 }
 
+static bool ring_is_empty(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *dev_priv = engine->i915;
+	const u32 base = engine->mmio_base;
+
+	return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
+		(I915_READ_FW(RING_TAIL(base)) & TAIL_ADDR);
+}
+
 static bool stop_ring(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	int ret;
 
-	if (INTEL_GEN(dev_priv) > 2) {
-		I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
-		if (intel_wait_for_register(dev_priv,
-					    RING_MI_MODE(engine->mmio_base),
-					    MODE_IDLE,
-					    MODE_IDLE,
-					    1000)) {
-			DRM_ERROR("%s : timed out trying to stop ring\n",
-				  engine->name);
-			/* Sometimes we observe that the idle flag is not
-			 * set even though the ring is empty. So double
-			 * check before giving up.
-			 */
-			if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
-				return false;
-		}
+	ret = intel_engine_stop_cs(engine);
+	if (ret == -ENODEV)
+		ret = 0;
+
+	if (ret) {
+		/*
+		 * Sometimes we observe that the idle flag is not
+		 * set even though the ring is empty. So double
+		 * check before giving up.
+		 */
+		if (!ring_is_empty(engine))
+			return false;
 	}
 
 	I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
@@ -718,8 +724,7 @@ static int init_ring_common(struct intel_engine_cs *engine)
 		goto out;
 	}
 
-	if (INTEL_GEN(dev_priv) > 2)
-		I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
+	intel_engine_cancel_stop_cs(engine);
 
 	/* Now awake, let it get started */
 	if (ring->tail != ring->head) {
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
  2019-02-28 16:01 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
@ 2019-02-28 16:22   ` Chris Wilson
  2019-02-28 16:33     ` Mika Kuoppala
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2019-02-28 16:22 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-28 16:01:33)
> Hardware cannot be in a middle of idle flow messaging
> when we pull the plug from ringbuffer. Disable idle
> messaging before we do so to avoid potential deadlock
> on engine initialization and reset.
> 
> v2: INVALID_MMIO_REG, unconditional enable (Chris)
> v3: comment (Chris), bspec reference
> 
> References: bspec/11751
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 64 +++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 2a94b92cfcd3..10cb6b22fe92 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -764,7 +764,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
>  
>  /**
>   * intel_engines_cleanup_common - cleans up the engine state created by
> - *                                the common initiailizers.
> + *                                the common initializers.
>   * @engine: Engine to cleanup.
>   *
>   * This cleans up everything created by the common helpers.
> @@ -865,6 +865,63 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>                       _MASKED_BIT_DISABLE(STOP_RING));
>  }
>  
> +static i915_reg_t get_idle_poll_reg(const struct intel_engine_cs *engine)
> +{
> +       if (engine->id != RCS)
> +               return INVALID_MMIO_REG;
> +
> +       if (IS_GEN(engine->i915, 9))
> +               return GEN9_RCS_FE_FSM2;
> +
> +       if (IS_GEN(engine->i915, 8))
> +               return GEN6_RCS_PWR_FSM;
> +
> +       return INVALID_MMIO_REG;
> +}
> +
> +static void disable_idle_messaging(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       i915_reg_t poll_reg;
> +
> +       poll_reg = get_idle_poll_reg(engine);
> +       if (!i915_mmio_reg_valid(poll_reg))
> +               return;
> +
> +       GEM_DEBUG_WARN_ON(I915_READ_FW(GEN6_RC_SLEEP_PSMI_CONTROL) &
> +                         GEN6_PSMI_SLEEP_MSG_DISABLE);
> +       /*
> +        * Hardware must not be in middle of idle flow signalling
> +        * when the RING_CTL is zeroed. In order to prevent this
> +        * we disable the messaging temporarily.
> +        */
> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
> +                     _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
> +
> +       /*
> +        * Poll the magic bits to ensure that engine state machine
> +        * is in valid state for stopping the ring.
> +        */
> +       if (__intel_wait_for_register_fw(dev_priv, poll_reg,
> +                                        0x7f, 0x30,
> +                                        5000, 0,
> +                                        NULL))
> +               DRM_DEBUG_DRIVER("psmi idle msg poll timeout\n");
> +}
> +
> +static void enable_idle_messaging(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       i915_reg_t poll_reg;
> +
> +       poll_reg = get_idle_poll_reg(engine);
> +       if (!i915_mmio_reg_valid(poll_reg))
> +               return;
> +
> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
> +                     _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
> +}
> +
>  int intel_engine_stop_ring(struct intel_engine_cs *engine)
>  {
>         struct drm_i915_private *dev_priv = engine->i915;
> @@ -880,9 +937,14 @@ int intel_engine_stop_ring(struct intel_engine_cs *engine)
>         I915_WRITE_FW(RING_TAIL(base), 0);
>         POSTING_READ_FW(RING_TAIL(base));
>  
> +       /* Idle messaging needs to be off during ring disable */

Ok, that's ott :)

The previous pair are a nice why, peeling back some of the layers of the
HW to see what's going on. This one is just repeating
disable_idle_messing().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
  2019-02-28 16:22   ` Chris Wilson
@ 2019-02-28 16:33     ` Mika Kuoppala
  0 siblings, 0 replies; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-02-28 16:01:33)
>> Hardware cannot be in a middle of idle flow messaging
>> when we pull the plug from ringbuffer. Disable idle
>> messaging before we do so to avoid potential deadlock
>> on engine initialization and reset.
>> 
>> v2: INVALID_MMIO_REG, unconditional enable (Chris)
>> v3: comment (Chris), bspec reference
>> 
>> References: bspec/11751
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_engine_cs.c | 64 +++++++++++++++++++++++++-
>>  1 file changed, 63 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>> index 2a94b92cfcd3..10cb6b22fe92 100644
>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>> @@ -764,7 +764,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
>>  
>>  /**
>>   * intel_engines_cleanup_common - cleans up the engine state created by
>> - *                                the common initiailizers.
>> + *                                the common initializers.
>>   * @engine: Engine to cleanup.
>>   *
>>   * This cleans up everything created by the common helpers.
>> @@ -865,6 +865,63 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>>                       _MASKED_BIT_DISABLE(STOP_RING));
>>  }
>>  
>> +static i915_reg_t get_idle_poll_reg(const struct intel_engine_cs *engine)
>> +{
>> +       if (engine->id != RCS)
>> +               return INVALID_MMIO_REG;
>> +
>> +       if (IS_GEN(engine->i915, 9))
>> +               return GEN9_RCS_FE_FSM2;
>> +
>> +       if (IS_GEN(engine->i915, 8))
>> +               return GEN6_RCS_PWR_FSM;
>> +
>> +       return INVALID_MMIO_REG;
>> +}
>> +
>> +static void disable_idle_messaging(struct intel_engine_cs *engine)
>> +{
>> +       struct drm_i915_private *dev_priv = engine->i915;
>> +       i915_reg_t poll_reg;
>> +
>> +       poll_reg = get_idle_poll_reg(engine);
>> +       if (!i915_mmio_reg_valid(poll_reg))
>> +               return;
>> +
>> +       GEM_DEBUG_WARN_ON(I915_READ_FW(GEN6_RC_SLEEP_PSMI_CONTROL) &
>> +                         GEN6_PSMI_SLEEP_MSG_DISABLE);
>> +       /*
>> +        * Hardware must not be in middle of idle flow signalling
>> +        * when the RING_CTL is zeroed. In order to prevent this
>> +        * we disable the messaging temporarily.
>> +        */
>> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
>> +                     _MASKED_BIT_ENABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
>> +
>> +       /*
>> +        * Poll the magic bits to ensure that engine state machine
>> +        * is in valid state for stopping the ring.
>> +        */
>> +       if (__intel_wait_for_register_fw(dev_priv, poll_reg,
>> +                                        0x7f, 0x30,
>> +                                        5000, 0,
>> +                                        NULL))
>> +               DRM_DEBUG_DRIVER("psmi idle msg poll timeout\n");
>> +}
>> +
>> +static void enable_idle_messaging(struct intel_engine_cs *engine)
>> +{
>> +       struct drm_i915_private *dev_priv = engine->i915;
>> +       i915_reg_t poll_reg;
>> +
>> +       poll_reg = get_idle_poll_reg(engine);
>> +       if (!i915_mmio_reg_valid(poll_reg))
>> +               return;
>> +
>> +       I915_WRITE_FW(GEN6_RC_SLEEP_PSMI_CONTROL,
>> +                     _MASKED_BIT_DISABLE(GEN6_PSMI_SLEEP_MSG_DISABLE));
>> +}
>> +
>>  int intel_engine_stop_ring(struct intel_engine_cs *engine)
>>  {
>>         struct drm_i915_private *dev_priv = engine->i915;
>> @@ -880,9 +937,14 @@ int intel_engine_stop_ring(struct intel_engine_cs *engine)
>>         I915_WRITE_FW(RING_TAIL(base), 0);
>>         POSTING_READ_FW(RING_TAIL(base));
>>  
>> +       /* Idle messaging needs to be off during ring disable */
>
> Ok, that's ott :)

Rest assured, Captain Obvious will scrap this before merging.

>
> The previous pair are a nice why, peeling back some of the layers of the
> HW to see what's going on. This one is just repeating
> disable_idle_messing().

There is this dream I had where hardware engineers
did review patches.

But it should be close enough of 'what's going on'

Thanks,
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:14 ` Mika Kuoppala
@ 2019-02-28 16:34   ` Chris Wilson
  2019-02-28 16:53     ` Mika Kuoppala
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2019-02-28 16:34 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-28 16:14:11)
> We have an exported function for stopping the engine before
> disabling a ringbuffer. Take it into use.
> 
> v2: use fw on empty check
> v3: tail is tail
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
>  2 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index df8f88142f1d..e35dc0386bf6 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>  {
>         struct drm_i915_private *dev_priv = engine->i915;
>  
> +       if (INTEL_GEN(dev_priv) < 3)
> +               return;
> +
>         GEM_TRACE("%s\n", engine->name);
>  
>         I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 1b96b0960adc..5fe28d9087b7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
>         flush_cs_tlb(engine);
>  }
>  
> +static bool ring_is_empty(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       const u32 base = engine->mmio_base;
> +
> +       return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
> +               (I915_READ_FW(RING_TAIL(base)) & TAIL_ADDR);
> +}
> +
>  static bool stop_ring(struct intel_engine_cs *engine)
>  {
>         struct drm_i915_private *dev_priv = engine->i915;
> +       int ret;
>  
> -       if (INTEL_GEN(dev_priv) > 2) {
> -               I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
> -               if (intel_wait_for_register(dev_priv,
> -                                           RING_MI_MODE(engine->mmio_base),
> -                                           MODE_IDLE,
> -                                           MODE_IDLE,
> -                                           1000)) {
> -                       DRM_ERROR("%s : timed out trying to stop ring\n",
> -                                 engine->name);
> -                       /* Sometimes we observe that the idle flag is not
> -                        * set even though the ring is empty. So double
> -                        * check before giving up.
> -                        */
> -                       if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
> -                               return false;
> -               }
> +       ret = intel_engine_stop_cs(engine);
> +       if (ret == -ENODEV)
> +               ret = 0;
> +
> +       if (ret) {
> +               /*
> +                * Sometimes we observe that the idle flag is not
> +                * set even though the ring is empty. So double
> +                * check before giving up.
> +                */
> +               if (!ring_is_empty(engine))
> +                       return false;

Hmm, thinking more about this, shouldn't we push this down to stop_cs()?

If that's reporting an error in a situation where we can determine that
the ring is idle anyway, we can report the stop_cs succeeded.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
  2019-02-28 16:01 ` [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer Mika Kuoppala
@ 2019-02-28 16:51   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-02-28 16:51 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-28 16:01:32)
> We use identical sequence of stopping ringbuffer on reset
> handing and on ring initialization. Make a function
> to handle both cases.
> 
> v2: intel_engine_stop_ring, cleaner stop_ring (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:34   ` Chris Wilson
@ 2019-02-28 16:53     ` Mika Kuoppala
  2019-02-28 17:00       ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-02-28 16:53 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-02-28 16:14:11)
>> We have an exported function for stopping the engine before
>> disabling a ringbuffer. Take it into use.
>> 
>> v2: use fw on empty check
>> v3: tail is tail
>> 
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
>>  2 files changed, 26 insertions(+), 18 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>> index df8f88142f1d..e35dc0386bf6 100644
>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>> @@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>>  {
>>         struct drm_i915_private *dev_priv = engine->i915;
>>  
>> +       if (INTEL_GEN(dev_priv) < 3)
>> +               return;
>> +
>>         GEM_TRACE("%s\n", engine->name);
>>  
>>         I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 1b96b0960adc..5fe28d9087b7 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
>>         flush_cs_tlb(engine);
>>  }
>>  
>> +static bool ring_is_empty(struct intel_engine_cs *engine)
>> +{
>> +       struct drm_i915_private *dev_priv = engine->i915;
>> +       const u32 base = engine->mmio_base;
>> +
>> +       return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
>> +               (I915_READ_FW(RING_TAIL(base)) & TAIL_ADDR);
>> +}
>> +
>>  static bool stop_ring(struct intel_engine_cs *engine)
>>  {
>>         struct drm_i915_private *dev_priv = engine->i915;
>> +       int ret;
>>  
>> -       if (INTEL_GEN(dev_priv) > 2) {
>> -               I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
>> -               if (intel_wait_for_register(dev_priv,
>> -                                           RING_MI_MODE(engine->mmio_base),
>> -                                           MODE_IDLE,
>> -                                           MODE_IDLE,
>> -                                           1000)) {
>> -                       DRM_ERROR("%s : timed out trying to stop ring\n",
>> -                                 engine->name);
>> -                       /* Sometimes we observe that the idle flag is not
>> -                        * set even though the ring is empty. So double
>> -                        * check before giving up.
>> -                        */
>> -                       if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
>> -                               return false;
>> -               }
>> +       ret = intel_engine_stop_cs(engine);
>> +       if (ret == -ENODEV)
>> +               ret = 0;
>> +
>> +       if (ret) {
>> +               /*
>> +                * Sometimes we observe that the idle flag is not
>> +                * set even though the ring is empty. So double
>> +                * check before giving up.
>> +                */
>> +               if (!ring_is_empty(engine))
>> +                       return false;
>
> Hmm, thinking more about this, shouldn't we push this down to stop_cs()?
>
> If that's reporting an error in a situation where we can determine that
> the ring is idle anyway, we can report the stop_cs succeeded.

Makes sense, I will take a look.

I felt small urge to deflate the 'stop'.

Would it be confusing if we just did
intel_engine_start|stop instead of stop_cs and
cancel_stop_cs?

So hmm:

intel_engine_start()
intel_engine_stop()

these would only toggle the STOP_RING

and for replacing stop_ring with:
intel_engine_empty_ring()

for zeroing the heads.

one 'stop' to rule the (ring)world!?

-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:53     ` Mika Kuoppala
@ 2019-02-28 17:00       ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-02-28 17:00 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-02-28 16:53:46)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-02-28 16:14:11)
> >> We have an exported function for stopping the engine before
> >> disabling a ringbuffer. Take it into use.
> >> 
> >> v2: use fw on empty check
> >> v3: tail is tail
> >> 
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >>  drivers/gpu/drm/i915/intel_engine_cs.c  |  3 ++
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++-----------
> >>  2 files changed, 26 insertions(+), 18 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> index df8f88142f1d..e35dc0386bf6 100644
> >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> @@ -856,6 +856,9 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
> >>  {
> >>         struct drm_i915_private *dev_priv = engine->i915;
> >>  
> >> +       if (INTEL_GEN(dev_priv) < 3)
> >> +               return;
> >> +
> >>         GEM_TRACE("%s\n", engine->name);
> >>  
> >>         I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> index 1b96b0960adc..5fe28d9087b7 100644
> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> @@ -604,26 +604,32 @@ static void ring_setup_status_page(struct intel_engine_cs *engine)
> >>         flush_cs_tlb(engine);
> >>  }
> >>  
> >> +static bool ring_is_empty(struct intel_engine_cs *engine)
> >> +{
> >> +       struct drm_i915_private *dev_priv = engine->i915;
> >> +       const u32 base = engine->mmio_base;
> >> +
> >> +       return (I915_READ_FW(RING_HEAD(base)) & HEAD_ADDR) ==
> >> +               (I915_READ_FW(RING_TAIL(base)) & TAIL_ADDR);
> >> +}
> >> +
> >>  static bool stop_ring(struct intel_engine_cs *engine)
> >>  {
> >>         struct drm_i915_private *dev_priv = engine->i915;
> >> +       int ret;
> >>  
> >> -       if (INTEL_GEN(dev_priv) > 2) {
> >> -               I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
> >> -               if (intel_wait_for_register(dev_priv,
> >> -                                           RING_MI_MODE(engine->mmio_base),
> >> -                                           MODE_IDLE,
> >> -                                           MODE_IDLE,
> >> -                                           1000)) {
> >> -                       DRM_ERROR("%s : timed out trying to stop ring\n",
> >> -                                 engine->name);
> >> -                       /* Sometimes we observe that the idle flag is not
> >> -                        * set even though the ring is empty. So double
> >> -                        * check before giving up.
> >> -                        */
> >> -                       if (I915_READ_HEAD(engine) != I915_READ_TAIL(engine))
> >> -                               return false;
> >> -               }
> >> +       ret = intel_engine_stop_cs(engine);
> >> +       if (ret == -ENODEV)
> >> +               ret = 0;
> >> +
> >> +       if (ret) {
> >> +               /*
> >> +                * Sometimes we observe that the idle flag is not
> >> +                * set even though the ring is empty. So double
> >> +                * check before giving up.
> >> +                */
> >> +               if (!ring_is_empty(engine))
> >> +                       return false;
> >
> > Hmm, thinking more about this, shouldn't we push this down to stop_cs()?
> >
> > If that's reporting an error in a situation where we can determine that
> > the ring is idle anyway, we can report the stop_cs succeeded.
> 
> Makes sense, I will take a look.
> 
> I felt small urge to deflate the 'stop'.
> 
> Would it be confusing if we just did
> intel_engine_start|stop instead of stop_cs and
> cancel_stop_cs?
> 
> So hmm:
> 
> intel_engine_start()
> intel_engine_stop()
> 
> these would only toggle the STOP_RING
> 
> and for replacing stop_ring with:
> intel_engine_empty_ring()

intel_engine_clear_ring() / reset_ring(). Hmm, clear_ring of those two.
> 
> for zeroing the heads.
> 
> one 'stop' to rule the (ring)world!?

The counter argument is that _start() is a little too broad. The appeal
of stop_cs() is that it describing what it is doing, poking at the bit
to stop the CS advancing and nothing more. It frequently doesn't
succeed...

So I think it's not a worthy change, but I never did feel totally
satisfied with stop_cs -- cs is too short, but we do have usage with
xCS.

intel_engine_stop_command_streamer,
intel_engine_halt_command_streamer,
intel_engine_pause_command_streamer,
?

But intel_engine_clear_ring() I could be sold on.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
                   ` (3 preceding siblings ...)
  2019-02-28 16:14 ` Mika Kuoppala
@ 2019-02-28 17:01 ` Patchwork
  2019-02-28 17:04 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2) Patchwork
  2019-02-28 19:24 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-02-28 17:01 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer
URL   : https://patchwork.freedesktop.org/series/57352/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5674 -> Patchwork_12331
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57352/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12331 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720] / [fdo#109799]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109779]: https://bugs.freedesktop.org/show_bug.cgi?id=109779
  [fdo#109799]: https://bugs.freedesktop.org/show_bug.cgi?id=109799


Participating hosts (44 -> 41)
------------------------------

  Additional (2): fi-icl-y fi-hsw-peppy 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5674 -> Patchwork_12331

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12331: 246c9a8ebbc1cfbb6374c1ebf310f30706593b5e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

246c9a8ebbc1 drm/i915: Disable PSMI idle messaging when stopping ring
994f53ea5eff drm/i915: Introduce intel_engine_stop_ringbuffer
c408614e8a4c drm/i915: Use intel_engine_stop_cs when stopping ringbuffer

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12331/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2)
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
                   ` (4 preceding siblings ...)
  2019-02-28 17:01 ` ✓ Fi.CI.BAT: success for series starting with [1/3] " Patchwork
@ 2019-02-28 17:04 ` Patchwork
  2019-02-28 19:24 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-02-28 17:04 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2)
URL   : https://patchwork.freedesktop.org/series/57352/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5674 -> Patchwork_12332
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57352/revisions/2/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12332 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720] / [fdo#109799]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109799]: https://bugs.freedesktop.org/show_bug.cgi?id=109799


Participating hosts (44 -> 39)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5674 -> Patchwork_12332

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12332: 6304b6caa2e3282ea6bd67138911d4a7c2c24815 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6304b6caa2e3 drm/i915: Disable PSMI idle messaging when stopping ring
d35a973d87dd drm/i915: Introduce intel_engine_stop_ringbuffer
869ff2bfc80a drm/i915: Use intel_engine_stop_cs when stopping ringbuffer

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12332/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2)
  2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
                   ` (5 preceding siblings ...)
  2019-02-28 17:04 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2) Patchwork
@ 2019-02-28 19:24 ` Patchwork
  6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-02-28 19:24 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2)
URL   : https://patchwork.freedesktop.org/series/57352/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5674_full -> Patchwork_12332_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12332_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_pwrite@stolen-normal:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +60

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108]

  * igt@kms_busy@basic-flip-d:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_chv_cursor_fail@pipe-a-64x64-bottom-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671]

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-glk:          PASS -> FAIL [fdo#109350]

  * igt@kms_draw_crc@draw-method-rgb565-pwrite-xtiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-skl:          PASS -> FAIL [fdo#103167] / [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
    - shard-skl:          PASS -> FAIL [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +17

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-move:
    - shard-skl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +13

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_rotation_crc@primary-rotation-90:
    - shard-skl:          PASS -> FAIL [fdo#103925] / [fdo#107815]

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894]

  * igt@perf_pmu@busy-check-all-vecs0:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +78

  * igt@tools_test@tools_test:
    - shard-kbl:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-snb:          FAIL [fdo#103375] -> PASS

  * igt@i915_pm_rpm@universal-planes-dpms:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
    - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS +1

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-offscreen:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          FAIL [fdo#109350] -> PASS

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-xtiled:
    - shard-skl:          FAIL [fdo#103184] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103925]: https://bugs.freedesktop.org/show_bug.cgi?id=103925
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350


Participating hosts (6 -> 6)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5674 -> Patchwork_12332

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12332: 6304b6caa2e3282ea6bd67138911d4a7c2c24815 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12332/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-02-28 19:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-28 16:01 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
2019-02-28 16:01 ` [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer Mika Kuoppala
2019-02-28 16:51   ` Chris Wilson
2019-02-28 16:01 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
2019-02-28 16:22   ` Chris Wilson
2019-02-28 16:33     ` Mika Kuoppala
2019-02-28 16:10 ` [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
2019-02-28 16:14 ` Mika Kuoppala
2019-02-28 16:34   ` Chris Wilson
2019-02-28 16:53     ` Mika Kuoppala
2019-02-28 17:00       ` Chris Wilson
2019-02-28 17:01 ` ✓ Fi.CI.BAT: success for series starting with [1/3] " Patchwork
2019-02-28 17:04 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer (rev2) Patchwork
2019-02-28 19:24 ` ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2019-02-27 16:58 [PATCH 1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer Mika Kuoppala
2019-02-27 16:58 ` [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring Mika Kuoppala
2019-02-27 17:14   ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox