[PATCH i-g-t v2 3/5] tests/intel/xe_pmu: Add tests to validate engine activity with single engine active
Riana Tauro
riana.tauro at intel.com
Fri May 9 14:20:46 UTC 2025
Hi Umesh
On 5/9/2025 5:14 AM, Umesh Nerlige Ramappa wrote:
> On Thu, May 08, 2025 at 11:20:04AM +0530, Riana Tauro wrote:
>> Add a test that validates by running workload on a single engine and
>> checks if others are idle.
>>
>> Also add a trailing idle test case.
>>
>> v2: use allocator
>>
>> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>> ---
>> tests/intel/xe_pmu.c | 114 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 113 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>> index 1ad927822..d78b8bea5 100644
>> --- a/tests/intel/xe_pmu.c
>> +++ b/tests/intel/xe_pmu.c
>> @@ -24,6 +24,14 @@
>> * SUBTEST: engine-activity-load
>> * Description: Test to validate engine activity stats by running a
>> workload
>> *
>> + * SUBTEST: engine-activity-single-load
>> + * Description: Test to validate engine activity by running workload
>> on one engine and check if
>> + * all other engines are idle
>> + *
>> + * SUBTEST: engine-activity-single-load-idle
>> + * Description: Test to validate engine activity by running workload
>> and trailing idle on one engine
>> + * and check if all other engines are idle
>> + *
>> * SUBTEST: all-fn-engine-activity-load
>> * Description: Test to validate engine activity by running load on
>> all functions simultaneously
>> *
>> @@ -51,6 +59,7 @@
>> /* flag masks */
>> #define TEST_LOAD BIT(0)
>> #define TEST_TRAILING_IDLE BIT(1)
>> +#define TEST_IDLE BIT(2)
>>
>> const double tolerance = 0.1;
>> static char xe_device[NAME_MAX];
>> @@ -165,6 +174,43 @@ static void check_and_end_cork(int fd, struct
>> xe_cork *cork)
>> xe_cork_sync_end(fd, cork);
>> }
>>
>> +static void log_all_engines(int num_engines, int skip_idx, int flag,
>> + uint64_t *before, uint64_t *after)
>
> s/log_all_engines/check_all_engines/ since you are checking idle/busy.
>
> s/skip_idx/target_idx/ or something else, since you are deciding if the
> target is idle/busy based on the flag passed.
Will fix these
>
>> +{
>> + uint64_t engine_active_ticks, engine_total_ticks;
>> + int engine_idx = 0;
>> +
>> + for (int idx = 0; idx < num_engines * 2; idx += 2) {
>> + engine_idx = idx >> 1;
>> +
>> + engine_active_ticks = after[idx] - before[idx];
>> + engine_total_ticks = after[idx + 1] - before[idx + 1];
>> +
>> + igt_debug("[%d] Engine active ticks: after %ld, before %ld
>> delta %ld\n", engine_idx,
>> + after[idx], before[idx], engine_active_ticks);
>> + igt_debug("[%d] Engine total ticks: after %ld, before %ld
>> delta %ld\n", engine_idx,
>> + after[idx + 1], before[idx + 1], engine_total_ticks);
>> +
>> + /*
>> + * If flag is load and skip_idx is set, then check all
>> engines for load except
>> + * the skip engine id and vice versa
>> + */
>> + if (flag == TEST_LOAD) {
>> + if (engine_idx == skip_idx)
>> + assert_within_epsilon(engine_active_ticks, 0.0f,
>> tolerance);
>> + else
>> + assert_within_epsilon(engine_active_ticks,
>> engine_total_ticks,
>> + tolerance);
>> + } else {
>
> Assuming this ^ is else if (flag == TEST_IDLE). I would make it explicit
> here.
Will fix this
>
>> + if (engine_idx == skip_idx)
>> + assert_within_epsilon(engine_active_ticks,
>> engine_total_ticks,
>> + tolerance);
>> + else
>> + assert_within_epsilon(engine_active_ticks, 0.0f,
>> tolerance);
>> + }
>> + }
>> +}
>
> Overall, I would split the above helper in two:
>
> check_all_idle_except_one()
> check_all_loaded_except_one()
I thought instead of duplicating code, can use a single function.
But asserts can be identified using test name
The functions aren't used by any other tests if i split.
I'll move this back into the test function call?
Thank you
Riana
>
> or any other name that conveys similar meaning. I feel it would be
> easier to read and debug once the asserts start kicking in. If you do
> that, you could drop the flag parameter.
>
> Rest looks fine.
>
> Thanks,
> Umesh
>
>> +
>> static void engine_activity(int fd, struct
>> drm_xe_engine_class_instance *eci, unsigned int flags)
>> {
>> uint64_t config, engine_active_ticks, engine_total_ticks,
>> before[2], after[2];
>> @@ -216,6 +262,63 @@ static void engine_activity(int fd, struct
>> drm_xe_engine_class_instance *eci, un
>> igt_assert(!engine_active_ticks);
>> }
>>
>> +static void engine_activity_load_single(int fd, int num_engines,
>> + struct drm_xe_engine_class_instance *eci,
>> + unsigned int flags)
>> +{
>> + uint64_t ahnd, before[2 * num_engines], after[2 * num_engines],
>> config;
>> + struct drm_xe_engine_class_instance *eci_;
>> + struct xe_cork *cork = NULL;
>> + int pmu_fd[num_engines * 2];
>> + int idx = 0, busy_idx = 0;
>> + uint32_t vm;
>> +
>> + pmu_fd[0] = -1;
>> + xe_for_each_engine(fd, eci_) {
>> + if (eci_->engine_class == eci->engine_class &&
>> + eci_->engine_instance == eci->engine_instance)
>> + busy_idx = idx >> 1;
>> +
>> + config = get_event_config(eci_->gt_id, eci_, "engine-active-
>> ticks");
>> + pmu_fd[idx++] = open_group(fd, config, pmu_fd[0]);
>> +
>> + config = get_event_config(eci_->gt_id, eci_, "engine-total-
>> ticks");
>> + pmu_fd[idx++] = open_group(fd, config, pmu_fd[0]);
>> + }
>> +
>> + vm = xe_vm_create(fd, 0, 0);
>> + ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
>> + if (flags & TEST_LOAD) {
>> + cork = xe_cork_create_opts(fd, eci, vm, 1, 1, .ahnd = ahnd);
>> + xe_cork_sync_start(fd, cork);
>> + }
>> +
>> + pmu_read_multi(pmu_fd[0], 2 * num_engines, before);
>> + usleep(SLEEP_DURATION * USEC_PER_SEC);
>> + if (flags & TEST_TRAILING_IDLE)
>> + xe_cork_sync_end(fd, cork);
>> + pmu_read_multi(pmu_fd[0], 2 * num_engines, after);
>> +
>> + if (flags & TEST_LOAD) {
>> + check_and_end_cork(fd, cork);
>> + xe_cork_destroy(fd, cork);
>> + }
>> +
>> + xe_vm_destroy(fd, vm);
>> + put_ahnd(ahnd);
>> +
>> + for (idx = 0; idx < num_engines * 2; idx += 2) {
>> + close(pmu_fd[idx]);
>> + close(pmu_fd[idx + 1]);
>> + }
>> +
>> + /*
>> + * TEST_IDLE is passed to log_all_engines to test if all other
>> engines are idle
>> + * except the busy_idx
>> + */
>> + log_all_engines(num_engines, busy_idx, TEST_IDLE, before, after);
>> +}
>> +
>> static void engine_activity_all_fn(int fd, struct
>> drm_xe_engine_class_instance *eci, int num_fns)
>> {
>> uint64_t config, engine_active_ticks, engine_total_ticks;
>> @@ -544,12 +647,13 @@ static void restore_gt_freq(int fd, uint32_t
>> *stash_min, uint32_t *stash_max)
>>
>> igt_main
>> {
>> - int fd, gt;
>> + int fd, gt, num_engines;
>> struct drm_xe_engine_class_instance *eci;
>>
>> igt_fixture {
>> fd = drm_open_driver(DRIVER_XE);
>> xe_perf_device(fd, xe_device, sizeof(xe_device));
>> + num_engines = xe_number_engines(fd);
>> }
>>
>> igt_describe("Validate PMU gt-c6 residency counters when idle");
>> @@ -571,6 +675,14 @@ igt_main
>> test_each_engine("engine-activity-load", fd, eci)
>> engine_activity(fd, eci, TEST_LOAD);
>>
>> + igt_describe("Validate engine activity of all engines when one
>> engine is loaded");
>> + test_each_engine("engine-activity-single-load", fd, eci)
>> + engine_activity_load_single(fd, num_engines, eci, TEST_LOAD);
>> +
>> + igt_describe("Validate engine activity of all engines with one
>> engine loaded and trailing idle");
>> + test_each_engine("engine-activity-single-load-idle", fd, eci)
>> + engine_activity_load_single(fd, num_engines, eci, TEST_LOAD |
>> TEST_TRAILING_IDLE);
>> +
>> igt_subtest_group {
>> unsigned int num_fns;
>>
>> --
>> 2.47.1
>>
More information about the igt-dev
mailing list