Re: [Intel-gfx] [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
Chris Wilson 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
>> Acked-by: Chris Wilson
>> Signed-off-by: Mika Kuoppala
>> ---
>> 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
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
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
> Acked-by: Chris Wilson
> Signed-off-by: Mika Kuoppala
> ---
> 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
[Intel-gfx] [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
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
Acked-by: Chris Wilson
Signed-off-by: Mika Kuoppala
---
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
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
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
> Signed-off-by: Mika Kuoppala
> ---
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Disable PSMI idle messaging when stopping ring
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
Signed-off-by: Mika Kuoppala
---
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
