[igt-dev] [PATCH i-g-t 1/4] tests/frontbuffer_tracking: Do not test sink crc

[igt-dev] [PATCH i-g-t 1/4] tests/frontbuffer_tracking: Do not test sink crc

[Dhinakaran Pandiyan]

eDP sink crc reads use vblank interrupts that cause PSR exit and
therefore makes them unsuitable for PSR testing. Besides that, reading
sink CRC via the AUX channel for testing when the HW also is most likely
is going to be using AUX channel is a recipe for inconsistent test
results. Thirdly, CRC's have been seen to be noisy or vary across sinks for
the same driver and test code. We tradeoff the ability to validate what the
sink is displaying for correctness.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 143 ++++++---------------------------------
 1 file changed, 21 insertions(+), 122 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index dbb8ba62..116a95bc 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -184,32 +184,12 @@ struct {
 	.can_test = false,
 };
 
-#define SINK_CRC_SIZE 12
-typedef struct {
-	char data[SINK_CRC_SIZE];
-} sink_crc_t;
-
-struct both_crcs {
-	igt_crc_t pipe;
-	sink_crc_t sink;
-};
-
 igt_pipe_crc_t *pipe_crc;
+igt_crc_t *wanted_crc;
 struct {
 	bool initialized;
-	struct both_crcs crc;
+	igt_crc_t crc;
 } blue_crcs[FORMAT_COUNT];
-struct both_crcs *wanted_crc;
-
-struct {
-	int fd;
-	bool supported;
-	bool reliable;
-} sink_crc = {
-	.fd = -1,
-	.supported = true,
-	.reliable = true,
-};
 
 /* The goal of this structure is to easily allow us to deal with cases where we
  * have a big framebuffer and the CRTC is just displaying a subregion of this
@@ -229,7 +209,7 @@ struct draw_pattern_info {
 	struct rect (*get_rect)(struct fb_region *fb, int r);
 
 	bool initialized[FORMAT_COUNT];
-	struct both_crcs *crcs[FORMAT_COUNT];
+	igt_crc_t *crcs[FORMAT_COUNT];
 };
 
 /* Draw big rectangles on the screen. */
@@ -991,44 +971,6 @@ static bool drrs_wait_until_rr_switch_to_low(void)
 #define drrs_enable()	drrs_set(1)
 #define drrs_disable()	drrs_set(0)
 
-static void get_sink_crc(sink_crc_t *crc, bool mandatory)
-{
-	int rc, errno_;
-
-	if (!sink_crc.supported) {
-		memcpy(crc, "unsupported!", SINK_CRC_SIZE);
-		return;
-	}
-
-	lseek(sink_crc.fd, 0, SEEK_SET);
-
-	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
-	errno_ = errno;
-
-	if (rc == -1 && errno_ == ENOTTY) {
-		igt_info("Sink CRC not supported: panel doesn't support it\n");
-		sink_crc.supported = false;
-	} else if (rc == -1 && errno_ == ETIMEDOUT) {
-		if (sink_crc.reliable) {
-			igt_info("Sink CRC is unreliable on this machine.\n");
-			sink_crc.reliable = false;
-		}
-
-		if (mandatory)
-			igt_skip("Sink CRC is unreliable on this machine.\n");
-	} else {
-		igt_assert_f(rc != -1, "Unexpected error: %d\n", errno_);
-		igt_assert(rc == SINK_CRC_SIZE);
-	}
-}
-
-static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
-{
-	return (memcmp(a->data, b->data, SINK_CRC_SIZE) == 0);
-}
-
-#define assert_sink_crc_equal(a, b) igt_assert(sink_crc_equal(a, b))
-
 static struct rect pat1_get_rect(struct fb_region *fb, int r)
 {
 	struct rect rect;
@@ -1262,30 +1204,23 @@ static void stop_busy_thread(void)
 	}
 }
 
-static void print_crc(const char *str, struct both_crcs *crc)
+static void print_crc(const char *str, igt_crc_t *crc)
 {
-	int i;
 	char *pipe_str;
 
-	pipe_str = igt_crc_to_string(&crc->pipe);
+	pipe_str = igt_crc_to_string(crc);
 
-	igt_debug("%s pipe:[%s] sink:[", str, pipe_str);
-	for (i = 0; i < SINK_CRC_SIZE; i++)
-		igt_debug("%c", crc->sink.data[i]);
-	igt_debug("]\n");
+	igt_debug("%s pipe:[%s]\n", str, pipe_str);
 
 	free(pipe_str);
 }
 
-static void collect_crcs(struct both_crcs *crcs, bool mandatory_sink_crc)
+static void collect_crc(igt_crc_t *crc)
 {
-	igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
-	get_sink_crc(&crcs->sink, mandatory_sink_crc);
+	igt_pipe_crc_collect_crc(pipe_crc, crc);
 }
 
-static void setup_sink_crc(void);
-
-static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
+static void init_blue_crc(enum pixel_format format)
 {
 	struct igt_fb blue;
 
@@ -1306,11 +1241,9 @@ static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
 	if (!pipe_crc) {
 		pipe_crc = igt_pipe_crc_new(drm.fd, prim_mode_params.pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 		igt_assert(pipe_crc);
-
-		setup_sink_crc();
 	}
 
-	collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
+	collect_crc(&blue_crcs[format].crc);
 
 	print_crc("Blue CRC:  ", &blue_crcs[format].crc);
 
@@ -1322,8 +1255,7 @@ static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
 }
 
 static void init_crcs(enum pixel_format format,
-		      struct draw_pattern_info *pattern,
-		      bool mandatory_sink_crc)
+		      struct draw_pattern_info *pattern)
 {
 	int r, r_;
 	struct igt_fb tmp_fbs[pattern->n_rects];
@@ -1359,7 +1291,7 @@ static void init_crcs(enum pixel_format format,
 		igt_plane_set_fb(prim_mode_params.primary.plane, &tmp_fbs[r]);
 		igt_display_commit(&drm.display);
 
-		collect_crcs(&pattern->crcs[format][r], mandatory_sink_crc);
+		collect_crc(&pattern->crcs[format][r]);
 	}
 
 	for (r = 0; r < pattern->n_rects; r++) {
@@ -1412,25 +1344,6 @@ static void teardown_modeset(void)
 		destroy_fbs(f);
 }
 
-static void setup_sink_crc(void)
-{
-	sink_crc_t crc;
-	drmModeConnectorPtr c;
-
-	c = prim_mode_params.output->config.connector;
-	if (c->connector_type != DRM_MODE_CONNECTOR_eDP) {
-		igt_info("Sink CRC not supported: primary screen is not eDP\n");
-		sink_crc.supported = false;
-		return;
-	}
-
-	sink_crc.fd = openat(drm.debugfs, "i915_sink_crc_eDP1", O_RDONLY);
-	igt_assert_lte(0, sink_crc.fd);
-
-	/* Do a first read to try to detect if it's supported. */
-	get_sink_crc(&crc, false);
-}
-
 static void setup_crcs(void)
 {
 	enum pixel_format f;
@@ -1486,9 +1399,6 @@ static void teardown_crcs(void)
 			free(pattern4.crcs[f]);
 	}
 
-	if (sink_crc.fd != -1)
-		close(sink_crc.fd);
-
 	igt_pipe_crc_free(pipe_crc);
 }
 
@@ -1695,23 +1605,18 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	return flags;
 }
 
-static void do_crc_assertions(int flags, bool mandatory_sink_crc)
+static void do_crc_assertions(int flags)
 {
-	struct both_crcs crc;
+	igt_crc_t crc;
 
 	if (!opt.check_crc || (flags & DONT_ASSERT_CRC))
 		return;
 
-	collect_crcs(&crc, mandatory_sink_crc);
+	collect_crc(&crc);
 	print_crc("Calculated CRC:", &crc);
 
 	igt_assert(wanted_crc);
-	igt_assert_crc_equal(&crc.pipe, &wanted_crc->pipe);
-	if (mandatory_sink_crc)
-		assert_sink_crc_equal(&crc.sink, &wanted_crc->sink);
-	else if (sink_crc.reliable &&
-		 !sink_crc_equal(&crc.sink, &wanted_crc->sink))
-		igt_info("Sink CRC differ, but not required\n");
+	igt_assert_crc_equal(&crc, wanted_crc);
 }
 
 static void do_status_assertions(int flags)
@@ -1770,7 +1675,6 @@ static void do_status_assertions(int flags)
 static void __do_assertions(const struct test_mode *t, int flags,
 			    int line)
 {
-	bool mandatory_sink_crc = t->feature & FEATURE_PSR;
 	flags = adjust_assertion_flags(t, flags);
 
 	igt_debug("checking asserts in line %i\n", line);
@@ -1779,7 +1683,7 @@ static void __do_assertions(const struct test_mode *t, int flags,
 
 	/* Check the CRC to make sure the drawing operations work
 	 * immediately, independently of the features being enabled. */
-	do_crc_assertions(flags, mandatory_sink_crc);
+	do_crc_assertions(flags);
 
 	/* Now we can flush things to make the test faster. */
 	do_flush(t);
@@ -1792,7 +1696,7 @@ static void __do_assertions(const struct test_mode *t, int flags,
 	 * would only delay the test suite while adding no value to the
 	 * test suite. */
 	if (t->screen == SCREEN_PRIM)
-		do_crc_assertions(flags, mandatory_sink_crc);
+		do_crc_assertions(flags);
 
 	if (fbc.supports_last_action && opt.fbc_check_last_action) {
 		if (flags & ASSERT_LAST_ACTION_CHANGED)
@@ -1865,8 +1769,6 @@ static void check_test_requirements(const struct test_mode *t)
 	if (t->feature & FEATURE_PSR) {
 		igt_require_f(psr.can_test,
 			      "Can't test PSR with the current outputs\n");
-		igt_require_f(sink_crc.supported,
-			      "Can't test PSR without sink CRCs\n");
 	}
 
 	if (t->feature & FEATURE_DRRS)
@@ -1945,9 +1847,9 @@ static void prepare_subtest_data(const struct test_mode *t,
 
 	unset_all_crtcs();
 
-	init_blue_crc(t->format, t->feature & FEATURE_PSR);
+	init_blue_crc(t->format);
 	if (pattern)
-		init_crcs(t->format, pattern, t->feature & FEATURE_PSR);
+		init_crcs(t->format, pattern);
 
 	enable_features_for_test(t);
 }
@@ -2016,7 +1918,7 @@ static void rte_subtest(const struct test_mode *t)
 	set_region_for_test(t, &scnd_mode_params.sprite);
 }
 
-static void update_wanted_crc(const struct test_mode *t, struct both_crcs *crc)
+static void update_wanted_crc(const struct test_mode *t, igt_crc_t *crc)
 {
 	if (t->screen == SCREEN_PRIM)
 		wanted_crc = crc;
@@ -3140,9 +3042,6 @@ static void tilingchange_subtest(const struct test_mode *t)
  *   If you get a failure here, you should run the more specific draw and flip
  *   subtests of each feature in order to discover what exactly is failing and
  *   why.
- *
- * TODO: do sink CRC assertions in case sink_crc.supported. Only do this after
- * our sink CRC code gets 100% reliable, in order to avoid CI false negatives.
  */
 static void basic_subtest(const struct test_mode *t)
 {
-- 
2.14.1

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

[igt-dev] [PATCH i-g-t 2/4] tests/kms_psr_sink_crc: Do not test sink crc

[Dhinakaran Pandiyan]

eDP sink crc reads use vblank interrupts that cause PSR exit and
therefore makes them unsuitable for PSR testing. Besides that, reading
sink CRC via the AUX channel for testing when the HW also is most likely
is going to be using AUX channel is a recipe for inconsistent test
results. Thirdly, CRC's have been seen to be noisy/inconsistent across
sinks. We tradeoff the ability to validate what the sink is displaying
for correctness.

We also make use of source PSR status register to check whether HW tracking
triggered PSR exit upon an exit event.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 60 ++++++++++++------------------------------------
 1 file changed, 15 insertions(+), 45 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 3115a5de..4a18609a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -28,12 +28,8 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
-
 #include "intel_bufmgr.h"
 
-#define CRC_BLACK "000000000000"
-#define CRC_LEN 12
-
 enum operations {
 	PAGE_FLIP,
 	MMAP_GTT,
@@ -222,36 +218,17 @@ static bool wait_psr_entry(data_t *data)
 	return false;
 }
 
-static void get_sink_crc(data_t *data, char *crc)
+static bool psr_inactive(data_t *data)
 {
-	if (igt_interactive_debug)
-		return;
-
-	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
-				     crc, CRC_LEN) == CRC_LEN,
-		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
-	igt_debug("sink CRC: %.*s\n", CRC_LEN, crc);
-
-	/* Black screen is always invalid */
-	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
-}
+	char buf[512];
 
-static bool is_green(char *crc)
-{
-	const char *mask = "0000FFFF0000";
-	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
-	if (igt_interactive_debug)
-		return false;
-
-	/* Check R and B components are 0 and G is non-zero */
-	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
-	       (*(p + 1) & *(mask_p + 1)) != 0;
+	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
+	return !(strstr(buf, "SRDENT") || strstr("SLEEP"));
 }
 
-static void assert_or_manual(bool condition, const char *expected)
+static inline void manual(const char *expected)
 {
-	igt_debug_manual_check("no-crc", expected);
-	igt_assert(igt_interactive_debug || condition);
+	igt_debug_manual_check("all", expected);
 }
 
 static bool drrs_disabled(data_t *data)
@@ -268,39 +245,32 @@ static void run_test(data_t *data)
 	uint32_t handle = data->fb_white.gem_handle;
 	igt_plane_t *test_plane = data->test_plane;
 	void *ptr;
-	char ref_crc[CRC_LEN];
-	char crc[CRC_LEN];
 	const char *expected = "";
 
 	/* Confirm that screen became Green */
-	get_sink_crc(data, ref_crc);
-	assert_or_manual(is_green(ref_crc), "screen GREEN");
+	manual("screen GREEN");
 
 	/* Confirm screen stays Green after PSR got active */
 	igt_assert(wait_psr_entry(data));
-	get_sink_crc(data, ref_crc);
-	assert_or_manual(is_green(ref_crc), "screen GREEN");
+	manual("screen GREEN");
 
 	/* Setting a secondary fb/plane */
 	igt_plane_set_fb(test_plane, &data->fb_white);
 	igt_display_commit(&data->display);
 
 	/* Confirm it is not Green anymore */
-	igt_assert(wait_psr_entry(data));
-	get_sink_crc(data, ref_crc);
 	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
-		assert_or_manual(!is_green(ref_crc), "screen WHITE");
+		manual("screen WHITE");
 	else
-		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
+		manual("GREEN background with WHITE box");
 
+	igt_assert(wait_psr_entry(data));
 	switch (data->op) {
 	case PAGE_FLIP:
 		/* Only in use when testing primary plane */
 		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
 					   data->fb_green.fb_id, 0, NULL) == 0);
-		get_sink_crc(data, crc);
-		assert_or_manual(is_green(crc), "screen GREEN");
-		expected = "still GREEN";
+		expected = "GREEN";
 		break;
 	case MMAP_GTT:
 		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -342,8 +312,8 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
-	get_sink_crc(data, crc);
-	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
+	assert(psr_inactive(data));
+	manual(expected);
 }
 
 static void test_cleanup(data_t *data) {
@@ -444,7 +414,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 to check the CRC test logic.";
+	       "  --no-psr\tRun test without PSR.";
 	static struct option long_options[] = {
 		{"no-psr", 0, 0, 'n'},
 		{ 0, 0, 0, 0 }
-- 
2.14.1

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

[igt-dev] [PATCH i-g-t 3/4] tests/kms_psr_sink_crc: Test PSR source HW status before PSR entry.

[Dhinakaran Pandiyan]

Wait for PSR HW status to show active before starting an PSR exit action.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 4a18609a..4eca51da 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -197,33 +197,24 @@ static bool sink_support(data_t *data)
 		strstr(buf, "Sink_Support: yes\n");
 }
 
-static bool psr_enabled(data_t *data)
+static bool psr_active(data_t *data, bool check_active)
 {
+	bool active;
 	char buf[512];
 
 	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 
-	return data->with_psr_disabled ||
-		strstr(buf, "HW Enabled & Active bit: yes\n");
+	active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
+		 (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
+	return check_active ? active : !active;
 }
 
 static bool wait_psr_entry(data_t *data)
 {
-	int timeout = 5;
-	while (timeout--) {
-		if (psr_enabled(data))
-			return true;
-		sleep(1);
-	}
-	return false;
-}
+	if (data->with_psr_disabled)
+		return true;
 
-static bool psr_inactive(data_t *data)
-{
-	char buf[512];
-
-	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
-	return !(strstr(buf, "SRDENT") || strstr("SLEEP"));
+	return igt_wait((psr_active(data, true)), 500, 1);
 }
 
 static inline void manual(const char *expected)
@@ -312,7 +303,7 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
-	assert(psr_inactive(data));
+	igt_assert(psr_active(data, false));
 	manual(expected);
 }
 
-- 
2.14.1

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

[igt-dev] [PATCH i-g-t 4/4] tests/psr: Rename kms_psr_sink_crc.c to kms_psr.c

[Dhinakaran Pandiyan]

We don't use sink CRC anymore in this test.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/Makefile.sources                  | 2 +-
 tests/intel-ci/fast-feedback.testlist   | 2 +-
 tests/{kms_psr_sink_crc.c => kms_psr.c} | 0
 tests/meson.build                       | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename tests/{kms_psr_sink_crc.c => kms_psr.c} (100%)

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 54b4a3c2..8fec6ffe 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -199,7 +199,7 @@ TESTS_progs = \
 	kms_plane_multiple \
 	kms_plane_scaling \
 	kms_properties \
-	kms_psr_sink_crc \
+	kms_psr \
 	kms_pwrite_crc \
 	kms_rmfb \
 	kms_rotation_crc \
diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index b08ef770..aa861d39 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -243,7 +243,7 @@ igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
-igt@kms_psr_sink_crc@basic
+igt@kms_psr@basic
 igt@kms_setmode@basic-clone-single-crtc
 igt@kms_sink_crc_basic
 igt@pm_backlight@basic-brightness
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr.c
similarity index 100%
rename from tests/kms_psr_sink_crc.c
rename to tests/kms_psr.c
diff --git a/tests/meson.build b/tests/meson.build
index 029b8043..704bd13f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -175,7 +175,7 @@ test_progs = [
 	'kms_plane_multiple',
 	'kms_plane_scaling',
 	'kms_properties',
-	'kms_psr_sink_crc',
+	'kms_psr',
 	'kms_pwrite_crc',
 	'kms_rmfb',
 	'kms_rotation_crc',
-- 
2.14.1

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] tests/frontbuffer_tracking: Do not test sink crc

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] tests/frontbuffer_tracking: Do not test sink crc
URL   : https://patchwork.freedesktop.org/series/46364/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4474 -> IGTPW_1561 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (45 -> 38) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-ctg-p8600 fi-glk-j4005 fi-pnv-d510 


== Build changes ==

    * IGT: IGT_4549 -> IGTPW_1561

  CI_DRM_4474: c298e0700edde3d016ae9b16d0ce2a098bee1022 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1561: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1561/
  IGT_4549: e19fd5549e9cf603251704117fc64f4068be5016 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_psr@basic
+igt@kms_psr@cursor_blt
+igt@kms_psr@cursor_mmap_cpu
+igt@kms_psr@cursor_mmap_gtt
+igt@kms_psr@cursor_plane_move
+igt@kms_psr@cursor_plane_onoff
+igt@kms_psr@cursor_render
+igt@kms_psr@dpms
+igt@kms_psr@no_drrs
+igt@kms_psr@primary_blt
+igt@kms_psr@primary_mmap_cpu
+igt@kms_psr@primary_mmap_gtt
+igt@kms_psr@primary_page_flip
+igt@kms_psr@primary_render
+igt@kms_psr@sprite_blt
+igt@kms_psr@sprite_mmap_cpu
+igt@kms_psr@sprite_mmap_gtt
+igt@kms_psr@sprite_plane_move
+igt@kms_psr@sprite_plane_onoff
+igt@kms_psr@sprite_render
+igt@kms_psr@suspend
-igt@kms_psr_sink_crc@basic
-igt@kms_psr_sink_crc@cursor_blt
-igt@kms_psr_sink_crc@cursor_mmap_cpu
-igt@kms_psr_sink_crc@cursor_mmap_gtt
-igt@kms_psr_sink_crc@cursor_plane_move
-igt@kms_psr_sink_crc@cursor_plane_onoff
-igt@kms_psr_sink_crc@cursor_render
-igt@kms_psr_sink_crc@dpms
-igt@kms_psr_sink_crc@no_drrs
-igt@kms_psr_sink_crc@primary_blt
-igt@kms_psr_sink_crc@primary_mmap_cpu
-igt@kms_psr_sink_crc@primary_mmap_gtt
-igt@kms_psr_sink_crc@primary_page_flip
-igt@kms_psr_sink_crc@primary_render
-igt@kms_psr_sink_crc@sprite_blt
-igt@kms_psr_sink_crc@sprite_mmap_cpu
-igt@kms_psr_sink_crc@sprite_mmap_gtt
-igt@kms_psr_sink_crc@sprite_plane_move
-igt@kms_psr_sink_crc@sprite_plane_onoff
-igt@kms_psr_sink_crc@sprite_render
-igt@kms_psr_sink_crc@suspend

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/4] tests/frontbuffer_tracking: Do not test sink crc

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] tests/frontbuffer_tracking: Do not test sink crc
URL   : https://patchwork.freedesktop.org/series/46364/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4549_full -> IGTPW_1561_full =

== Summary - WARNING ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          PASS -> SKIP +2

    igt@gem_mocs_settings@mocs-rc6-bsd2:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_fbcon_fbt@fbc-suspend:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

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

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105189)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#107161)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#107161)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925)

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

    
    ==== Possible fixes ====

    igt@gem_ctx_switch@basic-default-heavy:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_cursor_legacy@all-pipes-torture-move:
      shard-glk:          DMESG-WARN -> PASS
      shard-apl:          DMESG-WARN -> PASS
      shard-kbl:          DMESG-WARN -> PASS
      shard-hsw:          DMESG-WARN -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#107161) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#107161 https://bugs.freedesktop.org/show_bug.cgi?id=107161
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4549 -> IGTPW_1561
    * Linux: CI_DRM_4469 -> CI_DRM_4474

  CI_DRM_4469: 02e578b7aace48d33fa617dddb40621bd664c92c @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4474: c298e0700edde3d016ae9b16d0ce2a098bee1022 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1561: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1561/
  IGT_4549: e19fd5549e9cf603251704117fc64f4068be5016 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 1/4] tests/frontbuffer_tracking: Do not test sink crc

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 01:09:40AM -0700, Dhinakaran Pandiyan wrote:
> eDP sink crc reads use vblank interrupts that cause PSR exit and
> therefore makes them unsuitable for PSR testing. Besides that, reading
> sink CRC via the AUX channel for testing when the HW also is most likely
> is going to be using AUX channel is a recipe for inconsistent test
> results. Thirdly, CRC's have been seen to be noisy or vary across sinks for
> the same driver and test code. We tradeoff the ability to validate what the
> sink is displaying for correctness.
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  tests/kms_frontbuffer_tracking.c | 143 ++++++---------------------------------
>  1 file changed, 21 insertions(+), 122 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index dbb8ba62..116a95bc 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -184,32 +184,12 @@ struct {
>  	.can_test = false,
>  };
>  
> -#define SINK_CRC_SIZE 12
> -typedef struct {
> -	char data[SINK_CRC_SIZE];
> -} sink_crc_t;
> -
> -struct both_crcs {
> -	igt_crc_t pipe;
> -	sink_crc_t sink;
> -};
> -
>  igt_pipe_crc_t *pipe_crc;
> +igt_crc_t *wanted_crc;
>  struct {
>  	bool initialized;
> -	struct both_crcs crc;
> +	igt_crc_t crc;
>  } blue_crcs[FORMAT_COUNT];
> -struct both_crcs *wanted_crc;
> -
> -struct {
> -	int fd;
> -	bool supported;
> -	bool reliable;
> -} sink_crc = {
> -	.fd = -1,
> -	.supported = true,
> -	.reliable = true,
> -};
>  
>  /* The goal of this structure is to easily allow us to deal with cases where we
>   * have a big framebuffer and the CRTC is just displaying a subregion of this
> @@ -229,7 +209,7 @@ struct draw_pattern_info {
>  	struct rect (*get_rect)(struct fb_region *fb, int r);
>  
>  	bool initialized[FORMAT_COUNT];
> -	struct both_crcs *crcs[FORMAT_COUNT];
> +	igt_crc_t *crcs[FORMAT_COUNT];
>  };
>  
>  /* Draw big rectangles on the screen. */
> @@ -991,44 +971,6 @@ static bool drrs_wait_until_rr_switch_to_low(void)
>  #define drrs_enable()	drrs_set(1)
>  #define drrs_disable()	drrs_set(0)
>  
> -static void get_sink_crc(sink_crc_t *crc, bool mandatory)
> -{
> -	int rc, errno_;
> -
> -	if (!sink_crc.supported) {
> -		memcpy(crc, "unsupported!", SINK_CRC_SIZE);
> -		return;
> -	}
> -
> -	lseek(sink_crc.fd, 0, SEEK_SET);
> -
> -	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> -	errno_ = errno;
> -
> -	if (rc == -1 && errno_ == ENOTTY) {
> -		igt_info("Sink CRC not supported: panel doesn't support it\n");
> -		sink_crc.supported = false;
> -	} else if (rc == -1 && errno_ == ETIMEDOUT) {
> -		if (sink_crc.reliable) {
> -			igt_info("Sink CRC is unreliable on this machine.\n");
> -			sink_crc.reliable = false;
> -		}
> -
> -		if (mandatory)
> -			igt_skip("Sink CRC is unreliable on this machine.\n");
> -	} else {
> -		igt_assert_f(rc != -1, "Unexpected error: %d\n", errno_);
> -		igt_assert(rc == SINK_CRC_SIZE);
> -	}
> -}
> -
> -static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
> -{
> -	return (memcmp(a->data, b->data, SINK_CRC_SIZE) == 0);
> -}
> -
> -#define assert_sink_crc_equal(a, b) igt_assert(sink_crc_equal(a, b))
> -
>  static struct rect pat1_get_rect(struct fb_region *fb, int r)
>  {
>  	struct rect rect;
> @@ -1262,30 +1204,23 @@ static void stop_busy_thread(void)
>  	}
>  }
>  
> -static void print_crc(const char *str, struct both_crcs *crc)
> +static void print_crc(const char *str, igt_crc_t *crc)
>  {
> -	int i;
>  	char *pipe_str;
>  
> -	pipe_str = igt_crc_to_string(&crc->pipe);
> +	pipe_str = igt_crc_to_string(crc);
>  
> -	igt_debug("%s pipe:[%s] sink:[", str, pipe_str);
> -	for (i = 0; i < SINK_CRC_SIZE; i++)
> -		igt_debug("%c", crc->sink.data[i]);
> -	igt_debug("]\n");
> +	igt_debug("%s pipe:[%s]\n", str, pipe_str);
>  
>  	free(pipe_str);
>  }
>  
> -static void collect_crcs(struct both_crcs *crcs, bool mandatory_sink_crc)
> +static void collect_crc(igt_crc_t *crc)
>  {
> -	igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
> -	get_sink_crc(&crcs->sink, mandatory_sink_crc);
> +	igt_pipe_crc_collect_crc(pipe_crc, crc);
>  }
>  
> -static void setup_sink_crc(void);
> -
> -static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
> +static void init_blue_crc(enum pixel_format format)
>  {
>  	struct igt_fb blue;
>  
> @@ -1306,11 +1241,9 @@ static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
>  	if (!pipe_crc) {
>  		pipe_crc = igt_pipe_crc_new(drm.fd, prim_mode_params.pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
>  		igt_assert(pipe_crc);
> -
> -		setup_sink_crc();
>  	}
>  
> -	collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
> +	collect_crc(&blue_crcs[format].crc);
>  
>  	print_crc("Blue CRC:  ", &blue_crcs[format].crc);
>  
> @@ -1322,8 +1255,7 @@ static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
>  }
>  
>  static void init_crcs(enum pixel_format format,
> -		      struct draw_pattern_info *pattern,
> -		      bool mandatory_sink_crc)
> +		      struct draw_pattern_info *pattern)
>  {
>  	int r, r_;
>  	struct igt_fb tmp_fbs[pattern->n_rects];
> @@ -1359,7 +1291,7 @@ static void init_crcs(enum pixel_format format,
>  		igt_plane_set_fb(prim_mode_params.primary.plane, &tmp_fbs[r]);
>  		igt_display_commit(&drm.display);
>  
> -		collect_crcs(&pattern->crcs[format][r], mandatory_sink_crc);
> +		collect_crc(&pattern->crcs[format][r]);
>  	}
>  
>  	for (r = 0; r < pattern->n_rects; r++) {
> @@ -1412,25 +1344,6 @@ static void teardown_modeset(void)
>  		destroy_fbs(f);
>  }
>  
> -static void setup_sink_crc(void)
> -{
> -	sink_crc_t crc;
> -	drmModeConnectorPtr c;
> -
> -	c = prim_mode_params.output->config.connector;
> -	if (c->connector_type != DRM_MODE_CONNECTOR_eDP) {
> -		igt_info("Sink CRC not supported: primary screen is not eDP\n");
> -		sink_crc.supported = false;
> -		return;
> -	}
> -
> -	sink_crc.fd = openat(drm.debugfs, "i915_sink_crc_eDP1", O_RDONLY);
> -	igt_assert_lte(0, sink_crc.fd);
> -
> -	/* Do a first read to try to detect if it's supported. */
> -	get_sink_crc(&crc, false);
> -}
> -
>  static void setup_crcs(void)
>  {
>  	enum pixel_format f;
> @@ -1486,9 +1399,6 @@ static void teardown_crcs(void)
>  			free(pattern4.crcs[f]);
>  	}
>  
> -	if (sink_crc.fd != -1)
> -		close(sink_crc.fd);
> -
>  	igt_pipe_crc_free(pipe_crc);
>  }
>  
> @@ -1695,23 +1605,18 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>  	return flags;
>  }
>  
> -static void do_crc_assertions(int flags, bool mandatory_sink_crc)
> +static void do_crc_assertions(int flags)
>  {
> -	struct both_crcs crc;
> +	igt_crc_t crc;
>  
>  	if (!opt.check_crc || (flags & DONT_ASSERT_CRC))
>  		return;
>  
> -	collect_crcs(&crc, mandatory_sink_crc);
> +	collect_crc(&crc);
>  	print_crc("Calculated CRC:", &crc);
>  
>  	igt_assert(wanted_crc);
> -	igt_assert_crc_equal(&crc.pipe, &wanted_crc->pipe);
> -	if (mandatory_sink_crc)
> -		assert_sink_crc_equal(&crc.sink, &wanted_crc->sink);
> -	else if (sink_crc.reliable &&
> -		 !sink_crc_equal(&crc.sink, &wanted_crc->sink))
> -		igt_info("Sink CRC differ, but not required\n");
> +	igt_assert_crc_equal(&crc, wanted_crc);
>  }
>  
>  static void do_status_assertions(int flags)
> @@ -1770,7 +1675,6 @@ static void do_status_assertions(int flags)
>  static void __do_assertions(const struct test_mode *t, int flags,
>  			    int line)
>  {
> -	bool mandatory_sink_crc = t->feature & FEATURE_PSR;
>  	flags = adjust_assertion_flags(t, flags);
>  
>  	igt_debug("checking asserts in line %i\n", line);
> @@ -1779,7 +1683,7 @@ static void __do_assertions(const struct test_mode *t, int flags,
>  
>  	/* Check the CRC to make sure the drawing operations work
>  	 * immediately, independently of the features being enabled. */
> -	do_crc_assertions(flags, mandatory_sink_crc);
> +	do_crc_assertions(flags);
>  
>  	/* Now we can flush things to make the test faster. */
>  	do_flush(t);
> @@ -1792,7 +1696,7 @@ static void __do_assertions(const struct test_mode *t, int flags,
>  	 * would only delay the test suite while adding no value to the
>  	 * test suite. */
>  	if (t->screen == SCREEN_PRIM)
> -		do_crc_assertions(flags, mandatory_sink_crc);
> +		do_crc_assertions(flags);
>  
>  	if (fbc.supports_last_action && opt.fbc_check_last_action) {
>  		if (flags & ASSERT_LAST_ACTION_CHANGED)
> @@ -1865,8 +1769,6 @@ static void check_test_requirements(const struct test_mode *t)
>  	if (t->feature & FEATURE_PSR) {
>  		igt_require_f(psr.can_test,
>  			      "Can't test PSR with the current outputs\n");
> -		igt_require_f(sink_crc.supported,
> -			      "Can't test PSR without sink CRCs\n");
>  	}
>  
>  	if (t->feature & FEATURE_DRRS)
> @@ -1945,9 +1847,9 @@ static void prepare_subtest_data(const struct test_mode *t,
>  
>  	unset_all_crtcs();
>  
> -	init_blue_crc(t->format, t->feature & FEATURE_PSR);
> +	init_blue_crc(t->format);
>  	if (pattern)
> -		init_crcs(t->format, pattern, t->feature & FEATURE_PSR);
> +		init_crcs(t->format, pattern);
>  
>  	enable_features_for_test(t);
>  }
> @@ -2016,7 +1918,7 @@ static void rte_subtest(const struct test_mode *t)
>  	set_region_for_test(t, &scnd_mode_params.sprite);
>  }
>  
> -static void update_wanted_crc(const struct test_mode *t, struct both_crcs *crc)
> +static void update_wanted_crc(const struct test_mode *t, igt_crc_t *crc)
>  {
>  	if (t->screen == SCREEN_PRIM)
>  		wanted_crc = crc;
> @@ -3140,9 +3042,6 @@ static void tilingchange_subtest(const struct test_mode *t)
>   *   If you get a failure here, you should run the more specific draw and flip
>   *   subtests of each feature in order to discover what exactly is failing and
>   *   why.
> - *
> - * TODO: do sink CRC assertions in case sink_crc.supported. Only do this after
> - * our sink CRC code gets 100% reliable, in order to avoid CI false negatives.
>   */
>  static void basic_subtest(const struct test_mode *t)
>  {
> -- 
> 2.14.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/4] tests/kms_psr_sink_crc: Do not test sink crc

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 01:09:41AM -0700, Dhinakaran Pandiyan wrote:
> eDP sink crc reads use vblank interrupts that cause PSR exit and
> therefore makes them unsuitable for PSR testing. Besides that, reading
> sink CRC via the AUX channel for testing when the HW also is most likely
> is going to be using AUX channel is a recipe for inconsistent test
> results. Thirdly, CRC's have been seen to be noisy/inconsistent across
> sinks. We tradeoff the ability to validate what the sink is displaying
> for correctness.
> 
> We also make use of source PSR status register to check whether HW tracking
> triggered PSR exit upon an exit event.
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  tests/kms_psr_sink_crc.c | 60 ++++++++++++------------------------------------
>  1 file changed, 15 insertions(+), 45 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 3115a5de..4a18609a 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -28,12 +28,8 @@
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <string.h>
> -
>  #include "intel_bufmgr.h"
>  
> -#define CRC_BLACK "000000000000"
> -#define CRC_LEN 12
> -
>  enum operations {
>  	PAGE_FLIP,
>  	MMAP_GTT,
> @@ -222,36 +218,17 @@ static bool wait_psr_entry(data_t *data)
>  	return false;
>  }
>  
> -static void get_sink_crc(data_t *data, char *crc)
> +static bool psr_inactive(data_t *data)
>  {
> -	if (igt_interactive_debug)
> -		return;
> -
> -	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
> -				     crc, CRC_LEN) == CRC_LEN,
> -		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> -	igt_debug("sink CRC: %.*s\n", CRC_LEN, crc);
> -
> -	/* Black screen is always invalid */
> -	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
> -}
> +	char buf[512];
>  
> -static bool is_green(char *crc)
> -{
> -	const char *mask = "0000FFFF0000";
> -	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
> -	if (igt_interactive_debug)
> -		return false;
> -
> -	/* Check R and B components are 0 and G is non-zero */
> -	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
> -	       (*(p + 1) & *(mask_p + 1)) != 0;
> +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> +	return !(strstr(buf, "SRDENT") || strstr("SLEEP"));
>  }
>  
> -static void assert_or_manual(bool condition, const char *expected)
> +static inline void manual(const char *expected)
>  {
> -	igt_debug_manual_check("no-crc", expected);
> -	igt_assert(igt_interactive_debug || condition);
> +	igt_debug_manual_check("all", expected);
>  }
>  
>  static bool drrs_disabled(data_t *data)
> @@ -268,39 +245,32 @@ static void run_test(data_t *data)
>  	uint32_t handle = data->fb_white.gem_handle;
>  	igt_plane_t *test_plane = data->test_plane;
>  	void *ptr;
> -	char ref_crc[CRC_LEN];
> -	char crc[CRC_LEN];
>  	const char *expected = "";
>  
>  	/* Confirm that screen became Green */
> -	get_sink_crc(data, ref_crc);
> -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> +	manual("screen GREEN");
>  
>  	/* Confirm screen stays Green after PSR got active */
>  	igt_assert(wait_psr_entry(data));
> -	get_sink_crc(data, ref_crc);
> -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> +	manual("screen GREEN");
>  
>  	/* Setting a secondary fb/plane */
>  	igt_plane_set_fb(test_plane, &data->fb_white);
>  	igt_display_commit(&data->display);
>  
>  	/* Confirm it is not Green anymore */
> -	igt_assert(wait_psr_entry(data));
> -	get_sink_crc(data, ref_crc);
>  	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
> -		assert_or_manual(!is_green(ref_crc), "screen WHITE");
> +		manual("screen WHITE");
>  	else
> -		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
> +		manual("GREEN background with WHITE box");
>  
> +	igt_assert(wait_psr_entry(data));
>  	switch (data->op) {
>  	case PAGE_FLIP:
>  		/* Only in use when testing primary plane */
>  		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
>  					   data->fb_green.fb_id, 0, NULL) == 0);
> -		get_sink_crc(data, crc);
> -		assert_or_manual(is_green(crc), "screen GREEN");
> -		expected = "still GREEN";
> +		expected = "GREEN";
>  		break;
>  	case MMAP_GTT:
>  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> @@ -342,8 +312,8 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> -	get_sink_crc(data, crc);
> -	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
> +	assert(psr_inactive(data));
> +	manual(expected);
>  }
>  
>  static void test_cleanup(data_t *data) {
> @@ -444,7 +414,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 to check the CRC test logic.";
> +	       "  --no-psr\tRun test without PSR.";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
>  		{ 0, 0, 0, 0 }
> -- 
> 2.14.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/4] tests/kms_psr_sink_crc: Test PSR source HW status before PSR entry.

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 01:09:42AM -0700, Dhinakaran Pandiyan wrote:
> Wait for PSR HW status to show active before starting an PSR exit action.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  tests/kms_psr_sink_crc.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 4a18609a..4eca51da 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -197,33 +197,24 @@ static bool sink_support(data_t *data)
>  		strstr(buf, "Sink_Support: yes\n");
>  }
>  
> -static bool psr_enabled(data_t *data)
> +static bool psr_active(data_t *data, bool check_active)
>  {
> +	bool active;
>  	char buf[512];
>  
>  	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
>  
> -	return data->with_psr_disabled ||
> -		strstr(buf, "HW Enabled & Active bit: yes\n");
> +	active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> +		 (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> +	return check_active ? active : !active;
>  }
>  
>  static bool wait_psr_entry(data_t *data)
>  {
> -	int timeout = 5;
> -	while (timeout--) {
> -		if (psr_enabled(data))
> -			return true;
> -		sleep(1);
> -	}
> -	return false;
> -}
> +	if (data->with_psr_disabled)
> +		return true;
>  
> -static bool psr_inactive(data_t *data)
> -{
> -	char buf[512];
> -
> -	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> -	return !(strstr(buf, "SRDENT") || strstr("SLEEP"));
> +	return igt_wait((psr_active(data, true)), 500, 1);
>  }
>  
>  static inline void manual(const char *expected)
> @@ -312,7 +303,7 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> -	assert(psr_inactive(data));
> +	igt_assert(psr_active(data, false));
>  	manual(expected);
>  }
>  
> -- 
> 2.14.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests/psr: Rename kms_psr_sink_crc.c to kms_psr.c

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 01:09:43AM -0700, Dhinakaran Pandiyan wrote:
> We don't use sink CRC anymore in this test.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  tests/Makefile.sources                  | 2 +-
>  tests/intel-ci/fast-feedback.testlist   | 2 +-
>  tests/{kms_psr_sink_crc.c => kms_psr.c} | 0
>  tests/meson.build                       | 2 +-
>  4 files changed, 3 insertions(+), 3 deletions(-)
>  rename tests/{kms_psr_sink_crc.c => kms_psr.c} (100%)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 54b4a3c2..8fec6ffe 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -199,7 +199,7 @@ TESTS_progs = \
>  	kms_plane_multiple \
>  	kms_plane_scaling \
>  	kms_properties \
> -	kms_psr_sink_crc \
> +	kms_psr \
>  	kms_pwrite_crc \
>  	kms_rmfb \
>  	kms_rotation_crc \
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index b08ef770..aa861d39 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -243,7 +243,7 @@ igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> -igt@kms_psr_sink_crc@basic
> +igt@kms_psr@basic
>  igt@kms_setmode@basic-clone-single-crtc
>  igt@kms_sink_crc_basic
>  igt@pm_backlight@basic-brightness
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr.c
> similarity index 100%
> rename from tests/kms_psr_sink_crc.c
> rename to tests/kms_psr.c
> diff --git a/tests/meson.build b/tests/meson.build
> index 029b8043..704bd13f 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -175,7 +175,7 @@ test_progs = [
>  	'kms_plane_multiple',
>  	'kms_plane_scaling',
>  	'kms_properties',
> -	'kms_psr_sink_crc',
> +	'kms_psr',
>  	'kms_pwrite_crc',
>  	'kms_rmfb',
>  	'kms_rotation_crc',
> -- 
> 2.14.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] tests/psr: Rename kms_psr_sink_crc.c to kms_psr.c

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 04:21:04PM -0700, Rodrigo Vivi wrote:
> On Thu, Jul 12, 2018 at 01:09:43AM -0700, Dhinakaran Pandiyan wrote:
> > We don't use sink CRC anymore in this test.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

series pushed, thanks

> 
> > ---
> >  tests/Makefile.sources                  | 2 +-
> >  tests/intel-ci/fast-feedback.testlist   | 2 +-
> >  tests/{kms_psr_sink_crc.c => kms_psr.c} | 0
> >  tests/meson.build                       | 2 +-
> >  4 files changed, 3 insertions(+), 3 deletions(-)
> >  rename tests/{kms_psr_sink_crc.c => kms_psr.c} (100%)
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 54b4a3c2..8fec6ffe 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -199,7 +199,7 @@ TESTS_progs = \
> >  	kms_plane_multiple \
> >  	kms_plane_scaling \
> >  	kms_properties \
> > -	kms_psr_sink_crc \
> > +	kms_psr \
> >  	kms_pwrite_crc \
> >  	kms_rmfb \
> >  	kms_rotation_crc \
> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > index b08ef770..aa861d39 100644
> > --- a/tests/intel-ci/fast-feedback.testlist
> > +++ b/tests/intel-ci/fast-feedback.testlist
> > @@ -243,7 +243,7 @@ igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence
> >  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
> >  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
> >  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> > -igt@kms_psr_sink_crc@basic
> > +igt@kms_psr@basic
> >  igt@kms_setmode@basic-clone-single-crtc
> >  igt@kms_sink_crc_basic
> >  igt@pm_backlight@basic-brightness
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr.c
> > similarity index 100%
> > rename from tests/kms_psr_sink_crc.c
> > rename to tests/kms_psr.c
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 029b8043..704bd13f 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -175,7 +175,7 @@ test_progs = [
> >  	'kms_plane_multiple',
> >  	'kms_plane_scaling',
> >  	'kms_properties',
> > -	'kms_psr_sink_crc',
> > +	'kms_psr',
> >  	'kms_pwrite_crc',
> >  	'kms_rmfb',
> >  	'kms_rotation_crc',
> > -- 
> > 2.14.1
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev