[Intel-gfx] [PATCH i-g-t 0/2] tests/slpc: Add basic IGT test

[Intel-gfx] [PATCH i-g-t 0/2] tests/slpc: Add basic IGT test

[Vinay Belgaumkar]

Borrow some subtests from xe_guc_pc. Also add per GT debugfs helpers.

Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

Vinay Belgaumkar (2):
  lib/debugfs: Add per GT debugfs helpers
  i915_pm_freq_api: Add some basic SLPC igt tests

 lib/igt_debugfs.c             |  60 ++++++++++++++
 lib/igt_debugfs.h             |   4 +
 tests/i915/i915_pm_freq_api.c | 151 ++++++++++++++++++++++++++++++++++
 tests/meson.build             |   1 +
 4 files changed, 216 insertions(+)
 create mode 100644 tests/i915/i915_pm_freq_api.c

-- 
2.38.1

[Intel-gfx] [PATCH i-g-t 1/2] lib/debugfs: Add per GT debugfs helpers

[Vinay Belgaumkar]

These can be used to open per-gt debugfs files.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vinay Belgaumkar <viay.belgaumkar@intel.com>
---
 lib/igt_debugfs.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_debugfs.h |  4 ++++
 2 files changed, 64 insertions(+)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 05889bbe..afde2da6 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -217,6 +217,37 @@ int igt_debugfs_dir(int device)
 	return open(path, O_RDONLY);
 }
 
+/**
+ * igt_debugfs_gt_dir:
+ * @device: fd of the device
+ * @gt: GT instance number
+ *
+ * This opens the debugfs directory corresponding to device for use
+ * with igt_sysfs_get() and related functions.
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_debugfs_gt_dir(int device, unsigned int gt)
+{
+	int debugfs_gt_dir_fd;
+	char path[PATH_MAX];
+	char gtpath[16];
+	int ret;
+
+	if (!igt_debugfs_path(device, path, sizeof(path)))
+		return -1;
+
+	ret = snprintf(gtpath, sizeof(gtpath), "/gt%u", gt);
+	igt_assert(ret < sizeof(gtpath));
+	strncat(path, gtpath, sizeof(path) - 1);
+
+	debugfs_gt_dir_fd = open(path, O_RDONLY);
+	igt_debug_on_f(debugfs_gt_dir_fd < 0, "path: %s\n", path);
+
+	return debugfs_gt_dir_fd;
+}
+
 /**
  * igt_debugfs_connector_dir:
  * @device: fd of the device
@@ -313,6 +344,35 @@ bool igt_debugfs_exists(int device, const char *filename, int mode)
 	return false;
 }
 
+/**
+ * igt_debugfs_gt_open:
+ * @device: open i915 drm fd
+ * @gt: gt instance number
+ * @filename: name of the debugfs node to open
+ * @mode: mode bits as used by open()
+ *
+ * This opens a debugfs file as a Unix file descriptor. The filename should be
+ * relative to the drm device's root, i.e. without "drm/$minor".
+ *
+ * Returns:
+ * The Unix file descriptor for the debugfs file or -1 if that didn't work out.
+ */
+int
+igt_debugfs_gt_open(int device, unsigned int gt, const char *filename, int mode)
+{
+	int dir, ret;
+
+	dir = igt_debugfs_gt_dir(device, gt);
+	if (dir < 0)
+		return dir;
+
+	ret = openat(dir, filename, mode);
+
+	close(dir);
+
+	return ret;
+}
+
 /**
  * igt_debugfs_simple_read:
  * @dir: fd of the debugfs directory
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 4824344a..3e6194ad 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -45,6 +45,10 @@ void __igt_debugfs_write(int fd, const char *filename, const char *buf, int size
 int igt_debugfs_simple_read(int dir, const char *filename, char *buf, int size);
 bool igt_debugfs_search(int fd, const char *filename, const char *substring);
 
+int igt_debugfs_gt_dir(int device, unsigned int gt);
+int igt_debugfs_gt_open(int device, unsigned int gt, const char *filename,
+			int mode);
+
 /**
  * igt_debugfs_read:
  * @filename: name of the debugfs file
-- 
2.38.1

Re: [Intel-gfx] [PATCH i-g-t 1/2] lib/debugfs: Add per GT debugfs helpers

[Dixit, Ashutosh]

On Mon, 27 Mar 2023 19:00:27 -0700, Vinay Belgaumkar wrote:
>
> These can be used to open per-gt debugfs files.
>

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

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

[Vinay Belgaumkar]

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

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',
-- 
2.38.1

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] i915_pm_freq_api: Add some basic SLPC igt tests

[Dixit, Ashutosh]

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

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] i915_pm_freq_api: Add some basic SLPC igt tests

[Belgaumkar, Vinay]

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

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] i915_pm_freq_api: Add some basic SLPC igt tests

[Dixit, Ashutosh]

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

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] i915_pm_freq_api: Add some basic SLPC igt tests

[Belgaumkar, Vinay]

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