Re: [PATCH i-g-t 3/3] tests/intel/xe_pmu: Add idle engine activity test

[Riana Tauro <riana.tauro@intel.com>]

Hi Umesh

On 2/15/2025 12:38 AM, Umesh Nerlige Ramappa wrote:
> On Wed, Feb 12, 2025 at 03:28:29PM +0530, Riana Tauro wrote:
>> Add a test to validate engine is idle by reading PMU counters
>> (engine-active-ticks, engine-total-ticks) when idle
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> tests/intel/xe_pmu.c | 35 +++++++++++++++++++++++++++--------
>> 1 file changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>> index cbb825755..b47be23ac 100644
>> --- a/tests/intel/xe_pmu.c
>> +++ b/tests/intel/xe_pmu.c
>> @@ -21,6 +21,9 @@
>> #define SLEEP_DURATION 2 /* in seconds */
>> const double tolerance = 0.1;
>>
>> +/* flag masks */
>> +#define TEST_BUSY    BIT(0)
> 
> s/BUSY/ACTIVE/
Will fix this. copied the i915 names
Will change it throughout the tests
> 
>> +
>> #define assert_within_epsilon(x, ref, tolerance) \
>>     igt_assert_f((double)(x) <= (1.0 + (tolerance)) * (double)(ref) && \
>>              (double)(x) >= (1.0 - (tolerance)) * (double)(ref), \
>> @@ -79,11 +82,13 @@ static uint64_t get_event_config(int xe, unsigned 
>> int gt, struct drm_xe_engine_c
>> }
>>
>> /**
>> + * SUBTEST: engine-activity-idle
>> + * Description: Test to validate engine activity shows no load when idle
>> + *
>>  * SUBTEST: engine-activity
>> - * Description: Test to validate engine activity stats by running a 
>> workload and
>> - *              reading the active ticks and total ticks PMU counters
>> + * Description: Test to validate engine activity stats by running 
>> full load
>>  */
>> -static void engine_activity(int fd, struct 
>> drm_xe_engine_class_instance *eci)
>> +static void engine_activity(int fd, struct 
>> drm_xe_engine_class_instance *eci, unsigned int flags)
>> {
>>     uint64_t config, busy_ticks, total_ticks, before[2], after[2];
>>     struct xe_cork *cork = NULL;
>> @@ -97,14 +102,18 @@ static void engine_activity(int fd, struct 
>> drm_xe_engine_class_instance *eci)
>>     pmu_fd[1] = open_group(fd, config, pmu_fd[0]);
>>
>>     vm = xe_vm_create(fd, 0, 0);
>> -    cork = xe_cork_create_opts(fd, eci, vm, 1, 1);
>> -    xe_cork_sync_start(fd, cork);
>> +
>> +    if (flags & TEST_BUSY) {
>> +        cork = xe_cork_create_opts(fd, eci, vm, 1, 1);
>> +        xe_cork_sync_start(fd, cork);
>> +    }
>>
>>     pmu_read_multi(pmu_fd[0], 2, before);
>>     usleep(SLEEP_DURATION * USEC_PER_SEC);
> 
> Usually, I see 3 cases for a single type of test
> 
> 1) idle (no spinner is run)
> 2) active (spinner is run, both samples are captured while running).
> 3) active, then idle (spinner is run, capture second sample after ending 
> spinner)
> 
> Can you also please add case (3). Rest looks good.
Sure will add the trailing idle case

Thanks
Riana
> 
> Thanks,
> Umesh
>>     pmu_read_multi(pmu_fd[0], 2, after);
>>
>> -    xe_cork_sync_end(fd, cork);
>> +    if (flags & TEST_BUSY)
>> +        xe_cork_sync_end(fd, cork);
>>
>>     busy_ticks = after[0] - before[0];
>>     total_ticks = after[1] - before[1];
>> @@ -122,7 +131,10 @@ static void engine_activity(int fd, struct 
>> drm_xe_engine_class_instance *eci)
>>     close(pmu_fd[0]);
>>     close(pmu_fd[1]);
>>
>> -    assert_within_epsilon(busy_ticks, total_ticks, tolerance);
>> +    if (flags & TEST_BUSY)
>> +        assert_within_epsilon(busy_ticks, total_ticks, tolerance);
>> +    else
>> +        igt_assert(!busy_ticks);
>> }
>>
>> igt_main
>> @@ -134,12 +146,19 @@ igt_main
>>         fd = drm_open_driver(DRIVER_XE);
>>     }
>>
>> +    igt_describe("Validate there is no engine activity when idle");
>> +    igt_subtest_with_dynamic("engine-activity-idle")
>> +        xe_for_each_engine(fd, hwe)
>> +            igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe- 
>> >engine_class),
>> +                      hwe->engine_instance)
>> +                engine_activity(fd, hwe, 0);
>> +
>>     igt_describe("Validate engine activity with workload running by 
>> reading pmu counters");
>>     igt_subtest_with_dynamic("engine-activity")
>>         xe_for_each_engine(fd, hwe)
>>             igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe- 
>> >engine_class),
>>                       hwe->engine_instance)
>> -                engine_activity(fd, hwe);
>> +                engine_activity(fd, hwe, TEST_BUSY);
>>
>>     igt_fixture {
>>         close(fd);
>> -- 
>> 2.47.1
>>