igt-dev.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit
@ 2018-07-19 16:37 Tvrtko Ursulin
  2018-07-19 16:48 ` Chris Wilson
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2018-07-19 16:37 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Restore runtime PM state (via a newly added library function) when the
test which sets it up exit. This was we avoid running all subsequent sub-
tests in the aggressive runtime PM mode.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_pm.c     | 13 ++++++++++++-
 lib/igt_pm.h     |  1 +
 tests/perf_pmu.c |  3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 8ac132269d79..2e2eea5d3b2d 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -297,7 +297,13 @@ int pm_status_fd = -1;
 static char __igt_pm_runtime_autosuspend[64];
 static char __igt_pm_runtime_control[64];
 
-static void __igt_pm_runtime_exit_handler(int sig)
+/**
+ * igt_restore_runtime_pm:
+ *
+ * Restores the runtime PM configuration as it was before the call to
+ * igt_setup_runtime_pm.
+ */
+void igt_restore_runtime_pm(void)
 {
 	int fd;
 
@@ -326,6 +332,11 @@ static void __igt_pm_runtime_exit_handler(int sig)
 	close(fd);
 }
 
+static void __igt_pm_runtime_exit_handler(int sig)
+{
+	igt_restore_runtime_pm();
+}
+
 /**
  * igt_setup_runtime_pm:
  *
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index eced39f8801a..10cc6794e4e7 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -47,6 +47,7 @@ enum igt_runtime_pm_status {
 };
 
 bool igt_setup_runtime_pm(void);
+void igt_restore_runtime_pm(void);
 enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
 bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index a1d36ac4fa9d..9a20abb6b95c 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1441,6 +1441,9 @@ test_rc6(int gem_fd, unsigned int flags)
 	close(fw);
 	close(fd);
 
+	if (flags & TEST_RUNTIME_PM)
+		igt_restore_runtime_pm();
+
 	assert_within_epsilon(busy - prev, 0.0, tolerance);
 }
 
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
@ 2018-07-19 16:48 ` Chris Wilson
  2018-07-20  9:42   ` [Intel-gfx] [PATCH i-g-t v2] " Tvrtko Ursulin
  2018-07-19 17:02 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-07-19 16:48 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-07-19 17:37:56)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Restore runtime PM state (via a newly added library function) when the
> test which sets it up exit. This was we avoid running all subsequent sub-
> tests in the aggressive runtime PM mode.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/igt_pm.c     | 13 ++++++++++++-
>  lib/igt_pm.h     |  1 +
>  tests/perf_pmu.c |  3 +++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 8ac132269d79..2e2eea5d3b2d 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -297,7 +297,13 @@ int pm_status_fd = -1;
>  static char __igt_pm_runtime_autosuspend[64];
>  static char __igt_pm_runtime_control[64];
>  
> -static void __igt_pm_runtime_exit_handler(int sig)
> +/**
> + * igt_restore_runtime_pm:
> + *
> + * Restores the runtime PM configuration as it was before the call to
> + * igt_setup_runtime_pm.
> + */
> +void igt_restore_runtime_pm(void)
>  {
>         int fd;

Should we eat __igt_pm_runtime_autosuspend so we only apply it once for
each igt_setup_runtime_pm?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
  2018-07-19 16:48 ` Chris Wilson
@ 2018-07-19 17:02 ` Patchwork
  2018-07-19 20:36 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-19 17:02 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: tests/perf_pmu: Restore runtime PM at subtest exit
URL   : https://patchwork.freedesktop.org/series/46883/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4509 -> IGTPW_1608 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46883/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1608 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008


== Participating hosts (46 -> 42) ==

  Additional (1): fi-kbl-7560u 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4567 -> IGTPW_1608

  CI_DRM_4509: e84aa0b47beed78a5a12db93e76fb00eab5db160 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1608: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1608/
  IGT_4567: 7f85adc4050182f490c7a5c48db3d57cdb00af4e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1608/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
  2018-07-19 16:48 ` Chris Wilson
  2018-07-19 17:02 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-07-19 20:36 ` Patchwork
  2018-07-20 10:02 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Restore runtime PM at subtest exit (rev2) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-19 20:36 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: tests/perf_pmu: Restore runtime PM at subtest exit
URL   : https://patchwork.freedesktop.org/series/46883/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4567_full -> IGTPW_1608_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1608_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1608_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46883/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1608_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in IGTPW_1608_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-apl:          PASS -> INCOMPLETE (fdo#106886, fdo#103927)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-glk:          NOTRUN -> FAIL (fdo#106641)
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-snb:          PASS -> FAIL (fdo#103166)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105189) -> PASS

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_vblank@pipe-a-ts-continuation-suspend:
      shard-hsw:          FAIL (fdo#104894) -> PASS

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    igt@testdisplay:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4567 -> IGTPW_1608
    * Linux: CI_DRM_4507 -> CI_DRM_4509

  CI_DRM_4507: 3bbfaebaf3ba21d866c7823d9e4febf47b4b7b39 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4509: e84aa0b47beed78a5a12db93e76fb00eab5db160 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1608: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1608/
  IGT_4567: 7f85adc4050182f490c7a5c48db3d57cdb00af4e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1608/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Intel-gfx] [PATCH i-g-t v2] tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-19 16:48 ` Chris Wilson
@ 2018-07-20  9:42   ` Tvrtko Ursulin
  2018-07-20  9:50     ` [igt-dev] " Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2018-07-20  9:42 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Restore runtime PM state (via a newly added library function) when the
test which sets it up exit. This was we avoid running all subsequent sub-
tests in the aggressive runtime PM mode.

v2:
 * Skip double restore. (Chris Wilson)
 * Close previously leaked fd.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_pm.c     | 20 ++++++++++++++++++--
 lib/igt_pm.h     |  1 +
 tests/perf_pmu.c |  3 +++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 8ac132269d79..512ad9b31951 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -291,16 +291,24 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
 	free(file_name);
 }
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
-/* We just leak this on exit ... */
 int pm_status_fd = -1;
 
 static char __igt_pm_runtime_autosuspend[64];
 static char __igt_pm_runtime_control[64];
 
-static void __igt_pm_runtime_exit_handler(int sig)
+/**
+ * igt_restore_runtime_pm:
+ *
+ * Restores the runtime PM configuration as it was before the call to
+ * igt_setup_runtime_pm.
+ */
+void igt_restore_runtime_pm(void)
 {
 	int fd;
 
+	if (pm_status_fd < 0)
+		return;
+
 	igt_debug("Restoring runtime management to '%s' and '%s'\n",
 		  __igt_pm_runtime_autosuspend,
 		  __igt_pm_runtime_control);
@@ -324,6 +332,14 @@ static void __igt_pm_runtime_exit_handler(int sig)
 		igt_warn("Failed to restore runtime pm control to '%s'\n",
 			 __igt_pm_runtime_control);
 	close(fd);
+
+	close(pm_status_fd);
+	pm_status_fd = -1;
+}
+
+static void __igt_pm_runtime_exit_handler(int sig)
+{
+	igt_restore_runtime_pm();
 }
 
 /**
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index eced39f8801a..10cc6794e4e7 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -47,6 +47,7 @@ enum igt_runtime_pm_status {
 };
 
 bool igt_setup_runtime_pm(void);
+void igt_restore_runtime_pm(void);
 enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
 bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index a1d36ac4fa9d..9a20abb6b95c 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1441,6 +1441,9 @@ test_rc6(int gem_fd, unsigned int flags)
 	close(fw);
 	close(fd);
 
+	if (flags & TEST_RUNTIME_PM)
+		igt_restore_runtime_pm();
+
 	assert_within_epsilon(busy - prev, 0.0, tolerance);
 }
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-20  9:42   ` [Intel-gfx] [PATCH i-g-t v2] " Tvrtko Ursulin
@ 2018-07-20  9:50     ` Chris Wilson
  2018-07-20 12:12       ` [igt-dev] [PATCH i-g-t v3] " Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-07-20  9:50 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-07-20 10:42:55)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Restore runtime PM state (via a newly added library function) when the
> test which sets it up exit. This was we avoid running all subsequent sub-
> tests in the aggressive runtime PM mode.
> 
> v2:
>  * Skip double restore. (Chris Wilson)
>  * Close previously leaked fd.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/igt_pm.c     | 20 ++++++++++++++++++--
>  lib/igt_pm.h     |  1 +
>  tests/perf_pmu.c |  3 +++
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 8ac132269d79..512ad9b31951 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -291,16 +291,24 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
>         free(file_name);
>  }
>  #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
> -/* We just leak this on exit ... */
>  int pm_status_fd = -1;
>  
>  static char __igt_pm_runtime_autosuspend[64];
>  static char __igt_pm_runtime_control[64];
>  
> -static void __igt_pm_runtime_exit_handler(int sig)
> +/**
> + * igt_restore_runtime_pm:
> + *
> + * Restores the runtime PM configuration as it was before the call to
> + * igt_setup_runtime_pm.
> + */
> +void igt_restore_runtime_pm(void)
>  {
>         int fd;
>  
> +       if (pm_status_fd < 0)
> +               return;
> +
>         igt_debug("Restoring runtime management to '%s' and '%s'\n",
>                   __igt_pm_runtime_autosuspend,
>                   __igt_pm_runtime_control);

Hmm. Did we make our igt_debug() signal safe? The open/read/write/close
are ok.  igt_vlog() is definetly not signal-safe. Not the fault of this
patch, but I strongly advise that the debug/warn are removed from this
function and handled in the caller for !exit_handler.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Restore runtime PM at subtest exit (rev2)
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2018-07-19 20:36 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-20 10:02 ` Patchwork
  2018-07-20 13:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2018-07-20 13:39 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/perf_pmu: Restore runtime PM at subtest exit (rev3) Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-20 10:02 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: tests/perf_pmu: Restore runtime PM at subtest exit (rev2)
URL   : https://patchwork.freedesktop.org/series/46883/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4517 -> IGTPW_1612 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46883/revisions/2/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1612 that come from known issues:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_suspend@basic-s4-devices:
      {fi-kbl-8809g}:     INCOMPLETE (fdo#107139, fdo#103665) -> DMESG-WARN (fdo#107222, fdo#107139)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107222 https://bugs.freedesktop.org/show_bug.cgi?id=107222


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4568 -> IGTPW_1612

  CI_DRM_4517: 2a25e5014e7b215f4261b6479ac29dabc2633484 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1612/
  IGT_4568: 86f7b724ef18986bc58d35558d22e1ed3f8df4f9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1612/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t v3] tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-20  9:50     ` [igt-dev] " Chris Wilson
@ 2018-07-20 12:12       ` Tvrtko Ursulin
  2018-07-20 12:21         ` [Intel-gfx] " Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2018-07-20 12:12 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Restore runtime PM state (via a newly added library function) when the
test which sets it up exit. This was we avoid running all subsequent sub-
tests in the aggressive runtime PM mode.

v2:
 * Skip double restore. (Chris Wilson)
 * Close previously leaked fd.

v3:
 * Refactor atexit handlers to be signal safe. (Chris Wilson)
 * Restore audio runtime PM status.
 * Add a different PCI path to audio device.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_pm.c     | 152 ++++++++++++++++++++++++++++++++++++-----------
 lib/igt_pm.h     |   1 +
 tests/perf_pmu.c |   3 +
 3 files changed, 122 insertions(+), 34 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 8ac132269d79..1fe8f5f46399 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -63,36 +63,73 @@ enum {
 /* Remember to fix this if adding longer strings */
 #define MAX_POLICY_STRLEN	strlen(MAX_PERFORMANCE_STR)
 
+static const char *const __igt_pm_audio_runtime_control_paths[] = {
+	"/sys/bus/pci/devices/0000:00:03.0/power/control",
+	"/sys/bus/pci/devices/0000:00:1f.3/power/control",
+};
+
 static char __igt_pm_audio_runtime_power_save[64];
+static const char * __igt_pm_audio_runtime_control_path;
 static char __igt_pm_audio_runtime_control[64];
 
-static void __igt_pm_audio_runtime_exit_handler(int sig)
+static int __igt_pm_audio_restore_runtime_pm(void)
 {
 	int fd;
 
-	igt_debug("Restoring audio power management to '%s' and '%s'\n",
-		  __igt_pm_audio_runtime_power_save,
-		  __igt_pm_audio_runtime_control);
+	if (!__igt_pm_audio_runtime_power_save[0])
+		return 0;
 
 	fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY);
 	if (fd < 0)
-		return;
+		return errno;
+
 	if (write(fd, __igt_pm_audio_runtime_power_save,
 		  strlen(__igt_pm_audio_runtime_power_save)) !=
-	    strlen(__igt_pm_audio_runtime_power_save))
-		igt_warn("Failed to restore audio power_save to '%s'\n",
-			 __igt_pm_audio_runtime_power_save);
+	    strlen(__igt_pm_audio_runtime_power_save)) {
+		close(fd);
+		return errno;
+	}
+
 	close(fd);
 
-	fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY);
+	fd = open(__igt_pm_audio_runtime_control_path, O_WRONLY);
 	if (fd < 0)
-		return;
+		return errno;
+
 	if (write(fd, __igt_pm_audio_runtime_control,
 		  strlen(__igt_pm_audio_runtime_control)) !=
-	    strlen(__igt_pm_audio_runtime_control))
-		igt_warn("Failed to restore audio control to '%s'\n",
-			 __igt_pm_audio_runtime_control);
+	    strlen(__igt_pm_audio_runtime_control)) {
+		close(fd);
+		return errno;
+	}
+
 	close(fd);
+
+	__igt_pm_audio_runtime_power_save[0] = 0;
+
+	return 0;
+}
+
+static void igt_pm_audio_restore_runtime_pm(void)
+{
+	int ret;
+
+	if (!__igt_pm_audio_runtime_power_save[0])
+		return;
+
+	igt_debug("Restoring audio power management to '%s' and '%s'\n",
+		  __igt_pm_audio_runtime_power_save,
+		  __igt_pm_audio_runtime_control);
+
+	ret = __igt_pm_audio_restore_runtime_pm();
+	if (ret)
+		igt_warn("Failed to restore runtime audio PM! (errno=%d)\n",
+			 ret);
+}
+
+static void __igt_pm_audio_runtime_exit_handler(int sig)
+{
+	__igt_pm_audio_restore_runtime_pm();
 }
 
 static void strchomp(char *str)
@@ -116,7 +153,7 @@ static void strchomp(char *str)
  */
 void igt_pm_enable_audio_runtime_pm(void)
 {
-	int fd;
+	int fd, i;
 
 	/* Check if already enabled. */
 	if (__igt_pm_audio_runtime_power_save[0])
@@ -131,14 +168,23 @@ void igt_pm_enable_audio_runtime_pm(void)
 		igt_assert_eq(write(fd, "1\n", 2), 2);
 		close(fd);
 	}
-	fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_RDWR);
-	if (fd >= 0) {
-		igt_assert(read(fd, __igt_pm_audio_runtime_control,
-				sizeof(__igt_pm_audio_runtime_control)) > 0);
-		strchomp(__igt_pm_audio_runtime_control);
-		igt_assert_eq(write(fd, "auto\n", 5), 5);
-		close(fd);
+
+	for (i = 0; i < ARRAY_SIZE(__igt_pm_audio_runtime_control_paths); i++) {
+		const char *path = __igt_pm_audio_runtime_control_paths[i];
+
+		fd = open(path, O_RDWR);
+		if (fd >= 0) {
+			__igt_pm_audio_runtime_control_path = path;
+			igt_assert(read(fd, __igt_pm_audio_runtime_control,
+					sizeof(__igt_pm_audio_runtime_control)) > 0);
+			strchomp(__igt_pm_audio_runtime_control);
+			igt_assert_eq(write(fd, "auto\n", 5), 5);
+			close(fd);
+			break;
+		}
 	}
+	if (!__igt_pm_audio_runtime_control_path)
+		igt_warn("Failed to find audio PCI device!\n");
 
 	igt_debug("Saved audio power management as '%s' and '%s'\n",
 		  __igt_pm_audio_runtime_power_save,
@@ -291,39 +337,77 @@ void igt_pm_restore_sata_link_power_management(int8_t *pm_data)
 	free(file_name);
 }
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
-/* We just leak this on exit ... */
 int pm_status_fd = -1;
 
 static char __igt_pm_runtime_autosuspend[64];
 static char __igt_pm_runtime_control[64];
 
-static void __igt_pm_runtime_exit_handler(int sig)
+static int __igt_restore_runtime_pm(void)
 {
 	int fd;
 
-	igt_debug("Restoring runtime management to '%s' and '%s'\n",
-		  __igt_pm_runtime_autosuspend,
-		  __igt_pm_runtime_control);
+	if (pm_status_fd < 0)
+		return 0;
 
 	fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
 	if (fd < 0)
-		return;
+		return errno;
+
 	if (write(fd, __igt_pm_runtime_autosuspend,
 		  strlen(__igt_pm_runtime_autosuspend)) !=
-	    strlen(__igt_pm_runtime_autosuspend))
-		igt_warn("Failed to restore runtime pm autosuspend delay to '%s'\n",
-			 __igt_pm_runtime_autosuspend);
+	    strlen(__igt_pm_runtime_autosuspend)) {
+		close(fd);
+		return errno;
+	}
+
 	close(fd);
 
 	fd = open(POWER_DIR "/control", O_WRONLY);
 	if (fd < 0)
-		return;
+		return errno;
+
 	if (write(fd, __igt_pm_runtime_control,
 		  strlen(__igt_pm_runtime_control)) !=
-	    strlen(__igt_pm_runtime_control))
-		igt_warn("Failed to restore runtime pm control to '%s'\n",
-			 __igt_pm_runtime_control);
+	    strlen(__igt_pm_runtime_control)) {
+		close(fd);
+		return errno;
+	}
+
 	close(fd);
+
+	close(pm_status_fd);
+	pm_status_fd = -1;
+
+	return 0;
+}
+
+/**
+ * igt_restore_runtime_pm:
+ *
+ * Restores the runtime PM configuration as it was before the call to
+ * igt_setup_runtime_pm.
+ */
+void igt_restore_runtime_pm(void)
+{
+	int ret;
+
+	if (pm_status_fd < 0)
+		return;
+
+	igt_debug("Restoring runtime PM management to '%s' and '%s'\n",
+		  __igt_pm_runtime_autosuspend,
+		  __igt_pm_runtime_control);
+
+	ret = __igt_restore_runtime_pm();
+	if (ret)
+		igt_warn("Failed to restore runtime PM! (errno=%d)\n", ret);
+
+	igt_pm_audio_restore_runtime_pm();
+}
+
+static void __igt_pm_runtime_exit_handler(int sig)
+{
+	__igt_restore_runtime_pm();
 }
 
 /**
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index eced39f8801a..10cc6794e4e7 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -47,6 +47,7 @@ enum igt_runtime_pm_status {
 };
 
 bool igt_setup_runtime_pm(void);
+void igt_restore_runtime_pm(void);
 enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
 bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index a1d36ac4fa9d..9a20abb6b95c 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1441,6 +1441,9 @@ test_rc6(int gem_fd, unsigned int flags)
 	close(fw);
 	close(fd);
 
+	if (flags & TEST_RUNTIME_PM)
+		igt_restore_runtime_pm();
+
 	assert_within_epsilon(busy - prev, 0.0, tolerance);
 }
 
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v3] tests/perf_pmu: Restore runtime PM at subtest exit
  2018-07-20 12:12       ` [igt-dev] [PATCH i-g-t v3] " Tvrtko Ursulin
@ 2018-07-20 12:21         ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2018-07-20 12:21 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: intel-gfx

Quoting Tvrtko Ursulin (2018-07-20 13:12:22)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Restore runtime PM state (via a newly added library function) when the
> test which sets it up exit. This was we avoid running all subsequent sub-
> tests in the aggressive runtime PM mode.
> 
> v2:
>  * Skip double restore. (Chris Wilson)
>  * Close previously leaked fd.
> 
> v3:
>  * Refactor atexit handlers to be signal safe. (Chris Wilson)
>  * Restore audio runtime PM status.
>  * Add a different PCI path to audio device.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

(Just ship it already! But I do wonder if there's a better way to find
the control device (search by PCI vendor, class?) and if we shouldn't
make the control/value be uniform in their handling, then just iterate
over the area of (path, value) for restoring.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for tests/perf_pmu: Restore runtime PM at subtest exit (rev2)
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2018-07-20 10:02 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Restore runtime PM at subtest exit (rev2) Patchwork
@ 2018-07-20 13:33 ` Patchwork
  2018-07-20 13:39 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/perf_pmu: Restore runtime PM at subtest exit (rev3) Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-20 13:33 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: tests/perf_pmu: Restore runtime PM at subtest exit (rev2)
URL   : https://patchwork.freedesktop.org/series/46883/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4568_full -> IGTPW_1612_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1612_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1612_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46883/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1612_full:

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_requests:
      shard-kbl:          PASS -> SKIP +13

    igt@gem_mocs_settings@mocs-rc6-ctx-render:
      shard-kbl:          SKIP -> PASS

    igt@gem_ppgtt@flink-and-close-vma-leak:
      shard-hsw:          SKIP -> PASS +2

    
== Known issues ==

  Here are the changes found in IGTPW_1612_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_workarounds:
      shard-kbl:          PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925)

    igt@prime_busy@hang-bsd2:
      shard-snb:          SKIP -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@gem_exec_blt@normal-max:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS

    igt@testdisplay:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4568 -> IGTPW_1612
    * Linux: CI_DRM_4509 -> CI_DRM_4517

  CI_DRM_4509: e84aa0b47beed78a5a12db93e76fb00eab5db160 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4517: 2a25e5014e7b215f4261b6479ac29dabc2633484 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1612: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1612/
  IGT_4568: 86f7b724ef18986bc58d35558d22e1ed3f8df4f9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1612/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/perf_pmu: Restore runtime PM at subtest exit (rev3)
  2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2018-07-20 13:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-20 13:39 ` Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-20 13:39 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: tests/perf_pmu: Restore runtime PM at subtest exit (rev3)
URL   : https://patchwork.freedesktop.org/series/46883/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4517 -> IGTPW_1614 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1614 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1614, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46883/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1614:

  === IGT changes ===

    ==== Possible regressions ====

    igt@pm_rpm@basic-pci-d3-state:
      fi-bxt-j4205:       PASS -> WARN +1

    igt@pm_rpm@basic-rte:
      fi-glk-j4005:       PASS -> WARN +1
      fi-glk-dsi:         PASS -> WARN +1

    


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4568 -> IGTPW_1614

  CI_DRM_4517: 2a25e5014e7b215f4261b6479ac29dabc2633484 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1614: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1614/
  IGT_4568: 86f7b724ef18986bc58d35558d22e1ed3f8df4f9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1614/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-07-20 13:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-19 16:37 [igt-dev] [PATCH i-g-t] tests/perf_pmu: Restore runtime PM at subtest exit Tvrtko Ursulin
2018-07-19 16:48 ` Chris Wilson
2018-07-20  9:42   ` [Intel-gfx] [PATCH i-g-t v2] " Tvrtko Ursulin
2018-07-20  9:50     ` [igt-dev] " Chris Wilson
2018-07-20 12:12       ` [igt-dev] [PATCH i-g-t v3] " Tvrtko Ursulin
2018-07-20 12:21         ` [Intel-gfx] " Chris Wilson
2018-07-19 17:02 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2018-07-19 20:36 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2018-07-20 10:02 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Restore runtime PM at subtest exit (rev2) Patchwork
2018-07-20 13:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2018-07-20 13:39 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/perf_pmu: Restore runtime PM at subtest exit (rev3) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).