Re: [PATCH] drm/i915: Abandon the reset if we fail to stop the engines

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 26, 2017 at 01:12:12PM +0100, Chris Wilson wrote:
> Some machines, *cough* snb *cough*, fail catastrophically if asked to
> reset the GPU under certain conditions.

Did we try skipping the gen6_rps_disable() already?

> The initial guess is that this
> is when the rings are still busy at the time of the reset request
> (because that's a pattern we've seen elsewhere, hence why we do try
> gen3_stop_engines() before reset) so abandon the reset and leave the
> device wedged, if gen3_stop_engines() fails.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103240
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> Whee! Let's see how much breaks!
> -Chris
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 20e3c65c0999..c9a254b6125f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1372,20 +1372,23 @@ int i915_reg_read_ioctl(struct drm_device *dev,
>  	return ret;
>  }
>  
> -static void gen3_stop_engine(struct intel_engine_cs *engine)
> +static bool gen3_stop_engine(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *dev_priv = engine->i915;
>  	const u32 base = engine->mmio_base;
>  	const i915_reg_t mode = RING_MI_MODE(base);
>  
> +
>  	I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING));
>  	if (intel_wait_for_register_fw(dev_priv,
>  				       mode,
>  				       MODE_IDLE,
>  				       MODE_IDLE,
> -				       500))
> +				       500)) {
>  		DRM_DEBUG_DRIVER("%s: timed out on STOP_RING\n",
>  				 engine->name);
> +		return false;
> +	}
>  
>  	I915_WRITE_FW(RING_CTL(base), 0);
>  	I915_WRITE_FW(RING_HEAD(base), 0);
> @@ -1395,19 +1398,32 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
>  	if (I915_READ_FW(RING_HEAD(base)) != 0)
>  		DRM_DEBUG_DRIVER("%s: ring head not parked\n",
>  				 engine->name);
> +
> +	return true;
>  }
>  
> -static void i915_stop_engines(struct drm_i915_private *dev_priv,
> -			      unsigned engine_mask)
> +static int i915_stop_engines(struct drm_i915_private *dev_priv,
> +			     unsigned engine_mask)
>  {
>  	struct intel_engine_cs *engine;
>  	enum intel_engine_id id;
> +	bool idle;
>  
>  	if (INTEL_GEN(dev_priv) < 3)
> -		return;
> +		return true;
>  
> +	idle = true;
>  	for_each_engine_masked(engine, dev_priv, engine_mask, id)
> -		gen3_stop_engine(engine);
> +		idle &= gen3_stop_engine(engine);
> +	if (idle)
> +		return idle;
> +
> +	dev_err(dev_priv->drm.dev, "Failed to stop all engines\n");
> +	for_each_engine_masked(engine, dev_priv, engine_mask, id) {
> +		struct drm_printer p = drm_debug_printer(__func__);
> +		intel_engine_dump(engine, &p);
> +	}
> +	return false;
>  }
>  
>  static bool i915_reset_complete(struct pci_dev *pdev)
> @@ -1768,7 +1784,10 @@ int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned engine_mask)
>  		 *
>  		 * FIXME: Wa for more modern gens needs to be validated
>  		 */
> -		i915_stop_engines(dev_priv, engine_mask);
> +		if (!i915_stop_engines(dev_priv, engine_mask)) {
> +			ret = -EIO;
> +			break;
> +		}
>  
>  		ret = -ENODEV;
>  		if (reset)
> -- 
> 2.15.0.rc2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx