On 15/12/2017 17:05, Chris Wilson wrote:
Use the normal batch_duration_ns and display the sampled frequency:

        Frequency: min=100, max=750, boost=750 MHz
        Min frequency: requested 100.0, actual 100.0
        Max frequency: requested 755.6, actual 755.6

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
---
  tests/perf_pmu.c | 40 +++++++++++++++++++++++++---------------
  1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index d88287c17..61ae96d9a 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -931,9 +931,10 @@ test_interrupts(int gem_fd)
  static void
  test_frequency(int gem_fd)
  {
-       const uint64_t duration_ns = 2e9;

I think I remember why it was this long - because in the early days test was actually applying load, not modifying frequencies directly, so had to wait for the frequency to ramp up.

        uint32_t min_freq, max_freq, boost_freq;
-       uint64_t min[2], max[2], start[2];
+       uint64_t val[2], start[2];
+       double min[2], max[2];
+       unsigned long slept;
        igt_spin_t *spin;
        int fd, sysfs;
@@ -962,17 +963,19 @@ test_frequency(int gem_fd)
        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);
+ gem_quiescent_gpu(gem_fd); /* Idle to be sure the change takes effect */
+       spin = igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
        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);
+       slept = measured_usleep(batch_duration_ns / 1000);

Wouldn't it be more precise to read the pmu at this point?

+       igt_spin_batch_end(spin);
- pmu_read_multi(fd, 2, min);
-       min[0] -= start[0];
-       min[1] -= start[1];
+       pmu_read_multi(fd, 2, val);
+       min[0] = 1e9*(val[0] - start[0]) / slept;
+       min[1] = 1e9*(val[1] - start[1]) / slept;
igt_spin_batch_free(gem_fd, spin);
+       gem_quiescent_gpu(gem_fd); /* Don't leak busy bo into the next phase */
usleep(1e6); @@ -987,17 +990,19 @@ test_frequency(int gem_fd)
        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);
+ gem_quiescent_gpu(gem_fd);
+       spin = igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
        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);
+       slept = measured_usleep(batch_duration_ns / 1000);
+       igt_spin_batch_end(spin);
- pmu_read_multi(fd, 2, max);
-       max[0] -= start[0];
-       max[1] -= start[1];
+       pmu_read_multi(fd, 2, val);
+       max[0] = 1e9*(val[0] - start[0]) / slept;
+       max[1] = 1e9*(val[1] - start[1]) / slept;
igt_spin_batch_free(gem_fd, spin);
+       gem_quiescent_gpu(gem_fd);

Ah I see.. only for the spin batch. Why not then gem_sync or maybe we should add igt_spin_batch_free_sync?

Regards,

Tvrtko

/*
         * Restore min/max.
@@ -1008,8 +1013,13 @@ test_frequency(int gem_fd)
                         min_freq, igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz"));
        close(fd);
+ igt_info("Min frequency: requested %.1f, actual %.1f\n",
+                min[0], min[1]);
+       igt_info("Max frequency: requested %.1f, actual %.1f\n",
+                max[0], max[1]);
        igt_assert(min[0] < max[0]);
-       igt_assert(min[1] < max[1]);
+       igt_assert(min[1] <= min[0]);
+       igt_assert(max[1] <= max[0]);
  }
static bool wait_for_rc6(int fd)

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

Reply via email to