Re: [PATCH 06/32] drm/i915: Tighten reset_counter for reset status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 11, 2015 at 11:33:02AM +0000, Chris Wilson wrote:
> In the reset_counter, we use two bits to track a GPU hang and reset. The
> low bit is a "reset-in-progress" flag that we set to signal when we need
> to break waiters in order for the recovery task to grab the mutex. As
> soon as the recovery task has the mutex, we can clear that flag (which
> we do by incrementing the reset_counter thereby incrementing the gobal
> reset epoch). By clearing that flag when the recovery task holds the
> struct_mutex, we can forgo a second flag that simply tells GEM to ignore
> the "reset-in-progress" flag.
> 
> The second flag we store in the reset_counter is whether the
> reset failed and we consider the GPU terminally wedged. Whilst this flag
> is set, all access to the GPU (at least through GEM rather than direct mmio
> access) is verboten.
> 
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
>  drivers/gpu/drm/i915/i915_drv.c     | 39 ++++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/i915_drv.h     |  3 ---
>  drivers/gpu/drm/i915/i915_gem.c     | 27 +++++++++----------------
>  drivers/gpu/drm/i915/i915_irq.c     | 21 ++------------------
>  5 files changed, 36 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c26a4c087f49..d5f66bbdb160 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4672,7 +4672,7 @@ i915_wedged_get(void *data, u64 *val)
>  	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	*val = i915_reset_counter(&dev_priv->gpu_error);
> +	*val = i915_terminally_wedged(&dev_priv->gpu_error);
>  
>  	return 0;
>  }
> @@ -4691,7 +4691,7 @@ i915_wedged_set(void *data, u64 val)
>  	 * while it is writing to 'i915_wedged'
>  	 */
>  
> -	if (i915_reset_in_progress_or_wedged(&dev_priv->gpu_error))
> +	if (i915_reset_in_progress(&dev_priv->gpu_error))
>  		return -EAGAIN;
>  
>  	intel_runtime_pm_get(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8ddfcce92cf1..8bdc51bc00a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -858,23 +858,32 @@ int i915_resume_switcheroo(struct drm_device *dev)
>  int i915_reset(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	bool simulated;
> +	struct i915_gpu_error *error = &dev_priv->gpu_error;
> +	unsigned reset_counter;
>  	int ret;
>  
>  	intel_reset_gt_powersave(dev);
>  
>  	mutex_lock(&dev->struct_mutex);
>  
> -	i915_gem_reset(dev);
> +	/* Clear any previous failed attempts at recovery. Time to try again. */
> +	atomic_andnot(I915_WEDGED, &error->reset_counter);
>  
> -	simulated = dev_priv->gpu_error.stop_rings != 0;
> +	/* Clear the reset-in-progress flag and increment the reset epoch. */
> +	reset_counter = atomic_inc_return(&error->reset_counter);
> +	if (WARN_ON(__i915_reset_in_progress(reset_counter))) {
> +		ret = -EIO;
> +		goto error;
> +	}
> +
> +	i915_gem_reset(dev);
>  
>  	ret = intel_gpu_reset(dev);
>  
>  	/* Also reset the gpu hangman. */
> -	if (simulated) {
> +	if (error->stop_rings != 0) {
>  		DRM_INFO("Simulated gpu hang, resetting stop_rings\n");
> -		dev_priv->gpu_error.stop_rings = 0;
> +		error->stop_rings = 0;
>  		if (ret == -ENODEV) {
>  			DRM_INFO("Reset not implemented, but ignoring "
>  				 "error for simulated gpu hangs\n");
> @@ -887,8 +896,7 @@ int i915_reset(struct drm_device *dev)
>  
>  	if (ret) {
>  		DRM_ERROR("Failed to reset chip: %i\n", ret);
> -		mutex_unlock(&dev->struct_mutex);
> -		return ret;
> +		goto error;
>  	}
>  
>  	intel_overlay_reset(dev_priv);
> @@ -907,20 +915,14 @@ int i915_reset(struct drm_device *dev)
>  	 * was running at the time of the reset (i.e. we weren't VT
>  	 * switched away).
>  	 */
> -
> -	/* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */
> -	dev_priv->gpu_error.reload_in_reset = true;
> -
>  	ret = i915_gem_init_hw(dev);
> -
> -	dev_priv->gpu_error.reload_in_reset = false;
> -
> -	mutex_unlock(&dev->struct_mutex);
>  	if (ret) {
>  		DRM_ERROR("Failed hw init on reset %d\n", ret);
> -		return ret;
> +		goto error;
>  	}
>  
> +	mutex_unlock(&dev->struct_mutex);
> +
>  	/*
>  	 * rps/rc6 re-init is necessary to restore state lost after the
>  	 * reset and the re-install of gt irqs. Skip for ironlake per
> @@ -931,6 +933,11 @@ int i915_reset(struct drm_device *dev)
>  		intel_enable_gt_powersave(dev);
>  
>  	return 0;
> +
> +error:
> +	atomic_or(I915_WEDGED, &error->reset_counter);
> +	mutex_unlock(&dev->struct_mutex);
> +	return ret;
>  }
>  
>  static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 466caa0bc043..1043ddd670a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1384,9 +1384,6 @@ struct i915_gpu_error {
>  
>  	/* For missed irq/seqno simulation. */
>  	unsigned int test_irq_rings;
> -
> -	/* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset   */
> -	bool reload_in_reset;
>  };
>  
>  enum modeset_restore {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0b3e0534baa3..27e617b76418 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -85,9 +85,7 @@ i915_gem_wait_for_error(struct i915_gpu_error *error)
>  {
>  	int ret;
>  
> -#define EXIT_COND (!i915_reset_in_progress_or_wedged(error) || \
> -		   i915_terminally_wedged(error))
> -	if (EXIT_COND)
> +	if (!i915_reset_in_progress(error))
>  		return 0;
>  
>  	/*
> @@ -96,17 +94,16 @@ i915_gem_wait_for_error(struct i915_gpu_error *error)
>  	 * we should simply try to bail out and fail as gracefully as possible.
>  	 */
>  	ret = wait_event_interruptible_timeout(error->reset_queue,
> -					       EXIT_COND,
> +					       !i915_reset_in_progress(error),
>  					       10*HZ);
>  	if (ret == 0) {
>  		DRM_ERROR("Timed out waiting for the gpu reset to complete\n");
>  		return -EIO;
>  	} else if (ret < 0) {
>  		return ret;
> +	} else {
> +		return 0;
>  	}
> -#undef EXIT_COND
> -
> -	return 0;
>  }
>  
>  int i915_mutex_lock_interruptible(struct drm_device *dev)
> @@ -1114,22 +1111,16 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
>  		     bool interruptible)
>  {
>  	if (i915_reset_in_progress_or_wedged(error)) {
> +		/* Recovery complete, but the reset failed ... */
> +		if (i915_terminally_wedged(error))
> +			return -EIO;
> +
>  		/* Non-interruptible callers can't handle -EAGAIN, hence return
>  		 * -EIO unconditionally for these. */
>  		if (!interruptible)
>  			return -EIO;
>  
> -		/* Recovery complete, but the reset failed ... */
> -		if (i915_terminally_wedged(error))
> -			return -EIO;
> -
> -		/*
> -		 * Check if GPU Reset is in progress - we need intel_ring_begin
> -		 * to work properly to reinit the hw state while the gpu is
> -		 * still marked as reset-in-progress. Handle this with a flag.
> -		 */
> -		if (!error->reload_in_reset)
> -			return -EAGAIN;
> +		return -EAGAIN;
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 60dff3f89531..88206c0404d7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2434,7 +2434,6 @@ static void i915_error_wake_up(struct drm_i915_private *dev_priv,
>  static void i915_reset_and_wakeup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct i915_gpu_error *error = &dev_priv->gpu_error;
>  	char *error_event[] = { I915_ERROR_UEVENT "=1", NULL };
>  	char *reset_event[] = { I915_RESET_UEVENT "=1", NULL };
>  	char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL };
> @@ -2452,7 +2451,7 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
>  	 * the reset in-progress bit is only ever set by code outside of this
>  	 * work we don't need to worry about any other races.
>  	 */
> -	if (i915_reset_in_progress_or_wedged(error) && !i915_terminally_wedged(error)) {
> +	if (i915_reset_in_progress(&dev_priv->gpu_error)) {
>  		DRM_DEBUG_DRIVER("resetting chip\n");
>  		kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE,
>  				   reset_event);
> @@ -2480,25 +2479,9 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
>  
>  		intel_runtime_pm_put(dev_priv);
>  
> -		if (ret == 0) {
> -			/*
> -			 * After all the gem state is reset, increment the reset
> -			 * counter and wake up everyone waiting for the reset to
> -			 * complete.
> -			 *
> -			 * Since unlock operations are a one-sided barrier only,
> -			 * we need to insert a barrier here to order any seqno
> -			 * updates before
> -			 * the counter increment.
> -			 */
> -			smp_mb__before_atomic();
> -			atomic_inc(&dev_priv->gpu_error.reset_counter);
> -
> +		if (ret == 0)
>  			kobject_uevent_env(&dev->primary->kdev->kobj,
>  					   KOBJ_CHANGE, reset_done_event);
> -		} else {
> -			atomic_or(I915_WEDGED, &error->reset_counter);
> -		}
>  
>  		/*
>  		 * Note: The wake_up also serves as a memory barrier so that
> -- 
> 2.6.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux