Re: [PATCH 2/2] drm/i915: add i915_get_reset_stats_ioctl

[Ian Romanick <idr@freedesktop.org>]

On 11/08/2013 10:11 AM, Damien Lespiau wrote:
> On Wed, Oct 30, 2013 at 03:44:16PM +0200, Mika Kuoppala wrote:
>> This ioctl returns reset stats for specified context.
>>
>> The struct returned contains context loss counters.
>>
>> reset_count:    all resets across all contexts
>> batch_active:   active batches lost on resets
>> batch_pending:  pending batches lost on resets
>>
>> v2: get rid of state tracking completely and deliver only counts. Idea
>>     from Chris Wilson.
>>
>> v3: fix commit message
>>
>> v4: default context handled inside i915_gem_context_get_hang_stats
>>
>> v5: reset_count only for priviledged process
>>
>> v6: ctx=0 needs CAP_SYS_ADMIN for batch_* counters (Chris Wilson)
>>
>> v7: context hang stats never returns NULL
>>
>> v8: rebased on top of reworked context hang stats
>>     DRM_RENDER_ALLOW for ioctl
>>
>> v9: use DEFAULT_CONTEXT_ID. Improve comments for ioctl struct members
>>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: Ian Romanick <idr@freedesktop.org>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> The logic there looks good to me, I was just wondering if we wanted a
> bit more padding in case we want other counters or misc stuff.

I think the current padding should be sufficient.  The other things that
could occur aren't things that we should return counts of.  For those
kinds of things, we'd only want to return flags.  In fact, the current
user space only uses the counts as flags anyway (is the count non-zero?).

> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Also,

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>