[PATCH i-g-t 1/9] tests/kms_psr_sink_crc: Make blt visible to human eyes

[PATCH i-g-t 1/9] tests/kms_psr_sink_crc: Make blt visible to human eyes

[Rodrigo Vivi]

This will allow manual tests when crc isn't available.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 98b60cf..ad440a8 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -123,7 +123,7 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color)
 	COLOR_BLIT_COPY_BATCH_START(0);
 	OUT_BATCH((1 << 24) | (0xf0 << 16) | 0);
 	OUT_BATCH(0);
-	OUT_BATCH(1 << 16 | 4);
+	OUT_BATCH(0xfff << 16 | 0xfff);
 	OUT_RELOC(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
 	OUT_BATCH(color);
 	ADVANCE_BATCH();
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/9] tests/kms_psr_sink_crc: Make render visible to human eyes

[Rodrigo Vivi]

This will allow manual tests when crc isn't available.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index ad440a8..892bba6 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -139,7 +139,7 @@ static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
 	buf->bo = bo;
 	buf->stride = 4096;
 	buf->tiling = I915_TILING_X;
-	buf->size = 4096;
+	buf->size = 4;
 }
 
 static void fill_render(data_t *data, uint32_t handle, unsigned char color)
@@ -167,7 +167,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 	igt_assert(batch);
 
 	rendercopy(batch, NULL,
-		   &src_buf, 0, 0, 1, 1,
+		   &src_buf, 0, 0, 0xff, 0xff,
 		   &dst_buf, 0, 0);
 
 	intel_batchbuffer_free(batch);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/9] tests/kms_psr_sink_crc: Make mmaps visible to human eyes

[Rodrigo Vivi]

this will allow manual tests when crc isn't available.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 892bba6..865ac70 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -83,6 +83,7 @@ typedef struct {
 	drm_intel_bufmgr *bufmgr;
 	struct igt_fb fb_green, fb_white;
 	igt_plane_t *primary, *sprite, *cursor;
+	int mod_size;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -335,19 +336,21 @@ static void test_crc(data_t *data)
 		igt_assert(is_green(crc));
 		break;
 	case MMAP_GTT:
-		ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+				    PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-		memset(ptr, 0, 4);
-		munmap(ptr, 4096);
+		memset(ptr, 0xcc, data->mod_size);
+		munmap(ptr, data->mod_size);
 		break;
 	case MMAP_GTT_WAITING:
-		ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+				    PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 		/* Printing white on white so the screen shouldn't change */
-		memset(ptr, 0xff, 4);
+		memset(ptr, 0xff, data->mod_size);
 		get_sink_crc(data, crc);
 		igt_assert(strcmp(ref_crc, crc) == 0);
 
@@ -355,15 +358,15 @@ static void test_crc(data_t *data)
 		sleep(10);
 
 		/* Now lets print black to change the screen */
-		memset(ptr, 0, 4);
-		munmap(ptr, 4096);
+		memset(ptr, 0, data->mod_size);
+		munmap(ptr, data->mod_size);
 		break;
 	case MMAP_CPU:
-		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, 4096, PROT_WRITE);
+		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size, PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-		memset(ptr, 0, 4);
-		munmap(ptr, 4096);
+		memset(ptr, 0, data->mod_size);
+		munmap(ptr, data->mod_size);
 		gem_sw_finish(data->drm_fd, handle);
 		break;
 	case BLT:
@@ -431,6 +434,9 @@ static void run_test(data_t *data)
 		white_h = mode->hdisplay;
 		white_v = mode->vdisplay;
 
+		/* Ignoring pitch and bpp to avoid changing full screen */
+		data->mod_size = white_h * white_v;
+
 		switch (data->test_plane) {
 		case SPRITE:
 			data->sprite = igt_output_get_plane(output,
@@ -454,6 +460,9 @@ static void run_test(data_t *data)
 			igt_plane_set_fb(data->cursor, NULL);
 			create_cursor_fb(data);
 			igt_plane_set_position(data->cursor, 0, 0);
+
+			/* Cursor is 64 x 64, ignoring pitch and bbp again */
+			data->mod_size = 64 * 64;
 			break;
 		}
 
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/9] tests/kms_psr_sink_crc: Make plane_move visible to human eyes

[Rodrigo Vivi]

this will allow manual tests when crc isn't available.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 865ac70..155c25f 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -377,7 +377,7 @@ static void test_crc(data_t *data)
 		break;
 	case PLANE_MOVE:
 		/* Only in use when testing Sprite and Cursor */
-		igt_plane_set_position(test_plane, 1, 1);
+		igt_plane_set_position(test_plane, 500, 500);
 		igt_display_commit(&data->display);
 		break;
 	case PLANE_ONOFF:
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 5/9] lib/igt_aux: Don't remove echo from terminal

[Rodrigo Vivi]

It is simply annoying to loose terminal echo. When debugging many times you
CTRL+C on terminal to kill test while it is waiting for key and than you
keep without echo. And this function is used whey you are debugging in
manual modes. So let's keep ECHO.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 3051d84..49d1ec4 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -404,7 +404,7 @@ void igt_debug_wait_for_keypress(const char *key)
 
 	tcgetattr ( STDIN_FILENO, &oldt );
 	newt = oldt;
-	newt.c_lflag &= ~( ICANON | ECHO );
+	newt.c_lflag &= ~ICANON;
 	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
 	getchar();
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 6/9] lib: s/IGT_DEBUG_INTERACTIVE/--interactive-debug=var

[Rodrigo Vivi]

Use cmdline variable for interactive debug instead of env var.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c  | 20 ++++++++++----------
 lib/igt_aux.h  |  2 +-
 lib/igt_core.c |  6 ++++++
 lib/igt_core.h |  2 ++
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 49d1ec4..ff668d4 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -372,32 +372,32 @@ void igt_drop_root(void)
 
 /**
  * igt_debug_wait_for_keypress:
- * @key: env var lookup to to enable this wait
+ * @var: var lookup to to enable this wait
  *
  * Waits for a key press when run interactively and when the corresponding debug
- * key is set in the IGT_DEBUG_INTERACTIVE environment variable. Multiple keys
+ * var is set in the --interactive-debug=<var> variable. Multiple keys
  * can be specified as a comma-separated list or alternatively "all" if a wait
- * should happen for all keys.  When not connected to a terminal the environment
- * setting is ignored and execution immediately continues.
+ * should happen for all cases.
+ *
+ * When not connected to a terminal interactive_debug is ignored
+ * and execution immediately continues.
  *
  * This is useful for display tests where under certain situation manual
  * inspection of the display is useful. Or when running a testcase in the
  * background.
  */
-void igt_debug_wait_for_keypress(const char *key)
+void igt_debug_wait_for_keypress(const char *var)
 {
 	struct termios oldt, newt;
-	const char *env;
 
 	if (!isatty(STDIN_FILENO))
 		return;
 
-	env = getenv("IGT_DEBUG_INTERACTIVE");
-
-	if (!env)
+	if (!igt_interactive_debug)
 		return;
 
-	if (!strstr(env, key) && !strstr(env, "all"))
+	if (!strstr(igt_interactive_debug, var) &&
+	    !strstr(igt_interactive_debug, "all"))
 		return;
 
 	igt_info("Press any key to continue ...\n");
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 63e1b06..59022cd 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -60,7 +60,7 @@ void igt_system_suspend_autoresume(void);
 /* dropping priviledges */
 void igt_drop_root(void);
 
-void igt_debug_wait_for_keypress(const char *key);
+void igt_debug_wait_for_keypress(const char *var);
 
 enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_ACTIVE,
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 13a52a5..461b1d3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -225,6 +225,7 @@ enum {
  OPT_RUN_SUBTEST,
  OPT_DESCRIPTION,
  OPT_DEBUG,
+ OPT_INTERACTIVE_DEBUG,
  OPT_HELP = 'h'
 };
 
@@ -391,6 +392,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 	fprintf(f, "  --list-subtests\n"
 		   "  --run-subtest <pattern>\n"
 		   "  --debug\n"
+		   "  --interactive-debug <pattern>\n"
 		   "  --help-description\n"
 		   "  --help\n");
 	if (help_str)
@@ -423,6 +425,7 @@ static int common_init(int argc, char **argv,
 		{"run-subtest", 1, 0, OPT_RUN_SUBTEST},
 		{"help-description", 0, 0, OPT_DESCRIPTION},
 		{"debug", 0, 0, OPT_DEBUG},
+		{"interactive-debug", 1, 0, OPT_INTERACTIVE_DEBUG},
 		{"help", 0, 0, OPT_HELP},
 		{0, 0, 0, 0}
 	};
@@ -508,6 +511,9 @@ static int common_init(int argc, char **argv,
 	while ((c = getopt_long(argc, argv, short_opts, combined_opts,
 			       &option_index)) != -1) {
 		switch(c) {
+		case OPT_INTERACTIVE_DEBUG:
+			igt_interactive_debug = strdup(optarg);;
+			break;
 		case OPT_DEBUG:
 			igt_log_level = IGT_LOG_DEBUG;
 			break;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index a258348..20942e4 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -511,6 +511,8 @@ bool igt_run_in_simulation(void);
 
 void igt_skip_on_simulation(void);
 
+const char *igt_interactive_debug;
+
 /* structured logging */
 enum igt_log_level {
 	IGT_LOG_DEBUG,
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 7/9] tests/kms_psr_sink_crc: Add manual mode.

[Rodrigo Vivi]

Sink CRC is the most reliable way to test PSR. However in some platforms
apparently auto generated packages force panel to keep calculating CRC invalidating
our current sink crc check over debugfs.

So, this manual test help us to find possible gaps on this platforms where we cannot
trust on sink crc checks.

v2: Accept Daniel's suggestions:
    * Avoid strcpy
    * don't override assert definition
    * Make --interactive-debug for every testcases instead using local --manual

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 155c25f..e7a2ef4 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -244,6 +244,9 @@ static void get_sink_crc(data_t *data, char *crc) {
 	int ret;
 	FILE *file;
 
+	if (igt_interactive_debug)
+		return;
+
 	file = igt_debugfs_fopen("i915_sink_crc_eDP1", "r");
 	igt_require(file);
 
@@ -271,6 +274,9 @@ static bool is_green(char *crc)
 	unsigned int rh, gh, bh, mask;
 	int ret;
 
+	if (igt_interactive_debug)
+		return false;
+
 	sscanf(color_mask, "%4x", &mask);
 
 	memcpy(rs, &crc[0], 4);
@@ -293,6 +299,16 @@ static bool is_green(char *crc)
 		(bh & mask) == 0);
 }
 
+static void assert_or_manual(bool condition, const char *expected)
+{
+	if (igt_interactive_debug)
+		igt_info("Is %s?\n", expected);
+	else
+		igt_debug("%s\n", expected);
+	igt_debug_wait_for_keypress("manual");
+	igt_assert(igt_interactive_debug || condition);
+}
+
 static void test_crc(data_t *data)
 {
 	uint32_t handle = data->fb_white.gem_handle;
@@ -300,18 +316,19 @@ static void test_crc(data_t *data)
 	void *ptr;
 	char ref_crc[12];
 	char crc[12];
+	const char *expected = "";
 
 	igt_plane_set_fb(data->primary, &data->fb_green);
 	igt_display_commit(&data->display);
 
 	/* Confirm that screen became Green */
 	get_sink_crc(data, ref_crc);
-	igt_assert(is_green(ref_crc));
+	assert_or_manual(is_green(ref_crc), "screen GREEN");
 
 	/* Confirm screen stays Green after PSR got active */
 	igt_assert(wait_psr_entry(data, 10));
 	get_sink_crc(data, ref_crc);
-	igt_assert(is_green(ref_crc));
+	assert_or_manual(is_green(ref_crc), "screen GREEN");
 
 	/* Setting a secondary fb/plane */
 	switch (data->test_plane) {
@@ -325,7 +342,10 @@ static void test_crc(data_t *data)
 	/* Confirm it is not Green anymore */
 	igt_assert(wait_psr_entry(data, 10));
 	get_sink_crc(data, ref_crc);
-	igt_assert(!is_green(ref_crc));
+	if (data->test_plane == PRIMARY)
+		assert_or_manual(!is_green(ref_crc), "screen WHITE");
+	else
+		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
 
 	switch (data->op) {
 	case PAGE_FLIP:
@@ -333,7 +353,8 @@ static void test_crc(data_t *data)
 		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
 					   data->fb_green.fb_id, 0, NULL) == 0);
 		get_sink_crc(data, crc);
-		igt_assert(is_green(crc));
+		assert_or_manual(is_green(crc), "screen GREEN");
+		expected = "still GREEN";
 		break;
 	case MMAP_GTT:
 		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -342,6 +363,7 @@ static void test_crc(data_t *data)
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 		memset(ptr, 0xcc, data->mod_size);
 		munmap(ptr, data->mod_size);
+		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case MMAP_GTT_WAITING:
 		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -352,7 +374,11 @@ static void test_crc(data_t *data)
 		/* Printing white on white so the screen shouldn't change */
 		memset(ptr, 0xff, data->mod_size);
 		get_sink_crc(data, crc);
-		igt_assert(strcmp(ref_crc, crc) == 0);
+		if (data->test_plane == PRIMARY)
+			assert_or_manual(strcmp(ref_crc, crc) == 0, "screen WHITE");
+		else
+			assert_or_manual(strcmp(ref_crc, crc) == 0,
+			       "GREEN background with WHITE box");
 
 		igt_info("Waiting 10s...\n");
 		sleep(10);
@@ -360,6 +386,7 @@ static void test_crc(data_t *data)
 		/* Now lets print black to change the screen */
 		memset(ptr, 0, data->mod_size);
 		munmap(ptr, data->mod_size);
+		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case MMAP_CPU:
 		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size, PROT_WRITE);
@@ -368,26 +395,31 @@ static void test_crc(data_t *data)
 		memset(ptr, 0, data->mod_size);
 		munmap(ptr, data->mod_size);
 		gem_sw_finish(data->drm_fd, handle);
+		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case BLT:
 		fill_blt(data, handle, 0);
+		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case RENDER:
 		fill_render(data, handle, 0);
+		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case PLANE_MOVE:
 		/* Only in use when testing Sprite and Cursor */
 		igt_plane_set_position(test_plane, 500, 500);
 		igt_display_commit(&data->display);
+		expected = "White box moved to 500x500";
 		break;
 	case PLANE_ONOFF:
 		/* Only in use when testing Sprite and Cursor */
 		igt_plane_set_fb(test_plane, NULL);
 		igt_display_commit(&data->display);
+		expected = "screen GREEN";
 		break;
 	}
 	get_sink_crc(data, crc);
-	igt_assert(strcmp(ref_crc, crc) != 0);
+	assert_or_manual(strcmp(ref_crc, crc) != 0, expected);
 }
 
 static void test_cleanup(data_t *data) {
@@ -507,6 +539,10 @@ int main(int argc, char *argv[])
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
+		if ((IS_VALLEYVIEW(data.devid) || IS_CHERRYVIEW(data.devid)) &&
+		    !igt_interactive_debug)
+			igt_skip("Sink CRC is unreliable on this platform when PSR is enabled. Test available only on manual interactive debug mode. Run again with --interactive-debug=manual\n");
+
 		igt_skip_on(!psr_enabled(&data));
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 8/9] lib/igt_aux: Extend igt_debug_wait_for_keypres.

[Rodrigo Vivi]

Return key pressed and allow different messages.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c            | 9 ++++++---
 lib/igt_aux.h            | 2 +-
 lib/igt_kms.c            | 3 ++-
 tests/kms_psr_sink_crc.c | 5 +++--
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ff668d4..a6c5ba6 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -386,9 +386,10 @@ void igt_drop_root(void)
  * inspection of the display is useful. Or when running a testcase in the
  * background.
  */
-void igt_debug_wait_for_keypress(const char *var)
+char igt_debug_wait_for_keypress(const char *var, const char *msg)
 {
 	struct termios oldt, newt;
+	char key;
 
 	if (!isatty(STDIN_FILENO))
 		return;
@@ -400,14 +401,16 @@ void igt_debug_wait_for_keypress(const char *var)
 	    !strstr(igt_interactive_debug, "all"))
 		return;
 
-	igt_info("Press any key to continue ...\n");
+	igt_info(msg);
 
 	tcgetattr ( STDIN_FILENO, &oldt );
 	newt = oldt;
 	newt.c_lflag &= ~ICANON;
 	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
-	getchar();
+	key = getchar();
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
+
+	return key;
 }
 
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 59022cd..158e4c5 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -60,7 +60,7 @@ void igt_system_suspend_autoresume(void);
 /* dropping priviledges */
 void igt_drop_root(void);
 
-void igt_debug_wait_for_keypress(const char *var);
+char igt_debug_wait_for_keypress(const char *var, const char *msg);
 
 enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_ACTIVE,
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 1387d01..5c7b2c7 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1561,7 +1561,8 @@ static int do_display_commit(igt_display_t *display,
 
 	LOG_UNINDENT(display);
 
-	igt_debug_wait_for_keypress("modeset");
+	igt_debug_wait_for_keypress("modeset",
+				    "Press any key to continue ...\n");
 
 	return 0;
 }
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index e7a2ef4..8103ebb 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -256,7 +256,7 @@ static void get_sink_crc(data_t *data, char *crc) {
 	fclose(file);
 
 	igt_debug("%s\n", crc);
-	igt_debug_wait_for_keypress("crc");
+	igt_debug_wait_for_keypress("crc", "Press any key to continue ...\n");
 
 	/* The important value was already taken.
 	 * Now give a time for human eyes
@@ -305,7 +305,8 @@ static void assert_or_manual(bool condition, const char *expected)
 		igt_info("Is %s?\n", expected);
 	else
 		igt_debug("%s\n", expected);
-	igt_debug_wait_for_keypress("manual");
+	igt_debug_wait_for_keypress("manual",
+				    "Press any key to continue ...\n");
 	igt_assert(igt_interactive_debug || condition);
 }
 
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 9/9] tests/kms_psr_sink_crc: Use pressed key to pass/fail.

[Rodrigo Vivi]

This is useful when 1 person is running all tests and other one is reading log willing
to know what tests passed and which failed. So tester is able to run all tests without
stop and send log to developer.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 8103ebb..305b52d 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -301,12 +301,17 @@ static bool is_green(char *crc)
 
 static void assert_or_manual(bool condition, const char *expected)
 {
-	if (igt_interactive_debug)
-		igt_info("Is %s?\n", expected);
-	else
-		igt_debug("%s\n", expected);
-	igt_debug_wait_for_keypress("manual",
-				    "Press any key to continue ...\n");
+	char msg[50];
+	char c;
+
+	igt_debug("%s\n", expected);
+
+	sprintf(msg, "Is %s [Y/n]?\n", expected);
+	c = igt_debug_wait_for_keypress("manual", msg);
+
+	if (c && (c == 'n' || c == 'N'))
+		igt_fail(-1);
+
 	igt_assert(igt_interactive_debug || condition);
 }
 
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 6/9] lib: s/IGT_DEBUG_INTERACTIVE/--interactive-debug=var

[Daniel Vetter]

On Tue, Dec 09, 2014 at 09:01:54PM -0500, Rodrigo Vivi wrote:
> Use cmdline variable for interactive debug instead of env var.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Looks nice, two small comments below.

> ---
>  lib/igt_aux.c  | 20 ++++++++++----------
>  lib/igt_aux.h  |  2 +-
>  lib/igt_core.c |  6 ++++++
>  lib/igt_core.h |  2 ++
>  4 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 49d1ec4..ff668d4 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -372,32 +372,32 @@ void igt_drop_root(void)
>  
>  /**
>   * igt_debug_wait_for_keypress:
> - * @key: env var lookup to to enable this wait
> + * @var: var lookup to to enable this wait
>   *
>   * Waits for a key press when run interactively and when the corresponding debug
> - * key is set in the IGT_DEBUG_INTERACTIVE environment variable. Multiple keys
> + * var is set in the --interactive-debug=<var> variable. Multiple keys
>   * can be specified as a comma-separated list or alternatively "all" if a wait
> - * should happen for all keys.  When not connected to a terminal the environment
> - * setting is ignored and execution immediately continues.
> + * should happen for all cases.
> + *
> + * When not connected to a terminal interactive_debug is ignored
> + * and execution immediately continues.
>   *
>   * This is useful for display tests where under certain situation manual
>   * inspection of the display is useful. Or when running a testcase in the
>   * background.
>   */
> -void igt_debug_wait_for_keypress(const char *key)
> +void igt_debug_wait_for_keypress(const char *var)
>  {
>  	struct termios oldt, newt;
> -	const char *env;
>  
>  	if (!isatty(STDIN_FILENO))
>  		return;
>  
> -	env = getenv("IGT_DEBUG_INTERACTIVE");
> -
> -	if (!env)
> +	if (!igt_interactive_debug)
>  		return;
>  
> -	if (!strstr(env, key) && !strstr(env, "all"))
> +	if (!strstr(igt_interactive_debug, var) &&
> +	    !strstr(igt_interactive_debug, "all"))
>  		return;
>  
>  	igt_info("Press any key to continue ...\n");
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 63e1b06..59022cd 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -60,7 +60,7 @@ void igt_system_suspend_autoresume(void);
>  /* dropping priviledges */
>  void igt_drop_root(void);
>  
> -void igt_debug_wait_for_keypress(const char *key);
> +void igt_debug_wait_for_keypress(const char *var);
>  
>  enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 13a52a5..461b1d3 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -225,6 +225,7 @@ enum {
>   OPT_RUN_SUBTEST,
>   OPT_DESCRIPTION,
>   OPT_DEBUG,
> + OPT_INTERACTIVE_DEBUG,
>   OPT_HELP = 'h'
>  };
>  
> @@ -391,6 +392,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>  	fprintf(f, "  --list-subtests\n"
>  		   "  --run-subtest <pattern>\n"
>  		   "  --debug\n"
> +		   "  --interactive-debug <pattern>\n"
>  		   "  --help-description\n"
>  		   "  --help\n");
>  	if (help_str)
> @@ -423,6 +425,7 @@ static int common_init(int argc, char **argv,
>  		{"run-subtest", 1, 0, OPT_RUN_SUBTEST},
>  		{"help-description", 0, 0, OPT_DESCRIPTION},
>  		{"debug", 0, 0, OPT_DEBUG},
> +		{"interactive-debug", 1, 0, OPT_INTERACTIVE_DEBUG},
>  		{"help", 0, 0, OPT_HELP},
>  		{0, 0, 0, 0}
>  	};
> @@ -508,6 +511,9 @@ static int common_init(int argc, char **argv,
>  	while ((c = getopt_long(argc, argv, short_opts, combined_opts,
>  			       &option_index)) != -1) {
>  		switch(c) {
> +		case OPT_INTERACTIVE_DEBUG:
> +			igt_interactive_debug = strdup(optarg);;
> +			break;

Hm, while we change this to cmdline option I think we should make the
optarg optional and if it's not set, set it to "all".

>  		case OPT_DEBUG:
>  			igt_log_level = IGT_LOG_DEBUG;
>  			break;
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index a258348..20942e4 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -511,6 +511,8 @@ bool igt_run_in_simulation(void);
>  
>  void igt_skip_on_simulation(void);
>  
> +const char *igt_interactive_debug;

Can you please add a bit of gtkdoc for this, too, now that it's exported.

Thanks, Daniel

> +
>  /* structured logging */
>  enum igt_log_level {
>  	IGT_LOG_DEBUG,
> -- 
> 1.9.3
> 
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 8/9] lib/igt_aux: Extend igt_debug_wait_for_keypres.

[Daniel Vetter]

On Tue, Dec 09, 2014 at 09:01:56PM -0500, Rodrigo Vivi wrote:
> Return key pressed and allow different messages.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Hm, not sure how useful this really is - when the test fails I just hit ^C
and scream ;-)

What kind of use-case do you have in mind which requires such a structured
approach? In case of doubt I prefer my igt helpers to be really simple, so
if you think we really needs this we should add a new fancy version of
wait_for_keypress with the added msg and return code.
-Daniel

> ---
>  lib/igt_aux.c            | 9 ++++++---
>  lib/igt_aux.h            | 2 +-
>  lib/igt_kms.c            | 3 ++-
>  tests/kms_psr_sink_crc.c | 5 +++--
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index ff668d4..a6c5ba6 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -386,9 +386,10 @@ void igt_drop_root(void)
>   * inspection of the display is useful. Or when running a testcase in the
>   * background.
>   */
> -void igt_debug_wait_for_keypress(const char *var)
> +char igt_debug_wait_for_keypress(const char *var, const char *msg)
>  {
>  	struct termios oldt, newt;
> +	char key;
>  
>  	if (!isatty(STDIN_FILENO))
>  		return;
> @@ -400,14 +401,16 @@ void igt_debug_wait_for_keypress(const char *var)
>  	    !strstr(igt_interactive_debug, "all"))
>  		return;
>  
> -	igt_info("Press any key to continue ...\n");
> +	igt_info(msg);
>  
>  	tcgetattr ( STDIN_FILENO, &oldt );
>  	newt = oldt;
>  	newt.c_lflag &= ~ICANON;
>  	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
> -	getchar();
> +	key = getchar();
>  	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
> +
> +	return key;
>  }
>  
>  #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 59022cd..158e4c5 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -60,7 +60,7 @@ void igt_system_suspend_autoresume(void);
>  /* dropping priviledges */
>  void igt_drop_root(void);
>  
> -void igt_debug_wait_for_keypress(const char *var);
> +char igt_debug_wait_for_keypress(const char *var, const char *msg);
>  
>  enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 1387d01..5c7b2c7 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1561,7 +1561,8 @@ static int do_display_commit(igt_display_t *display,
>  
>  	LOG_UNINDENT(display);
>  
> -	igt_debug_wait_for_keypress("modeset");
> +	igt_debug_wait_for_keypress("modeset",
> +				    "Press any key to continue ...\n");
>  
>  	return 0;
>  }
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index e7a2ef4..8103ebb 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -256,7 +256,7 @@ static void get_sink_crc(data_t *data, char *crc) {
>  	fclose(file);
>  
>  	igt_debug("%s\n", crc);
> -	igt_debug_wait_for_keypress("crc");
> +	igt_debug_wait_for_keypress("crc", "Press any key to continue ...\n");
>  
>  	/* The important value was already taken.
>  	 * Now give a time for human eyes
> @@ -305,7 +305,8 @@ static void assert_or_manual(bool condition, const char *expected)
>  		igt_info("Is %s?\n", expected);
>  	else
>  		igt_debug("%s\n", expected);
> -	igt_debug_wait_for_keypress("manual");
> +	igt_debug_wait_for_keypress("manual",
> +				    "Press any key to continue ...\n");
>  	igt_assert(igt_interactive_debug || condition);
>  }
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx