[igt-dev] [PATCH i-g-t] lib/pm: Fix some runtime PM issues
Tvrtko Ursulin
tursulin at ursulin.net
Fri Jul 20 14:03:19 UTC 2018
From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
1.
It seems that on many systems the hardcoded PCI path in
igt_pm_audio_setup_runtime_pm is not correct.
Add some more paths to work around the issue. More robust solution would
be to look for a symlink of a specific format and use that, such as:
# ls -ld /sys/bus/pci/drivers/snd_hda_intel/0000\:00\:1f.3/
drwxr-xr-x 7 root root 0 Jul 20 14:48 /sys/bus/pci/drivers/snd_hda_intel/0000:00:1f.3/
But that's more work, so leave it for a rainy day.
2.
Cleanup handlers were not signal safe - rewrite them.
3.
Along the way I can also export the explicit cleanup function to be called
from individual test cases where that is wanted.
4.
Call the cleanup explicitly from perf_pmu/rc6-runtime-pm.
v2:
* Skip double restore. (Chris Wilson)
* Close previously leaked fd.
v3:
* Refactor atexit handlers to be signal safe. (Chris Wilson)
* Restore audio runtime PM status.
* Add a different PCI path to audio device.
v4:
* Patch renamed and commit message improved.
* Add another possible PCI bus location.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> # v3
---
Dear Santa,
Please don't make this split this up - at least not today since it is
Friday and an unusually warm day! :)
---
lib/igt_pm.c | 153 ++++++++++++++++++++++++++++++++++++-----------
lib/igt_pm.h | 1 +
tests/perf_pmu.c | 3 +
3 files changed, 123 insertions(+), 34 deletions(-)
diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 8ac132269d79..18c5eef9a43b 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -63,36 +63,74 @@ enum {
/* Remember to fix this if adding longer strings */
#define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR)
+static const char *const __igt_pm_audio_runtime_control_paths[] = {
+ "/sys/bus/pci/devices/0000:00:02.0/power/control",
+ "/sys/bus/pci/devices/0000:00:03.0/power/control",
+ "/sys/bus/pci/devices/0000:00:1f.3/power/control",
+};
+
static char __igt_pm_audio_runtime_power_save[64];
+static const char * __igt_pm_audio_runtime_control_path;
static char __igt_pm_audio_runtime_control[64];
-static void __igt_pm_audio_runtime_exit_handler(int sig)
+static int __igt_pm_audio_restore_runtime_pm(void)
{
int fd;
- igt_debug("Restoring audio power management to '%s' and '%s'\n",
- __igt_pm_audio_runtime_power_save,
- __igt_pm_audio_runtime_control);
+ if (!__igt_pm_audio_runtime_power_save[0])
+ return 0;
fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY);
if (fd < 0)
- return;
+ return errno;
+
if (write(fd, __igt_pm_audio_runtime_power_save,
strlen(__igt_pm_audio_runtime_power_save)) !=
- strlen(__igt_pm_audio_runtime_power_save))
- igt_warn("Failed to restore audio power_save to '%s'\n",
- __igt_pm_audio_runtime_power_save);
+ strlen(__igt_pm_audio_runtime_power_save)) {
+ close(fd);
+ return errno;
+ }
+
close(fd);
- fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY);
+ fd = open(__igt_pm_audio_runtime_control_path, O_WRONLY);
if (fd < 0)
- return;
+ return errno;
+
if (write(fd, __igt_pm_audio_runtime_control,
strlen(__igt_pm_audio_runtime_control)) !=
- strlen(__igt_pm_audio_runtime_control))
- igt_warn("Failed to restore audio control to '%s'\n",
- __igt_pm_audio_runtime_control);
+ strlen(__igt_pm_audio_runtime_control)) {
+ close(fd);
+ return errno;
+ }
+
close(fd);
+
+ __igt_pm_audio_runtime_power_save[0] = 0;
+
+ return 0;
+}
+
+static void igt_pm_audio_restore_runtime_pm(void)
+{
+ int ret;
+
+ if (!__igt_pm_audio_runtime_power_save[0])
+ return;
+
+ igt_debug("Restoring audio power management to '%s' and '%s'\n",
+ __igt_pm_audio_runtime_power_save,
+ __igt_pm_audio_runtime_control);
+
+ ret = __igt_pm_audio_restore_runtime_pm();
+ if (ret)
+ igt_warn("Failed to restore runtime audio PM! (errno=%d)\n",
+ ret);
+}
+
+static void __igt_pm_audio_runtime_exit_handler(int sig)
+{
+ __igt_pm_audio_restore_runtime_pm();
}
static void strchomp(char *str)
@@ -116,7 +154,7 @@ static void strchomp(char *str)
*/
void igt_pm_enable_audio_runtime_pm(void)
{
- int fd;
+ int fd, i;
/* Check if already enabled. */
if (__igt_pm_audio_runtime_power_save[0])
@@ -131,14 +169,23 @@ void igt_pm_enable_audio_runtime_pm(void)
igt_assert_eq(write(fd, "1\n", 2), 2);
close(fd);
}
- fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_RDWR);
- if (fd >= 0) {
- igt_assert(read(fd, __igt_pm_audio_runtime_control,
- sizeof(__igt_pm_audio_runtime_control)) > 0);
- strchomp(__igt_pm_audio_runtime_control);
- igt_assert_eq(write(fd, "auto\n", 5), 5);
- close(fd);
+
+ for (i = 0; i < ARRAY_SIZE(__igt_pm_audio_runtime_control_paths); i++) {
+ const char *path = __igt_pm_audio_runtime_control_paths[i];
+
+ fd = open(path, O_RDWR);
+ if (fd >= 0) {
+ __igt_pm_audio_runtime_control_path = path;
+ igt_assert(read(fd, __igt_pm_audio_runtime_control,
+ sizeof(__igt_pm_audio_runtime_control)) > 0);
+ strchomp(__igt_pm_audio_runtime_control);
+ igt_assert_eq(write(fd, "auto\n", 5), 5);
+ close(fd);
+ break;
+ }
}
+ if (!__igt_pm_audio_runtime_control_path)
+ igt_warn("Failed to find audio PCI device!\n");
igt_debug("Saved audio power management as '%s' and '%s'\n",
__igt_pm_audio_runtime_power_save,
@@ -291,39 +338,77 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
free(file_name);
}
#define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
-/* We just leak this on exit ... */
int pm_status_fd = -1;
static char __igt_pm_runtime_autosuspend[64];
static char __igt_pm_runtime_control[64];
-static void __igt_pm_runtime_exit_handler(int sig)
+static int __igt_restore_runtime_pm(void)
{
int fd;
- igt_debug("Restoring runtime management to '%s' and '%s'\n",
- __igt_pm_runtime_autosuspend,
- __igt_pm_runtime_control);
+ if (pm_status_fd < 0)
+ return 0;
fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
if (fd < 0)
- return;
+ return errno;
+
if (write(fd, __igt_pm_runtime_autosuspend,
strlen(__igt_pm_runtime_autosuspend)) !=
- strlen(__igt_pm_runtime_autosuspend))
- igt_warn("Failed to restore runtime pm autosuspend delay to '%s'\n",
- __igt_pm_runtime_autosuspend);
+ strlen(__igt_pm_runtime_autosuspend)) {
+ close(fd);
+ return errno;
+ }
+
close(fd);
fd = open(POWER_DIR "/control", O_WRONLY);
if (fd < 0)
- return;
+ return errno;
+
if (write(fd, __igt_pm_runtime_control,
strlen(__igt_pm_runtime_control)) !=
- strlen(__igt_pm_runtime_control))
- igt_warn("Failed to restore runtime pm control to '%s'\n",
- __igt_pm_runtime_control);
+ strlen(__igt_pm_runtime_control)) {
+ close(fd);
+ return errno;
+ }
+
close(fd);
+
+ close(pm_status_fd);
+ pm_status_fd = -1;
+
+ return 0;
+}
+
+/**
+ * igt_restore_runtime_pm:
+ *
+ * Restores the runtime PM configuration as it was before the call to
+ * igt_setup_runtime_pm.
+ */
+void igt_restore_runtime_pm(void)
+{
+ int ret;
+
+ if (pm_status_fd < 0)
+ return;
+
+ igt_debug("Restoring runtime PM management to '%s' and '%s'\n",
+ __igt_pm_runtime_autosuspend,
+ __igt_pm_runtime_control);
+
+ ret = __igt_restore_runtime_pm();
+ if (ret)
+ igt_warn("Failed to restore runtime PM! (errno=%d)\n", ret);
+
+ igt_pm_audio_restore_runtime_pm();
+}
+
+static void __igt_pm_runtime_exit_handler(int sig)
+{
+ __igt_restore_runtime_pm();
}
/**
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index eced39f8801a..10cc6794e4e7 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -47,6 +47,7 @@ enum igt_runtime_pm_status {
};
bool igt_setup_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);
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index a1d36ac4fa9d..9a20abb6b95c 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1441,6 +1441,9 @@ test_rc6(int gem_fd, unsigned int flags)
close(fw);
close(fd);
+ if (flags & TEST_RUNTIME_PM)
+ igt_restore_runtime_pm();
+
assert_within_epsilon(busy - prev, 0.0, tolerance);
}
--
2.17.1
More information about the igt-dev
mailing list