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

* [igt-dev] [PATCH i-g-t] tests/frontbuffer_tracking: Do not test sink crc
@ 2018-07-02  8:25 Dhinakaran Pandiyan
  2018-07-02 13:46 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-02  8:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Dhinakaran Pandiyan

eDP sink crc reads use vblank interrupts that cause PSR exit and
therefore makes them unsuitable for PSR testing.

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread