[Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Don't setup crc in _new

Daniel Vetter daniel.vetter at ffwll.ch
Mon Nov 24 16:23:52 CET 2014


The problem is that this causes writes to registers, and if the pipe
is off they might go nowhere (e.g. when runtime pm is enabled).
Furthermore we can only really check once the modeset setup is done,
but again most tests set up the CRC structure before calling
igt_commit and friends. We could add crc restore support to the
kernel's rpm code, but that will end up being rather invasive and
fragile hard-to-test code.

Now originally this was needed back when CRC support wasn't available
everywhere. But that's fixed now.

So given all this just drop that sanity check and make sure that we
only touch the debugfs file (and so the hw state) when we know the
pipe is running in the desired configuration.

A complementary kernel patch will try to catch offenders by returning
-EIO if the pipe is off.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86092
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
 lib/igt_debugfs.c           | 17 ++++++-----------
 tests/kms_cursor_crc.c      | 12 ++----------
 tests/kms_fbc_crc.c         | 12 ------------
 tests/kms_mmio_vs_cs_flip.c | 29 -----------------------------
 tests/kms_pipe_crc_basic.c  |  3 ---
 tests/kms_rotation_crc.c    | 13 +++----------
 tests/kms_universal_plane.c |  3 ---
 7 files changed, 11 insertions(+), 78 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index b30f5e491047..0b098eedbebd 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -294,6 +294,9 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
 {
 	char buf[64];
 
+	/* Stop first just to make sure we don't have lingering state left. */
+	igt_pipe_crc_stop(pipe_crc);
+
 	sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
 		pipe_crc_source_name(pipe_crc->source));
 	errno = 0;
@@ -362,9 +365,9 @@ void igt_require_pipe_crc(void)
  *
  * This sets up a new pipe CRC capture object for the given @pipe and @source.
  *
- * Returns: A pipe CRC object if the given @pipe and @source is available, NULL
- * otherwise. Tests can use this to intelligently skip if they require a
- * specific pipe CRC source to function properly.
+ * Returns: A pipe CRC object if the given @pipe and @source. The library
+ * assumes that the source is always available since recent kernels support at
+ * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
  */
 igt_pipe_crc_t *
 igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
@@ -388,14 +391,6 @@ igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
 	pipe_crc->pipe = pipe;
 	pipe_crc->source = source;
 
-	/* make sure this source is actually supported */
-	if (!igt_pipe_crc_do_start(pipe_crc)) {
-		igt_pipe_crc_free(pipe_crc);
-		return NULL;
-	}
-
-	igt_pipe_crc_stop(pipe_crc);
-
 	return pipe_crc;
 }
 
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 5772ed4585e8..7a8f34a6db74 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -230,7 +230,7 @@ static void test_crc_random(data_t *data)
 	}
 }
 
-static bool prepare_crtc(data_t *data, igt_output_t *output,
+static void prepare_crtc(data_t *data, igt_output_t *output,
 			 int cursor_w, int cursor_h)
 {
 	drmModeModeInfo *mode;
@@ -267,11 +267,6 @@ static bool prepare_crtc(data_t *data, igt_output_t *output,
 
 	data->pipe_crc = igt_pipe_crc_new(data->pipe,
 					  INTEL_PIPE_CRC_SOURCE_AUTO);
-	if (!data->pipe_crc) {
-		igt_info("auto crc not supported on this connector with pipe %i\n",
-			 data->pipe);
-		return false;
-	}
 
 	/* x/y position where the cursor is still fully visible */
 	data->left = 0;
@@ -289,8 +284,6 @@ static bool prepare_crtc(data_t *data, igt_output_t *output,
 
 	/* get reference crc w/o cursor */
 	igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
-
-	return true;
 }
 
 static void cleanup_crtc(data_t *data, igt_output_t *output)
@@ -325,8 +318,7 @@ static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
 		for_each_pipe(display, p) {
 			data->pipe = p;
 
-			if (!prepare_crtc(data, output, cursor_w, cursor_h))
-				continue;
+			prepare_crtc(data, output, cursor_w, cursor_h);
 
 			valid_tests++;
 
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 6789f13094f6..32c3d9899d1a 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -359,18 +359,6 @@ static bool prepare_test(data_t *data, enum test_mode test_mode)
 
 	pipe_crc = igt_pipe_crc_new(data->pipe,
 				    INTEL_PIPE_CRC_SOURCE_AUTO);
-	if (!pipe_crc) {
-		igt_info("auto crc not supported on this connector with crtc %i\n",
-			 data->pipe);
-
-		igt_plane_set_fb(data->primary, NULL);
-		igt_output_set_pipe(output, PIPE_ANY);
-		igt_display_commit(display);
-
-		igt_remove_fb(data->drm_fd, &data->fb[0]);
-		igt_remove_fb(data->drm_fd, &data->fb[1]);
-		return false;
-	}
 
 	data->pipe_crc = pipe_crc;
 
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
index 37b5a60b514f..e50a91124b0f 100644
--- a/tests/kms_mmio_vs_cs_flip.c
+++ b/tests/kms_mmio_vs_cs_flip.c
@@ -247,21 +247,6 @@ test_plane(data_t *data, igt_output_t *output, enum pipe pipe, enum igt_plane pl
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-	if (!data->pipe_crc) {
-		igt_info("auto crc not supported on this connector with crtc %i\n",
-			 pipe);
-
-		igt_plane_set_fb(primary, NULL);
-		igt_plane_set_fb(sprite, NULL);
-		igt_output_set_pipe(output, PIPE_ANY);
-		igt_display_commit(&data->display);
-
-		igt_remove_fb(data->drm_fd, &red_fb);
-		igt_remove_fb(data->drm_fd, &green_fb);
-		igt_remove_fb(data->drm_fd, &blue_fb);
-
-		return false;
-	}
 
 	/* set red fb and grab reference crc */
 	igt_plane_set_fb(primary, &red_fb);
@@ -410,20 +395,6 @@ test_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-	if (!data->pipe_crc) {
-		igt_info("auto crc not supported on this connector with crtc %i\n",
-			 pipe);
-
-		igt_plane_set_fb(primary, NULL);
-		igt_output_set_pipe(output, PIPE_ANY);
-		igt_display_commit(&data->display);
-
-		igt_remove_fb(data->drm_fd, &red_fb);
-		igt_remove_fb(data->drm_fd, &green_fb);
-		igt_remove_fb(data->drm_fd, &blue_fb);
-
-		return false;
-	}
 
 	/* set red fb and grab reference crc */
 	igt_plane_set_fb(primary, &red_fb);
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 13bb073a9a34..7380c8a1ea31 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -151,9 +151,6 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 
 		pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 
-		if (!pipe_crc)
-			return 0;
-
 		igt_pipe_crc_start(pipe_crc);
 
 		/* wait for N_CRCS vblanks and the corresponding N_CRCS CRCs */
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 41e9912e9c91..7f09df313c16 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -79,7 +79,7 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 	cairo_destroy(cr);
 }
 
-static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
+static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 			 igt_plane_t *plane)
 {
 	drmModeModeInfo *mode;
@@ -91,11 +91,6 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	/* create the pipe_crc object for this pipe */
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-	if (!data->pipe_crc) {
-		igt_info("auto crc not supported on this connector with pipe %i\n",
-			 pipe);
-		return false;
-	}
 
 	mode = igt_output_get_mode(output);
 
@@ -149,8 +144,6 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 		paint_squares(data, &data->fb, mode, IGT_ROTATION_0, plane);
 		igt_plane_set_fb(plane, &data->fb);
 	}
-
-	return true;
 }
 
 static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
@@ -200,8 +193,8 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			plane = igt_output_get_plane(output, plane_type);
 			igt_require(igt_plane_supports_rotation(plane));
 
-			if (!prepare_crtc(data, output, pipe, plane))
-				continue;
+			prepare_crtc(data, output, pipe, plane);
+
 			igt_display_commit2(display, commit);
 
 			/* collect unrotated CRC */
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 228715f0aaa4..58e46915c877 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -62,9 +62,6 @@ functional_test_init(functional_test_t *test, igt_output_t *output, enum pipe pi
 	drmModeModeInfo *mode;
 
 	test->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-	igt_skip_on_f(!test->pipe_crc,
-		      "auto crc not supported on this connector with pipe %i\n", pipe);
-
 
 	igt_output_set_pipe(output, pipe);
 
-- 
2.1.1




More information about the Intel-gfx mailing list