On Fri, Dec 11, 2015 at 10:18:35PM +0000, Chris Wilson wrote:
> igt likes to inject GPU hangs into its command streams. However, as we
> expect these hangs, we don't actually want them recorded in the dmesg
> output or stored in the i915_error_state (usually). To accomodate this
> allow userspace to set a flag on the context that any hang emanating
> from that context will not be recorded. We still do the error capture
> (otherwise how do we find the guilty context and know its intent?) as
> part of the reason for random GPU hang injection is to exercise the race
> conditions between the error capture and normal execution.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>

Hm, I do like that we exercise the full paths all the time, increasing
chances for fireworks. What's the motivation here? Is there some
substantial speed-up?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  8 ++++++--
>  drivers/gpu/drm/i915/i915_gem_context.c | 13 ++++++++++++
>  drivers/gpu/drm/i915/i915_gpu_error.c   | 36 
> ++++++++++++++-------------------
>  include/uapi/drm/i915_drm.h             |  1 +
>  4 files changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b33091c2c39e..c511b3cbf9b2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -493,6 +493,7 @@ struct drm_i915_error_state {
>       struct timeval time;
>  
>       char error_msg[128];
> +     bool simulated;
>       int iommu;
>       u32 reset_count;
>       u32 suspend_count;
> @@ -845,7 +846,9 @@ struct i915_ctx_hang_stats {
>  /* This must match up with the value previously used for execbuf2.rsvd1. */
>  #define DEFAULT_CONTEXT_HANDLE 0
>  
> -#define CONTEXT_NO_ZEROMAP (1<<0)
> +#define CONTEXT_NO_ZEROMAP           (1<<0)
> +#define CONTEXT_NO_ERROR_CAPTURE     (1<<1)
> +
>  /**
>   * struct intel_context - as the name implies, represents a context.
>   * @ref: reference count.
> @@ -870,11 +873,12 @@ struct intel_context {
>       int user_handle;
>       uint8_t remap_slice;
>       struct drm_i915_private *i915;
> -     int flags;
>       struct drm_i915_file_private *file_priv;
>       struct i915_ctx_hang_stats hang_stats;
>       struct i915_hw_ppgtt *ppgtt;
>  
> +     unsigned flags;
> +
>       /* Legacy ring buffer submission */
>       struct {
>               struct drm_i915_gem_object *rcs_state;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 900ffd044db8..d9998ab9d94d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -938,6 +938,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
> *dev, void *data,
>               else
>                       args->value = to_i915(dev)->gtt.base.total;
>               break;
> +     case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
> +             args->value = !!(ctx->flags & CONTEXT_NO_ERROR_CAPTURE);
> +             break;
>       default:
>               ret = -EINVAL;
>               break;
> @@ -983,6 +986,16 @@ int i915_gem_context_setparam_ioctl(struct drm_device 
> *dev, void *data,
>                       ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
>               }
>               break;
> +     case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
> +             if (args->size) {
> +                     ret = -EINVAL;
> +             } else {
> +                     if (args->value)
> +                             ctx->flags |= CONTEXT_NO_ERROR_CAPTURE;
> +                     else
> +                             ctx->flags &= ~CONTEXT_NO_ERROR_CAPTURE;
> +             }
> +             break;
>       default:
>               ret = -EINVAL;
>               break;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 3e137fc701cf..cb0d6f347d42 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -995,7 +995,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
>  
>       for (i = 0; i < I915_NUM_RINGS; i++) {
>               struct intel_engine_cs *ring = &dev_priv->ring[i];
> -             struct intel_ringbuffer *rbuf;
> +             struct intel_ringbuffer *rbuf = NULL;
>  
>               error->ring[i].pid = -1;
>  
> @@ -1039,23 +1039,15 @@ static void i915_gem_record_rings(struct drm_device 
> *dev,
>                               }
>                               rcu_read_unlock();
>                       }
> -             }
>  
> -             if (i915.enable_execlists) {
> -                     /* TODO: This is only a small fix to keep basic error
> -                      * capture working, but we need to add more information
> -                      * for it to be useful (e.g. dump the context being
> -                      * executed).
> -                      */
> -                     if (request)
> -                             rbuf = request->ctx->engine[ring->id].ringbuf;
> -                     else
> -                             rbuf = 
> ring->default_context->engine[ring->id].ringbuf;
> -             } else
> -                     rbuf = ring->buffer;
> +                     error->simulated |= request->ctx->flags & 
> CONTEXT_NO_ERROR_CAPTURE;
> +                     rbuf = request->ringbuf;
> +             }
>  
> -             error->ring[i].cpu_ring_head = rbuf->head;
> -             error->ring[i].cpu_ring_tail = rbuf->tail;
> +             if (rbuf) {
> +                     error->ring[i].cpu_ring_head = rbuf->head;
> +                     error->ring[i].cpu_ring_tail = rbuf->tail;
> +             }
>  
>               error->ring[i].ringbuffer =
>                       i915_error_ggtt_object_create(dev_priv, rbuf->obj);
> @@ -1345,12 +1337,14 @@ void i915_capture_error_state(struct drm_device *dev, 
> bool wedged,
>       i915_error_capture_msg(dev, error, wedged, error_msg);
>       DRM_INFO("%s\n", error->error_msg);
>  
> -     spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
> -     if (dev_priv->gpu_error.first_error == NULL) {
> -             dev_priv->gpu_error.first_error = error;
> -             error = NULL;
> +     if (!error->simulated) {
> +             spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
> +             if (dev_priv->gpu_error.first_error == NULL) {
> +                     dev_priv->gpu_error.first_error = error;
> +                     error = NULL;
> +             }
> +             spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
>       }
> -     spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
>  
>       if (error) {
>               i915_error_state_free(&error->ref);
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index acf21026c78a..7fee4416dcc7 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1140,6 +1140,7 @@ struct drm_i915_gem_context_param {
>  #define I915_CONTEXT_PARAM_BAN_PERIOD        0x1
>  #define I915_CONTEXT_PARAM_NO_ZEROMAP        0x2
>  #define I915_CONTEXT_PARAM_GTT_SIZE  0x3
> +#define I915_CONTEXT_PARAM_NO_ERROR_CAPTURE  0x4
>       __u64 value;
>  };
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Reply via email to