[igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
Sundaresan, Sujaritha
sujaritha.sundaresan at intel.com
Thu Nov 9 05:04:36 UTC 2023
On 11/8/2023 2:35 PM, Sundaresan, Sujaritha wrote:
>
> On 11/8/2023 2:34 PM, Gupta, Anshuman wrote:
>>
>>> -----Original Message-----
>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan at intel.com>
>>> Sent: Wednesday, November 8, 2023 2:25 PM
>>> To: Gupta, Anshuman <anshuman.gupta at intel.com>; igt-
>>> dev at lists.freedesktop.org
>>> Subject: Re: [PATCH] tests/intel: Add multi gt support for
>>> rc6_residency test
>>>
>>>
>>> On 11/8/2023 2:22 PM, Gupta, Anshuman wrote:
>>>>> -----Original Message-----
>>>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan at intel.com>
>>>>> Sent: Wednesday, November 8, 2023 12:13 PM
>>>>> To: igt-dev at lists.freedesktop.org
>>>>> Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Sundaresan,
>>> Sujaritha
>>>>> <sujaritha.sundaresan at intel.com>
>>>>> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency
>>>>> test
>>>>>
>>>>> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy
>>>>> tests.
>>>> Please break this patch like below.
>>>> 1) If any common refactoring required, not applicable if no such
>>>> refactoring.
>>>> 2) Add multi gt support for rc6_idle.
>>>> 3) Add multi gt support for rc6_fence.
>>>> 4) Add multi gt support for rc6_accuracy test.
>>>> 5) Add drpc debug support.
>>> Okay I will make this a 4 patch series then.
>>>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
>>>>> ---
>>>>> tests/intel/i915_pm_rc6_residency.c | 134
>>>>> ++++++++++++++++++--------
>>> --
>>>>> 1 file changed, 85 insertions(+), 49 deletions(-)
>>>>>
>>>>> diff --git a/tests/intel/i915_pm_rc6_residency.c
>>>>> b/tests/intel/i915_pm_rc6_residency.c
>>>>> index b266680ac..f865fbe94 100644
>>>>> --- a/tests/intel/i915_pm_rc6_residency.c
>>>>> +++ b/tests/intel/i915_pm_rc6_residency.c
>>>>> @@ -36,6 +36,7 @@
>>>>> #include "i915/gem.h"
>>>>> #include "i915/gem_create.h"
>>>>> #include "igt.h"
>>>>> +#include "igt_debugfs.h"
>>>>> #include "igt_perf.h"
>>>>> #include "igt_power.h"
>>>>> #include "igt_sysfs.h"
>>>>> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
>>>>> return enabled;
>>>>> }
>>>>>
>>>>> -static bool has_rc6_residency(const char *name)
>>>>> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
>>>>> {
>>>>> unsigned long residency;
>>>>> - char path[128];
>>>>>
>>>>> - sprintf(path, "power/%s_residency_ms", name);
>>>>> - return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
>>>>> + return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
>>>>> }
>>>>>
>>>>> -static unsigned long read_rc6_residency(const char *name)
>>>>> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
>>>>> +id)
>>>>> {
>>>>> unsigned long residency;
>>>>> - char path[128];
>>>>>
>>>>> residency = 0;
>>>>> - sprintf(path, "power/%s_residency_ms", name);
>>>>> - igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) ==
>>>>> 1);
>>>>> + igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency)
>>>>> == 1);
>>>>> return residency;
>>>>> }
>>>>>
>>>>> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int
>>>>> diff,
>>>>> const char *name_of_rc6_residency) {
>>>>> double ratio;
>>>>> + int gt = 0, gt_current;
>>>>>
>>>>> ratio = (double)diff / duration;
>>>>>
>>>>> - igt_info("Residency in %s or deeper state: %u ms (sleep
>>>>> duration %u
>>>>> ms) (%.1f%% of expected duration)\n",
>>>>> - name_of_rc6_residency, diff, duration, 100*ratio);
>>>>> + gt_current = igt_sysfs_get_u32(gt, "id");
>>>>> +
>>>>> + igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
>>>>> duration %u ms) (%.1f%% of expected duration)\n",
>>>>> + gt_current, name_of_rc6_residency, diff, duration,
>>>>> 100*ratio);
>>>>> igt_assert_f(ratio > 0.9 && ratio < 1.05,
>>>>> "Sysfs RC6 residency counter is inaccurate.\n");
>>>>> } @@ -
>>>> Print gt id in igt_assert_f failed message .
>>>> Thanks,
>>>> Anshuman Gupta.
>>> I thought maybe giving out the gt id as part of info would be
>>> useful, but sure
>>> I'll move it to the assert.
>> IMO it will be noticeable as part of failure message, but if you can
>> think it is more readable
>> in igt_info message. Please keep it in at igt_info.
>> Thanks,
>> Anshuman Gupta.
>
> Sure I will take a call as I am splitting out the series.
>
> Thanks,
>
> Suja
Hi Anshuman,
The gt id looks more cleaner and more visible in the gt info message. So
I have left it there for the the next series.
Thanks,
Suja
>
>>> Will be better fro debug.
>>>
>>> Thanks for the review.
>>>
>>> Suja
>>>
>>>>> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
>>>>> return ts.tv_sec * 1000 + ts.tv_nsec / 1000000; }
>>>>>
>>>>> -static void read_residencies(int devid, unsigned int mask,
>>>>> +static void read_residencies(int devid, int dirfd, unsigned int
>>>>> +mask,
>>>>> struct residencies *res)
>>>>> {
>>>>> res->duration = gettime_ms();
>>>>>
>>>>> if (mask & RC6_ENABLED)
>>>>> - res->rc6 = read_rc6_residency("rc6");
>>>>> + res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>>
>>>>> if ((mask & RC6_ENABLED) &&
>>>>> (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
>>>>> - res->media_rc6 = read_rc6_residency("media_rc6");
>>>>> + res->media_rc6 = read_rc6_residency(dirfd,
>>>>> MEDIA_RC6_RESIDENCY_MS);
>>>>>
>>>>> if (mask & RC6P_ENABLED)
>>>>> - res->rc6p = read_rc6_residency("rc6p");
>>>>> + res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
>>>>>
>>>>> if (mask & RC6PP_ENABLED)
>>>>> - res->rc6pp = read_rc6_residency("rc6pp");
>>>>> + res->rc6pp = read_rc6_residency(dirfd,
>>>>> RC6PP_RESIDENCY_MS);
>>>>>
>>>>> res->duration += (gettime_ms() - res->duration) / 2; }
>>>>>
>>>>> -static void measure_residencies(int devid, unsigned int mask,
>>>>> +static void measure_residencies(int devid, int dirfd, unsigned int
>>>>> +mask,
>>>>> struct residencies *res)
>>>>> {
>>>>> struct residencies start = { };
>>>>> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
>>>>> unsigned int mask,
>>>>> * measurement, since the valid counter range is different on
>>>>> * different platforms and so fixing it up would be
>>>>> non-trivial.
>>>>> */
>>>>> - read_residencies(devid, mask, &end);
>>>>> + read_residencies(devid, dirfd, mask, &end);
>>>>> igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>> end.duration, end.rc6, end.media_rc6, end.rc6p,
>>>>> end.rc6pp);
>>>>> for (retry = 0; retry < 2; retry++) {
>>>>> start = end;
>>>>> sleep(SLEEP_DURATION);
>>>>> - read_residencies(devid, mask, &end);
>>>>> + read_residencies(devid, dirfd, mask, &end);
>>>>>
>>>>> igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>> end.duration,
>>>>> @@ -196,7 +196,7 @@ static void measure_residencies(int devid,
>>>>> unsigned int mask,
>>>>> res->rc6 += res->rc6p;
>>>>> }
>>>>>
>>>>> -static bool wait_for_rc6(void)
>>>>> +static bool wait_for_rc6(int dirfd)
>>>>> {
>>>>> struct timespec tv = {};
>>>>> unsigned long start, now;
>>>>> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
>>>>> usleep(160 * 1000);
>>>>>
>>>>> /* Then poll for RC6 to start ticking */
>>>>> - now = read_rc6_residency("rc6");
>>>>> + now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>> do {
>>>>> start = now;
>>>>> usleep(5000);
>>>>> - now = read_rc6_residency("rc6");
>>>>> + now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>> if (now - start > 1)
>>>>> return true;
>>>>> } while (!igt_seconds_elapsed(&tv)); @@ -234,14 +234,27 @@
>>>>> static
>>>>> uint64_t pmu_read_single(int fd)
>>>>> return __pmu_read_single(fd, NULL);
>>>>> }
>>>>>
>>>>> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
>>>>> - igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
>>>>> - (x) >= (ref) * (1.0 - (tol_down)/100.), \
>>>>> - "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
>>>>> %.3g)\n",\
>>>>> - #x, #ref, (double)(x), (tol_up), (tol_down),
>>>>> (double)(ref))
>>>>> +#define __assert_within_epsilon(x, ref, tol_up, tol_down,
>>>>> debug_data) \
>>>>> + igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref)
>>>>> && \
>>>>> + (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
>>>>> + "'%s' != '%s' (%f not within +%.1f%%/-%.1f%%
>>>>> tolerance of
>>>>> %f)\n %s\n",\
>>>>> + #x, #ref, (double)(x), \
>>>>> + (tol_up) * 100.0, (tol_down) * 100.0, \
>>>>> + (double)(ref), debug_data)
>>>>> +
>>>>> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
>>>>> + __assert_within_epsilon(x, ref, tolerance, tolerance,
>>>>> debug_data)
>>>>>
>>>>> -#define assert_within_epsilon(x, ref, tolerance) \
>>>>> - __assert_within_epsilon(x, ref, tolerance, tolerance)
>>>>> +char *drpc;
>>>>> +
>>>>> +static char *get_drpc(int i915, int gt_id) {
>>>>> + int gt_dir;
>>>>> +
>>>>> + gt_dir = igt_debugfs_gt_dir(i915, gt_id);
>>>>> + igt_assert(gt_dir != -1);
>>>>> + return igt_sysfs_get(gt_dir, "drpc"); }
>>>>>
>>>>> static bool __pmu_wait_for_rc6(int fd)
>>>>> {
>>>>> @@ -376,7 +389,7 @@ static void kill_children(int sig)
>>>>> signal(sig, old);
>>>>> }
>>>>>
>>>>> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>>> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
>>>>> +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!
>>>>> */ @@ -
>>>>> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>>> uint64_t
>>> flags)
>>>>> struct igt_power gpu;
>>>>> int fd;
>>>>>
>>>>> - fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>>> + fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>> igt_drop_caches_set(i915, DROP_IDLE);
>>>>> igt_require(__pmu_wait_for_rc6(fd));
>>>>> igt_power_open(i915, &gpu, "gpu"); @@ -418,7 +431,10 @@ static
>>>>> void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>>> "Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>> idle, (idle * 1e9) / slept);
>>>>> }
>>>>> - assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>>> +
>>>>> + drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>>> +
>>>>> + assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>>
>>>>> done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
>>> 1, 0);
>>>>> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>>> uint64_t flags)
>>>>> igt_assert(cycles >= SLEEP_DURATION);
>>>>>
>>>>> /* While very nearly idle, expect full RC6 */
>>>>> - assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>>> + assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>>> +
>>>>> + free(drpc);
>>>>> + drpc = NULL;
>>>>> }
>>>>>
>>>>> munmap(done, 4096);
>>>>> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>>> uint64_t flags)
>>>>> }
>>>>> }
>>>>>
>>>>> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>>> +static void rc6_fence(int i915, 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! */
>>>>> const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>>>>> const struct intel_execution_engine2 *e;
>>>>> + const intel_ctx_t *ctx;
>>>>> struct power_sample sample[2];
>>>>> unsigned long slept;
>>>>> uint64_t rc6, ts[2], ahnd;
>>>>> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>>> *ctx)
>>>>>
>>>>> igt_require_sw_sync();
>>>>>
>>>>> - fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>>> + fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>> igt_drop_caches_set(i915, DROP_IDLE);
>>>>> igt_require(__pmu_wait_for_rc6(fd));
>>>>> igt_power_open(i915, &gpu, "gpu"); @@ -506,9 +526,12 @@ static
>>>>> void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>>> "Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>> idle, (idle * 1e9) / slept);
>>>>> }
>>>>> - assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>>> + drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>>> +
>>>>> + assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>>
>>>>> /* Submit but delay execution, we should be idle and
>>>>> conserving
>>>>> power */
>>>>> + ctx = intel_ctx_create_for_gt(i915, gt);
>>>>> ahnd = get_reloc_ahnd(i915, ctx->id);
>>>>> for_each_ctx_engine(i915, ctx, e) {
>>>>> igt_spin_t *spin;
>>>>> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const
>>>>> intel_ctx_t
>>>>> *ctx)
>>>>>
>>>>> close(timeline);
>>>>>
>>>>> - assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>>> + assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>>> gem_quiescent_gpu(i915);
>>>>> +
>>>>> + free(drpc);
>>>>> + drpc = NULL;
>>>>> }
>>>>> put_ahnd(ahnd);
>>>>> + intel_ctx_destroy(i915,ctx);
>>>>>
>>>>> igt_power_close(&gpu);
>>>>> close(fd);
>>>>> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>>> *ctx) igt_main {
>>>>> int i915 = -1;
>>>>> + unsigned int dirfd, gt;
>>>>> const intel_ctx_t *ctx;
>>>>>
>>>>> /* Use drm_open_driver to verify device existence */ @@
>>>>> -572,19
>>>>> +600,25 @@ igt_main
>>>>> igt_require_gem(i915);
>>>>> gem_quiescent_gpu(i915);
>>>>>
>>>>> - for_each_ctx_engine(i915, ctx, e) {
>>>>> - if (e->instance == 0) {
>>>>> - igt_dynamic_f("%s", e->name)
>>>>> - rc6_idle(i915, ctx->id, e->flags);
>>>>> + 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);
>>>>> + }
>>>>> }
>>>>> + intel_ctx_destroy(i915, ctx);
>>>>> }
>>>>> }
>>>>>
>>>>> - igt_subtest("rc6-fence") {
>>>>> + igt_subtest_with_dynamic("rc6-fence") {
>>>>> igt_require_gem(i915);
>>>>> gem_quiescent_gpu(i915);
>>>>>
>>>>> - rc6_fence(i915, ctx);
>>>>> + i915_for_each_gt(i915, dirfd, gt)
>>>>> + igt_dynamic_f("gt%u", gt)
>>>>> + rc6_fence(i915, gt);
>>>>> }
>>>>>
>>>>> igt_subtest_group {
>>>>> @@ -595,21 +629,23 @@ igt_main
>>>>> devid = intel_get_drm_devid(i915);
>>>>> sysfs = igt_sysfs_open(i915);
>>>>>
>>>>> - igt_require(has_rc6_residency("rc6"));
>>>>> + igt_require(has_rc6_residency(dirfd,
>>>>> RC6_RESIDENCY_MS));
>>>>>
>>>>> /* Make sure rc6 counters are running */
>>>>> igt_drop_caches_set(i915, DROP_IDLE);
>>>>> - igt_require(wait_for_rc6());
>>>>> + igt_require(wait_for_rc6(dirfd));
>>>>>
>>>>> rc6_enabled = get_rc6_enabled_mask();
>>>>> igt_require(rc6_enabled & RC6_ENABLED);
>>>>> }
>>>>>
>>>>> igt_subtest("rc6-accuracy") {
>>>>> - struct residencies res;
>>>>> + for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>>>>> + struct residencies res;
>>>>>
>>>>> - measure_residencies(devid, rc6_enabled, &res);
>>>>> - residency_accuracy(res.rc6, res.duration, "rc6");
>>>>> + measure_residencies(devid, dirfd,
>>>>> rc6_enabled, &res);
>>>>> + residency_accuracy(res.rc6, res.duration,
>>>>> "rc6");
>>>>> + }
>>>>> }
>>>>>
>>>>> igt_subtest("media-rc6-accuracy") { @@ -617,7 +653,7 @@
>>> igt_main
>>>>> igt_require(IS_VALLEYVIEW(devid) ||
>>> IS_CHERRYVIEW(devid));
>>>>> - measure_residencies(devid, rc6_enabled, &res);
>>>>> + measure_residencies(devid, sysfs, rc6_enabled, &res);
>>>>> residency_accuracy(res.media_rc6, res.duration,
>>> "media_rc6");
>>>>> }
>>>>>
>>>>> @@ -626,7 +662,7 @@ igt_main
>>>>> }
>>>>>
>>>>> igt_fixture {
>>>>> - intel_ctx_destroy(i915, ctx);
>>>>> + free(drpc);
>>>>> drm_close_driver(i915);
>>>>> }
>>>>> }
>>>>> --
>>>>> 2.25.1
More information about the igt-dev
mailing list