[igt-dev] [PATCH i-g-t 0/6] PC state igt test

[igt-dev] [PATCH i-g-t 0/6] PC state igt test

[Anshuman Gupta]

Pakage C state, PC8 and PC10 igt test.

Anshuman Gupta (6):
  i915/i915_pm_rpm: Enable PC8+ residency test
  i915/i915_pm_rpm.c: create PC state subtest group
  i915/i915_pm_rpm: enable modeset-pc8-residency-stress test
  i915/i915_pm_rpm: gem-execbuf-stress-pc8 use powetop
  i915/i915_pm_rpm: Add PC10 display off test
  i915/i915_pm_rpm: Add PC10 idle display on test

 tests/i915/i915_pm_rpm.c | 135 +++++++++++++++++++++++++++++++++------
 1 file changed, 114 insertions(+), 21 deletions(-)

-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 1/6] i915/i915_pm_rpm: Enable PC8+ residency test

[Anshuman Gupta]

Enabled pc8-residency test for each platform that supports pc8.
Bifurcate pc8-residency is display-on and display-off test case.

Negative testing of PC8 residency on HASWELL/BROADWELL while
display being "on" is removed as it saves CI time and there is
no ROI of such testing.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 48 ++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 6d46c320..af55b569 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -52,6 +52,16 @@
 #include "igt_debugfs.h"
 #include "igt_device.h"
 #include "igt_edid.h"
+#include "igt_psr.h"
+
+#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
+/*
+ * Below PKG CST limit mask and PC8 bits are meant for
+ * HSW,BDW SKL,ICL and Goldmont Microarch.
+ * Refer IA S/W developers manual vol3c part3 chapter:35
+ */
+#define  PKG_CST_LIMIT_MASK		0xF
+#define  PKG_CST_LIMIT_C8		0x6
 
 #define MSR_PC8_RES	0x630
 #define MSR_PC9_RES	0x631
@@ -301,7 +311,6 @@ static void init_modeset_cached_params(struct mode_set_data *data)
 					    SCREEN_TYPE_LPSP);
 	non_lpsp = init_modeset_params_for_type(data, &non_lpsp_mode_params,
 						SCREEN_TYPE_NON_LPSP);
-
 	if (lpsp)
 		default_mode_params = &lpsp_mode_params;
 	else if (non_lpsp)
@@ -728,10 +737,6 @@ static void setup_pc8(void)
 {
 	has_pc8 = false;
 
-	/* Only Haswell supports the PC8 feature. */
-	if (!IS_HASWELL(ms_data.devid) && !IS_BROADWELL(ms_data.devid))
-		return;
-
 	/* Make sure our Kernel supports MSR and the module is loaded. */
 	igt_require(modprobe("msr") == 0);
 
@@ -827,20 +832,27 @@ static void basic_subtest(void)
 	/* XXX Also we can test wake up via exec nop */
 }
 
-static void pc8_residency_subtest(void)
+static void pc8_residency_subtest(bool display_on)
 {
 	igt_require(has_pc8);
 
-	/* Make sure PC8+ residencies move! */
-	disable_all_screens(&ms_data);
-	igt_assert_f(pc8_plus_residency_changed(30),
-		     "Machine is not reaching PC8+ states, please check its "
-		     "configuration.\n");
+	if (IS_HASWELL(ms_data.devid) || IS_BROADWELL(ms_data.devid))
+		igt_require_f(!display_on, "pc8 with display on not supported\n");
 
-	/* Make sure PC8+ residencies stop! */
-	enable_one_screen(&ms_data);
-	igt_assert_f(!pc8_plus_residency_changed(10),
-		     "PC8+ residency didn't stop with screen enabled.\n");
+	if (!display_on) {
+		/* Make sure PC8+ residencies move! */
+		disable_all_screens(&ms_data);
+		igt_assert_f(pc8_plus_residency_changed(30),
+			     "Machine is not reaching PC8+ states with all screens disabled.\n");
+	} else {
+		/* check pc8 with psr disabled on any output */
+		psr_disable(drm_fd, debugfs);
+		enable_one_screen(&ms_data);
+		igt_assert_f(pc8_plus_residency_changed(30),
+			     "Machine is not reaching PC8+ states with a screen enabled.\n");
+		/* Restore PSR with PSR_MODE_1 for rest of IGT */
+		psr_enable(drm_fd, debugfs, PSR_MODE_1);
+	}
 }
 
 static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
@@ -2075,8 +2087,10 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		reg_read_ioctl_subtest();
 	igt_subtest("i2c")
 		i2c_subtest();
-	igt_subtest("pc8-residency")
-		pc8_residency_subtest();
+	igt_subtest("pc8-residency-display-on")
+		pc8_residency_subtest(true);
+	igt_subtest("pc8-residency-display-off")
+		pc8_residency_subtest(false);
 	igt_subtest("debugfs-read")
 		debugfs_read_subtest();
 	igt_subtest("debugfs-forcewake-user")
-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Anshuman Gupta]

Create a separate igt test group and move package C
state in to this subgroup.
Run powertop --auto-tune to tune SOC power configuration
for package C state tests.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index af55b569..42bc44d9 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -832,6 +832,25 @@ static void basic_subtest(void)
 	/* XXX Also we can test wake up via exec nop */
 }
 
+static bool setup_powertop(void)
+{
+	FILE *fp;
+	char tmp[512];
+
+	fp = popen("powertop --auto-tune", "r");
+	if (fp == NULL) {
+		igt_info("Failed to run powertop\n");
+		perror("popen");
+		return false;
+	}
+
+	while (fgets(tmp, sizeof(tmp), fp) != NULL)
+		igt_info("%s\n", tmp);
+
+	pclose(fp);
+	return true;
+}
+
 static void pc8_residency_subtest(bool display_on)
 {
 	igt_require(has_pc8);
@@ -2082,15 +2101,23 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	igt_subtest_with_dynamic("universal-planes-dpms")
 		planes_subtest(true, true);
 
+	igt_describe("package C state residency test");
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(setup_powertop());
+		}
+
+		igt_subtest("pc8-residency-display-on")
+			pc8_residency_subtest(true);
+		igt_subtest("pc8-residency-display-off")
+			pc8_residency_subtest(false);
+	}
+
 	/* Misc */
 	igt_subtest("reg-read-ioctl")
 		reg_read_ioctl_subtest();
 	igt_subtest("i2c")
 		i2c_subtest();
-	igt_subtest("pc8-residency-display-on")
-		pc8_residency_subtest(true);
-	igt_subtest("pc8-residency-display-off")
-		pc8_residency_subtest(false);
 	igt_subtest("debugfs-read")
 		debugfs_read_subtest();
 	igt_subtest("debugfs-forcewake-user")
-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 3/6] i915/i915_pm_rpm: enable modeset-pc8-residency-stress test

[Anshuman Gupta]

Enable modeset-pc8-residency-stress to check PC8 residency
after modeset.
Negative testing of PC8 residency on HASWELL/BROADWELL while
display being "on" is removed as it saves CI time and there is
no ROI of such testing.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 42bc44d9..34ec9a6d 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -878,8 +878,10 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
 {
 	int i;
 
-	if (wait_flags & WAIT_PC8_RES)
+	if (wait_flags & WAIT_PC8_RES) {
 		igt_require(has_pc8);
+		igt_require(setup_powertop());
+	}
 
 	if (wait_flags & WAIT_EXTRA)
 		rounds /= 2;
@@ -902,8 +904,6 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
 		igt_require(enable_one_screen_with_type(&ms_data, type));
 		if (wait_flags & WAIT_STATUS)
 			igt_assert(wait_for_active());
-		if (wait_flags & WAIT_PC8_RES)
-			igt_assert(!pc8_plus_residency_changed(5));
 		if (wait_flags & WAIT_EXTRA)
 			sleep(5);
 	}
-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 4/6] i915/i915_pm_rpm: gem-execbuf-stress-pc8 use powetop

[Anshuman Gupta]

We require to auto-tune powertop before checking Package
C state. Invoke "powertop --auto-tune" before checking
PC8 residencies.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 34ec9a6d..d284fa54 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -1391,8 +1391,10 @@ static void gem_execbuf_stress_subtest(int rounds, int wait_flags)
 
 	igt_require_gem(drm_fd);
 
-	if (wait_flags & WAIT_PC8_RES)
+	if (wait_flags & WAIT_PC8_RES) {
 		igt_require(has_pc8);
+		igt_require(setup_powertop());
+	}
 
 	i = 0;
 	batch_buf[i++] = MI_NOOP;
-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 5/6] i915/i915_pm_rpm: Add PC10 display off test

[Anshuman Gupta]

Add a test to validate PC10 residencies while all output's
display is off.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index d284fa54..d771d733 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -179,6 +179,16 @@ static bool pc8_plus_residency_changed(unsigned int timeout_sec)
 			timeout_sec * 1000, 100);
 }
 
+static bool pc10_residency_changed(unsigned int timeout_sec)
+{
+	uint64_t res_pc10;
+
+	res_pc10 = get_residency(MSR_PC10_RES);
+
+	return igt_wait(res_pc10 != get_residency(MSR_PC10_RES),
+			timeout_sec * 1000, 100);
+}
+
 static enum pc8_status get_pc8_status(void)
 {
 	ssize_t n_read;
@@ -874,6 +884,19 @@ static void pc8_residency_subtest(bool display_on)
 	}
 }
 
+static void pc10_residency_subtest(bool display_on)
+{
+	igt_require(has_pc8);
+	igt_require(AT_LEAST_GEN(ms_data.devid, 9));
+
+	if (!display_on) {
+		/* Make sure PC10 residencies move! */
+		disable_all_screens_and_wait(&ms_data);
+		igt_assert_f(pc10_residency_changed(30),
+			     "Machine is not reaching PC10 state with all screens disabled.\n");
+	}
+}
+
 static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
 {
 	int i;
@@ -2113,6 +2136,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 			pc8_residency_subtest(true);
 		igt_subtest("pc8-residency-display-off")
 			pc8_residency_subtest(false);
+		igt_subtest("pc10-residency-display-off")
+			pc10_residency_subtest(false);
 	}
 
 	/* Misc */
-- 
2.26.2

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

[igt-dev] [PATCH i-g-t 6/6] i915/i915_pm_rpm: Add PC10 idle display on test

[Anshuman Gupta]

Add a test which validates PC10 residencies with
psr idle display on.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/i915/i915_pm_rpm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index d771d733..d2ea8d1b 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -79,6 +79,7 @@ enum pc8_status {
 enum screen_type {
 	SCREEN_TYPE_LPSP,
 	SCREEN_TYPE_NON_LPSP,
+	SCREEN_TYPE_PSR,
 	SCREEN_TYPE_ANY,
 };
 
@@ -128,6 +129,7 @@ struct modeset_params {
 
 struct modeset_params lpsp_mode_params;
 struct modeset_params non_lpsp_mode_params;
+struct modeset_params psr_mode_params;
 struct modeset_params *default_mode_params;
 
 static int modprobe(const char *driver)
@@ -293,6 +295,10 @@ static bool init_modeset_params_for_type(struct mode_set_data *data,
 		    i915_output_is_lpsp_capable(drm_fd, output))
 			continue;
 
+		if (type == SCREEN_TYPE_PSR &&
+		    !psr_sink_support(drm_fd, debugfs, PSR_MODE_1))
+			continue;
+
 		connector = c;
 		mode = igt_output_get_mode(output);
 		break;
@@ -327,6 +333,8 @@ static void init_modeset_cached_params(struct mode_set_data *data)
 		default_mode_params = &non_lpsp_mode_params;
 	else
 		default_mode_params = NULL;
+
+	init_modeset_params_for_type(data, &psr_mode_params, SCREEN_TYPE_PSR);
 }
 
 static bool set_mode_for_params(struct modeset_params *params)
@@ -358,6 +366,9 @@ static bool enable_one_screen_with_type(struct mode_set_data *data,
 	case SCREEN_TYPE_NON_LPSP:
 		params = &non_lpsp_mode_params;
 		break;
+	case SCREEN_TYPE_PSR:
+		params = &psr_mode_params;
+		break;
 	default:
 		igt_assert(0);
 	}
@@ -368,6 +379,12 @@ static bool enable_one_screen_with_type(struct mode_set_data *data,
 	return set_mode_for_params(params);
 }
 
+static void enable_psr_screen(struct mode_set_data *data)
+{
+	psr_enable(drm_fd, debugfs, PSR_MODE_1);
+	igt_require(enable_one_screen_with_type(data, SCREEN_TYPE_PSR));
+}
+
 static void enable_one_screen(struct mode_set_data *data)
 {
 	/* SKIP if there are no connected screens. */
@@ -894,6 +911,12 @@ static void pc10_residency_subtest(bool display_on)
 		disable_all_screens_and_wait(&ms_data);
 		igt_assert_f(pc10_residency_changed(30),
 			     "Machine is not reaching PC10 state with all screens disabled.\n");
+	} else {
+		/* check pc10 with psr screen */
+		enable_psr_screen(&ms_data);
+		igt_assert(psr_wait_entry(debugfs, PSR_MODE_1));
+		igt_assert_f(pc10_residency_changed(30),
+			     "Machine is not reaching PC10 state with idle display on\n");
 	}
 }
 
@@ -2136,6 +2159,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 			pc8_residency_subtest(true);
 		igt_subtest("pc8-residency-display-off")
 			pc8_residency_subtest(false);
+		igt_subtest("pc10-residency-display-on")
+			pc10_residency_subtest(true);
 		igt_subtest("pc10-residency-display-off")
 			pc10_residency_subtest(false);
 	}
-- 
2.26.2

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

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Anshuman Gupta]

On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > Create a separate igt test group and move package C
> > state in to this subgroup.
> > Run powertop --auto-tune to tune SOC power configuration
> > for package C state tests.
> > 
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
> >  1 file changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > index af55b569..42bc44d9 100644
> > --- a/tests/i915/i915_pm_rpm.c
> > +++ b/tests/i915/i915_pm_rpm.c
> > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> >         /* XXX Also we can test wake up via exec nop */
> >  }
> >  
> > +static bool setup_powertop(void)
> > +{
> > +       FILE *fp;
> > +       char tmp[512];
> > +
> > +       fp = popen("powertop --auto-tune", "r");
> 
> Doesn't this defeat the point of having it work out of the box?
> -Chris
Yes, but there is no other way as PC state are being dependent on other
non-gfx devices as well. These test kept disabled since gen9 platforms.
Thanks,
Anshuman Gupta.

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

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Chris Wilson]

Quoting Anshuman Gupta (2020-12-09 16:06:38)
> Create a separate igt test group and move package C
> state in to this subgroup.
> Run powertop --auto-tune to tune SOC power configuration
> for package C state tests.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index af55b569..42bc44d9 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -832,6 +832,25 @@ static void basic_subtest(void)
>         /* XXX Also we can test wake up via exec nop */
>  }
>  
> +static bool setup_powertop(void)
> +{
> +       FILE *fp;
> +       char tmp[512];
> +
> +       fp = popen("powertop --auto-tune", "r");

Doesn't this defeat the point of having it work out of the box?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Chris Wilson]

Quoting Anshuman Gupta (2020-12-09 16:25:02)
> On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > Create a separate igt test group and move package C
> > > state in to this subgroup.
> > > Run powertop --auto-tune to tune SOC power configuration
> > > for package C state tests.
> > > 
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
> > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > index af55b569..42bc44d9 100644
> > > --- a/tests/i915/i915_pm_rpm.c
> > > +++ b/tests/i915/i915_pm_rpm.c
> > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > >         /* XXX Also we can test wake up via exec nop */
> > >  }
> > >  
> > > +static bool setup_powertop(void)
> > > +{
> > > +       FILE *fp;
> > > +       char tmp[512];
> > > +
> > > +       fp = popen("powertop --auto-tune", "r");
> > 
> > Doesn't this defeat the point of having it work out of the box?
> > -Chris
> Yes, but there is no other way as PC state are being dependent on other
> non-gfx devices as well. These test kept disabled since gen9 platforms.

Could you be explicit in the steps required so that it is clear that
powertop is not working around our own bugs?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for PC state igt test

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 11683 bytes --]

== Series Details ==

Series: PC state igt test
URL   : https://patchwork.freedesktop.org/series/84737/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9464 -> IGTPW_5264
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/index.html

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-cfl-8109u:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-cfl-8109u/igt@i915_pm_rpm@basic-rte.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-cfl-8109u/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6600u:       [PASS][3] -> [WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
    - fi-cfl-8700k:       [PASS][5] -> [WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
    - fi-bsw-kefka:       [PASS][7] -> [WARN][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bsw-kefka/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bsw-kefka/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-x1275:       [PASS][9] -> [WARN][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
    - fi-tgl-u2:          [PASS][11] -> [WARN][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-tgl-u2/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-tgl-u2/igt@i915_pm_rpm@module-reload.html
    - fi-skl-guc:         [PASS][13] -> [WARN][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
    - fi-bsw-n3050:       [PASS][15] -> [WARN][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
    - fi-skl-6700k2:      [PASS][17] -> [WARN][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
    - fi-cfl-guc:         [PASS][19] -> [WARN][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
    - fi-icl-y:           [PASS][21] -> [WARN][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-icl-y/igt@i915_pm_rpm@module-reload.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-icl-y/igt@i915_pm_rpm@module-reload.html
    - fi-cml-s:           [PASS][23] -> [WARN][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-cml-s/igt@i915_pm_rpm@module-reload.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-cml-s/igt@i915_pm_rpm@module-reload.html
    - fi-bxt-dsi:         [PASS][25] -> [WARN][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bxt-dsi/igt@i915_pm_rpm@module-reload.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bxt-dsi/igt@i915_pm_rpm@module-reload.html
    - fi-cml-u2:          [PASS][27] -> [WARN][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-cml-u2/igt@i915_pm_rpm@module-reload.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-cml-u2/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-7500u:       [PASS][29] -> [WARN][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-7500u/igt@i915_pm_rpm@module-reload.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-7500u/igt@i915_pm_rpm@module-reload.html
    - fi-hsw-4770:        [PASS][31] -> [WARN][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-hsw-4770/igt@i915_pm_rpm@module-reload.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-hsw-4770/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-soraka:      [PASS][33] -> [WARN][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-soraka/igt@i915_pm_rpm@module-reload.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-soraka/igt@i915_pm_rpm@module-reload.html
    - fi-bdw-5557u:       [PASS][35] -> [WARN][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bdw-5557u/igt@i915_pm_rpm@module-reload.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bdw-5557u/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-r:           [PASS][37] -> [WARN][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-r/igt@i915_pm_rpm@module-reload.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-r/igt@i915_pm_rpm@module-reload.html
    - fi-apl-guc:         [PASS][39] -> [WARN][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
    - fi-bsw-nick:        [PASS][41] -> [WARN][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bsw-nick/igt@i915_pm_rpm@module-reload.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bsw-nick/igt@i915_pm_rpm@module-reload.html
    - fi-glk-dsi:         [PASS][43] -> [WARN][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html
    - fi-icl-u2:          [PASS][45] -> [WARN][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-icl-u2/igt@i915_pm_rpm@module-reload.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-icl-u2/igt@i915_pm_rpm@module-reload.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-byt-j1900:       [INCOMPLETE][47] ([i915#142] / [i915#2405]) -> [WARN][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-guc:         [FAIL][49] ([i915#579]) -> [WARN][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_pm_rpm@module-reload:
    - {fi-ehl-1}:         [PASS][51] -> [WARN][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-ehl-1/igt@i915_pm_rpm@module-reload.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-ehl-1/igt@i915_pm_rpm@module-reload.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-byt-j1900:       NOTRUN -> [SKIP][53] ([fdo#109271]) +17 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-byt-j1900/igt@amdgpu/amd_basic@userptr.html

  * igt@i915_pm_rpm@module-reload:
    - fi-tgl-y:           [PASS][54] -> [WARN][55] ([i915#2411])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-tgl-y/igt@i915_pm_rpm@module-reload.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-tgl-y/igt@i915_pm_rpm@module-reload.html

  * igt@vgem_basic@setversion:
    - fi-tgl-y:           [PASS][56] -> [DMESG-WARN][57] ([i915#402]) +2 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-tgl-y/igt@vgem_basic@setversion.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-tgl-y/igt@vgem_basic@setversion.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - fi-bsw-n3050:       [DMESG-FAIL][58] ([i915#2675]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@sanitycheck:
    - fi-kbl-7500u:       [DMESG-WARN][60] ([i915#2605]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-kbl-7500u/igt@i915_selftest@live@sanitycheck.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-kbl-7500u/igt@i915_selftest@live@sanitycheck.html

  * igt@vgem_basic@create:
    - fi-tgl-y:           [DMESG-WARN][62] ([i915#402]) -> [PASS][63] +2 similar issues
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9464/fi-tgl-y/igt@vgem_basic@create.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/fi-tgl-y/igt@vgem_basic@create.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#142]: https://gitlab.freedesktop.org/drm/intel/issues/142
  [i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2605]: https://gitlab.freedesktop.org/drm/intel/issues/2605
  [i915#2675]: https://gitlab.freedesktop.org/drm/intel/issues/2675
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579


Participating hosts (43 -> 39)
------------------------------

  Missing    (4): fi-ilk-m540 fi-dg1-1 fi-bdw-samus fi-hsw-4200u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5885 -> IGTPW_5264

  CI-20190529: 20190529
  CI_DRM_9464: a2561d7ce07920c1fc05013c87d21d3c8b05149f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5264: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/index.html
  IGT_5885: d99f644b1868b9c92435b05ebfafa230721cd677 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@i915_pm_rpm@pc8-residency-display-off
+igt@i915_pm_rpm@pc8-residency-display-on
+igt@i915_pm_rpm@pc10-residency-display-off
+igt@i915_pm_rpm@pc10-residency-display-on
-igt@i915_pm_rpm@pc8-residency

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5264/index.html

[-- Attachment #1.2: Type: text/html, Size: 12973 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Anshuman Gupta]

On 2020-12-09 at 16:47:01 +0000, Chris Wilson wrote:
> Quoting Anshuman Gupta (2020-12-09 16:25:02)
> > On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > > Create a separate igt test group and move package C
> > > > state in to this subgroup.
> > > > Run powertop --auto-tune to tune SOC power configuration
> > > > for package C state tests.
> > > > 
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > >  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
> > > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > > index af55b569..42bc44d9 100644
> > > > --- a/tests/i915/i915_pm_rpm.c
> > > > +++ b/tests/i915/i915_pm_rpm.c
> > > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > > >         /* XXX Also we can test wake up via exec nop */
> > > >  }
> > > >  
> > > > +static bool setup_powertop(void)
> > > > +{
> > > > +       FILE *fp;
> > > > +       char tmp[512];
> > > > +
> > > > +       fp = popen("powertop --auto-tune", "r");
> > > 
> > > Doesn't this defeat the point of having it work out of the box?
May be misunderstood your comment, is it PC state or powertop should work
out of box ?
> > > -Chris
> > Yes, but there is no other way as PC state are being dependent on other
> > non-gfx devices as well. These test kept disabled since gen9 platforms.
> 
> Could you be explicit in the steps required so that it is clear that
> powertop is not working around our own bugs?
We need to run powertop --auto-tune to tune SOC power configuration in order
to attain deeper package C state.
Without running "powertop --auto-tune" we may not hit to PC8/PC10 state
(other non-gfx devices can also block the PC8/PC10 state).
We don't want to fail these test due to other non-gfx devices, so it is
better to tune SOC power configuration by power-top.

Thanks,
Anshuman Gupta.
  
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Chris Wilson]

Quoting Anshuman Gupta (2020-12-10 05:02:48)
> On 2020-12-09 at 16:47:01 +0000, Chris Wilson wrote:
> > Quoting Anshuman Gupta (2020-12-09 16:25:02)
> > > On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > > > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > > > Create a separate igt test group and move package C
> > > > > state in to this subgroup.
> > > > > Run powertop --auto-tune to tune SOC power configuration
> > > > > for package C state tests.
> > > > > 
> > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > > ---
> > > > >  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
> > > > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > > > index af55b569..42bc44d9 100644
> > > > > --- a/tests/i915/i915_pm_rpm.c
> > > > > +++ b/tests/i915/i915_pm_rpm.c
> > > > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > > > >         /* XXX Also we can test wake up via exec nop */
> > > > >  }
> > > > >  
> > > > > +static bool setup_powertop(void)
> > > > > +{
> > > > > +       FILE *fp;
> > > > > +       char tmp[512];
> > > > > +
> > > > > +       fp = popen("powertop --auto-tune", "r");
> > > > 
> > > > Doesn't this defeat the point of having it work out of the box?
> May be misunderstood your comment, is it PC state or powertop should work
> out of box ?

Powermanagement should not require the user to configure it before it
can work.

powertop in particular may tweak the gfx, which is verboten.

Manually perform any configuration you think is required, and warn if the
initial configuration does not support reaching the lowest pc state the
system can. File bugs.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Rodrigo Vivi]

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

On Thu, Dec 10, 2020 at 2:42 AM Chris Wilson <chris@chris-wilson.co.uk>
wrote:

> Quoting Anshuman Gupta (2020-12-10 05:02:48)
> > On 2020-12-09 at 16:47:01 +0000, Chris Wilson wrote:
> > > Quoting Anshuman Gupta (2020-12-09 16:25:02)
> > > > On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > > > > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > > > > Create a separate igt test group and move package C
> > > > > > state in to this subgroup.
> > > > > > Run powertop --auto-tune to tune SOC power configuration
> > > > > > for package C state tests.
> > > > > >
> > > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > > > ---
> > > > > >  tests/i915/i915_pm_rpm.c | 35
> +++++++++++++++++++++++++++++++----
> > > > > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > > > > index af55b569..42bc44d9 100644
> > > > > > --- a/tests/i915/i915_pm_rpm.c
> > > > > > +++ b/tests/i915/i915_pm_rpm.c
> > > > > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > > > > >         /* XXX Also we can test wake up via exec nop */
> > > > > >  }
> > > > > >
> > > > > > +static bool setup_powertop(void)
> > > > > > +{
> > > > > > +       FILE *fp;
> > > > > > +       char tmp[512];
> > > > > > +
> > > > > > +       fp = popen("powertop --auto-tune", "r");
> > > > >
> > > > > Doesn't this defeat the point of having it work out of the box?
> > May be misunderstood your comment, is it PC state or powertop should work
> > out of box ?
>
> Powermanagement should not require the user to configure it before it
> can work.
>
> powertop in particular may tweak the gfx, which is verboten.
>
> Manually perform any configuration you think is required, and warn if the
> initial configuration does not support reaching the lowest pc state the
> system can. File bugs.
>

I understand why Anshuman did this. There are many many components
out there that are blocking the PC10 and this powertop --auto-tune
is trying to take care of all the corner cases to get the "best" of all the
components so we can try to garantee that we are not getting blocked by
other components.

However this is very very dangerous. It toggles a lot of unsafe and
untested cases.

I have lost a laptop right after a powertop --auto-tune and have never used
that
in my life again. Coincidence? Maybe... but I don't want to take the risk
in my
machine and I definitely cannot recommend its use.


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

[-- Attachment #2: Type: text/html, Size: 4110 bytes --]

RE: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group

[Gupta, Anshuman]

[-- Attachment #1: Type: text/plain, Size: 3638 bytes --]



From: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Sent: Friday, January 22, 2021 1:26 AM
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 2/6] i915/i915_pm_rpm.c: create PC state subtest group



On Thu, Dec 10, 2020 at 2:42 AM Chris Wilson <chris@chris-wilson.co.uk<mailto:chris@chris-wilson.co.uk>> wrote:
Quoting Anshuman Gupta (2020-12-10 05:02:48)
> On 2020-12-09 at 16:47:01 +0000, Chris Wilson wrote:
> > Quoting Anshuman Gupta (2020-12-09 16:25:02)
> > > On 2020-12-09 at 16:25:55 +0000, Chris Wilson wrote:
> > > > Quoting Anshuman Gupta (2020-12-09 16:06:38)
> > > > > Create a separate igt test group and move package C
> > > > > state in to this subgroup.
> > > > > Run powertop --auto-tune to tune SOC power configuration
> > > > > for package C state tests.
> > > > >
> > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com<mailto:anshuman.gupta@intel.com>>
> > > > > ---
> > > > >  tests/i915/i915_pm_rpm.c | 35 +++++++++++++++++++++++++++++++----
> > > > >  1 file changed, 31 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> > > > > index af55b569..42bc44d9 100644
> > > > > --- a/tests/i915/i915_pm_rpm.c
> > > > > +++ b/tests/i915/i915_pm_rpm.c
> > > > > @@ -832,6 +832,25 @@ static void basic_subtest(void)
> > > > >         /* XXX Also we can test wake up via exec nop */
> > > > >  }
> > > > >
> > > > > +static bool setup_powertop(void)
> > > > > +{
> > > > > +       FILE *fp;
> > > > > +       char tmp[512];
> > > > > +
> > > > > +       fp = popen("powertop --auto-tune", "r");
> > > >
> > > > Doesn't this defeat the point of having it work out of the box?
> May be misunderstood your comment, is it PC state or powertop should work
> out of box ?

Powermanagement should not require the user to configure it before it
can work.

powertop in particular may tweak the gfx, which is verboten.

Manually perform any configuration you think is required, and warn if the
initial configuration does not support reaching the lowest pc state the
system can. File bugs.

I understand why Anshuman did this. There are many many components
out there that are blocking the PC10 and this powertop --auto-tune
is trying to take care of all the corner cases to get the "best" of all the
components so we can try to garantee that we are not getting blocked by other components.

However this is very very dangerous. It toggles a lot of unsafe and untested cases.

I have lost a laptop right after a powertop --auto-tune and have never used that
in my life again. Coincidence? Maybe... but I don't want to take the risk in my
machine and I definitely cannot recommend its use.
Thanks Rodrigo for comment.
Actually as part of manual validation powertop being used by different PnP Val team.
Without using powertop surely we are not going to hit PC8/PC10 in that case I am afraid that
these test can’t be part of automatic validation in CI.
We may need to find alternative in some sort of OS level Power Management Policy decision maker,
which can decide conservative power saving oriented policy.
Runtime idle  with display on which leads to PC10 and s0ix effectively for client products,
Ubuntu OS looks like not really tuned for that.

Thanks,
Anshuman Gupta.

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

[-- Attachment #2: Type: text/html, Size: 8326 bytes --]