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

[Antonio Argenziano]

On 07/03/18 17:18, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-03-08 00:55:47)
>>
>>
>> On 07/03/18 14:49, Chris Wilson wrote:
>>> +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);
>>> +
>>> +     gem_quiescent_gpu(fd);
>>> +     measured = measure_frequency(pmu, 10000);
>>> +     igt_info("Initial (idle) freq: %.1fMHz\n",measured);
>>> +     igt_require(measured >= min - 50 && measured <= min + 50);
>>> +
>>> +     for (uint32_t freq = min + 50; freq <= max; freq += 100) {
>>> +             set_freq(fd, ctx, freq, freq);
>>> +
>>> +             gem_quiescent_gpu(fd);
>>> +             spin = __igt_spin_batch_new(fd, ctx, engine, 0);
>>> +             usleep(10000);
>>> +
>>> +             measured = measure_frequency(pmu, 50000);
>>> +             igt_debugfs_dump(fd, "i915_rps_boost_info");
>>> +
>>> +             igt_spin_batch_free(fd, spin);
>>> +             igt_info("%s(single): Measured %.1fMHz, expected %dMhz\n",
>>> +                      e->name, measured, freq);
>>> +             igt_assert(measured > freq - 100 && measured < freq + 100);
>>> +     }
>>> +     gem_quiescent_gpu(fd);
>>
>> Check frequency has gone back to ~min.
> 
> It's not that interesting a test (covered already by pmu) as we
> essentially lie anyway over idle.

Agreed.

> 
>> I would suggest to split here into two sub-tests.
>>
>>> +     spin = __igt_spin_batch_new(fd, ctx, engine, 0);
>>> +     for (uint32_t freq = min + 50; freq <= max; freq += 100) {
>>> +             igt_spin_t *kick;
>>> +
>>> +             set_freq(fd, ctx, freq, freq);
>>> +
>>> +             /*
>>> +              * When requesting a new frequency on the currently
>>> +              * executing context, it does not take effect until the
>>> +              * next context switch. In this case, we trigger a lite
>>> +              * restore.
>>
>> Is this enforced by the ABI?
> 
> Enforced? No. The comment is precisely because it's not checked on
> calling whether the context is currently on the HW and trying hard to be
> sure that no one expects us to do that check. i.e. that set_freq()
> doesn't change frequency itself, but doesn't rule it out either as it
> may appear to have that effect due to many external factors.

That is what I thought :).

I see that you had a new version with more tests, I'll have a look at that.

Thanks,
Antonio

> -Chris
>