[igt-dev] [PATCH i-g-t 1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status

[igt-dev] [PATCH i-g-t 1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status

[José Roberto de Souza]

Sometimes 512 bytes is not enoght to store the whole
i915_edp_psr_status potentially causing fail in tests that depends
in the content that is out of this 512 bytes.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 0ddfb64f..c7bc523c 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -25,10 +25,12 @@
 #include "igt_sysfs.h"
 #include <errno.h>
 
+#define PSR_STATUS_MAX_LEN 1024
+
 static bool psr_active(int debugfs_fd, bool check_active)
 {
 	bool active;
-	char buf[512];
+	char buf[PSR_STATUS_MAX_LEN];
 
 	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				sizeof(buf));
-- 
2.19.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/8] tests: Share the code handling PSR debugfs parsing

[José Roberto de Souza]

The same code checking if sink supports PSR was spread into 3 tests,
better move it to lib and reuse.
Also kms_fbcon_fbt was doing its own handling to wait for PSR to get
enabled while it is already available in lib.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c                    |  9 +++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_fbcon_fbt.c            | 26 ++------------------------
 tests/kms_frontbuffer_tracking.c |  8 +-------
 tests/kms_psr.c                  |  8 +-------
 5 files changed, 14 insertions(+), 38 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index c7bc523c..d68f4b49 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -131,3 +131,12 @@ bool psr_disable(int debugfs_fd)
 {
 	return psr_set(debugfs_fd, false);
 }
+
+bool psr_supported(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+	return strstr(buf, "Sink_Support: yes\n");
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index b9693822..a4fcf325 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -32,5 +32,6 @@ bool psr_wait_entry(int debugfs_fd);
 bool psr_wait_exit(int debugfs_fd);
 bool psr_enable(int debugfs_fd);
 bool psr_disable(int debugfs_fd);
+bool psr_supported(int debugfs_fd);
 
 #endif
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 24d3ad90..32407709 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -168,19 +168,6 @@ static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
 	igt_assert_eq(rc, 0);
 }
 
-static bool psr_supported_on_chipset(int debugfs_fd)
-{
-	char buf[256];
-	int ret;
-
-	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
-				      buf, sizeof(buf));
-	if (ret < 0)
-		return false;
-
-	return strstr(buf, "Sink_Support: yes\n");
-}
-
 static bool connector_can_psr(drmModeConnectorPtr connector)
 {
 	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
@@ -195,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
 	igt_debug("PSR status: %s\n", buf);
 }
 
-static bool psr_is_enabled(int debugfs_fd)
-{
-	char buf[256];
-
-	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
-				sizeof(buf));
-	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
-}
-
 static bool psr_wait_until_enabled(int debugfs_fd)
 {
-	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
+	bool r = psr_wait_entry(debugfs_fd);
 
 	psr_print_status(debugfs_fd);
 	return r;
@@ -239,7 +217,7 @@ struct feature {
 	.connector_possible_fn = connector_can_fbc,
 	.enable = fbc_modparam_enable,
 }, psr = {
-	.supported_on_chipset = psr_supported_on_chipset,
+	.supported_on_chipset = psr_supported,
 	.wait_until_enabled = psr_wait_until_enabled,
 	.connector_possible_fn = connector_can_psr,
 	.enable = psr_debugfs_enable,
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 276ef83c..c419557e 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1419,13 +1419,7 @@ static void teardown_fbc(void)
 
 static bool psr_sink_has_support(void)
 {
-	char buf[256];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
-		return false;
-
-	return strstr(buf, "Sink_Support: yes\n");
+	return psr_supported(drm.debugfs);
 }
 
 static void setup_psr(void)
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index d00e552f..116fe409 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -191,13 +191,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 
 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 data->with_psr_disabled ||
-		strstr(buf, "Sink_Support: yes\n");
+	return data->with_psr_disabled || psr_supported(data->debugfs_fd);
 }
 
 static bool psr_wait_entry_if_enabled(data_t *data)
-- 
2.19.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/8] lib/psr: Add support to new modified i915_edp_psr_status output

[José Roberto de Souza]

The kernel patch 'drm/i915: Refactor PSR status debugfs' changed the
output of i915_edp_psr_status, so adding support to the new output
here while keeping the support to the old one for a while.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index d68f4b49..eecee459 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -34,8 +34,10 @@ static bool psr_active(int debugfs_fd, bool check_active)
 
 	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				sizeof(buf));
-	active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
-		(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
+
+	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
+		 strstr(buf, "Source PSR ctl: enabled");
+	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
 	return check_active ? active : !active;
 }
 
@@ -138,5 +140,6 @@ bool psr_supported(int debugfs_fd)
 
 	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				sizeof(buf));
-	return strstr(buf, "Sink_Support: yes\n");
+	return strstr(buf, "Sink_Support: yes\n") ||
+	       strstr(buf, "Sink support: yes");
 }
-- 
2.19.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/8] lib/psr: Make psr_active() only cares about PSR1

[José Roberto de Souza]

PSR2 will have it own function to detect if is active so we can drop
the sleep state search and also to make sure that PSR1 is active lets
search for "PSR1 enabled".

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index eecee459..c6d6390f 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -37,7 +37,8 @@ static bool psr_active(int debugfs_fd, bool check_active)
 
 	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
 		 strstr(buf, "Source PSR ctl: enabled");
-	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
+	active &= !!strstr(buf, "SRDENT");
+	active &= !!strstr(buf, "Status: PSR1 enabled");
 	return check_active ? active : !active;
 }
 
-- 
2.19.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/8] lib/psr: Add PSR2 functions

[José Roberto de Souza]

Add the same kind of functions that we have for PSR1 but for PSR2 to
make easy write tests.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_psr.h |  7 ++++
 2 files changed, 106 insertions(+)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index c6d6390f..6232a77f 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -144,3 +144,102 @@ bool psr_supported(int debugfs_fd)
 	return strstr(buf, "Sink_Support: yes\n") ||
 	       strstr(buf, "Sink support: yes");
 }
+
+static bool psr2_set(int debugfs_fd, bool enable)
+{
+	int ret;
+
+	ret = has_psr_debugfs(debugfs_fd);
+	/* Test PSR2 requires newer kernel that supports PSR debugfs api */
+	igt_require(ret == 0);
+
+	ret = psr_write(debugfs_fd, enable ? "0x2" : "0x1");
+	igt_assert(ret > 0);
+
+	/* Restore original value on exit */
+	if (psr_restore_debugfs_fd == -1) {
+		psr_restore_debugfs_fd = dup(debugfs_fd);
+		igt_assert(psr_restore_debugfs_fd >= 0);
+		igt_install_exit_handler(restore_psr_debugfs);
+	}
+
+	return ret;
+}
+
+bool psr2_enable(int debugfs_fd)
+{
+	return psr2_set(debugfs_fd, true);
+}
+
+bool psr2_disable(int debugfs_fd)
+{
+	return psr2_set(debugfs_fd, false);
+}
+
+bool psr2_supported(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+	return strstr(buf, "Sink support: yes [0x00000003]");
+}
+
+static bool psr2_in_deep_sleep(int debugfs_fd)
+{
+	bool active;
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+
+	active = strstr(buf, "Status: PSR2 enabled") &&
+		 strstr(buf, "Source PSR ctl: enabled") &&
+		 strstr(buf, "Source PSR status: DEEP_SLEEP");
+
+	return active;
+}
+
+static bool psr2_disabled(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+
+	return strstr(buf, "Status: disabled");
+}
+
+bool psr2_wait_deep_sleep(int debugfs_fd)
+{
+	/*
+	 * DEEP_SLEEP is only achieved after 15 idle frames so the timeout
+	 * needs to be this long to avoid test fail in 30hz modesets
+	 */
+	return igt_wait(psr2_in_deep_sleep(debugfs_fd), 1000, 20);
+}
+
+bool psr2_wait_disable(int debugfs_fd)
+{
+	return igt_wait(psr2_disabled(debugfs_fd), 40, 10);
+}
+
+static bool psr2_status_update(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+			        sizeof(buf));
+
+	/*
+	 * As it waits for DEEP_SLEEP state when enabling PSR2 any state
+	 * different than it means that PSR2 hardware is working in update
+	 * the sink
+	 */
+	return !strstr(buf, "Source PSR status: DEEP_SLEEP");
+}
+
+bool psr2_wait_update(int debugfs_fd)
+{
+	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index a4fcf325..cabe8687 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -34,4 +34,11 @@ bool psr_enable(int debugfs_fd);
 bool psr_disable(int debugfs_fd);
 bool psr_supported(int debugfs_fd);
 
+bool psr2_wait_deep_sleep(int debugfs_fd);
+bool psr2_wait_update(int debugfs_fd);
+bool psr2_wait_disable(int debugfs_fd);
+bool psr2_enable(int debugfs_fd);
+bool psr2_disable(int debugfs_fd);
+bool psr2_supported(int debugfs_fd);
+
 #endif
-- 
2.19.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/8] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

[José Roberto de Souza]

The main tests for PSR1 check if hardware tracking is detecting
changes in planes when modifing it in different ways and now
those tests will also run for PSR2 if supported by source and sink.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_psr.c | 118 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 111 insertions(+), 7 deletions(-)

diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 116fe409..d87b8ea1 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -60,6 +60,7 @@ typedef struct {
 	int drm_fd;
 	int debugfs_fd;
 	enum operations op;
+	bool op_psr2;
 	uint32_t devid;
 	uint32_t crtc_id;
 	igt_display_t display;
@@ -71,6 +72,8 @@ typedef struct {
 	drmModeModeInfo *mode;
 	igt_output_t *output;
 	bool with_psr_disabled;
+	bool with_psr2_disabled;
+	bool supports_psr2;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -194,12 +197,22 @@ static bool sink_support(data_t *data)
 	return data->with_psr_disabled || psr_supported(data->debugfs_fd);
 }
 
-static bool psr_wait_entry_if_enabled(data_t *data)
+static bool sink_support_psr2(data_t *data)
 {
-	if (data->with_psr_disabled)
-		return true;
+	return data->with_psr2_disabled || psr2_supported(data->debugfs_fd);
+}
 
-	return psr_wait_entry(data->debugfs_fd);
+static bool psr_wait_entry_if_enabled(data_t *data)
+{
+	if (!data->op_psr2) {
+		if (data->with_psr_disabled)
+			return true;
+		return psr_wait_entry(data->debugfs_fd);
+	} else {
+		if (data->with_psr2_disabled)
+			return true;
+		return psr2_wait_deep_sleep(data->debugfs_fd);
+	}
 }
 
 static inline void manual(const char *expected)
@@ -289,11 +302,16 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
-	igt_assert(psr_wait_exit(data->debugfs_fd));
+
+	if (!data->op_psr2)
+		igt_assert(psr_wait_exit(data->debugfs_fd));
+	else
+		igt_assert(psr2_wait_update(data->debugfs_fd));
 	manual(expected);
 }
 
-static void test_cleanup(data_t *data) {
+static void test_cleanup(data_t *data)
+{
 	igt_plane_t *primary;
 
 	primary = igt_output_get_plane_type(data->output,
@@ -304,6 +322,11 @@ static void test_cleanup(data_t *data) {
 
 	igt_remove_fb(data->drm_fd, &data->fb_green);
 	igt_remove_fb(data->drm_fd, &data->fb_white);
+
+	/* switch to PSR1 again */
+	if (data->op_psr2 && !data->with_psr_disabled)
+		psr_enable(data->debugfs_fd);
+	data->op_psr2 = false;
 }
 
 static void setup_test_plane(data_t *data, int test_plane)
@@ -311,6 +334,11 @@ static void setup_test_plane(data_t *data, int test_plane)
 	uint32_t white_h, white_v;
 	igt_plane_t *primary, *sprite, *cursor;
 
+	if (data->op_psr2 && !data->with_psr_disabled) {
+		igt_require(data->supports_psr2);//op_psr2 is set in case this fails?
+		psr2_enable(data->debugfs_fd);
+	}
+
 	igt_create_color_fb(data->drm_fd,
 			    data->mode->hdisplay, data->mode->vdisplay,
 			    DRM_FORMAT_XRGB8888,
@@ -381,6 +409,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
 	case 'n':
 		data->with_psr_disabled = true;
 		break;
+	case 'm':
+		data->with_psr2_disabled = true;
+		break;
 	default:
 		igt_assert(0);
 	}
@@ -391,9 +422,11 @@ static int opt_handler(int opt, int opt_index, void *_data)
 int main(int argc, char *argv[])
 {
 	const char *help_str =
-	       "  --no-psr\tRun test without PSR.";
+	       "  --no-psr\tRun test without PSR.\n"
+	       "  --no-psr2\tRun test without PSR2.";
 	static struct option long_options[] = {
 		{"no-psr", 0, 0, 'n'},
+		{"no-psr2", 0, 0, 'm'},
 		{ 0, 0, 0, 0 }
 	};
 	data_t data = {};
@@ -414,6 +447,7 @@ int main(int argc, char *argv[])
 
 		igt_require_f(sink_support(&data),
 			      "Sink does not support PSR\n");
+		data.supports_psr2 = sink_support_psr2(&data);
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
 		igt_assert(data.bufmgr);
@@ -428,6 +462,13 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest("psr2_basic") {
+		data.op_psr2 = true;
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
+		igt_assert(psr_wait_entry_if_enabled(&data));
+		test_cleanup(&data);
+	}
+
 	igt_subtest("no_drrs") {
 		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(psr_wait_entry_if_enabled(&data));
@@ -435,6 +476,14 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest("psr2_no_drrs") {
+		data.op_psr2 = true;
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
+		igt_assert(psr_wait_entry_if_enabled(&data));
+		igt_assert(drrs_disabled(&data));
+		test_cleanup(&data);
+	}
+
 	for (op = PAGE_FLIP; op <= RENDER; op++) {
 		igt_subtest_f("primary_%s", op_str(op)) {
 			data.op = op;
@@ -445,6 +494,17 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	for (op = PAGE_FLIP; op <= RENDER; op++) {
+		igt_subtest_f("psr2_primary_%s", op_str(op)) {
+			data.op_psr2 = true;
+			data.op = op;
+			setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
+			igt_assert(psr_wait_entry_if_enabled(&data));
+			run_test(&data);
+			test_cleanup(&data);
+		}
+	}
+
 	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
 		igt_subtest_f("sprite_%s", op_str(op)) {
 			data.op = op;
@@ -455,6 +515,17 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
+		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
+			data.op_psr2 = true;
+			data.op = op;
+			setup_test_plane(&data, DRM_PLANE_TYPE_OVERLAY);
+			igt_assert(psr_wait_entry_if_enabled(&data));
+			run_test(&data);
+			test_cleanup(&data);
+		}
+	}
+
 	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
 		igt_subtest_f("cursor_%s", op_str(op)) {
 			data.op = op;
@@ -465,6 +536,17 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
+		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
+			data.op_psr2 = true;
+			data.op = op;
+			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
+			igt_assert(psr_wait_entry_if_enabled(&data));
+			run_test(&data);
+			test_cleanup(&data);
+		}
+	}
+
 	igt_subtest_f("dpms") {
 		data.op = RENDER;
 		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
@@ -474,6 +556,16 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest_f("psr2_dpms") {
+		data.op_psr2 = true;
+		data.op = RENDER;
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
+		igt_assert(psr_wait_entry_if_enabled(&data));
+		dpms_off_on(&data);
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
 	igt_subtest_f("suspend") {
 		data.op = PLANE_ONOFF;
 		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
@@ -485,6 +577,18 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest_f("psr2_suspend") {
+		data.op_psr2 = true;
+		data.op = PLANE_ONOFF;
+		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
+		igt_assert(psr_wait_entry_if_enabled(&data));
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_NONE);
+		igt_assert(psr_wait_entry_if_enabled(&data));
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
 	igt_fixture {
 		if (!data.with_psr_disabled)
 			psr_disable(data.debugfs_fd);
-- 
2.19.2

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

[igt-dev] [PATCH i-g-t 7/8] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[José Roberto de Souza]

Lets run the same PSR1 basic tests for PSR2 to get PSR2 regressions
faster.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index 6d42792c..2f61d38d 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -228,6 +228,10 @@ igt@kms_psr@primary_page_flip
 igt@kms_psr@cursor_plane_move
 igt@kms_psr@sprite_plane_onoff
 igt@kms_psr@primary_mmap_gtt
+igt@kms_psr@psr2_primary_page_flip
+igt@kms_psr@psr2_cursor_plane_move
+igt@kms_psr@psr2_sprite_plane_onoff
+igt@kms_psr@psr2_primary_mmap_gtt
 igt@kms_setmode@basic-clone-single-crtc
 igt@pm_backlight@basic-brightness
 igt@pm_rpm@basic-pci-d3-state
-- 
2.19.2

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

[igt-dev] [PATCH i-g-t 8/8] test: Add PSR2 selective update tests

[José Roberto de Souza]

This tests checks if hardware is able to do selective update when
screen changes.
PSR2 don't trigger interruptions and the 'PSR2 SU status' register
is not kept loaded all the times, so it is necessary keep polling
PSR status debugfs until those values are loaded.
When loaded it is compared with the expected value, if matches
the polling is stopped and the test passed otherwise it will
wait until fail_timerfd to expire to fail the test.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c          |  22 +++
 lib/igt_psr.h          |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_psr2_su.c    | 363 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 5 files changed, 388 insertions(+)
 create mode 100644 tests/kms_psr2_su.c

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 6232a77f..7ed2159c 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -27,6 +27,8 @@
 
 #define PSR_STATUS_MAX_LEN 1024
 
+#define EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_IN_FRAME_MASK(i) (0x3FF << ((i) * 10))
+
 static bool psr_active(int debugfs_fd, bool check_active)
 {
 	bool active;
@@ -243,3 +245,23 @@ bool psr2_wait_update(int debugfs_fd)
 {
 	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
 }
+
+bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t *num_su_blocks)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	char *str;
+	uint32_t val;
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+			        sizeof(buf));
+	str = strstr(buf, "PSR2 SU status: 0x");
+	if (!str)
+		return false;
+
+	str = &str[strlen("PSR2 SU status: 0x")];
+	str[8] = 0;
+	val = (uint32_t)strtol(str, NULL, 16);
+	*num_su_blocks = val & EDP_PSR2_SU_STATUS_NUM_SU_BLOCKS_IN_FRAME_MASK(0);
+
+	return true;
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index cabe8687..60b635de 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -40,5 +40,6 @@ bool psr2_wait_disable(int debugfs_fd);
 bool psr2_enable(int debugfs_fd);
 bool psr2_disable(int debugfs_fd);
 bool psr2_supported(int debugfs_fd);
+bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t *num_su_blocks);
 
 #endif
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index eedde1e8..3c79eec0 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -78,6 +78,7 @@ TESTS_progs = \
 	kms_plane_scaling \
 	kms_properties \
 	kms_psr \
+	kms_psr2_su \
 	kms_pwrite_crc \
 	kms_rmfb \
 	kms_rotation_crc \
diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
new file mode 100644
index 00000000..0e08b28e
--- /dev/null
+++ b/tests/kms_psr2_su.c
@@ -0,0 +1,363 @@
+/*
+ * Copyright © 2018 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 <poll.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/timerfd.h>
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test PSR2 selective update");
+
+#define SQUARE_SIZE 100
+
+enum operations {
+	PAGE_FLIP,
+	FRONTBUFFER,
+	LAST
+};
+
+static const char *op_str(enum operations op)
+{
+	static const char * const name[] = {
+		[PAGE_FLIP] = "page_flip",
+		[FRONTBUFFER] = "frontbuffer"
+	};
+
+	return name[op];
+}
+
+typedef struct {
+	struct igt_fb fb[2];
+	struct pollfd pollfds[2];
+	pthread_t thread_id;
+	int drm_fd;
+	int debugfs_fd;
+	int flip_timerfd;
+	int fail_timerfd;
+	uint32_t changes;
+	volatile bool run;
+	volatile bool sucess;
+
+	enum operations op;
+	uint32_t devid;
+	uint32_t crtc_id;
+	igt_display_t display;
+	drm_intel_bufmgr *bufmgr;
+	int mod_size;
+	int mod_stride;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+} data_t;
+
+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->crtc_id = output->config.crtc->crtc_id;
+		data->output = output;
+		data->mode = igt_output_get_mode(output);
+
+		return;
+	}
+}
+
+static void display_init(data_t *data)
+{
+	igt_display_require(&data->display, data->drm_fd);
+	setup_output(data);
+}
+
+static void display_fini(data_t *data)
+{
+	igt_display_fini(&data->display);
+}
+
+static void *debugfs_thread(void *ptr)
+{
+	data_t *data = ptr;
+	uint16_t expected_num_su_blocks;
+
+	/* each selective update block is 4 line tall */
+	expected_num_su_blocks = SQUARE_SIZE / 4;
+	expected_num_su_blocks += SQUARE_SIZE % 4 ? 1 : 0;
+
+	while (data->run) {
+		uint16_t num_su_blocks;
+		bool r;
+
+		r = psr2_read_last_num_su_blocks_val(data->debugfs_fd,
+						     &num_su_blocks);
+		if (r && num_su_blocks == expected_num_su_blocks) {
+			data->run = false;
+			data->sucess = true;
+			break;
+		}
+
+		usleep(1);
+	}
+
+	return NULL;
+}
+
+static void prepare(data_t *data)
+{
+	struct itimerspec interval;
+	igt_plane_t *primary;
+	int r;
+
+	/* all green frame */
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_DRM_FORMAT_MOD_NONE,
+			    0.0, 1.0, 0.0,
+			    &data->fb[0]);
+
+	if (data->op == PAGE_FLIP) {
+		cairo_t *cr;
+
+		igt_create_color_fb(data->drm_fd,
+				    data->mode->hdisplay, data->mode->vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    LOCAL_DRM_FORMAT_MOD_NONE,
+				    0.0, 1.0, 0.0,
+				    &data->fb[1]);
+
+		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
+		/* a white square */
+		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE, SQUARE_SIZE,
+				      1.0, 1.0, 1.0, 1.0);
+		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
+	}
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+
+	igt_display_commit(&data->display);
+	igt_plane_set_fb(primary, &data->fb[0]);
+	igt_display_commit(&data->display);
+
+	igt_assert(psr2_wait_deep_sleep(data->debugfs_fd));
+
+	data->run = true;
+	data->sucess = false;
+	data->changes = 0;
+
+	r = pthread_create(&data->thread_id, NULL, debugfs_thread, data);
+	if (r)
+		igt_warn("Error starting thread: %i\n", r);
+
+	interval.it_value.tv_nsec = 0;
+	interval.it_value.tv_sec = 3;
+	interval.it_interval.tv_nsec = interval.it_value.tv_nsec;
+	interval.it_interval.tv_sec = interval.it_value.tv_sec;
+	r = timerfd_settime(data->fail_timerfd, 0, &interval, NULL);
+	igt_require_f(r != -1, "Error setting timerfd\n");
+}
+
+static void update_screen(data_t *data)
+{
+	data->changes++;
+
+	switch (data->op) {
+	case PAGE_FLIP: {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane_type(data->output,
+						    DRM_PLANE_TYPE_PRIMARY);
+
+		igt_plane_set_fb(primary, &data->fb[data->changes & 1]);
+		igt_display_commit(&data->display);
+		break;
+	}
+	case FRONTBUFFER: {
+		drmModeClip clip;
+		cairo_t *cr;
+		int r;
+
+		clip.x1 = clip.y1 = 0;
+		clip.x2 = clip.y2 = SQUARE_SIZE;
+
+		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
+
+		if (data->changes & 1) {
+			/* go back to all green frame with with square */
+			igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
+		} else {
+			/* go back to all green frame */
+			igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 0, 1.0, 0, 1.0);
+		}
+
+		r = drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id, &clip, 1);
+		igt_assert(r == 0 || r == -ENOSYS);
+		break;
+	}
+	default:
+		igt_assert_f(data->op, "Operation not handled\n");
+	}
+}
+
+static void run(data_t *data)
+{
+	while (data->run) {
+		uint64_t exp;
+		int r;
+
+		r = poll(data->pollfds,
+			 sizeof(data->pollfds) / sizeof(data->pollfds[0]), -1);
+		if (r < 0)
+			break;
+
+		/* flip_timerfd timeout */
+		if (data->pollfds[0].revents & POLLIN) {
+			r = read(data->pollfds[0].fd, &exp, sizeof(exp));
+
+			if (r != sizeof(uint64_t)) {
+				igt_warn("read a not expected number of bytes from flip_timerfd: %i\n", r);
+			} else if (exp)
+				update_screen(data);
+		}
+
+		/* fail_timerfd timeout */
+		if (data->pollfds[1].revents & POLLIN) {
+			r = read(data->pollfds[1].fd, &exp, sizeof(exp));
+
+			if (r != sizeof(uint64_t)) {
+				igt_warn("read a not expected number of bytes from fail_timerfd: %i\n", r);
+			} else if (exp)
+				break;
+		}
+	}
+
+	data->run = false;
+	pthread_join(data->thread_id, NULL);
+
+	igt_debug("Changes: %u\n", data->changes);
+	igt_assert(data->sucess);
+}
+
+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[0]);
+	if (data->op == PAGE_FLIP)
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+}
+
+int main(int argc, char *argv[])
+{
+	data_t data = {};
+	enum operations op;
+
+	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
+				    NULL, NULL, NULL);
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		struct itimerspec interval;
+		int r;
+
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+		kmstest_set_vt_graphics_mode();
+		data.devid = intel_get_drm_devid(data.drm_fd);
+
+		igt_require_f(psr2_supported(data.debugfs_fd),
+			      "Sink does not support PSR2\n");
+
+		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
+		igt_assert(data.bufmgr);
+		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
+
+		display_init(&data);
+
+		igt_require(psr2_supported(data.debugfs_fd));
+		igt_require(psr2_enable(data.debugfs_fd));
+		igt_require(psr2_wait_deep_sleep(data.debugfs_fd));
+
+		data.flip_timerfd = timerfd_create(CLOCK_MONOTONIC,
+						   TFD_NONBLOCK);
+		igt_require(data.flip_timerfd != -1);
+		interval.it_value.tv_nsec = NSEC_PER_SEC / 15;
+		interval.it_value.tv_sec = 0;
+		interval.it_interval.tv_nsec = interval.it_value.tv_nsec;
+		interval.it_interval.tv_sec = interval.it_value.tv_sec;
+		r = timerfd_settime(data.flip_timerfd, 0, &interval, NULL);
+		igt_require_f(r != -1, "Error setting timerfd\n");
+
+		data.fail_timerfd = timerfd_create(CLOCK_MONOTONIC,
+						   TFD_NONBLOCK);
+		igt_require(data.fail_timerfd != -1);
+
+		data.pollfds[0].fd = data.flip_timerfd;
+		data.pollfds[0].events = POLLIN;
+		data.pollfds[0].revents = 0;
+
+		data.pollfds[1].fd = data.fail_timerfd;
+		data.pollfds[1].events = POLLIN;
+		data.pollfds[1].revents = 0;
+	}
+
+	for (op = PAGE_FLIP; op < LAST; op++) {
+		igt_subtest_f("%s", op_str(op)) {
+			data.op = op;
+			prepare(&data);
+			run(&data);
+			cleanup(&data);
+		}
+	}
+
+	igt_fixture {
+		close(data.debugfs_fd);
+		drm_intel_bufmgr_destroy(data.bufmgr);
+		display_fini(&data);
+	}
+
+	igt_exit();
+}
diff --git a/tests/meson.build b/tests/meson.build
index b8a6e61b..c557333c 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -49,6 +49,7 @@ test_progs = [
 	'kms_plane_scaling',
 	'kms_properties',
 	'kms_psr',
+	'kms_psr2_su',
 	'kms_pwrite_crc',
 	'kms_rmfb',
 	'kms_rotation_crc',
-- 
2.19.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 3/8] lib/psr: Add support to new modified i915_edp_psr_status output

[Souza, Jose]

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

kernel patches changing the debugfs can be found here:

https://patchwork.freedesktop.org/series/53510/

On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> The kernel patch 'drm/i915: Refactor PSR status debugfs' changed the
> output of i915_edp_psr_status, so adding support to the new output
> here while keeping the support to the old one for a while.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index d68f4b49..eecee459 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -34,8 +34,10 @@ static bool psr_active(int debugfs_fd, bool
> check_active)
>  
>  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
>  				sizeof(buf));
> -	active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> -		(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> +
> +	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
> +		 strstr(buf, "Source PSR ctl: enabled");
> +	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
>  	return check_active ? active : !active;
>  }
>  
> @@ -138,5 +140,6 @@ bool psr_supported(int debugfs_fd)
>  
>  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
>  				sizeof(buf));
> -	return strstr(buf, "Sink_Support: yes\n");
> +	return strstr(buf, "Sink_Support: yes\n") ||
> +	       strstr(buf, "Sink support: yes");
>  }

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status
URL   : https://patchwork.freedesktop.org/series/53511/
State : success

== Summary ==

CI Bug Log - changes from IGT_4741 -> IGTPW_2120
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - {fi-kbl-7567u}:     SKIP -> PASS +33

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_psr@cursor_plane_move:
    - fi-kbl-r:           PASS -> FAIL [fdo#107383] +3
    - fi-whl-u:           PASS -> FAIL [fdo#107383] +3
    - {fi-icl-u3}:        PASS -> FAIL [fdo#107383] +3

  * igt@kms_psr@primary_mmap_gtt:
    - fi-kbl-7560u:       PASS -> FAIL [fdo#107383] +3

  * igt@kms_psr@primary_page_flip:
    - fi-skl-6600u:       PASS -> FAIL [fdo#107383] +3

  * igt@kms_psr@sprite_plane_onoff:
    - fi-skl-6700hq:      PASS -> FAIL [fdo#107383] +3

  * {igt@runner@aborted}:
    - {fi-icl-y}:         NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-kbl-7567u}:     DMESG-WARN [fdo#105602] / [fdo#108529] -> PASS +1

  * igt@i915_selftest@live_coherency:
    - fi-gdg-551:         DMESG-FAIL [fdo#107164] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS

  * igt@pm_rpm@module-reload:
    - {fi-kbl-7567u}:     DMESG-WARN [fdo#108529] -> PASS

  
#### Warnings ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - {fi-kbl-7567u}:     DMESG-FAIL [fdo#105079] -> DMESG-WARN [fdo#108473]

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

  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107164]: https://bugs.freedesktop.org/show_bug.cgi?id=107164
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744


Participating hosts (49 -> 45)
------------------------------

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


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

    * IGT: IGT_4741 -> IGTPW_2120

  CI_DRM_5263: 823664600f1dc6b351612283cd13836b0d768251 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2120: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2120/
  IGT_4741: 5c8f89f67c7b32014bc22421e48f3c0cf4e5ca3a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_psr2_su@frontbuffer
+igt@kms_psr2_su@page_flip
+igt@kms_psr@psr2_basic
+igt@kms_psr@psr2_cursor_blt
+igt@kms_psr@psr2_cursor_mmap_cpu
+igt@kms_psr@psr2_cursor_mmap_gtt
+igt@kms_psr@psr2_cursor_plane_move
+igt@kms_psr@psr2_cursor_plane_onoff
+igt@kms_psr@psr2_cursor_render
+igt@kms_psr@psr2_dpms
+igt@kms_psr@psr2_no_drrs
+igt@kms_psr@psr2_primary_blt
+igt@kms_psr@psr2_primary_mmap_cpu
+igt@kms_psr@psr2_primary_mmap_gtt
+igt@kms_psr@psr2_primary_page_flip
+igt@kms_psr@psr2_primary_render
+igt@kms_psr@psr2_sprite_blt
+igt@kms_psr@psr2_sprite_mmap_cpu
+igt@kms_psr@psr2_sprite_mmap_gtt
+igt@kms_psr@psr2_sprite_plane_move
+igt@kms_psr@psr2_sprite_plane_onoff
+igt@kms_psr@psr2_sprite_render
+igt@kms_psr@psr2_suspend

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status
URL   : https://patchwork.freedesktop.org/series/53511/
State : success

== Summary ==

CI Bug Log - changes from IGT_4741_full -> IGTPW_2120_full
====================================================

Summary
-------

  **WARNING**

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

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

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

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

### IGT changes ###

#### Warnings ####

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          PASS -> SKIP
    - shard-snb:          SKIP -> PASS

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fence@basic-await-default:
    - shard-hsw:          PASS -> FAIL [fdo#108888]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-glk:          PASS -> FAIL [fdo#103232] +4
    - shard-apl:          PASS -> FAIL [fdo#103232] +7

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-kbl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +4

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2
    - shard-kbl:          PASS -> FAIL [fdo#103166] +2

  * {igt@kms_rotation_crc@multiplane-rotation-cropping-top}:
    - shard-glk:          PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS +1
    - shard-kbl:          FAIL [fdo#107725] / [fdo#108145] -> PASS

  * igt@kms_color@pipe-a-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1
    - shard-glk:          FAIL [fdo#103232] -> PASS +4

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS +4

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * {igt@kms_rotation_crc@multiplane-rotation-cropping-top}:
    - shard-kbl:          DMESG-FAIL -> PASS

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

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108888]: https://bugs.freedesktop.org/show_bug.cgi?id=108888
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


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

    * IGT: IGT_4741 -> IGTPW_2120

  CI_DRM_5263: 823664600f1dc6b351612283cd13836b0d768251 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2120: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2120/
  IGT_4741: 5c8f89f67c7b32014bc22421e48f3c0cf4e5ca3a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 1/8] lib/psr: Increase the buffer lenght that stores the output of i915_edp_psr_status

[Dhinakaran Pandiyan]

On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> Sometimes 512 bytes is not enoght to store the whole
                             ^enough
Not enough because of SU block counts?

> i915_edp_psr_status potentially causing fail in tests that depends
> in the content that is out of this 512 bytes.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 0ddfb64f..c7bc523c 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -25,10 +25,12 @@
>  #include "igt_sysfs.h"
>  #include <errno.h>
>  
> +#define PSR_STATUS_MAX_LEN 1024
Move this to igt_psr.h so that we can use the same size in kms_psr.c.

> +
>  static bool psr_active(int debugfs_fd, bool check_active)
>  {
>  	bool active;
> -	char buf[512];
> +	char buf[PSR_STATUS_MAX_LEN];
>  
>  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
>  				sizeof(buf));

_______________________________________________
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/8] tests: Share the code handling PSR debugfs parsing

[Dhinakaran Pandiyan]

On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> The same code checking if sink supports PSR was spread into 3 tests,
> better move it to lib and reuse.
Agreed.

> Also kms_fbcon_fbt was doing its own handling to wait for PSR to get
> enabled while it is already available in lib.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c                    |  9 +++++++++
>  lib/igt_psr.h                    |  1 +
>  tests/kms_fbcon_fbt.c            | 26 ++------------------------
>  tests/kms_frontbuffer_tracking.c |  8 +-------
>  tests/kms_psr.c                  |  8 +-------
>  5 files changed, 14 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index c7bc523c..d68f4b49 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -131,3 +131,12 @@ bool psr_disable(int debugfs_fd)
>  {
>  	return psr_set(debugfs_fd, false);
>  }
> +
> +bool psr_supported(int debugfs_fd)
psr_sink_support() in order to be descriptive about what support we are
referring to.

> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +	return strstr(buf, "Sink_Support: yes\n");
> +}
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index b9693822..a4fcf325 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -32,5 +32,6 @@ bool psr_wait_entry(int debugfs_fd);
>  bool psr_wait_exit(int debugfs_fd);
>  bool psr_enable(int debugfs_fd);
>  bool psr_disable(int debugfs_fd);
> +bool psr_supported(int debugfs_fd);
>  
>  #endif
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 24d3ad90..32407709 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -168,19 +168,6 @@ static void set_mode_for_one_screen(struct
> drm_info *drm, struct igt_fb *fb,
>  	igt_assert_eq(rc, 0);
>  }
>  
> -static bool psr_supported_on_chipset(int debugfs_fd)
> -{
> -	char buf[256];
> -	int ret;
> -
> -	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status",
> -				      buf, sizeof(buf));
> -	if (ret < 0)
> -		return false;
> -
> -	return strstr(buf, "Sink_Support: yes\n");
> -}
> -
>  static bool connector_can_psr(drmModeConnectorPtr connector)
>  {
>  	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> @@ -195,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
>  	igt_debug("PSR status: %s\n", buf);
>  }
>  
> -static bool psr_is_enabled(int debugfs_fd)
> -{
> -	char buf[256];
> -
> -	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> -				sizeof(buf));
> -	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
> -}
> -
>  static bool psr_wait_until_enabled(int debugfs_fd)
>  {
> -	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
> +	bool r = psr_wait_entry(debugfs_fd);
Please split this change.

>  
>  	psr_print_status(debugfs_fd);
>  	return r;
> @@ -239,7 +217,7 @@ struct feature {
>  	.connector_possible_fn = connector_can_fbc,
>  	.enable = fbc_modparam_enable,
>  }, psr = {
> -	.supported_on_chipset = psr_supported_on_chipset,
> +	.supported_on_chipset = psr_supported,
>  	.wait_until_enabled = psr_wait_until_enabled,
>  	.connector_possible_fn = connector_can_psr,
>  	.enable = psr_debugfs_enable,
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 276ef83c..c419557e 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1419,13 +1419,7 @@ static void teardown_fbc(void)
>  
>  static bool psr_sink_has_support(void)
>  {
> -	char buf[256];
> -
> -	debugfs_read("i915_edp_psr_status", buf);
> -	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
> -		return false;
> -
> -	return strstr(buf, "Sink_Support: yes\n");
> +	return psr_supported(drm.debugfs);
Suggest inlining at the only call site.

>  }
>  
>  static void setup_psr(void)
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index d00e552f..116fe409 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -191,13 +191,7 @@ static void fill_render(data_t *data, uint32_t
> handle, unsigned char color)
>  
>  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 data->with_psr_disabled ||
> -		strstr(buf, "Sink_Support: yes\n");
> +	return data->with_psr_disabled || psr_supported(data-
> >debugfs_fd);
>  }
>  
>  static bool psr_wait_entry_if_enabled(data_t *data)

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

Re: [igt-dev] [PATCH i-g-t 3/8] lib/psr: Add support to new modified i915_edp_psr_status output

[Dhinakaran Pandiyan]

On Tue, 2018-12-04 at 15:11 -0800, Souza, Jose wrote:
> kernel patches changing the debugfs can be found here:
> 
> https://patchwork.freedesktop.org/series/53510/
> 
> On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > The kernel patch 'drm/i915: Refactor PSR status debugfs' changed
> > the
> > output of i915_edp_psr_status, so adding support to the new output
> > here while keeping the support to the old one for a while.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index d68f4b49..eecee459 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -34,8 +34,10 @@ static bool psr_active(int debugfs_fd, bool
> > check_active)
> >  
> >  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> >  				sizeof(buf));
> > -	active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> > -		(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > +
> > +	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
> > +		 strstr(buf, "Source PSR ctl: enabled");
> > +	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
nit:	          ^ Is this needed, wouldn't the logical OR take care
of boolean conversion? Using the bit-wise assignment after converting
to a boolean also looks odd. And 
active = active && (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
fits withing the 80 character limit.

With or without change,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> >  	return check_active ? active : !active;
> >  }
> >  
> > @@ -138,5 +140,6 @@ bool psr_supported(int debugfs_fd)
> >  
> >  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> >  				sizeof(buf));
> > -	return strstr(buf, "Sink_Support: yes\n");
> > +	return strstr(buf, "Sink_Support: yes\n") ||
> > +	       strstr(buf, "Sink support: yes");
> >  }

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

Re: [igt-dev] [PATCH i-g-t 4/8] lib/psr: Make psr_active() only cares about PSR1

[Dhinakaran Pandiyan]

On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> PSR2 will have it own function to detect if is active so we can drop
> the sleep state search and also to make sure that PSR1 is active lets
> search for "PSR1 enabled".

How do you plan to handle PSR2 in kms_frontbuffer_tracking or
fbt_fbcon? Have new PSR2 sub tests for them?


Would it be better to have some sort of PSR state maintained in the psr
library so that the functions know whether to test for PSR1 or PSR2?

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index eecee459..c6d6390f 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -37,7 +37,8 @@ static bool psr_active(int debugfs_fd, bool
> check_active)
>  
>  	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
>  		 strstr(buf, "Source PSR ctl: enabled");
> -	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> +	active &= !!strstr(buf, "SRDENT");
> +	active &= !!strstr(buf, "Status: PSR1 enabled");
>  	return check_active ? active : !active;
>  }
>  

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

Re: [igt-dev] [PATCH i-g-t 6/8] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

[Dhinakaran Pandiyan]

On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> The main tests for PSR1 check if hardware tracking is detecting
> changes in planes when modifing it in different ways and now
> those tests will also run for PSR2 if supported by source and sink.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_psr.c | 118 +++++++++++++++++++++++++++++++++++++++++++++-
> --
>  1 file changed, 111 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 116fe409..d87b8ea1 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -60,6 +60,7 @@ typedef struct {
>  	int drm_fd;
>  	int debugfs_fd;
>  	enum operations op;
> +	bool op_psr2;
>  	uint32_t devid;
>  	uint32_t crtc_id;
>  	igt_display_t display;
> @@ -71,6 +72,8 @@ typedef struct {
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
>  	bool with_psr_disabled;
> +	bool with_psr2_disabled;
Do we need this? Is there a use case running PSR2 tests with PSR1
enabled? 


> +	bool supports_psr2;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -194,12 +197,22 @@ static bool sink_support(data_t *data)
>  	return data->with_psr_disabled || psr_supported(data-
> >debugfs_fd);
>  }
>  
> -static bool psr_wait_entry_if_enabled(data_t *data)
> +static bool sink_support_psr2(data_t *data)
>  {
> -	if (data->with_psr_disabled)
> -		return true;
> +	return data->with_psr2_disabled || psr2_supported(data-
> >debugfs_fd);
> +}
>  
> -	return psr_wait_entry(data->debugfs_fd);
> +static bool psr_wait_entry_if_enabled(data_t *data)
> +{
> +	if (!data->op_psr2) {
> +		if (data->with_psr_disabled)
> +			return true;
> +		return psr_wait_entry(data->debugfs_fd);
> +	} else {
> +		if (data->with_psr2_disabled)
> +			return true;
> +		return psr2_wait_deep_sleep(data->debugfs_fd);
> +	}
>  }
>  
>  static inline void manual(const char *expected)
> @@ -289,11 +302,16 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> -	igt_assert(psr_wait_exit(data->debugfs_fd));
> +
> +	if (!data->op_psr2)
> +		igt_assert(psr_wait_exit(data->debugfs_fd));
> +	else
> +		igt_assert(psr2_wait_update(data->debugfs_fd));
Move the PSR1/PSR2 logic inside psr_wait_exit() ?

>  	manual(expected);
>  }
>  
> -static void test_cleanup(data_t *data) {
> +static void test_cleanup(data_t *data)
> +{
>  	igt_plane_t *primary;
>  
>  	primary = igt_output_get_plane_type(data->output,
> @@ -304,6 +322,11 @@ static void test_cleanup(data_t *data) {
>  
>  	igt_remove_fb(data->drm_fd, &data->fb_green);
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
> +
> +	/* switch to PSR1 again */
> +	if (data->op_psr2 && !data->with_psr_disabled)
> +		psr_enable(data->debugfs_fd);
> +	data->op_psr2 = false;
>  }
>  
>  static void setup_test_plane(data_t *data, int test_plane)
> @@ -311,6 +334,11 @@ static void setup_test_plane(data_t *data, int
> test_plane)
>  	uint32_t white_h, white_v;
>  	igt_plane_t *primary, *sprite, *cursor;
>  
> +	if (data->op_psr2 && !data->with_psr_disabled) {
> +		igt_require(data->supports_psr2);//op_psr2 is set in
> case this fails?
> +		psr2_enable(data->debugfs_fd);
> +	}
> +
>  	igt_create_color_fb(data->drm_fd,
>  			    data->mode->hdisplay, data->mode->vdisplay,
>  			    DRM_FORMAT_XRGB8888,
> @@ -381,6 +409,9 @@ static int opt_handler(int opt, int opt_index,
> void *_data)
>  	case 'n':
>  		data->with_psr_disabled = true;
>  		break;
> +	case 'm':
> +		data->with_psr2_disabled = true;
> +		break;
>  	default:
>  		igt_assert(0);
>  	}
> @@ -391,9 +422,11 @@ static int opt_handler(int opt, int opt_index,
> void *_data)
>  int main(int argc, char *argv[])
>  {
>  	const char *help_str =
> -	       "  --no-psr\tRun test without PSR.";
> +	       "  --no-psr\tRun test without PSR.\n"
> +	       "  --no-psr2\tRun test without PSR2.";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
> +		{"no-psr2", 0, 0, 'm'},
>  		{ 0, 0, 0, 0 }
>  	};
>  	data_t data = {};
> @@ -414,6 +447,7 @@ int main(int argc, char *argv[])
>  
>  		igt_require_f(sink_support(&data),
>  			      "Sink does not support PSR\n");
> +		data.supports_psr2 = sink_support_psr2(&data);
>  
>  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> 4096);
>  		igt_assert(data.bufmgr);
> @@ -428,6 +462,13 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest("psr2_basic") {
> +		data.op_psr2 = true;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		test_cleanup(&data);
> +	}
> +
>  	igt_subtest("no_drrs") {
>  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(psr_wait_entry_if_enabled(&data));
> @@ -435,6 +476,14 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest("psr2_no_drrs") {
> +		data.op_psr2 = true;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		igt_assert(drrs_disabled(&data));
> +		test_cleanup(&data);
> +	}
> +
>  	for (op = PAGE_FLIP; op <= RENDER; op++) {
>  		igt_subtest_f("primary_%s", op_str(op)) {
>  			data.op = op;
> @@ -445,6 +494,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data,
> DRM_PLANE_TYPE_PRIMARY);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("sprite_%s", op_str(op)) {
>  			data.op = op;
> @@ -455,6 +515,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data,
> DRM_PLANE_TYPE_OVERLAY);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("cursor_%s", op_str(op)) {
>  			data.op = op;
> @@ -465,6 +536,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	igt_subtest_f("dpms") {
>  		data.op = RENDER;
>  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> @@ -474,6 +556,16 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest_f("psr2_dpms") {
> +		data.op_psr2 = true;
> +		data.op = RENDER;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		dpms_off_on(&data);
> +		run_test(&data);
> +		test_cleanup(&data);
> +	}
> +
>  	igt_subtest_f("suspend") {
>  		data.op = PLANE_ONOFF;
>  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> @@ -485,6 +577,18 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest_f("psr2_suspend") {
> +		data.op_psr2 = true;
> +		data.op = PLANE_ONOFF;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> +					      SUSPEND_TEST_NONE);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		run_test(&data);
> +		test_cleanup(&data);
> +	}
> +
How about using a loop to iterate PSR1 and PSR2 modes?
>  	igt_fixture {
>  		if (!data.with_psr_disabled)
>  			psr_disable(data.debugfs_fd);

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

Re: [igt-dev] [PATCH i-g-t 4/8] lib/psr: Make psr_active() only cares about PSR1

[Souza, Jose]

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

On Tue, 2018-12-11 at 16:16 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > PSR2 will have it own function to detect if is active so we can
> > drop
> > the sleep state search and also to make sure that PSR1 is active
> > lets
> > search for "PSR1 enabled".
> 
> How do you plan to handle PSR2 in kms_frontbuffer_tracking or
> fbt_fbcon? Have new PSR2 sub tests for them?

My plan is have a PSR and PSR2 function to make functions really simple
and easy to read.

> 
> 
> Would it be better to have some sort of PSR state maintained in the
> psr
> library so that the functions know whether to test for PSR1 or PSR2?
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index eecee459..c6d6390f 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -37,7 +37,8 @@ static bool psr_active(int debugfs_fd, bool
> > check_active)
> >  
> >  	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
> >  		 strstr(buf, "Source PSR ctl: enabled");
> > -	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > +	active &= !!strstr(buf, "SRDENT");
> > +	active &= !!strstr(buf, "Status: PSR1 enabled");
> >  	return check_active ? active : !active;
> >  }
> >  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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/8] tests: Share the code handling PSR debugfs parsing

[Souza, Jose]

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

On Tue, 2018-12-11 at 15:06 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > The same code checking if sink supports PSR was spread into 3
> > tests,
> > better move it to lib and reuse.
> Agreed.
> 
> > Also kms_fbcon_fbt was doing its own handling to wait for PSR to
> > get
> > enabled while it is already available in lib.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c                    |  9 +++++++++
> >  lib/igt_psr.h                    |  1 +
> >  tests/kms_fbcon_fbt.c            | 26 ++------------------------
> >  tests/kms_frontbuffer_tracking.c |  8 +-------
> >  tests/kms_psr.c                  |  8 +-------
> >  5 files changed, 14 insertions(+), 38 deletions(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index c7bc523c..d68f4b49 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -131,3 +131,12 @@ bool psr_disable(int debugfs_fd)
> >  {
> >  	return psr_set(debugfs_fd, false);
> >  }
> > +
> > +bool psr_supported(int debugfs_fd)
> psr_sink_support() in order to be descriptive about what support we
> are
> referring to.

Done

> 
> > +{
> > +	char buf[PSR_STATUS_MAX_LEN];
> > +
> > +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> > +				sizeof(buf));
> > +	return strstr(buf, "Sink_Support: yes\n");
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index b9693822..a4fcf325 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -32,5 +32,6 @@ bool psr_wait_entry(int debugfs_fd);
> >  bool psr_wait_exit(int debugfs_fd);
> >  bool psr_enable(int debugfs_fd);
> >  bool psr_disable(int debugfs_fd);
> > +bool psr_supported(int debugfs_fd);
> >  
> >  #endif
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index 24d3ad90..32407709 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -168,19 +168,6 @@ static void set_mode_for_one_screen(struct
> > drm_info *drm, struct igt_fb *fb,
> >  	igt_assert_eq(rc, 0);
> >  }
> >  
> > -static bool psr_supported_on_chipset(int debugfs_fd)
> > -{
> > -	char buf[256];
> > -	int ret;
> > -
> > -	ret = igt_debugfs_simple_read(debugfs_fd,
> > "i915_edp_psr_status",
> > -				      buf, sizeof(buf));
> > -	if (ret < 0)
> > -		return false;
> > -
> > -	return strstr(buf, "Sink_Support: yes\n");
> > -}
> > -
> >  static bool connector_can_psr(drmModeConnectorPtr connector)
> >  {
> >  	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> > @@ -195,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
> >  	igt_debug("PSR status: %s\n", buf);
> >  }
> >  
> > -static bool psr_is_enabled(int debugfs_fd)
> > -{
> > -	char buf[256];
> > -
> > -	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> > -				sizeof(buf));
> > -	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
> > -}
> > -
> >  static bool psr_wait_until_enabled(int debugfs_fd)
> >  {
> > -	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
> > +	bool r = psr_wait_entry(debugfs_fd);
> Please split this change.

Done and changing commit message from this patch

> 
> >  
> >  	psr_print_status(debugfs_fd);
> >  	return r;
> > @@ -239,7 +217,7 @@ struct feature {
> >  	.connector_possible_fn = connector_can_fbc,
> >  	.enable = fbc_modparam_enable,
> >  }, psr = {
> > -	.supported_on_chipset = psr_supported_on_chipset,
> > +	.supported_on_chipset = psr_supported,
> >  	.wait_until_enabled = psr_wait_until_enabled,
> >  	.connector_possible_fn = connector_can_psr,
> >  	.enable = psr_debugfs_enable,
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 276ef83c..c419557e 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1419,13 +1419,7 @@ static void teardown_fbc(void)
> >  
> >  static bool psr_sink_has_support(void)
> >  {
> > -	char buf[256];
> > -
> > -	debugfs_read("i915_edp_psr_status", buf);
> > -	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
> > -		return false;
> > -
> > -	return strstr(buf, "Sink_Support: yes\n");
> > +	return psr_supported(drm.debugfs);
> Suggest inlining at the only call site.

Done

> 
> >  }
> >  
> >  static void setup_psr(void)
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index d00e552f..116fe409 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -191,13 +191,7 @@ static void fill_render(data_t *data, uint32_t
> > handle, unsigned char color)
> >  
> >  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 data->with_psr_disabled ||
> > -		strstr(buf, "Sink_Support: yes\n");
> > +	return data->with_psr_disabled || psr_supported(data-
> > > debugfs_fd);
> >  }
> >  
> >  static bool psr_wait_entry_if_enabled(data_t *data)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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 6/8] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

[Souza, Jose]

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

On Tue, 2018-12-11 at 17:20 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > The main tests for PSR1 check if hardware tracking is detecting
> > changes in planes when modifing it in different ways and now
> > those tests will also run for PSR2 if supported by source and sink.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_psr.c | 118
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > --
> >  1 file changed, 111 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 116fe409..d87b8ea1 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -60,6 +60,7 @@ typedef struct {
> >  	int drm_fd;
> >  	int debugfs_fd;
> >  	enum operations op;
> > +	bool op_psr2;
> >  	uint32_t devid;
> >  	uint32_t crtc_id;
> >  	igt_display_t display;
> > @@ -71,6 +72,8 @@ typedef struct {
> >  	drmModeModeInfo *mode;
> >  	igt_output_t *output;
> >  	bool with_psr_disabled;
> > +	bool with_psr2_disabled;
> Do we need this? Is there a use case running PSR2 tests with PSR1
> enabled? 

Huum yeah, I will drop the with_psr2_disabled.

> 
> 
> > +	bool supports_psr2;
> >  } data_t;
> >  
> >  static void create_cursor_fb(data_t *data)
> > @@ -194,12 +197,22 @@ static bool sink_support(data_t *data)
> >  	return data->with_psr_disabled || psr_supported(data-
> > > debugfs_fd);
> >  }
> >  
> > -static bool psr_wait_entry_if_enabled(data_t *data)
> > +static bool sink_support_psr2(data_t *data)
> >  {
> > -	if (data->with_psr_disabled)
> > -		return true;
> > +	return data->with_psr2_disabled || psr2_supported(data-
> > > debugfs_fd);
> > +}
> >  
> > -	return psr_wait_entry(data->debugfs_fd);
> > +static bool psr_wait_entry_if_enabled(data_t *data)
> > +{
> > +	if (!data->op_psr2) {
> > +		if (data->with_psr_disabled)
> > +			return true;
> > +		return psr_wait_entry(data->debugfs_fd);
> > +	} else {
> > +		if (data->with_psr2_disabled)
> > +			return true;
> > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > +	}
> >  }
> >  
> >  static inline void manual(const char *expected)
> > @@ -289,11 +302,16 @@ static void run_test(data_t *data)
> >  		expected = "screen GREEN";
> >  		break;
> >  	}
> > -	igt_assert(psr_wait_exit(data->debugfs_fd));
> > +
> > +	if (!data->op_psr2)
> > +		igt_assert(psr_wait_exit(data->debugfs_fd));
> > +	else
> > +		igt_assert(psr2_wait_update(data->debugfs_fd));
> Move the PSR1/PSR2 logic inside psr_wait_exit() ?

Again I would like to keep psr and psr2 functions, it is more easy to
read.

> 
> >  	manual(expected);
> >  }
> >  
> > -static void test_cleanup(data_t *data) {
> > +static void test_cleanup(data_t *data)
> > +{
> >  	igt_plane_t *primary;
> >  
> >  	primary = igt_output_get_plane_type(data->output,
> > @@ -304,6 +322,11 @@ static void test_cleanup(data_t *data) {
> >  
> >  	igt_remove_fb(data->drm_fd, &data->fb_green);
> >  	igt_remove_fb(data->drm_fd, &data->fb_white);
> > +
> > +	/* switch to PSR1 again */
> > +	if (data->op_psr2 && !data->with_psr_disabled)
> > +		psr_enable(data->debugfs_fd);
> > +	data->op_psr2 = false;
> >  }
> >  
> >  static void setup_test_plane(data_t *data, int test_plane)
> > @@ -311,6 +334,11 @@ static void setup_test_plane(data_t *data, int
> > test_plane)
> >  	uint32_t white_h, white_v;
> >  	igt_plane_t *primary, *sprite, *cursor;
> >  
> > +	if (data->op_psr2 && !data->with_psr_disabled) {
> > +		igt_require(data->supports_psr2);//op_psr2 is set in
> > case this fails?
> > +		psr2_enable(data->debugfs_fd);
> > +	}
> > +
> >  	igt_create_color_fb(data->drm_fd,
> >  			    data->mode->hdisplay, data->mode->vdisplay,
> >  			    DRM_FORMAT_XRGB8888,
> > @@ -381,6 +409,9 @@ static int opt_handler(int opt, int opt_index,
> > void *_data)
> >  	case 'n':
> >  		data->with_psr_disabled = true;
> >  		break;
> > +	case 'm':
> > +		data->with_psr2_disabled = true;
> > +		break;
> >  	default:
> >  		igt_assert(0);
> >  	}
> > @@ -391,9 +422,11 @@ static int opt_handler(int opt, int opt_index,
> > void *_data)
> >  int main(int argc, char *argv[])
> >  {
> >  	const char *help_str =
> > -	       "  --no-psr\tRun test without PSR.";
> > +	       "  --no-psr\tRun test without PSR.\n"
> > +	       "  --no-psr2\tRun test without PSR2.";
> >  	static struct option long_options[] = {
> >  		{"no-psr", 0, 0, 'n'},
> > +		{"no-psr2", 0, 0, 'm'},
> >  		{ 0, 0, 0, 0 }
> >  	};
> >  	data_t data = {};
> > @@ -414,6 +447,7 @@ int main(int argc, char *argv[])
> >  
> >  		igt_require_f(sink_support(&data),
> >  			      "Sink does not support PSR\n");
> > +		data.supports_psr2 = sink_support_psr2(&data);
> >  
> >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> > 4096);
> >  		igt_assert(data.bufmgr);
> > @@ -428,6 +462,13 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest("psr2_basic") {
> > +		data.op_psr2 = true;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	igt_subtest("no_drrs") {
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> >  		igt_assert(psr_wait_entry_if_enabled(&data));
> > @@ -435,6 +476,14 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest("psr2_no_drrs") {
> > +		data.op_psr2 = true;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		igt_assert(drrs_disabled(&data));
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	for (op = PAGE_FLIP; op <= RENDER; op++) {
> >  		igt_subtest_f("primary_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -445,6 +494,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data,
> > DRM_PLANE_TYPE_PRIMARY);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> >  		igt_subtest_f("sprite_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -455,6 +515,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data,
> > DRM_PLANE_TYPE_OVERLAY);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> >  		igt_subtest_f("cursor_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -465,6 +536,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	igt_subtest_f("dpms") {
> >  		data.op = RENDER;
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > @@ -474,6 +556,16 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest_f("psr2_dpms") {
> > +		data.op_psr2 = true;
> > +		data.op = RENDER;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		dpms_off_on(&data);
> > +		run_test(&data);
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	igt_subtest_f("suspend") {
> >  		data.op = PLANE_ONOFF;
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > @@ -485,6 +577,18 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest_f("psr2_suspend") {
> > +		data.op_psr2 = true;
> > +		data.op = PLANE_ONOFF;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > +					      SUSPEND_TEST_NONE);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		run_test(&data);
> > +		test_cleanup(&data);
> > +	}
> > +
> How about using a loop to iterate PSR1 and PSR2 modes?

Having a test for PSR and PSR2 is better to keep track of bug and for
us to fix, so we know for sure in what PSR version it is failling.

> >  	igt_fixture {
> >  		if (!data.with_psr_disabled)
> >  			psr_disable(data.debugfs_fd);

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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 4/8] lib/psr: Make psr_active() only cares about PSR1

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 11:47 -0800, Souza, Jose wrote:
> On Tue, 2018-12-11 at 16:16 -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-12-04 at 15:09 -0800, José Roberto de Souza wrote:
> > > PSR2 will have it own function to detect if is active so we can
> > > drop
> > > the sleep state search and also to make sure that PSR1 is active
> > > lets
> > > search for "PSR1 enabled".
> > 
> > How do you plan to handle PSR2 in kms_frontbuffer_tracking or
> > fbt_fbcon? Have new PSR2 sub tests for them?
> 
> My plan is have a PSR and PSR2 function to make functions really
> simple
> and easy to read.
> 

We'll end up with 

if (psr2)
	/*test PSR2 exit */
else
	/* test PSR1 exit */

in every single sub-test.


Since, the general format for both PSR1 and PSR2 tests is
1) Wait and check for PSR entry
2) Do some operation
3) Wait and check for PSR exit


Isn't it better to use the psr_wait_entry() and psr_wait_exit()
interfaces in all tests and deal with PSR1 and PSR2 specific way of
testing inside them?

> > 
> > 
> > Would it be better to have some sort of PSR state maintained in the
> > psr
> > library so that the functions know whether to test for PSR1 or
> > PSR2?
> > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  lib/igt_psr.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > > index eecee459..c6d6390f 100644
> > > --- a/lib/igt_psr.c
> > > +++ b/lib/igt_psr.c
> > > @@ -37,7 +37,8 @@ static bool psr_active(int debugfs_fd, bool
> > > check_active)
> > >  
> > >  	active = strstr(buf, "HW Enabled & Active bit: yes\n") ||
> > >  		 strstr(buf, "Source PSR ctl: enabled");
> > > -	active &= !!(strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > > +	active &= !!strstr(buf, "SRDENT");
> > > +	active &= !!strstr(buf, "Status: PSR1 enabled");
> > >  	return check_active ? active : !active;
> > >  }
> > >  

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