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

[Chris Wilson]

Quoting Sagar Arun Kamble (2018-03-14 08:15:15)
> 
> 
> On 3/13/2018 7:28 PM, 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
> > v3: Do an up/down ramp for individual freq request, check nothing
> > changes after each invalid request
> > v4: Check the frequencies reported by the kernel across the entire
> > range.
> > v5: Rewrite sandwich to create a sandwich between multiple concurrent
> > engines.
> > v6: Exercise sysfs overrides.
> > v7: Reset min/max of default context after independent(); don't ask
> > about failure
> > v8: Check transition beyond randomly chosen frequencies as well as
> > up/down ramps.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Praveen Paneri <praveen.paneri at intel.com>
> > Cc: Sagar A Kamble <sagar.a.kamble at intel.com>
> > Cc: Antonio Argenziano <antonio.argenziano at intel.com>
> > Reviewed-by: Antonio Argenziano <antonio.argenziano at intel.com> #v5
> There are few stray whitespaces in __pmu_within_tolerance, pmu_assert.
> Otherwise looks good to me.
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> 
> Can you please clarify few things below:
> > ---
> <snip>
> > +
> > +static void sysfs_clamp(int fd, const struct intel_execution_engine *e)
> > +{
> > +#define N_STEPS 10
> > +     const unsigned int engine = e->exec_id | e->flags;
> > +     uint32_t ctx = gem_context_create(fd);
> > +     uint32_t sys_min, sys_max;
> > +     uint32_t min, max;
> > +     double measured;
> > +     igt_spin_t *spin;
> > +     int pmu;
> > +
> > +     get_sysfs_freq(&sys_min, &sys_max);
> > +     igt_info("System min freq: %dMHz; max freq: %dMHz\n", sys_min, sys_max);
> > +
> > +     get_freq(fd, ctx, &min, &max);
> > +     igt_info("Context min freq: %dMHz; max freq: %dMHz\n", min, max);
> > +
> > +     pmu = perf_i915_open(I915_PMU_REQUESTED_FREQUENCY);
> > +     igt_require(pmu >= 0);
> > +
> > +     for (int outer = 0; outer <= 2*N_STEPS; outer++) {
> > +             int ofrac = outer > N_STEPS ? 2*N_STEPS - outer : outer;
> > +             uint32_t ofreq = min + (max - min) * ofrac / N_STEPS;
> > +             uint32_t cur, discard;
> > +
> > +             for (int inner = 0; inner <= 2*N_STEPS; inner++) {
> > +                     int ifrac = inner > N_STEPS ? 2*N_STEPS - inner : inner;
> > +                     uint32_t ifreq = min + (max - min) * ifrac / N_STEPS;
> > +
> > +                     set_freq(fd, ctx, ifreq, ifreq);
> > +
> > +                     gem_quiescent_gpu(fd);
> > +                     spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> > +                     usleep(10000);
> > +
> > +                     set_sysfs_freq(ofreq, ofreq);
> > +                     get_sysfs_freq(&cur, &discard);
> We don't sleep here because we know that we set the frequency in sysfs?

sysfs is a synchronous interface, yes.

> > +
> > +                     measured = measure_frequency(pmu, SAMPLE_PERIOD);
> > +                     igt_debugfs_dump(fd, "i915_rps_boost_info");
> > +
> > +                     set_sysfs_freq(sys_min, sys_max);
> > +
> > +                     igt_spin_batch_free(fd, spin);
> > +                     igt_info("%s(sysfs): Measured %.1fMHz, context %dMhz, expected %dMhz\n",
> > +                                     e->name, measured, ifreq, cur);
> > +                     pmu_assert(measured, cur);
> > +             }
> > +     }
> > +     gem_quiescent_gpu(fd);
> > +
> > +     close(pmu);
> > +     gem_context_destroy(fd, ctx);
> > +
> > +#undef N_STEPS
> > +}
> > +
> ...
> > +static void disable_boost(int fd)
> > +{
> > +     char *value;
> > +
> > +     value = igt_sysfs_get(fd, "gt_RPn_freq_mhz");
> > +     igt_sysfs_set(fd, "gt_boost_freq_mhz", value);
> Why is this needed? kernel will not clamp boost freq as well within 
> ctx_freq_min/max?

Boosting is a separate mechanism than ctx->freq, as it is performed on
behalf of *another* client.

> Kernel disabling boost seems more effective than setting boost_freq to Rpn.

This is how we tell the kernel to disable boost, by setting it to a
value that never applies.

The tests try to avoid triggering boosts, but I felt it was sensible to
override the mechanism entirely. We still need various random sleeps
inside the tests in order to give the worker a chance to run, which is a
nuisance.
-Chris