[igt-dev] [PATCH i-g-t v4 0/2] Enabling PC8+ residency for all GEN9+ platforms v4

[igt-dev] [PATCH i-g-t v4 0/2] Enabling PC8+ residency for all GEN9+ platforms v4

[Anshuman Gupta]

This patch series enable PC8+ residency test, earlier these tests
were only enabled for Haswell and Broadwell.

I have addressed the below review comment provided by Imre
on old series (https://patchwork.freedesktop.org/series/57035/)

1. Made this test future proof to enable for all Gen9+ platform.
2. Added a pc8_needs_screen_off flag to distinguish between HSW/BDW and Gen9+
   Platform.
3. Since Gen9 onwards PC8+ residency doesn't require display to be turned off,
   this patch series tests PC8 residency with all screens on.

Addressed review comment provided by Ram on revision 3
(patch commit log describes the changes).

Tested on Gen9 KBL Platform, unable to test on ICL, because ICL systems at local
BA setup are not entering to PC2 itself.

Planning a separate series for a subtest to validate pc8 with multiple pipes
usages and all planes enabled with max resolution, to create maximum memory
bandwidth scenario.

Anshuman Gupta (2):
  tests/i915/i915_pm_rpm: Enable PC8+ residency test for all Gen9+
  tests/i915/i915_pm_rpm: modeset-pc8-residency-stress

 tests/i915/i915_pm_rpm.c | 101 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 91 insertions(+), 10 deletions(-)

-- 
2.7.4

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

[igt-dev] [PATCH i-g-t v4 1/2] tests/i915/i915_pm_rpm: Enable PC8+ residency test for all Gen9+

[Anshuman Gupta]

Enabled has_pc8 global for ICL and Gen9+.
Modified PC8+ residency sub-test with all screen enabled.

v2:Fixed the issue of skipped test on HSW.
   Improved the code comment for MSR_PKG_CST_CONFIG_CONTROL mask and PC8
   bits, it holds good for SKL/ICL and Goldmont microarchitecture.
   Code readabilty improvement.

v3:Removed the connected_screens global. [Ram]
   Removed pc8_needs_screen_off from mode_set_data structure,
   made it global, aligning to has_pc8 and has_runtime_pm globals. [Ram]
   Reuse connector lcoal variable in init_modeset_params_for_all_screen(). [Ram]
   Addressed Coding guide lines comments. [Ram]

v4:Improved the code comment for MSR_PKG_CST_CONFIG_CONTROL mask and PC8 bits. [Ram]
   Introduced set_screens_mode_params() function to fix warning of line exceeding
   80 char, fixed this warning at other places too. [Ram]
   Using ms_data modset_data global structure. [Ram]
   Introduced macros for timeout values, given to PC8+ residency check function. [Ram]

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

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index be296f5..70e8ac7 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -52,7 +52,11 @@
 #include "igt_device.h"
 
 #define MSR_PKG_CST_CONFIG_CONTROL	0xE2
-/* HSW/BDW: */
+/*
+ * 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
 
@@ -64,6 +68,10 @@
 #define MAX_ENCODERS	32
 #define MAX_CRTCS	16
 
+#define TIME_OUT_SEC_5   5
+#define TIME_OUT_SEC_10 10
+#define TIME_OUT_SEC_30 30
+
 enum pc8_status {
 	PC8_ENABLED,
 	PC8_DISABLED
@@ -90,7 +98,7 @@ enum plane_type {
 
 int drm_fd, msr_fd, pc8_status_fd;
 int debugfs;
-bool has_runtime_pm, has_pc8;
+bool has_runtime_pm, has_pc8, pc8_needs_screen_off;
 struct mode_set_data ms_data;
 
 /* Stuff used when creating FBs and mode setting. */
@@ -121,9 +129,25 @@ struct modeset_params {
 struct modeset_params lpsp_mode_params;
 struct modeset_params non_lpsp_mode_params;
 struct modeset_params *default_mode_params;
+struct modeset_params *screens_mode_params[MAX_CONNECTORS];
 
 static int8_t *pm_data = NULL;
 
+static inline void set_screens_mode_param(drmModeConnectorPtr connector,
+					  struct modeset_params *params)
+{
+	drmModeModeInfoPtr mode = NULL;
+
+	mode = &connector->modes[0];
+	igt_create_pattern_fb(drm_fd, mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+			      &params->fb);
+	params->crtc_id = kmstest_find_crtc_for_connector(drm_fd, ms_data.res,
+							  connector, 0);
+	params->connector_id = connector->connector_id;
+	params->mode = mode;
+}
+
 static int modprobe(const char *driver)
 {
 	return igt_kmod_load(driver, NULL);
@@ -297,6 +321,28 @@ static bool init_modeset_params_for_type(struct mode_set_data *data,
 	return true;
 }
 
+static void init_modeset_params_for_all_screen(void)
+{
+	drmModeConnectorPtr connector = NULL;
+	int screen = 0;
+
+	if (!ms_data.res)
+		return;
+
+	for (int i = 0; i < ms_data.res->count_connectors; i++) {
+		connector = ms_data.connectors[i];
+
+		if (connector->connection == DRM_MODE_CONNECTED
+		    && connector->count_modes) {
+			screens_mode_params[screen] =
+				malloc(sizeof(struct modeset_params));
+			set_screens_mode_param(connector,
+					       screens_mode_params[screen]);
+			screen++;
+		}
+	}
+}
+
 static void init_modeset_cached_params(struct mode_set_data *data)
 {
 	bool lpsp, non_lpsp;
@@ -305,6 +351,7 @@ 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);
+	init_modeset_params_for_all_screen();
 
 	if (lpsp)
 		default_mode_params = &lpsp_mode_params;
@@ -353,6 +400,22 @@ static bool enable_one_screen_with_type(struct mode_set_data *data,
 	return set_mode_for_params(params);
 }
 
+static void enable_all_screens(void)
+{
+	struct modeset_params *params = NULL;
+
+	/* SKIP if there are no connected screens. */
+	igt_require(screens_mode_params[0]);
+
+	for (int i = 0; i < MAX_CONNECTORS ; i++) {
+		params = screens_mode_params[i];
+		if (params)
+			set_mode_for_params(params);
+		else
+			break;
+	}
+}
+
 static void enable_one_screen(struct mode_set_data *data)
 {
 	/* SKIP if there are no connected screens. */
@@ -685,8 +748,12 @@ 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))
+	if (IS_HASWELL(ms_data.devid) || IS_BROADWELL(ms_data.devid))
+		pc8_needs_screen_off = true;
+	else if (AT_LEAST_GEN(ms_data.devid, 9))
+		pc8_needs_screen_off = false;
+	/* Only Haswell supports the PC8 feature on lesser than GEN9. */
+	else
 		return;
 
 	/* Make sure our Kernel supports MSR and the module is loaded. */
@@ -803,14 +870,22 @@ static void pc8_residency_subtest(void)
 
 	/* Make sure PC8+ residencies move! */
 	disable_all_screens(&ms_data);
-	igt_assert_f(pc8_plus_residency_changed(30),
+	igt_assert_f(pc8_plus_residency_changed(TIME_OUT_SEC_30),
 		     "Machine is not reaching PC8+ states, please check its "
 		     "configuration.\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 (pc8_needs_screen_off) {
+		enable_one_screen(&ms_data);
+		igt_assert_f(!pc8_plus_residency_changed(TIME_OUT_SEC_10),
+			     "PC8+ residency didn't stop with "
+			     "screen enabled.\n");
+	} else {
+		enable_all_screens();
+		igt_assert_f(pc8_plus_residency_changed(TIME_OUT_SEC_10),
+			     "Machine is not reaching PC8+ states "
+			     "with all screen enabled.\n");
+	}
 }
 
 static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
-- 
2.7.4

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

[igt-dev] [PATCH i-g-t v4 2/2] tests/i915/i915_pm_rpm: modeset-pc8-residency-stress

[Anshuman Gupta]

Introduced pc8_needs_screen_off flag in order to differentiate
between HASWELL/BROADWELL and AT_LEAST_GEN9. GEN9 onwards
PC8+ residency does't require display to be turned on.

v3:Removed pc8_needs_screen_off from mode_set_data structure,
   made it global, aligning to has_pc8 and has_runtime_pm globals. [Ram]
   Made modeset_subtest() to tests PC8+ residency after enabling a screen,
   earlier it expects PC8+ residency to stop on HSW/BDW.

v4:Fixed conditional code for pc8_needs_screen_off. [Ram]
   Used macros for timeout values, given to PC8+ residency check function. [Ram]
   Changed the screen on timeout to check pc8+ residency to 10 seconds.

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

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 70e8ac7..092ee64 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -907,7 +907,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(30));
+			igt_assert(pc8_plus_residency_changed(TIME_OUT_SEC_30));
 		if (wait_flags & WAIT_EXTRA)
 			sleep(5);
 
@@ -917,7 +917,13 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
 		if (wait_flags & WAIT_STATUS)
 			igt_assert(wait_for_active());
 		if (wait_flags & WAIT_PC8_RES)
-			igt_assert(!pc8_plus_residency_changed(5));
+			if (pc8_needs_screen_off)
+				igt_assert(!pc8_plus_residency_changed
+					   (TIME_OUT_SEC_5));
+			else
+				igt_assert(pc8_plus_residency_changed
+					   (TIME_OUT_SEC_10));
+
 		if (wait_flags & WAIT_EXTRA)
 			sleep(5);
 	}
-- 
2.7.4

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

[igt-dev] ✗ Fi.CI.BAT: failure for Enabling PC8+ residency for all GEN9+ platforms (rev4)

[Patchwork]

== Series Details ==

Series: Enabling PC8+ residency for all GEN9+ platforms (rev4)
URL   : https://patchwork.freedesktop.org/series/57640/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5831 -> IGTPW_2724
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2724 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2724, 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/57640/revisions/4/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-byt-n2820:       PASS -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +1

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


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

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u 


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

    * IGT: IGT_4911 -> IGTPW_2724

  CI_DRM_5831: 8cac0cc264d2a6af0b33370b542b12d516e022c5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2724: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2724/
  IGT_4911: d9fe699ea45406e279b78d1afdb4d57a205a3c99 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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