| Message ID | 20190227165850.17277-2-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Use intel_engine_stop_cs when stopping ringbuffer | expand |
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 <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 | 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@chris-wilson.co.uk> -Chris
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);
We use identical sequence of stopping ringbuffer on reset handing and on ring initialization. Make a function to handle both cases. 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 | 10 ++------- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ 4 files changed, 33 insertions(+), 25 deletions(-)