[igt-dev] [PATCH i-g-t v2 1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it

[igt-dev] [PATCH i-g-t v2 1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it

[José Roberto de Souza]

So every function reading i915_edp_psr_status can allocate a buffer
long enough.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c                    | 2 +-
 lib/igt_psr.h                    | 2 ++
 tests/kms_fbcon_fbt.c            | 6 +++---
 tests/kms_frontbuffer_tracking.c | 2 +-
 tests/kms_psr.c                  | 2 +-
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 0ddfb64f..c105bb6e 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -28,7 +28,7 @@
 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));
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index b9693822..96f7bedf 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -28,6 +28,8 @@
 #include "igt_core.h"
 #include "igt_aux.h"
 
+#define PSR_STATUS_MAX_LEN 512
+
 bool psr_wait_entry(int debugfs_fd);
 bool psr_wait_exit(int debugfs_fd);
 bool psr_enable(int debugfs_fd);
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 24d3ad90..0ad53266 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -170,7 +170,7 @@ static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
 
 static bool psr_supported_on_chipset(int debugfs_fd)
 {
-	char buf[256];
+	char buf[PSR_STATUS_MAX_LEN];
 	int ret;
 
 	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
@@ -188,7 +188,7 @@ static bool connector_can_psr(drmModeConnectorPtr connector)
 
 static void psr_print_status(int debugfs_fd)
 {
-	static char buf[256];
+	static char buf[PSR_STATUS_MAX_LEN];
 
 	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				sizeof(buf));
@@ -197,7 +197,7 @@ static void psr_print_status(int debugfs_fd)
 
 static bool psr_is_enabled(int debugfs_fd)
 {
-	char buf[256];
+	char buf[PSR_STATUS_MAX_LEN];
 
 	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				sizeof(buf));
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 5ab28319..c366fecf 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1419,7 +1419,7 @@ static void teardown_fbc(void)
 
 static bool psr_sink_has_support(void)
 {
-	char buf[256];
+	char buf[PSR_STATUS_MAX_LEN];
 
 	debugfs_read("i915_edp_psr_status", buf);
 	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index d00e552f..20b69892 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -191,7 +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];
+	char buf[PSR_STATUS_MAX_LEN];
 
 	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
 			 buf, sizeof(buf));
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 2/9] tests/psr: Share the code check if sink supports PSR

[José Roberto de Souza]

The same code checking if sink supports PSR was spread into 3 tests,
better move it to lib and reuse.

v2: splitted previous patch into this one and the next one(Dhinakaran)

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                    | 10 ++++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_fbcon_fbt.c            | 15 +--------------
 tests/kms_frontbuffer_tracking.c | 13 +------------
 tests/kms_psr.c                  |  8 +-------
 5 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index c105bb6e..82012e6d 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -129,3 +129,13 @@ bool psr_disable(int debugfs_fd)
 {
 	return psr_set(debugfs_fd, false);
 }
+
+bool psr_sink_support(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	int ret;
+
+	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				      sizeof(buf));
+	return ret > 0 && strstr(buf, "Sink_Support: yes\n");
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 96f7bedf..d3336c2d 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -34,5 +34,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_sink_support(int debugfs_fd);
 
 #endif
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 0ad53266..2823b47a 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[PSR_STATUS_MAX_LEN];
-	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);
@@ -239,7 +226,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_sink_support,
 	.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 c366fecf..42f4c289 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1417,17 +1417,6 @@ static void teardown_fbc(void)
 {
 }
 
-static bool psr_sink_has_support(void)
-{
-	char buf[PSR_STATUS_MAX_LEN];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
-		return false;
-
-	return strstr(buf, "Sink_Support: yes\n");
-}
-
 static void setup_psr(void)
 {
 	if (prim_mode_params.output->config.connector->connector_type !=
@@ -1436,7 +1425,7 @@ static void setup_psr(void)
 		return;
 	}
 
-	if (!psr_sink_has_support()) {
+	if (!psr_sink_support(drm.debugfs)) {
 		igt_info("Can't test PSR: not supported by sink.\n");
 		return;
 	}
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 20b69892..855679b0 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[PSR_STATUS_MAX_LEN];
-
-	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_sink_support(data->debugfs_fd);
 }
 
 static bool psr_wait_entry_if_enabled(data_t *data)
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 3/9] tests/kms_fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled

[José Roberto de Souza]

kms_fbcon_fbt was doing its own handling to wait for PSR to get
enabled while it is already available in lib.

v2: splitted previous patch into this one and the previous one(Dhinakaran)

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 2823b47a..3db60c0d 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -182,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[PSR_STATUS_MAX_LEN];
-
-	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;
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 4/9] 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>
Reviewed-by: 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 82012e6d..8efc9216 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -32,8 +32,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 = active && (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
 	return check_active ? active : !active;
 }
 
@@ -137,5 +139,6 @@ bool psr_sink_support(int debugfs_fd)
 
 	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
 				      sizeof(buf));
-	return ret > 0 && strstr(buf, "Sink_Support: yes\n");
+	return ret > 0 && (strstr(buf, "Sink_Support: yes\n") ||
+			   strstr(buf, "Sink support: yes"));
 }
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 5/9] 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 as it is a PSR2 state.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 8efc9216..55f61c25 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -35,7 +35,7 @@ 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 = active && (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
+	active = active && strstr(buf, "SRDENT");
 	return check_active ? active : !active;
 }
 
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 6/9] 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 | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_psr.h |  6 ++++
 2 files changed, 98 insertions(+)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 55f61c25..0f4b2e23 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -142,3 +142,95 @@ bool psr_sink_support(int debugfs_fd)
 	return ret > 0 && (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_sink_support(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	int ret;
+
+	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				      sizeof(buf));
+	/*
+	 * i915 requires PSR version 0x03 that is PSR2 + SU with Y-coordinate to
+	 * support PSR2
+	 */
+	return ret > 0 && strstr(buf, "Sink support: yes [0x03]");
+}
+
+static bool psr2_in_deep_sleep(int debugfs_fd)
+{
+	bool deep_sleep;
+	char buf[PSR_STATUS_MAX_LEN];
+	int ret;
+
+	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				      sizeof(buf));
+	if (ret < 0)
+		return false;
+
+	deep_sleep = strstr(buf, "PSR mode: PSR2 enabled\n") &&
+		     strstr(buf, "Source PSR status: DEEP_SLEEP");
+
+	return deep_sleep;
+}
+
+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);
+}
+
+static bool psr2_status_update(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	int ret;
+
+	ret = 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 ret > 0 && !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 d3336c2d..2b8d1a90 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -36,4 +36,10 @@ bool psr_enable(int debugfs_fd);
 bool psr_disable(int debugfs_fd);
 bool psr_sink_support(int debugfs_fd);
 
+bool psr2_wait_deep_sleep(int debugfs_fd);
+bool psr2_wait_update(int debugfs_fd);
+bool psr2_enable(int debugfs_fd);
+bool psr2_disable(int debugfs_fd);
+bool psr2_sink_support(int debugfs_fd);
+
 #endif
-- 
2.20.0

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

[igt-dev] [PATCH i-g-t v2 7/9] 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 | 112 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 105 insertions(+), 7 deletions(-)

diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 855679b0..d4d38320 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,7 @@ typedef struct {
 	drmModeModeInfo *mode;
 	igt_output_t *output;
 	bool with_psr_disabled;
+	bool supports_psr2;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
 	return data->with_psr_disabled || psr_sink_support(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_psr_disabled || psr2_sink_support(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_psr_disabled)
+			return true;
+		return psr2_wait_deep_sleep(data->debugfs_fd);
+	}
 }
 
 static inline void manual(const char *expected)
@@ -289,11 +301,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 +321,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 +333,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);
+		psr2_enable(data->debugfs_fd);
+	}
+
 	igt_create_color_fb(data->drm_fd,
 			    data->mode->hdisplay, data->mode->vdisplay,
 			    DRM_FORMAT_XRGB8888,
@@ -391,7 +418,7 @@ 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/PSR2.\n";
 	static struct option long_options[] = {
 		{"no-psr", 0, 0, 'n'},
 		{ 0, 0, 0, 0 }
@@ -414,6 +441,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 +456,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 +470,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 +488,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 +509,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 +530,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 +550,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 +571,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.20.0

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

[igt-dev] [PATCH i-g-t v2 8/9] 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.20.0

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

[igt-dev] [PATCH i-g-t v2 9/9] 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.

v2: Using new SU blocks debugfs output

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          |  23 +++
 lib/igt_psr.h          |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_psr2_su.c    | 362 +++++++++++++++++++++++++++++++++++++++++
 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 0f4b2e23..a8b9e4ce 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -234,3 +234,26 @@ bool psr2_wait_update(int debugfs_fd)
 {
 	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
 }
+
+#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
+
+bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t *num_su_blocks)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	char *str;
+	int ret;
+
+	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				      sizeof(buf));
+	if (ret < 0)
+		return false;
+
+	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
+	if (!str)
+		return false;
+
+	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
+	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
+
+	return true;
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 2b8d1a90..349eb0c2 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -41,5 +41,6 @@ bool psr2_wait_update(int debugfs_fd);
 bool psr2_enable(int debugfs_fd);
 bool psr2_disable(int debugfs_fd);
 bool psr2_sink_support(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..67c02f25
--- /dev/null
+++ b/tests/kms_psr2_su.c
@@ -0,0 +1,362 @@
+/*
+ * 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 lines 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_sink_support(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_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.20.0

_______________________________________________
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,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it
URL   : https://patchwork.freedesktop.org/series/54694/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5361 -> IGTPW_2182
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

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

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

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       DMESG-WARN [fdo#102614] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767


Participating hosts (52 -> 43)
------------------------------

  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ilk-650 fi-ctg-p8600 fi-icl-y fi-bdw-samus 


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

    * IGT: IGT_4756 -> IGTPW_2182

  CI_DRM_5361: 076417527f6abadb519f6d5e7cec575973888607 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2182: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2182/
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ 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_2182/
_______________________________________________
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,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it
URL   : https://patchwork.freedesktop.org/series/54694/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5361_full -> IGTPW_2182_full
====================================================

Summary
-------

  **WARNING**

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

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

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

### IGT changes ###

#### Warnings ####

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@shrink:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#108784]

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-d:
    - shard-apl:          SKIP -> INCOMPLETE [fdo#103927]

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

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

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

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

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
    - shard-glk:          PASS -> FAIL [fdo#107791]

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

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          PASS -> FAIL [fdo#103167] +7

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

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

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

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

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

  
#### Possible fixes ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

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

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          FAIL [fdo#103232] -> PASS

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

  * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
    - shard-glk:          FAIL [fdo#103060] -> PASS

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

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

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS +1

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

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

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

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-WARN [fdo#105604] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107791]: https://bugs.freedesktop.org/show_bug.cgi?id=107791
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [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_4756 -> IGTPW_2182
    * Piglit: piglit_4509 -> None

  CI_DRM_5361: 076417527f6abadb519f6d5e7cec575973888607 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2182: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2182/
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v2 2/9] tests/psr: Share the code check if sink supports PSR

[Dhinakaran Pandiyan]

On Thu, 2019-01-03 at 06:36 -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.
> 
> v2: splitted previous patch into this one and the next
> one(Dhinakaran)
> 
> 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>

Patches 1 and 2,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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

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

[Dhinakaran Pandiyan]

On Thu, 2019-01-03 at 06:36 -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 | 112 +++++++++++++++++++++++++++++++++++++++++++++-
> --
>  1 file changed, 105 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 855679b0..d4d38320 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,7 @@ typedef struct {
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
>  	bool with_psr_disabled;
> +	bool supports_psr2;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
>  	return data->with_psr_disabled || psr_sink_support(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_psr_disabled || psr2_sink_support(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_psr_disabled)
> +			return true;
> +		return psr2_wait_deep_sleep(data->debugfs_fd);
> +	}
>  }


Unless I am missing something, we can simplify all these changes by
pushing the PSR mode down through the call chain.

 
-static bool psr_active(int debugfs_fd, bool check_active)
+static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
check_active)
 {
        bool active;
        char buf[PSR_STATUS_MAX_LEN];
+       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
 
        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"));
+                strstr(buf, s);
        return check_active ? active : !active;
 }
 
-bool psr_wait_entry(int debugfs_fd)
+bool psr_wait_entry(int debugfs_fd, int psr_mode)
 {
-       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
+       return igt_wait(psr_active(debugfs_fd, true, psr_mode), 500,
20);
 }
 
-bool psr_wait_exit(int debugfs_fd)
+bool psr_wait_exit(int debugfs_fd, int psr_mode)
 {
        return igt_wait(psr_active(debugfs_fd, false), 40, 10);
 }
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 855679b0..bb90b994 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t *data)
        if (data->with_psr_disabled)
                return true;
 
-       return psr_wait_entry(data->debugfs_fd);
+       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
 }
 
 static inline void manual(const char *expected)
@@ -289,7 +289,7 @@ static void run_test(data_t *data)
                expected = "screen GREEN";
                break;
        }
-       igt_assert(psr_wait_exit(data->debugfs_fd));
+       igt_assert(psr_wait_exit(data->debugfs_fd, data->psr_mode));
        manual(expected);
 }



>  
>  static inline void manual(const char *expected)
> @@ -289,11 +301,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 +321,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 +333,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);
> +		psr2_enable(data->debugfs_fd);
> +	}
> +
>  	igt_create_color_fb(data->drm_fd,
>  			    data->mode->hdisplay, data->mode->vdisplay,
>  			    DRM_FORMAT_XRGB8888,
> @@ -391,7 +418,7 @@ 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/PSR2.\n";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
>  		{ 0, 0, 0, 0 }
> @@ -414,6 +441,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 +456,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 +470,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 +488,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 +509,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 +530,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 +550,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 +571,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);

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

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v2,1/9] lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it (rev2)
URL   : https://patchwork.freedesktop.org/series/54694/
State : failure

== Summary ==

Applying: lib/psr: Add a macro with the maximum lenght of i915_edp_psr_status and use it
Applying: tests/psr: Share the code check if sink supports PSR
Applying: tests/kms_fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
Applying: lib/psr: Add support to new modified i915_edp_psr_status output
Applying: lib/psr: Make psr_active() only cares about PSR1
Applying: lib/psr: Add PSR2 functions
Applying: tests/psr: Add the same test coverage that we have for PSR1 to PSR2
Using index info to reconstruct a base tree...
Patch failed at 0007 tests/psr: Add the same test coverage that we have for PSR1 to PSR2
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [igt-dev] [PATCH i-g-t v2 6/9] lib/psr: Add PSR2 functions

[Dhinakaran Pandiyan]

On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> 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 | 92
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_psr.h |  6 ++++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 55f61c25..0f4b2e23 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -142,3 +142,95 @@ bool psr_sink_support(int debugfs_fd)
>  	return ret > 0 && (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);
> +}

Pass PSR1/PSR2 as an argument from the caller and handle that in
psr_set(), we can avoid duplicating all this boilerplate code.

> +
> +bool psr2_sink_support(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	int ret;
> +
> +	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status", buf,
> +				      sizeof(buf));
> +	/*
> +	 * i915 requires PSR version 0x03 that is PSR2 + SU with Y-
> coordinate to
> +	 * support PSR2
> +	 */
> +	return ret > 0 && strstr(buf, "Sink support: yes [0x03]");
> +}
Same here.

> +
> +static bool psr2_in_deep_sleep(int debugfs_fd)
> +{
> +	bool deep_sleep;
> +	char buf[PSR_STATUS_MAX_LEN];
> +	int ret;
> +
> +	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status", buf,
> +				      sizeof(buf));
> +	if (ret < 0)
> +		return false;
> +
> +	deep_sleep = strstr(buf, "PSR mode: PSR2 enabled\n") &&
> +		     strstr(buf, "Source PSR status: DEEP_SLEEP");
> +
> +	return deep_sleep;
> +}
> +
> +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);
> +}
> +
> +static bool psr2_status_update(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	int ret;
> +
> +	ret = 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 ret > 0 && !strstr(buf, "Source PSR status:
> DEEP_SLEEP");
> +}
> +
> +bool psr2_wait_update(int debugfs_fd)
> +{
> +	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
> +}

The logic looks very similar to PSR1, please see my comment for the
next patch.

> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index d3336c2d..2b8d1a90 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -36,4 +36,10 @@ bool psr_enable(int debugfs_fd);
>  bool psr_disable(int debugfs_fd);
>  bool psr_sink_support(int debugfs_fd);
>  
> +bool psr2_wait_deep_sleep(int debugfs_fd);
> +bool psr2_wait_update(int debugfs_fd);
> +bool psr2_enable(int debugfs_fd);
> +bool psr2_disable(int debugfs_fd);
> +bool psr2_sink_support(int debugfs_fd);
> +
>  #endif

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

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

[Souza, Jose]

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

On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-03 at 06:36 -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 | 112
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > --
> >  1 file changed, 105 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 855679b0..d4d38320 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,7 @@ typedef struct {
> >  	drmModeModeInfo *mode;
> >  	igt_output_t *output;
> >  	bool with_psr_disabled;
> > +	bool supports_psr2;
> >  } data_t;
> >  
> >  static void create_cursor_fb(data_t *data)
> > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> >  	return data->with_psr_disabled || psr_sink_support(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_psr_disabled || psr2_sink_support(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_psr_disabled)
> > +			return true;
> > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > +	}
> >  }
> 
> Unless I am missing something, we can simplify all these changes by
> pushing the PSR mode down through the call chain.
> 
>  
> -static bool psr_active(int debugfs_fd, bool check_active)
> +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> check_active)
>  {

I added the PSR2 functions with other names to avoid any
misinterpretation as PSR2 could be active in deep sleep state and doing
selective update and in the states between and in the kms_psr2_su we
need to check both states to properly test.
 
I'm okay in adding the PSR mode to enable(), disable(), sink_support()
and reuse the internal functions too but in the other functions the
code saved is not worth. What do you think DK?

>         bool active;
>         char buf[PSR_STATUS_MAX_LEN];
> +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
>  
>         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"));
> +                strstr(buf, s);
>         return check_active ? active : !active;
>  }
>  
> -bool psr_wait_entry(int debugfs_fd)
> +bool psr_wait_entry(int debugfs_fd, int psr_mode)
>  {
> -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> +       return igt_wait(psr_active(debugfs_fd, true, psr_mode), 500,
> 20);
>  }
>  
> -bool psr_wait_exit(int debugfs_fd)
> +bool psr_wait_exit(int debugfs_fd, int psr_mode)
>  {
>         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
>  }
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 855679b0..bb90b994 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> *data)
>         if (data->with_psr_disabled)
>                 return true;
>  
> -       return psr_wait_entry(data->debugfs_fd);
> +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
>  }
>  
>  static inline void manual(const char *expected)
> @@ -289,7 +289,7 @@ static void run_test(data_t *data)
>                 expected = "screen GREEN";
>                 break;
>         }
> -       igt_assert(psr_wait_exit(data->debugfs_fd));
> +       igt_assert(psr_wait_exit(data->debugfs_fd, data->psr_mode));
>         manual(expected);
>  }
> 
> 
> 
> >  
> >  static inline void manual(const char *expected)
> > @@ -289,11 +301,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 +321,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 +333,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);
> > +		psr2_enable(data->debugfs_fd);
> > +	}
> > +
> >  	igt_create_color_fb(data->drm_fd,
> >  			    data->mode->hdisplay, data->mode->vdisplay,
> >  			    DRM_FORMAT_XRGB8888,
> > @@ -391,7 +418,7 @@ 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/PSR2.\n";
> >  	static struct option long_options[] = {
> >  		{"no-psr", 0, 0, 'n'},
> >  		{ 0, 0, 0, 0 }
> > @@ -414,6 +441,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 +456,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 +470,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 +488,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 +509,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 +530,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 +550,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 +571,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);

[-- 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 v2 7/9] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

[Dhinakaran Pandiyan]

On Thu, 2019-01-10 at 17:12 -0800, Souza, Jose wrote:
> On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> > On Thu, 2019-01-03 at 06:36 -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 | 112
> > > +++++++++++++++++++++++++++++++++++++++++++++-
> > > --
> > >  1 file changed, 105 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 855679b0..d4d38320 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,7 @@ typedef struct {
> > >  	drmModeModeInfo *mode;
> > >  	igt_output_t *output;
> > >  	bool with_psr_disabled;
> > > +	bool supports_psr2;
> > >  } data_t;
> > >  
> > >  static void create_cursor_fb(data_t *data)
> > > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> > >  	return data->with_psr_disabled || psr_sink_support(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_psr_disabled || psr2_sink_support(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_psr_disabled)
> > > +			return true;
> > > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > > +	}
> > >  }
> > 
> > Unless I am missing something, we can simplify all these changes by
> > pushing the PSR mode down through the call chain.
> > 
> >  
> > -static bool psr_active(int debugfs_fd, bool check_active)
> > +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> > check_active)
> >  {
> 
> I added the PSR2 functions with other names to avoid any
> misinterpretation as PSR2 could be active in deep sleep state and
> doing
> selective update and in the states between and in the kms_psr2_su we
> need to check both states to properly test.

Selective update testing does not need modifying the behavior of any of
these functions, does it? Both PSR1 and PSR2(non-SU) tests wait until
PSR is in an active state, do some operation and then verify we are out
of the state. Since they follow the same test steps, I believe it is
better to share the interface. We also want kms_fbt_fbcon and
kms_frontbuffer_tracking to setup PSR2, that is another reason
psr_wait_entry()/psr_wait_exit() in the library should handle PSR1 v/s
PSR2 instead of the callers in kms_psr.


>  
> I'm okay in adding the PSR mode to enable(), disable(),
> sink_support()
> and reuse the internal functions too but in the other functions the
> code saved is not worth. What do you think DK?
> 
> >         bool active;
> >         char buf[PSR_STATUS_MAX_LEN];
> > +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
> >  
> >         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"));
> > +                strstr(buf, s);
> >         return check_active ? active : !active;
> >  }
> >  
> > -bool psr_wait_entry(int debugfs_fd)
> > +bool psr_wait_entry(int debugfs_fd, int psr_mode)
> >  {
> > -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> > +       return igt_wait(psr_active(debugfs_fd, true, psr_mode),
> > 500,
> > 20);
> >  }
> >  
> > -bool psr_wait_exit(int debugfs_fd)
> > +bool psr_wait_exit(int debugfs_fd, int psr_mode)
> >  {
> >         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> >  }
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 855679b0..bb90b994 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> > *data)
> >         if (data->with_psr_disabled)
> >                 return true;
> >  
> > -       return psr_wait_entry(data->debugfs_fd);
> > +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
> >  }
> >  
> >  static inline void manual(const char *expected)
> > @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> >                 expected = "screen GREEN";
> >                 break;
> >         }
> > -       igt_assert(psr_wait_exit(data->debugfs_fd));
> > +       igt_assert(psr_wait_exit(data->debugfs_fd, data-
> > >psr_mode));
> >         manual(expected);
> >  }
> > 
> > 
> > 
> > >  
> > >  static inline void manual(const char *expected)
> > > @@ -289,11 +301,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 +321,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 +333,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);
> > > +		psr2_enable(data->debugfs_fd);
> > > +	}
> > > +
> > >  	igt_create_color_fb(data->drm_fd,
> > >  			    data->mode->hdisplay, data->mode->vdisplay,
> > >  			    DRM_FORMAT_XRGB8888,
> > > @@ -391,7 +418,7 @@ 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/PSR2.\n";
> > >  	static struct option long_options[] = {
> > >  		{"no-psr", 0, 0, 'n'},
> > >  		{ 0, 0, 0, 0 }
> > > @@ -414,6 +441,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 +456,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 +470,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 +488,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 +509,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 +530,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 +550,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 +571,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);

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

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

[Souza, Jose]

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

On Fri, 2019-01-11 at 11:49 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-10 at 17:12 -0800, Souza, Jose wrote:
> > On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> > > On Thu, 2019-01-03 at 06:36 -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 | 112
> > > > +++++++++++++++++++++++++++++++++++++++++++++-
> > > > --
> > > >  1 file changed, 105 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > index 855679b0..d4d38320 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,7 @@ typedef struct {
> > > >  	drmModeModeInfo *mode;
> > > >  	igt_output_t *output;
> > > >  	bool with_psr_disabled;
> > > > +	bool supports_psr2;
> > > >  } data_t;
> > > >  
> > > >  static void create_cursor_fb(data_t *data)
> > > > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> > > >  	return data->with_psr_disabled ||
> > > > psr_sink_support(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_psr_disabled ||
> > > > psr2_sink_support(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_psr_disabled)
> > > > +			return true;
> > > > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > > > +	}
> > > >  }
> > > 
> > > Unless I am missing something, we can simplify all these changes
> > > by
> > > pushing the PSR mode down through the call chain.
> > > 
> > >  
> > > -static bool psr_active(int debugfs_fd, bool check_active)
> > > +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> > > check_active)
> > >  {
> > 
> > I added the PSR2 functions with other names to avoid any
> > misinterpretation as PSR2 could be active in deep sleep state and
> > doing
> > selective update and in the states between and in the kms_psr2_su
> > we
> > need to check both states to properly test.
> 
> Selective update testing does not need modifying the behavior of any
> of
> these functions, does it? Both PSR1 and PSR2(non-SU) tests wait until
> PSR is in an active state, do some operation and then verify we are
> out
> of the state.

Yes but after perform some operation it can keep into a active state in
PSR2 that is why psr_wait_exit() is not a good name. Same for
psr_wait_entry() it is wait entry in deep sleep? or wait entry in PSR
active?

> Since they follow the same test steps, I believe it is
> better to share the interface. We also want kms_fbt_fbcon and
> kms_frontbuffer_tracking to setup PSR2, that is another reason
> psr_wait_entry()/psr_wait_exit() in the library should handle PSR1
> v/s
> PSR2 instead of the callers in kms_psr.
> 
> 
> >  
> > I'm okay in adding the PSR mode to enable(), disable(),
> > sink_support()
> > and reuse the internal functions too but in the other functions the
> > code saved is not worth. What do you think DK?
> > 
> > >         bool active;
> > >         char buf[PSR_STATUS_MAX_LEN];
> > > +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
> > >  
> > >         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"));
> > > +                strstr(buf, s);
> > >         return check_active ? active : !active;
> > >  }
> > >  
> > > -bool psr_wait_entry(int debugfs_fd)
> > > +bool psr_wait_entry(int debugfs_fd, int psr_mode)
> > >  {
> > > -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> > > +       return igt_wait(psr_active(debugfs_fd, true, psr_mode),
> > > 500,
> > > 20);
> > >  }
> > >  
> > > -bool psr_wait_exit(int debugfs_fd)
> > > +bool psr_wait_exit(int debugfs_fd, int psr_mode)
> > >  {
> > >         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> > >  }
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 855679b0..bb90b994 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> > > *data)
> > >         if (data->with_psr_disabled)
> > >                 return true;
> > >  
> > > -       return psr_wait_entry(data->debugfs_fd);
> > > +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
> > >  }
> > >  
> > >  static inline void manual(const char *expected)
> > > @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> > >                 expected = "screen GREEN";
> > >                 break;
> > >         }
> > > -       igt_assert(psr_wait_exit(data->debugfs_fd));
> > > +       igt_assert(psr_wait_exit(data->debugfs_fd, data-
> > > > psr_mode));
> > >         manual(expected);
> > >  }
> > > 
> > > 
> > > 
> > > >  
> > > >  static inline void manual(const char *expected)
> > > > @@ -289,11 +301,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 +321,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 +333,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);
> > > > +		psr2_enable(data->debugfs_fd);
> > > > +	}
> > > > +
> > > >  	igt_create_color_fb(data->drm_fd,
> > > >  			    data->mode->hdisplay, data->mode-
> > > > >vdisplay,
> > > >  			    DRM_FORMAT_XRGB8888,
> > > > @@ -391,7 +418,7 @@ 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/PSR2.\n";
> > > >  	static struct option long_options[] = {
> > > >  		{"no-psr", 0, 0, 'n'},
> > > >  		{ 0, 0, 0, 0 }
> > > > @@ -414,6 +441,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 +456,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 +470,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 +488,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(&d
> > > > ata));
> > > > +			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 +509,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(&d
> > > > ata));
> > > > +			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 +530,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(&d
> > > > ata));
> > > > +			run_test(&data);
> > > > +			test_cleanup(&data);
> > > > +		}
> > > > +	}
> > > > +
> > > >  	igt_subtest_f("dpms") {
> > > >  		data.op = RENDER;
> > > >  		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > @@ -474,6 +550,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 +571,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);

[-- 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 v2 9/9] test: Add PSR2 selective update tests

[Dhinakaran Pandiyan]

On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> 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.
Does this mean flipping once while polling SU status registers is not
sufficient?


> 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.
This patch warrants some explanation about the timeouts used and how
they were selected.

> 
> v2: Using new SU blocks debugfs output
> 
> 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          |  23 +++
>  lib/igt_psr.h          |   1 +
>  tests/Makefile.sources |   1 +
>  tests/kms_psr2_su.c    | 362
> +++++++++++++++++++++++++++++++++++++++++
>  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 0f4b2e23..a8b9e4ce 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -234,3 +234,26 @@ bool psr2_wait_update(int debugfs_fd)
>  {
>  	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
>  }
> +
> +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> +
> +bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> *num_su_blocks)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	char *str;
> +	int ret;
> +
> +	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status", buf,
> +				      sizeof(buf));
> +	if (ret < 0)
> +		return false;
> +
> +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> +	if (!str)
> +		return false;
> +
> +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> +
> +	return true;
> +}
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 2b8d1a90..349eb0c2 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -41,5 +41,6 @@ bool psr2_wait_update(int debugfs_fd);
>  bool psr2_enable(int debugfs_fd);
>  bool psr2_disable(int debugfs_fd);
>  bool psr2_sink_support(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..67c02f25
> --- /dev/null
> +++ b/tests/kms_psr2_su.c
> @@ -0,0 +1,362 @@
> +/*
> + * 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 lines 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);
> +	}
> +

What happens if you start this thread and then flip just once? Do you
see the expected block count in debugfs at some instance after the
flip?

> +	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_PRIM
> ARY);
> +
> +		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 */
Wouldn't it be simpler to flip a fixed number of times?


> +		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_sink_support(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_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;
What's the rationale for this number?

> +		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',

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

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

[Souza, Jose]

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

On Fri, 2019-01-11 at 15:15 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > 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.
> Does this mean flipping once while polling SU status registers is not
> sufficient?

After a flip in DEEP_SLEEP it will do a PSR exit and PSR active
sequence, it will only do a selective update while in SLEEP state that
is why more than one flip is necessary and it will vary depending of
the values that is set to frames to activate PSR and frames to enter
deep sleep.

> 
> 
> > 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.
> This patch warrants some explanation about the timeouts used and how
> they were selected.

The flip timeout is a value to achieve 15hz flips and the error timeout
will be removed as you suggested bellow, I will add a comment when
setting the interval to the flip timerfd.

> 
> > v2: Using new SU blocks debugfs output
> > 
> > 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          |  23 +++
> >  lib/igt_psr.h          |   1 +
> >  tests/Makefile.sources |   1 +
> >  tests/kms_psr2_su.c    | 362
> > +++++++++++++++++++++++++++++++++++++++++
> >  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 0f4b2e23..a8b9e4ce 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -234,3 +234,26 @@ bool psr2_wait_update(int debugfs_fd)
> >  {
> >  	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
> >  }
> > +
> > +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> > +
> > +bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> > *num_su_blocks)
> > +{
> > +	char buf[PSR_STATUS_MAX_LEN];
> > +	char *str;
> > +	int ret;
> > +
> > +	ret = igt_debugfs_simple_read(debugfs_fd,
> > "i915_edp_psr_status", buf,
> > +				      sizeof(buf));
> > +	if (ret < 0)
> > +		return false;
> > +
> > +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> > +	if (!str)
> > +		return false;
> > +
> > +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> > +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> > +
> > +	return true;
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 2b8d1a90..349eb0c2 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -41,5 +41,6 @@ bool psr2_wait_update(int debugfs_fd);
> >  bool psr2_enable(int debugfs_fd);
> >  bool psr2_disable(int debugfs_fd);
> >  bool psr2_sink_support(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..67c02f25
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,362 @@
> > +/*
> > + * 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 lines 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);
> > +	}
> > +
> 
> What happens if you start this thread and then flip just once? Do you
> see the expected block count in debugfs at some instance after the
> flip?

As said above no, as we wait for DEEP_SLEEP state and even in SLEEP
state sometimes HW decides to go to PSR exit and PSR active flow
instead of do a selective update.

> 
> > +	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_PRIM
> > ARY);
> > +
> > +		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 */
> Wouldn't it be simpler to flip a fixed number of times?

Yeah it would be simpler, I will change to that.

> 
> 
> > +		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_sink_support(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_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;
> What's the rationale for this number?


15hz flips, I will add a comment here.



> 
> > +		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',

[-- 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 v2 9/9] test: Add PSR2 selective update tests

[Dhinakaran Pandiyan]

On Mon, 2019-01-14 at 14:48 -0800, Souza, Jose wrote:
> On Fri, 2019-01-11 at 15:15 -0800, Dhinakaran Pandiyan wrote:
> > On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > > 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.
> > 
> > Does this mean flipping once while polling SU status registers is
> > not
> > sufficient?
> 
> After a flip in DEEP_SLEEP it will do a PSR exit and PSR active
> sequence, it will only do a selective update while in SLEEP state
> that
> is why more than one flip is necessary and it will vary depending of
> the values that is set to frames to activate PSR and frames to enter
> deep sleep.
> 
I am seeing that the block counts are zero even when a flip is done
with PSR2_STATUS showing SLEEP state. Not sure what's going on.

> > 
> > 
> > > 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.
> > 
> > This patch warrants some explanation about the timeouts used and
> > how
> > they were selected.
> 
> The flip timeout is a value to achieve 15hz flips and the error 
> timeout
> will be removed as you suggested bellow, I will add a comment when
> setting the interval to the flip timerfd.
> 
> > 
> > > v2: Using new SU blocks debugfs output
> > > 
> > > 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          |  23 +++
> > >  lib/igt_psr.h          |   1 +
> > >  tests/Makefile.sources |   1 +
> > >  tests/kms_psr2_su.c    | 362
> > > +++++++++++++++++++++++++++++++++++++++++
> > >  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 0f4b2e23..a8b9e4ce 100644
> > > --- a/lib/igt_psr.c
> > > +++ b/lib/igt_psr.c
> > > @@ -234,3 +234,26 @@ bool psr2_wait_update(int debugfs_fd)
> > >  {
> > >  	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
> > >  }
> > > +
> > > +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> > > +
> > > +bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> > > *num_su_blocks)
> > > +{
> > > +	char buf[PSR_STATUS_MAX_LEN];
> > > +	char *str;
> > > +	int ret;
> > > +
> > > +	ret = igt_debugfs_simple_read(debugfs_fd,
> > > "i915_edp_psr_status", buf,
> > > +				      sizeof(buf));
> > > +	if (ret < 0)
> > > +		return false;
> > > +
> > > +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> > > +	if (!str)
> > > +		return false;
> > > +
> > > +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> > > +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> > > +
> > > +	return true;
> > > +}
> > > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > > index 2b8d1a90..349eb0c2 100644
> > > --- a/lib/igt_psr.h
> > > +++ b/lib/igt_psr.h
> > > @@ -41,5 +41,6 @@ bool psr2_wait_update(int debugfs_fd);
> > >  bool psr2_enable(int debugfs_fd);
> > >  bool psr2_disable(int debugfs_fd);
> > >  bool psr2_sink_support(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..67c02f25
> > > --- /dev/null
> > > +++ b/tests/kms_psr2_su.c
> > > @@ -0,0 +1,362 @@
> > > +/*
> > > + * 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 lines 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);
> > > +	}
> > > +
> > 
> > What happens if you start this thread and then flip just once? Do
> > you
> > see the expected block count in debugfs at some instance after the
> > flip?
> 
> As said above no, as we wait for DEEP_SLEEP state and even in SLEEP
> state sometimes HW decides to go to PSR exit and PSR active flow
> instead of do a selective update.
> 
> > 
> > > +	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_PRIM
> > > ARY);
> > > +
> > > +		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 */
> > 
> > Wouldn't it be simpler to flip a fixed number of times?
> 
> Yeah it would be simpler, I will change to that.
> 
> > 
> > 
> > > +		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_sink_support(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_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;
> > 
> > What's the rationale for this number?
> 
> 
> 15hz flips, I will add a comment here.
Right, why 15 Hz? :) I'd like the commit message to explain that.

> 
> 
> 
> > 
> > > +		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',

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