public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW
@ 2014-05-28 18:23 Damien Lespiau
  2014-05-28 18:23 ` [PATCH 1/6] kms_pipe_crc_basic: Split the main test function a bit more Damien Lespiau
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:23 UTC (permalink / raw)
  To: intel-gfx

We currenly have a bug on HSW where asking for the panel fitter CRC when the
power well is down returns 0xffffffff, but kms_pipe_crc_basic passes as it only
tests that the CRCs obtained for a FB are consistent over time. We consistently
read 0xffffffff, so the test was passing.

With this series we now scan out 2 different fbs and make sure the CRCs for
those two fbs are indeed, different. This makes:

$ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-A-frame-sequence

fail when the power well is down, while:

$ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-B-frame-sequence

succeeds, using pipe B turning the power well on.

-- 
Damien

Damien Lespiau (6):
  kms_pipe_crc_basic: Split the main test function a bit more
  kms_pipe_crc_basic: Cycle between 2 differently colored buffer
  kms_pipe_crc_basic: Make the number of CRCs a parameter
  kms_pipe_crc_basic: Bump the number of collected CRCs to 60
  kms_pipe_crc_basic: Add a bit a debugging output
  lib: Add a --igt-debug command line option

 lib/igt_core.c             |  5 +++
 tests/kms_pipe_crc_basic.c | 97 +++++++++++++++++++++++++++++++++-------------
 2 files changed, 76 insertions(+), 26 deletions(-)

-- 
1.8.3.1

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

* [PATCH 1/6] kms_pipe_crc_basic: Split the main test function a bit more
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
@ 2014-05-28 18:23 ` Damien Lespiau
  2014-05-28 18:24 ` [PATCH 2/6] kms_pipe_crc_basic: Cycle between 2 differently colored buffer Damien Lespiau
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:23 UTC (permalink / raw)
  To: intel-gfx

Let's put the per-output test in its own function to get rid of 1 level
of indentation. We'll need it to cycle through 2 different framebuffers
to make sure we compute different CRCs if the fbs are different.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_pipe_crc_basic.c | 101 ++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 46 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 9eec4e6..0eedac4 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -105,76 +105,85 @@ static void test_bad_command(data_t *data, const char *cmd)
 
 #define TEST_SEQUENCE (1<<0)
 
-static void test_read_crc(data_t *data, int pipe, unsigned flags)
+static int
+test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
+			 unsigned flags)
 {
 	igt_display_t *display = &data->display;
+	igt_plane_t *primary;
+	drmModeModeInfo *mode;
 	igt_pipe_crc_t *pipe_crc;
 	igt_crc_t *crcs = NULL;
-	int valid_connectors = 0;
-	igt_output_t *output;
 
-	igt_skip_on(pipe >= data->display.n_pipes);
+	igt_output_set_pipe(output, pipe);
 
-	for_each_connected_output(display, output) {
-		igt_plane_t *primary;
-		drmModeModeInfo *mode;
+	mode = igt_output_get_mode(output);
+	igt_create_color_fb(data->drm_fd,
+				mode->hdisplay, mode->vdisplay,
+				DRM_FORMAT_XRGB8888,
+				false, /* tiled */
+				0.0, 1.0, 0.0,
+				&data->fb);
 
-		igt_output_set_pipe(output, pipe);
+	primary = igt_output_get_plane(output, 0);
+	igt_plane_set_fb(primary, &data->fb);
 
-		igt_info("%s: Testing connector %s using pipe %c\n",
-			 igt_subtest_name(), igt_output_name(output),
-			 pipe_name(pipe));
+	igt_display_commit(display);
 
-		mode = igt_output_get_mode(output);
-		igt_create_color_fb(data->drm_fd,
-					mode->hdisplay, mode->vdisplay,
-					DRM_FORMAT_XRGB8888,
-					false, /* tiled */
-					0.0, 1.0, 0.0,
-					&data->fb);
+	pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 
-		primary = igt_output_get_plane(output, 0);
-		igt_plane_set_fb(primary, &data->fb);
+	if (!pipe_crc)
+		return 0;
 
-		igt_display_commit(display);
+	igt_pipe_crc_start(pipe_crc);
 
-		pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+	/* wait for 3 vblanks and the corresponding 3 CRCs */
+	igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
 
-		if (!pipe_crc)
-			continue;
-		valid_connectors++;
+	igt_pipe_crc_stop(pipe_crc);
 
-		igt_pipe_crc_start(pipe_crc);
+	/* ensure the CRCs are not all 0s */
+	igt_assert(!igt_crc_is_null(&crcs[0]));
+	igt_assert(!igt_crc_is_null(&crcs[1]));
+	igt_assert(!igt_crc_is_null(&crcs[2]));
 
-		/* wait for 3 vblanks and the corresponding 3 CRCs */
-		igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
+	/* and ensure that they'are all equal, we haven't changed the fb */
+	igt_assert(igt_crc_equal(&crcs[0], &crcs[1]));
+	igt_assert(igt_crc_equal(&crcs[1], &crcs[2]));
 
-		igt_pipe_crc_stop(pipe_crc);
+	if (flags & TEST_SEQUENCE) {
+		igt_assert(crcs[0].frame + 1 == crcs[1].frame);
+		igt_assert(crcs[1].frame + 1 == crcs[2].frame);
+	}
 
-		/* ensure the CRCs are not all 0s */
-		igt_assert(!igt_crc_is_null(&crcs[0]));
-		igt_assert(!igt_crc_is_null(&crcs[1]));
-		igt_assert(!igt_crc_is_null(&crcs[2]));
+	free(crcs);
+	igt_pipe_crc_free(pipe_crc);
+	igt_remove_fb(data->drm_fd, &data->fb);
+	igt_plane_set_fb(primary, NULL);
 
-		/* and ensure that they'are all equal, we haven't changed the fb */
-		igt_assert(igt_crc_equal(&crcs[0], &crcs[1]));
-		igt_assert(igt_crc_equal(&crcs[1], &crcs[2]));
+	igt_output_set_pipe(output, PIPE_ANY);
 
-		if (flags & TEST_SEQUENCE) {
-			igt_assert(crcs[0].frame + 1 == crcs[1].frame);
-			igt_assert(crcs[1].frame + 1 == crcs[2].frame);
-		}
+	return 1;
+}
 
-		free(crcs);
-		igt_pipe_crc_free(pipe_crc);
-		igt_remove_fb(data->drm_fd, &data->fb);
-		igt_plane_set_fb(primary, NULL);
+static void test_read_crc(data_t *data, int pipe, unsigned flags)
+{
+	igt_display_t *display = &data->display;
+	int valid_connectors = 0;
+	igt_output_t *output;
+
+	igt_skip_on(pipe >= data->display.n_pipes);
+
+	for_each_connected_output(display, output) {
+
+		igt_info("%s: Testing connector %s using pipe %c\n",
+			 igt_subtest_name(), igt_output_name(output),
+			 pipe_name(pipe));
 
-		igt_output_set_pipe(output, PIPE_ANY);
+		valid_connectors += test_read_crc_for_output(data, pipe, output, flags);
 	}
 
 	igt_require_f(valid_connectors, "No connector found for pipe %i\n", pipe);
-
 }
 
 data_t data = {0, };
-- 
1.8.3.1

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

* [PATCH 2/6] kms_pipe_crc_basic: Cycle between 2 differently colored buffer
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
  2014-05-28 18:23 ` [PATCH 1/6] kms_pipe_crc_basic: Split the main test function a bit more Damien Lespiau
@ 2014-05-28 18:24 ` Damien Lespiau
  2014-05-28 18:24 ` [PATCH 3/6] kms_pipe_crc_basic: Make the number of CRCs a parameter Damien Lespiau
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:24 UTC (permalink / raw)
  To: intel-gfx

Instead of just testing if the CRCs are stable, we also test 2 different
fbs to make sure that the CRC is actually changing.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_pipe_crc_basic.c | 95 +++++++++++++++++++++++++++++-----------------
 1 file changed, 61 insertions(+), 34 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 0eedac4..0b78ed0 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -39,6 +39,14 @@ typedef struct {
 	struct igt_fb fb;
 } data_t;
 
+static struct {
+	double r, g, b;
+	igt_crc_t crc;
+} colors[2] = {
+	{ .r = 0.0, .g = 1.0, .b = 0.0 },
+	{ .r = 0.0, .g = 1.0, .b = 1.0 },
+};
+
 static uint64_t submit_batch(int fd, unsigned ring_id)
 {
 	const uint32_t batch[] = { MI_NOOP,
@@ -114,54 +122,73 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 	drmModeModeInfo *mode;
 	igt_pipe_crc_t *pipe_crc;
 	igt_crc_t *crcs = NULL;
+	int c, j;
 
-	igt_output_set_pipe(output, pipe);
+	for (c = 0; c < ARRAY_SIZE(colors); c++) {
+		igt_output_set_pipe(output, pipe);
 
-	mode = igt_output_get_mode(output);
-	igt_create_color_fb(data->drm_fd,
-				mode->hdisplay, mode->vdisplay,
-				DRM_FORMAT_XRGB8888,
-				false, /* tiled */
-				0.0, 1.0, 0.0,
-				&data->fb);
+		mode = igt_output_get_mode(output);
+		igt_create_color_fb(data->drm_fd,
+					mode->hdisplay, mode->vdisplay,
+					DRM_FORMAT_XRGB8888,
+					false, /* tiled */
+					colors[c].r,
+					colors[c].g,
+					colors[c].b,
+					&data->fb);
 
-	primary = igt_output_get_plane(output, 0);
-	igt_plane_set_fb(primary, &data->fb);
+		primary = igt_output_get_plane(output, 0);
+		igt_plane_set_fb(primary, &data->fb);
 
-	igt_display_commit(display);
+		igt_display_commit(display);
 
-	pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+		pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 
-	if (!pipe_crc)
-		return 0;
+		if (!pipe_crc)
+			return 0;
 
-	igt_pipe_crc_start(pipe_crc);
+		igt_pipe_crc_start(pipe_crc);
 
-	/* wait for 3 vblanks and the corresponding 3 CRCs */
-	igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
+		/* wait for 3 vblanks and the corresponding 3 CRCs */
+		igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
 
-	igt_pipe_crc_stop(pipe_crc);
+		igt_pipe_crc_stop(pipe_crc);
 
-	/* ensure the CRCs are not all 0s */
-	igt_assert(!igt_crc_is_null(&crcs[0]));
-	igt_assert(!igt_crc_is_null(&crcs[1]));
-	igt_assert(!igt_crc_is_null(&crcs[2]));
+		/*
+		 * save the CRC in colors so it can be compared to the CRC of
+		 * other fbs
+		 */
+		colors[c].crc = crcs[0];
 
-	/* and ensure that they'are all equal, we haven't changed the fb */
-	igt_assert(igt_crc_equal(&crcs[0], &crcs[1]));
-	igt_assert(igt_crc_equal(&crcs[1], &crcs[2]));
+		/*
+		 * make sure the CRC of this fb is different from the ones of
+		 * previous fbs
+		 */
+		for (j = 0; j < c; j++)
+			igt_assert(!igt_crc_equal(&colors[j].crc,
+						  &colors[c].crc));
 
-	if (flags & TEST_SEQUENCE) {
-		igt_assert(crcs[0].frame + 1 == crcs[1].frame);
-		igt_assert(crcs[1].frame + 1 == crcs[2].frame);
-	}
+		/* ensure the CRCs are not all 0s */
+		igt_assert(!igt_crc_is_null(&crcs[0]));
+		igt_assert(!igt_crc_is_null(&crcs[1]));
+		igt_assert(!igt_crc_is_null(&crcs[2]));
+
+		/* and ensure that they'are all equal, we haven't changed the fb */
+		igt_assert(igt_crc_equal(&crcs[0], &crcs[1]));
+		igt_assert(igt_crc_equal(&crcs[1], &crcs[2]));
 
-	free(crcs);
-	igt_pipe_crc_free(pipe_crc);
-	igt_remove_fb(data->drm_fd, &data->fb);
-	igt_plane_set_fb(primary, NULL);
+		if (flags & TEST_SEQUENCE) {
+			igt_assert(crcs[0].frame + 1 == crcs[1].frame);
+			igt_assert(crcs[1].frame + 1 == crcs[2].frame);
+		}
+
+		free(crcs);
+		igt_pipe_crc_free(pipe_crc);
+		igt_remove_fb(data->drm_fd, &data->fb);
+		igt_plane_set_fb(primary, NULL);
 
-	igt_output_set_pipe(output, PIPE_ANY);
+		igt_output_set_pipe(output, PIPE_ANY);
+	}
 
 	return 1;
 }
-- 
1.8.3.1

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

* [PATCH 3/6] kms_pipe_crc_basic: Make the number of CRCs a parameter
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
  2014-05-28 18:23 ` [PATCH 1/6] kms_pipe_crc_basic: Split the main test function a bit more Damien Lespiau
  2014-05-28 18:24 ` [PATCH 2/6] kms_pipe_crc_basic: Cycle between 2 differently colored buffer Damien Lespiau
@ 2014-05-28 18:24 ` Damien Lespiau
  2014-05-28 18:24 ` [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60 Damien Lespiau
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:24 UTC (permalink / raw)
  To: intel-gfx

Let's make the test a bit more generic and have the number of CRCs we're
collecting a define so it can be changed easily.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_pipe_crc_basic.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 0b78ed0..b48921b 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -111,6 +111,8 @@ static void test_bad_command(data_t *data, const char *cmd)
 	fclose(ctl);
 }
 
+#define N_CRCS	3
+
 #define TEST_SEQUENCE (1<<0)
 
 static int
@@ -149,8 +151,8 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 
 		igt_pipe_crc_start(pipe_crc);
 
-		/* wait for 3 vblanks and the corresponding 3 CRCs */
-		igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
+		/* wait for N_CRCS vblanks and the corresponding N_CRCS CRCs */
+		igt_pipe_crc_get_crcs(pipe_crc, N_CRCS, &crcs);
 
 		igt_pipe_crc_stop(pipe_crc);
 
@@ -169,18 +171,16 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 						  &colors[c].crc));
 
 		/* ensure the CRCs are not all 0s */
-		igt_assert(!igt_crc_is_null(&crcs[0]));
-		igt_assert(!igt_crc_is_null(&crcs[1]));
-		igt_assert(!igt_crc_is_null(&crcs[2]));
+		for (j = 0; j < N_CRCS; j++)
+			igt_assert(!igt_crc_is_null(&crcs[j]));
 
 		/* and ensure that they'are all equal, we haven't changed the fb */
-		igt_assert(igt_crc_equal(&crcs[0], &crcs[1]));
-		igt_assert(igt_crc_equal(&crcs[1], &crcs[2]));
+		for (j = 0; j < (N_CRCS - 1); j++)
+			igt_assert(igt_crc_equal(&crcs[j], &crcs[j + 1]));
 
-		if (flags & TEST_SEQUENCE) {
-			igt_assert(crcs[0].frame + 1 == crcs[1].frame);
-			igt_assert(crcs[1].frame + 1 == crcs[2].frame);
-		}
+		if (flags & TEST_SEQUENCE)
+			for (j = 0; j < (N_CRCS - 1); j++)
+				igt_assert(crcs[j].frame + 1 == crcs[j + 1].frame);
 
 		free(crcs);
 		igt_pipe_crc_free(pipe_crc);
-- 
1.8.3.1

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

* [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
                   ` (2 preceding siblings ...)
  2014-05-28 18:24 ` [PATCH 3/6] kms_pipe_crc_basic: Make the number of CRCs a parameter Damien Lespiau
@ 2014-05-28 18:24 ` Damien Lespiau
  2014-05-28 19:32   ` Daniel Vetter
  2014-05-28 18:24 ` [PATCH 5/6] kms_pipe_crc_basic: Add a bit a debugging output Damien Lespiau
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:24 UTC (permalink / raw)
  To: intel-gfx

So we have a change to see something on the screen, while still being
relatively short (~1s per sub-test).

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_pipe_crc_basic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index b48921b..c4abdf7 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -111,7 +111,7 @@ static void test_bad_command(data_t *data, const char *cmd)
 	fclose(ctl);
 }
 
-#define N_CRCS	3
+#define N_CRCS	60
 
 #define TEST_SEQUENCE (1<<0)
 
-- 
1.8.3.1

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

* [PATCH 5/6] kms_pipe_crc_basic: Add a bit a debugging output
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
                   ` (3 preceding siblings ...)
  2014-05-28 18:24 ` [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60 Damien Lespiau
@ 2014-05-28 18:24 ` Damien Lespiau
  2014-05-28 18:24 ` [PATCH 6/6] lib: Add a --igt-debug command line option Damien Lespiau
  2014-07-07 16:43 ` [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
  6 siblings, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:24 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_pipe_crc_basic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index c4abdf7..5727b61 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -127,8 +127,13 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 	int c, j;
 
 	for (c = 0; c < ARRAY_SIZE(colors); c++) {
+		char *crc_str;
+
 		igt_output_set_pipe(output, pipe);
 
+		igt_debug("Clearing the fb with color (%.02lf,%.02lf,%.02lf)\n",
+			  colors[c].r, colors[c].g, colors[c].b);
+
 		mode = igt_output_get_mode(output);
 		igt_create_color_fb(data->drm_fd,
 					mode->hdisplay, mode->vdisplay,
@@ -162,6 +167,10 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 		 */
 		colors[c].crc = crcs[0];
 
+		crc_str = igt_crc_to_string(&crcs[0]);
+		igt_debug("CRC for this fb: %s\n", crc_str);
+		free(crc_str);
+
 		/*
 		 * make sure the CRC of this fb is different from the ones of
 		 * previous fbs
-- 
1.8.3.1

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

* [PATCH 6/6] lib: Add a --igt-debug command line option
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
                   ` (4 preceding siblings ...)
  2014-05-28 18:24 ` [PATCH 5/6] kms_pipe_crc_basic: Add a bit a debugging output Damien Lespiau
@ 2014-05-28 18:24 ` Damien Lespiau
  2014-05-28 19:28   ` Daniel Vetter
  2014-07-07 16:43 ` [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
  6 siblings, 1 reply; 10+ messages in thread
From: Damien Lespiau @ 2014-05-28 18:24 UTC (permalink / raw)
  To: intel-gfx

Sometimes it's practical to be able to set the logging level to debug
from the command line arguments.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 6e553cf..9d59ab4 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -274,6 +274,7 @@ static void print_usage(const char *command_str, const char *help_str,
 	FILE *f = output_on_stderr ? stderr : stdout;
 
 	fprintf(f, "Usage: %s [OPTIONS]\n"
+		   "  --igt-debug\n"
 		   "  --list-subtests\n"
 		   "  --run-subtest <pattern>\n", command_str);
 	if (help_str)
@@ -317,6 +318,7 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
 {
 	int c, option_index = 0;
 	static struct option long_options[] = {
+		{"igt-debug", 0, 0, 'd'},
 		{"list-subtests", 0, 0, 'l'},
 		{"run-subtest", 1, 0, 'r'},
 		{"help", 0, 0, 'h'},
@@ -357,6 +359,9 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
 	while ((c = getopt_long(argc, argv, short_opts, combined_opts,
 			       &option_index)) != -1) {
 		switch(c) {
+		case 'd':
+			setenv("IGT_LOG_LEVEL", "debug", 1);
+			break;
 		case 'l':
 			if (!run_single_subtest)
 				list_subtests = true;
-- 
1.8.3.1

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

* Re: [PATCH 6/6] lib: Add a --igt-debug command line option
  2014-05-28 18:24 ` [PATCH 6/6] lib: Add a --igt-debug command line option Damien Lespiau
@ 2014-05-28 19:28   ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-05-28 19:28 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Wed, May 28, 2014 at 07:24:04PM +0100, Damien Lespiau wrote:
> Sometimes it's practical to be able to set the logging level to debug
> from the command line arguments.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

The problem with this is that it won't work with simple igt tests which
don't use this for cmdline parsing. And I kinda don't like inconsistent
interfaces - I've originally considered this but then decided against it
since I didn't want to do the churn of going through all simple tests.

But now we have igt_simple_main which covers most simple tests, so this
would be feasible again. Care to do this right?

For bikesheds: I'd vote for -d --debug only, and maybe set the
igt_log_level variable directly.
-Daniel

> ---
>  lib/igt_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 6e553cf..9d59ab4 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -274,6 +274,7 @@ static void print_usage(const char *command_str, const char *help_str,
>  	FILE *f = output_on_stderr ? stderr : stdout;
>  
>  	fprintf(f, "Usage: %s [OPTIONS]\n"
> +		   "  --igt-debug\n"
>  		   "  --list-subtests\n"
>  		   "  --run-subtest <pattern>\n", command_str);
>  	if (help_str)
> @@ -317,6 +318,7 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
>  {
>  	int c, option_index = 0;
>  	static struct option long_options[] = {
> +		{"igt-debug", 0, 0, 'd'},
>  		{"list-subtests", 0, 0, 'l'},
>  		{"run-subtest", 1, 0, 'r'},
>  		{"help", 0, 0, 'h'},
> @@ -357,6 +359,9 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
>  	while ((c = getopt_long(argc, argv, short_opts, combined_opts,
>  			       &option_index)) != -1) {
>  		switch(c) {
> +		case 'd':
> +			setenv("IGT_LOG_LEVEL", "debug", 1);
> +			break;
>  		case 'l':
>  			if (!run_single_subtest)
>  				list_subtests = true;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60
  2014-05-28 18:24 ` [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60 Damien Lespiau
@ 2014-05-28 19:32   ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-05-28 19:32 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Wed, May 28, 2014 at 07:24:02PM +0100, Damien Lespiau wrote:
> So we have a change to see something on the screen, while still being
> relatively short (~1s per sub-test).
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

igt_wait_for_keypress is geared at such use-cases ... Maybe we should have
a default timeout in case you don't actually want to run it interactively.
-Daniel

> ---
>  tests/kms_pipe_crc_basic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index b48921b..c4abdf7 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -111,7 +111,7 @@ static void test_bad_command(data_t *data, const char *cmd)
>  	fclose(ctl);
>  }
>  
> -#define N_CRCS	3
> +#define N_CRCS	60
>  
>  #define TEST_SEQUENCE (1<<0)
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW
  2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
                   ` (5 preceding siblings ...)
  2014-05-28 18:24 ` [PATCH 6/6] lib: Add a --igt-debug command line option Damien Lespiau
@ 2014-07-07 16:43 ` Damien Lespiau
  6 siblings, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2014-07-07 16:43 UTC (permalink / raw)
  To: intel-gfx

On Wed, May 28, 2014 at 07:23:58PM +0100, Damien Lespiau wrote:
> We currenly have a bug on HSW where asking for the panel fitter CRC when the
> power well is down returns 0xffffffff, but kms_pipe_crc_basic passes as it only
> tests that the CRCs obtained for a FB are consistent over time. We consistently
> read 0xffffffff, so the test was passing.
> 
> With this series we now scan out 2 different fbs and make sure the CRCs for
> those two fbs are indeed, different. This makes:
> 
> $ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-A-frame-sequence
> 
> fail when the power well is down, while:
> 
> $ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-B-frame-sequence
> 
> succeeds, using pipe B turning the power well on.
> 
> -- 
> Damien
> 
> Damien Lespiau (6):
>   kms_pipe_crc_basic: Split the main test function a bit more
>   kms_pipe_crc_basic: Cycle between 2 differently colored buffer
>   kms_pipe_crc_basic: Make the number of CRCs a parameter
>   kms_pipe_crc_basic: Bump the number of collected CRCs to 60
>   kms_pipe_crc_basic: Add a bit a debugging output
>   lib: Add a --igt-debug command line option

So merged 4 of those patches, excluding the ones with review comments
from Daniel, there are not the meat of the series anyway.

-- 
Damien

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

end of thread, other threads:[~2014-07-07 16:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-28 18:23 [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau
2014-05-28 18:23 ` [PATCH 1/6] kms_pipe_crc_basic: Split the main test function a bit more Damien Lespiau
2014-05-28 18:24 ` [PATCH 2/6] kms_pipe_crc_basic: Cycle between 2 differently colored buffer Damien Lespiau
2014-05-28 18:24 ` [PATCH 3/6] kms_pipe_crc_basic: Make the number of CRCs a parameter Damien Lespiau
2014-05-28 18:24 ` [PATCH 4/6] kms_pipe_crc_basic: Bump the number of collected CRCs to 60 Damien Lespiau
2014-05-28 19:32   ` Daniel Vetter
2014-05-28 18:24 ` [PATCH 5/6] kms_pipe_crc_basic: Add a bit a debugging output Damien Lespiau
2014-05-28 18:24 ` [PATCH 6/6] lib: Add a --igt-debug command line option Damien Lespiau
2014-05-28 19:28   ` Daniel Vetter
2014-07-07 16:43 ` [PATCH i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW Damien Lespiau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox