[Intel-gfx] [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
We use identical sequence of stopping ringbuffer on reset
handing and on ring initialization. Make a function
to handle both cases.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_reset.c | 18 +---
drivers/gpu/drm/i915/intel_engine_cs.c | 28 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++---
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
4 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reset.c
b/drivers/gpu/drm/i915/i915_reset.c
index 55d6123dbba4..4191398b5125 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_ringbuffer(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 3feb0f74c239..a8e47cfa6e35 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -826,6 +826,34 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs
*engine)
_MASKED_BIT_DISABLE(STOP_RING));
}
+int intel_engine_stop_ringbuffer(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 5363dad1208d..8936a9051760 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -622,15 +622,9 @@ static bool stop_ring(struct intel_engine_cs *engine)
return false;
}
- I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
+ ret = intel_engine_stop_ringbuffer(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);
-
- return (I915_READ_HEAD(engine) & HEAD_ADDR) == 0;
+ return ret == 0;
}
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 de8dba7565b0..10eb0f9a1432 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -858,6 +858,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_ringbuffer(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
[Intel-gfx] [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
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
Signed-off-by: Mika Kuoppala
---
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
__
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
Quoting Mika Kuoppala (2019-02-27 16:58:49)
> We use identical sequence of stopping ringbuffer on reset
> handing and on ring initialization. Make a function
> to handle both cases.
>
> Cc: Chris Wilson
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_reset.c | 18 +---
> drivers/gpu/drm/i915/intel_engine_cs.c | 28 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++---
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
> 4 files changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c
> b/drivers/gpu/drm/i915/i915_reset.c
> index 55d6123dbba4..4191398b5125 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_ringbuffer(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 3feb0f74c239..a8e47cfa6e35 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -826,6 +826,34 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs
> *engine)
> _MASKED_BIT_DISABLE(STOP_RING));
> }
>
> +int intel_engine_stop_ringbuffer(struct intel_engine_cs *engine)
I think intel_engine_stop_ring() works better, since it is the set of
RING registers we are controlling and are not concerned here with the
backing buffer.
> +{
> + 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 5363dad1208d..8936a9051760 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -622,15 +622,9 @@ static bool stop_ring(struct intel_engine_cs *engine)
> return false;
> }
>
> - I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));
> + ret = intel_engine_stop_ringbuffer(engine);
Maybe
if (intel_engine_stop_ring(engine))
return false;
return false;
looks tidier?
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Introduce intel_engine_stop_ringbuffer
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 > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
