[Intel-gfx] [PATCH] drm/i915/pmu: Do not assume fixed hrtimer period

[Tvrtko Ursulin]

On 04/06/2018 14:03, Chris Wilson wrote:
> Quoting Chris Wilson (2018-06-04 13:59:12)
>> Quoting Tvrtko Ursulin (2018-06-04 13:52:39)
>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>
>>> As Chris has discovered on his Ivybridge, and later automated test runs
>>> have confirmed, on most of our platforms hrtimer faced with heavy GPU load
>>> ca occasionally become sufficiently imprecise to affect PMU sampling
>>
>> s/ca/can/
>>
>>> calculations.
>>>
>>> This means we cannot assume sampling frequency is what we asked for, but
>>> we need to measure the interval ourselves.
>>>
>>> This patch is similar to Chris' original proposal for per-engine counters,
>>> but instead of introducing a new set to work around the problem with
>>> frequency sampling, it swaps around the way internal frequency accounting
>>> is done. Instead of accumulating current frequency and dividing by
>>> sampling frequency on readout, it accumulates frequency scaled by each
>>> period.
>>
>> My ulterior motive failed to win favour ;)
>>   
>>> Testcase: igt/perf_pmu/*busy* # snb, ivb, hsw
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> I should point out that even with this fix (or rather my patch), we can

I did not mean to steal it, just that you seemed very uncompromising on 
the new counters approach. If you prefer you can respin your patch with 
this approach for frequency counters, it is fine by me.

> still see errors of 25-30us, enough to fail the test. However, without
> the fix our errors can be orders of magnitude worse (e.g. reporting 80us
> busy instead of 500us).

Yeah I guess if the timer is delayed it is delayed and all samples just 
won't be there. I don't see a way to fix that elegantly. Even practically.

Regards,

Tvrtko