Quoting Sagar Arun Kamble (2018-03-13 12:38:04)
>
>
> On 3/10/2018 3:05 AM, Chris Wilson wrote:
> > +static void single(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 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);
> This igt_require can go to igt_fixture below.
Why?
> > +
> > + for (int step = 0; step <= 2*N_STEPS; step++) {
> > + int frac = step > N_STEPS ? 2*N_STEPS - step : step;
> > + uint32_t freq = min + (max - min) * frac / N_STEPS;
> > + uint32_t cur, discard;
> > +
> > + set_freq(fd, ctx, freq, freq);
> > + get_freq(fd, ctx, &cur, &discard);
> > +
> > + gem_quiescent_gpu(fd);
> > + spin = __igt_spin_batch_new(fd, ctx, engine, 0);
> > + usleep(10000);
> > +
> > + measured = measure_frequency(pmu, SAMPLE_PERIOD);
> > + 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, cur);
> > + igt_assert(measured > cur - 100 && measured < cur + 100);
> Is this margin of 100Mhz for PMU accuracy?
Yes, even then it sometimes exceeds it.
> > + }
> > + gem_quiescent_gpu(fd);
> > +
> > + close(pmu);
> > + gem_context_destroy(fd, ctx);
> > +
> > +#undef N_STEPS
> > +}
> > +static void inflight(int fd, const struct intel_execution_engine *e)
> > +{
...
> > + measured = measure_frequency(pmu, SAMPLE_PERIOD);
> > + igt_debugfs_dump(fd, "i915_rps_boost_info");
> > + igt_info("%s(plug): Measured %.1fMHz, expected %dMhz\n",
> > + e->name, measured, min);
> > + igt_assert(measured > min - 100 && measured < min + 100);
> > +
> > + ctx = gem_context_create(fd);
> > + set_freq(fd, ctx, max, max);
> this set_freq can be removed.
No, we want to check it obeys the later restriction and not this one.
> > + work[0] = __igt_spin_batch_new(fd, ctx, engine, 0);
> > +
> > + /* work is now queued but not executing */
> > + freq = (max + min) / 2;
> > + set_freq(fd, ctx, freq, freq);
> > + get_freq(fd, ctx, &freq, &discard);
> > + gem_context_destroy(fd, ctx);
> > + for (unsigned int n = 0; n < nengine; n++) {
> > + igt_debug("[%d]: [%d, %d]\n", n, min[n], max[n]);
> > + if (min[n] < req.min)
> > + req.min = min[n];
> > + if (max[n] > req.max)
> > + req.max = max[n];
> > + }
> I thought policy i915 will be implementing is max of mins and min of maxes.
But the only policy I want userspace to be aware of is that the actual
frequency will be inside their desired range. (Unless contradicted by
the system). That's the ABI we want; how we handle it internally should
be left open with the prospect of either changing it or making it
adjustable.
> > +static void invalid_context(int fd, uint32_t ctx, uint32_t min, uint32_t max)
> > +{
> > + const struct test {
> > + uint32_t min, max;
> > + } tests[] = {
> > + { min - 50, max - 50 },
> > + { min - 50, max },
> > + { min - 50, max + 50 },
> > + { min, max + 50 },
> > + { min + 50, max + 50 },
> > +
> > + { min - 50, min - 50 },
> > +
> > + { min - 50, min },
> This one is similar to { min - 50, max } where max is in range but min
> is outside.
> Similarly on max side. what is the reasoning for these cases?
Checking two wrongs don't make a right.
> > + { min + 50, min },
> > + { min, min - 50 },
> > +
> > + { max + 50, max },
> > + { max, max + 50 },
> > + { max, max - 50 },
> > +
> > + { max + 50, max + 50 },
> > +
> > + {}
> > + };
> > +
> > + for (const struct test *t = tests; t->min | t->max; t++) {
> > + uint32_t cur_min, cur_max;
> > +
> > + igt_assert_f(__set_freq(fd, ctx, t->min, t->max) == -EINVAL,
> > + "Failed to reject invalid [%d, %d] (valid range [%d, %d]) on context %d\n",
> > + t->min, t->max, min, max, ctx);
> > +
> > + get_freq(fd, 0, &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > + }
> > +}
> > +
> > +static void independent(int fd)
> > +{
> > + uint32_t min, max;
> > + uint32_t cur_min, cur_max;
> > + uint32_t ctx[2];
> > +
> > + get_freq(fd, 0, &min, &max);
> > +
> > + set_freq(fd, 0, max, max);
> > + ctx[0] = gem_context_create(fd);
> > + get_freq(fd, ctx[0], &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > +
> > + set_freq(fd, 0, min, min);
> > + get_freq(fd, ctx[0], &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > +
> > + ctx[1] = gem_context_create(fd);
> > + get_freq(fd, ctx[1], &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > +
> > + set_freq(fd, ctx[1], max, max);
> > + get_freq(fd, ctx[0], &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > +
> > + get_freq(fd, 0, &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, min);
> > +
> > + get_freq(fd, ctx[1], &cur_min, &cur_max);
> > + igt_assert_eq(cur_min, max);
> > + igt_assert_eq(cur_max, max);
> > + gem_context_destroy(fd, ctx[1]);
> > +
> > + get_freq(fd, ctx[0], &cur_min, &cur_max);
> There is no set_freq between earlier get_freq and this one for ctx[0] so
> we can skip one.
The point of this test is to say for sure that actions on one context do
not affect another.
> > + igt_assert_eq(cur_min, min);
> > + igt_assert_eq(cur_max, max);
> > + gem_context_destroy(fd, ctx[0]);
> Need to restore min/max for default context?
No, we assert it isn't changed.
So you think we should assert again to be sure.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
