* [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 11:08 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 11:08 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx 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. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Imre Deak <imre.deak@intel.com> --- lib/igt_pm.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 5bf5b2e23cdc..0c8b5e8e9257 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 (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,10 +349,16 @@ 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"); + /* Save previous values and install exit handler to restore them. */ + igt_assert(read(fd, __igt_pm_runtime_autosuspend, + sizeof(__igt_pm_runtime_autosuspend)) > 0); + strchomp(__igt_pm_runtime_autosuspend); + igt_install_exit_handler(__igt_pm_runtime_exit_handler); + /* If we fail to write to the file, it means this system doesn't support * runtime PM. */ size = write(fd, "0\n", 2); @@ -278,6 +372,13 @@ bool igt_setup_runtime_pm(void) 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 11:08 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 11:08 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin 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. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Imre Deak <imre.deak@intel.com> --- lib/igt_pm.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 5bf5b2e23cdc..0c8b5e8e9257 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 (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,10 +349,16 @@ 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"); + /* Save previous values and install exit handler to restore them. */ + igt_assert(read(fd, __igt_pm_runtime_autosuspend, + sizeof(__igt_pm_runtime_autosuspend)) > 0); + strchomp(__igt_pm_runtime_autosuspend); + igt_install_exit_handler(__igt_pm_runtime_exit_handler); + /* If we fail to write to the file, it means this system doesn't support * runtime PM. */ size = write(fd, "0\n", 2); @@ -278,6 +372,13 @@ bool igt_setup_runtime_pm(void) 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 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin @ 2018-02-28 11:12 ` Chris Wilson -1 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 11:12 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > 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. Otoh, if behaviour changes in subsequent tests, we likely have a lack of testing :( -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 11:12 ` Chris Wilson 0 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 11:12 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > 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. Otoh, if behaviour changes in subsequent tests, we likely have a lack of testing :( -Chris _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:12 ` [igt-dev] [Intel-gfx] " Chris Wilson @ 2018-02-28 11:38 ` Tvrtko Ursulin -1 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 11:38 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx On 28/02/2018 11:12, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-02-28 11:08:29) >> 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. > > Otoh, if behaviour changes in subsequent tests, we likely have a lack of > testing :( Yeah, and I am not saying it does - haven't spotted anything like that, just that it leaves the auto-suspend with zero delay afterwards, compared to otherwise default 10s. Maybe it is good for coverage, even with the downside of randomness considering shard runs, or maybe it needs to be more explicit. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 11:38 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 11:38 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx On 28/02/2018 11:12, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-02-28 11:08:29) >> 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. > > Otoh, if behaviour changes in subsequent tests, we likely have a lack of > testing :( Yeah, and I am not saying it does - haven't spotted anything like that, just that it leaves the auto-suspend with zero delay afterwards, compared to otherwise default 10s. Maybe it is good for coverage, even with the downside of randomness considering shard runs, or maybe it needs to be more explicit. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:38 ` [Intel-gfx] " Tvrtko Ursulin @ 2018-02-28 12:27 ` Chris Wilson -1 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 12:27 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:38:01) > > On 28/02/2018 11:12, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > >> 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. > > > > Otoh, if behaviour changes in subsequent tests, we likely have a lack of > > testing :( > > Yeah, and I am not saying it does - haven't spotted anything like that, > just that it leaves the auto-suspend with zero delay afterwards, > compared to otherwise default 10s. > > Maybe it is good for coverage, even with the downside of randomness > considering shard runs, or maybe it needs to be more explicit. I actually thought (or at least picked up the idea) we enabled 0 autosuspend delay throughout igt. And that it's only because most of time we have a display connected that prevents rpm madness. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 12:27 ` Chris Wilson 0 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 12:27 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:38:01) > > On 28/02/2018 11:12, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > >> 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. > > > > Otoh, if behaviour changes in subsequent tests, we likely have a lack of > > testing :( > > Yeah, and I am not saying it does - haven't spotted anything like that, > just that it leaves the auto-suspend with zero delay afterwards, > compared to otherwise default 10s. > > Maybe it is good for coverage, even with the downside of randomness > considering shard runs, or maybe it needs to be more explicit. I actually thought (or at least picked up the idea) we enabled 0 autosuspend delay throughout igt. And that it's only because most of time we have a display connected that prevents rpm madness. -Chris _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 12:27 ` [igt-dev] [Intel-gfx] " Chris Wilson @ 2018-02-28 14:33 ` Tvrtko Ursulin -1 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 14:33 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx On 28/02/2018 12:27, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-02-28 11:38:01) >> >> On 28/02/2018 11:12, Chris Wilson wrote: >>> Quoting Tvrtko Ursulin (2018-02-28 11:08:29) >>>> 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. >>> >>> Otoh, if behaviour changes in subsequent tests, we likely have a lack of >>> testing :( >> >> Yeah, and I am not saying it does - haven't spotted anything like that, >> just that it leaves the auto-suspend with zero delay afterwards, >> compared to otherwise default 10s. >> >> Maybe it is good for coverage, even with the downside of randomness >> considering shard runs, or maybe it needs to be more explicit. > > I actually thought (or at least picked up the idea) we enabled 0 > autosuspend delay throughout igt. And that it's only because most of time > we have a display connected that prevents rpm madness. Yes fair point, the effect of this patch is only visible on esoteric headless setups like mine. :) Anyway, unless I am missing something, I think it is conceptually correct to restore, as long as the zero autosuspend delay is not a global IGT setup but a helper called by a few tests. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 14:33 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 14:33 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx On 28/02/2018 12:27, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-02-28 11:38:01) >> >> On 28/02/2018 11:12, Chris Wilson wrote: >>> Quoting Tvrtko Ursulin (2018-02-28 11:08:29) >>>> 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. >>> >>> Otoh, if behaviour changes in subsequent tests, we likely have a lack of >>> testing :( >> >> Yeah, and I am not saying it does - haven't spotted anything like that, >> just that it leaves the auto-suspend with zero delay afterwards, >> compared to otherwise default 10s. >> >> Maybe it is good for coverage, even with the downside of randomness >> considering shard runs, or maybe it needs to be more explicit. > > I actually thought (or at least picked up the idea) we enabled 0 > autosuspend delay throughout igt. And that it's only because most of time > we have a display connected that prevents rpm madness. Yes fair point, the effect of this patch is only visible on esoteric headless setups like mine. :) Anyway, unless I am missing something, I think it is conceptually correct to restore, as long as the zero autosuspend delay is not a global IGT setup but a helper called by a few tests. Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin (?) (?) @ 2018-02-28 12:50 ` Patchwork 2018-02-28 15:17 ` Tvrtko Ursulin -1 siblings, 1 reply; 31+ messages in thread From: Patchwork @ 2018-02-28 12:50 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit URL : https://patchwork.freedesktop.org/series/39106/ State : failure == Summary == IGT patchset tested on top of latest successful build d3dc9c619791c8cb88d176fc7b3d8aa5e802055d lib/igt_draw: Fix bo leak in gpu draw routines with latest DRM-Tip kernel build CI_DRM_3846 9a02ae14ae02 drm-tip: 2018y-02m-28d-10h-13m-18s UTC integration manifest No testlist changes. ---- Possible new issues: Test pm_rpm: Subgroup basic-pci-d3-state: skip -> FAIL (fi-bdw-gvtdvm) skip -> FAIL (fi-blb-e6850) skip -> FAIL (fi-bwr-2160) skip -> FAIL (fi-elk-e7500) skip -> FAIL (fi-gdg-551) skip -> FAIL (fi-ilk-650) skip -> FAIL (fi-ivb-3520m) skip -> FAIL (fi-ivb-3770) skip -> FAIL (fi-pnv-d510) skip -> FAIL (fi-skl-gvtdvm) skip -> FAIL (fi-snb-2520m) skip -> FAIL (fi-snb-2600) Subgroup basic-rte: skip -> FAIL (fi-bdw-gvtdvm) skip -> FAIL (fi-blb-e6850) skip -> FAIL (fi-bwr-2160) skip -> FAIL (fi-elk-e7500) skip -> FAIL (fi-gdg-551) skip -> FAIL (fi-ilk-650) skip -> FAIL (fi-ivb-3520m) skip -> FAIL (fi-ivb-3770) skip -> FAIL (fi-pnv-d510) skip -> FAIL (fi-skl-gvtdvm) skip -> FAIL (fi-snb-2520m) skip -> FAIL (fi-snb-2600) fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:416s fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:2 skip:22 time:428s fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:2 skip:62 time:375s fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:489s fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:2 skip:103 time:285s fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:483s fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:471s fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:457s fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:394s fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:563s fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:574s fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:2 skip:57 time:415s fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:3 skip:106 time:285s fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:507s fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:390s fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:2 skip:58 time:410s fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:2 skip:27 time:453s fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:2 skip:31 time:410s fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:457s fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:484s fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:455s fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:496s fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:2 skip:63 time:587s fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:427s fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:508s fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:518s fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:491s fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:474s fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:407s fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:2 skip:21 time:429s fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:2 skip:38 time:518s fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:2 skip:38 time:393s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1021/issues.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] ✗ Fi.CI.BAT: failure for lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 12:50 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork @ 2018-02-28 15:17 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 15:17 UTC (permalink / raw) To: igt-dev, Patchwork, Tvrtko Ursulin On 28/02/2018 12:50, Patchwork wrote: > == Series Details == > > Series: lib/igt_pm: Restore runtime pm state on test exit > URL : https://patchwork.freedesktop.org/series/39106/ > State : failure > > == Summary == > > IGT patchset tested on top of latest successful build > d3dc9c619791c8cb88d176fc7b3d8aa5e802055d lib/igt_draw: Fix bo leak in gpu draw routines > > with latest DRM-Tip kernel build CI_DRM_3846 > 9a02ae14ae02 drm-tip: 2018y-02m-28d-10h-13m-18s UTC integration manifest > > No testlist changes. > > ---- Possible new issues: > > Test pm_rpm: > Subgroup basic-pci-d3-state: > skip -> FAIL (fi-bdw-gvtdvm) > skip -> FAIL (fi-blb-e6850) > skip -> FAIL (fi-bwr-2160) > skip -> FAIL (fi-elk-e7500) > skip -> FAIL (fi-gdg-551) > skip -> FAIL (fi-ilk-650) > skip -> FAIL (fi-ivb-3520m) > skip -> FAIL (fi-ivb-3770) > skip -> FAIL (fi-pnv-d510) > skip -> FAIL (fi-skl-gvtdvm) > skip -> FAIL (fi-snb-2520m) > skip -> FAIL (fi-snb-2600) > Subgroup basic-rte: > skip -> FAIL (fi-bdw-gvtdvm) > skip -> FAIL (fi-blb-e6850) > skip -> FAIL (fi-bwr-2160) > skip -> FAIL (fi-elk-e7500) > skip -> FAIL (fi-gdg-551) > skip -> FAIL (fi-ilk-650) > skip -> FAIL (fi-ivb-3520m) > skip -> FAIL (fi-ivb-3770) > skip -> FAIL (fi-pnv-d510) > skip -> FAIL (fi-skl-gvtdvm) > skip -> FAIL (fi-snb-2520m) > skip -> FAIL (fi-snb-2600) Must not assert on reads if runtime pm is not supported... Regards, Tvrtko _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin @ 2018-02-28 14:23 ` Chris Wilson -1 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 14:23 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > 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. > > 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> -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 14:23 ` Chris Wilson 0 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-02-28 14:23 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx Quoting Tvrtko Ursulin (2018-02-28 11:08:29) > 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. > > 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> -Chris _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin @ 2018-02-28 15:35 ` Tvrtko Ursulin -1 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 15:35 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx 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 --- 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); 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit @ 2018-02-28 15:35 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-02-28 15:35 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx 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 --- 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); 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 15:35 ` [Intel-gfx] " Tvrtko Ursulin @ 2018-03-01 8:12 ` Chris Wilson -1 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-03-01 8:12 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit @ 2018-03-01 8:12 ` Chris Wilson 0 siblings, 0 replies; 31+ messages in thread From: Chris Wilson @ 2018-03-01 8:12 UTC (permalink / raw) To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx 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 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit 2018-02-28 15:35 ` [Intel-gfx] " Tvrtko Ursulin @ 2018-03-02 9:29 ` Imre Deak -1 siblings, 0 replies; 31+ messages in thread From: Imre Deak @ 2018-03-02 9:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx 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 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit @ 2018-03-02 9:29 ` Imre Deak 0 siblings, 0 replies; 31+ messages in thread From: Imre Deak @ 2018-03-02 9:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin 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 > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH i-g-t v3] lib/igt_pm: Restore runtime pm state on test exit 2018-03-02 9:29 ` [igt-dev] " Imre Deak @ 2018-03-02 9:51 ` Tvrtko Ursulin -1 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-03-02 9:51 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx 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. v3: Install exit handler before the write to close the Ctrl^C race. (Imre) 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> # v2 Acked-by: Imre Deak <imre.deak@intel.com> --- 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..8ac132269d79 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_install_exit_handler(__igt_pm_audio_runtime_exit_handler); igt_assert_eq(write(fd, "1\n", 2), 2); 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,29 @@ 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; + } + + strchomp(__igt_pm_runtime_autosuspend); + igt_install_exit_handler(__igt_pm_runtime_exit_handler); + size = write(fd, "0\n", 2); close(fd); @@ -278,6 +383,13 @@ bool igt_setup_runtime_pm(void) 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t v3] lib/igt_pm: Restore runtime pm state on test exit @ 2018-03-02 9:51 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-03-02 9:51 UTC (permalink / raw) To: igt-dev; +Cc: Intel-gfx 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. v3: Install exit handler before the write to close the Ctrl^C race. (Imre) 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> # v2 Acked-by: Imre Deak <imre.deak@intel.com> --- 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..8ac132269d79 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_install_exit_handler(__igt_pm_audio_runtime_exit_handler); igt_assert_eq(write(fd, "1\n", 2), 2); 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,29 @@ 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; + } + + strchomp(__igt_pm_runtime_autosuspend); + igt_install_exit_handler(__igt_pm_runtime_exit_handler); + size = write(fd, "0\n", 2); close(fd); @@ -278,6 +383,13 @@ bool igt_setup_runtime_pm(void) 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 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit 2018-03-02 9:29 ` [igt-dev] " Imre Deak @ 2018-03-02 9:56 ` Tvrtko Ursulin -1 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-03-02 9:56 UTC (permalink / raw) To: imre.deak, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx 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 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit @ 2018-03-02 9:56 ` Tvrtko Ursulin 0 siblings, 0 replies; 31+ messages in thread From: Tvrtko Ursulin @ 2018-03-02 9:56 UTC (permalink / raw) To: imre.deak, Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx 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 > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit 2018-03-02 9:56 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin @ 2018-03-02 12:29 ` Imre Deak -1 siblings, 0 replies; 31+ messages in thread From: Imre Deak @ 2018-03-02 12:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, 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 > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] lib/igt_pm: Restore runtime pm state on test exit @ 2018-03-02 12:29 ` Imre Deak 0 siblings, 0 replies; 31+ messages in thread From: Imre Deak @ 2018-03-02 12:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev, 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 > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_pm: Restore runtime pm state on test exit (rev2) 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin ` (4 preceding siblings ...) (?) @ 2018-02-28 16:33 ` Patchwork -1 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2018-02-28 16:33 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit (rev2) URL : https://patchwork.freedesktop.org/series/39106/ State : success == Summary == IGT patchset tested on top of latest successful build 0d47ec161b4eca1b41c5348604aa05b105e5d1cf tests/perf: Fix build warning with latest DRM-Tip kernel build CI_DRM_3849 4d8ce7ac845a drm-tip: 2018y-02m-28d-14h-17m-44s UTC integration manifest No testlist changes. ---- Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927 Test prime_vgem: Subgroup basic-fence-flip: pass -> FAIL (fi-ilk-650) fdo#104008 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:416s fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:375s fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:483s fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:285s fi-bxt-dsi total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:26 fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:484s fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:470s fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:461s fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:394s fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:557s fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:590s fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:416s fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:283s fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:508s fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:388s fi-ilk-650 total:288 pass:227 dwarn:0 dfail:0 fail:1 skip:60 time:407s fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:456s fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:420s fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:453s fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:489s fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:452s fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:491s fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:586s fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:434s fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:501s fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:522s fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:478s fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:409s fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:435s fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:528s fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:395s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1025/issues.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for lib/igt_pm: Restore runtime pm state on test exit (rev2) 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin ` (5 preceding siblings ...) (?) @ 2018-02-28 20:22 ` Patchwork -1 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2018-02-28 20:22 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit (rev2) URL : https://patchwork.freedesktop.org/series/39106/ State : failure == Summary == ---- Possible new issues: Test kms_color: Subgroup pipe-a-ctm-0-25: pass -> FAIL (shard-apl) ---- Known issues: Test gem_eio: Subgroup in-flight-contexts: pass -> INCOMPLETE (shard-apl) fdo#104945 Test kms_chv_cursor_fail: Subgroup pipe-b-256x256-left-edge: dmesg-warn -> PASS (shard-snb) fdo#105185 Test kms_flip: Subgroup 2x-flip-vs-expired-vblank-interruptible: pass -> FAIL (shard-hsw) fdo#102887 Subgroup 2x-plain-flip-ts-check: pass -> FAIL (shard-hsw) fdo#100368 +1 Test kms_frontbuffer_tracking: Subgroup fbc-1p-pri-indfb-multidraw: fail -> PASS (shard-snb) fdo#103167 Test kms_rotation_crc: Subgroup primary-rotation-180: pass -> FAIL (shard-snb) fdo#103925 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 Test kms_vblank: Subgroup pipe-a-accuracy-idle: pass -> FAIL (shard-hsw) fdo#102583 fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945 fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583 shard-apl total:3425 pass:1801 dwarn:1 dfail:0 fail:8 skip:1613 time:11927s shard-hsw total:3460 pass:1763 dwarn:1 dfail:0 fail:5 skip:1690 time:11556s shard-snb total:3460 pass:1358 dwarn:1 dfail:0 fail:2 skip:2099 time:6698s Blacklisted hosts: shard-kbl total:3425 pass:1920 dwarn:1 dfail:0 fail:9 skip:1494 time:9160s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1025/shards.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for lib/igt_pm: Restore runtime pm state on test exit (rev3) 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin ` (6 preceding siblings ...) (?) @ 2018-03-02 18:21 ` Patchwork -1 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2018-03-02 18:21 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit (rev3) URL : https://patchwork.freedesktop.org/series/39106/ State : failure == Summary == IGT patchset tested on top of latest successful build bddfb8dd3c1767f13d2af578d5c3d897fddf0dcd igt/gem_ctx_switch: Exercise all engines at once with latest DRM-Tip kernel build CI_DRM_3867 4f4e4dd52a30 drm-tip: 2018y-03m-02d-16h-28m-21s UTC integration manifest No testlist changes. ---- Possible new issues: Test prime_vgem: Subgroup basic-fence-flip: pass -> FAIL (fi-hsw-4770) fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:420s fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:429s fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:493s fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:279s fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:478s fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:483s fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:470s fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:457s fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:395s fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:560s fi-cfl-u total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:495s fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:587s fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:417s fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:289s fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:508s fi-hsw-4770 total:288 pass:260 dwarn:0 dfail:0 fail:1 skip:27 time:388s fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:411s fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:454s fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:414s fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:453s fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:491s fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:446s fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:496s fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:583s fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:429s fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:506s fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:519s fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:489s fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:479s fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:410s fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:430s fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:517s fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:393s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1043/issues.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_pm: Restore runtime pm state on test exit (rev3) 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin ` (7 preceding siblings ...) (?) @ 2018-03-05 9:04 ` Patchwork -1 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2018-03-05 9:04 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit (rev3) URL : https://patchwork.freedesktop.org/series/39106/ State : success == Summary == IGT patchset tested on top of latest successful build ec872b7dc1fe68d153aaceaf74d55865719a76da tests/perf_pmu: Handle CPU hotplug failures better with latest DRM-Tip kernel build CI_DRM_3868 c07ef2c77d14 drm-tip: 2018y-03m-03d-08h-39m-05s UTC integration manifest No testlist changes. ---- Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 +1 Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: pass -> FAIL (fi-gdg-551) fdo#102575 Test kms_chamelium: Subgroup dp-edid-read: fail -> PASS (fi-kbl-7500u) fdo#102505 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505 fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:418s fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:423s fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:490s fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:280s fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:480s fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:485s fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:470s fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:463s fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:395s fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:563s fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:418s fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:290s fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:512s fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:391s fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:410s fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:451s fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:413s fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:453s fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:493s fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:451s fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:494s fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:585s fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:423s fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:499s fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:524s fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:486s fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:473s fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:409s fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:437s fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33 fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:406s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1050/issues.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_pm: Restore runtime pm state on test exit (rev3) 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin ` (8 preceding siblings ...) (?) @ 2018-03-05 10:20 ` Patchwork -1 siblings, 0 replies; 31+ messages in thread From: Patchwork @ 2018-03-05 10:20 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: igt-dev == Series Details == Series: lib/igt_pm: Restore runtime pm state on test exit (rev3) URL : https://patchwork.freedesktop.org/series/39106/ State : success == Summary == ---- Possible new issues: Test kms_vblank: Subgroup pipe-a-ts-continuation-suspend: skip -> PASS (shard-snb) Test pm_rc6_residency: Subgroup rc6-accuracy: skip -> PASS (shard-snb) ---- Known issues: Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (shard-hsw) fdo#103928 Test kms_rotation_crc: Subgroup primary-rotation-180: fail -> PASS (shard-snb) fdo#103925 Test kms_sysfs_edid_timing: warn -> PASS (shard-apl) fdo#100047 Test kms_vblank: Subgroup pipe-a-ts-continuation-suspend: pass -> INCOMPLETE (shard-hsw) fdo#103540 fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 shard-apl total:3392 pass:1780 dwarn:1 dfail:0 fail:7 skip:1603 time:11820s shard-hsw total:3377 pass:1727 dwarn:1 dfail:0 fail:1 skip:1646 time:11555s shard-snb total:3468 pass:1365 dwarn:2 dfail:0 fail:1 skip:2100 time:7099s Blacklisted hosts: shard-kbl total:3392 pass:1901 dwarn:4 dfail:0 fail:7 skip:1479 time:9022s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1050/shards.html _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2018-03-05 10:20 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-28 11:08 [PATCH i-g-t] lib/igt_pm: Restore runtime pm state on test exit Tvrtko Ursulin 2018-02-28 11:08 ` [igt-dev] " Tvrtko Ursulin 2018-02-28 11:12 ` Chris Wilson 2018-02-28 11:12 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-02-28 11:38 ` Tvrtko Ursulin 2018-02-28 11:38 ` [Intel-gfx] " Tvrtko Ursulin 2018-02-28 12:27 ` Chris Wilson 2018-02-28 12:27 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-02-28 14:33 ` Tvrtko Ursulin 2018-02-28 14:33 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin 2018-02-28 12:50 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork 2018-02-28 15:17 ` Tvrtko Ursulin 2018-02-28 14:23 ` [PATCH i-g-t] " Chris Wilson 2018-02-28 14:23 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-02-28 15:35 ` [PATCH i-g-t v2] " Tvrtko Ursulin 2018-02-28 15:35 ` [Intel-gfx] " Tvrtko Ursulin 2018-03-01 8:12 ` Chris Wilson 2018-03-01 8:12 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-03-02 9:29 ` Imre Deak 2018-03-02 9:29 ` [igt-dev] " Imre Deak 2018-03-02 9:51 ` [PATCH i-g-t v3] " Tvrtko Ursulin 2018-03-02 9:51 ` [Intel-gfx] " Tvrtko Ursulin 2018-03-02 9:56 ` [PATCH i-g-t v2] " Tvrtko Ursulin 2018-03-02 9:56 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin 2018-03-02 12:29 ` Imre Deak 2018-03-02 12:29 ` [igt-dev] [Intel-gfx] " Imre Deak 2018-02-28 16:33 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_pm: Restore runtime pm state on test exit (rev2) Patchwork 2018-02-28 20:22 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork 2018-03-02 18:21 ` [igt-dev] ✗ Fi.CI.BAT: failure for lib/igt_pm: Restore runtime pm state on test exit (rev3) Patchwork 2018-03-05 9:04 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork 2018-03-05 10:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.