On 30/06/2023 20:16, Belgaumkar, Vinay wrote:
On 5/23/2023 3:51 AM, Tvrtko Ursulin wrote:All the above spin functions have been duplicated across 2-3 tests, time to create a lib for them?From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Exercise a bunch of up and down rps thresholds to verify hardware is happy with them all. To limit the overall runtime relies on probability and number of runs to approach complete coverage. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> --- tests/i915/i915_pm_rps.c | 232 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c index 050d68a16559..acff59207311 100644 --- a/tests/i915/i915_pm_rps.c +++ b/tests/i915/i915_pm_rps.c @@ -39,8 +39,10 @@ #include "i915/gem.h" #include "i915/gem_create.h" #include "igt.h" +#include "igt_aux.h" #include "igt_dummyload.h" #include "igt_perf.h" +#include "igt_rand.h" #include "igt_sysfs.h" /** * TEST: i915 pm rps @@ -914,6 +916,200 @@ static void pm_rps_exit_handler(int sig) close(drm_fd); }+static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,+ const struct intel_execution_engine2 *e) +{ + struct igt_spin_factory opts = { + .ahnd = ahnd, + .ctx = ctx, + .engine = e->flags, + }; + + if (gem_class_can_store_dword(fd, e->class)) + opts.flags |= IGT_SPIN_POLL_RUN; + + return __igt_spin_factory(fd, &opts); +} + +static unsigned long __spin_wait(int fd, igt_spin_t *spin) +{ + struct timespec start = { }; + + igt_nsec_elapsed(&start); + + if (igt_spin_has_poll(spin)) { + unsigned long timeout = 0; + + while (!igt_spin_has_started(spin)) { + unsigned long t = igt_nsec_elapsed(&start); + + igt_assert(gem_bo_busy(fd, spin->handle)); + if ((t - timeout) > 250e6) { + timeout = t; + igt_warn("Spinner not running after %.2fms\n", + (double)t / 1e6); + igt_assert(t < 2e9); + } + } + } else { + igt_debug("__spin_wait - usleep mode\n"); + usleep(500e3); /* Better than nothing! */ + } + + igt_assert(gem_bo_busy(fd, spin->handle)); + return igt_nsec_elapsed(&start); +} ++static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,+ const struct intel_execution_engine2 *e) +{ + igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e); + + __spin_wait(fd, spin); + + return spin; +}
Three is the magic number so yes, a reasonable ask - done locally. Holding off the re-send though until the 2nd point below is discussed.
+ +static struct i915_engine_class_instance +find_dword_engine(int i915, const unsigned int gt) +{ + struct i915_engine_class_instance *engines, ci = { -1, -1 }; + unsigned int i, count; + + engines = gem_list_engines(i915, 1u << gt, ~0u, &count); + igt_assert(engines); + + for (i = 0; i < count; i++) { + if (!gem_class_can_store_dword(i915, engines[i].engine_class)) + continue; + + ci = engines[i]; + break; + } + + free(engines); + + return ci; +} ++static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,+ const intel_ctx_t **ctx) +{ + struct i915_engine_class_instance ci = { -1, -1 }; + struct intel_execution_engine2 e = { }; + + ci = find_dword_engine(i915, gt); + + igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID); + + if (gem_has_contexts(i915)) { + e.class = ci.engine_class; + e.instance = ci.engine_instance; + e.flags = 0; + *ctx = intel_ctx_create_for_engine(i915, e.class, e.instance); + } else { + igt_require(gt == 0); /* Impossible anyway. */ + e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT); + e.instance = 0; + e.flags = I915_EXEC_DEFAULT; + *ctx = intel_ctx_0(i915); + } + + igt_debug("Using engine %u:%u\n", e.class, e.instance); + + return __spin_sync(i915, ahnd, *ctx, &e); +} + +#define TEST_IDLE 0x1 +#define TEST_PARK 0x2+static void test_thresholds(int i915, unsigned int gt, unsigned int flags)+{ + uint64_t ahnd = get_reloc_ahnd(i915, 0); + const unsigned int points = 10; + unsigned int def_up, def_down; + igt_spin_t *spin = NULL; + const intel_ctx_t *ctx; + unsigned int *ta, *tb; + unsigned int i; + int sysfs; + + sysfs = igt_sysfs_gt_open(i915, gt); + igt_require(sysfs >= 0); + + /* Feature test */ + def_up = igt_sysfs_get_u32(sysfs, "rps_up_threshold_pct"); + def_down = igt_sysfs_get_u32(sysfs, "rps_down_threshold_pct"); + igt_require(def_up && def_down); + + /* Check invalid percentages are rejected */+ igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false); + igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);+ + /*+ * Invent some random up-down thresholds, but always include 0 and 100+ * just to have some wild edge cases. + */ + ta = calloc(points, sizeof(unsigned int)); + tb = calloc(points, sizeof(unsigned int)); + igt_require(ta && tb); + + ta[0] = tb[0] = 0; + ta[1] = tb[1] = 100; + hars_petruska_f54_1_random_seed(time(NULL)); + for (i = 2; i < points; i++) { + ta[i] = hars_petruska_f54_1_random_unsafe_max(100); + tb[i] = hars_petruska_f54_1_random_unsafe_max(100); + } + igt_permute_array(ta, points, igt_exchange_int); + igt_permute_array(tb, points, igt_exchange_int); ++ /* Exercise the thresholds with a GPU load to trigger park/unpark etc */+ for (i = 0; i < points; i++) {+ igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]); + igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true); + igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);Do we care if threshold_down > threshold_up?
We should avoid creating a policy unless there is no other choice. I don't know - can we categorically say it never make sense to have down greater than up? For any GPU, number of discrete frequencies, workload, use case? You are more at home in this area.
Regards, Tvrtko
Thanks, Vinay.+ + if (flags & TEST_IDLE) { + gem_quiescent_gpu(i915); + } else if (spin) { + intel_ctx_destroy(i915, ctx); + igt_spin_free(i915, spin); + spin = NULL; + if (flags & TEST_PARK) { + gem_quiescent_gpu(i915); + usleep(500000); + } + } + spin = spin_sync_gt(i915, ahnd, gt, &ctx); + usleep(1000000); + if (flags & TEST_IDLE) { + intel_ctx_destroy(i915, ctx); + igt_spin_free(i915, spin); + if (flags & TEST_PARK) { + gem_quiescent_gpu(i915); + usleep(500000); + } + spin = NULL; + } + } + + if (spin) { + intel_ctx_destroy(i915, ctx); + igt_spin_free(i915, spin); + } + + gem_quiescent_gpu(i915); + + /* Restore defaults */+ igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true); + igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);+ + free(ta); + free(tb); + close(sysfs); + put_ahnd(ahnd); +} + igt_main { igt_fixture { @@ -993,4 +1189,40 @@ igt_main waitboost(drm_fd, true); igt_disallow_hang(drm_fd, hang); } + + igt_subtest_with_dynamic("thresholds-idle") { + int tmp, gt; + + i915_for_each_gt(drm_fd, tmp, gt) { + igt_dynamic_f("gt%u", gt) + test_thresholds(drm_fd, gt, TEST_IDLE); + } + } + + igt_subtest_with_dynamic("thresholds") { + int tmp, gt; + + i915_for_each_gt(drm_fd, tmp, gt) { + igt_dynamic_f("gt%u", gt) + test_thresholds(drm_fd, gt, 0); + } + } + + igt_subtest_with_dynamic("thresholds-park") { + int tmp, gt; + + i915_for_each_gt(drm_fd, tmp, gt) { + igt_dynamic_f("gt%u", gt) + test_thresholds(drm_fd, gt, TEST_PARK); + } + } + + igt_subtest_with_dynamic("thresholds-idle-park") { + int tmp, gt; + + i915_for_each_gt(drm_fd, tmp, gt) { + igt_dynamic_f("gt%u", gt) + test_thresholds(drm_fd, gt, TEST_IDLE | TEST_PARK); + } + } }
