[igt-dev] [PATCH igt] igt: Add gem_ctx_freq to exercise requesting freq on a ctx

[Chris Wilson]

Quoting Antonio Argenziano (2018-03-09 00:45:42)
> 
> 
> On 08/03/18 09:13, Chris Wilson wrote:
> > Exercise some new API that allows applications to request that
> > individual contexts are executed within a desired frequency range.
> > 
> > v2: Split single/continuous set_freq subtests
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Paneri, Praveen <praveen.paneri at intel.com>
> > Cc: Kamble, Sagar A <sagar.a.kamble at intel.com>
> > Cc: Antonio Argenziano <antonio.argenziano at intel.com>
> > ---
> >   tests/Makefile.am      |   1 +
> >   tests/Makefile.sources |   1 +
> >   tests/gem_ctx_freq.c   | 604 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   tests/meson.build      |   1 +
> >   4 files changed, 607 insertions(+)
> >   create mode 100644 tests/gem_ctx_freq.c
> > 
> 
> > +static void single(int fd, const struct intel_execution_engine *e)
> > +{
> > +     const unsigned int engine = e->exec_id | e->flags;
> > +     uint32_t ctx = gem_context_create(fd);
> > +     uint32_t min, max;
> > +     double measured;
> > +     igt_spin_t *spin;
> > +     int pmu;
> > +
> > +     get_freq(fd, ctx, &min, &max);
> > +     igt_info("Min freq: %dMHz; Max freq: %dMHz\n", min, max);
> > +
> > +     pmu = perf_i915_open(I915_PMU_REQUESTED_FREQUENCY);
> > +     igt_require(pmu >= 0);
> > +
> > +     for (uint32_t freq = min + 50; freq <= max; freq += 100) {
> 
> Although it is done in the smoke test, it would be interesting if freq's 
> values were a bit randomized.

It was intentionally increment only originally because the
implementation was fast to upclock and slow to downclock. Given the
challenge of the smoketest, I've shelved that laziness until it can pass
that not particularly challenging test.

I did have in mind doing an up/down ramp. We should also check that
changing min while keeping max fixed should not affect a 100% load like
the spinners.
 
> > +             uint32_t cur, discard;
> > +
> > +             set_freq(fd, ctx, freq, freq);
> > +             get_freq(fd, ctx, &cur, &discard);
> 
> igt_assert_eq(freq, cur)?

Not quite. The trick is that the interface is not strictly idempotent,
since we pass in MHz, the driver converts that into freq bins and spits
it back out to the nearest MHz. So cur is not strictly freq, it just
happens that 50MHz is the bin size on gen9.

The idea here is that we grab the adjusted freq from the driver to
validate with.

> > +
> > +             gem_quiescent_gpu(fd);
> > +             spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> > +             usleep(10000);
> 
> I guess here we wait for the frequency changes to take effect, maybe a 
> small comment would help.

And for the batch to be submitted. Asynchronous interfaces make it hard
to pinpoint when things occur. We keep muttering about extending
igt_spin_t to have an option for wait-for-exec (basically have it write
out a dword that we can spin on)

usleep(10000); /* wait for execution + freq change */

> > +static void sandwich(int fd)
> > +{
> > +     uint32_t ctx = gem_context_create(fd);
> > +     unsigned int engine;
> > +     uint32_t min, max;
> > +     igt_spin_t *spin;
> > +     int pmu;
> > +
> > +     pmu = perf_i915_open(I915_PMU_REQUESTED_FREQUENCY);
> > +     igt_require(pmu >= 0);
> > +
> > +     spin = igt_spin_batch_new(fd, ctx, 0, 0);
> > +     get_freq(fd, ctx, &min, &max);
> > +     set_freq(fd, ctx, min, min);
> > +     for_each_physical_engine(fd, engine) {
> > +             struct drm_i915_gem_exec_object2 obj = {
> > +                     .handle = spin->handle,
> > +             };
> > +             struct drm_i915_gem_execbuffer2 eb = {
> > +                     .buffer_count = 1,
> > +                     .buffers_ptr = to_user_pointer(&obj),
> > +                     .flags = engine,
> > +                     .rsvd1 = ctx,
> > +             };
> > +             uint32_t cur, discard;
> > +             double measured;
> > +
> > +             min += 50;
> > +             if (min > max)
> > +                     break;
> > +
> > +             set_freq(fd, ctx, min, min);
> > +             get_freq(fd, ctx, &cur, &discard);
> > +
> > +             gem_execbuf(fd, &eb);
> > +             usleep(10000);
> > +
> > +             measured = measure_frequency(pmu, SAMPLE_PERIOD);
> > +             igt_debugfs_dump(fd, "i915_rps_boost_info");
> > +
> > +             igt_info("Measured %.1fMHz, expected %dMhz\n", measured, cur);
> > +             igt_assert(measured > cur - 100 && measured < cur + 100);
> 
> Does the frequency change after each execbuf?

Yes. This turned into just an exercise across engines, bumping the
desired freq. I wanted to try and validate the min/max bounds, but ran
into the challenge that the gpu is only running at the highest frequency
it can get because of the busy spinner :)

> > +static void smoketest(int fd, int timeout)
> > +{
> > +     unsigned int engines[16];
> 
> use a macro instead of magic number 16.

#define THIS_IS_FAR_MORE_MAGIC_THAN_A_MEANINGLESS_BARE_NUMBER_THAT_YOU_SHOULD_NOT_BE_READING_ANYTHING_INTO 16
/rant

> > +static void invalid_param(int fd)
> > +{
> 
> gem_ctx_param is going to be upset again pretty soon ;).

Poor thing.
-Chris