[Intel-gfx] [PATCH igt 2/2] igt/perf_pmu: Replace hard-coded sleep before rc6 with a probe

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-12-05 12:05:14)
> 
> On 05/12/2017 10:56, Chris Wilson wrote:
> > Instead of trying to sleep for 2 evaluations intervals and then assuming
> > that rc6 is working, poll the rc6 residency instead.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103929
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >   tests/perf_pmu.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> > index e872f4e55..65bc734da 100644
> > --- a/tests/perf_pmu.c
> > +++ b/tests/perf_pmu.c
> > @@ -1008,6 +1008,20 @@ static unsigned long rc6_enable_us(void)
> >       return 2 * 160 * 1000;
> >   }
> >   
> > +static bool wait_for_rc6(int fd)
> > +{
> > +     struct timespec tv = {};
> > +     uint64_t start, now;
> > +
> > +     start = pmu_read_single(fd);
> > +     do {
> > +             usleep(50);
> 
> Not needlessly fast?

Since we expect the EI to be 160us, I was erring on the side of
optimism. i.e. if the idling took long enough, rc6 would be close to
starting on our return. However, the code is trying to be safe just in
case, for whatever unknown reason, it takes longer. This pair of tests,
I am assuming are only concerned with the accuracy of the counter rather
than investigating kernel/device behaviour. (We should have some other
pm_rc6 tests that try to check that rc6 kicks off in a timely manner,
but we don't publish the expectations for rc6 behaviour. Not sure?)
 
> > +             now = pmu_read_single(fd);
> > +     } while (start == now && !igt_seconds_elapsed(&tv));
> 
> So up to one second wait.

Expectation is that it should only take 320us for it to kick off, so 1s
seems a reasonable upper bound. Maybe 2s?
 
> > +
> > +     return start != now;
> > +}
> > +
> >   static void
> >   test_rc6(int gem_fd)
> >   {
> > @@ -1019,7 +1033,7 @@ test_rc6(int gem_fd)
> >       fd = open_pmu(I915_PMU_RC6_RESIDENCY);
> >   
> >       gem_quiescent_gpu(gem_fd);
> > -     usleep(rc6_enable_us()); /* wait for the rc6 cycle counter to kick in */
> > +     igt_require(wait_for_rc6(fd));
> 
> Eliminate now unused rc6_enable_us() ?

I thought I did, my mistake.
-Chris