[Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: convert macros to functions

Daniel Vetter daniel.vetter at ffwll.ch
Fri Aug 4 16:21:24 UTC 2017


I guess this was done to have a better indication of which testcase
and function failed, but igt nowadays dumps an entire stacktrace. And
macros of this magnitude mean the line number is entirely meaningless,
since it doesn't point at a specific check.

Reason I've started to looking into this is that in our full igt CI
runs we have a serious problem with the fbc testcases randomly
failing with

(kms_frontbuffer_tracking:1609) CRITICAL: Test assertion failure function enable_prim_screen_and_wait, file kms_frontbuffer_tracking.c:1771:
(kms_frontbuffer_tracking:1609) CRITICAL: Failed assertion: false
(kms_frontbuffer_tracking:1609) CRITICAL: FBC disabled

And that's not entirely helpful. Also, macros of this magnitude are
just horrible to read.

Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
 tests/kms_frontbuffer_tracking.c | 166 ++++++++++++++++++++-------------------
 1 file changed, 84 insertions(+), 82 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index e03524f1c45b..8d11dc065623 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1677,88 +1677,90 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	return flags;
 }
 
-#define do_crc_assertions(flags, mandatory_sink_crc) do {		\
-	int flags__ = (flags);						\
-	struct both_crcs crc_;						\
-									\
-	if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))		\
-		break;							\
-									\
-	collect_crcs(&crc_, mandatory_sink_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"); 	\
-} while (0)
-
-#define do_status_assertions(flags_) do {				\
-	if (!opt.check_status) {					\
-		/* Make sure we settle before continuing. */		\
-		sleep(1);						\
-		break;							\
-	}								\
-									\
-	if (flags_ & ASSERT_FBC_ENABLED) {				\
-		igt_require(!fbc_not_enough_stolen());			\
-		igt_require(!fbc_stride_not_supported());		\
-		if (!fbc_wait_until_enabled()) {			\
-			fbc_print_status();				\
-			igt_assert_f(false, "FBC disabled\n");		\
-		}							\
-									\
-		if (opt.fbc_check_compression)				\
-			igt_assert(fbc_wait_for_compression());		\
-	} else if (flags_ & ASSERT_FBC_DISABLED) {			\
-		igt_assert(!fbc_wait_until_enabled());			\
-	}								\
-									\
-	if (flags_ & ASSERT_PSR_ENABLED) {				\
-		if (!psr_wait_until_enabled()) {			\
-			psr_print_status();				\
-			igt_assert_f(false, "PSR disabled\n");		\
-		}							\
-	} else if (flags_ & ASSERT_PSR_DISABLED) {			\
-		igt_assert(!psr_wait_until_enabled());			\
-	}								\
-} while (0)
-
-#define do_assertions(flags) do {					\
-	int flags_ = adjust_assertion_flags(t, (flags));		\
-	bool mandatory_sink_crc = t->feature & FEATURE_PSR;		\
-									\
-	wait_user(2, "Paused before assertions.");			\
-									\
-	/* Check the CRC to make sure the drawing operations work	\
-	 * immediately, independently of the features being enabled. */	\
-	do_crc_assertions(flags_, mandatory_sink_crc);			\
-									\
-	/* Now we can flush things to make the test faster. */		\
-	do_flush(t);							\
-									\
-	do_status_assertions(flags_);					\
-									\
-	/* Check CRC again to make sure the compressed screen is ok,	\
-	 * except if we're not drawing on the primary screen. On this	\
-	 * case, the first check should be enough and a new CRC check	\
-	 * 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);		\
-									\
-	if (fbc.supports_last_action && opt.fbc_check_last_action) {	\
-		if (flags_ & ASSERT_LAST_ACTION_CHANGED)		\
-			igt_assert(fbc_last_action_changed());		\
-		else if (flags_ & ASSERT_NO_ACTION_CHANGE)		\
-			igt_assert(!fbc_last_action_changed());		\
-	}								\
-									\
-	wait_user(1, "Paused after assertions.");			\
-} while (0)
+static void do_crc_assertions(int flags, bool mandatory_sink_crc)
+{
+	struct both_crcs crc;
+
+	if (!opt.check_crc || (flags & DONT_ASSERT_CRC))
+		return;
+
+	collect_crcs(&crc, mandatory_sink_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");
+}
+
+static void do_status_assertions(int flags)
+{
+	if (!opt.check_status) {
+		/* Make sure we settle before continuing. */
+		sleep(1);
+		return;
+	}
+
+	if (flags & ASSERT_FBC_ENABLED) {
+		igt_require(!fbc_not_enough_stolen());
+		igt_require(!fbc_stride_not_supported());
+		if (!fbc_wait_until_enabled()) {
+			fbc_print_status();
+			igt_assert_f(false, "FBC disabled\n");
+		}
+
+		if (opt.fbc_check_compression)
+			igt_assert(fbc_wait_for_compression());
+	} else if (flags & ASSERT_FBC_DISABLED) {
+		igt_assert(!fbc_wait_until_enabled());
+	}
+
+	if (flags & ASSERT_PSR_ENABLED) {
+		if (!psr_wait_until_enabled()) {
+			psr_print_status();
+			igt_assert_f(false, "PSR disabled\n");
+		}
+	} else if (flags & ASSERT_PSR_DISABLED) {
+		igt_assert(!psr_wait_until_enabled());
+	}
+}
+
+static void do_assertions(int flags)
+{
+	flags = adjust_assertion_flags(t, flags);
+	bool mandatory_sink_crc = t->feature & FEATURE_PSR;
+
+	wait_user(2, "Paused before assertions.");
+
+	/* Check the CRC to make sure the drawing operations work
+	 * immediately, independently of the features being enabled. */
+	do_crc_assertions(flags, mandatory_sink_crc);
+
+	/* Now we can flush things to make the test faster. */
+	do_flush(t);
+
+	do_status_assertions(flags);
+
+	/* Check CRC again to make sure the compressed screen is ok,
+	 * except if we're not drawing on the primary screen. On this
+	 * case, the first check should be enough and a new CRC check
+	 * 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);
+
+	if (fbc.supports_last_action && opt.fbc_check_last_action) {
+		if (flags & ASSERT_LAST_ACTION_CHANGED)
+			igt_assert(fbc_last_action_changed());
+		else if (flags_ & ASSERT_NO_ACTION_CHANGE)
+			igt_assert(!fbc_last_action_changed());
+	}
+
+	wait_user(1, "Paused after assertions.");
+}
 
 static void enable_prim_screen_and_wait(const struct test_mode *t)
 {
-- 
2.5.5



More information about the Intel-gfx mailing list