[Intel-gfx] [PATCH 2/5] lib/debugfs: Add igt_assert_crc_equal

Daniel Vetter daniel.vetter at ffwll.ch
Fri Mar 13 10:43:05 PDT 2015


Because of hash collisions tests should only ever compare crc
checksums for equality. Checking for inequality can result in random
failures.

To ensure this only expose and igt_assert function and use that.
Follow-up patches will rework the code for tests which don't follow
this requirement and try to compare for CRC inequality.

v2: Rebase on top of Matt's kms_plane changes.

Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
 lib/igt_debugfs.c           | 21 ++++++++++++++++++++-
 lib/igt_debugfs.h           |  1 +
 tests/kms_cursor_crc.c      |  4 ++--
 tests/kms_fbc_crc.c         |  4 ++--
 tests/kms_flip_tiling.c     |  2 +-
 tests/kms_mmio_vs_cs_flip.c |  4 ++--
 tests/kms_pipe_crc_basic.c  |  2 +-
 tests/kms_plane.c           |  8 ++++----
 tests/kms_pwrite_crc.c      |  2 +-
 tests/kms_rotation_crc.c    |  4 ++--
 tests/kms_universal_plane.c | 14 +++++++-------
 11 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index a2cec45a1460..dddd6c350b45 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -63,7 +63,7 @@
  * another either for equality or difference. Otherwise CRCs must be treated as
  * completely opaque values. Note that not even CRCs from different pipes or tap
  * points on the same platform can be compared. Hence only use igt_crc_is_null()
- * and igt_crc_equal() to inspect CRC values captured by the same
+ * and igt_assert_crc_equal() to inspect CRC values captured by the same
  * #igt_pipe_crc_t object.
  *
  * # Other debugfs interface wrappers
@@ -235,6 +235,25 @@ bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b)
 }
 
 /**
+ * igt_assert_crc_equal:
+ * @a: first pipe CRC value
+ * @b: second pipe CRC value
+ *
+ * Compares two CRC values and fails the testcase if they don't match with
+ * igt_fail(). Note that due to CRC collisions CRC based testcase can only
+ * assert that CRCs match, never that they are different. Otherwise there might
+ * be random testcase failures when different screen contents end up with the
+ * same CRC by chance.
+ */
+void igt_assert_crc_equal(igt_crc_t *a, igt_crc_t *b)
+{
+	int i;
+
+	for (i = 0; i < a->n_words; i++)
+		igt_assert_eq_u32(a->crc[i], b->crc[i]);
+}
+
+/**
  * igt_crc_to_string:
  * @crc: pipe CRC value to print
  *
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 828502957a98..f158c3389fda 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -87,6 +87,7 @@ enum intel_pipe_crc_source {
 
 bool igt_crc_is_null(igt_crc_t *crc);
 bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b);
+void igt_assert_crc_equal(igt_crc_t *a, igt_crc_t *b);
 char *igt_crc_to_string(igt_crc_t *crc);
 
 void igt_require_pipe_crc(void);
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 3de7e021f3f3..0bfe15ee03d4 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -134,7 +134,7 @@ static void do_single_test(data_t *data, int x, int y)
 	igt_wait_for_vblank(data->drm_fd, data->pipe);
 	igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
 	/* Clear screen afterwards */
-	igt_assert(igt_crc_equal(&crc, &ref_crc));
+	igt_assert_crc_equal(&crc, &ref_crc);
 
 	igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
 			0.0, 0.0, 0.0);
@@ -451,7 +451,7 @@ static void test_cursor_size(data_t *data)
 		/* Clear screen afterwards */
 		igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
 				0.0, 0.0, 0.0);
-		igt_assert(igt_crc_equal(&crc[i], &ref_crc));
+		igt_assert_crc_equal(&crc[i], &ref_crc);
 	}
 }
 
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 32c3d9899d1a..c42bab9a035f 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -269,7 +269,7 @@ static void test_crc(data_t *data, enum test_mode mode)
 	igt_pipe_crc_stop(pipe_crc);
 	igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
 	if (mode == TEST_PAGE_FLIP)
-		igt_assert(igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+		igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
 	else
 		igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
 	free(crcs);
@@ -287,7 +287,7 @@ static void test_crc(data_t *data, enum test_mode mode)
 	igt_pipe_crc_stop(pipe_crc);
 	igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
 	if (mode == TEST_PAGE_FLIP)
-		igt_assert(igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+		igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
 	else
 		igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
 	free(crcs);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index 32f167ab1771..c3d7bcbfe429 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -110,7 +110,7 @@ test_flip_changes_tiling(data_t *data, igt_output_t *output)
 
 	/* get a crc and compare with the reference */
 	igt_pipe_crc_collect_crc(pipe_crc, &crc);
-	igt_assert(igt_crc_equal(&reference_crc, &crc));
+	igt_assert_crc_equal(&reference_crc, &crc);
 
 	/* clean up */
 	igt_plane_set_fb(primary, NULL);
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
index b77f7aee96b8..f0a99266714c 100644
--- a/tests/kms_mmio_vs_cs_flip.c
+++ b/tests/kms_mmio_vs_cs_flip.c
@@ -319,7 +319,7 @@ test_plane(data_t *data, igt_output_t *output, enum pipe pipe, enum igt_plane pl
 	igt_output_set_pipe(output, PIPE_ANY);
 	igt_display_commit(&data->display);
 
-	igt_assert(igt_crc_equal(&ref_crc, &crc));
+	igt_assert_crc_equal(&ref_crc, &crc);
 
 	return true;
 }
@@ -468,7 +468,7 @@ test_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 	igt_output_set_pipe(output, PIPE_ANY);
 	igt_display_commit(&data->display);
 
-	igt_assert(igt_crc_equal(&ref_crc, &crc));
+	igt_assert_crc_equal(&ref_crc, &crc);
 
 	return true;
 }
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index a658b39df019..32c08671e5bf 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -182,7 +182,7 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 
 		/* and ensure that they'are all equal, we haven't changed the fb */
 		for (j = 0; j < (N_CRCS - 1); j++)
-			igt_assert(igt_crc_equal(&crcs[j], &crcs[j + 1]));
+			igt_assert_crc_equal(&crcs[j], &crcs[j + 1]);
 
 		if (flags & TEST_SEQUENCE)
 			for (j = 0; j < (N_CRCS - 1); j++)
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index b66ab1d27f90..29f8be77626d 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -207,11 +207,11 @@ test_plane_position_with_output(data_t *data,
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc2);
 
 	if (flags & TEST_POSITION_FULLY_COVERED)
-		igt_assert(igt_crc_equal(&test.reference_crc, &crc));
+		igt_assert_crc_equal(&test.reference_crc, &crc);
 	else
 		igt_assert(!igt_crc_equal(&test.reference_crc, &crc));
 
-	igt_assert(igt_crc_equal(&crc, &crc2));
+	igt_assert_crc_equal(&crc, &crc2);
 
 	igt_plane_set_fb(primary, NULL);
 	igt_plane_set_fb(sprite, NULL);
@@ -333,9 +333,9 @@ test_plane_panning_with_output(data_t *data,
 	igt_debug_wait_for_keypress("crc");
 
 	if (flags & TEST_PANNING_TOP_LEFT)
-		igt_assert(igt_crc_equal(&test.red_crc, &crc));
+		igt_assert_crc_equal(&test.red_crc, &crc);
 	else
-		igt_assert(igt_crc_equal(&test.blue_crc, &crc));
+		igt_assert_crc_equal(&test.blue_crc, &crc);
 
 	igt_plane_set_fb(primary, NULL);
 
diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
index aa1d3a305a70..f6adb04a39a9 100644
--- a/tests/kms_pwrite_crc.c
+++ b/tests/kms_pwrite_crc.c
@@ -100,7 +100,7 @@ static void test(data_t *data)
 
 	/* check that the crc is as expected, which requires that caches got flushed */
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
-	igt_assert(igt_crc_equal(&crc, &data->ref_crc));
+	igt_assert_crc_equal(&crc, &data->ref_crc);
 }
 
 static bool prepare_crtc(data_t *data)
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 7f09df313c16..ccc2f3617111 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -204,7 +204,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			igt_display_commit2(display, commit);
 
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
-			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
+			igt_assert_crc_equal(&data->ref_crc, &crc_output);
 
 			/* check the rotation state has been reset when the VT
 			 * mode is restored */
@@ -212,7 +212,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			kmstest_set_vt_graphics_mode();
 			prepare_crtc(data, output, pipe, plane);
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
-			igt_assert(igt_crc_equal(&crc_unrotated, &crc_output));
+			igt_assert_crc_equal(&crc_unrotated, &crc_output);
 
 
 			valid_tests++;
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index a851994a2471..3672f158c351 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -261,37 +261,37 @@ functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_display_commit2(display, COMMIT_LEGACY);
 
 	/* Blue bg + red sprite should be same under both types of API's */
-	igt_assert(igt_crc_equal(&test.crc_2, &test.crc_4));
+	igt_assert_crc_equal(&test.crc_2, &test.crc_4);
 
 	/* Disabling primary plane should be same as black primary */
-	igt_assert(igt_crc_equal(&test.crc_1, &test.crc_5));
+	igt_assert_crc_equal(&test.crc_1, &test.crc_5);
 
 	/* Re-enabling primary should return to blue properly */
-	igt_assert(igt_crc_equal(&test.crc_2, &test.crc_6));
+	igt_assert_crc_equal(&test.crc_2, &test.crc_6);
 
 	/*
 	 * We should be able to setup plane FB's while CRTC is disabled and
 	 * then have them pop up correctly when the CRTC is re-enabled.
 	 */
-	igt_assert(igt_crc_equal(&test.crc_2, &test.crc_7));
+	igt_assert_crc_equal(&test.crc_2, &test.crc_7);
 
 	/*
 	 * We should be able to modeset with the primary plane off
 	 * successfully
 	 */
-	igt_assert(igt_crc_equal(&test.crc_3, &test.crc_8));
+	igt_assert_crc_equal(&test.crc_3, &test.crc_8);
 
 	/*
 	 * We should be able to move the primary plane completely offscreen
 	 * and have it disable successfully.
 	 */
-	igt_assert(igt_crc_equal(&test.crc_5, &test.crc_9));
+	igt_assert_crc_equal(&test.crc_5, &test.crc_9);
 
 	/*
 	 * We should be able to explicitly disable an already
 	 * implicitly-disabled primary plane
 	 */
-	igt_assert(igt_crc_equal(&test.crc_5, &test.crc_10));
+	igt_assert_crc_equal(&test.crc_5, &test.crc_10);
 
 	igt_plane_set_fb(primary, NULL);
 	igt_plane_set_fb(sprite, NULL);
-- 
1.9.3



More information about the Intel-gfx mailing list