[igt-dev] [PATCH i-g-t v14 1/6] lib/igt_pm: igt lib helper routines to support DC5/6 tests

Anshuman Gupta anshuman.gupta at intel.com
Thu Sep 19 17:30:28 UTC 2019


From: Jyoti Yadav <jyoti.r.yadav at intel.com>

This patch does the following changes to lib/igt_pm.c

-dmc_loaded() will be used by new test i915_pm_dc.c which will validate
 Display C States. So moving the same to igt_pm library.

-Introduced igt_disable_runtime_pm() in order to disable runtime suspend
 for the function which support dc9.

-Changed the igt_pm_enable_sata_link_power_management() and
 igt_pm_restore_sata_link_power_management() in order to save
 and restore the sata link power policy by an exit handler.

v2: Simplify the comment section.
v3: Remove . from the subject line.
v4: Rebased, resolve conflicts in pm_rpm.c
    Included patch set version change log.
v5: Listing actual change in patch set changelog to make review easier.
v6: igt's lib added support for disabling runtime suspend,
    change in commit log. rebased due to test name pm_rpm changed
    to i915_pm_rpm.
v7: Addressed review comment by saving POWER_DIR values in
    igt_disable_runtime_pm(). [Imre]
v8: Addressed the review comment, igt_pm_enable_sata_link_power_management
    function to restore the original SATA link power policy if things fail
    by using an exit handler. [Imre]
v9: IGT failure fixture in i915_pm_backlight and i915_pm_rpm.
v10:Review comment fixup in sata_link_power_management
    lib functions. [Imre]
v11:Add igt_assert_fd(pm_status_fd) in igt_disable_runtime_pm().
    [Imre & Petri]
v12: Refactor igt_pm_pc8_plus_residencies_enabled() from
     supports_pc8_plus_residencies().
     Changed igt_disable_runtime_pm() return type. [Imre]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
Reviewed-by: Imre Deak <imre.deak at intel.com>
---
 lib/igt_pm.c                   | 215 ++++++++++++++++++++++++++-------
 lib/igt_pm.h                   |   7 +-
 tests/i915/i915_pm_backlight.c |   6 +-
 tests/i915/i915_pm_rpm.c       |  39 +-----
 4 files changed, 179 insertions(+), 88 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index fd22273a..64ce240e 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -38,6 +38,7 @@
 #include "drmtest.h"
 #include "igt_pm.h"
 #include "igt_aux.h"
+#include "igt_sysfs.h"
 
 /**
  * SECTION:igt_pm
@@ -58,16 +59,30 @@ enum {
 	POLICY_MIN_POWER = 2
 };
 
+#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
+/*
+ * Below PKG CST limit mask and PC8 bits are meant for
+ * HSW,BDW SKL,ICL and Goldmont Microarch and future platforms.
+ * Refer IA S/W developers manual vol3c part3 chapter:35
+ */
+#define  PKG_CST_LIMIT_MASK		0xF
+#define  PKG_CST_LIMIT_C8		0x6
+
 #define MAX_PERFORMANCE_STR	"max_performance\n"
 #define MEDIUM_POWER_STR	"medium_power\n"
 #define MIN_POWER_STR		"min_power\n"
 /* Remember to fix this if adding longer strings */
 #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
+int8_t *__sata_pm_policies;
+int __scsi_host_cnt;
 
 static char __igt_pm_audio_runtime_power_save[64];
 static char * __igt_pm_audio_runtime_control_path;
 static char __igt_pm_audio_runtime_control[64];
 
+static void __igt_pm_sata_link_pm_exit_handler(int sig);
+static void __igt_pm_restore_sata_link_power_management(void);
+
 static int __igt_pm_audio_restore_runtime_pm(void)
 {
 	int fd;
@@ -280,39 +295,26 @@ void igt_pm_enable_audio_runtime_pm(void)
 		igt_debug("Failed to enable audio runtime PM! (%d)\n", -err);
 }
 
-/**
- * igt_pm_enable_sata_link_power_management:
- *
- * Enable the min_power policy for SATA link power management.
- * Without this we cannot reach deep runtime power states.
- *
- * We don't have any assertions on open since the system might not have
- * a SATA host.
- *
- * Returns:
- * An opaque pointer to the data needed to restore the default values
- * after the test has terminated, or NULL if SATA link power management
- * is not supported. This pointer should be freed when no longer used
- * (typically after having called restore_sata_link_power_management()).
- */
-int8_t *igt_pm_enable_sata_link_power_management(void)
+static void __igt_pm_enable_sata_link_power_management(void)
 {
 	int fd, i;
 	ssize_t len;
 	char *buf;
 	char *file_name;
-	int8_t *link_pm_policies = NULL;
+	int8_t policy;
 
 	file_name = malloc(PATH_MAX);
 	buf = malloc(MAX_POLICY_STRLEN + 1);
 
-	for (i = 0; ; i++) {
-		int8_t policy;
-
+	for (__scsi_host_cnt = 0; ; __scsi_host_cnt++) {
 		snprintf(file_name, PATH_MAX,
 			 "/sys/class/scsi_host/host%d/link_power_management_policy",
-			 i);
+			 __scsi_host_cnt);
 
+		/*
+		 * We don't have any assertions on open since the system
+		 * might not have a SATA host.
+		 */
 		fd = open(file_name, O_RDWR);
 		if (fd < 0)
 			break;
@@ -332,12 +334,26 @@ int8_t *igt_pm_enable_sata_link_power_management(void)
 		else
 			policy = POLICY_UNKNOWN;
 
-		if (!(i % 256))
-			link_pm_policies = realloc(link_pm_policies,
-						   (i / 256 + 1) * 256 + 1);
+		if (!(__scsi_host_cnt % 256))
+			__sata_pm_policies = realloc(__sata_pm_policies,
+						     (__scsi_host_cnt / 256 + 1)
+						     * 256 + 1);
+
+		__sata_pm_policies[__scsi_host_cnt] = policy;
+		close(fd);
+	}
+
+	igt_install_exit_handler(__igt_pm_sata_link_pm_exit_handler);
+
+	for (i = 0; i < __scsi_host_cnt; i++) {
+		snprintf(file_name, PATH_MAX,
+			 "/sys/class/scsi_host/host%d/link_power_management_policy",
+			 i);
+		fd = open(file_name, O_RDWR);
+		if (fd < 0)
+			break;
 
-		link_pm_policies[i] = policy;
-		link_pm_policies[i + 1] = 0;
+		policy = __sata_pm_policies[i];
 
 		/* If the policy is something we don't know about,
 		 * don't touch it, since we might potentially break things.
@@ -355,39 +371,25 @@ int8_t *igt_pm_enable_sata_link_power_management(void)
 	}
 	free(buf);
 	free(file_name);
-
-	return link_pm_policies;
 }
 
-/**
- * igt_pm_restore_sata_link_power_management:
- * @pm_data: An opaque pointer with saved link PM policies;
- *           If NULL is passed we force enable the "max_performance" policy.
- *
- * Restore the link power management policies to the values
- * prior to enabling min_power.
- *
- * Caveat: If the system supports hotplugging and hotplugging takes
- *         place during our testing so that the hosts change numbers
- *         we might restore the settings to the wrong hosts.
- */
-void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
-
+static void __igt_pm_restore_sata_link_power_management(void)
 {
 	int fd, i;
 	char *file_name;
 
+	if (!__sata_pm_policies)
+		return;
+
 	/* Disk runtime PM policies. */
 	file_name = malloc(PATH_MAX);
-	for (i = 0; ; i++) {
+	for (i = 0; i < __scsi_host_cnt; i++) {
 		int8_t policy;
 
-		if (!pm_data)
-			policy = POLICY_MAX_PERFORMANCE;
-		else if (pm_data[i] == POLICY_UNKNOWN)
+		if (__sata_pm_policies[i] == POLICY_UNKNOWN)
 			continue;
 		else
-			policy = pm_data[i];
+			policy = __sata_pm_policies[i];
 
 		snprintf(file_name, PATH_MAX,
 			 "/sys/class/scsi_host/host%d/link_power_management_policy",
@@ -421,7 +423,48 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
 		close(fd);
 	}
 	free(file_name);
+	free(__sata_pm_policies);
+	__sata_pm_policies = NULL;
+}
+
+/**
+ * igt_pm_enable_sata_link_power_management:
+ *
+ * Enable the min_power policy for SATA link power management.
+ * Without this we cannot reach deep runtime power states.
+ */
+void igt_pm_enable_sata_link_power_management(void)
+{
+	/* Check if has been already saved. */
+	if (__sata_pm_policies)
+		return;
+
+	 __igt_pm_enable_sata_link_power_management();
+}
+
+/**
+ * igt_pm_restore_sata_link_power_management:
+ *
+ * Restore the link power management policies to the values
+ * prior to enabling min_power.
+ *
+ * Caveat: If the system supports hotplugging and hotplugging takes
+ *         place during our testing so that the hosts change numbers
+ *         we might restore the settings to the wrong hosts.
+ */
+void igt_pm_restore_sata_link_power_management(void)
+{
+	if (!__sata_pm_policies)
+		return;
+
+	 __igt_pm_restore_sata_link_power_management();
+}
+
+static void __igt_pm_sata_link_pm_exit_handler(int sig)
+{
+	__igt_pm_restore_sata_link_power_management();
 }
+
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
 /* We just leak this on exit ... */
 int pm_status_fd = -1;
@@ -585,6 +628,34 @@ bool igt_setup_runtime_pm(void)
 	return true;
 }
 
+/**
+ * igt_disable_runtime_pm:
+ *
+ * Disable the runtime pm for i915 device.
+ * igt_disable_runtime_pm assumes that igt_setup_runtime_pm has already
+ * called to save runtime autosuspend and control attributes.
+ */
+void igt_disable_runtime_pm(void)
+{
+	int fd;
+	ssize_t size;
+	char buf[6];
+
+	igt_assert_fd(pm_status_fd);
+
+	/* We know we support runtime PM, let's try to disable it now. */
+	fd = open(POWER_DIR "/control", O_RDWR);
+	igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
+
+	size = write(fd, "on\n", 3);
+	igt_assert(size == 3);
+	lseek(fd, 0, SEEK_SET);
+	size = read(fd, buf, ARRAY_SIZE(buf));
+	igt_assert(size == 3);
+	igt_assert(strncmp(buf, "on\n", 3) == 0);
+	close(fd);
+}
+
 /**
  * igt_get_runtime_pm_status:
  *
@@ -628,3 +699,53 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
 {
 	return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
 }
+
+/**
+ * dmc_loaded:
+ * @debugfs: fd to the debugfs dir.
+ *
+ * Check whether DMC FW is loaded or not. DMC FW is require for few Display C
+ * states like DC5 and DC6. FW does the Context Save and Restore during Display
+ * C States entry and exit.
+ *
+ * Returns:
+ * True if DMC FW is loaded otherwise false.
+ */
+bool igt_pm_dmc_loaded(int debugfs)
+{
+	char buf[15];
+	int len;
+
+	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
+	if (len < 0)
+		return true; /* no CSR support, no DMC requirement */
+
+	buf[len] = '\0';
+
+	igt_info("DMC: %s\n", buf);
+	return strstr(buf, "fw loaded: yes");
+}
+
+/**
+ * igt_pm_pc8_plus_residencies_enabled:
+ * @msr_fd: fd to /dev/cpu/0/msr
+ * Check whether BIOS has disabled the PC8 package deeper state.
+ *
+ * Returns:
+ * True if PC8+ package deeper state enabled on machine otherwise false.
+ */
+bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
+{
+	int rc;
+	uint64_t val;
+
+	rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PKG_CST_CONFIG_CONTROL);
+	if (rc != sizeof(val))
+		return false;
+	if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
+		igt_info("PKG C-states limited below PC8 by the BIOS\n");
+		return false;
+	}
+
+	return true;
+}
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index 10cc6794..7dc24174 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -25,8 +25,8 @@
 #define IGT_PM_H
 
 void igt_pm_enable_audio_runtime_pm(void);
-int8_t *igt_pm_enable_sata_link_power_management(void);
-void igt_pm_restore_sata_link_power_management(int8_t *pm_data);
+void igt_pm_enable_sata_link_power_management(void);
+void igt_pm_restore_sata_link_power_management(void);
 
 /**
  * igt_runtime_pm_status:
@@ -47,8 +47,11 @@ enum igt_runtime_pm_status {
 };
 
 bool igt_setup_runtime_pm(void);
+void igt_disable_runtime_pm(void);
 void igt_restore_runtime_pm(void);
 enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
 bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
+bool igt_pm_dmc_loaded(int debugfs);
+bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
 
 #endif /* IGT_PM_H */
diff --git a/tests/i915/i915_pm_backlight.c b/tests/i915/i915_pm_backlight.c
index 4c1bff5b..9a5f4c37 100644
--- a/tests/i915/i915_pm_backlight.c
+++ b/tests/i915/i915_pm_backlight.c
@@ -47,7 +47,6 @@ struct context {
 #define FADESPEED 100 /* milliseconds between steps */
 
 IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
-static int8_t *pm_data = NULL;
 
 static int backlight_read(int *result, const char *fname)
 {
@@ -235,7 +234,7 @@ igt_main
 		igt_plane_set_fb(primary, &fb);
 
 		igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
-		pm_data = igt_pm_enable_sata_link_power_management();
+		igt_pm_enable_sata_link_power_management();
 	}
 
 	igt_subtest("basic-brightness")
@@ -255,8 +254,7 @@ igt_main
 
 		igt_display_fini(&display);
 		igt_remove_fb(display.drm_fd, &fb);
-		igt_pm_restore_sata_link_power_management(pm_data);
-		free(pm_data);
+		igt_pm_restore_sata_link_power_management();
 		close(display.drm_fd);
 	}
 }
diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 2168ff72..20c4ff90 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -52,11 +52,6 @@
 #include "igt_device.h"
 #include "igt_edid.h"
 
-#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
-/* HSW/BDW: */
-#define  PKG_CST_LIMIT_MASK		0xF
-#define  PKG_CST_LIMIT_C8		0x6
-
 #define MSR_PC8_RES	0x630
 #define MSR_PC9_RES	0x631
 #define MSR_PC10_RES	0x632
@@ -123,8 +118,6 @@ struct modeset_params lpsp_mode_params;
 struct modeset_params non_lpsp_mode_params;
 struct modeset_params *default_mode_params;
 
-static int8_t *pm_data = NULL;
-
 static int modprobe(const char *driver)
 {
 	return igt_kmod_load(driver, NULL);
@@ -146,15 +139,7 @@ static bool supports_pc8_plus_residencies(void)
 	if (rc != sizeof(val))
 		return false;
 
-	rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PKG_CST_CONFIG_CONTROL);
-	if (rc != sizeof(val))
-		return false;
-	if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
-		igt_info("PKG C-states limited below PC8 by the BIOS\n");
-		return false;
-	}
-
-	return true;
+	return igt_pm_pc8_plus_residencies_enabled(msr_fd);
 }
 
 static uint64_t get_residency(uint32_t type)
@@ -755,21 +740,6 @@ static void setup_pc8(void)
 	has_pc8 = true;
 }
 
-static bool dmc_loaded(void)
-{
-	char buf[15];
-	int len;
-
-	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
-	if (len < 0)
-	    return true; /* no CSR support, no DMC requirement */
-
-	buf[len] = '\0';
-
-	igt_info("DMC: %s\n", buf);
-	return strstr(buf, "fw loaded: yes");
-}
-
 static void dump_file(int dir, const char *filename)
 {
 	char *contents;
@@ -796,7 +766,7 @@ static bool setup_environment(void)
 
 	init_mode_set_data(&ms_data);
 
-	pm_data = igt_pm_enable_sata_link_power_management();
+	igt_pm_enable_sata_link_power_management();
 
 	has_runtime_pm = igt_setup_runtime_pm();
 	setup_pc8();
@@ -804,7 +774,7 @@ static bool setup_environment(void)
 	igt_info("Runtime PM support: %d\n", has_runtime_pm);
 	igt_info("PC8 residency support: %d\n", has_pc8);
 	igt_require(has_runtime_pm);
-	igt_require(dmc_loaded());
+	igt_require(igt_pm_dmc_loaded(debugfs));
 
 out:
 	disable_all_screens(&ms_data);
@@ -821,8 +791,7 @@ static void teardown_environment(void)
 
 	igt_restore_runtime_pm();
 
-	igt_pm_restore_sata_link_power_management(pm_data);
-	free(pm_data);
+	igt_pm_restore_sata_link_power_management();
 
 	fini_mode_set_data(&ms_data);
 
-- 
2.21.0



More information about the igt-dev mailing list