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

Anshuman Gupta
2019-09-06T14:12:14Z

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 is_bios_limits_pc8_plus_residencies() from
supports_pc8_plus_residencies().
Changed igt_disable_runtime_pm()return type. [Imre]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@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..540a9f92 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");
+}
+
+/**
+ * is_bios_limits_pc8_plus_residencies:
+
+ * Check whether BIOS has disabled the PC8 package deeper state.
+ *
+ * Returns:
+ * True if PC8+ package deeper state enabled on machine otherwise false.
+ */
+bool is_bios_limits_pc8_plus_residencies(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..f4d6c496 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 is_bios_limits_pc8_plus_residencies(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..d0a0bc90 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 is_bios_limits_pc8_plus_residencies(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);