| Message ID | 20180228153506.13035-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tvrtko Ursulin (2018-02-28 15:35:06) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Some tests (the ones which call igt_setup_runtime_pm and > igt_pm_enable_audio_runtime_pm) change default system configuration and > never restore it. > > The configured runtime suspend is aggressive and may influence behaviour > of subsequent tests, so it is better to restore to previous values on test > exit. > > This way system behaviour, with regards to a random sequence of executed > tests, will be more consistent from one run to another. > > v2: Read failure means no runtime pm support so don't assert on it. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, Feb 28, 2018 at 03:35:06PM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Some tests (the ones which call igt_setup_runtime_pm and > igt_pm_enable_audio_runtime_pm) change default system configuration and > never restore it. > > The configured runtime suspend is aggressive and may influence behaviour > of subsequent tests, so it is better to restore to previous values on test > exit. > > This way system behaviour, with regards to a random sequence of executed > tests, will be more consistent from one run to another. > > v2: Read failure means no runtime pm support so don't assert on it. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1 Agreed about having a consistent expected state for each test, not sure why we didn't restore these settings :/ Btw, I feel somewhat the same about test results being affected by previous tests, but not sure if anything should/can be done about that. Acked-by: Imre Deak <imre.deak@intel.com> Some nits below. > --- > lib/igt_pm.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 117 insertions(+), 5 deletions(-) > > diff --git a/lib/igt_pm.c b/lib/igt_pm.c > index 5bf5b2e23cdc..04e2b89cca95 100644 > --- a/lib/igt_pm.c > +++ b/lib/igt_pm.c > @@ -63,6 +63,46 @@ enum { > /* Remember to fix this if adding longer strings */ > #define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR) > > +static char __igt_pm_audio_runtime_power_save[64]; > +static char __igt_pm_audio_runtime_control[64]; > + > +static void __igt_pm_audio_runtime_exit_handler(int sig) > +{ > + int fd; > + > + igt_debug("Restoring audio power management to '%s' and '%s'\n", > + __igt_pm_audio_runtime_power_save, > + __igt_pm_audio_runtime_control); > + > + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); > + if (fd < 0) > + return; > + 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); > + close(fd); > + > + fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); > + if (fd < 0) > + return; > + 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); > + close(fd); > +} > + > +static void strchomp(char *str) > +{ > + int len = strlen(str); > + > + if (len && str[len - 1] == '\n') > + str[len - 1] = 0; > +} > + > /** > * igt_pm_enable_audio_runtime_pm: > * > @@ -78,16 +118,32 @@ void igt_pm_enable_audio_runtime_pm(void) > { > int fd; > > - fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); > + /* Check if already enabled. */ > + if (__igt_pm_audio_runtime_power_save[0]) > + return; > + > + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_RDWR); > if (fd >= 0) { > + igt_assert(read(fd, __igt_pm_audio_runtime_power_save, > + sizeof(__igt_pm_audio_runtime_power_save)) > 0); > + strchomp(__igt_pm_audio_runtime_power_save); > igt_assert_eq(write(fd, "1\n", 2), 2); > + igt_install_exit_handler(__igt_pm_audio_runtime_exit_handler); Read/install_handler/write would avoid a potential race with ^C. There's also link_power_management_policy which is only restored during normal exit. > close(fd); > } > - fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); > + 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); > } > + > + igt_debug("Saved audio power management as '%s' and '%s'\n", > + __igt_pm_audio_runtime_power_save, > + __igt_pm_audio_runtime_control); > + > /* Give some time for it to react. */ > sleep(1); > } > @@ -238,6 +294,38 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data) > /* 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) > +{ > + int fd; > + > + igt_debug("Restoring runtime management to '%s' and '%s'\n", > + __igt_pm_runtime_autosuspend, > + __igt_pm_runtime_control); > + > + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); > + if (fd < 0) > + return; > + 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); > + close(fd); > + > + fd = open(POWER_DIR "/control", O_WRONLY); > + if (fd < 0) > + return; > + 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); > + close(fd); > +} > + > /** > * igt_setup_runtime_pm: > * > @@ -261,12 +349,26 @@ bool igt_setup_runtime_pm(void) > /* Our implementation uses autosuspend. Try to set it to 0ms so the test > * suite goes faster and we have a higher probability of triggering race > * conditions. */ > - fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); > + fd = open(POWER_DIR "/autosuspend_delay_ms", O_RDWR); > igt_assert_f(fd >= 0, > "Can't open " POWER_DIR "/autosuspend_delay_ms\n"); > > - /* If we fail to write to the file, it means this system doesn't support > - * runtime PM. */ > + /* > + * Save previous values to be able to install exit handler to restore > + * them on test exit. > + */ > + size = read(fd, __igt_pm_runtime_autosuspend, > + sizeof(__igt_pm_runtime_autosuspend)); > + > + /* > + * If we fail to read from the file, it means this system doesn't > + * support runtime PM. > + */ > + if (size <= 0) { > + close(fd); > + return false; > + } > + > size = write(fd, "0\n", 2); > > close(fd); > @@ -274,10 +376,20 @@ bool igt_setup_runtime_pm(void) > if (size != 2) > return false; > > + strchomp(__igt_pm_runtime_autosuspend); > + igt_install_exit_handler(__igt_pm_runtime_exit_handler); > + > /* We know we support runtime PM, let's try to enable it now. */ > fd = open(POWER_DIR "/control", O_RDWR); > igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n"); > > + igt_assert(read(fd, __igt_pm_runtime_control, > + sizeof(__igt_pm_runtime_control)) > 0); > + strchomp(__igt_pm_runtime_control); > + > + igt_debug("Saved runtime power management as '%s' and '%s'\n", > + __igt_pm_runtime_autosuspend, __igt_pm_runtime_control); > + > size = write(fd, "auto\n", 5); > igt_assert(size == 5); > > -- > 2.14.1 >
On 02/03/2018 09:29, Imre Deak wrote: > On Wed, Feb 28, 2018 at 03:35:06PM +0000, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Some tests (the ones which call igt_setup_runtime_pm and >> igt_pm_enable_audio_runtime_pm) change default system configuration and >> never restore it. >> >> The configured runtime suspend is aggressive and may influence behaviour >> of subsequent tests, so it is better to restore to previous values on test >> exit. >> >> This way system behaviour, with regards to a random sequence of executed >> tests, will be more consistent from one run to another. >> >> v2: Read failure means no runtime pm support so don't assert on it. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Imre Deak <imre.deak@intel.com> >> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1 > > Agreed about having a consistent expected state for each test, not sure > why we didn't restore these settings :/ Btw, I feel somewhat the same > about test results being affected by previous tests, but not sure if > anything should/can be done about that. > > Acked-by: Imre Deak <imre.deak@intel.com> > > Some nits below. > >> --- >> lib/igt_pm.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 117 insertions(+), 5 deletions(-) >> >> diff --git a/lib/igt_pm.c b/lib/igt_pm.c >> index 5bf5b2e23cdc..04e2b89cca95 100644 >> --- a/lib/igt_pm.c >> +++ b/lib/igt_pm.c >> @@ -63,6 +63,46 @@ enum { >> /* Remember to fix this if adding longer strings */ >> #define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR) >> >> +static char __igt_pm_audio_runtime_power_save[64]; >> +static char __igt_pm_audio_runtime_control[64]; >> + >> +static void __igt_pm_audio_runtime_exit_handler(int sig) >> +{ >> + int fd; >> + >> + igt_debug("Restoring audio power management to '%s' and '%s'\n", >> + __igt_pm_audio_runtime_power_save, >> + __igt_pm_audio_runtime_control); >> + >> + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); >> + if (fd < 0) >> + return; >> + 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); >> + close(fd); >> + >> + fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); >> + if (fd < 0) >> + return; >> + 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); >> + close(fd); >> +} >> + >> +static void strchomp(char *str) >> +{ >> + int len = strlen(str); >> + >> + if (len && str[len - 1] == '\n') >> + str[len - 1] = 0; >> +} >> + >> /** >> * igt_pm_enable_audio_runtime_pm: >> * >> @@ -78,16 +118,32 @@ void igt_pm_enable_audio_runtime_pm(void) >> { >> int fd; >> >> - fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); >> + /* Check if already enabled. */ >> + if (__igt_pm_audio_runtime_power_save[0]) >> + return; >> + >> + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_RDWR); >> if (fd >= 0) { >> + igt_assert(read(fd, __igt_pm_audio_runtime_power_save, >> + sizeof(__igt_pm_audio_runtime_power_save)) > 0); >> + strchomp(__igt_pm_audio_runtime_power_save); >> igt_assert_eq(write(fd, "1\n", 2), 2); >> + igt_install_exit_handler(__igt_pm_audio_runtime_exit_handler); > > Read/install_handler/write would avoid a potential race with ^C. There's also Well spotted, done in v3. > link_power_management_policy which is only restored during normal exit. This one already has code to restore (igt_pm_restore_sata_link_power_management) so maybe best to decide where to put the responsibility of installing the exit handler in a follow up patch? Make igt_pm_enable_sata_link_power_management do it, or have the caller do it? Former would be inline with this patch, and then probably we can unexport igt_pm_restore_sata_link_power_management. Or does it already handle this for normal exit since it is calling it from igt_fixture? Regards, Tvrtko > >> close(fd); >> } >> - fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); >> + 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); >> } >> + >> + igt_debug("Saved audio power management as '%s' and '%s'\n", >> + __igt_pm_audio_runtime_power_save, >> + __igt_pm_audio_runtime_control); >> + >> /* Give some time for it to react. */ >> sleep(1); >> } >> @@ -238,6 +294,38 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data) >> /* 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) >> +{ >> + int fd; >> + >> + igt_debug("Restoring runtime management to '%s' and '%s'\n", >> + __igt_pm_runtime_autosuspend, >> + __igt_pm_runtime_control); >> + >> + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); >> + if (fd < 0) >> + return; >> + 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); >> + close(fd); >> + >> + fd = open(POWER_DIR "/control", O_WRONLY); >> + if (fd < 0) >> + return; >> + 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); >> + close(fd); >> +} >> + >> /** >> * igt_setup_runtime_pm: >> * >> @@ -261,12 +349,26 @@ bool igt_setup_runtime_pm(void) >> /* Our implementation uses autosuspend. Try to set it to 0ms so the test >> * suite goes faster and we have a higher probability of triggering race >> * conditions. */ >> - fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); >> + fd = open(POWER_DIR "/autosuspend_delay_ms", O_RDWR); >> igt_assert_f(fd >= 0, >> "Can't open " POWER_DIR "/autosuspend_delay_ms\n"); >> >> - /* If we fail to write to the file, it means this system doesn't support >> - * runtime PM. */ >> + /* >> + * Save previous values to be able to install exit handler to restore >> + * them on test exit. >> + */ >> + size = read(fd, __igt_pm_runtime_autosuspend, >> + sizeof(__igt_pm_runtime_autosuspend)); >> + >> + /* >> + * If we fail to read from the file, it means this system doesn't >> + * support runtime PM. >> + */ >> + if (size <= 0) { >> + close(fd); >> + return false; >> + } >> + >> size = write(fd, "0\n", 2); >> >> close(fd); >> @@ -274,10 +376,20 @@ bool igt_setup_runtime_pm(void) >> if (size != 2) >> return false; >> >> + strchomp(__igt_pm_runtime_autosuspend); >> + igt_install_exit_handler(__igt_pm_runtime_exit_handler); >> + >> /* We know we support runtime PM, let's try to enable it now. */ >> fd = open(POWER_DIR "/control", O_RDWR); >> igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n"); >> >> + igt_assert(read(fd, __igt_pm_runtime_control, >> + sizeof(__igt_pm_runtime_control)) > 0); >> + strchomp(__igt_pm_runtime_control); >> + >> + igt_debug("Saved runtime power management as '%s' and '%s'\n", >> + __igt_pm_runtime_autosuspend, __igt_pm_runtime_control); >> + >> size = write(fd, "auto\n", 5); >> igt_assert(size == 5); >> >> -- >> 2.14.1 >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Fri, Mar 02, 2018 at 09:56:26AM +0000, Tvrtko Ursulin wrote: > > On 02/03/2018 09:29, Imre Deak wrote: > > On Wed, Feb 28, 2018 at 03:35:06PM +0000, Tvrtko Ursulin wrote: > > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > > > > Some tests (the ones which call igt_setup_runtime_pm and > > > igt_pm_enable_audio_runtime_pm) change default system configuration and > > > never restore it. > > > > > > The configured runtime suspend is aggressive and may influence behaviour > > > of subsequent tests, so it is better to restore to previous values on test > > > exit. > > > > > > This way system behaviour, with regards to a random sequence of executed > > > tests, will be more consistent from one run to another. > > > > > > v2: Read failure means no runtime pm support so don't assert on it. > > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > Cc: Imre Deak <imre.deak@intel.com> > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1 > > > > Agreed about having a consistent expected state for each test, not sure > > why we didn't restore these settings :/ Btw, I feel somewhat the same > > about test results being affected by previous tests, but not sure if > > anything should/can be done about that. > > > > Acked-by: Imre Deak <imre.deak@intel.com> > > > > Some nits below. > > > > > --- > > > lib/igt_pm.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 117 insertions(+), 5 deletions(-) > > > > > > diff --git a/lib/igt_pm.c b/lib/igt_pm.c > > > index 5bf5b2e23cdc..04e2b89cca95 100644 > > > --- a/lib/igt_pm.c > > > +++ b/lib/igt_pm.c > > > @@ -63,6 +63,46 @@ enum { > > > /* Remember to fix this if adding longer strings */ > > > #define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR) > > > +static char __igt_pm_audio_runtime_power_save[64]; > > > +static char __igt_pm_audio_runtime_control[64]; > > > + > > > +static void __igt_pm_audio_runtime_exit_handler(int sig) > > > +{ > > > + int fd; > > > + > > > + igt_debug("Restoring audio power management to '%s' and '%s'\n", > > > + __igt_pm_audio_runtime_power_save, > > > + __igt_pm_audio_runtime_control); > > > + > > > + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); > > > + if (fd < 0) > > > + return; > > > + 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); > > > + close(fd); > > > + > > > + fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); > > > + if (fd < 0) > > > + return; > > > + 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); > > > + close(fd); > > > +} > > > + > > > +static void strchomp(char *str) > > > +{ > > > + int len = strlen(str); > > > + > > > + if (len && str[len - 1] == '\n') > > > + str[len - 1] = 0; > > > +} > > > + > > > /** > > > * igt_pm_enable_audio_runtime_pm: > > > * > > > @@ -78,16 +118,32 @@ void igt_pm_enable_audio_runtime_pm(void) > > > { > > > int fd; > > > - fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); > > > + /* Check if already enabled. */ > > > + if (__igt_pm_audio_runtime_power_save[0]) > > > + return; > > > + > > > + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_RDWR); > > > if (fd >= 0) { > > > + igt_assert(read(fd, __igt_pm_audio_runtime_power_save, > > > + sizeof(__igt_pm_audio_runtime_power_save)) > 0); > > > + strchomp(__igt_pm_audio_runtime_power_save); > > > igt_assert_eq(write(fd, "1\n", 2), 2); > > > + igt_install_exit_handler(__igt_pm_audio_runtime_exit_handler); > > > > Read/install_handler/write would avoid a potential race with ^C. There's also > > Well spotted, done in v3. Ok, for that one: Reviewed-by: Imre Deak <imre.deak@intel.com> > > > link_power_management_policy which is only restored during normal exit. > > This one already has code to restore > (igt_pm_restore_sata_link_power_management) so maybe best to decide where to > put the responsibility of installing the exit handler in a follow up patch? > Make igt_pm_enable_sata_link_power_management do it, or have the caller do > it? Former would be inline with this patch, and then probably we can > unexport igt_pm_restore_sata_link_power_management. Or does it already > handle this for normal exit since it is calling it from igt_fixture? Yes, it's handled for normal exit via igt_fixture, but I think it should be restored the same way as you did here. But yea, it's fine to do that as a follow-up. > > Regards, > > Tvrtko > > > > > > close(fd); > > > } > > > - fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); > > > + 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); > > > } > > > + > > > + igt_debug("Saved audio power management as '%s' and '%s'\n", > > > + __igt_pm_audio_runtime_power_save, > > > + __igt_pm_audio_runtime_control); > > > + > > > /* Give some time for it to react. */ > > > sleep(1); > > > } > > > @@ -238,6 +294,38 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data) > > > /* 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) > > > +{ > > > + int fd; > > > + > > > + igt_debug("Restoring runtime management to '%s' and '%s'\n", > > > + __igt_pm_runtime_autosuspend, > > > + __igt_pm_runtime_control); > > > + > > > + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); > > > + if (fd < 0) > > > + return; > > > + 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); > > > + close(fd); > > > + > > > + fd = open(POWER_DIR "/control", O_WRONLY); > > > + if (fd < 0) > > > + return; > > > + 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); > > > + close(fd); > > > +} > > > + > > > /** > > > * igt_setup_runtime_pm: > > > * > > > @@ -261,12 +349,26 @@ bool igt_setup_runtime_pm(void) > > > /* Our implementation uses autosuspend. Try to set it to 0ms so the test > > > * suite goes faster and we have a higher probability of triggering race > > > * conditions. */ > > > - fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); > > > + fd = open(POWER_DIR "/autosuspend_delay_ms", O_RDWR); > > > igt_assert_f(fd >= 0, > > > "Can't open " POWER_DIR "/autosuspend_delay_ms\n"); > > > - /* If we fail to write to the file, it means this system doesn't support > > > - * runtime PM. */ > > > + /* > > > + * Save previous values to be able to install exit handler to restore > > > + * them on test exit. > > > + */ > > > + size = read(fd, __igt_pm_runtime_autosuspend, > > > + sizeof(__igt_pm_runtime_autosuspend)); > > > + > > > + /* > > > + * If we fail to read from the file, it means this system doesn't > > > + * support runtime PM. > > > + */ > > > + if (size <= 0) { > > > + close(fd); > > > + return false; > > > + } > > > + > > > size = write(fd, "0\n", 2); > > > close(fd); > > > @@ -274,10 +376,20 @@ bool igt_setup_runtime_pm(void) > > > if (size != 2) > > > return false; > > > + strchomp(__igt_pm_runtime_autosuspend); > > > + igt_install_exit_handler(__igt_pm_runtime_exit_handler); > > > + > > > /* We know we support runtime PM, let's try to enable it now. */ > > > fd = open(POWER_DIR "/control", O_RDWR); > > > igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n"); > > > + igt_assert(read(fd, __igt_pm_runtime_control, > > > + sizeof(__igt_pm_runtime_control)) > 0); > > > + strchomp(__igt_pm_runtime_control); > > > + > > > + igt_debug("Saved runtime power management as '%s' and '%s'\n", > > > + __igt_pm_runtime_autosuspend, __igt_pm_runtime_control); > > > + > > > size = write(fd, "auto\n", 5); > > > igt_assert(size == 5); > > > -- > > > 2.14.1 > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 5bf5b2e23cdc..04e2b89cca95 100644 --- a/lib/igt_pm.c +++ b/lib/igt_pm.c @@ -63,6 +63,46 @@ enum { /* Remember to fix this if adding longer strings */ #define MAX_POLICY_STRLEN strlen(MAX_PERFORMANCE_STR) +static char __igt_pm_audio_runtime_power_save[64]; +static char __igt_pm_audio_runtime_control[64]; + +static void __igt_pm_audio_runtime_exit_handler(int sig) +{ + int fd; + + igt_debug("Restoring audio power management to '%s' and '%s'\n", + __igt_pm_audio_runtime_power_save, + __igt_pm_audio_runtime_control); + + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); + if (fd < 0) + return; + 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); + close(fd); + + fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); + if (fd < 0) + return; + 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); + close(fd); +} + +static void strchomp(char *str) +{ + int len = strlen(str); + + if (len && str[len - 1] == '\n') + str[len - 1] = 0; +} + /** * igt_pm_enable_audio_runtime_pm: * @@ -78,16 +118,32 @@ void igt_pm_enable_audio_runtime_pm(void) { int fd; - fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY); + /* Check if already enabled. */ + if (__igt_pm_audio_runtime_power_save[0]) + return; + + fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_RDWR); if (fd >= 0) { + igt_assert(read(fd, __igt_pm_audio_runtime_power_save, + sizeof(__igt_pm_audio_runtime_power_save)) > 0); + strchomp(__igt_pm_audio_runtime_power_save); igt_assert_eq(write(fd, "1\n", 2), 2); + igt_install_exit_handler(__igt_pm_audio_runtime_exit_handler); close(fd); } - fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY); + 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); } + + igt_debug("Saved audio power management as '%s' and '%s'\n", + __igt_pm_audio_runtime_power_save, + __igt_pm_audio_runtime_control); + /* Give some time for it to react. */ sleep(1); } @@ -238,6 +294,38 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data) /* 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) +{ + int fd; + + igt_debug("Restoring runtime management to '%s' and '%s'\n", + __igt_pm_runtime_autosuspend, + __igt_pm_runtime_control); + + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); + if (fd < 0) + return; + 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); + close(fd); + + fd = open(POWER_DIR "/control", O_WRONLY); + if (fd < 0) + return; + 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); + close(fd); +} + /** * igt_setup_runtime_pm: * @@ -261,12 +349,26 @@ bool igt_setup_runtime_pm(void) /* Our implementation uses autosuspend. Try to set it to 0ms so the test * suite goes faster and we have a higher probability of triggering race * conditions. */ - fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); + fd = open(POWER_DIR "/autosuspend_delay_ms", O_RDWR); igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/autosuspend_delay_ms\n"); - /* If we fail to write to the file, it means this system doesn't support - * runtime PM. */ + /* + * Save previous values to be able to install exit handler to restore + * them on test exit. + */ + size = read(fd, __igt_pm_runtime_autosuspend, + sizeof(__igt_pm_runtime_autosuspend)); + + /* + * If we fail to read from the file, it means this system doesn't + * support runtime PM. + */ + if (size <= 0) { + close(fd); + return false; + } + size = write(fd, "0\n", 2); close(fd); @@ -274,10 +376,20 @@ bool igt_setup_runtime_pm(void) if (size != 2) return false; + strchomp(__igt_pm_runtime_autosuspend); + igt_install_exit_handler(__igt_pm_runtime_exit_handler); + /* We know we support runtime PM, let's try to enable it now. */ fd = open(POWER_DIR "/control", O_RDWR); igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n"); + igt_assert(read(fd, __igt_pm_runtime_control, + sizeof(__igt_pm_runtime_control)) > 0); + strchomp(__igt_pm_runtime_control); + + igt_debug("Saved runtime power management as '%s' and '%s'\n", + __igt_pm_runtime_autosuspend, __igt_pm_runtime_control); + size = write(fd, "auto\n", 5); igt_assert(size == 5);