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

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

On 3/14/2018 2:33 PM, Chris Wilson wrote:
> 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@chris-wilson.co.uk>
>>> Cc: Praveen Paneri <praveen.paneri@intel.com>
>>> Cc: Sagar A Kamble <sagar.a.kamble@intel.com>
>>> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
>>> Reviewed-by: Antonio Argenziano <antonio.argenziano@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@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.
Right. I meant i915 min|max_freq_context in your upcoming patch.
boost_freq is clamped against max_hw and min_user|soft|context
Understood that setting it to Rpn will make it get clamped in the 
expected range :)
Thanks for clarification.
>> 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

-- 
Thanks,
Sagar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx