Re: [i-g-t,1/2] tests/i915/pm_rc6_residency: Use sysfs to achieve peak frequency

["Anirban, Sk" <sk.anirban@intel.com>]

Hi Riana,

On 22-05-2025 09:04, Riana Tauro wrote:
> Hi Anirban
>
> On 5/14/2025 12:07 AM, sk.anirban@intel.com wrote:
>> From: Sk Anirban <sk.anirban@intel.com>
>>
>> Leverage IGT spinner to trigger frequency scaling and
>> set RPS minimum frequency to RP0 for achieving peak frequency.
>> This method replaces the existing waitboost mechanism used
>> to reach maximum frequency.
>>
>> v2:
>>   - Disable power consumption check for dgfx
>>
>> v3:
>>   - Implement igt exit handler
>>
>> v4:
>>   - Cosmetic changes (Riana)
>>
>> v5:
>>   - Limit the power consumption test to GT0 only
>>
>> v6:
>>   - Fix frequency restoration (Riana)
>>
>> Signed-off-by: Sk Anirban <sk.anirban@intel.com>
>> ---
>>   tests/intel/i915_pm_rc6_residency.c | 136 +++++++++++++++++++---------
>>   1 file changed, 91 insertions(+), 45 deletions(-)
>>
>> diff --git a/tests/intel/i915_pm_rc6_residency.c 
>> b/tests/intel/i915_pm_rc6_residency.c
>> index c9128481d..b400460c7 100644
>> --- a/tests/intel/i915_pm_rc6_residency.c
>> +++ b/tests/intel/i915_pm_rc6_residency.c
>> @@ -264,17 +264,6 @@ static bool __pmu_wait_for_rc6(int fd)
>>       return false;
>>   }
>>   -static uint32_t batch_create(int fd)
>> -{
>> -    const uint32_t bbe = MI_BATCH_BUFFER_END;
>> -    uint32_t handle;
>> -
>> -    handle = gem_create(fd, 4096);
>> -    gem_write(fd, handle, 0, &bbe, sizeof(bbe));
>> -
>> -    return handle;
>> -}
>> -
>>   static int open_pmu(int i915, uint64_t config)
>>   {
>>       int fd;
>> @@ -286,45 +275,88 @@ static int open_pmu(int i915, uint64_t config)
>>       return fd;
>>   }
>>   -#define WAITBOOST 0x1
>> +#define FREQUENT_BOOST 0x1
>>   #define ONCE 0x2
>>     static void sighandler(int sig)
>>   {
>>   }
>>   -static void bg_load(int i915, uint32_t ctx_id, uint64_t 
>> engine_flags, unsigned int flags, unsigned long *ctl)
>> +static uint32_t get_freq(int dirfd, uint8_t id)
>> +{
>> +    uint32_t val;
>> +
>> +    igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
>> +
>> +    return val;
>> +}
>> +
>> +static int set_freq(int dirfd, uint8_t id, uint32_t val)
>> +{
>> +    return igt_sysfs_rps_printf(dirfd, id, "%u", val);
>> +}
>> +
>> +uint32_t stash_min;
>> +int i915 = -1;
>> +
>> +static void restore_freq(int sig)
>> +{
>> +    int dirfd;
>> +    int gt = 0;
>> +
>> +    i915 = drm_open_driver(DRIVER_INTEL);
>
> Why open again. The reason for making it static was re-use
> Is i915 not open when exit handler is run?
The exit handler is getting called after the driver closure so 
re-opening it to the restore frequency.
>
> Rest of the code looks good.
>
> Also respond on the BAT failures as the test is same and are legacy 
> issues
Yeah, testing rev again so that ci team can re-report the BAT failures.
>
> Thanks
> Riana
Thanks,
Anirban
>
>> +    dirfd = igt_sysfs_gt_open(i915, 0);
>> +    igt_sysfs_gt_open(i915, gt);
>> +
>> +    igt_assert_lt(0, set_freq(dirfd, RPS_MIN_FREQ_MHZ, stash_min));
>> +
>> +    drm_close_driver(i915);
>> +    close(dirfd);
>> +}
>> +
>> +static void bg_load(const intel_ctx_t *ctx, int dirfd, uint64_t 
>> engine_flags,
>> +            unsigned int flags, unsigned long *ctl, unsigned int gt)
>>   {
>>       const bool has_execlists = intel_gen(intel_get_drm_devid(i915)) 
>> >= 8;
>> -    struct drm_i915_gem_exec_object2 obj = {
>> -        .handle = batch_create(i915),
>> -    };
>> -    struct drm_i915_gem_execbuffer2 execbuf = {
>> -        .buffers_ptr = to_user_pointer(&obj),
>> -        .buffer_count = 1,
>> -        .flags = engine_flags,
>> -        .rsvd1 = ctx_id,
>> -    };
>>       struct sigaction act = {
>>           .sa_handler = sighandler
>>       };
>> +    int64_t timeout = 1;
>> +    uint64_t ahnd;
>> +    int rp0;
>>   +    ahnd = get_reloc_ahnd(i915, ctx->id);
>> +    rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>>       sigaction(SIGINT, &act, NULL);
>>       do {
>>           uint64_t submit, wait, elapsed;
>>           struct timespec tv = {};
>> +        igt_spin_t *spin;
>>             igt_nsec_elapsed(&tv);
>> -
>> -        gem_execbuf(i915, &execbuf);
>> +        spin = igt_spin_new(i915,
>> +                    .ahnd = ahnd,
>> +                    .ctx = ctx,
>> +                    .engine = engine_flags);
>>           submit = igt_nsec_elapsed(&tv);
>> -        if (flags & WAITBOOST) {
>> -            gem_sync(i915, obj.handle);
>> +        if (flags & FREQUENT_BOOST) {
>> +            /* Set MIN freq to RP0 to achieve the peak freq */
>> +            igt_assert_lt(0, set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0));
>> +            igt_assert(gem_bo_busy(i915, spin->handle));
>> +            gem_wait(i915, spin->handle, &timeout);
>> +
>> +            /* Restore the MIN freq back to default */
>> +            igt_assert_lt(0, set_freq(dirfd, RPS_MIN_FREQ_MHZ, 
>> stash_min));
>> +            igt_spin_end(spin);
>> +            igt_spin_free(i915, spin);
>> +            gem_quiescent_gpu(i915);
>>               if (flags & ONCE)
>> -                flags &= ~WAITBOOST;
>> +                flags &= ~FREQUENT_BOOST;
>>           } else  {
>> -            while (gem_bo_busy(i915, obj.handle))
>> -                usleep(0);
>> +            igt_assert(gem_bo_busy(i915, spin->handle));
>> +            igt_spin_end(spin);
>> +            igt_spin_free(i915, spin);
>> +            gem_quiescent_gpu(i915);
>>           }
>>           wait = igt_nsec_elapsed(&tv);
>>   @@ -350,6 +382,7 @@ static void bg_load(int i915, uint32_t ctx_id, 
>> uint64_t engine_flags, unsigned i
>>           /* aim for ~1% busy */
>>           usleep(min_t(elapsed, elapsed / 10, 50 * 1000));
>>       } while (!READ_ONCE(*ctl));
>> +    put_ahnd(ahnd);
>>   }
>>     static void kill_children(int sig)
>> @@ -361,9 +394,9 @@ static void kill_children(int sig)
>>       signal(sig, old);
>>   }
>>   -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags, 
>> unsigned int gt)
>> +static void rc6_idle(const intel_ctx_t *ctx, uint64_t flags, 
>> unsigned int gt)
>>   {
>> -    const int64_t duration_ns = SLEEP_DURATION * (int64_t)NSEC_PER_SEC;
>> +    const int64_t duration_ns = 2 * SLEEP_DURATION * 
>> (int64_t)NSEC_PER_SEC;
>>       const int tolerance = 20; /* Some RC6 is better than none! */
>>       const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>>       struct {
>> @@ -371,10 +404,11 @@ static void rc6_idle(int i915, uint32_t ctx_id, 
>> uint64_t flags, unsigned int gt)
>>           unsigned int flags;
>>           double power;
>>       } phases[] = {
>> +        { "once", FREQUENT_BOOST | ONCE },
>>           { "normal", 0 },
>> -        { "boost", WAITBOOST },
>> -        { "once", WAITBOOST | ONCE },
>> +        { "boost", FREQUENT_BOOST }
>>       };
>> +    int dirfd = igt_sysfs_gt_open(i915, gt);
>>       struct power_sample sample[2];
>>       unsigned long slept, cycles;
>>       unsigned long *done;
>> @@ -407,12 +441,21 @@ static void rc6_idle(int i915, uint32_t ctx_id, 
>> uint64_t flags, unsigned int gt)
>>         assert_within_epsilon_debug(rc6, ts[1] - ts[0], 5, drpc);
>>   +    if (gt) {
>> +        close(fd);
>> +        close(dirfd);
>> +        igt_power_close(&gpu);
>> +        return;
>> +    }
>> +
>>       done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
>>   +    stash_min = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
>> +
>>       for (int p = 0; p < ARRAY_SIZE(phases); p++) {
>>           memset(done, 0, 2 * sizeof(*done));
>>           igt_fork(child, 1) /* Setup up a very light load */
>> -            bg_load(i915, ctx_id, flags, phases[p].flags, done);
>> +            bg_load(ctx, dirfd, flags, phases[p].flags, done, gt);
>>             igt_power_get_energy(&gpu, &sample[0]);
>>           cycles = -READ_ONCE(done[1]);
>> @@ -451,19 +494,20 @@ static void rc6_idle(int i915, uint32_t ctx_id, 
>> uint64_t flags, unsigned int gt)
>>         munmap(done, 4096);
>>       close(fd);
>> +    close(dirfd);
>>         igt_power_close(&gpu);
>>   -    if (phases[1].power - phases[0].power > 10) {
>> -        igt_assert_f(2 * phases[2].power - phases[0].power <= 
>> phases[1].power,
>> +    if (phases[2].power - phases[1].power > 20 && 
>> !gem_has_lmem(i915)) {
>> +        igt_assert_f(2 * phases[0].power - phases[1].power <= 
>> phases[2].power,
>>                    "Exceeded energy expectations for single busy wait 
>> load\n"
>>                    "Used %.1fmW, min %.1fmW, max %.1fmW, expected 
>> less than %.1fmW\n",
>> -                 phases[2].power, phases[0].power, phases[1].power,
>> -                 phases[0].power + (phases[1].power - 
>> phases[0].power) / 2);
>> +                 phases[0].power, phases[1].power, phases[2].power,
>> +                 phases[1].power + (phases[2].power - 
>> phases[1].power) / 2);
>>       }
>>   }
>>   -static void rc6_fence(int i915, unsigned int gt)
>> +static void rc6_fence(unsigned int gt)
>>   {
>>       const int64_t duration_ns = SLEEP_DURATION * 
>> (int64_t)NSEC_PER_SEC;
>>       const int tolerance = 20; /* Some RC6 is better than none! */
>> @@ -557,7 +601,7 @@ static void rc6_fence(int i915, unsigned int gt)
>>       close(fd);
>>   }
>>   -static unsigned int rc6_enabled_mask(int i915, int dirfd)
>> +static unsigned int rc6_enabled_mask(int dirfd)
>>   {
>>       igt_require(has_rc6_residency(dirfd, RC6_RESIDENCY_MS));
>>   @@ -570,7 +614,6 @@ static unsigned int rc6_enabled_mask(int i915, 
>> int dirfd)
>>     igt_main
>>   {
>> -    int i915 = -1;
>>       unsigned int dirfd, gt;
>>       const intel_ctx_t *ctx;
>>   @@ -582,19 +625,22 @@ igt_main
>>       igt_subtest_with_dynamic("rc6-idle") {
>>           const struct intel_execution_engine2 *e;
>>   +        igt_install_exit_handler(restore_freq);
>>           igt_require_gem(i915);
>>           gem_quiescent_gpu(i915);
>> +        intel_allocator_multiprocess_start();
>>             i915_for_each_gt(i915, dirfd, gt) {
>>               ctx = intel_ctx_create_for_gt(i915, gt);
>>               for_each_ctx_engine(i915, ctx, e) {
>>                   if (e->instance == 0) {
>>                       igt_dynamic_f("gt%u-%s", gt, e->name)
>> -                        rc6_idle(i915, ctx->id, e->flags, gt);
>> +                        rc6_idle(ctx, e->flags, gt);
>>                   }
>>               }
>>               intel_ctx_destroy(i915, ctx);
>>           }
>> +        intel_allocator_multiprocess_stop();
>>       }
>>         igt_subtest_with_dynamic("rc6-fence") {
>> @@ -603,7 +649,7 @@ igt_main
>>             i915_for_each_gt(i915, dirfd, gt)
>>               igt_dynamic_f("gt%u", gt)
>> -                rc6_fence(i915, gt);
>> +                rc6_fence(gt);
>>       }
>>         igt_subtest_group {
>> @@ -621,7 +667,7 @@ igt_main
>>                   igt_dynamic_f("gt%u", gt) {
>>                       struct residencies res;
>>   -                    rc6_enabled = rc6_enabled_mask(i915, dirfd);
>> +                    rc6_enabled = rc6_enabled_mask(dirfd);
>>                       igt_require(rc6_enabled & RC6_ENABLED);
>>                         measure_residencies(devid, dirfd, 
>> rc6_enabled, &res);
>> @@ -635,7 +681,7 @@ igt_main
>>                 igt_require(IS_VALLEYVIEW(devid) || 
>> IS_CHERRYVIEW(devid));
>>   -            rc6_enabled = rc6_enabled_mask(i915, sysfs);
>> +            rc6_enabled = rc6_enabled_mask(sysfs);
>>               igt_require(rc6_enabled & RC6_ENABLED);
>>                 measure_residencies(devid, sysfs, rc6_enabled, &res);
>