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@xxxxxxxxxxxxxxxxxx> -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx