Re: [PATCH 4/6] drm/i915: Allow the module to load even if we fail to setup rings

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed,  2 Apr 2014 16:36:09 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Even without enabling the ringbuffers to allow command execution, we can
> still control the display engines to enable modesetting. So make the
> ringbuffer initialization failure soft, and mark the GPU as wedged
> instead.
> 
> v2: Only treat an EIO from ring initialisation as a soft failure, and
> abort module load for any other failure, such as allocation failures.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0d5f8fb15677..850ec98239f4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4450,15 +4450,11 @@ i915_gem_init_hw(struct drm_device *dev)
>  	 * the do_switch), but before enabling PPGTT. So don't move this.
>  	 */
>  	ret = i915_gem_context_enable(dev_priv);
> -	if (ret) {
> +	if (ret && ret != -EIO) {
>  		DRM_ERROR("Context enable failed %d\n", ret);
> -		goto err_out;
> +		i915_gem_cleanup_ringbuffer(dev);
>  	}
>  
> -	return 0;
> -
> -err_out:
> -	i915_gem_cleanup_ringbuffer(dev);
>  	return ret;
>  }
>  
> @@ -4485,18 +4481,20 @@ int i915_gem_init(struct drm_device *dev)
>  	}
>  
>  	ret = i915_gem_init_hw(dev);
> -	mutex_unlock(&dev->struct_mutex);
> -	if (ret) {
> -		WARN_ON(dev_priv->mm.aliasing_ppgtt);
> -		i915_gem_context_fini(dev);
> -		drm_mm_takedown(&dev_priv->gtt.base.mm);
> -		return ret;
> +	if (ret == -EIO) {
> +		/* Allow ring initialisation to fail by marking the GPU as
> +		 * wedged. But we only want to do this where the GPU is angry,
> +		 * for all other failure, such as an allocation failure, bail.
> +		 */
> +		atomic_set_mask(I915_WEDGED, &dev_priv->gpu_error.reset_counter);
> +		ret = 0;
>  	}
> +	mutex_unlock(&dev->struct_mutex);
>  
>  	/* Allow hardware batchbuffers unless told otherwise, but not for KMS. */
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		dev_priv->dri1.allow_batchbuffer = 1;
> -	return 0;
> +	return ret;
>  }
>  
>  void

Will we still have some loud spewing into the log in this case?  If so,
then
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center