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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux