[igt-dev] [PATCH i-g-t v1 0/6] Added new test file pm_dc.c.

[igt-dev] [PATCH i-g-t v1 0/6] Added new test file pm_dc.c.

[Jyoti Yadav]

This patch series adds new tests to validate Display C states.
Mainly DC5 and Dc6 for various platforms like Icelake, Skylake,
Broxton.

Jyoti Yadav (6):
  [intel-gfx][igt-dev] lib/igt_pm : Moves Dmc_loaded() function into
    library.      Because it will be used by new test pm_dc.c which will
    validate Display C States.
  [intel-gfx][igt-dev] tests/pm_dc : Added new test to verify Display C
    States.     Currently this test validate DC5 upon PSR entry for
    Icelake, Skylake and Broxton platform.
  [intel-gfx][igt-dev] tests/pm_dc : Added test to validate DC6 state   
     on Gen11 platform. On Gen11 Platform, DC6 is achieved with PSR
    entry.
  [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate 
       DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF
    sequence.
  [intel-gfx][igt-dev] lib/intel_reg : This patch adds Display C States
    related residency counters.
  [intel-gfx][igt-dev] tests/Makefile : Added new file tests/pm_dc.c for
    compilation.

 lib/igt_pm.c           |  29 +++++++
 lib/igt_pm.h           |   1 +
 lib/intel_reg.h        |   5 ++
 tests/Makefile.sources |   1 +
 tests/pm_dc.c          | 224 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/pm_rpm.c         |  17 +---
 6 files changed, 261 insertions(+), 16 deletions(-)
 create mode 100644 tests/pm_dc.c

-- 
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 v1 1/6] [intel-gfx] lib/igt_pm : Moves Dmc_loaded() function into library. Because it will be used by new test pm_dc.c which will validate Display C States.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 lib/igt_pm.c   | 29 +++++++++++++++++++++++++++++
 lib/igt_pm.h   |  1 +
 tests/pm_rpm.c | 17 +----------------
 3 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 4902723..ae87ab4 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -38,6 +38,7 @@
 #include "drmtest.h"
 #include "igt_pm.h"
 #include "igt_aux.h"
+#include "igt_sysfs.h"
 
 /**
  * SECTION:igt_pm
@@ -620,3 +621,31 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
 {
 	return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
 }
+
+/**
+ * dmc_loaded:
+ * @debugfs : indicator that debugfs dir /sys/kerenl/debug/dri/0/i915 is
+ * available or not.
+
+ * Check whether DMC FW is loaded or not. DMC FW is require for few Display C
+ * states like DC5 and DC6. FW does the Context Save and Restore during Display
+ * C States entry and exit.
+ *
+ * Returns:
+ * True if DMC FW is loaded otherwise false.
+ */
+bool dmc_loaded(int debugfs)
+{
+	igt_require(debugfs != -1);
+	char buf[15];
+	int len;
+
+	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
+	if (len < 0)
+		return true; /* no CSR support, no DMC requirement */
+
+	buf[len] = '\0';
+
+	igt_info("DMC: %s\n", buf);
+	return strstr(buf, "fw loaded: yes");
+}
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index 10cc679..b4a98aa 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -50,5 +50,6 @@ 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);
+bool dmc_loaded(int debugfs);
 
 #endif /* IGT_PM_H */
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index c24fd95..ebb624b 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -693,21 +693,6 @@ static void setup_pc8(void)
 	has_pc8 = true;
 }
 
-static bool dmc_loaded(void)
-{
-	char buf[15];
-	int len;
-
-	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
-	if (len < 0)
-	    return true; /* no CSR support, no DMC requirement */
-
-	buf[len] = '\0';
-
-	igt_info("DMC: %s\n", buf);
-	return strstr(buf, "fw loaded: yes");
-}
-
 static bool setup_environment(void)
 {
 	if (has_runtime_pm)
@@ -730,7 +715,7 @@ static bool setup_environment(void)
 	igt_info("Runtime PM support: %d\n", has_runtime_pm);
 	igt_info("PC8 residency support: %d\n", has_pc8);
 	igt_require(has_runtime_pm);
-	igt_require(dmc_loaded());
+	igt_require(dmc_loaded(debugfs));
 
 out:
 	disable_all_screens(&ms_data);
-- 
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 v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 tests/pm_dc.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 180 insertions(+)
 create mode 100644 tests/pm_dc.c

diff --git a/tests/pm_dc.c b/tests/pm_dc.c
new file mode 100644
index 0000000..db38ccf
--- /dev/null
+++ b/tests/pm_dc.c
@@ -0,0 +1,180 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt.h"
+#include "igt_sysfs.h"
+#include "igt_psr.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include "intel_bufmgr.h"
+#include "intel_io.h"
+
+
+typedef struct {
+	int drm_fd;
+	int debugfs_fd;
+	uint32_t devid;
+	igt_display_t display;
+	struct igt_fb fb_white;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+} data_t;
+
+bool has_runtime_pm;
+
+static void setup_output(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+
+	for_each_pipe_with_valid_output(display, pipe, output) {
+		drmModeConnectorPtr c = output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
+			continue;
+
+		igt_output_set_pipe(output, pipe);
+		data->output = output;
+		data->mode = igt_output_get_mode(output);
+
+		return;
+	}
+}
+
+static void display_init(data_t *data)
+{
+	igt_display_init(&data->display, data->drm_fd);
+	setup_output(data);
+}
+
+static void display_fini(data_t *data)
+{
+	igt_display_fini(&data->display);
+}
+
+static bool sink_support(data_t *data)
+{
+	char buf[512];
+
+	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
+			 buf, sizeof(buf));
+
+	return strstr(buf, "Sink_Support: yes\n");
+}
+
+static void cleanup(data_t *data)
+{
+	igt_plane_t *primary;
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	igt_display_commit(&data->display);
+	igt_remove_fb(data->drm_fd, &data->fb_white);
+}
+
+static void setup_primary(data_t *data)
+{
+	igt_plane_t *primary;
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_I915_FORMAT_MOD_X_TILED,
+			    1.0, 1.0, 1.0,
+			    &data->fb_white);
+	igt_display_commit(&data->display);
+
+	igt_plane_set_fb(primary, &data->fb_white);
+	igt_display_commit(&data->display);
+}
+
+static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
+{
+	uint32_t dc3_dc5_count;
+
+	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
+		dc3_dc5_count = INREG(SKL_CSR_DC3_DC5_COUNT);
+	else if (IS_BROXTON(dev_id))
+		dc3_dc5_count = INREG(BXT_CSR_DC3_DC5_COUNT);
+	else {
+		igt_warn("Currently DC5 support is for ICL, SKL and BXT Platform (%s)\n",
+			__func__);
+		return -EPERM;
+	}
+	return dc3_dc5_count;
+}
+
+static void test_dc5(data_t *data)
+{
+	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
+
+	dc3_dc5_counter_before_psr = read_dc3_dc5_counter(data->devid);
+	setup_primary(data);
+	igt_assert(psr_wait_entry(data->debugfs_fd));
+	dc3_dc5_counter_after_psr = read_dc3_dc5_counter(data->devid);
+	igt_require_f(dc3_dc5_counter_after_psr > dc3_dc5_counter_before_psr,
+			"DC5 State is not achieved\n");
+}
+
+int main(int argc, char *argv[])
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+	igt_fixture {
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+		igt_require(data.debugfs_fd != -1);
+		kmstest_set_vt_graphics_mode();
+		data.devid = intel_get_drm_devid(data.drm_fd);
+		psr_enable(data.debugfs_fd);
+		has_runtime_pm = igt_setup_runtime_pm();
+		igt_info("Runtime PM support: %d\n", has_runtime_pm);
+		igt_require(has_runtime_pm);
+		igt_require(dmc_loaded(data.debugfs_fd));
+
+		igt_require_f(sink_support(&data),
+			      "Sink does not support PSR\n");
+		display_init(&data);
+	}
+
+	igt_subtest("DC5_state") {
+		test_dc5(&data);
+		cleanup(&data);
+	}
+	igt_fixture {
+		psr_disable(data.debugfs_fd);
+		close(data.debugfs_fd);
+		display_fini(&data);
+	}
+
+	igt_exit();
+}
-- 
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 v1 3/6] [intel-gfx] tests/pm_dc : Added test to validate DC6 state on Gen11 platform. On Gen11 Platform, DC6 is achieved with PSR entry.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 tests/pm_dc.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/pm_dc.c b/tests/pm_dc.c
index db38ccf..b609fad 100644
--- a/tests/pm_dc.c
+++ b/tests/pm_dc.c
@@ -132,6 +132,20 @@ static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
 	return dc3_dc5_count;
 }
 
+static uint32_t read_dc5_dc6_counter(uint32_t dev_id)
+{
+	uint32_t dc5_dc6_count;
+
+	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
+		dc5_dc6_count = INREG(SKL_CSR_DC5_DC6_COUNT);
+	else {
+		igt_warn("Currently DC6 support is for ICL, SKL Platform (%s)\n",
+			__func__);
+		return -EPERM;
+	}
+	return dc5_dc6_count;
+}
+
 static void test_dc5(data_t *data)
 {
 	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
@@ -147,6 +161,7 @@ static void test_dc5(data_t *data)
 int main(int argc, char *argv[])
 {
 	data_t data = {};
+	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
 
 	igt_skip_on_simulation();
 	igt_fixture {
@@ -170,6 +185,14 @@ int main(int argc, char *argv[])
 		test_dc5(&data);
 		cleanup(&data);
 	}
+	igt_subtest("DC6_state_on_gen11") {
+		dc5_dc6_counter_before_psr = read_dc5_dc6_counter(data.devid);
+		test_dc5(&data);
+		dc5_dc6_counter_after_psr = read_dc5_dc6_counter(data.devid);
+		igt_require_f(dc5_dc6_counter_after_psr > dc5_dc6_counter_before_psr,
+				"DC6 State is not achieved\n");
+		cleanup(&data);
+	}
 	igt_fixture {
 		psr_disable(data.debugfs_fd);
 		close(data.debugfs_fd);
-- 
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 v1 4/6] [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF sequence.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 tests/pm_dc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tests/pm_dc.c b/tests/pm_dc.c
index b609fad..0f18ece 100644
--- a/tests/pm_dc.c
+++ b/tests/pm_dc.c
@@ -146,6 +146,18 @@ static uint32_t read_dc5_dc6_counter(uint32_t dev_id)
 	return dc5_dc6_count;
 }
 
+static void dpms_off_on(data_t *data)
+{
+	for (int i = 0; i < data->display.n_outputs; i++) {
+		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
+					   DRM_MODE_DPMS_OFF);
+	}
+	for (int i = 0; i < data->display.n_outputs; i++) {
+		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
+				   DRM_MODE_DPMS_ON);
+	}
+}
+
 static void test_dc5(data_t *data)
 {
 	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
@@ -162,6 +174,7 @@ int main(int argc, char *argv[])
 {
 	data_t data = {};
 	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
+	uint32_t dc5_dc6_counter_before_suspend, dc5_dc6_counter_after_resume;
 
 	igt_skip_on_simulation();
 	igt_fixture {
@@ -193,6 +206,14 @@ int main(int argc, char *argv[])
 				"DC6 State is not achieved\n");
 		cleanup(&data);
 	}
+	igt_subtest("DC6_state_on_gen9") {
+		dc5_dc6_counter_before_suspend = read_dc5_dc6_counter(data.devid);
+		dpms_off_on(&data);
+		dc5_dc6_counter_after_resume = read_dc5_dc6_counter(data.devid);
+		igt_require_f(dc5_dc6_counter_after_resume > dc5_dc6_counter_before_suspend,
+				"DC6 State is not achieved\n");
+		cleanup(&data);
+	}
 	igt_fixture {
 		psr_disable(data.debugfs_fd);
 		close(data.debugfs_fd);
-- 
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 v1 5/6] [intel-gfx] lib/intel_reg : This patch adds Display C States related residency counters.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 lib/intel_reg.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index f85fb74..4f424f0 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3621,4 +3621,9 @@ typedef enum {
 #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
 #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
 
+/* Display C states related residency counter registers. */
+#define SKL_CSR_DC3_DC5_COUNT  0x80030
+#define SKL_CSR_DC5_DC6_COUNT  0x8002C
+#define BXT_CSR_DC3_DC5_COUNT  0x80038
+
 #endif /* _I810_REG_H */
-- 
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 v1 6/6] [intel-gfx] tests/Makefile : Added new file tests/pm_dc.c for compilation.

[Jyoti Yadav]

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 tests/Makefile.sources | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f..d491420 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -214,6 +214,7 @@ TESTS_progs = \
 	pm_lpsp \
 	pm_rc6_residency \
 	pm_rpm \
+	pm_dc \
 	pm_rps \
 	pm_sseu \
 	prime_busy \
-- 
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: success for Added new test file pm_dc.c.

[Patchwork]

== Series Details ==

Series: Added new test file pm_dc.c.
URL   : https://patchwork.freedesktop.org/series/50229/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4874 -> IGTPW_1876 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      NOTRUN -> DMESG-WARN (fdo#105998)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cfl-8109u:       INCOMPLETE (fdo#106070) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-cnl-u:           FAIL (fdo#107336) -> PASS

    
    ==== Warnings ====

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u:           DMESG-WARN (fdo#108070) -> INCOMPLETE (fdo#107713, fdo#107901)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107901 https://bugs.freedesktop.org/show_bug.cgi?id=107901
  fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070


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

  Additional (1): fi-skl-6700hq 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-bdw-gvtdvm fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * IGT: IGT_4649 -> IGTPW_1876

  CI_DRM_4874: 7bb42432ef1cc63b4a6a223e19455f6c4f9a1538 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1876: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1876/
  IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1876/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 Added new test file pm_dc.c.

[Patchwork]

== Series Details ==

Series: Added new test file pm_dc.c.
URL   : https://patchwork.freedesktop.org/series/50229/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4649_full -> IGTPW_1876_full =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_await@wide-contexts:
      shard-apl:          PASS -> FAIL (fdo#106680)

    igt@kms_busy@extended-modeset-hang-newfb-render-d:
      shard-snb:          SKIP -> INCOMPLETE (fdo#105411)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

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

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-wc:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_cursor_legacy@pipe-c-forked-move:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

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

    igt@kms_universal_plane@universal-plane-gen9-features-pipe-a:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +14

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4649 -> IGTPW_1876
    * Linux: CI_DRM_4859 -> CI_DRM_4874

  CI_DRM_4859: 841654aee134e50b21bf8f2aab17da8d0afd14c4 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4874: 7bb42432ef1cc63b4a6a223e19455f6c4f9a1538 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1876: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1876/
  IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v1 1/6] [intel-gfx] lib/igt_pm : Moves Dmc_loaded() function into library. Because it will be used by new test pm_dc.c which will validate Display C States.

[Imre Deak]

On Wed, Sep 26, 2018 at 11:59:18AM -0400, Jyoti Yadav wrote:

Please use a tag format for the one line description like
lib/igt_pm:
(so no space before :), and always provide a longer description
in the message body. I guess it got mangled into your message
subject due to a missing new line.

Any reason you sent this to the intel-gfx list too? For IGT patches I'd
use only the IGT ML.

> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  lib/igt_pm.c   | 29 +++++++++++++++++++++++++++++
>  lib/igt_pm.h   |  1 +
>  tests/pm_rpm.c | 17 +----------------
>  3 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 4902723..ae87ab4 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -38,6 +38,7 @@
>  #include "drmtest.h"
>  #include "igt_pm.h"
>  #include "igt_aux.h"
> +#include "igt_sysfs.h"
>  
>  /**
>   * SECTION:igt_pm
> @@ -620,3 +621,31 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
>  {
>  	return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
>  }
> +
> +/**
> + * dmc_loaded:
> + * @debugfs : indicator that debugfs dir /sys/kerenl/debug/dri/0/i915 is
> + * available or not.

It's an fd to the debugfs dir.

> +
> + * Check whether DMC FW is loaded or not. DMC FW is require for few Display C
> + * states like DC5 and DC6. FW does the Context Save and Restore during Display
> + * C States entry and exit.
> + *
> + * Returns:
> + * True if DMC FW is loaded otherwise false.
> + */
> +bool dmc_loaded(int debugfs)

Missing igt_pm_ prefix.

> +{
> +	igt_require(debugfs != -1);
> +	char buf[15];
> +	int len;
> +
> +	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
> +	if (len < 0)
> +		return true; /* no CSR support, no DMC requirement */
> +
> +	buf[len] = '\0';
> +
> +	igt_info("DMC: %s\n", buf);

igt_debug() seems more proper for a lib function.

> +	return strstr(buf, "fw loaded: yes");
> +}
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 10cc679..b4a98aa 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -50,5 +50,6 @@ 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);
> +bool dmc_loaded(int debugfs);
>  
>  #endif /* IGT_PM_H */
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index c24fd95..ebb624b 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -693,21 +693,6 @@ static void setup_pc8(void)
>  	has_pc8 = true;
>  }
>  
> -static bool dmc_loaded(void)
> -{
> -	char buf[15];
> -	int len;
> -
> -	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
> -	if (len < 0)
> -	    return true; /* no CSR support, no DMC requirement */
> -
> -	buf[len] = '\0';
> -
> -	igt_info("DMC: %s\n", buf);
> -	return strstr(buf, "fw loaded: yes");
> -}
> -
>  static bool setup_environment(void)
>  {
>  	if (has_runtime_pm)
> @@ -730,7 +715,7 @@ static bool setup_environment(void)
>  	igt_info("Runtime PM support: %d\n", has_runtime_pm);
>  	igt_info("PC8 residency support: %d\n", has_pc8);
>  	igt_require(has_runtime_pm);
> -	igt_require(dmc_loaded());
> +	igt_require(dmc_loaded(debugfs));
>  
>  out:
>  	disable_all_screens(&ms_data);
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.

[Imre Deak]

On Wed, Sep 26, 2018 at 11:59:19AM -0400, Jyoti Yadav wrote:
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/pm_dc.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 180 insertions(+)
>  create mode 100644 tests/pm_dc.c
> 
> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> new file mode 100644
> index 0000000..db38ccf
> --- /dev/null
> +++ b/tests/pm_dc.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "igt_psr.h"
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include "intel_bufmgr.h"
> +#include "intel_io.h"
> +
> +
> +typedef struct {
> +	int drm_fd;
> +	int debugfs_fd;
> +	uint32_t devid;
> +	igt_display_t display;
> +	struct igt_fb fb_white;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +} data_t;
> +
> +bool has_runtime_pm;
> +
> +static void setup_output(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +
> +	for_each_pipe_with_valid_output(display, pipe, output) {
> +		drmModeConnectorPtr c = output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> +			continue;
> +
> +		igt_output_set_pipe(output, pipe);
> +		data->output = output;
> +		data->mode = igt_output_get_mode(output);
> +
> +		return;
> +	}
> +}
> +
> +static void display_init(data_t *data)
> +{
> +	igt_display_init(&data->display, data->drm_fd);
> +	setup_output(data);
> +}
> +
> +static void display_fini(data_t *data)
> +{
> +	igt_display_fini(&data->display);
> +}
> +
> +static bool sink_support(data_t *data)
> +{
> +	char buf[512];
> +
> +	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> +			 buf, sizeof(buf));
> +
> +	return strstr(buf, "Sink_Support: yes\n");
> +}
> +
> +static void cleanup(data_t *data)
> +{
> +	igt_plane_t *primary;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_white);
> +}
> +
> +static void setup_primary(data_t *data)
> +{
> +	igt_plane_t *primary;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_create_color_fb(data->drm_fd,
> +			    data->mode->hdisplay, data->mode->vdisplay,
> +			    DRM_FORMAT_XRGB8888,
> +			    LOCAL_I915_FORMAT_MOD_X_TILED,
> +			    1.0, 1.0, 1.0,
> +			    &data->fb_white);
> +	igt_display_commit(&data->display);
> +
> +	igt_plane_set_fb(primary, &data->fb_white);
> +	igt_display_commit(&data->display);
> +}
> +
> +static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
> +{
> +	uint32_t dc3_dc5_count;
> +
> +	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
> +		dc3_dc5_count = INREG(SKL_CSR_DC3_DC5_COUNT);
> +	else if (IS_BROXTON(dev_id))
> +		dc3_dc5_count = INREG(BXT_CSR_DC3_DC5_COUNT);

The counters are available already in the DMC debugfs file, so no need
to duplicate the readout and platform check for that.

> +	else {
> +		igt_warn("Currently DC5 support is for ICL, SKL and BXT Platform (%s)\n",
> +			__func__);
> +		return -EPERM;

We want to skip the test if the DC counters don't exist, so we'd need an
igt_require() instead of the error return.

> +	}
> +	return dc3_dc5_count;
> +}
> +
> +static void test_dc5(data_t *data)
> +{
> +	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;

You could use shorter names.

> +
> +	dc3_dc5_counter_before_psr = read_dc3_dc5_counter(data->devid);
> +	setup_primary(data);
> +	igt_assert(psr_wait_entry(data->debugfs_fd));
> +	dc3_dc5_counter_after_psr = read_dc3_dc5_counter(data->devid);

I wonder if this would always work or if we'd need to wait for the
counter to increase with a timeout.

> +	igt_require_f(dc3_dc5_counter_after_psr > dc3_dc5_counter_before_psr,
> +			"DC5 State is not achieved\n");
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	data_t data = {};
> +
> +	igt_skip_on_simulation();
> +	igt_fixture {
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> +		igt_require(data.debugfs_fd != -1);
> +		kmstest_set_vt_graphics_mode();
> +		data.devid = intel_get_drm_devid(data.drm_fd);
> +		psr_enable(data.debugfs_fd);

psr_enable() should be called from the PSR specific subtest.

> +		has_runtime_pm = igt_setup_runtime_pm();
> +		igt_info("Runtime PM support: %d\n", has_runtime_pm);
> +		igt_require(has_runtime_pm);
> +		igt_require(dmc_loaded(data.debugfs_fd));
> +
> +		igt_require_f(sink_support(&data),
> +			      "Sink does not support PSR\n");
> +		display_init(&data);
> +	}
> +
> +	igt_subtest("DC5_state") {
> +		test_dc5(&data);
> +		cleanup(&data);
> +	}
> +	igt_fixture {
> +		psr_disable(data.debugfs_fd);

We want to _restore_ the PSR state not disable it, and that's done
already from an exit handler.

> +		close(data.debugfs_fd);
> +		display_fini(&data);
> +	}
> +
> +	igt_exit();
> +}
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 3/6] [intel-gfx] tests/pm_dc : Added test to validate DC6 state on Gen11 platform. On Gen11 Platform, DC6 is achieved with PSR entry.

[Imre Deak]

On Wed, Sep 26, 2018 at 11:59:20AM -0400, Jyoti Yadav wrote:
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/pm_dc.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> index db38ccf..b609fad 100644
> --- a/tests/pm_dc.c
> +++ b/tests/pm_dc.c
> @@ -132,6 +132,20 @@ static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
>  	return dc3_dc5_count;
>  }
>  
> +static uint32_t read_dc5_dc6_counter(uint32_t dev_id)
> +{
> +	uint32_t dc5_dc6_count;
> +
> +	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
> +		dc5_dc6_count = INREG(SKL_CSR_DC5_DC6_COUNT);
> +	else {
> +		igt_warn("Currently DC6 support is for ICL, SKL Platform (%s)\n",
> +			__func__);
> +		return -EPERM;
> +	}
> +	return dc5_dc6_count;
> +}
> +
>  static void test_dc5(data_t *data)
>  {
>  	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
> @@ -147,6 +161,7 @@ static void test_dc5(data_t *data)
>  int main(int argc, char *argv[])
>  {
>  	data_t data = {};
> +	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
>  
>  	igt_skip_on_simulation();
>  	igt_fixture {
> @@ -170,6 +185,14 @@ int main(int argc, char *argv[])
>  		test_dc5(&data);
>  		cleanup(&data);
>  	}
> +	igt_subtest("DC6_state_on_gen11") {
> +		dc5_dc6_counter_before_psr = read_dc5_dc6_counter(data.devid);
> +		test_dc5(&data);
> +		dc5_dc6_counter_after_psr = read_dc5_dc6_counter(data.devid);

Hm, why is this GEN11 specific? It should run anywhere where DC6 is
available, so all GEN9+ machines, except GEN9_LP. So I think instead of
this and the previous subtest we'd need a psr-dc5 and psr-dc6 subtests,
skipping each if the corresponding DC state isn't available on the
platform.

> +		igt_require_f(dc5_dc6_counter_after_psr > dc5_dc6_counter_before_psr,
> +				"DC6 State is not achieved\n");
> +		cleanup(&data);
> +	}
>  	igt_fixture {
>  		psr_disable(data.debugfs_fd);
>  		close(data.debugfs_fd);
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 4/6] [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF sequence.

[Imre Deak]

On Wed, Sep 26, 2018 at 11:59:21AM -0400, Jyoti Yadav wrote:
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/pm_dc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> index b609fad..0f18ece 100644
> --- a/tests/pm_dc.c
> +++ b/tests/pm_dc.c
> @@ -146,6 +146,18 @@ static uint32_t read_dc5_dc6_counter(uint32_t dev_id)
>  	return dc5_dc6_count;
>  }
>  
> +static void dpms_off_on(data_t *data)
> +{
> +	for (int i = 0; i < data->display.n_outputs; i++) {
> +		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
> +					   DRM_MODE_DPMS_OFF);
> +	}
> +	for (int i = 0; i < data->display.n_outputs; i++) {
> +		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
> +				   DRM_MODE_DPMS_ON);
> +	}

Again, not sure if it's guaranteed that the residency counter will
increase with instant off/on, probably we'd need to wait for the
increment with a timeout.

> +}
> +
>  static void test_dc5(data_t *data)
>  {
>  	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
> @@ -162,6 +174,7 @@ int main(int argc, char *argv[])
>  {
>  	data_t data = {};
>  	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
> +	uint32_t dc5_dc6_counter_before_suspend, dc5_dc6_counter_after_resume;

It's not really suspend/resume and I don't think you need separate vars
for each subtest.

>  
>  	igt_skip_on_simulation();
>  	igt_fixture {
> @@ -193,6 +206,14 @@ int main(int argc, char *argv[])
>  				"DC6 State is not achieved\n");
>  		cleanup(&data);
>  	}
> +	igt_subtest("DC6_state_on_gen9") {

This isn't GEN9 specific, so could be called dc6-dpms-off. I suppose you
also want a dc5-dpms-off test.

> +		dc5_dc6_counter_before_suspend = read_dc5_dc6_counter(data.devid);
> +		dpms_off_on(&data);
> +		dc5_dc6_counter_after_resume = read_dc5_dc6_counter(data.devid);
> +		igt_require_f(dc5_dc6_counter_after_resume > dc5_dc6_counter_before_suspend,
> +				"DC6 State is not achieved\n");
> +		cleanup(&data);
> +	}
>  	igt_fixture {
>  		psr_disable(data.debugfs_fd);
>  		close(data.debugfs_fd);
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 6/6] [intel-gfx] tests/Makefile : Added new file tests/pm_dc.c for compilation.

[Imre Deak]

On Wed, Sep 26, 2018 at 11:59:23AM -0400, Jyoti Yadav wrote:
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/Makefile.sources | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c84933f..d491420 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -214,6 +214,7 @@ TESTS_progs = \
>  	pm_lpsp \
>  	pm_rc6_residency \
>  	pm_rpm \
> +	pm_dc \

tests/meson.build also needs to be updated.

CI didn't run the new subtests, not sure why, maybe it depends on
meson.build?

>  	pm_rps \
>  	pm_sseu \
>  	prime_busy \
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 4/6] [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF sequence.

[Yadav, Jyoti R]

On 10/2/2018 7:06 PM, Imre Deak wrote:
> On Wed, Sep 26, 2018 at 11:59:21AM -0400, Jyoti Yadav wrote:
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>> ---
>>   tests/pm_dc.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
>> index b609fad..0f18ece 100644
>> --- a/tests/pm_dc.c
>> +++ b/tests/pm_dc.c
>> @@ -146,6 +146,18 @@ static uint32_t read_dc5_dc6_counter(uint32_t dev_id)
>>   	return dc5_dc6_count;
>>   }
>>   
>> +static void dpms_off_on(data_t *data)
>> +{
>> +	for (int i = 0; i < data->display.n_outputs; i++) {
>> +		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
>> +					   DRM_MODE_DPMS_OFF);
>> +	}
>> +	for (int i = 0; i < data->display.n_outputs; i++) {
>> +		kmstest_set_connector_dpms(data->drm_fd, data->display.outputs[i].config.connector,
>> +				   DRM_MODE_DPMS_ON);
>> +	}
> Again, not sure if it's guaranteed that the residency counter will
> increase with instant off/on, probably we'd need to wait for the
> increment with a timeout.

while doing some basic testing on SKL, I could see that residency 
counters were increasing. So no need to go for timeout.
>
>> +}
>> +
>>   static void test_dc5(data_t *data)
>>   {
>>   	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
>> @@ -162,6 +174,7 @@ int main(int argc, char *argv[])
>>   {
>>   	data_t data = {};
>>   	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
>> +	uint32_t dc5_dc6_counter_before_suspend, dc5_dc6_counter_after_resume;
> It's not really suspend/resume and I don't think you need separate vars
> for each subtest.
ok , Will keep one set of variable with shorter name.
>
>>   
>>   	igt_skip_on_simulation();
>>   	igt_fixture {
>> @@ -193,6 +206,14 @@ int main(int argc, char *argv[])
>>   				"DC6 State is not achieved\n");
>>   		cleanup(&data);
>>   	}
>> +	igt_subtest("DC6_state_on_gen9") {
> This isn't GEN9 specific, so could be called dc6-dpms-off. I suppose you
> also want a dc5-dpms-off test.
AS on Gen9,  (I am sure about SKL, not sure about other Gen9 platforms), 
on SKL, DC6 is achieved with DPMS off. but on Gen 11, DC6 is achieved 
with PSR. That's why i named test like "Dc6_state_on_gen9" and 
"Dc6_state_on_gen11".
Also on Gen9, DC5 is mainly achieved with PSR, on DPMS off directly we 
can enter into DC6. So in case of DPMS off scenario, i was mainly 
checking Dc6 counter, instead of checking DC5 and Dc6 counters both.
>
>> +		dc5_dc6_counter_before_suspend = read_dc5_dc6_counter(data.devid);
>> +		dpms_off_on(&data);
>> +		dc5_dc6_counter_after_resume = read_dc5_dc6_counter(data.devid);
>> +		igt_require_f(dc5_dc6_counter_after_resume > dc5_dc6_counter_before_suspend,
>> +				"DC6 State is not achieved\n");
>> +		cleanup(&data);
>> +	}
>>   	igt_fixture {
>>   		psr_disable(data.debugfs_fd);
>>   		close(data.debugfs_fd);
>> -- 
>> 2.7.4
>>

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

Re: [igt-dev] [PATCH i-g-t v1 4/6] [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF sequence.

[Imre Deak]

On Wed, Oct 03, 2018 at 10:07:27AM +0530, Yadav, Jyoti R wrote:
> > > [...]
> > > @@ -193,6 +206,14 @@ int main(int argc, char *argv[])
> > >   				"DC6 State is not achieved\n");
> > >   		cleanup(&data);
> > >   	}
> > > +	igt_subtest("DC6_state_on_gen9") {
> > This isn't GEN9 specific, so could be called dc6-dpms-off. I suppose you
> > also want a dc5-dpms-off test.
>
> AS on Gen9,  (I am sure about SKL, not sure about other Gen9
> platforms), on SKL, DC6 is achieved with DPMS off. but on Gen 11, DC6
> is achieved with PSR.

It must be also achieved with DPMS off everywhere where DC6 is
available.

> That's why i named test like "Dc6_state_on_gen9" and
> "Dc6_state_on_gen11".

No need to have the platform name in the subtest name, let's just do the
tests everywhere where the corresponding DC counter is available and
skip it elsewhere.

> Also on Gen9, DC5 is mainly achieved with PSR, on DPMS off directly we
> can enter into DC6.

Still, it makes sense to have a separate test for DC5, so we have a
better clue what goes wrong in case of failures.

> So in case of DPMS off scenario, i was mainly checking Dc6 counter,
> instead of checking DC5 and Dc6 counters both.

I still think we should have the following subtests:

dc5-psr
dc6-psr
dc5-dpms-off
dc6-dpms-off

And run each of these where the counter is available, skip them
otherwise.

> > 
> > > +		dc5_dc6_counter_before_suspend = read_dc5_dc6_counter(data.devid);
> > > +		dpms_off_on(&data);
> > > +		dc5_dc6_counter_after_resume = read_dc5_dc6_counter(data.devid);
> > > +		igt_require_f(dc5_dc6_counter_after_resume > dc5_dc6_counter_before_suspend,
> > > +				"DC6 State is not achieved\n");
> > > +		cleanup(&data);
> > > +	}
> > >   	igt_fixture {
> > >   		psr_disable(data.debugfs_fd);
> > >   		close(data.debugfs_fd);
> > > -- 
> > > 2.7.4
> > > 
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.

[Petri Latvala]

On Wed, Sep 26, 2018 at 11:59:19AM -0400, Jyoti Yadav wrote:
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/pm_dc.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 180 insertions(+)
>  create mode 100644 tests/pm_dc.c
> 
> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> new file mode 100644
> index 0000000..db38ccf
> --- /dev/null
> +++ b/tests/pm_dc.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright © 2013 Intel Corporation

Surely this wasn't written in 2013?


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

Re: [igt-dev] [PATCH i-g-t v1 6/6] [intel-gfx] tests/Makefile : Added new file tests/pm_dc.c for compilation.

[Petri Latvala]

On Tue, Oct 02, 2018 at 04:38:01PM +0300, Imre Deak wrote:
> On Wed, Sep 26, 2018 at 11:59:23AM -0400, Jyoti Yadav wrote:
> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> > ---
> >  tests/Makefile.sources | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index c84933f..d491420 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -214,6 +214,7 @@ TESTS_progs = \
> >  	pm_lpsp \
> >  	pm_rc6_residency \
> >  	pm_rpm \
> > +	pm_dc \
> 
> tests/meson.build also needs to be updated.
> 
> CI didn't run the new subtests, not sure why, maybe it depends on
> meson.build?
> 

CI builds with meson, yes.

Please make this change in the same commit that adds the test.


-- 
Petri Latvala




> >  	pm_rps \
> >  	pm_sseu \
> >  	prime_busy \
> > -- 
> > 2.7.4
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.

[Dhinakaran Pandiyan]

On Tuesday, October 2, 2018 6:25:13 AM PDT Imre Deak wrote:
> On Wed, Sep 26, 2018 at 11:59:19AM -0400, Jyoti Yadav wrote:

Please fix your patch format, I don't see a commit message.

What is the reason to not check for DC6? From my understanding, DC6 is the 
target state for the single pipe eDP with PSR configuration. 

-DK

> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> > ---
> > 
> >  tests/pm_dc.c | 180
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> >  changed, 180 insertions(+)
> >  create mode 100644 tests/pm_dc.c
> > 
> > diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> > new file mode 100644
> > index 0000000..db38ccf
> > --- /dev/null
> > +++ b/tests/pm_dc.c
> > @@ -0,0 +1,180 @@
> > +/*
> > + * Copyright © 2013 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > a
> > + * copy of this software and associated documentation files (the
> > "Software"), + * to deal in the Software without restriction, including
> > without limitation + * the rights to use, copy, modify, merge, publish,
> > distribute, sublicense, + * and/or sell copies of the Software, and to
> > permit persons to whom the + * Software is furnished to do so, subject to
> > the following conditions: + *
> > + * The above copyright notice and this permission notice (including the
> > next + * paragraph) shall be included in all copies or substantial
> > portions of the + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND
> > NONINFRINGEMENT.  IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS
> > BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN
> > ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN
> > CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE
> > SOFTWARE.
> > + *
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_psr.h"
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include "intel_bufmgr.h"
> > +#include "intel_io.h"
> > +
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	uint32_t devid;
> > +	igt_display_t display;
> > +	struct igt_fb fb_white;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +} data_t;
> > +
> > +bool has_runtime_pm;
> > +
> > +static void setup_output(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe_with_valid_output(display, pipe, output) {
> > +		drmModeConnectorPtr c = output->config.connector;
> > +
> > +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > +			continue;
> > +
> > +		igt_output_set_pipe(output, pipe);
> > +		data->output = output;
> > +		data->mode = igt_output_get_mode(output);
> > +
> > +		return;
> > +	}
> > +}
> > +
> > +static void display_init(data_t *data)
> > +{
> > +	igt_display_init(&data->display, data->drm_fd);
> > +	setup_output(data);
> > +}
> > +
> > +static void display_fini(data_t *data)
> > +{
> > +	igt_display_fini(&data->display);
> > +}
> > +
> > +static bool sink_support(data_t *data)
> > +{
> > +	char buf[512];
> > +
> > +	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> > +			 buf, sizeof(buf));
> > +
> > +	return strstr(buf, "Sink_Support: yes\n");
> > +}
> > +
> > +static void cleanup(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_display_commit(&data->display);
> > +	igt_remove_fb(data->drm_fd, &data->fb_white);
> > +}
> > +
> > +static void setup_primary(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_create_color_fb(data->drm_fd,
> > +			    data->mode->hdisplay, data->mode->vdisplay,
> > +			    DRM_FORMAT_XRGB8888,
> > +			    LOCAL_I915_FORMAT_MOD_X_TILED,
> > +			    1.0, 1.0, 1.0,
> > +			    &data->fb_white);
> > +	igt_display_commit(&data->display);
> > +
> > +	igt_plane_set_fb(primary, &data->fb_white);
> > +	igt_display_commit(&data->display);
> > +}
> > +
> > +static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
> > +{
> > +	uint32_t dc3_dc5_count;
> > +
> > +	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
> > +		dc3_dc5_count = INREG(SKL_CSR_DC3_DC5_COUNT);
> > +	else if (IS_BROXTON(dev_id))
> > +		dc3_dc5_count = INREG(BXT_CSR_DC3_DC5_COUNT);
> 
> The counters are available already in the DMC debugfs file, so no need
> to duplicate the readout and platform check for that.
> 
> > +	else {
> > +		igt_warn("Currently DC5 support is for ICL, SKL and BXT Platform
> > (%s)\n", +			__func__);
> > +		return -EPERM;
> 
> We want to skip the test if the DC counters don't exist, so we'd need an
> igt_require() instead of the error return.
> 
> > +	}
> > +	return dc3_dc5_count;
> > +}
> > +
> > +static void test_dc5(data_t *data)
> > +{
> > +	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
> 
> You could use shorter names.
> 
> > +
> > +	dc3_dc5_counter_before_psr = read_dc3_dc5_counter(data->devid);
> > +	setup_primary(data);
> > +	igt_assert(psr_wait_entry(data->debugfs_fd));
> > +	dc3_dc5_counter_after_psr = read_dc3_dc5_counter(data->devid);
> 
> I wonder if this would always work or if we'd need to wait for the
> counter to increase with a timeout.
> 
> > +	igt_require_f(dc3_dc5_counter_after_psr > dc3_dc5_counter_before_psr,
> > +			"DC5 State is not achieved\n");
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	data_t data = {};
> > +
> > +	igt_skip_on_simulation();
> > +	igt_fixture {
> > +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > +		igt_require(data.debugfs_fd != -1);
> > +		kmstest_set_vt_graphics_mode();
> > +		data.devid = intel_get_drm_devid(data.drm_fd);
> > +		psr_enable(data.debugfs_fd);
> 
> psr_enable() should be called from the PSR specific subtest.
> 
> > +		has_runtime_pm = igt_setup_runtime_pm();
> > +		igt_info("Runtime PM support: %d\n", has_runtime_pm);
> > +		igt_require(has_runtime_pm);
> > +		igt_require(dmc_loaded(data.debugfs_fd));
> > +
> > +		igt_require_f(sink_support(&data),
> > +			      "Sink does not support PSR\n");
> > +		display_init(&data);
> > +	}
> > +
> > +	igt_subtest("DC5_state") {
> > +		test_dc5(&data);
> > +		cleanup(&data);
> > +	}
> > +	igt_fixture {
> > +		psr_disable(data.debugfs_fd);
> 
> We want to _restore_ the PSR state not disable it, and that's done
> already from an exit handler.
> 
> > +		close(data.debugfs_fd);
> > +		display_fini(&data);
> > +	}
> > +
> > +	igt_exit();
> > +}
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev




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

Re: [igt-dev] [PATCH i-g-t v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.

[Dhinakaran Pandiyan]

On Tuesday, October 2, 2018 6:25:13 AM PDT Imre Deak wrote:
> On Wed, Sep 26, 2018 at 11:59:19AM -0400, Jyoti Yadav wrote:
> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> > ---
> > 
> >  tests/pm_dc.c | 180
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> >  changed, 180 insertions(+)
> >  create mode 100644 tests/pm_dc.c
> > 
> > diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> > new file mode 100644
> > index 0000000..db38ccf
> > --- /dev/null
> > +++ b/tests/pm_dc.c
> > @@ -0,0 +1,180 @@
> > +/*
> > + * Copyright © 2013 Intel Corporation

This has to be updated?

> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > a
> > + * copy of this software and associated documentation files (the
> > "Software"), + * to deal in the Software without restriction, including
> > without limitation + * the rights to use, copy, modify, merge, publish,
> > distribute, sublicense, + * and/or sell copies of the Software, and to
> > permit persons to whom the + * Software is furnished to do so, subject to
> > the following conditions: + *
> > + * The above copyright notice and this permission notice (including the
> > next + * paragraph) shall be included in all copies or substantial
> > portions of the + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND
> > NONINFRINGEMENT.  IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS
> > BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN
> > ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN
> > CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE
> > SOFTWARE.
> > + *
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_psr.h"
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include "intel_bufmgr.h"
> > +#include "intel_io.h"
> > +
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	uint32_t devid;
> > +	igt_display_t display;
> > +	struct igt_fb fb_white;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +} data_t;
> > +
> > +bool has_runtime_pm;
> > +
> > +static void setup_output(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe_with_valid_output(display, pipe, output) {
> > +		drmModeConnectorPtr c = output->config.connector;
> > +
> > +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > +			continue;
> > +
> > +		igt_output_set_pipe(output, pipe);
> > +		data->output = output;
> > +		data->mode = igt_output_get_mode(output);
> > +
> > +		return;
> > +	}
> > +}
> > +
> > +static void display_init(data_t *data)
> > +{
> > +	igt_display_init(&data->display, data->drm_fd);
> > +	setup_output(data);
> > +}
> > +
> > +static void display_fini(data_t *data)
> > +{
> > +	igt_display_fini(&data->display);
> > +}
> > +
> > +static bool sink_support(data_t *data)
> > +{
> > +	char buf[512];
> > +
> > +	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> > +			 buf, sizeof(buf));
> > +
> > +	return strstr(buf, "Sink_Support: yes\n");
> > +}
> > +
> > +static void cleanup(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_display_commit(&data->display);
> > +	igt_remove_fb(data->drm_fd, &data->fb_white);
> > +}
> > +
> > +static void setup_primary(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_create_color_fb(data->drm_fd,
> > +			    data->mode->hdisplay, data->mode->vdisplay,
> > +			    DRM_FORMAT_XRGB8888,
> > +			    LOCAL_I915_FORMAT_MOD_X_TILED,
> > +			    1.0, 1.0, 1.0,
> > +			    &data->fb_white);
> > +	igt_display_commit(&data->display);
> > +
> > +	igt_plane_set_fb(primary, &data->fb_white);
> > +	igt_display_commit(&data->display);
> > +}
> > +
> > +static uint32_t read_dc3_dc5_counter(uint32_t dev_id)
> > +{
> > +	uint32_t dc3_dc5_count;
> > +
> > +	if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
> > +		dc3_dc5_count = INREG(SKL_CSR_DC3_DC5_COUNT);
> > +	else if (IS_BROXTON(dev_id))
> > +		dc3_dc5_count = INREG(BXT_CSR_DC3_DC5_COUNT);
> 
> The counters are available already in the DMC debugfs file, so no need
> to duplicate the readout and platform check for that.
> 
> > +	else {
> > +		igt_warn("Currently DC5 support is for ICL, SKL and BXT Platform
> > (%s)\n", +			__func__);
> > +		return -EPERM;
> 
> We want to skip the test if the DC counters don't exist, so we'd need an
> igt_require() instead of the error return.
> 
> > +	}
> > +	return dc3_dc5_count;
> > +}
> > +
> > +static void test_dc5(data_t *data)
> > +{
> > +	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
> 
> You could use shorter names.
> 
> > +
> > +	dc3_dc5_counter_before_psr = read_dc3_dc5_counter(data->devid);
> > +	setup_primary(data);
> > +	igt_assert(psr_wait_entry(data->debugfs_fd));
> > +	dc3_dc5_counter_after_psr = read_dc3_dc5_counter(data->devid);
> 
> I wonder if this would always work or if we'd need to wait for the
> counter to increase with a timeout.

PSR needs some time to become active (depends on the idle frame count) and DMC 
takes over after that, so you'll need to wait after entry. Around a second 
should be good,  I think.

> 
> > +	igt_require_f(dc3_dc5_counter_after_psr > dc3_dc5_counter_before_psr,
> > +			"DC5 State is not achieved\n");

This should be a failure.
igt_assert_f(after > before, " before %d after %d"...)

> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	data_t data = {};
> > +
> > +	igt_skip_on_simulation();
> > +	igt_fixture {
> > +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > +		igt_require(data.debugfs_fd != -1);
> > +		kmstest_set_vt_graphics_mode();
> > +		data.devid = intel_get_drm_devid(data.drm_fd);
> > +		psr_enable(data.debugfs_fd);
> 
> psr_enable() should be called from the PSR specific subtest.
> 
> > +		has_runtime_pm = igt_setup_runtime_pm();
> > +		igt_info("Runtime PM support: %d\n", has_runtime_pm);
> > +		igt_require(has_runtime_pm);
> > +		igt_require(dmc_loaded(data.debugfs_fd));
> > +
> > +		igt_require_f(sink_support(&data),
> > +			      "Sink does not support PSR\n");
> > +		display_init(&data);
> > +	}
> > +
> > +	igt_subtest("DC5_state") {
> > +		test_dc5(&data);
> > +		cleanup(&data);
> > +	}
> > +	igt_fixture {
> > +		psr_disable(data.debugfs_fd);
> 
> We want to _restore_ the PSR state not disable it, and that's done
> already from an exit handler.
> 
> > +		close(data.debugfs_fd);
> > +		display_fini(&data);
> > +	}
> > +
> > +	igt_exit();
> > +}
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev




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

Re: [igt-dev] [PATCH i-g-t v1 4/6] [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF sequence.

[Dhinakaran Pandiyan]

On Tuesday, October 2, 2018 9:37:27 PM PDT Yadav, Jyoti R wrote:
> On 10/2/2018 7:06 PM, Imre Deak wrote:
> > On Wed, Sep 26, 2018 at 11:59:21AM -0400, Jyoti Yadav wrote:
> >> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> >> ---
> >> 
> >>   tests/pm_dc.c | 21 +++++++++++++++++++++
> >>   1 file changed, 21 insertions(+)
> >> 
> >> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> >> index b609fad..0f18ece 100644
> >> --- a/tests/pm_dc.c
> >> +++ b/tests/pm_dc.c
> >> @@ -146,6 +146,18 @@ static uint32_t read_dc5_dc6_counter(uint32_t
> >> dev_id)
> >> 
> >>   	return dc5_dc6_count;
> >>   
> >>   }
> >> 
> >> +static void dpms_off_on(data_t *data)
> >> +{
> >> +	for (int i = 0; i < data->display.n_outputs; i++) {
> >> +		kmstest_set_connector_dpms(data->drm_fd,
> >> data->display.outputs[i].config.connector, +					   
DRM_MODE_DPMS_OFF);
> >> +	}
> >> +	for (int i = 0; i < data->display.n_outputs; i++) {
> >> +		kmstest_set_connector_dpms(data->drm_fd,
> >> data->display.outputs[i].config.connector, +				   
DRM_MODE_DPMS_ON);
> >> +	}
> > 
> > Again, not sure if it's guaranteed that the residency counter will
> > increase with instant off/on, probably we'd need to wait for the
> > increment with a timeout.
> 
> while doing some basic testing on SKL, I could see that residency
> counters were increasing. So no need to go for timeout.
> 
> >> +}
> >> +
> >> 
> >>   static void test_dc5(data_t *data)
> >>   {
> >>   
> >>   	uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
> >> 
> >> @@ -162,6 +174,7 @@ int main(int argc, char *argv[])
> >> 
> >>   {
> >>   
> >>   	data_t data = {};
> >>   	uint32_t dc5_dc6_counter_before_psr, dc5_dc6_counter_after_psr;
> >> 
> >> +	uint32_t dc5_dc6_counter_before_suspend, dc5_dc6_counter_after_resume;
> > 
> > It's not really suspend/resume and I don't think you need separate vars
> > for each subtest.
> 
> ok , Will keep one set of variable with shorter name.
> 
> >>   	igt_skip_on_simulation();
> >>   	igt_fixture {
> >> 
> >> @@ -193,6 +206,14 @@ int main(int argc, char *argv[])
> >> 
> >>   				"DC6 State is not achieved\n");
> >>   		
> >>   		cleanup(&data);
> >>   	
> >>   	}
> >> 
> >> +	igt_subtest("DC6_state_on_gen9") {
> > 
> > This isn't GEN9 specific, so could be called dc6-dpms-off. I suppose you
> > also want a dc5-dpms-off test.
> 
> AS on Gen9,  (I am sure about SKL, not sure about other Gen9 platforms),
> on SKL, DC6 is achieved with DPMS off. but on Gen 11, DC6 is achieved
> with PSR. That's why i named test like "Dc6_state_on_gen9" and
> "Dc6_state_on_gen11".
> Also on Gen9, DC5 is mainly achieved with PSR, on DPMS off directly we
We need to find out what's happening here, from my understanding PSR should be 
sufficient for DC6 on gen-9.

-DK

> can enter into DC6. So in case of DPMS off scenario, i was mainly
> checking Dc6 counter, instead of checking DC5 and Dc6 counters both.
> 
> >> +		dc5_dc6_counter_before_suspend = read_dc5_dc6_counter(data.devid);
> >> +		dpms_off_on(&data);
> >> +		dc5_dc6_counter_after_resume = read_dc5_dc6_counter(data.devid);
> >> +		igt_require_f(dc5_dc6_counter_after_resume >
> >> dc5_dc6_counter_before_suspend, +				"DC6 State is not achieved
\n");
> >> +		cleanup(&data);
> >> +	}
> >> 
> >>   	igt_fixture {
> >>   	
> >>   		psr_disable(data.debugfs_fd);
> >>   		close(data.debugfs_fd);
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev




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

Re: [igt-dev] [PATCH i-g-t v1 0/6] Added new test file pm_dc.c.

[Dhinakaran Pandiyan]

On Wednesday, September 26, 2018 8:59:17 AM PDT Jyoti Yadav wrote:
> This patch series adds new tests to validate Display C states.
> Mainly DC5 and Dc6 for various platforms like Icelake, Skylake,
> Broxton.

Another useful sub-test is to check vblank count has incremented across DC6 
entry-exit. We had to workaround the issue of frame counter getting zeroed in 
the driver, it'll be good to have a test to make sure it keeps working.

-DK


> 
> Jyoti Yadav (6):
>   [intel-gfx][igt-dev] lib/igt_pm : Moves Dmc_loaded() function into
>     library.      Because it will be used by new test pm_dc.c which will
>     validate Display C States.
>   [intel-gfx][igt-dev] tests/pm_dc : Added new test to verify Display C
>     States.     Currently this test validate DC5 upon PSR entry for
>     Icelake, Skylake and Broxton platform.
>   [intel-gfx][igt-dev] tests/pm_dc : Added test to validate DC6 state
>      on Gen11 platform. On Gen11 Platform, DC6 is achieved with PSR
>     entry.
>   [intel-gfx][intel-dev] tests/pm_dc : This patch adds test to validate
>        DC6 on SKL. DC6 on SKL is achieved with RPM enabled and DPMS OFF
>     sequence.
>   [intel-gfx][igt-dev] lib/intel_reg : This patch adds Display C States
>     related residency counters.
>   [intel-gfx][igt-dev] tests/Makefile : Added new file tests/pm_dc.c for
>     compilation.
> 
>  lib/igt_pm.c           |  29 +++++++
>  lib/igt_pm.h           |   1 +
>  lib/intel_reg.h        |   5 ++
>  tests/Makefile.sources |   1 +
>  tests/pm_dc.c          | 224
> +++++++++++++++++++++++++++++++++++++++++++++++++ tests/pm_rpm.c         | 
> 17 +---
>  6 files changed, 261 insertions(+), 16 deletions(-)
>  create mode 100644 tests/pm_dc.c




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