diff mbox

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

Message ID 1396452971-25654-4-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 2, 2014, 3:36 p.m. UTC
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(-)

Comments

Jesse Barnes April 2, 2014, 10:11 p.m. UTC | #1
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>
Chris Wilson April 3, 2014, 6:42 a.m. UTC | #2
On Wed, Apr 02, 2014 at 03:11:40PM -0700, Jesse Barnes wrote:
> 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.

> Will we still have some loud spewing into the log in this case?  If so,

We have the usual *ERROR*, but we lack an indication that the GPU is now
wedged.
-Chris
diff mbox

Patch

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