[Intel-gfx] [PATCH i-g-t v6 7/9] tests/perf_pmu: Tests for i915 PMU API

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-10-10 15:17:54)
> +static void
> +busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
> +              const unsigned int num_engines)
> +{
> +       const struct intel_execution_engine2 *e_;
> +       uint64_t val[num_engines];
> +       int fd[num_engines];
> +       igt_spin_t *spin;
> +       unsigned int busy_idx, i;
> +
> +       i = 0;
> +       fd[0] = -1;
> +       for_each_engine_class_instance(fd, e_) {
> +               if (!gem_has_engine(gem_fd, e_->class, e_->instance))
> +                       continue;
> +               else if (e == e_)
> +                       busy_idx = i;
> +
> +               fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class,
> +                                                         e_->instance),
> +                                    fd[0]);
> +       }
igt_assert(i == num_engines);

Feels like a bug waiting to happen; a trap.

> +static void
> +test_frequency(int gem_fd)
> +{
> +       const uint64_t duration_ns = 2e9;
> +       uint32_t min_freq, max_freq, boost_freq;
> +       uint64_t min[2], max[2], start[2];
> +       igt_spin_t *spin;
> +       int fd, sysfs;
> +
> +       sysfs = igt_sysfs_open(gem_fd, NULL);
> +       igt_require(sysfs >= 0);
> +
> +       min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
> +       max_freq = igt_sysfs_get_u32(sysfs, "gt_RP0_freq_mhz");
> +       boost_freq = igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz");
> +       igt_require(min_freq > 0 && max_freq > 0 && boost_freq > 0);
> +       igt_require(max_freq > min_freq);
> +       igt_require(boost_freq > min_freq);
> +
> +       fd = open_group(I915_PMU_REQUESTED_FREQUENCY, -1);
> +       open_group(I915_PMU_ACTUAL_FREQUENCY, fd);
> +
> +       /*
> +        * Set GPU to min frequency and read PMU counters.
> +        */
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", min_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == min_freq);
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", min_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == min_freq);
> +
> +       pmu_read_multi(fd, 2, start);
> +
> +       spin = igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
> +       igt_spin_batch_set_timeout(spin, duration_ns);
> +       gem_sync(gem_fd, spin->handle);
> +
> +       pmu_read_multi(fd, 2, min);
> +       min[0] -= start[0];
> +       min[1] -= start[1];
> +
> +       igt_spin_batch_free(gem_fd, spin);
> +
> +       usleep(1e6);
> +
> +       /*
> +        * Set GPU to max frequency and read PMU counters.
> +        */
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", max_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == max_freq);
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", boost_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == boost_freq);
> +
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", max_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == max_freq);
> +
> +       pmu_read_multi(fd, 2, start);
> +
> +       spin = igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
> +       igt_spin_batch_set_timeout(spin, duration_ns);
> +       gem_sync(gem_fd, spin->handle);
> +
> +       pmu_read_multi(fd, 2, max);
> +       max[0] -= start[0];
> +       max[1] -= start[1];
> +
> +       igt_spin_batch_free(gem_fd, spin);
> +
> +       /*
> +        * Restore min/max.
> +        */
> +       igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", min_freq));
> +       igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == min_freq);

The test is done at this point, and you are just being neat and tidy for
the next user. We don't need to do anything but warn:

igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz");
if (igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") != min_freq)
	igt_warn("Unable to restore min frequency to save value [%d MHz], now %d MHz\n",
		 min_freq, igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz"));
> +
> +       close(fd);

Add to the list of subtests that want a destructor (for clean error
paths).

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris