Re: [PATCH 039/190] drm/i915: Remove stop-rings debugfs interface

[Arun Siluvery <arun.siluvery@linux.intel.com>]

On 11/01/2016 09:16, Chris Wilson wrote:
> Now that we have (near) universal GPU recovery code, we can inject a
> real hang from userspace and not need any fakery. Not only does this
> mean that the testing is far more realistic, but we can simplify the
> kernel in the process.
>
> v2: Replace the i915_stop_rings with a dummy implementation as igt
> encodified its existence until we can release an update.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c     | 19 +------------------
>   drivers/gpu/drm/i915/i915_drv.c         | 17 ++---------------
>   drivers/gpu/drm/i915/i915_drv.h         | 19 -------------------
>   drivers/gpu/drm/i915/i915_gem.c         | 13 +++----------
>   drivers/gpu/drm/i915/intel_lrc.c        |  5 -----
>   drivers/gpu/drm/i915/intel_ringbuffer.c |  8 --------
>   drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
>   7 files changed, 6 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 567f8db4c70a..6172649b7e56 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4752,30 +4752,13 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
>   static int
>   i915_ring_stop_get(void *data, u64 *val)
>   {
> -	struct drm_device *dev = data;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	*val = dev_priv->gpu_error.stop_rings;
> -
> +	*val = 0;
>   	return 0;
>   }
>
>   static int
>   i915_ring_stop_set(void *data, u64 val)
>   {
> -	struct drm_device *dev = data;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int ret;
> -
> -	DRM_DEBUG_DRIVER("Stopping rings 0x%08llx\n", val);
> -
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		return ret;
> -
> -	dev_priv->gpu_error.stop_rings = val;
> -	mutex_unlock(&dev->struct_mutex);
> -
>   	return 0;
>   }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 442e1217e442..e9f85fd0542f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -891,24 +891,11 @@ int i915_reset(struct drm_device *dev)
>   		goto error;
>   	}
>
> +	pr_notice("drm/i915: Resetting chip after gpu hang\n");
> +
>   	i915_gem_reset(dev);
>
>   	ret = intel_gpu_reset(dev);
> -
> -	/* Also reset the gpu hangman. */
> -	if (error->stop_rings != 0) {
> -		DRM_INFO("Simulated gpu hang, resetting stop_rings\n");
> -		error->stop_rings = 0;
> -		if (ret == -ENODEV) {
> -			DRM_INFO("Reset not implemented, but ignoring "
> -				 "error for simulated gpu hangs\n");
> -			ret = 0;
> -		}
> -	}
> -
> -	if (i915_stop_ring_allow_warn(dev_priv))
> -		pr_notice("drm/i915: Resetting chip after gpu hang\n");
> -
>   	if (ret) {
>   		if (ret != -ENODEV)
>   			DRM_ERROR("Failed to reset chip: %i\n", ret);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ec6f3e9e74d..c3b795f1566b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1371,13 +1371,6 @@ struct i915_gpu_error {
>   	 */
>   	wait_queue_head_t reset_queue;
>
> -	/* Userspace knobs for gpu hang simulation;
> -	 * combines both a ring mask, and extra flags
> -	 */
> -	u32 stop_rings;
> -#define I915_STOP_RING_ALLOW_BAN       (1 << 31)
> -#define I915_STOP_RING_ALLOW_WARN      (1 << 30)
> -
>   	/* For missed irq/seqno simulation. */
>   	unsigned long test_irq_rings;
>   };
> @@ -3030,18 +3023,6 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error)
>   	return ((i915_reset_counter(error) & ~I915_WEDGED) + 1) / 2;
>   }
>
> -static inline bool i915_stop_ring_allow_ban(struct drm_i915_private *dev_priv)
> -{
> -	return dev_priv->gpu_error.stop_rings == 0 ||
> -		dev_priv->gpu_error.stop_rings & I915_STOP_RING_ALLOW_BAN;
> -}
> -
> -static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv)
> -{
> -	return dev_priv->gpu_error.stop_rings == 0 ||
> -		dev_priv->gpu_error.stop_rings & I915_STOP_RING_ALLOW_WARN;
> -}
> -
>   void i915_gem_reset(struct drm_device *dev);
>   bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
>   int __must_check i915_gem_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3948e85eaa48..ea9344503bf6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2633,21 +2633,14 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
>   {
>   	unsigned long elapsed;
>
> -	elapsed = get_seconds() - ctx->hang_stats.guilty_ts;
> -
>   	if (ctx->hang_stats.banned)
>   		return true;
>
> +	elapsed = get_seconds() - ctx->hang_stats.guilty_ts;
>   	if (ctx->hang_stats.ban_period_seconds &&
>   	    elapsed <= ctx->hang_stats.ban_period_seconds) {
> -		if (!i915_gem_context_is_default(ctx)) {
> -			DRM_DEBUG("context hanging too fast, banning!\n");
> -			return true;
> -		} else if (i915_stop_ring_allow_ban(dev_priv)) {
> -			if (i915_stop_ring_allow_warn(dev_priv))
> -				DRM_ERROR("gpu hanging too fast, banning!\n");
> -			return true;
> -		}
> +		DRM_DEBUG("context hanging too fast, banning!\n");
> +		return true;
>   	}
>
>   	return false;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index b1ede2e9b372..b634e7d7a92b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -756,16 +756,11 @@ static int logical_ring_wait_for_space(struct drm_i915_gem_request *req,
>   static void
>   intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
>   {
> -	struct intel_engine_cs *ring = request->ring;
>   	struct drm_i915_private *dev_priv = request->i915;
>
>   	intel_logical_ring_advance(request->ringbuf);
> -
>   	request->tail = request->ringbuf->tail;
>
> -	if (intel_ring_stopped(ring))
> -		return;
> -
>   	if (dev_priv->guc.execbuf_client)
>   		i915_guc_submit(dev_priv->guc.execbuf_client, request);
>   	else
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 5625f56a2db1..d9bb6458fa60 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -59,18 +59,10 @@ int intel_ring_space(struct intel_ringbuffer *ringbuf)
>   	return ringbuf->space;
>   }
>
> -bool intel_ring_stopped(struct intel_engine_cs *ring)
> -{
> -	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> -	return dev_priv->gpu_error.stop_rings & intel_ring_flag(ring);
> -}
> -
>   static void __intel_ring_advance(struct intel_engine_cs *ring)
>   {
>   	struct intel_ringbuffer *ringbuf = ring->buffer;
>   	ringbuf->tail &= ringbuf->size - 1;
> -	if (intel_ring_stopped(ring))
> -		return;
>   	ring->write_tail(ring, ringbuf->tail);
>   }
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 73da75fa47c1..eecf9c7ae2b8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -487,7 +487,6 @@ static inline void intel_ring_advance(struct intel_engine_cs *ring)
>   int __intel_ring_space(int head, int tail, int size);
>   void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
>   int intel_ring_space(struct intel_ringbuffer *ringbuf);
> -bool intel_ring_stopped(struct intel_engine_cs *ring);
>
>   int __must_check intel_ring_idle(struct intel_engine_cs *ring);
>   void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno);
>
one less thing to worry about, looks good to me,
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>

regards
Arun

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx