[igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Arkadiusz Hiler]

On some devices BIOS limits possible Package C-states via setting one of
the MSRs. The test now skips if the limit is set to a shallower PC-state
than PC8.

The timeout for PC8+ residency change is lowered to 30s. During testing
the entry always happened in ~10s, so thrice that should be a safe bet.
(active USB keyboard, network and screen, no powertop --auto-tune)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Martin Peres <martin.peres@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 tests/pm_rpm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index d2b837fe..51d8f366 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -50,6 +50,11 @@
 #include "igt_sysfs.h"
 #include "igt_debugfs.h"
 
+#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
+/* HSW/BDW: */
+#define  PKG_CST_LIMIT_MASK		0xF
+#define  PKG_CST_LIMIT_C8		0x6
+
 #define MSR_PC8_RES	0x630
 #define MSR_PC9_RES	0x631
 #define MSR_PC10_RES	0x632
@@ -139,6 +144,14 @@ static bool supports_pc8_plus_residencies(void)
 	if (rc != sizeof(val))
 		return false;
 
+	rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PKG_CST_CONFIG_CONTROL);
+	if (rc != sizeof(val))
+		return false;
+	if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
+		igt_info("PKG C-states limited below PC8 by the BIOS\n");
+		return false;
+	}
+
 	return true;
 }
 
@@ -728,7 +741,7 @@ static void pc8_residency_subtest(void)
 
 	/* Make sure PC8+ residencies move! */
 	disable_all_screens(&ms_data);
-	igt_assert_f(pc8_plus_residency_changed(120),
+	igt_assert_f(pc8_plus_residency_changed(30),
 		     "Machine is not reaching PC8+ states, please check its "
 		     "configuration.\n");
 
@@ -757,7 +770,7 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
 		if (wait_flags & WAIT_STATUS)
 			igt_assert(wait_for_suspended());
 		if (wait_flags & WAIT_PC8_RES)
-			igt_assert(pc8_plus_residency_changed(120));
+			igt_assert(pc8_plus_residency_changed(30));
 		if (wait_flags & WAIT_EXTRA)
 			sleep(5);
 
@@ -1262,7 +1275,7 @@ static void gem_execbuf_stress_subtest(int rounds, int wait_flags)
 		if (wait_flags & WAIT_STATUS)
 			igt_assert(wait_for_suspended());
 		if (wait_flags & WAIT_PC8_RES)
-			igt_assert(pc8_plus_residency_changed(120));
+			igt_assert(pc8_plus_residency_changed(30));
 		if (wait_flags & WAIT_EXTRA)
 			sleep(5);
 	}
-- 
2.14.3

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

[igt-dev] ✓ Fi.CI.BAT: success for pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Patchwork]

== Series Details ==

Series: pm_rpm: Skip PC8 tests when set limit doesn't allow entry
URL   : https://patchwork.freedesktop.org/series/43048/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4166 -> IGTPW_1347 =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1347 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1347, 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/43048/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-c:
      fi-skl-guc:         PASS -> FAIL (fdo#103191, fdo#104724)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       DMESG-WARN (fdo#106248) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248


== Participating hosts (41 -> 37) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4475 -> IGTPW_1347

  CI_DRM_4166: 974787811b2e4047519b0f81930e9d15f0cebd01 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1347: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1347/
  IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Patchwork]

== Series Details ==

Series: pm_rpm: Skip PC8 tests when set limit doesn't allow entry
URL   : https://patchwork.freedesktop.org/series/43048/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4475_full -> IGTPW_1347_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1347_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1347_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/43048/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          SKIP -> PASS +1

    igt@gem_pwrite@big-cpu-random:
      shard-glk:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
      shard-apl:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_plane@plane-panning-top-left-pipe-a-planes:
      shard-apl:          PASS -> FAIL (fdo#103166, fdo#104724)

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

    igt@perf_pmu@busy-idle-check-all-vcs0:
      shard-snb:          PASS -> FAIL (fdo#105106)

    
    ==== Possible fixes ====

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

    igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
      shard-apl:          FAIL -> PASS

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

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@pm_rps@min-max-config-idle:
      shard-hsw:          FAIL -> 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#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105106 https://bugs.freedesktop.org/show_bug.cgi?id=105106
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (4): pig-snb-2600 pig-glk-j5005 pig-skl-6600 pig-hsw-4770r 


== Build changes ==

    * IGT: IGT_4475 -> IGTPW_1347
    * Linux: CI_DRM_4163 -> CI_DRM_4166

  CI_DRM_4163: 8e1dab6e913be7d014eb9bc355ec65b6b56dcd56 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4166: 974787811b2e4047519b0f81930e9d15f0cebd01 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1347: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1347/
  IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Chris Wilson]

Quoting Arkadiusz Hiler (2018-05-11 12:31:14)
> On some devices BIOS limits possible Package C-states via setting one of
> the MSRs. The test now skips if the limit is set to a shallower PC-state
> than PC8.
> 
> The timeout for PC8+ residency change is lowered to 30s. During testing
> the entry always happened in ~10s, so thrice that should be a safe bet.
> (active USB keyboard, network and screen, no powertop --auto-tune)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Martin Peres <martin.peres@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tests/pm_rpm.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index d2b837fe..51d8f366 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -50,6 +50,11 @@
>  #include "igt_sysfs.h"
>  #include "igt_debugfs.h"
>  
> +#define MSR_PKG_CST_CONFIG_CONTROL     0xE2
> +/* HSW/BDW: */
> +#define  PKG_CST_LIMIT_MASK            0xF
> +#define  PKG_CST_LIMIT_C8              0x6
> +
>  #define MSR_PC8_RES    0x630
>  #define MSR_PC9_RES    0x631
>  #define MSR_PC10_RES   0x632
> @@ -139,6 +144,14 @@ static bool supports_pc8_plus_residencies(void)
>         if (rc != sizeof(val))
>                 return false;
>  
> +       rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PKG_CST_CONFIG_CONTROL);
> +       if (rc != sizeof(val))
> +               return false;
> +       if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
> +               igt_info("PKG C-states limited below PC8 by the BIOS\n");
> +               return false;
> +       }

This seems to tally with peeking into intel_idle.c; though it looks
possible for intel_idle to also prevent deep C-states. I guess that is
another bridge to cross later.

> +
>         return true;
>  }
>  
> @@ -728,7 +741,7 @@ static void pc8_residency_subtest(void)
>  
>         /* Make sure PC8+ residencies move! */
>         disable_all_screens(&ms_data);
> -       igt_assert_f(pc8_plus_residency_changed(120),
> +       igt_assert_f(pc8_plus_residency_changed(30),
>                      "Machine is not reaching PC8+ states, please check its "
>                      "configuration.\n");
>  
> @@ -757,7 +770,7 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
>                 if (wait_flags & WAIT_STATUS)
>                         igt_assert(wait_for_suspended());
>                 if (wait_flags & WAIT_PC8_RES)
> -                       igt_assert(pc8_plus_residency_changed(120));
> +                       igt_assert(pc8_plus_residency_changed(30));
>                 if (wait_flags & WAIT_EXTRA)
>                         sleep(5);
>  
> @@ -1262,7 +1275,7 @@ static void gem_execbuf_stress_subtest(int rounds, int wait_flags)
>                 if (wait_flags & WAIT_STATUS)
>                         igt_assert(wait_for_suspended());
>                 if (wait_flags & WAIT_PC8_RES)
> -                       igt_assert(pc8_plus_residency_changed(120));
> +                       igt_assert(pc8_plus_residency_changed(30));

Change the waits in a second patch. If we start to suspect an issue, you
don't want the bisect to end up with both changes.

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

Re: [igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Paulo Zanoni]

Em Sex, 2018-05-11 às 14:31 +0300, Arkadiusz Hiler escreveu:
> On some devices BIOS limits possible Package C-states via setting one
> of
> the MSRs. The test now skips if the limit is set to a shallower PC-
> state
> than PC8.
> 
> The timeout for PC8+ residency change is lowered to 30s. During
> testing
> the entry always happened in ~10s,

That's related to our autosuspend_delay_ms value, and related to the
random chance of having non-graphics activity in the machine. 120s is
definitely way too conservative.

I didn't check the specific MSR register bits, so I can't give a R-B:

Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

(if you split this as Martin suggested, feel free to add the Acked-by
tag to both patches)


>  so thrice that should be a safe bet.
> (active USB keyboard, network and screen, no powertop --auto-tune)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Martin Peres <martin.peres@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tests/pm_rpm.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index d2b837fe..51d8f366 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -50,6 +50,11 @@
>  #include "igt_sysfs.h"
>  #include "igt_debugfs.h"
>  
> +#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
> +/* HSW/BDW: */
> +#define  PKG_CST_LIMIT_MASK		0xF
> +#define  PKG_CST_LIMIT_C8		0x6
> +
>  #define MSR_PC8_RES	0x630
>  #define MSR_PC9_RES	0x631
>  #define MSR_PC10_RES	0x632
> @@ -139,6 +144,14 @@ static bool supports_pc8_plus_residencies(void)
>  	if (rc != sizeof(val))
>  		return false;
>  
> +	rc = pread(msr_fd, &val, sizeof(uint64_t),
> MSR_PKG_CST_CONFIG_CONTROL);
> +	if (rc != sizeof(val))
> +		return false;
> +	if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
> +		igt_info("PKG C-states limited below PC8 by the
> BIOS\n");
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> @@ -728,7 +741,7 @@ static void pc8_residency_subtest(void)
>  
>  	/* Make sure PC8+ residencies move! */
>  	disable_all_screens(&ms_data);
> -	igt_assert_f(pc8_plus_residency_changed(120),
> +	igt_assert_f(pc8_plus_residency_changed(30),
>  		     "Machine is not reaching PC8+ states, please
> check its "
>  		     "configuration.\n");
>  
> @@ -757,7 +770,7 @@ static void modeset_subtest(enum screen_type
> type, int rounds, int wait_flags)
>  		if (wait_flags & WAIT_STATUS)
>  			igt_assert(wait_for_suspended());
>  		if (wait_flags & WAIT_PC8_RES)
> -			igt_assert(pc8_plus_residency_changed(120));
> +			igt_assert(pc8_plus_residency_changed(30));
>  		if (wait_flags & WAIT_EXTRA)
>  			sleep(5);
>  
> @@ -1262,7 +1275,7 @@ static void gem_execbuf_stress_subtest(int
> rounds, int wait_flags)
>  		if (wait_flags & WAIT_STATUS)
>  			igt_assert(wait_for_suspended());
>  		if (wait_flags & WAIT_PC8_RES)
> -			igt_assert(pc8_plus_residency_changed(120));
> +			igt_assert(pc8_plus_residency_changed(30));
>  		if (wait_flags & WAIT_EXTRA)
>  			sleep(5);
>  	}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Arkadiusz Hiler]

On Fri, May 11, 2018 at 02:12:00PM +0100, Chris Wilson wrote:
> Change the waits in a second patch. If we start to suspect an issue, you
> don't want the bisect to end up with both changes.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

Split and pushed with your r-b and Paulo's ack.

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