* [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
@ 2017-09-07 12:15 Katarzyna Dec
2017-09-07 12:25 ` Szwichtenberg, Radoslaw
` (8 more replies)
0 siblings, 9 replies; 14+ messages in thread
From: Katarzyna Dec @ 2017-09-07 12:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Added comments in tricky places for better feature understanding.
Added IGT_TEST_DESCRIPTION and short description for non-obvious
subtests.
Changed name of 'magic' checkit() function to something meaningfull.
Changed junk struct and stuff array names.
Made some minor coding style changes.
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
---
tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 64 insertions(+), 44 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index e79f0ea7..1eeb6c6a 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -40,6 +40,8 @@
#include "intel_bufmgr.h"
+IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
+
static int drm_fd;
static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
@@ -56,12 +58,19 @@ enum {
static int origfreqs[NUMFREQ];
-struct junk {
+struct sysfs_file {
const char *name;
const char *mode;
FILE *filp;
-} stuff[] = {
- { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
+} sysfs_files[] = {
+ { "cur", "r", NULL },
+ { "min", "rb+", NULL },
+ { "max", "rb+", NULL },
+ { "RP0", "r", NULL },
+ { "RP1", "r", NULL },
+ { "RPn", "r", NULL },
+ { "boost", "rb+", NULL },
+ { NULL, NULL, NULL }
};
static int readval(FILE *filp)
@@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
int i;
for (i = 0; i < NUMFREQ; i++)
- freqs[i] = readval(stuff[i].filp);
+ freqs[i] = readval(sysfs_files[i].filp);
}
static void nsleep(unsigned long ns)
@@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
#define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
-static void checkit(const int *freqs)
+static void check_freq_constraints(const int *freqs)
{
igt_assert_lte(freqs[MIN], freqs[MAX]);
igt_assert_lte(freqs[CUR], freqs[MAX]);
@@ -162,7 +171,7 @@ static void dump(const int *freqs)
igt_debug("gt freq (MHz):");
for (i = 0; i < NUMFREQ; i++)
- igt_debug(" %s=%d", stuff[i].name, freqs[i]);
+ igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
igt_debug("\n");
}
@@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
idx = MAX;
old_freq = freqs[idx];
- writeval_nocheck(stuff[idx].filp, target);
+ writeval_nocheck(sysfs_files[idx].filp, target);
read_freqs(freqs);
ret = freqs[idx];
- writeval_nocheck(stuff[idx].filp, old_freq);
+ writeval_nocheck(sysfs_files[idx].filp, old_freq);
return ret;
}
+/*
+ * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
+ * on subtest run different check after each modification.
+ */
static void min_max_config(void (*check)(void), bool load_gpu)
{
int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
@@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
check();
igt_debug("\nSet min=RPn and max=RP0...\n");
- writeval(stuff[MIN].filp, origfreqs[RPn]);
- writeval(stuff[MAX].filp, origfreqs[RP0]);
+ writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
+ writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
if (load_gpu)
do_load_gpu();
check();
igt_debug("\nIncrease min to midpoint...\n");
- writeval(stuff[MIN].filp, fmid);
+ writeval(sysfs_files[MIN].filp, fmid);
if (load_gpu)
do_load_gpu();
check();
igt_debug("\nIncrease min to RP0...\n");
- writeval(stuff[MIN].filp, origfreqs[RP0]);
+ writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
if (load_gpu)
do_load_gpu();
check();
igt_debug("\nIncrease min above RP0 (invalid)...\n");
- writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
+ writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
check();
igt_debug("\nDecrease max to RPn (invalid)...\n");
- writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
+ writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
check();
igt_debug("\nDecrease min to midpoint...\n");
- writeval(stuff[MIN].filp, fmid);
+ writeval(sysfs_files[MIN].filp, fmid);
if (load_gpu)
do_load_gpu();
check();
igt_debug("\nDecrease min to RPn...\n");
- writeval(stuff[MIN].filp, origfreqs[RPn]);
+ writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
if (load_gpu)
do_load_gpu();
check();
igt_debug("\nDecrease min below RPn (invalid)...\n");
- writeval_inval(stuff[MIN].filp, 0);
+ writeval_inval(sysfs_files[MIN].filp, 0);
check();
igt_debug("\nDecrease max to midpoint...\n");
- writeval(stuff[MAX].filp, fmid);
+ writeval(sysfs_files[MAX].filp, fmid);
check();
igt_debug("\nDecrease max to RPn...\n");
- writeval(stuff[MAX].filp, origfreqs[RPn]);
+ writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
check();
igt_debug("\nDecrease max below RPn (invalid)...\n");
- writeval_inval(stuff[MAX].filp, 0);
+ writeval_inval(sysfs_files[MAX].filp, 0);
check();
igt_debug("\nIncrease min to RP0 (invalid)...\n");
- writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
+ writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
check();
igt_debug("\nIncrease max to midpoint...\n");
- writeval(stuff[MAX].filp, fmid);
+ writeval(sysfs_files[MAX].filp, fmid);
check();
igt_debug("\nIncrease max to RP0...\n");
- writeval(stuff[MAX].filp, origfreqs[RP0]);
+ writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
check();
igt_debug("\nIncrease max above RP0 (invalid)...\n");
- writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
+ writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
check();
- writeval(stuff[MIN].filp, origfreqs[MIN]);
- writeval(stuff[MAX].filp, origfreqs[MAX]);
+ writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
+ writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
}
static void basic_check(void)
@@ -491,7 +504,7 @@ static void basic_check(void)
read_freqs(freqs);
dump(freqs);
- checkit(freqs);
+ check_freq_constraints(freqs);
}
#define IDLE_WAIT_TIMESTEP_MSEC 250
@@ -506,7 +519,7 @@ static void idle_check(void)
do {
read_freqs(freqs);
dump(freqs);
- checkit(freqs);
+ check_freq_constraints(freqs);
if (freqs[CUR] == freqs[RPn])
break;
usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
@@ -529,7 +542,7 @@ static void loaded_check(void)
do {
read_freqs(freqs);
dump(freqs);
- checkit(freqs);
+ check_freq_constraints(freqs);
if (freqs[CUR] >= freqs[MAX])
break;
usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
@@ -547,6 +560,8 @@ static void stabilize_check(int *out)
int freqs[NUMFREQ];
int wait = 0;
+ /* Monitor frequencies until HW will stabilize cur frequency.
+ * It should happen within allotted time */
read_freqs(freqs);
dump(freqs);
usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
@@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
fmid = get_hw_rounded_freq(fmid);
/* Set max freq to less then boost freq */
- writeval(stuff[MAX].filp, fmid);
+ writeval(sysfs_files[MAX].filp, fmid);
/* Put boost on the same engine as low load */
engine = I915_EXEC_RENDER;
@@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
igt_spin_batch_free(fd, load);
/* Set max freq to original softmax */
- writeval(stuff[MAX].filp, origfreqs[MAX]);
+ writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
}
static void waitboost(int fd, bool reset)
@@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
static void pm_rps_exit_handler(int sig)
{
- if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
- writeval(stuff[MAX].filp, origfreqs[MAX]);
- writeval(stuff[MIN].filp, origfreqs[MIN]);
+ if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
+ writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
+ writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
} else {
- writeval(stuff[MIN].filp, origfreqs[MIN]);
- writeval(stuff[MAX].filp, origfreqs[MAX]);
+ writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
+ writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
}
load_helper_deinit();
@@ -652,7 +667,7 @@ igt_main
igt_fixture {
const int device = drm_get_card();
- struct junk *junk = stuff;
+ struct sysfs_file *sysfs_file = sysfs_files;
int ret;
/* Use drm_open_driver to verify device existence */
@@ -663,16 +678,17 @@ igt_main
do {
int val = -1;
char *path;
- ret = asprintf(&path, sysfs_base_path, device, junk->name);
+
+ ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
igt_assert(ret != -1);
- junk->filp = fopen(path, junk->mode);
- igt_require(junk->filp);
- setbuf(junk->filp, NULL);
+ sysfs_file->filp = fopen(path, sysfs_file->mode);
+ igt_require(sysfs_file->filp);
+ setbuf(sysfs_file->filp, NULL);
- val = readval(junk->filp);
+ val = readval(sysfs_file->filp);
igt_assert(val >= 0);
- junk++;
- } while (junk->name != NULL);
+ sysfs_file++;
+ } while (sysfs_file->name != NULL);
read_freqs(origfreqs);
@@ -684,18 +700,22 @@ igt_main
igt_subtest("basic-api")
min_max_config(basic_check, false);
+ /* Verify the constraints, check if we can reach idle */
igt_subtest("min-max-config-idle")
min_max_config(idle_check, true);
+ /* Verify the constraints with high load, check if we can reach max */
igt_subtest("min-max-config-loaded") {
load_helper_run(HIGH);
min_max_config(loaded_check, false);
load_helper_stop();
}
+ /* Checks if we achieve boost using gem_wait */
igt_subtest("waitboost")
waitboost(drm_fd, false);
+ /* Test boost frequency after GPU reset */
igt_subtest("reset") {
igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
waitboost(drm_fd, true);
--
2.13.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
@ 2017-09-07 12:25 ` Szwichtenberg, Radoslaw
2017-09-07 12:38 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Szwichtenberg, Radoslaw @ 2017-09-07 12:25 UTC (permalink / raw)
To: Dec, Katarzyna, intel-gfx@lists.freedesktop.org; +Cc: daniel.vetter@ffwll.ch
On Thu, 2017-09-07 at 14:15 +0200, Katarzyna Dec wrote:
> Added comments in tricky places for better feature understanding.
> Added IGT_TEST_DESCRIPTION and short description for non-obvious
> subtests.
> Changed name of 'magic' checkit() function to something meaningfull.
> Changed junk struct and stuff array names.
> Made some minor coding style changes.
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
Acked-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
2017-09-07 12:25 ` Szwichtenberg, Radoslaw
@ 2017-09-07 12:38 ` Patchwork
2017-09-07 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-09-07 12:38 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx
== Series Details ==
Series: pm_rps: [RFC] RPS tests documentation update
URL : https://patchwork.freedesktop.org/series/29947/
State : failure
== Summary ==
IGT patchset tested on top of latest successful build
4a1c8daff2005e2cbfe980d63bc0a0fb09cb017d igt/gem_ringfill: Prime execbuf before measuring ring size
with latest DRM-Tip kernel build CI_DRM_3054
00f9b49384df drm-tip: 2017y-09m-07d-09h-58m-50s UTC integration manifest
Test chamelium:
Subgroup vga-hpd-fast:
skip -> FAIL (fi-skl-6700k)
Subgroup common-hpd-after-suspend:
pass -> DMESG-WARN (fi-skl-6700k)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
Test drv_module_reload:
Subgroup basic-reload-inject:
dmesg-fail -> DMESG-WARN (fi-cfl-s) k.org#196765
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:456s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:360s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:562s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:253s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:524s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:528s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:521s
fi-cfl-s total:289 pass:249 dwarn:4 dfail:0 fail:0 skip:36 time:481s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:445s
fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:624s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:453s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:427s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:511s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:484s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:521s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:604s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:598s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:527s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:474s
fi-skl-6700k total:289 pass:264 dwarn:1 dfail:0 fail:1 skip:23 time:559s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:518s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:442s
fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:496s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:560s
fi-snb-2600 total:289 pass:248 dwarn:0 dfail:0 fail:2 skip:39 time:410s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_157/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
2017-09-07 12:25 ` Szwichtenberg, Radoslaw
2017-09-07 12:38 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-09-07 13:13 ` Patchwork
2017-09-07 13:43 ` [PATCH i-g-t] " Arkadiusz Hiler
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-09-07 13:13 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx
== Series Details ==
Series: pm_rps: [RFC] RPS tests documentation update
URL : https://patchwork.freedesktop.org/series/29947/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
4a1c8daff2005e2cbfe980d63bc0a0fb09cb017d igt/gem_ringfill: Prime execbuf before measuring ring size
with latest DRM-Tip kernel build CI_DRM_3054
00f9b49384df drm-tip: 2017y-09m-07d-09h-58m-50s UTC integration manifest
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
fail -> PASS (fi-snb-2600) fdo#100215
Test pm_rpm:
Subgroup basic-pci-d3-state:
skip -> PASS (fi-cfl-s) fdo#102294
Test drv_module_reload:
Subgroup basic-reload-inject:
dmesg-fail -> DMESG-WARN (fi-cfl-s) k.org#196765
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:471s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:438s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:368s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:561s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:257s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:527s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:533s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:525s
fi-cfl-s total:289 pass:250 dwarn:4 dfail:0 fail:0 skip:35 time:459s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:444s
fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:616s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:447s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:430s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:432s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:506s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:480s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:514s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:604s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:600s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:531s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:543s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:526s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:447s
fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:497s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:571s
fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:411s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_158/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (2 preceding siblings ...)
2017-09-07 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2017-09-07 13:43 ` Arkadiusz Hiler
2017-09-07 15:10 ` ✓ Fi.CI.IGT: success for " Patchwork
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Arkadiusz Hiler @ 2017-09-07 13:43 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx, Daniel Vetter
On Thu, Sep 07, 2017 at 02:15:14PM +0200, Katarzyna Dec wrote:
> Added comments in tricky places for better feature understanding.
> Added IGT_TEST_DESCRIPTION and short description for non-obvious
> subtests.
> Changed name of 'magic' checkit() function to something meaningfull.
> Changed junk struct and stuff array names.
> Made some minor coding style changes.
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
> tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 64 insertions(+), 44 deletions(-)
>
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index e79f0ea7..1eeb6c6a 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -40,6 +40,8 @@
>
> #include "intel_bufmgr.h"
>
> +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
> +
> static int drm_fd;
>
> static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
> @@ -56,12 +58,19 @@ enum {
>
> static int origfreqs[NUMFREQ];
>
> -struct junk {
> +struct sysfs_file {
> const char *name;
> const char *mode;
> FILE *filp;
> -} stuff[] = {
> - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
> +} sysfs_files[] = {
> + { "cur", "r", NULL },
> + { "min", "rb+", NULL },
> + { "max", "rb+", NULL },
> + { "RP0", "r", NULL },
> + { "RP1", "r", NULL },
> + { "RPn", "r", NULL },
> + { "boost", "rb+", NULL },
> + { NULL, NULL, NULL }
> };
>
> static int readval(FILE *filp)
> @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
> int i;
>
> for (i = 0; i < NUMFREQ; i++)
> - freqs[i] = readval(stuff[i].filp);
> + freqs[i] = readval(sysfs_files[i].filp);
> }
>
> static void nsleep(unsigned long ns)
> @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
> #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
>
> -static void checkit(const int *freqs)
> +static void check_freq_constraints(const int *freqs)
> {
> igt_assert_lte(freqs[MIN], freqs[MAX]);
> igt_assert_lte(freqs[CUR], freqs[MAX]);
> @@ -162,7 +171,7 @@ static void dump(const int *freqs)
>
> igt_debug("gt freq (MHz):");
> for (i = 0; i < NUMFREQ; i++)
> - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
> + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
>
> igt_debug("\n");
> }
> @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
> idx = MAX;
>
> old_freq = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, target);
> + writeval_nocheck(sysfs_files[idx].filp, target);
> read_freqs(freqs);
> ret = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, old_freq);
> + writeval_nocheck(sysfs_files[idx].filp, old_freq);
>
> return ret;
> }
>
> +/*
> + * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
> + * on subtest run different check after each modification.
> + */
> static void min_max_config(void (*check)(void), bool load_gpu)
> {
> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
> check();
>
> igt_debug("\nSet min=RPn and max=RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min above RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
> check();
>
> igt_debug("\nDecrease max to RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min to RPn...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min below RPn (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, 0);
> + writeval_inval(sysfs_files[MIN].filp, 0);
> check();
>
> igt_debug("\nDecrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nDecrease max to RPn...\n");
> - writeval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease max below RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, 0);
> + writeval_inval(sysfs_files[MAX].filp, 0);
> check();
>
> igt_debug("\nIncrease min to RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nIncrease max to RP0...\n");
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max above RP0 (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
> check();
>
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void basic_check(void)
> @@ -491,7 +504,7 @@ static void basic_check(void)
>
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> }
>
> #define IDLE_WAIT_TIMESTEP_MSEC 250
> @@ -506,7 +519,7 @@ static void idle_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] == freqs[RPn])
> break;
> usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
> @@ -529,7 +542,7 @@ static void loaded_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] >= freqs[MAX])
> break;
> usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
> @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
> int freqs[NUMFREQ];
> int wait = 0;
>
> + /* Monitor frequencies until HW will stabilize cur frequency.
> + * It should happen within allotted time */
> read_freqs(freqs);
> dump(freqs);
> usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
> @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
>
> fmid = get_hw_rounded_freq(fmid);
> /* Set max freq to less then boost freq */
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
>
> /* Put boost on the same engine as low load */
> engine = I915_EXEC_RENDER;
> @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
> igt_spin_batch_free(fd, load);
>
> /* Set max freq to original softmax */
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void waitboost(int fd, bool reset)
> @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
>
> static void pm_rps_exit_handler(int sig)
> {
> - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> } else {
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> load_helper_deinit();
> @@ -652,7 +667,7 @@ igt_main
>
> igt_fixture {
> const int device = drm_get_card();
> - struct junk *junk = stuff;
> + struct sysfs_file *sysfs_file = sysfs_files;
> int ret;
>
> /* Use drm_open_driver to verify device existence */
> @@ -663,16 +678,17 @@ igt_main
> do {
> int val = -1;
> char *path;
> - ret = asprintf(&path, sysfs_base_path, device, junk->name);
> +
> + ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
> igt_assert(ret != -1);
> - junk->filp = fopen(path, junk->mode);
> - igt_require(junk->filp);
> - setbuf(junk->filp, NULL);
> + sysfs_file->filp = fopen(path, sysfs_file->mode);
> + igt_require(sysfs_file->filp);
> + setbuf(sysfs_file->filp, NULL);
>
> - val = readval(junk->filp);
> + val = readval(sysfs_file->filp);
> igt_assert(val >= 0);
> - junk++;
> - } while (junk->name != NULL);
> + sysfs_file++;
> + } while (sysfs_file->name != NULL);
>
> read_freqs(origfreqs);
>
> @@ -684,18 +700,22 @@ igt_main
> igt_subtest("basic-api")
> min_max_config(basic_check, false);
>
> + /* Verify the constraints, check if we can reach idle */
Nitpick: either end the sentence with a comma or don't start it with
an uppercase letter.
Other than that - looks good to me. It's everything I had in mind when
discussing with Vinay.
You've commented on the surprising parts (e.g. waitboost does not wait
for boost but causes it using gem_wait, therefore the name).
You've renamed the oddities (checkit, junk) to something more meaningful
instead of bloating them with comments.
And you have not overdid it, withholding on adding comments in places
where they would be just redundant.
IMHO pm_rps can serve as a good example of how things should look like.
Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
But let's wait and see what others (at least those CCed) have to say
before popping a champagne on this.
--
Cheers,
Arek
> igt_subtest("min-max-config-idle")
> min_max_config(idle_check, true);
>
> + /* Verify the constraints with high load, check if we can reach max */
> igt_subtest("min-max-config-loaded") {
> load_helper_run(HIGH);
> min_max_config(loaded_check, false);
> load_helper_stop();
> }
>
> + /* Checks if we achieve boost using gem_wait */
> igt_subtest("waitboost")
> waitboost(drm_fd, false);
>
> + /* Test boost frequency after GPU reset */
> igt_subtest("reset") {
> igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> waitboost(drm_fd, true);
> --
> 2.13.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.IGT: success for pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (3 preceding siblings ...)
2017-09-07 13:43 ` [PATCH i-g-t] " Arkadiusz Hiler
@ 2017-09-07 15:10 ` Patchwork
2017-09-07 15:53 ` Patchwork
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-09-07 15:10 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx
== Series Details ==
Series: pm_rps: [RFC] RPS tests documentation update
URL : https://patchwork.freedesktop.org/series/29947/
State : success
== Summary ==
Test perf:
Subgroup polling:
pass -> FAIL (shard-hsw) fdo#102252
Test pm_lpsp:
Subgroup screens-disabled:
fail -> PASS (shard-hsw) fdo#102579
Test kms_atomic_transition:
Subgroup plane-use-after-nonblocking-unbind:
incomplete -> FAIL (shard-hsw) fdo#101847
Test kms_setmode:
Subgroup basic:
fail -> PASS (shard-hsw) fdo#99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102579 https://bugs.freedesktop.org/show_bug.cgi?id=102579
fdo#101847 https://bugs.freedesktop.org/show_bug.cgi?id=101847
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-hsw total:2265 pass:1236 dwarn:0 dfail:0 fail:13 skip:1016 time:9626s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_157/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.IGT: success for pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (4 preceding siblings ...)
2017-09-07 15:10 ` ✓ Fi.CI.IGT: success for " Patchwork
@ 2017-09-07 15:53 ` Patchwork
2017-09-07 18:28 ` [PATCH i-g-t] " Belgaumkar, Vinay
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-09-07 15:53 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx
== Series Details ==
Series: pm_rps: [RFC] RPS tests documentation update
URL : https://patchwork.freedesktop.org/series/29947/
State : success
== Summary ==
Test perf:
Subgroup polling:
pass -> FAIL (shard-hsw) fdo#102252
Test kms_atomic_transition:
Subgroup plane-use-after-nonblocking-unbind:
incomplete -> FAIL (shard-hsw) fdo#101847
Test pm_lpsp:
Subgroup screens-disabled:
fail -> PASS (shard-hsw) fdo#102579
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#101847 https://bugs.freedesktop.org/show_bug.cgi?id=101847
fdo#102579 https://bugs.freedesktop.org/show_bug.cgi?id=102579
shard-hsw total:2265 pass:1235 dwarn:0 dfail:0 fail:14 skip:1016 time:9686s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_158/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (5 preceding siblings ...)
2017-09-07 15:53 ` Patchwork
@ 2017-09-07 18:28 ` Belgaumkar, Vinay
2017-09-08 7:19 ` Katarzyna Dec
2017-09-08 7:30 ` Szwichtenberg, Radoslaw
2017-09-12 11:28 ` Petri Latvala
2017-09-12 11:49 ` Arkadiusz Hiler
8 siblings, 2 replies; 14+ messages in thread
From: Belgaumkar, Vinay @ 2017-09-07 18:28 UTC (permalink / raw)
To: Katarzyna Dec, intel-gfx; +Cc: Daniel Vetter
On 9/7/2017 5:15 AM, Katarzyna Dec wrote:
> Added comments in tricky places for better feature understanding.
> Added IGT_TEST_DESCRIPTION and short description for non-obvious
> subtests.
> Changed name of 'magic' checkit() function to something meaningfull.
> Changed junk struct and stuff array names.
> Made some minor coding style changes.
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
> tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 64 insertions(+), 44 deletions(-)
>
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index e79f0ea7..1eeb6c6a 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -40,6 +40,8 @@
>
> #include "intel_bufmgr.h"
>
> +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
> +
> static int drm_fd;
>
> static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
> @@ -56,12 +58,19 @@ enum {
>
> static int origfreqs[NUMFREQ];
>
> -struct junk {
> +struct sysfs_file {
> const char *name;
> const char *mode;
> FILE *filp;
> -} stuff[] = {
> - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
> +} sysfs_files[] = {
> + { "cur", "r", NULL },
> + { "min", "rb+", NULL },
> + { "max", "rb+", NULL },
> + { "RP0", "r", NULL },
> + { "RP1", "r", NULL },
> + { "RPn", "r", NULL },
> + { "boost", "rb+", NULL },
> + { NULL, NULL, NULL }
> };
>
> static int readval(FILE *filp)
> @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
> int i;
>
> for (i = 0; i < NUMFREQ; i++)
> - freqs[i] = readval(stuff[i].filp);
> + freqs[i] = readval(sysfs_files[i].filp);
> }
>
> static void nsleep(unsigned long ns)
> @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
> #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
>
> -static void checkit(const int *freqs)
> +static void check_freq_constraints(const int *freqs)
> {
> igt_assert_lte(freqs[MIN], freqs[MAX]);
> igt_assert_lte(freqs[CUR], freqs[MAX]);
> @@ -162,7 +171,7 @@ static void dump(const int *freqs)
>
> igt_debug("gt freq (MHz):");
> for (i = 0; i < NUMFREQ; i++)
> - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
> + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
>
> igt_debug("\n");
> }
> @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
> idx = MAX;
>
> old_freq = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, target);
> + writeval_nocheck(sysfs_files[idx].filp, target);
> read_freqs(freqs);
> ret = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, old_freq);
> + writeval_nocheck(sysfs_files[idx].filp, old_freq);
>
> return ret;
> }
>
> +/*
> + * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
> + * on subtest run different check after each modification.
> + */
> static void min_max_config(void (*check)(void), bool load_gpu)
> {
> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
> check();
>
> igt_debug("\nSet min=RPn and max=RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min above RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
> check();
>
> igt_debug("\nDecrease max to RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min to RPn...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min below RPn (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, 0);
> + writeval_inval(sysfs_files[MIN].filp, 0);
> check();
>
> igt_debug("\nDecrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nDecrease max to RPn...\n");
> - writeval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease max below RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, 0);
> + writeval_inval(sysfs_files[MAX].filp, 0);
> check();
>
> igt_debug("\nIncrease min to RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nIncrease max to RP0...\n");
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max above RP0 (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
> check();
>
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void basic_check(void)
> @@ -491,7 +504,7 @@ static void basic_check(void)
>
> read_freqs(freqs);
> dump(freqs);
Almost feel like we should create a new function read_and_dump_freqs()
instead of calling 2 functions multiple times. We should keep the old
read_freqs() since it is called in a loop on one occasion. Something to
think about maybe?
> - checkit(freqs);
> + check_freq_constraints(freqs);
> }
>
> #define IDLE_WAIT_TIMESTEP_MSEC 250
> @@ -506,7 +519,7 @@ static void idle_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] == freqs[RPn])
> break;
> usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
> @@ -529,7 +542,7 @@ static void loaded_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] >= freqs[MAX])
> break;
> usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
> @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
> int freqs[NUMFREQ];
> int wait = 0;
>
> + /* Monitor frequencies until HW will stabilize cur frequency.
> + * It should happen within allotted time */
> read_freqs(freqs);
> dump(freqs);
> usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
> @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
>
> fmid = get_hw_rounded_freq(fmid);
> /* Set max freq to less then boost freq */
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
>
> /* Put boost on the same engine as low load */
> engine = I915_EXEC_RENDER;
> @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
> igt_spin_batch_free(fd, load);
>
> /* Set max freq to original softmax */
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void waitboost(int fd, bool reset)
> @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
>
> static void pm_rps_exit_handler(int sig)
> {
> - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> } else {
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> load_helper_deinit();
> @@ -652,7 +667,7 @@ igt_main
>
> igt_fixture {
> const int device = drm_get_card();
> - struct junk *junk = stuff;
> + struct sysfs_file *sysfs_file = sysfs_files;
> int ret;
>
> /* Use drm_open_driver to verify device existence */
> @@ -663,16 +678,17 @@ igt_main
> do {
> int val = -1;
> char *path;
> - ret = asprintf(&path, sysfs_base_path, device, junk->name);
> +
> + ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
> igt_assert(ret != -1);
> - junk->filp = fopen(path, junk->mode);
> - igt_require(junk->filp);
> - setbuf(junk->filp, NULL);
> + sysfs_file->filp = fopen(path, sysfs_file->mode);
> + igt_require(sysfs_file->filp);
> + setbuf(sysfs_file->filp, NULL);
>
> - val = readval(junk->filp);
> + val = readval(sysfs_file->filp);
> igt_assert(val >= 0);
> - junk++;
> - } while (junk->name != NULL);
> + sysfs_file++;
> + } while (sysfs_file->name != NULL);
>
> read_freqs(origfreqs);
>
> @@ -684,18 +700,22 @@ igt_main
> igt_subtest("basic-api")
> min_max_config(basic_check, false);
>
> + /* Verify the constraints, check if we can reach idle */
> igt_subtest("min-max-config-idle")
> min_max_config(idle_check, true);
>
> + /* Verify the constraints with high load, check if we can reach max */
> igt_subtest("min-max-config-loaded") {
> load_helper_run(HIGH);
> min_max_config(loaded_check, false);
> load_helper_stop();
> }
>
> + /* Checks if we achieve boost using gem_wait */
We should mention this is doing gem_wait on a spinning batch, hence the
boost.
Other than this and the above comment, LGTM.
Acked-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> igt_subtest("waitboost")
> waitboost(drm_fd, false);
>
> + /* Test boost frequency after GPU reset */
> igt_subtest("reset") {
> igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> waitboost(drm_fd, true);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 18:28 ` [PATCH i-g-t] " Belgaumkar, Vinay
@ 2017-09-08 7:19 ` Katarzyna Dec
2017-09-08 16:48 ` Belgaumkar, Vinay
2017-09-08 7:30 ` Szwichtenberg, Radoslaw
1 sibling, 1 reply; 14+ messages in thread
From: Katarzyna Dec @ 2017-09-08 7:19 UTC (permalink / raw)
To: Belgaumkar, Vinay, intel-gfx; +Cc: DanielVetter
On Thu, Sep 07, 2017 at 11:28:38AM -0700, Belgaumkar, Vinay wrote:
>
>
> On 9/7/2017 5:15 AM, Katarzyna Dec wrote:
> > Added comments in tricky places for better feature understanding.
> > Added IGT_TEST_DESCRIPTION and short description for non-obvious
> > subtests.
> > Changed name of 'magic' checkit() function to something meaningfull.
> > Changed junk struct and stuff array names.
> > Made some minor coding style changes.
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
> > ---
> > tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 64 insertions(+), 44 deletions(-)
> >
> > diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> > index e79f0ea7..1eeb6c6a 100644
> > --- a/tests/pm_rps.c
> > +++ b/tests/pm_rps.c
> > @@ -40,6 +40,8 @@
> >
> > #include "intel_bufmgr.h"
> >
> > +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
> > +
> > static int drm_fd;
> >
> > static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
> > @@ -56,12 +58,19 @@ enum {
> >
> > static int origfreqs[NUMFREQ];
> >
> > -struct junk {
> > +struct sysfs_file {
> > const char *name;
> > const char *mode;
> > FILE *filp;
> > -} stuff[] = {
> > - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
> > +} sysfs_files[] = {
> > + { "cur", "r", NULL },
> > + { "min", "rb+", NULL },
> > + { "max", "rb+", NULL },
> > + { "RP0", "r", NULL },
> > + { "RP1", "r", NULL },
> > + { "RPn", "r", NULL },
> > + { "boost", "rb+", NULL },
> > + { NULL, NULL, NULL }
> > };
> >
> > static int readval(FILE *filp)
> > @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
> > int i;
> >
> > for (i = 0; i < NUMFREQ; i++)
> > - freqs[i] = readval(stuff[i].filp);
> > + freqs[i] = readval(sysfs_files[i].filp);
> > }
> >
> > static void nsleep(unsigned long ns)
> > @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
> > #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> > #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
> >
> > -static void checkit(const int *freqs)
> > +static void check_freq_constraints(const int *freqs)
> > {
> > igt_assert_lte(freqs[MIN], freqs[MAX]);
> > igt_assert_lte(freqs[CUR], freqs[MAX]);
> > @@ -162,7 +171,7 @@ static void dump(const int *freqs)
> >
> > igt_debug("gt freq (MHz):");
> > for (i = 0; i < NUMFREQ; i++)
> > - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
> > + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
> >
> > igt_debug("\n");
> > }
> > @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
> > idx = MAX;
> >
> > old_freq = freqs[idx];
> > - writeval_nocheck(stuff[idx].filp, target);
> > + writeval_nocheck(sysfs_files[idx].filp, target);
> > read_freqs(freqs);
> > ret = freqs[idx];
> > - writeval_nocheck(stuff[idx].filp, old_freq);
> > + writeval_nocheck(sysfs_files[idx].filp, old_freq);
> >
> > return ret;
> > }
> >
> > +/*
> > + * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
> > + * on subtest run different check after each modification.
> > + */
> > static void min_max_config(void (*check)(void), bool load_gpu)
> > {
> > int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> > @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
> > check();
> >
> > igt_debug("\nSet min=RPn and max=RP0...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RPn]);
> > - writeval(stuff[MAX].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min to midpoint...\n");
> > - writeval(stuff[MIN].filp, fmid);
> > + writeval(sysfs_files[MIN].filp, fmid);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min to RP0...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min above RP0 (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
> > + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
> > check();
> >
> > igt_debug("\nDecrease max to RPn (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
> > + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
> > check();
> >
> > igt_debug("\nDecrease min to midpoint...\n");
> > - writeval(stuff[MIN].filp, fmid);
> > + writeval(sysfs_files[MIN].filp, fmid);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nDecrease min to RPn...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nDecrease min below RPn (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, 0);
> > + writeval_inval(sysfs_files[MIN].filp, 0);
> > check();
> >
> > igt_debug("\nDecrease max to midpoint...\n");
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> > check();
> >
> > igt_debug("\nDecrease max to RPn...\n");
> > - writeval(stuff[MAX].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
> > check();
> >
> > igt_debug("\nDecrease max below RPn (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, 0);
> > + writeval_inval(sysfs_files[MAX].filp, 0);
> > check();
> >
> > igt_debug("\nIncrease min to RP0 (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
> > + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
> > check();
> >
> > igt_debug("\nIncrease max to midpoint...\n");
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> > check();
> >
> > igt_debug("\nIncrease max to RP0...\n");
> > - writeval(stuff[MAX].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> > check();
> >
> > igt_debug("\nIncrease max above RP0 (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
> > + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
> > check();
> >
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > static void basic_check(void)
> > @@ -491,7 +504,7 @@ static void basic_check(void)
> >
> > read_freqs(freqs);
> > dump(freqs);
>
> Almost feel like we should create a new function read_and_dump_freqs()
> instead of calling 2 functions multiple times. We should keep the old
> read_freqs() since it is called in a loop on one occasion. Something to
> think about maybe?
If anybody reviewing this patch feel that it would be better to change
basic_check function name, I will do it :)
Also, as far as I know this function is used only in this file, so there
is no reason to add duplicate it.
>
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > }
> >
> > #define IDLE_WAIT_TIMESTEP_MSEC 250
> > @@ -506,7 +519,7 @@ static void idle_check(void)
> > do {
> > read_freqs(freqs);
> > dump(freqs);
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > if (freqs[CUR] == freqs[RPn])
> > break;
> > usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
> > @@ -529,7 +542,7 @@ static void loaded_check(void)
> > do {
> > read_freqs(freqs);
> > dump(freqs);
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > if (freqs[CUR] >= freqs[MAX])
> > break;
> > usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
> > @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
> > int freqs[NUMFREQ];
> > int wait = 0;
> >
> > + /* Monitor frequencies until HW will stabilize cur frequency.
> > + * It should happen within allotted time */
> > read_freqs(freqs);
> > dump(freqs);
> > usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
> > @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
> >
> > fmid = get_hw_rounded_freq(fmid);
> > /* Set max freq to less then boost freq */
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> >
> > /* Put boost on the same engine as low load */
> > engine = I915_EXEC_RENDER;
> > @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
> > igt_spin_batch_free(fd, load);
> >
> > /* Set max freq to original softmax */
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > static void waitboost(int fd, bool reset)
> > @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
> >
> > static void pm_rps_exit_handler(int sig)
> > {
> > - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > } else {
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > load_helper_deinit();
> > @@ -652,7 +667,7 @@ igt_main
> >
> > igt_fixture {
> > const int device = drm_get_card();
> > - struct junk *junk = stuff;
> > + struct sysfs_file *sysfs_file = sysfs_files;
> > int ret;
> >
> > /* Use drm_open_driver to verify device existence */
> > @@ -663,16 +678,17 @@ igt_main
> > do {
> > int val = -1;
> > char *path;
> > - ret = asprintf(&path, sysfs_base_path, device, junk->name);
> > +
> > + ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
> > igt_assert(ret != -1);
> > - junk->filp = fopen(path, junk->mode);
> > - igt_require(junk->filp);
> > - setbuf(junk->filp, NULL);
> > + sysfs_file->filp = fopen(path, sysfs_file->mode);
> > + igt_require(sysfs_file->filp);
> > + setbuf(sysfs_file->filp, NULL);
> >
> > - val = readval(junk->filp);
> > + val = readval(sysfs_file->filp);
> > igt_assert(val >= 0);
> > - junk++;
> > - } while (junk->name != NULL);
> > + sysfs_file++;
> > + } while (sysfs_file->name != NULL);
> >
> > read_freqs(origfreqs);
> >
> > @@ -684,18 +700,22 @@ igt_main
> > igt_subtest("basic-api")
> > min_max_config(basic_check, false);
> >
> > + /* Verify the constraints, check if we can reach idle */
> > igt_subtest("min-max-config-idle")
> > min_max_config(idle_check, true);
> >
> > + /* Verify the constraints with high load, check if we can reach max */
> > igt_subtest("min-max-config-loaded") {
> > load_helper_run(HIGH);
> > min_max_config(loaded_check, false);
> > load_helper_stop();
> > }
> >
> > + /* Checks if we achieve boost using gem_wait */
>
> We should mention this is doing gem_wait on a spinning batch, hence the
> boost.
I think that additional information is not needed here. I got 2 reasons:
- we assume that subtest comment will be oneliner
- do we need such details here?
Thanks,
Kasia
>
> Other than this and the above comment, LGTM.
>
> Acked-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
>
> > igt_subtest("waitboost")
> > waitboost(drm_fd, false);
> >
> > + /* Test boost frequency after GPU reset */
> > igt_subtest("reset") {
> > igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> > waitboost(drm_fd, true);
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 18:28 ` [PATCH i-g-t] " Belgaumkar, Vinay
2017-09-08 7:19 ` Katarzyna Dec
@ 2017-09-08 7:30 ` Szwichtenberg, Radoslaw
1 sibling, 0 replies; 14+ messages in thread
From: Szwichtenberg, Radoslaw @ 2017-09-08 7:30 UTC (permalink / raw)
To: Dec, Katarzyna, intel-gfx@lists.freedesktop.org,
Belgaumkar, Vinay
Cc: daniel.vetter@ffwll.ch
On Thu, 2017-09-07 at 11:28 -0700, Belgaumkar, Vinay wrote:
>
> On 9/7/2017 5:15 AM, Katarzyna Dec wrote:
> > Added comments in tricky places for better feature understanding.
> > Added IGT_TEST_DESCRIPTION and short description for non-obvious
> > subtests.
> > Changed name of 'magic' checkit() function to something meaningfull.
> > Changed junk struct and stuff array names.
> > Made some minor coding style changes.
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
> > ---
> > tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++------------------
> > -----
> > 1 file changed, 64 insertions(+), 44 deletions(-)
> >
> > diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> > index e79f0ea7..1eeb6c6a 100644
> > --- a/tests/pm_rps.c
> > +++ b/tests/pm_rps.c
> > @@ -40,6 +40,8 @@
> >
> > #include "intel_bufmgr.h"
> >
> > +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency
> > changes");
> > +
> > static int drm_fd;
> >
> > static const char sysfs_base_path[] =
> > "/sys/class/drm/card%d/gt_%s_freq_mhz";
> > @@ -56,12 +58,19 @@ enum {
> >
> > static int origfreqs[NUMFREQ];
> >
> > -struct junk {
> > +struct sysfs_file {
> > const char *name;
> > const char *mode;
> > FILE *filp;
> > -} stuff[] = {
> > - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL
> > }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, {
> > "boost", "rb+", NULL }, { NULL, NULL, NULL }
> > +} sysfs_files[] = {
> > + { "cur", "r", NULL },
> > + { "min", "rb+", NULL },
> > + { "max", "rb+", NULL },
> > + { "RP0", "r", NULL },
> > + { "RP1", "r", NULL },
> > + { "RPn", "r", NULL },
> > + { "boost", "rb+", NULL },
> > + { NULL, NULL, NULL }
> > };
> >
> > static int readval(FILE *filp)
> > @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
> > int i;
> >
> > for (i = 0; i < NUMFREQ; i++)
> > - freqs[i] = readval(stuff[i].filp);
> > + freqs[i] = readval(sysfs_files[i].filp);
> > }
> >
> > static void nsleep(unsigned long ns)
> > @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno,
> > bool readback_check)
> > #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> > #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
> >
> > -static void checkit(const int *freqs)
> > +static void check_freq_constraints(const int *freqs)
> > {
> > igt_assert_lte(freqs[MIN], freqs[MAX]);
> > igt_assert_lte(freqs[CUR], freqs[MAX]);
> > @@ -162,7 +171,7 @@ static void dump(const int *freqs)
> >
> > igt_debug("gt freq (MHz):");
> > for (i = 0; i < NUMFREQ; i++)
> > - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
> > + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
> >
> > igt_debug("\n");
> > }
> > @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
> > idx = MAX;
> >
> > old_freq = freqs[idx];
> > - writeval_nocheck(stuff[idx].filp, target);
> > + writeval_nocheck(sysfs_files[idx].filp, target);
> > read_freqs(freqs);
> > ret = freqs[idx];
> > - writeval_nocheck(stuff[idx].filp, old_freq);
> > + writeval_nocheck(sysfs_files[idx].filp, old_freq);
> >
> > return ret;
> > }
> >
> > +/*
> > + * Modify softlimit MIN and MAX freqs to valid and invalid levels.
> > Depending
> > + * on subtest run different check after each modification.
> > + */
> > static void min_max_config(void (*check)(void), bool load_gpu)
> > {
> > int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> > @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool
> > load_gpu)
> > check();
> >
> > igt_debug("\nSet min=RPn and max=RP0...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RPn]);
> > - writeval(stuff[MAX].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min to midpoint...\n");
> > - writeval(stuff[MIN].filp, fmid);
> > + writeval(sysfs_files[MIN].filp, fmid);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min to RP0...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nIncrease min above RP0 (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
> > + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
> > check();
> >
> > igt_debug("\nDecrease max to RPn (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
> > + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
> > check();
> >
> > igt_debug("\nDecrease min to midpoint...\n");
> > - writeval(stuff[MIN].filp, fmid);
> > + writeval(sysfs_files[MIN].filp, fmid);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nDecrease min to RPn...\n");
> > - writeval(stuff[MIN].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> > if (load_gpu)
> > do_load_gpu();
> > check();
> >
> > igt_debug("\nDecrease min below RPn (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, 0);
> > + writeval_inval(sysfs_files[MIN].filp, 0);
> > check();
> >
> > igt_debug("\nDecrease max to midpoint...\n");
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> > check();
> >
> > igt_debug("\nDecrease max to RPn...\n");
> > - writeval(stuff[MAX].filp, origfreqs[RPn]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
> > check();
> >
> > igt_debug("\nDecrease max below RPn (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, 0);
> > + writeval_inval(sysfs_files[MAX].filp, 0);
> > check();
> >
> > igt_debug("\nIncrease min to RP0 (invalid)...\n");
> > - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
> > + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
> > check();
> >
> > igt_debug("\nIncrease max to midpoint...\n");
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> > check();
> >
> > igt_debug("\nIncrease max to RP0...\n");
> > - writeval(stuff[MAX].filp, origfreqs[RP0]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> > check();
> >
> > igt_debug("\nIncrease max above RP0 (invalid)...\n");
> > - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
> > + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
> > check();
> >
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > static void basic_check(void)
> > @@ -491,7 +504,7 @@ static void basic_check(void)
> >
> > read_freqs(freqs);
> > dump(freqs);
>
> Almost feel like we should create a new function read_and_dump_freqs()
> instead of calling 2 functions multiple times. We should keep the old
> read_freqs() since it is called in a loop on one occasion. Something to
> think about maybe?
I disagree. Those are perfectly fine two functions that have very descriptive
names. I think that it is a good practice to have one function to do only one
thing (if this makes sense) because it does not limit its reuse and is easier to
understand by the reader.
-Radek
>
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > }
> >
> > #define IDLE_WAIT_TIMESTEP_MSEC 250
> > @@ -506,7 +519,7 @@ static void idle_check(void)
> > do {
> > read_freqs(freqs);
> > dump(freqs);
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > if (freqs[CUR] == freqs[RPn])
> > break;
> > usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
> > @@ -529,7 +542,7 @@ static void loaded_check(void)
> > do {
> > read_freqs(freqs);
> > dump(freqs);
> > - checkit(freqs);
> > + check_freq_constraints(freqs);
> > if (freqs[CUR] >= freqs[MAX])
> > break;
> > usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
> > @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
> > int freqs[NUMFREQ];
> > int wait = 0;
> >
> > + /* Monitor frequencies until HW will stabilize cur frequency.
> > + * It should happen within allotted time */
> > read_freqs(freqs);
> > dump(freqs);
> > usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
> > @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
> >
> > fmid = get_hw_rounded_freq(fmid);
> > /* Set max freq to less then boost freq */
> > - writeval(stuff[MAX].filp, fmid);
> > + writeval(sysfs_files[MAX].filp, fmid);
> >
> > /* Put boost on the same engine as low load */
> > engine = I915_EXEC_RENDER;
> > @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
> > igt_spin_batch_free(fd, load);
> >
> > /* Set max freq to original softmax */
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > static void waitboost(int fd, bool reset)
> > @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
> >
> > static void pm_rps_exit_handler(int sig)
> > {
> > - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > } else {
> > - writeval(stuff[MIN].filp, origfreqs[MIN]);
> > - writeval(stuff[MAX].filp, origfreqs[MAX]);
> > + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> > + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> > }
> >
> > load_helper_deinit();
> > @@ -652,7 +667,7 @@ igt_main
> >
> > igt_fixture {
> > const int device = drm_get_card();
> > - struct junk *junk = stuff;
> > + struct sysfs_file *sysfs_file = sysfs_files;
> > int ret;
> >
> > /* Use drm_open_driver to verify device existence */
> > @@ -663,16 +678,17 @@ igt_main
> > do {
> > int val = -1;
> > char *path;
> > - ret = asprintf(&path, sysfs_base_path, device,
> > junk->name);
> > +
> > + ret = asprintf(&path, sysfs_base_path, device,
> > sysfs_file->name);
> > igt_assert(ret != -1);
> > - junk->filp = fopen(path, junk->mode);
> > - igt_require(junk->filp);
> > - setbuf(junk->filp, NULL);
> > + sysfs_file->filp = fopen(path, sysfs_file->mode);
> > + igt_require(sysfs_file->filp);
> > + setbuf(sysfs_file->filp, NULL);
> >
> > - val = readval(junk->filp);
> > + val = readval(sysfs_file->filp);
> > igt_assert(val >= 0);
> > - junk++;
> > - } while (junk->name != NULL);
> > + sysfs_file++;
> > + } while (sysfs_file->name != NULL);
> >
> > read_freqs(origfreqs);
> >
> > @@ -684,18 +700,22 @@ igt_main
> > igt_subtest("basic-api")
> > min_max_config(basic_check, false);
> >
> > + /* Verify the constraints, check if we can reach idle */
> > igt_subtest("min-max-config-idle")
> > min_max_config(idle_check, true);
> >
> > + /* Verify the constraints with high load, check if we can reach max
> > */
> > igt_subtest("min-max-config-loaded") {
> > load_helper_run(HIGH);
> > min_max_config(loaded_check, false);
> > load_helper_stop();
> > }
> >
> > + /* Checks if we achieve boost using gem_wait */
>
> We should mention this is doing gem_wait on a spinning batch, hence the
> boost.
>
> Other than this and the above comment, LGTM.
>
> Acked-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
>
> > igt_subtest("waitboost")
> > waitboost(drm_fd, false);
> >
> > + /* Test boost frequency after GPU reset */
> > igt_subtest("reset") {
> > igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> > waitboost(drm_fd, true);
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-08 7:19 ` Katarzyna Dec
@ 2017-09-08 16:48 ` Belgaumkar, Vinay
2017-09-12 11:16 ` Michał Winiarski
0 siblings, 1 reply; 14+ messages in thread
From: Belgaumkar, Vinay @ 2017-09-08 16:48 UTC (permalink / raw)
To: Katarzyna Dec, intel-gfx; +Cc: DanielVetter
On 9/8/2017 12:19 AM, Katarzyna Dec wrote:
> On Thu, Sep 07, 2017 at 11:28:38AM -0700, Belgaumkar, Vinay wrote:
>>
>>
>> On 9/7/2017 5:15 AM, Katarzyna Dec wrote:
>>> Added comments in tricky places for better feature understanding.
>>> Added IGT_TEST_DESCRIPTION and short description for non-obvious
>>> subtests.
>>> Changed name of 'magic' checkit() function to something meaningfull.
>>> Changed junk struct and stuff array names.
>>> Made some minor coding style changes.
>>>
>>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>> Cc: Petri Latvala <petri.latvala@intel.com>
>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
>>> ---
>>> tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
>>> 1 file changed, 64 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
>>> index e79f0ea7..1eeb6c6a 100644
>>> --- a/tests/pm_rps.c
>>> +++ b/tests/pm_rps.c
>>> @@ -40,6 +40,8 @@
>>>
>>> #include "intel_bufmgr.h"
>>>
>>> +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
>>> +
>>> static int drm_fd;
>>>
>>> static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
>>> @@ -56,12 +58,19 @@ enum {
>>>
>>> static int origfreqs[NUMFREQ];
>>>
>>> -struct junk {
>>> +struct sysfs_file {
>>> const char *name;
>>> const char *mode;
>>> FILE *filp;
>>> -} stuff[] = {
>>> - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
>>> +} sysfs_files[] = {
>>> + { "cur", "r", NULL },
>>> + { "min", "rb+", NULL },
>>> + { "max", "rb+", NULL },
>>> + { "RP0", "r", NULL },
>>> + { "RP1", "r", NULL },
>>> + { "RPn", "r", NULL },
>>> + { "boost", "rb+", NULL },
>>> + { NULL, NULL, NULL }
>>> };
>>>
>>> static int readval(FILE *filp)
>>> @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
>>> int i;
>>>
>>> for (i = 0; i < NUMFREQ; i++)
>>> - freqs[i] = readval(stuff[i].filp);
>>> + freqs[i] = readval(sysfs_files[i].filp);
>>> }
>>>
>>> static void nsleep(unsigned long ns)
>>> @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
>>> #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
>>> #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
>>>
>>> -static void checkit(const int *freqs)
>>> +static void check_freq_constraints(const int *freqs)
>>> {
>>> igt_assert_lte(freqs[MIN], freqs[MAX]);
>>> igt_assert_lte(freqs[CUR], freqs[MAX]);
>>> @@ -162,7 +171,7 @@ static void dump(const int *freqs)
>>>
>>> igt_debug("gt freq (MHz):");
>>> for (i = 0; i < NUMFREQ; i++)
>>> - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
>>> + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
>>>
>>> igt_debug("\n");
>>> }
>>> @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
>>> idx = MAX;
>>>
>>> old_freq = freqs[idx];
>>> - writeval_nocheck(stuff[idx].filp, target);
>>> + writeval_nocheck(sysfs_files[idx].filp, target);
>>> read_freqs(freqs);
>>> ret = freqs[idx];
>>> - writeval_nocheck(stuff[idx].filp, old_freq);
>>> + writeval_nocheck(sysfs_files[idx].filp, old_freq);
>>>
>>> return ret;
>>> }
>>>
>>> +/*
>>> + * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
>>> + * on subtest run different check after each modification.
>>> + */
>>> static void min_max_config(void (*check)(void), bool load_gpu)
>>> {
>>> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
>>> @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
>>> check();
>>>
>>> igt_debug("\nSet min=RPn and max=RP0...\n");
>>> - writeval(stuff[MIN].filp, origfreqs[RPn]);
>>> - writeval(stuff[MAX].filp, origfreqs[RP0]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
>>> if (load_gpu)
>>> do_load_gpu();
>>> check();
>>>
>>> igt_debug("\nIncrease min to midpoint...\n");
>>> - writeval(stuff[MIN].filp, fmid);
>>> + writeval(sysfs_files[MIN].filp, fmid);
>>> if (load_gpu)
>>> do_load_gpu();
>>> check();
>>>
>>> igt_debug("\nIncrease min to RP0...\n");
>>> - writeval(stuff[MIN].filp, origfreqs[RP0]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
>>> if (load_gpu)
>>> do_load_gpu();
>>> check();
>>>
>>> igt_debug("\nIncrease min above RP0 (invalid)...\n");
>>> - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
>>> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
>>> check();
>>>
>>> igt_debug("\nDecrease max to RPn (invalid)...\n");
>>> - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
>>> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
>>> check();
>>>
>>> igt_debug("\nDecrease min to midpoint...\n");
>>> - writeval(stuff[MIN].filp, fmid);
>>> + writeval(sysfs_files[MIN].filp, fmid);
>>> if (load_gpu)
>>> do_load_gpu();
>>> check();
>>>
>>> igt_debug("\nDecrease min to RPn...\n");
>>> - writeval(stuff[MIN].filp, origfreqs[RPn]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
>>> if (load_gpu)
>>> do_load_gpu();
>>> check();
>>>
>>> igt_debug("\nDecrease min below RPn (invalid)...\n");
>>> - writeval_inval(stuff[MIN].filp, 0);
>>> + writeval_inval(sysfs_files[MIN].filp, 0);
>>> check();
>>>
>>> igt_debug("\nDecrease max to midpoint...\n");
>>> - writeval(stuff[MAX].filp, fmid);
>>> + writeval(sysfs_files[MAX].filp, fmid);
>>> check();
>>>
>>> igt_debug("\nDecrease max to RPn...\n");
>>> - writeval(stuff[MAX].filp, origfreqs[RPn]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
>>> check();
>>>
>>> igt_debug("\nDecrease max below RPn (invalid)...\n");
>>> - writeval_inval(stuff[MAX].filp, 0);
>>> + writeval_inval(sysfs_files[MAX].filp, 0);
>>> check();
>>>
>>> igt_debug("\nIncrease min to RP0 (invalid)...\n");
>>> - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
>>> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
>>> check();
>>>
>>> igt_debug("\nIncrease max to midpoint...\n");
>>> - writeval(stuff[MAX].filp, fmid);
>>> + writeval(sysfs_files[MAX].filp, fmid);
>>> check();
>>>
>>> igt_debug("\nIncrease max to RP0...\n");
>>> - writeval(stuff[MAX].filp, origfreqs[RP0]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
>>> check();
>>>
>>> igt_debug("\nIncrease max above RP0 (invalid)...\n");
>>> - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
>>> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
>>> check();
>>>
>>> - writeval(stuff[MIN].filp, origfreqs[MIN]);
>>> - writeval(stuff[MAX].filp, origfreqs[MAX]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>> }
>>>
>>> static void basic_check(void)
>>> @@ -491,7 +504,7 @@ static void basic_check(void)
>>>
>>> read_freqs(freqs);
>>> dump(freqs);
>>
>> Almost feel like we should create a new function read_and_dump_freqs()
>> instead of calling 2 functions multiple times. We should keep the old
>> read_freqs() since it is called in a loop on one occasion. Something to
>> think about maybe?
> If anybody reviewing this patch feel that it would be better to change
> basic_check function name, I will do it :)
> Also, as far as I know this function is used only in this file, so there
> is no reason to add duplicate it.
>>
>>> - checkit(freqs);
>>> + check_freq_constraints(freqs);
>>> }
>>>
>>> #define IDLE_WAIT_TIMESTEP_MSEC 250
>>> @@ -506,7 +519,7 @@ static void idle_check(void)
>>> do {
>>> read_freqs(freqs);
>>> dump(freqs);
>>> - checkit(freqs);
>>> + check_freq_constraints(freqs);
>>> if (freqs[CUR] == freqs[RPn])
>>> break;
>>> usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
>>> @@ -529,7 +542,7 @@ static void loaded_check(void)
>>> do {
>>> read_freqs(freqs);
>>> dump(freqs);
>>> - checkit(freqs);
>>> + check_freq_constraints(freqs);
>>> if (freqs[CUR] >= freqs[MAX])
>>> break;
>>> usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
>>> @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
>>> int freqs[NUMFREQ];
>>> int wait = 0;
>>>
>>> + /* Monitor frequencies until HW will stabilize cur frequency.
>>> + * It should happen within allotted time */
>>> read_freqs(freqs);
>>> dump(freqs);
>>> usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
>>> @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
>>>
>>> fmid = get_hw_rounded_freq(fmid);
>>> /* Set max freq to less then boost freq */
>>> - writeval(stuff[MAX].filp, fmid);
>>> + writeval(sysfs_files[MAX].filp, fmid);
>>>
>>> /* Put boost on the same engine as low load */
>>> engine = I915_EXEC_RENDER;
>>> @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
>>> igt_spin_batch_free(fd, load);
>>>
>>> /* Set max freq to original softmax */
>>> - writeval(stuff[MAX].filp, origfreqs[MAX]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>> }
>>>
>>> static void waitboost(int fd, bool reset)
>>> @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
>>>
>>> static void pm_rps_exit_handler(int sig)
>>> {
>>> - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
>>> - writeval(stuff[MAX].filp, origfreqs[MAX]);
>>> - writeval(stuff[MIN].filp, origfreqs[MIN]);
>>> + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
>>> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
>>> } else {
>>> - writeval(stuff[MIN].filp, origfreqs[MIN]);
>>> - writeval(stuff[MAX].filp, origfreqs[MAX]);
>>> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
>>> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>> }
>>>
>>> load_helper_deinit();
>>> @@ -652,7 +667,7 @@ igt_main
>>>
>>> igt_fixture {
>>> const int device = drm_get_card();
>>> - struct junk *junk = stuff;
>>> + struct sysfs_file *sysfs_file = sysfs_files;
>>> int ret;
>>>
>>> /* Use drm_open_driver to verify device existence */
>>> @@ -663,16 +678,17 @@ igt_main
>>> do {
>>> int val = -1;
>>> char *path;
>>> - ret = asprintf(&path, sysfs_base_path, device, junk->name);
>>> +
>>> + ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
>>> igt_assert(ret != -1);
>>> - junk->filp = fopen(path, junk->mode);
>>> - igt_require(junk->filp);
>>> - setbuf(junk->filp, NULL);
>>> + sysfs_file->filp = fopen(path, sysfs_file->mode);
>>> + igt_require(sysfs_file->filp);
>>> + setbuf(sysfs_file->filp, NULL);
>>>
>>> - val = readval(junk->filp);
>>> + val = readval(sysfs_file->filp);
>>> igt_assert(val >= 0);
>>> - junk++;
>>> - } while (junk->name != NULL);
>>> + sysfs_file++;
>>> + } while (sysfs_file->name != NULL);
>>>
>>> read_freqs(origfreqs);
>>>
>>> @@ -684,18 +700,22 @@ igt_main
>>> igt_subtest("basic-api")
>>> min_max_config(basic_check, false);
>>>
>>> + /* Verify the constraints, check if we can reach idle */
>>> igt_subtest("min-max-config-idle")
>>> min_max_config(idle_check, true);
>>>
>>> + /* Verify the constraints with high load, check if we can reach max */
>>> igt_subtest("min-max-config-loaded") {
>>> load_helper_run(HIGH);
>>> min_max_config(loaded_check, false);
>>> load_helper_stop();
>>> }
>>>
>>> + /* Checks if we achieve boost using gem_wait */
>>
>> We should mention this is doing gem_wait on a spinning batch, hence the
>> boost.
> I think that additional information is not needed here. I got 2 reasons:
> - we assume that subtest comment will be oneliner
> - do we need such details here?
This current form gives the idea that we can get turbo boost by simply
doing a gem_wait, which is incorrect. We either say "Checks if we can
achieve boost" or then give the complete info - "Checks if we can
achieve boost using gem_wait on a spinning batch". Either ways, it is
still single line.
>
> Thanks,
> Kasia
>>
>> Other than this and the above comment, LGTM.
>>
>> Acked-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>
>>
>>> igt_subtest("waitboost")
>>> waitboost(drm_fd, false);
>>>
>>> + /* Test boost frequency after GPU reset */
>>> igt_subtest("reset") {
>>> igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
>>> waitboost(drm_fd, true);
>>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-08 16:48 ` Belgaumkar, Vinay
@ 2017-09-12 11:16 ` Michał Winiarski
0 siblings, 0 replies; 14+ messages in thread
From: Michał Winiarski @ 2017-09-12 11:16 UTC (permalink / raw)
To: Belgaumkar, Vinay; +Cc: intel-gfx, DanielVetter
On Fri, Sep 08, 2017 at 09:48:17AM -0700, Belgaumkar, Vinay wrote:
[SNIP]
> > > >
> > > > + /* Checks if we achieve boost using gem_wait */
> > >
> > > We should mention this is doing gem_wait on a spinning batch, hence the
> > > boost.
> > I think that additional information is not needed here. I got 2 reasons:
> > - we assume that subtest comment will be oneliner
> > - do we need such details here?
>
> This current form gives the idea that we can get turbo boost by simply doing
> a gem_wait, which is incorrect. We either say "Checks if we can achieve
> boost" or then give the complete info - "Checks if we can achieve boost
> using gem_wait on a spinning batch". Either ways, it is still single line.
Yes, the request needs to be incomplete to grant boost.
No, stating that we're granting boost on spinning batches is confusing because
of terminology overload (spinbatch is a specific type of GPU load that we're
using).
-Michał
>
> >
> > Thanks,
> > Kasia
> > >
> > > Other than this and the above comment, LGTM.
> > >
> > > Acked-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > >
> > >
> > > > igt_subtest("waitboost")
> > > > waitboost(drm_fd, false);
> > > >
> > > > + /* Test boost frequency after GPU reset */
> > > > igt_subtest("reset") {
> > > > igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> > > > waitboost(drm_fd, true);
> > > >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (6 preceding siblings ...)
2017-09-07 18:28 ` [PATCH i-g-t] " Belgaumkar, Vinay
@ 2017-09-12 11:28 ` Petri Latvala
2017-09-12 11:49 ` Arkadiusz Hiler
8 siblings, 0 replies; 14+ messages in thread
From: Petri Latvala @ 2017-09-12 11:28 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx, Daniel Vetter
On Thu, Sep 07, 2017 at 02:15:14PM +0200, Katarzyna Dec wrote:
> Added comments in tricky places for better feature understanding.
> Added IGT_TEST_DESCRIPTION and short description for non-obvious
> subtests.
> Changed name of 'magic' checkit() function to something meaningfull.
> Changed junk struct and stuff array names.
> Made some minor coding style changes.
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
LGTM. I especially like changing the identifiers for more
self-explanatory forms.
Acked-by: Petri Latvala <petri.latvala@intel.com>
> ---
> tests/pm_rps.c | 108 ++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 64 insertions(+), 44 deletions(-)
>
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index e79f0ea7..1eeb6c6a 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -40,6 +40,8 @@
>
> #include "intel_bufmgr.h"
>
> +IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
> +
> static int drm_fd;
>
> static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
> @@ -56,12 +58,19 @@ enum {
>
> static int origfreqs[NUMFREQ];
>
> -struct junk {
> +struct sysfs_file {
> const char *name;
> const char *mode;
> FILE *filp;
> -} stuff[] = {
> - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL }
> +} sysfs_files[] = {
> + { "cur", "r", NULL },
> + { "min", "rb+", NULL },
> + { "max", "rb+", NULL },
> + { "RP0", "r", NULL },
> + { "RP1", "r", NULL },
> + { "RPn", "r", NULL },
> + { "boost", "rb+", NULL },
> + { NULL, NULL, NULL }
> };
>
> static int readval(FILE *filp)
> @@ -81,7 +90,7 @@ static void read_freqs(int *freqs)
> int i;
>
> for (i = 0; i < NUMFREQ; i++)
> - freqs[i] = readval(stuff[i].filp);
> + freqs[i] = readval(sysfs_files[i].filp);
> }
>
> static void nsleep(unsigned long ns)
> @@ -143,7 +152,7 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
> #define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> #define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
>
> -static void checkit(const int *freqs)
> +static void check_freq_constraints(const int *freqs)
> {
> igt_assert_lte(freqs[MIN], freqs[MAX]);
> igt_assert_lte(freqs[CUR], freqs[MAX]);
> @@ -162,7 +171,7 @@ static void dump(const int *freqs)
>
> igt_debug("gt freq (MHz):");
> for (i = 0; i < NUMFREQ; i++)
> - igt_debug(" %s=%d", stuff[i].name, freqs[i]);
> + igt_debug(" %s=%d", sysfs_files[i].name, freqs[i]);
>
> igt_debug("\n");
> }
> @@ -387,14 +396,18 @@ static int get_hw_rounded_freq(int target)
> idx = MAX;
>
> old_freq = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, target);
> + writeval_nocheck(sysfs_files[idx].filp, target);
> read_freqs(freqs);
> ret = freqs[idx];
> - writeval_nocheck(stuff[idx].filp, old_freq);
> + writeval_nocheck(sysfs_files[idx].filp, old_freq);
>
> return ret;
> }
>
> +/*
> + * Modify softlimit MIN and MAX freqs to valid and invalid levels. Depending
> + * on subtest run different check after each modification.
> + */
> static void min_max_config(void (*check)(void), bool load_gpu)
> {
> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> @@ -411,78 +424,78 @@ static void min_max_config(void (*check)(void), bool load_gpu)
> check();
>
> igt_debug("\nSet min=RPn and max=RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min to RP0...\n");
> - writeval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RP0]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nIncrease min above RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0] + 1000);
> check();
>
> igt_debug("\nDecrease max to RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease min to midpoint...\n");
> - writeval(stuff[MIN].filp, fmid);
> + writeval(sysfs_files[MIN].filp, fmid);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min to RPn...\n");
> - writeval(stuff[MIN].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MIN].filp, origfreqs[RPn]);
> if (load_gpu)
> do_load_gpu();
> check();
>
> igt_debug("\nDecrease min below RPn (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, 0);
> + writeval_inval(sysfs_files[MIN].filp, 0);
> check();
>
> igt_debug("\nDecrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nDecrease max to RPn...\n");
> - writeval(stuff[MAX].filp, origfreqs[RPn]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RPn]);
> check();
>
> igt_debug("\nDecrease max below RPn (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, 0);
> + writeval_inval(sysfs_files[MAX].filp, 0);
> check();
>
> igt_debug("\nIncrease min to RP0 (invalid)...\n");
> - writeval_inval(stuff[MIN].filp, origfreqs[RP0]);
> + writeval_inval(sysfs_files[MIN].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max to midpoint...\n");
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
> check();
>
> igt_debug("\nIncrease max to RP0...\n");
> - writeval(stuff[MAX].filp, origfreqs[RP0]);
> + writeval(sysfs_files[MAX].filp, origfreqs[RP0]);
> check();
>
> igt_debug("\nIncrease max above RP0 (invalid)...\n");
> - writeval_inval(stuff[MAX].filp, origfreqs[RP0] + 1000);
> + writeval_inval(sysfs_files[MAX].filp, origfreqs[RP0] + 1000);
> check();
>
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void basic_check(void)
> @@ -491,7 +504,7 @@ static void basic_check(void)
>
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> }
>
> #define IDLE_WAIT_TIMESTEP_MSEC 250
> @@ -506,7 +519,7 @@ static void idle_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] == freqs[RPn])
> break;
> usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC);
> @@ -529,7 +542,7 @@ static void loaded_check(void)
> do {
> read_freqs(freqs);
> dump(freqs);
> - checkit(freqs);
> + check_freq_constraints(freqs);
> if (freqs[CUR] >= freqs[MAX])
> break;
> usleep(1000 * LOADED_WAIT_TIMESTEP_MSEC);
> @@ -547,6 +560,8 @@ static void stabilize_check(int *out)
> int freqs[NUMFREQ];
> int wait = 0;
>
> + /* Monitor frequencies until HW will stabilize cur frequency.
> + * It should happen within allotted time */
> read_freqs(freqs);
> dump(freqs);
> usleep(1000 * STABILIZE_WAIT_TIMESTEP_MSEC);
> @@ -573,7 +588,7 @@ static void boost_freq(int fd, int *boost_freqs)
>
> fmid = get_hw_rounded_freq(fmid);
> /* Set max freq to less then boost freq */
> - writeval(stuff[MAX].filp, fmid);
> + writeval(sysfs_files[MAX].filp, fmid);
>
> /* Put boost on the same engine as low load */
> engine = I915_EXEC_RENDER;
> @@ -592,7 +607,7 @@ static void boost_freq(int fd, int *boost_freqs)
> igt_spin_batch_free(fd, load);
>
> /* Set max freq to original softmax */
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> static void waitboost(int fd, bool reset)
> @@ -634,12 +649,12 @@ static void waitboost(int fd, bool reset)
>
> static void pm_rps_exit_handler(int sig)
> {
> - if (origfreqs[MIN] > readval(stuff[MAX].filp)) {
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> + if (origfreqs[MIN] > readval(sysfs_files[MAX].filp)) {
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> } else {
> - writeval(stuff[MIN].filp, origfreqs[MIN]);
> - writeval(stuff[MAX].filp, origfreqs[MAX]);
> + writeval(sysfs_files[MIN].filp, origfreqs[MIN]);
> + writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
> }
>
> load_helper_deinit();
> @@ -652,7 +667,7 @@ igt_main
>
> igt_fixture {
> const int device = drm_get_card();
> - struct junk *junk = stuff;
> + struct sysfs_file *sysfs_file = sysfs_files;
> int ret;
>
> /* Use drm_open_driver to verify device existence */
> @@ -663,16 +678,17 @@ igt_main
> do {
> int val = -1;
> char *path;
> - ret = asprintf(&path, sysfs_base_path, device, junk->name);
> +
> + ret = asprintf(&path, sysfs_base_path, device, sysfs_file->name);
> igt_assert(ret != -1);
> - junk->filp = fopen(path, junk->mode);
> - igt_require(junk->filp);
> - setbuf(junk->filp, NULL);
> + sysfs_file->filp = fopen(path, sysfs_file->mode);
> + igt_require(sysfs_file->filp);
> + setbuf(sysfs_file->filp, NULL);
>
> - val = readval(junk->filp);
> + val = readval(sysfs_file->filp);
> igt_assert(val >= 0);
> - junk++;
> - } while (junk->name != NULL);
> + sysfs_file++;
> + } while (sysfs_file->name != NULL);
>
> read_freqs(origfreqs);
>
> @@ -684,18 +700,22 @@ igt_main
> igt_subtest("basic-api")
> min_max_config(basic_check, false);
>
> + /* Verify the constraints, check if we can reach idle */
> igt_subtest("min-max-config-idle")
> min_max_config(idle_check, true);
>
> + /* Verify the constraints with high load, check if we can reach max */
> igt_subtest("min-max-config-loaded") {
> load_helper_run(HIGH);
> min_max_config(loaded_check, false);
> load_helper_stop();
> }
>
> + /* Checks if we achieve boost using gem_wait */
> igt_subtest("waitboost")
> waitboost(drm_fd, false);
>
> + /* Test boost frequency after GPU reset */
> igt_subtest("reset") {
> igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> waitboost(drm_fd, true);
> --
> 2.13.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
` (7 preceding siblings ...)
2017-09-12 11:28 ` Petri Latvala
@ 2017-09-12 11:49 ` Arkadiusz Hiler
8 siblings, 0 replies; 14+ messages in thread
From: Arkadiusz Hiler @ 2017-09-12 11:49 UTC (permalink / raw)
To: Katarzyna Dec; +Cc: intel-gfx, Daniel Vetter
On Thu, Sep 07, 2017 at 02:15:14PM +0200, Katarzyna Dec wrote:
> Added comments in tricky places for better feature understanding.
> Added IGT_TEST_DESCRIPTION and short description for non-obvious
> subtests.
> Changed name of 'magic' checkit() function to something meaningfull.
> Changed junk struct and stuff array names.
> Made some minor coding style changes.
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Signed-off: Katarzyna Dec <katarzyna.dec@intel.com>
Gathered all Acks here and pushed, as there is no use in further
bikesheeding the wording ;-)
It can be fine-tuned in the following patches if it proves necessary
anyway.
Even if it's not perfect it makes pm_rps a really good, real-world
example how things should look like.
Thanks for the patch and the reviews!
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-09-12 11:50 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-07 12:15 [PATCH i-g-t] pm_rps: [RFC] RPS tests documentation update Katarzyna Dec
2017-09-07 12:25 ` Szwichtenberg, Radoslaw
2017-09-07 12:38 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-09-07 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
2017-09-07 13:43 ` [PATCH i-g-t] " Arkadiusz Hiler
2017-09-07 15:10 ` ✓ Fi.CI.IGT: success for " Patchwork
2017-09-07 15:53 ` Patchwork
2017-09-07 18:28 ` [PATCH i-g-t] " Belgaumkar, Vinay
2017-09-08 7:19 ` Katarzyna Dec
2017-09-08 16:48 ` Belgaumkar, Vinay
2017-09-12 11:16 ` Michał Winiarski
2017-09-08 7:30 ` Szwichtenberg, Radoslaw
2017-09-12 11:28 ` Petri Latvala
2017-09-12 11:49 ` Arkadiusz Hiler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox