[Intel-gfx] [PATCH v7 06/11] drm/i915: Enable i915 perf stream for Haswell OA unit

[Matthew Auld]

On 26 October 2016 at 00:51, Robert Bragg <robert at sixbynine.org> wrote:
>
>
> On Tue, Oct 25, 2016 at 10:35 PM, Matthew Auld
> <matthew.william.auld at gmail.com> wrote:
>>
>> On 25 October 2016 at 00:19, Robert Bragg <robert at sixbynine.org> wrote:
>
>
>>
>>
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index 3448d05..ea24814 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1764,6 +1764,11 @@ struct intel_wm_config {
>>
>> >
>> >  struct drm_i915_private {
>> > @@ -2149,16 +2164,46 @@ struct drm_i915_private {
>> >
>> >         struct {
>> >                 bool initialized;
>> > +
>> >                 struct mutex lock;
>> >                 struct list_head streams;
>> >
>> > +               spinlock_t hook_lock;
>> > +
>> >                 struct {
>> > -                       u32 metrics_set;
>> > +                       struct i915_perf_stream *exclusive_stream;
>> > +
>> > +                       u32 specific_ctx_id;
>> Can we just get rid of this, now that the vma remains pinned we can
>> simply get the ggtt address at the time of configuring the OA_CONTROL
>> register ?
>
>
> I considered that, but would ideally prefer to keep it considering the gen8+
> patches to come. For gen8+ (with execlists) the context ID isn't a gtt
> offset.
>
>>
>>
>> > +
>> > +                       struct hrtimer poll_check_timer;
>> > +                       wait_queue_head_t poll_wq;
>> > +                       atomic_t pollin;
>> > +
>>
>
>>
>> > +/* The maximum exponent the hardware accepts is 63 (essentially it
>> > selects one
>> > + * of the 64bit timestamp bits to trigger reports from) but there's
>> > currently
>> > + * no known use case for sampling as infrequently as once per 47
>> > thousand years.
>> > + *
>> > + * Since the timestamps included in OA reports are only 32bits it seems
>> > + * reasonable to limit the OA exponent where it's still possible to
>> > account for
>> > + * overflow in OA report timestamps.
>> > + */
>> > +#define OA_EXPONENT_MAX 31
>> > +
>> > +#define INVALID_CTX_ID 0xffffffff
>> We shouldn't need this anymore.
>
>
> yeah I removed it and then added it back, just for the sake of explicitly
> setting the specific_ctx_id to an invalid ID when closing the exclusive
> stream - though resetting the value isn't strictly necessary.
Can we not make the specific_ctx_id per-stream, the gem context
already is, then we don't need to be concerned with resetting it ?