Re: [PATCH] drm/i915: Clear stop-engine for a pardoned reset

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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

> If we pardon a per-engine reset, we may leave the STOP_RING bit asserted
> in RING_MI_MODE resulting in the engine hanging. Unconditionally clear
> it on the per-engine exit path as we know that either we skipped the
> reset and so need the cancellation, or the reset was successful and the
> cancellation is a no-op, or there was an error and we will follow up
> with a full-reset or wedging (both of which will stop the engines again
> as required).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  1 +
>  drivers/gpu/drm/i915/intel_engine_cs.c  | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9dce55182c3a..41111f2a9c39 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2079,6 +2079,7 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
>  		goto out;
>  
>  out:
> +	intel_engine_cancel_stop_cs(engine);
>  	i915_gem_reset_finish_engine(engine);

Should we just lift the whole stop/start dance into
gem_reset_prepare|finish_engine()s?

-Mika

>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 99d5a24219c1..8628567d8f6e 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -788,6 +788,16 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
>  	return err;
>  }
>  
> +void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *dev_priv = engine->i915;
> +
> +	GEM_TRACE("%s\n", engine->name);
> +
> +	I915_WRITE_FW(RING_MI_MODE(engine->mmio_base),
> +		      _MASKED_BIT_DISABLE(STOP_RING));
> +}
> +
>  const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
>  {
>  	switch (type) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 9090885d57de..3f6920dd7880 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -906,6 +906,7 @@ int intel_init_blt_ring_buffer(struct intel_engine_cs *engine);
>  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);
>  
>  u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
>  u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine);
> -- 
> 2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx