diff mbox series

[i-g-t,2/2] i915_pm_freq_api: Add some basic SLPC igt tests

Message ID 20230328020028.2143954-3-vinay.belgaumkar@intel.com (mailing list archive)
State New, archived
Headers show
Series tests/slpc: Add basic IGT test | expand

Commit Message

Vinay Belgaumkar March 28, 2023, 2 a.m. UTC 
Validate basic api for GT freq control. Also test
interaction with GT reset. We skip rps tests with
SLPC enabled, this will re-introduce some coverage.
SLPC selftests are already covering some other workload
related scenarios.

v2: Rename test (Rodrigo)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 tests/i915/i915_pm_freq_api.c | 151 ++++++++++++++++++++++++++++++++++
 tests/meson.build             |   1 +
 2 files changed, 152 insertions(+)
 create mode 100644 tests/i915/i915_pm_freq_api.c

Comments

Dixit, Ashutosh March 31, 2023, 11:56 p.m. UTC | #1 
On Mon, 27 Mar 2023 19:00:28 -0700, Vinay Belgaumkar wrote:
>

Hi Vinay,

> +/*
> + * Too many intermediate components and steps before freq is adjusted
> + * Specially if workload is under execution, so let's wait 100 ms.
> + */
> +#define ACT_FREQ_LATENCY_US 100000
> +
> +static uint32_t get_freq(int dirfd, uint8_t id)
> +{
> +	uint32_t val;
> +
> +	igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);

igt_assert?

> +static void test_freq_basic_api(int dirfd, int gt)
> +{
> +	uint32_t rpn, rp0, rpe;
> +
> +	/* Save frequencies */
> +	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
> +	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
> +	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
> +	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
> +
> +	/*
> +	 * Negative bound tests
> +	 * RPn is the floor
> +	 * RP0 is the ceiling
> +	 */
> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);

Is this supposed to be RPS_MAX_FREQ_MHZ?

> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
> +

After addressing the above, this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

Also, before merging it would be good to see the results of the new
tests. So could you add a HAX patch adding the new tests to
fast-feedback.testlist and resend the series?

Thanks.
--
Ashutosh
Vinay Belgaumkar April 3, 2023, 3:23 p.m. UTC | #2 
On 3/31/2023 4:56 PM, Dixit, Ashutosh wrote:
> On Mon, 27 Mar 2023 19:00:28 -0700, Vinay Belgaumkar wrote:
> Hi Vinay,
>
>> +/*
>> + * Too many intermediate components and steps before freq is adjusted
>> + * Specially if workload is under execution, so let's wait 100 ms.
>> + */
>> +#define ACT_FREQ_LATENCY_US 100000
>> +
>> +static uint32_t get_freq(int dirfd, uint8_t id)
>> +{
>> +	uint32_t val;
>> +
>> +	igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
> igt_assert?
ok.
>
>> +static void test_freq_basic_api(int dirfd, int gt)
>> +{
>> +	uint32_t rpn, rp0, rpe;
>> +
>> +	/* Save frequencies */
>> +	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
>> +	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>> +	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
>> +	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
>> +
>> +	/*
>> +	 * Negative bound tests
>> +	 * RPn is the floor
>> +	 * RP0 is the ceiling
>> +	 */
>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
> Is this supposed to be RPS_MAX_FREQ_MHZ?
We could do this check for max as well. But this is trying to see if min 
can be set to below rpn.
>
>> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
>> +
> After addressing the above, this is:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
> Also, before merging it would be good to see the results of the new
> tests. So could you add a HAX patch adding the new tests to
> fast-feedback.testlist and resend the series?

Sure, will do. Thanks for the review.

Vinay.

>
> Thanks.
> --
> Ashutosh
Dixit, Ashutosh April 3, 2023, 3:36 p.m. UTC | #3 
On Mon, 03 Apr 2023 08:23:45 -0700, Belgaumkar, Vinay wrote:
>
>
> On 3/31/2023 4:56 PM, Dixit, Ashutosh wrote:
> > On Mon, 27 Mar 2023 19:00:28 -0700, Vinay Belgaumkar wrote:
> > Hi Vinay,
> >
> >> +/*
> >> + * Too many intermediate components and steps before freq is adjusted
> >> + * Specially if workload is under execution, so let's wait 100 ms.
> >> + */
> >> +#define ACT_FREQ_LATENCY_US 100000
> >> +
> >> +static uint32_t get_freq(int dirfd, uint8_t id)
> >> +{
> >> +	uint32_t val;
> >> +
> >> +	igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
> > igt_assert?
> ok.
> >
> >> +static void test_freq_basic_api(int dirfd, int gt)
> >> +{
> >> +	uint32_t rpn, rp0, rpe;
> >> +
> >> +	/* Save frequencies */
> >> +	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
> >> +	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
> >> +	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
> >> +	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
> >> +
> >> +	/*
> >> +	 * Negative bound tests
> >> +	 * RPn is the floor
> >> +	 * RP0 is the ceiling
> >> +	 */
> >> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
> >> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
> >> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
> > Is this supposed to be RPS_MAX_FREQ_MHZ?
> We could do this check for max as well. But this is trying to see if min
> can be set to below rpn.

In that case this statement is the same as the first one (2 lines
above). Is that needed?



> >> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
> >> +
> > After addressing the above, this is:
> >
> > Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >
> > Also, before merging it would be good to see the results of the new
> > tests. So could you add a HAX patch adding the new tests to
> > fast-feedback.testlist and resend the series?
>
> Sure, will do. Thanks for the review.
>
> Vinay.
>
> >
> > Thanks.
> > --
> > Ashutosh
Vinay Belgaumkar April 3, 2023, 5:22 p.m. UTC | #4 
On 4/3/2023 8:36 AM, Dixit, Ashutosh wrote:
> On Mon, 03 Apr 2023 08:23:45 -0700, Belgaumkar, Vinay wrote:
>>
>> On 3/31/2023 4:56 PM, Dixit, Ashutosh wrote:
>>> On Mon, 27 Mar 2023 19:00:28 -0700, Vinay Belgaumkar wrote:
>>> Hi Vinay,
>>>
>>>> +/*
>>>> + * Too many intermediate components and steps before freq is adjusted
>>>> + * Specially if workload is under execution, so let's wait 100 ms.
>>>> + */
>>>> +#define ACT_FREQ_LATENCY_US 100000
>>>> +
>>>> +static uint32_t get_freq(int dirfd, uint8_t id)
>>>> +{
>>>> +	uint32_t val;
>>>> +
>>>> +	igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
>>> igt_assert?
>> ok.
>>>> +static void test_freq_basic_api(int dirfd, int gt)
>>>> +{
>>>> +	uint32_t rpn, rp0, rpe;
>>>> +
>>>> +	/* Save frequencies */
>>>> +	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
>>>> +	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>>>> +	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
>>>> +	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
>>>> +
>>>> +	/*
>>>> +	 * Negative bound tests
>>>> +	 * RPn is the floor
>>>> +	 * RP0 is the ceiling
>>>> +	 */
>>>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
>>>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
>>>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
>>> Is this supposed to be RPS_MAX_FREQ_MHZ?
>> We could do this check for max as well. But this is trying to see if min
>> can be set to below rpn.
> In that case this statement is the same as the first one (2 lines
> above). Is that needed?

ah, yes. Need more coffee. That should be RPS_MAX_FREQ_MHZ.

Thanks,

Vinay.

>
>
>
>>>> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
>>>> +
>>> After addressing the above, this is:
>>>
>>> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>>
>>> Also, before merging it would be good to see the results of the new
>>> tests. So could you add a HAX patch adding the new tests to
>>> fast-feedback.testlist and resend the series?
>> Sure, will do. Thanks for the review.
>>
>> Vinay.
>>
>>> Thanks.
>>> --
>>> Ashutosh
diff mbox series

Patch

diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
new file mode 100644
index 00000000..f1027d1c
--- /dev/null
+++ b/tests/i915/i915_pm_freq_api.c
@@ -0,0 +1,151 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h"
+#include "i915/gem.h"
+#include "igt_sysfs.h"
+#include "igt.h"
+
+IGT_TEST_DESCRIPTION("Test SLPC freq API");
+/*
+ * Too many intermediate components and steps before freq is adjusted
+ * Specially if workload is under execution, so let's wait 100 ms.
+ */
+#define ACT_FREQ_LATENCY_US 100000
+
+static uint32_t get_freq(int dirfd, uint8_t id)
+{
+	uint32_t val;
+
+	igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
+
+	return val;
+}
+
+static int set_freq(int dirfd, uint8_t id, uint32_t val)
+{
+	return igt_sysfs_rps_printf(dirfd, id, "%u", val);
+}
+
+static void test_freq_basic_api(int dirfd, int gt)
+{
+	uint32_t rpn, rp0, rpe;
+
+	/* Save frequencies */
+	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
+	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
+	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
+	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
+
+	/*
+	 * Negative bound tests
+	 * RPn is the floor
+	 * RP0 is the ceiling
+	 */
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
+
+	/* Assert min requests are respected from rp0 to rpn */
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rp0);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpe) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpe);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+
+	/* Assert max requests are respected from rpn to rp0 */
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpe) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpe);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rp0);
+
+}
+
+static void test_reset(int i915, int dirfd, int gt)
+{
+	uint32_t rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
+	int fd;
+
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
+	usleep(ACT_FREQ_LATENCY_US);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+
+	/* Manually trigger a GT reset */
+	fd = igt_debugfs_gt_open(i915, gt, "reset", O_WRONLY);
+	igt_require(fd >= 0);
+	igt_ignore_warn(write(fd, "1\n", 2));
+	close(fd);
+
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
+}
+
+igt_main
+{
+	int i915 = -1;
+	uint32_t *stash_min, *stash_max;
+
+	igt_fixture {
+		int num_gts, dirfd, gt;
+
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+		/* i915_pm_rps already covers execlist path */
+		igt_require(gem_using_guc_submission(i915));
+
+		num_gts = igt_sysfs_get_num_gt(i915);
+		stash_min = (uint32_t*)malloc(sizeof(uint32_t) * num_gts);
+		stash_max = (uint32_t*)malloc(sizeof(uint32_t) * num_gts);
+
+		/* Save curr min and max across GTs */
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
+			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
+			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
+		}
+	}
+
+	igt_describe("Test basic API for controlling min/max GT frequency");
+	igt_subtest_with_dynamic_f("freq-basic-api") {
+		int dirfd, gt;
+
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt)
+			igt_dynamic_f("gt%u", gt)
+				test_freq_basic_api(dirfd, gt);
+	}
+
+	igt_describe("Test basic freq API works after a reset");
+	igt_subtest_with_dynamic_f("freq-reset") {
+		int dirfd, gt;
+
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt)
+			igt_dynamic_f("gt%u", gt)
+				test_reset(i915, dirfd, gt);
+	}
+
+	igt_fixture {
+		int dirfd, gt;
+		/* Restore frequencies */
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
+			igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, stash_max[gt]) > 0);
+			igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, stash_min[gt]) > 0);
+		}
+		close(i915);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 7d2168be..072cae30 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -202,6 +202,7 @@  i915_progs = [
 	'gem_workarounds',
 	'i915_fb_tiling',
 	'i915_getparams_basic',
+	'i915_pm_freq_api',
 	'i915_hangman',
 	'i915_hwmon',
 	'i915_module_load',