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

[Intel-gfx] [PATCH i-g-t 0/2] tests/i915/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_guc_pc: Add some basic SLPC igt tests

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

-- 
2.38.1

[igt-dev] [PATCH i-g-t 0/2] tests/i915/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_guc_pc: Add some basic SLPC igt tests

 lib/igt_debugfs.c        |  60 ++++++++++++++++
 lib/igt_debugfs.h        |   4 ++
 tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
 tests/meson.build        |   1 +
 4 files changed, 216 insertions(+)
 create mode 100644 tests/i915/i915_guc_pc.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

[igt-dev] [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

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

[Vinay Belgaumkar]

Use the xe_guc_pc test for i915 as well. 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.

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

diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
new file mode 100644
index 00000000..f9a0ed83
--- /dev/null
+++ b/tests/i915/i915_guc_pc.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 GuC PM features like SLPC and its interactions");
+/*
+ * 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..3ebd3bf2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -202,6 +202,7 @@ i915_progs = [
 	'gem_workarounds',
 	'i915_fb_tiling',
 	'i915_getparams_basic',
+	'i915_guc_pc',
 	'i915_hangman',
 	'i915_hwmon',
 	'i915_module_load',
-- 
2.38.1

[igt-dev] [PATCH i-g-t 2/2] i915_guc_pc: Add some basic SLPC igt tests

[Vinay Belgaumkar]

Use the xe_guc_pc test for i915 as well. 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.

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

diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
new file mode 100644
index 00000000..f9a0ed83
--- /dev/null
+++ b/tests/i915/i915_guc_pc.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 GuC PM features like SLPC and its interactions");
+/*
+ * 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..3ebd3bf2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -202,6 +202,7 @@ i915_progs = [
 	'gem_workarounds',
 	'i915_fb_tiling',
 	'i915_getparams_basic',
+	'i915_guc_pc',
 	'i915_hangman',
 	'i915_hwmon',
 	'i915_module_load',
-- 
2.38.1

[igt-dev] ✓ Fi.CI.BAT: success for tests/i915/slpc: Add basic IGT test

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3289 bytes --]

== Series Details ==

Series: tests/i915/slpc: Add basic IGT test
URL   : https://patchwork.freedesktop.org/series/115627/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12914 -> IGTPW_8681
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html

Participating hosts (38 -> 36)
------------------------------

  Missing    (2): fi-kbl-soraka fi-snb-2520m 

Known issues
------------

  Here are the changes found in IGTPW_8681 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-9:          [PASS][1] -> [ABORT][2] ([i915#7913] / [i915#7979])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/bat-dg2-9/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/bat-dg2-9/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@migrate:
    - bat-adlp-9:         [PASS][3] -> [DMESG-FAIL][4] ([i915#7699])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/bat-adlp-9/igt@i915_selftest@live@migrate.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/bat-adlp-9/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg1-5:          [PASS][5] -> [ABORT][6] ([i915#4983])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/bat-dg1-5/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/bat-dg1-5/igt@i915_selftest@live@workarounds.html

  * igt@kms_pipe_crc_basic@read-crc:
    - bat-dg2-11:         NOTRUN -> [SKIP][7] ([i915#5354])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/bat-dg2-11/igt@kms_pipe_crc_basic@read-crc.html

  
#### Warnings ####

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         [DMESG-FAIL][8] ([i915#6367]) -> [DMESG-FAIL][9] ([i915#6367] / [i915#7996])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/bat-rpls-1/igt@i915_selftest@live@slpc.html

  
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7979]: https://gitlab.freedesktop.org/drm/intel/issues/7979
  [i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7218 -> IGTPW_8681

  CI-20190529: 20190529
  CI_DRM_12914: 6e5f96153989e454041848f66a5227be9bd0bbc3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8681: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html
  IGT_7218: 8036123f781059c54a31240756794b17bd3d15dc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@i915_guc_pc@freq-basic-api
+igt@i915_guc_pc@freq-reset

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html

[-- Attachment #2: Type: text/html, Size: 4057 bytes --]

[igt-dev] ✓ Fi.CI.IGT: success for tests/i915/slpc: Add basic IGT test

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 27698 bytes --]

== Series Details ==

Series: tests/i915/slpc: Add basic IGT test
URL   : https://patchwork.freedesktop.org/series/115627/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12914_full -> IGTPW_8681_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html

Participating hosts (7 -> 8)
------------------------------

  Additional (1): shard-rkl0 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_8681_full:

### IGT changes ###

#### Possible regressions ####

  * {igt@i915_guc_pc@freq-basic-api} (NEW):
    - {shard-tglu}:       NOTRUN -> [SKIP][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-6/igt@i915_guc_pc@freq-basic-api.html
    - {shard-rkl}:        NOTRUN -> [SKIP][2] +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-4/igt@i915_guc_pc@freq-basic-api.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_module_load@reload:
    - {shard-dg1}:        [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-dg1-16/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-dg1-17/igt@i915_module_load@reload.html

  
New tests
---------

  New tests have been introduced between CI_DRM_12914_full and IGTPW_8681_full:

### New IGT tests (3) ###

  * igt@i915_guc_pc@freq-basic-api:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@i915_guc_pc@freq-reset:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@i915_guc_pc@freq-reset@gt0:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  

Known issues
------------

  Here are the changes found in IGTPW_8681_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2842]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-glk9/igt@gem_exec_fair@basic-pace@vcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk1/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_lmem_swapping@verify-random-ccs:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk7/igt@gem_lmem_swapping@verify-random-ccs.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-glk:          NOTRUN -> [WARN][8] ([i915#2658])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk5/igt@gem_pwrite@basic-exhaustion.html

  * {igt@i915_guc_pc@freq-reset} (NEW):
    - shard-apl:          NOTRUN -> [SKIP][9] ([fdo#109271]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-apl2/igt@i915_guc_pc@freq-reset.html

  * igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#3886]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk6/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_cursor_crc@cursor-random-max-size:
    - shard-glk:          NOTRUN -> [SKIP][11] ([fdo#109271]) +44 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk1/igt@kms_cursor_crc@cursor-random-max-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-glk:          [PASS][12] -> [FAIL][13] ([i915#2346])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-glk7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-suspend@a-vga1:
    - shard-snb:          [PASS][14] -> [DMESG-WARN][15] ([i915#5090])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-snb4/igt@kms_flip@flip-vs-suspend@a-vga1.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-snb4/igt@kms_flip@flip-vs-suspend@a-vga1.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-sf:
    - shard-snb:          NOTRUN -> [SKIP][16] ([fdo#109271]) +8 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-snb5/igt@kms_psr2_sf@overlay-plane-move-continuous-sf.html

  
#### Possible fixes ####

  * igt@fbdev@nullptr:
    - {shard-rkl}:        [SKIP][17] ([i915#2582]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-3/igt@fbdev@nullptr.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-2/igt@fbdev@nullptr.html
    - {shard-tglu}:       [SKIP][19] ([i915#2582]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@fbdev@nullptr.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-7/igt@fbdev@nullptr.html

  * {igt@gem_barrier_race@remote-request@rcs0}:
    - {shard-rkl}:        [ABORT][21] ([i915#8211]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-3/igt@gem_barrier_race@remote-request@rcs0.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-3/igt@gem_barrier_race@remote-request@rcs0.html
    - {shard-tglu}:       [ABORT][23] ([i915#8211]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@gem_barrier_race@remote-request@rcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-3/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_ctx_persistence@engines-hang@bcs0:
    - {shard-rkl}:        [SKIP][25] ([i915#6252]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-5/igt@gem_ctx_persistence@engines-hang@bcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-4/igt@gem_ctx_persistence@engines-hang@bcs0.html

  * igt@gem_exec_balancer@fairslice:
    - {shard-rkl}:        [SKIP][27] ([i915#6259]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-5/igt@gem_exec_balancer@fairslice.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-2/igt@gem_exec_balancer@fairslice.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - {shard-rkl}:        [FAIL][29] ([i915#2842]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-3/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_reloc@basic-wc:
    - {shard-rkl}:        [SKIP][31] ([i915#3281]) -> [PASS][32] +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@gem_exec_reloc@basic-wc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@gem_exec_reloc@basic-wc.html

  * igt@gem_exec_suspend@basic-s0@smem:
    - {shard-rkl}:        [FAIL][33] ([fdo#103375]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@gem_exec_suspend@basic-s0@smem.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@gem_exec_whisper@basic-fds-priority-all:
    - {shard-tglu}:       [INCOMPLETE][35] ([i915#6755]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-5/igt@gem_exec_whisper@basic-fds-priority-all.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-1/igt@gem_exec_whisper@basic-fds-priority-all.html

  * igt@gem_pwrite@basic-self:
    - {shard-rkl}:        [SKIP][37] ([i915#3282]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@gem_pwrite@basic-self.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@gem_pwrite@basic-self.html

  * igt@gem_workarounds@suspend-resume:
    - {shard-rkl}:        [ABORT][39] ([i915#5122]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@gem_workarounds@suspend-resume.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@gem_workarounds@suspend-resume.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - {shard-rkl}:        [SKIP][41] ([i915#2527]) -> [PASS][42] +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-2/igt@gen9_exec_parse@batch-invalid-length.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@i915_module_load@reload:
    - shard-snb:          [ABORT][43] ([i915#4528]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-snb2/igt@i915_module_load@reload.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-snb4/igt@i915_module_load@reload.html

  * igt@i915_pm_dc@dc6-dpms:
    - {shard-rkl}:        [SKIP][45] ([i915#3361]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-5/igt@i915_pm_dc@dc6-dpms.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-apl:          [SKIP][47] ([fdo#109271]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-apl1/igt@i915_pm_dc@dc9-dpms.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-apl6/igt@i915_pm_dc@dc9-dpms.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-tglu}:       [SKIP][49] ([i915#1397]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-3/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * {igt@i915_power@sanity}:
    - {shard-rkl}:        [SKIP][51] ([i915#7984]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@i915_power@sanity.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@i915_power@sanity.html

  * igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_rc_ccs:
    - {shard-tglu}:       [SKIP][53] ([i915#1845] / [i915#7651]) -> [PASS][54] +20 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_rc_ccs.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-1/igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_rc_ccs.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][55] ([i915#2346]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-glk7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@forked-move@pipe-b:
    - {shard-rkl}:        [INCOMPLETE][57] ([i915#8011]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-6/igt@kms_cursor_legacy@forked-move@pipe-b.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-3/igt@kms_cursor_legacy@forked-move@pipe-b.html
    - {shard-dg1}:        [INCOMPLETE][59] ([i915#8011]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-dg1-14/igt@kms_cursor_legacy@forked-move@pipe-b.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-dg1-18/igt@kms_cursor_legacy@forked-move@pipe-b.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1:
    - shard-glk:          [FAIL][61] ([i915#79]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - {shard-rkl}:        [SKIP][63] ([i915#1849] / [i915#4098]) -> [PASS][64] +8 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-5/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt:
    - {shard-tglu}:       [SKIP][65] ([i915#1849]) -> [PASS][66] +9 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html

  * {igt@kms_plane@invalid-pixel-format-settings}:
    - {shard-tglu}:       [SKIP][67] ([i915#8152]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@kms_plane@invalid-pixel-format-settings.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-7/igt@kms_plane@invalid-pixel-format-settings.html

  * igt@kms_psr@primary_mmap_cpu:
    - {shard-rkl}:        [SKIP][69] ([i915#1072]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@kms_psr@primary_mmap_cpu.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@kms_psr@primary_mmap_cpu.html

  * igt@kms_psr_stress_test@invalidate-primary-flip-overlay:
    - {shard-rkl}:        [SKIP][71] ([i915#5461]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html

  * igt@kms_rotation_crc@sprite-rotation-90:
    - {shard-rkl}:        [SKIP][73] ([i915#1845] / [i915#4098]) -> [PASS][74] +9 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-4/igt@kms_rotation_crc@sprite-rotation-90.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-6/igt@kms_rotation_crc@sprite-rotation-90.html

  * igt@kms_universal_plane@universal-plane-pipe-c-sanity:
    - {shard-tglu}:       [SKIP][75] ([i915#1845]) -> [PASS][76] +11 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-tglu-9/igt@kms_universal_plane@universal-plane-pipe-c-sanity.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-tglu-8/igt@kms_universal_plane@universal-plane-pipe-c-sanity.html

  * {igt@perf@gen12-group-exclusive-stream-sample-oa}:
    - {shard-rkl}:        [SKIP][77] -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-5/igt@perf@gen12-group-exclusive-stream-sample-oa.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-2/igt@perf@gen12-group-exclusive-stream-sample-oa.html

  * igt@perf@gen8-unprivileged-single-ctx-counters:
    - {shard-rkl}:        [SKIP][79] ([i915#2436]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-3/igt@perf@gen8-unprivileged-single-ctx-counters.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@perf@gen8-unprivileged-single-ctx-counters.html

  * igt@prime_vgem@basic-read:
    - {shard-rkl}:        [SKIP][81] ([fdo#109295] / [i915#3291] / [i915#3708]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12914/shard-rkl-1/igt@prime_vgem@basic-read.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/shard-rkl-5/igt@prime_vgem@basic-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1850]: https://gitlab.freedesktop.org/drm/intel/issues/1850
  [i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2436]: https://gitlab.freedesktop.org/drm/intel/issues/2436
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3639]: https://gitlab.freedesktop.org/drm/intel/issues/3639
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3826]: https://gitlab.freedesktop.org/drm/intel/issues/3826
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3938]: https://gitlab.freedesktop.org/drm/intel/issues/3938
  [i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#3966]: https://gitlab.freedesktop.org/drm/intel/issues/3966
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#5090]: https://gitlab.freedesktop.org/drm/intel/issues/5090
  [i915#5115]: https://gitlab.freedesktop.org/drm/intel/issues/5115
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6247]: https://gitlab.freedesktop.org/drm/intel/issues/6247
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6252]: https://gitlab.freedesktop.org/drm/intel/issues/6252
  [i915#6258]: https://gitlab.freedesktop.org/drm/intel/issues/6258
  [i915#6259]: https://gitlab.freedesktop.org/drm/intel/issues/6259
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
  [i915#6755]: https://gitlab.freedesktop.org/drm/intel/issues/6755
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7037]: https://gitlab.freedesktop.org/drm/intel/issues/7037
  [i915#7052]: https://gitlab.freedesktop.org/drm/intel/issues/7052
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7128]: https://gitlab.freedesktop.org/drm/intel/issues/7128
  [i915#7294]: https://gitlab.freedesktop.org/drm/intel/issues/7294
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7949]: https://gitlab.freedesktop.org/drm/intel/issues/7949
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#7984]: https://gitlab.freedesktop.org/drm/intel/issues/7984
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8018]: https://gitlab.freedesktop.org/drm/intel/issues/8018
  [i915#8152]: https://gitlab.freedesktop.org/drm/intel/issues/8152
  [i915#8154]: https://gitlab.freedesktop.org/drm/intel/issues/8154
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7218 -> IGTPW_8681
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12914: 6e5f96153989e454041848f66a5227be9bd0bbc3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8681: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html
  IGT_7218: 8036123f781059c54a31240756794b17bd3d15dc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8681/index.html

[-- Attachment #2: Type: text/html, Size: 22112 bytes --]

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

[Rodrigo Vivi]

On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
> Use the xe_guc_pc test for i915 as well. 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.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

you probably meant 'Cc:'

> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>  tests/meson.build        |   1 +
>  2 files changed, 152 insertions(+)
>  create mode 100644 tests/i915/i915_guc_pc.c
> 
> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
> new file mode 100644
> index 00000000..f9a0ed83
> --- /dev/null
> +++ b/tests/i915/i915_guc_pc.c

since 'guc_pc' is not a thing in i915 I'm afraid this will cause
confusion later.

I know, guc_slpc also doesn't make a lot of sense here...

Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
or maybe name it i915_pm_freq_api or something like that?

> @@ -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 GuC PM features like SLPC and its interactions");
> +/*
> + * 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..3ebd3bf2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -202,6 +202,7 @@ i915_progs = [
>  	'gem_workarounds',
>  	'i915_fb_tiling',
>  	'i915_getparams_basic',
> +	'i915_guc_pc',
>  	'i915_hangman',
>  	'i915_hwmon',
>  	'i915_module_load',
> -- 
> 2.38.1
> 

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

[Rodrigo Vivi]

On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
> Use the xe_guc_pc test for i915 as well. 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.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

you probably meant 'Cc:'

> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>  tests/meson.build        |   1 +
>  2 files changed, 152 insertions(+)
>  create mode 100644 tests/i915/i915_guc_pc.c
> 
> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
> new file mode 100644
> index 00000000..f9a0ed83
> --- /dev/null
> +++ b/tests/i915/i915_guc_pc.c

since 'guc_pc' is not a thing in i915 I'm afraid this will cause
confusion later.

I know, guc_slpc also doesn't make a lot of sense here...

Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
or maybe name it i915_pm_freq_api or something like that?

> @@ -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 GuC PM features like SLPC and its interactions");
> +/*
> + * 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..3ebd3bf2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -202,6 +202,7 @@ i915_progs = [
>  	'gem_workarounds',
>  	'i915_fb_tiling',
>  	'i915_getparams_basic',
> +	'i915_guc_pc',
>  	'i915_hangman',
>  	'i915_hwmon',
>  	'i915_module_load',
> -- 
> 2.38.1
> 

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

[Belgaumkar, Vinay]

On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
> On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
>> Use the xe_guc_pc test for i915 as well. 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.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> you probably meant 'Cc:'
Added you as Signed-off-by since you are the original author in xe igt.
>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>>   tests/meson.build        |   1 +
>>   2 files changed, 152 insertions(+)
>>   create mode 100644 tests/i915/i915_guc_pc.c
>>
>> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
>> new file mode 100644
>> index 00000000..f9a0ed83
>> --- /dev/null
>> +++ b/tests/i915/i915_guc_pc.c
> since 'guc_pc' is not a thing in i915 I'm afraid this will cause
> confusion later.
>
> I know, guc_slpc also doesn't make a lot of sense here...
>
> Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
> or maybe name it i915_pm_freq_api or something like that?

Sure. I was trying to make these guc/slpc specific since host trubo/RPS 
already has coverage in IGT.

Thanks,

Vinay.

>
>> @@ -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 GuC PM features like SLPC and its interactions");
>> +/*
>> + * 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..3ebd3bf2 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -202,6 +202,7 @@ i915_progs = [
>>   	'gem_workarounds',
>>   	'i915_fb_tiling',
>>   	'i915_getparams_basic',
>> +	'i915_guc_pc',
>>   	'i915_hangman',
>>   	'i915_hwmon',
>>   	'i915_module_load',
>> -- 
>> 2.38.1
>>

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

[Belgaumkar, Vinay]

On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
> On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
>> Use the xe_guc_pc test for i915 as well. 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.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> you probably meant 'Cc:'
Added you as Signed-off-by since you are the original author in xe igt.
>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>>   tests/meson.build        |   1 +
>>   2 files changed, 152 insertions(+)
>>   create mode 100644 tests/i915/i915_guc_pc.c
>>
>> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
>> new file mode 100644
>> index 00000000..f9a0ed83
>> --- /dev/null
>> +++ b/tests/i915/i915_guc_pc.c
> since 'guc_pc' is not a thing in i915 I'm afraid this will cause
> confusion later.
>
> I know, guc_slpc also doesn't make a lot of sense here...
>
> Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
> or maybe name it i915_pm_freq_api or something like that?

Sure. I was trying to make these guc/slpc specific since host trubo/RPS 
already has coverage in IGT.

Thanks,

Vinay.

>
>> @@ -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 GuC PM features like SLPC and its interactions");
>> +/*
>> + * 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..3ebd3bf2 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -202,6 +202,7 @@ i915_progs = [
>>   	'gem_workarounds',
>>   	'i915_fb_tiling',
>>   	'i915_getparams_basic',
>> +	'i915_guc_pc',
>>   	'i915_hangman',
>>   	'i915_hwmon',
>>   	'i915_module_load',
>> -- 
>> 2.38.1
>>

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

[Rodrigo Vivi]

On Mon, Mar 27, 2023 at 04:29:55PM -0700, Belgaumkar, Vinay wrote:
> 
> On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
> > On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
> > > Use the xe_guc_pc test for i915 as well. 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.
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > you probably meant 'Cc:'
> Added you as Signed-off-by since you are the original author in xe igt.

I do understand you did with the best of intentions here. But since with
the new Xe driver we are going to hit many cases like this. Please allow
me to use this case here to bring some thoughts.

First of all, there's a very common misunderstanding of the meaning of the
'Signed-off-by:' (sob).

**hint**: It does *not* mean 'authorship'!

Although we are in an IGT patch, let's use the kernel definition so we
are aligned in some well documented rule:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=signed%20off#developer-s-certificate-of-origin-1-1

So, like defined on the official rules above, in this very specific case,
when you created the patch, your 'sob' certified ('b') that:
"The contribution is based upon previous work that, to the best of my knowledge,
 is covered under an appropriate open source license and I have the right under
that license to submit that work with modifications"

Any extra Sob would be added as the patch could be in its transportation.

"Any further SoBs (Signed-off-by:’s) following the author’s SoB are from people
handling and transporting the patch, but were not involved in its development.
SoB chains should reflect the real route a patch took as it was propagated to
the maintainers and ultimately to Linus, with the first SoB entry signalling
primary authorship of a single author."

Same as 'c' of the certificate of origin: "The contribution was provided directly
to me by some other person who certified (a), (b) or (c) and I have not modified it.

It is very important to highlight this transportation rules because there
are many new devs that think that we maintainers are stealing ownership.
As you can see, this is not the case, but the rule.

Back to your case, since I had never seen this patch in my life before it hit
the mailing list, I couldn't have certified this patch in any possible way,
so the forged sob is the improper approach.

It is very hard to define a written rule on what to do with the code copied
from one driver to the other. In many cases the recognition is important,
but in other cases it is even hard to find who was actually the true author of
that code.

There are many options available. A simple one could be 'Cc' the person and
write in the commit message that the code was based on the other driver from
that person, but maybe there are better options available. So let's say that
when in doubt: ask. Contact the original author and ask what he/she has
to suggest. Maybe just this mention and cc would be enough, maybe even with
an acked-by or with the explicit sob, or maybe with some other tag like
'co-developed-by'.

Thanks,
Rodrigo.

> > 
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > ---
> > >   tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
> > >   tests/meson.build        |   1 +
> > >   2 files changed, 152 insertions(+)
> > >   create mode 100644 tests/i915/i915_guc_pc.c
> > > 
> > > diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
> > > new file mode 100644
> > > index 00000000..f9a0ed83
> > > --- /dev/null
> > > +++ b/tests/i915/i915_guc_pc.c
> > since 'guc_pc' is not a thing in i915 I'm afraid this will cause
> > confusion later.
> > 
> > I know, guc_slpc also doesn't make a lot of sense here...
> > 
> > Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
> > or maybe name it i915_pm_freq_api or something like that?
> 
> Sure. I was trying to make these guc/slpc specific since host trubo/RPS
> already has coverage in IGT.
> 
> Thanks,
> 
> Vinay.
> 
> > 
> > > @@ -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 GuC PM features like SLPC and its interactions");
> > > +/*
> > > + * 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..3ebd3bf2 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -202,6 +202,7 @@ i915_progs = [
> > >   	'gem_workarounds',
> > >   	'i915_fb_tiling',
> > >   	'i915_getparams_basic',
> > > +	'i915_guc_pc',
> > >   	'i915_hangman',
> > >   	'i915_hwmon',
> > >   	'i915_module_load',
> > > -- 
> > > 2.38.1
> > > 

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

[Rodrigo Vivi]

On Mon, Mar 27, 2023 at 04:29:55PM -0700, Belgaumkar, Vinay wrote:
> 
> On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
> > On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
> > > Use the xe_guc_pc test for i915 as well. 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.
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > you probably meant 'Cc:'
> Added you as Signed-off-by since you are the original author in xe igt.

I do understand you did with the best of intentions here. But since with
the new Xe driver we are going to hit many cases like this. Please allow
me to use this case here to bring some thoughts.

First of all, there's a very common misunderstanding of the meaning of the
'Signed-off-by:' (sob).

**hint**: It does *not* mean 'authorship'!

Although we are in an IGT patch, let's use the kernel definition so we
are aligned in some well documented rule:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=signed%20off#developer-s-certificate-of-origin-1-1

So, like defined on the official rules above, in this very specific case,
when you created the patch, your 'sob' certified ('b') that:
"The contribution is based upon previous work that, to the best of my knowledge,
 is covered under an appropriate open source license and I have the right under
that license to submit that work with modifications"

Any extra Sob would be added as the patch could be in its transportation.

"Any further SoBs (Signed-off-by:’s) following the author’s SoB are from people
handling and transporting the patch, but were not involved in its development.
SoB chains should reflect the real route a patch took as it was propagated to
the maintainers and ultimately to Linus, with the first SoB entry signalling
primary authorship of a single author."

Same as 'c' of the certificate of origin: "The contribution was provided directly
to me by some other person who certified (a), (b) or (c) and I have not modified it.

It is very important to highlight this transportation rules because there
are many new devs that think that we maintainers are stealing ownership.
As you can see, this is not the case, but the rule.

Back to your case, since I had never seen this patch in my life before it hit
the mailing list, I couldn't have certified this patch in any possible way,
so the forged sob is the improper approach.

It is very hard to define a written rule on what to do with the code copied
from one driver to the other. In many cases the recognition is important,
but in other cases it is even hard to find who was actually the true author of
that code.

There are many options available. A simple one could be 'Cc' the person and
write in the commit message that the code was based on the other driver from
that person, but maybe there are better options available. So let's say that
when in doubt: ask. Contact the original author and ask what he/she has
to suggest. Maybe just this mention and cc would be enough, maybe even with
an acked-by or with the explicit sob, or maybe with some other tag like
'co-developed-by'.

Thanks,
Rodrigo.

> > 
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > ---
> > >   tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
> > >   tests/meson.build        |   1 +
> > >   2 files changed, 152 insertions(+)
> > >   create mode 100644 tests/i915/i915_guc_pc.c
> > > 
> > > diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
> > > new file mode 100644
> > > index 00000000..f9a0ed83
> > > --- /dev/null
> > > +++ b/tests/i915/i915_guc_pc.c
> > since 'guc_pc' is not a thing in i915 I'm afraid this will cause
> > confusion later.
> > 
> > I know, guc_slpc also doesn't make a lot of sense here...
> > 
> > Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
> > or maybe name it i915_pm_freq_api or something like that?
> 
> Sure. I was trying to make these guc/slpc specific since host trubo/RPS
> already has coverage in IGT.
> 
> Thanks,
> 
> Vinay.
> 
> > 
> > > @@ -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 GuC PM features like SLPC and its interactions");
> > > +/*
> > > + * 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..3ebd3bf2 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -202,6 +202,7 @@ i915_progs = [
> > >   	'gem_workarounds',
> > >   	'i915_fb_tiling',
> > >   	'i915_getparams_basic',
> > > +	'i915_guc_pc',
> > >   	'i915_hangman',
> > >   	'i915_hwmon',
> > >   	'i915_module_load',
> > > -- 
> > > 2.38.1
> > > 

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

[Belgaumkar, Vinay]

On 3/28/2023 11:53 AM, Rodrigo Vivi wrote:
> On Mon, Mar 27, 2023 at 04:29:55PM -0700, Belgaumkar, Vinay wrote:
>> On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
>>> On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
>>>> Use the xe_guc_pc test for i915 as well. 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.
>>>>
>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> you probably meant 'Cc:'
>> Added you as Signed-off-by since you are the original author in xe igt.
> I do understand you did with the best of intentions here. But since with
> the new Xe driver we are going to hit many cases like this. Please allow
> me to use this case here to bring some thoughts.
>
> First of all, there's a very common misunderstanding of the meaning of the
> 'Signed-off-by:' (sob).
>
> **hint**: It does *not* mean 'authorship'!
>
> Although we are in an IGT patch, let's use the kernel definition so we
> are aligned in some well documented rule:
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=signed%20off#developer-s-certificate-of-origin-1-1
>
> So, like defined on the official rules above, in this very specific case,
> when you created the patch, your 'sob' certified ('b') that:
> "The contribution is based upon previous work that, to the best of my knowledge,
>   is covered under an appropriate open source license and I have the right under
> that license to submit that work with modifications"
>
> Any extra Sob would be added as the patch could be in its transportation.
>
> "Any further SoBs (Signed-off-by:’s) following the author’s SoB are from people
> handling and transporting the patch, but were not involved in its development.
> SoB chains should reflect the real route a patch took as it was propagated to
> the maintainers and ultimately to Linus, with the first SoB entry signalling
> primary authorship of a single author."
>
> Same as 'c' of the certificate of origin: "The contribution was provided directly
> to me by some other person who certified (a), (b) or (c) and I have not modified it.
>
> It is very important to highlight this transportation rules because there
> are many new devs that think that we maintainers are stealing ownership.
> As you can see, this is not the case, but the rule.
>
> Back to your case, since I had never seen this patch in my life before it hit
> the mailing list, I couldn't have certified this patch in any possible way,
> so the forged sob is the improper approach.
>
> It is very hard to define a written rule on what to do with the code copied
> from one driver to the other. In many cases the recognition is important,
> but in other cases it is even hard to find who was actually the true author of
> that code.
>
> There are many options available. A simple one could be 'Cc' the person and
> write in the commit message that the code was based on the other driver from
> that person, but maybe there are better options available. So let's say that
> when in doubt: ask. Contact the original author and ask what he/she has
> to suggest. Maybe just this mention and cc would be enough, maybe even with
> an acked-by or with the explicit sob, or maybe with some other tag like
> 'co-developed-by'.

Ok, makes sense. I have sent out another patch with you Cc'd.

Thanks,

Vinay.

>
> Thanks,
> Rodrigo.
>
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>    tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>>>>    tests/meson.build        |   1 +
>>>>    2 files changed, 152 insertions(+)
>>>>    create mode 100644 tests/i915/i915_guc_pc.c
>>>>
>>>> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
>>>> new file mode 100644
>>>> index 00000000..f9a0ed83
>>>> --- /dev/null
>>>> +++ b/tests/i915/i915_guc_pc.c
>>> since 'guc_pc' is not a thing in i915 I'm afraid this will cause
>>> confusion later.
>>>
>>> I know, guc_slpc also doesn't make a lot of sense here...
>>>
>>> Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
>>> or maybe name it i915_pm_freq_api or something like that?
>> Sure. I was trying to make these guc/slpc specific since host trubo/RPS
>> already has coverage in IGT.
>>
>> Thanks,
>>
>> Vinay.
>>
>>>> @@ -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 GuC PM features like SLPC and its interactions");
>>>> +/*
>>>> + * 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..3ebd3bf2 100644
>>>> --- a/tests/meson.build
>>>> +++ b/tests/meson.build
>>>> @@ -202,6 +202,7 @@ i915_progs = [
>>>>    	'gem_workarounds',
>>>>    	'i915_fb_tiling',
>>>>    	'i915_getparams_basic',
>>>> +	'i915_guc_pc',
>>>>    	'i915_hangman',
>>>>    	'i915_hwmon',
>>>>    	'i915_module_load',
>>>> -- 
>>>> 2.38.1
>>>>

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

[Belgaumkar, Vinay]

On 3/28/2023 11:53 AM, Rodrigo Vivi wrote:
> On Mon, Mar 27, 2023 at 04:29:55PM -0700, Belgaumkar, Vinay wrote:
>> On 3/26/2023 4:04 AM, Rodrigo Vivi wrote:
>>> On Fri, Mar 24, 2023 at 03:49:59PM -0700, Vinay Belgaumkar wrote:
>>>> Use the xe_guc_pc test for i915 as well. 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.
>>>>
>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> you probably meant 'Cc:'
>> Added you as Signed-off-by since you are the original author in xe igt.
> I do understand you did with the best of intentions here. But since with
> the new Xe driver we are going to hit many cases like this. Please allow
> me to use this case here to bring some thoughts.
>
> First of all, there's a very common misunderstanding of the meaning of the
> 'Signed-off-by:' (sob).
>
> **hint**: It does *not* mean 'authorship'!
>
> Although we are in an IGT patch, let's use the kernel definition so we
> are aligned in some well documented rule:
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=signed%20off#developer-s-certificate-of-origin-1-1
>
> So, like defined on the official rules above, in this very specific case,
> when you created the patch, your 'sob' certified ('b') that:
> "The contribution is based upon previous work that, to the best of my knowledge,
>   is covered under an appropriate open source license and I have the right under
> that license to submit that work with modifications"
>
> Any extra Sob would be added as the patch could be in its transportation.
>
> "Any further SoBs (Signed-off-by:’s) following the author’s SoB are from people
> handling and transporting the patch, but were not involved in its development.
> SoB chains should reflect the real route a patch took as it was propagated to
> the maintainers and ultimately to Linus, with the first SoB entry signalling
> primary authorship of a single author."
>
> Same as 'c' of the certificate of origin: "The contribution was provided directly
> to me by some other person who certified (a), (b) or (c) and I have not modified it.
>
> It is very important to highlight this transportation rules because there
> are many new devs that think that we maintainers are stealing ownership.
> As you can see, this is not the case, but the rule.
>
> Back to your case, since I had never seen this patch in my life before it hit
> the mailing list, I couldn't have certified this patch in any possible way,
> so the forged sob is the improper approach.
>
> It is very hard to define a written rule on what to do with the code copied
> from one driver to the other. In many cases the recognition is important,
> but in other cases it is even hard to find who was actually the true author of
> that code.
>
> There are many options available. A simple one could be 'Cc' the person and
> write in the commit message that the code was based on the other driver from
> that person, but maybe there are better options available. So let's say that
> when in doubt: ask. Contact the original author and ask what he/she has
> to suggest. Maybe just this mention and cc would be enough, maybe even with
> an acked-by or with the explicit sob, or maybe with some other tag like
> 'co-developed-by'.

Ok, makes sense. I have sent out another patch with you Cc'd.

Thanks,

Vinay.

>
> Thanks,
> Rodrigo.
>
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>    tests/i915/i915_guc_pc.c | 151 +++++++++++++++++++++++++++++++++++++++
>>>>    tests/meson.build        |   1 +
>>>>    2 files changed, 152 insertions(+)
>>>>    create mode 100644 tests/i915/i915_guc_pc.c
>>>>
>>>> diff --git a/tests/i915/i915_guc_pc.c b/tests/i915/i915_guc_pc.c
>>>> new file mode 100644
>>>> index 00000000..f9a0ed83
>>>> --- /dev/null
>>>> +++ b/tests/i915/i915_guc_pc.c
>>> since 'guc_pc' is not a thing in i915 I'm afraid this will cause
>>> confusion later.
>>>
>>> I know, guc_slpc also doesn't make a lot of sense here...
>>>
>>> Should we then try to move this code to the 'tests/i915/i915_pm_rps.c'
>>> or maybe name it i915_pm_freq_api or something like that?
>> Sure. I was trying to make these guc/slpc specific since host trubo/RPS
>> already has coverage in IGT.
>>
>> Thanks,
>>
>> Vinay.
>>
>>>> @@ -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 GuC PM features like SLPC and its interactions");
>>>> +/*
>>>> + * 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..3ebd3bf2 100644
>>>> --- a/tests/meson.build
>>>> +++ b/tests/meson.build
>>>> @@ -202,6 +202,7 @@ i915_progs = [
>>>>    	'gem_workarounds',
>>>>    	'i915_fb_tiling',
>>>>    	'i915_getparams_basic',
>>>> +	'i915_guc_pc',
>>>>    	'i915_hangman',
>>>>    	'i915_hwmon',
>>>>    	'i915_module_load',
>>>> -- 
>>>> 2.38.1
>>>>