Quoting Carlos Santa (2019-03-22 23:41:14)
> From: Michel Thierry <michel.thierry@xxxxxxxxx>
>
> Users/tests relying on the total reset count will start seeing a smaller
> number since most of the hangs can be handled by engine reset.
> Note that if reset engine x, context a running on engine y will be unaware
> and unaffected.
>
> To start the discussion, include just a total engine reset count. If it
> is deemed useful, it can be extended to report each engine separately.
>
> Our igt's gem_reset_stats test will need changes to ignore the pad field,
> since it can now return reset_engine_count.
>
> v2: s/engine_reset/reset_engine/, use union in uapi to not break compatibility.
> v3: Keep rejecting attempts to use pad as input (Antonio)
> v4: Rebased.
> v5: Rebased.
> Get rid of the union to store pad/engine count (Chris)
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Antonio Argenziano <antonio.argenziano@xxxxxxxxx>
> Cc: Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
> Signed-off-by: Carlos Santa <carlos.santa@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 12 ++++++++++--
> include/uapi/drm/i915_drm.h | 4 ++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 21208a865380..9625b5f7faf7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -1350,6 +1350,8 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_i915_reset_stats *args = data;
> struct i915_gem_context *ctx;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> int ret;
>
> if (args->flags || args->pad)
> @@ -1368,10 +1370,16 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
> * we should wrap the hangstats with a seqlock.
> */
>
> - if (capable(CAP_SYS_ADMIN))
> + if (capable(CAP_SYS_ADMIN)) {
> args->reset_count = i915_reset_count(&dev_priv->gpu_error);
> - else
> + for_each_engine(engine, dev_priv, id)
> + args->reset_engine_count +=
> + i915_reset_engine_count(&dev_priv->gpu_error,
> + engine);
Do we really care about device-vs-engine here? Amalgamating all engine
resets into one variable is barely any more information than
amalgamating them with the device count.
> + } else {
> args->reset_count = 0;
> + args->reset_engine_count = 0;
> + }
>
> args->batch_active = atomic_read(&ctx->guilty_count);
> args->batch_pending = atomic_read(&ctx->active_count);
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index aa2d4c73a97d..5e7bc6412880 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1459,6 +1459,9 @@ struct drm_i915_reset_stats {
> /* All resets since boot/module reload, for all contexts */
> __u32 reset_count;
>
> + /* Engine resets since boot/module reload, for all contexts */
> + __u32 reset_engine_count;
You cannot insert elements into the middle of an uABI struct. You can
only extend. To avoid problems with sw layout, always introduce a new
struct.
> +
> /* Number of batches lost when active in GPU, for this context */
> __u32 batch_active;
>
> @@ -1466,6 +1469,7 @@ struct drm_i915_reset_stats {
> __u32 batch_pending;
>
> __u32 pad;
> +
> };
>
> struct drm_i915_gem_userptr {
> --
> 2.17.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
