| Message ID | 20180720094255.22668-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tvrtko Ursulin (2018-07-20 10:42:55) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Restore runtime PM state (via a newly added library function) when the > test which sets it up exit. This was we avoid running all subsequent sub- > tests in the aggressive runtime PM mode. > > v2: > * Skip double restore. (Chris Wilson) > * Close previously leaked fd. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > lib/igt_pm.c | 20 ++++++++++++++++++-- > lib/igt_pm.h | 1 + > tests/perf_pmu.c | 3 +++ > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/lib/igt_pm.c b/lib/igt_pm.c > index 8ac132269d79..512ad9b31951 100644 > --- a/lib/igt_pm.c > +++ b/lib/igt_pm.c > @@ -291,16 +291,24 @@ 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) > +/** > + * 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 fd; > > + if (pm_status_fd < 0) > + return; > + > igt_debug("Restoring runtime management to '%s' and '%s'\n", > __igt_pm_runtime_autosuspend, > __igt_pm_runtime_control); Hmm. Did we make our igt_debug() signal safe? The open/read/write/close are ok. igt_vlog() is definetly not signal-safe. Not the fault of this patch, but I strongly advise that the debug/warn are removed from this function and handled in the caller for !exit_handler. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 8ac132269d79..512ad9b31951 100644 --- a/lib/igt_pm.c +++ b/lib/igt_pm.c @@ -291,16 +291,24 @@ 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) +/** + * 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 fd; + if (pm_status_fd < 0) + return; + igt_debug("Restoring runtime management to '%s' and '%s'\n", __igt_pm_runtime_autosuspend, __igt_pm_runtime_control); @@ -324,6 +332,14 @@ static void __igt_pm_runtime_exit_handler(int sig) igt_warn("Failed to restore runtime pm control to '%s'\n", __igt_pm_runtime_control); close(fd); + + close(pm_status_fd); + pm_status_fd = -1; +} + +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); }