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

[PATCH i-g-t 01/10] 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 ba6fb1d..0a56705 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 02/10] 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 0a56705..5085fb3 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 03/10] 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 5085fb3..dbc09e4 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)
@@ -350,19 +351,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);
 
@@ -370,15 +373,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:
@@ -447,6 +450,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,
@@ -471,6 +477,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 04/10] 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 dbc09e4..b7873b4 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -392,7 +392,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 05/10] 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

v3: Sink CRC can be unreliable for other platforms as well so let's skip and warn
    when we detect the misbehaviour instead hardcoded per platform.

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index b7873b4..24f5ca8 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -259,11 +259,14 @@ 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);
 
 	ret = fscanf(file, "%s\n", crc);
-	igt_require(ret > 0);
+	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=manual\n");
 
 	fclose(file);
 
@@ -286,6 +289,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);
@@ -308,6 +314,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;
@@ -315,18 +331,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) {
@@ -340,7 +357,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:
@@ -348,7 +368,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,
@@ -357,6 +378,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,
@@ -367,7 +389,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);
@@ -375,6 +401,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);
@@ -383,26 +410,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) {
-- 
1.9.3

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

[PATCH i-g-t 06/10] lib/igt_aux: Introduce igt_debug_warn_and_wait_for_key.

[Rodrigo Vivi]

This is an extention of igt_debug_wait_for_keypress that also can have
customized message and return key pressed.

v2: This is actualy a v2. V1 was an extension of original
    igt_debug_wait_for_keypress but it was nacked.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 131ff4b..0783ac2 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -448,6 +448,52 @@ void igt_debug_wait_for_keypress(const char *var)
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
 }
 
+/**
+ * igt_debug_wait_for_key:
+ * @var: var lookup to to enable this wait
+ * @msg: message to be printed before wait for key
+ *
+ * Waits for a key press when run interactively and when the corresponding debug
+ * 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 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.
+ *
+ * Returns: key pressed block
+ */
+char igt_debug_warn_and_wait_for_key(const char *var, const char *msg)
+{
+	struct termios oldt, newt;
+	char key = 0;
+
+	if (!isatty(STDIN_FILENO))
+		return key;
+
+	if (!igt_interactive_debug)
+		return key;
+
+	if (!strstr(igt_interactive_debug, var) &&
+	    !strstr(igt_interactive_debug, "all"))
+		return key;
+
+	igt_info("%s", msg);
+
+	tcgetattr ( STDIN_FILENO, &oldt );
+	newt = oldt;
+	newt.c_lflag &= ~ICANON;
+	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
+	key = getchar();
+	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
+
+	return key;
+}
+
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
 /* We just leak this on exit ... */
 int pm_status_fd = -1;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 0c361f2..146712c 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
 void igt_drop_root(void);
 
 void igt_debug_wait_for_keypress(const char *var);
+char igt_debug_warn_and_wait_for_key(const char *var, const char *msg);
 
 enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_ACTIVE,
-- 
1.9.3

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

[PATCH i-g-t 07/10] 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.

v2: Rebased after igt_debug_warn_and_wait_for_key

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 24f5ca8..ce50cdd 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -316,11 +316,21 @@ 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");
+	char msg[50];
+	char c;
+
+	igt_debug("%s\n", expected);
+
+	sprintf(msg, "Is %s [Y/n]? ", expected);
+	c = igt_debug_warn_and_wait_for_key("manual", msg);
+
+	if (c) {
+		if (c == 'n' || c == 'N')
+			igt_fail(-1);
+		else
+			igt_info("\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 08/10] tests/kms_psr_sink_crc: remove timeout option from wait_psr_entry.

[Rodrigo Vivi]

No functional changes. Just making timeout unique for any case.

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index ce50cdd..733083a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -245,8 +245,9 @@ static bool psr_active(data_t *data)
 	return strcmp(str, "yes") == 0;
 }
 
-static bool wait_psr_entry(data_t *data, int timeout)
+static bool wait_psr_entry(data_t *data)
 {
+	int timeout = 10;
 	while (timeout--) {
 		if (psr_active(data))
 			return true;
@@ -351,7 +352,7 @@ static void test_crc(data_t *data)
 	assert_or_manual(is_green(ref_crc), "screen GREEN");
 
 	/* Confirm screen stays Green after PSR got active */
-	igt_assert(wait_psr_entry(data, 10));
+	igt_assert(wait_psr_entry(data));
 	get_sink_crc(data, ref_crc);
 	assert_or_manual(is_green(ref_crc), "screen GREEN");
 
@@ -365,7 +366,7 @@ static void test_crc(data_t *data)
 	igt_display_commit(&data->display);
 
 	/* Confirm it is not Green anymore */
-	igt_assert(wait_psr_entry(data, 10));
+	igt_assert(wait_psr_entry(data));
 	get_sink_crc(data, ref_crc);
 	if (data->test_plane == PRIMARY)
 		assert_or_manual(!is_green(ref_crc), "screen WHITE");
-- 
1.9.3

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

[PATCH i-g-t 09/10] test/kms_psr_sink_crc: Split plane setup operations

[Rodrigo Vivi]

No functional changes. This reorg will allow to do some
operations like dpms off/on with different places to wait
for psr to get active.

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 733083a..8d09b0a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -84,6 +84,8 @@ typedef struct {
 	struct igt_fb fb_green, fb_white;
 	igt_plane_t *primary, *sprite, *cursor;
 	int mod_size;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -101,9 +103,32 @@ static void create_cursor_fb(data_t *data)
 	igt_assert(cairo_status(cr) == 0);
 }
 
+
+static void setup_output(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+
+	for_each_connected_output(display, output) {
+		drmModeConnectorPtr c = output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
+		    c->connection != DRM_MODE_CONNECTED)
+			continue;
+
+		igt_output_set_pipe(output, PIPE_ANY);
+		data->crtc_id = output->config.crtc->crtc_id;
+		data->output = output;
+		data->mode = igt_output_get_mode(output);
+
+		return;
+	}
+}
+
 static void display_init(data_t *data)
 {
 	igt_display_init(&data->display, data->drm_fd);
+	setup_output(data);
 }
 
 static void display_fini(data_t *data)
@@ -335,7 +360,7 @@ static void assert_or_manual(bool condition, const char *expected)
 	igt_assert(igt_interactive_debug || condition);
 }
 
-static void test_crc(data_t *data)
+static void run_test(data_t *data)
 {
 	uint32_t handle = data->fb_white.gem_handle;
 	igt_plane_t *test_plane;
@@ -344,9 +369,6 @@ static void test_crc(data_t *data)
 	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);
 	assert_or_manual(is_green(ref_crc), "screen GREEN");
@@ -461,77 +483,60 @@ static void test_cleanup(data_t *data) {
 	igt_remove_fb(data->drm_fd, &data->fb_white);
 }
 
-static void run_test(data_t *data)
+static void setup_test_plane(data_t *data)
 {
-	igt_display_t *display = &data->display;
-	igt_output_t *output;
-	drmModeModeInfo *mode;
 	uint32_t white_h, white_v;
 
-	for_each_connected_output(display, output) {
-		drmModeConnectorPtr c = output->config.connector;
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_I915_FORMAT_MOD_X_TILED,
+			    0.0, 1.0, 0.0,
+			    &data->fb_green);
 
-		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
-		    c->connection != DRM_MODE_CONNECTED)
-			continue;
+	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
+	igt_plane_set_fb(data->primary, NULL);
 
-		igt_output_set_pipe(output, PIPE_ANY);
-		data->crtc_id = output->config.crtc->crtc_id;
+	white_h = data->mode->hdisplay;
+	white_v = data->mode->vdisplay;
 
-		mode = igt_output_get_mode(output);
+	/* 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(data->output,
+						    IGT_PLANE_2);
+		igt_plane_set_fb(data->sprite, NULL);
+		/* To make it different for human eyes let's make
+		 * sprite visible in only one quarter of the primary
+		 */
+		white_h = white_h/2;
+		white_v = white_v/2;
+	case PRIMARY:
 		igt_create_color_fb(data->drm_fd,
-				    mode->hdisplay, mode->vdisplay,
+				    white_h, white_v,
 				    DRM_FORMAT_XRGB8888,
 				    LOCAL_I915_FORMAT_MOD_X_TILED,
-				    0.0, 1.0, 0.0,
-				    &data->fb_green);
-
-		data->primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-		igt_plane_set_fb(data->primary, NULL);
-
-		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,
-							    IGT_PLANE_2);
-			igt_plane_set_fb(data->sprite, NULL);
-			/* To make it different for human eyes let's make
-			 * sprite visible in only one quarter of the primary
-			 */
-			white_h = white_h/2;
-			white_v = white_v/2;
-		case PRIMARY:
-			igt_create_color_fb(data->drm_fd,
-					    white_h, white_v,
-					    DRM_FORMAT_XRGB8888,
-					    LOCAL_I915_FORMAT_MOD_X_TILED,
-					    1.0, 1.0, 1.0,
-					    &data->fb_white);
-			break;
-		case CURSOR:
-			data->cursor = igt_output_get_plane(output,
-							    IGT_PLANE_CURSOR);
-			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.0, 1.0, 1.0,
+				    &data->fb_white);
+		break;
+	case CURSOR:
+		data->cursor = igt_output_get_plane(data->output,
+						    IGT_PLANE_CURSOR);
+		igt_plane_set_fb(data->cursor, NULL);
+		create_cursor_fb(data);
+		igt_plane_set_position(data->cursor, 0, 0);
 
-		igt_display_commit(&data->display);
+		/* Cursor is 64 x 64, ignoring pitch and bbp again */
+		data->mod_size = 64 * 64;
+		break;
+	}
 
-		test_crc(data);
+	igt_display_commit(&data->display);
 
-		test_cleanup(data);
-	}
+	igt_plane_set_fb(data->primary, &data->fb_green);
+	igt_display_commit(&data->display);
 }
 
 static int opt_handler(int opt, int opt_index)
@@ -580,7 +585,10 @@ int main(int argc, char *argv[])
 		igt_subtest_f("primary_%s", op_str(op)) {
 			data.test_plane = PRIMARY;
 			data.op = op;
+			setup_test_plane(&data);
+			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
+			test_cleanup(&data);
 		}
 	}
 
@@ -588,7 +596,10 @@ int main(int argc, char *argv[])
 		igt_subtest_f("sprite_%s", op_str(op)) {
 			data.test_plane = SPRITE;
 			data.op = op;
+			setup_test_plane(&data);
+			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
+			test_cleanup(&data);
 		}
 	}
 
@@ -596,7 +607,10 @@ int main(int argc, char *argv[])
 		igt_subtest_f("cursor_%s", op_str(op)) {
 			data.test_plane = CURSOR;
 			data.op = op;
+			setup_test_plane(&data);
+			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
+			test_cleanup(&data);
 		}
 	}
 
-- 
1.9.3

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

[PATCH i-g-t 10/10] test/kms_psr_sink_crc: Add dpms off/on tests.

[Rodrigo Vivi]

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 8d09b0a..38013df 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -539,6 +539,15 @@ static void setup_test_plane(data_t *data)
 	igt_display_commit(&data->display);
 }
 
+static void dpms_off_on(data_t data)
+{
+	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+				   DRM_MODE_DPMS_OFF);
+	sleep(1);
+	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+				   DRM_MODE_DPMS_ON);
+}
+
 static int opt_handler(int opt, int opt_index)
 {
 	switch (opt) {
@@ -614,6 +623,30 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	igt_subtest_f("dpms_off_psr_active") {
+		data.test_plane = PRIMARY;
+		data.op = RENDER;
+		setup_test_plane(&data);
+		igt_assert(wait_psr_entry(&data));
+
+		dpms_off_on(data);
+
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
+	igt_subtest_f("dpms_off_psr_exit") {
+		data.test_plane = SPRITE;
+		data.op = PLANE_ONOFF;
+		setup_test_plane(&data);
+
+		dpms_off_on(data);
+
+		igt_assert(wait_psr_entry(&data));
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
 	igt_fixture {
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
-- 
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 02/10] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Fri, Mar 13, 2015 at 06:19:19PM -0400, Rodrigo Vivi wrote:
> 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 0a56705..5085fb3 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;

4 looks a bit small ...
-Daniel

>  }
>  
>  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

-- 
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 07/10] tests/kms_psr_sink_crc: Use pressed key to pass/fail.

[Daniel Vetter]

On Fri, Mar 13, 2015 at 06:19:24PM -0400, Rodrigo Vivi wrote:
> 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.
> 
> v2: Rebased after igt_debug_warn_and_wait_for_key
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 24f5ca8..ce50cdd 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -316,11 +316,21 @@ 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");
> +	char msg[50];
> +	char c;
> +
> +	igt_debug("%s\n", expected);
> +
> +	sprintf(msg, "Is %s [Y/n]? ", expected);
> +	c = igt_debug_warn_and_wait_for_key("manual", msg);
> +
> +	if (c) {
> +		if (c == 'n' || c == 'N')
> +			igt_fail(-1);
> +		else
> +			igt_info("\n");
> +	}

Imo it'd be better to move this check into the helper too. And then give
it a name that makes it clear that it's a checkpoint for the user for
manual testing. I don't have any good ideas, but maybe
igt_interactive_debug_manual_check?

And imo you can fold this one in with the previous patch - easier to
understand if you add the user together with the function.

And one more aside: Should we document the different interactive debug
thigns somewhere?
-Daniel
-- 
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 07/10] tests/kms_psr_sink_crc: Use pressed key to pass/fail.

[Daniel Vetter]

On Mon, Mar 16, 2015 at 10:04:28AM +0100, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 06:19:24PM -0400, Rodrigo Vivi wrote:
> > 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.
> > 
> > v2: Rebased after igt_debug_warn_and_wait_for_key
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 24f5ca8..ce50cdd 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -316,11 +316,21 @@ 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");
> > +	char msg[50];
> > +	char c;
> > +
> > +	igt_debug("%s\n", expected);
> > +
> > +	sprintf(msg, "Is %s [Y/n]? ", expected);
> > +	c = igt_debug_warn_and_wait_for_key("manual", msg);
> > +
> > +	if (c) {
> > +		if (c == 'n' || c == 'N')
> > +			igt_fail(-1);
> > +		else
> > +			igt_info("\n");
> > +	}
> 
> Imo it'd be better to move this check into the helper too. And then give
> it a name that makes it clear that it's a checkpoint for the user for
> manual testing. I don't have any good ideas, but maybe
> igt_interactive_debug_manual_check?

Maybe better igt_debug_manual_check for consistency with
igt_debug_wait_for_keypress.
-Daniel
-- 
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 02/10] tests/kms_psr_sink_crc: Make render visible to human eyes

[Vivi, Rodrigo]

On Mon, 2015-03-16 at 09:59 +0100, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 06:19:19PM -0400, Rodrigo Vivi wrote:
> > 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 0a56705..5085fb3 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;
> 
> 4 looks a bit small ...

agree, but as far as I can remember with 4 I can have visible changes
and with 4096 I couldn't see any thing on screen even with psr
disabled. 

> -Daniel
> 
> >  }
> >  
> >  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
> 

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

[PATCH] lib/igt_aux: Introduce igt_interactive_debug_manual_check.

[Rodrigo Vivi]

This is an extention of igt_debug_wait_for_keypress that also can have
customized message and return key pressed.

v2: This is actualy a v2. V1 was an extension of original
    igt_debug_wait_for_keypress but it was nacked.

v3: Make [Y/n] check inside aux function as suggested by Daniel.
    Also renaming and adding first use case along with the axu function.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c            | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h            |  1 +
 tests/kms_psr_sink_crc.c | 11 ++++------
 3 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 131ff4b..8b08678 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -448,6 +448,63 @@ void igt_debug_wait_for_keypress(const char *var)
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
 }
 
+/**
+ * igt_interactive_debug_manual_check:
+ * @var: var lookup to to enable this wait
+ * @expected: message to be printed as expected behaviour before wait for keys Y/n
+ *
+ * Waits for a key press when run interactively and when the corresponding debug
+ * var is set in the --interactive-debug=<var> variable. Multiple vars
+ * can be specified as a comma-separated list or alternatively "all" if a wait
+ * should happen for all cases.
+ *
+ * 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.
+ *
+ * When not connected to a terminal interactive_debug is ignored
+ * and execution immediately continues. For this reason by default this function
+ * returns true. It returns false only when N/n is pressed indicating the
+ * user ins't seeing what was expected.
+ *
+ * Returns: False on N/n is pressed. True otherwise.
+ */
+bool igt_interactive_debug_manual_check(const char *var, const char *expected)
+{
+	struct termios oldt, newt;
+	char key;
+
+	if (!isatty(STDIN_FILENO))
+		return true;
+
+	if (!igt_interactive_debug)
+		return true;
+
+	if (!strstr(igt_interactive_debug, var) &&
+	    !strstr(igt_interactive_debug, "all"))
+		return true;
+
+	igt_info("Is %s [Y/n]", expected);
+
+	tcgetattr ( STDIN_FILENO, &oldt );
+	newt = oldt;
+	newt.c_lflag &= ~ICANON;
+	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
+	key = getchar();
+	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
+
+
+	if (key) {
+		if (key == 'n' || key == 'N') {
+			igt_info("\n");
+			return false;
+		} else
+			igt_info("\n");
+	}
+
+	return true;
+}
+
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
 /* We just leak this on exit ... */
 int pm_status_fd = -1;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 0c361f2..de909a1 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
 void igt_drop_root(void);
 
 void igt_debug_wait_for_keypress(const char *var);
+bool igt_interactive_debug_manual_check(const char *var, const char *expected);
 
 enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_ACTIVE,
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 24f5ca8..fcf9c66 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -266,7 +266,7 @@ static void get_sink_crc(data_t *data, char *crc) {
 	igt_require(file);
 
 	ret = fscanf(file, "%s\n", crc);
-	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=manual\n");
+	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
 
 	fclose(file);
 
@@ -316,12 +316,9 @@ 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");
-	igt_assert(igt_interactive_debug || condition);
+	int ret;
+	ret = igt_interactive_debug_manual_check("no-crc", expected);
+	igt_assert(ret || condition);
 }
 
 static void test_crc(data_t *data)
-- 
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 02/10] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Mon, Mar 16, 2015 at 08:23:19PM +0000, Vivi, Rodrigo wrote:
> On Mon, 2015-03-16 at 09:59 +0100, Daniel Vetter wrote:
> > On Fri, Mar 13, 2015 at 06:19:19PM -0400, Rodrigo Vivi wrote:
> > > 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 0a56705..5085fb3 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;
> > 
> > 4 looks a bit small ...
> 
> agree, but as far as I can remember with 4 I can have visible changes
> and with 4096 I couldn't see any thing on screen even with psr
> disabled. 

Well it's not even used in the rendercopy code. And a size of 4 really
doesn't make any sense at all. And the visible part is just 1 pixel only
anyway, so pretty hard to spot.

The problem could be that the buffer is too small actually, since you have
only a 1 line in total. The 3d sampler usually insists upon more. Maybe
reduce the stride a bit instead?
-Daniel

> 
> > -Daniel
> > 
> > >  }
> > >  
> > >  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
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] lib/igt_aux: Introduce igt_interactive_debug_manual_check.

[Daniel Vetter]

On Mon, Mar 16, 2015 at 06:44:05PM -0400, Rodrigo Vivi wrote:
> This is an extention of igt_debug_wait_for_keypress that also can have
> customized message and return key pressed.
> 
> v2: This is actualy a v2. V1 was an extension of original
>     igt_debug_wait_for_keypress but it was nacked.
> 
> v3: Make [Y/n] check inside aux function as suggested by Daniel.
>     Also renaming and adding first use case along with the axu function.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  lib/igt_aux.c            | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_aux.h            |  1 +
>  tests/kms_psr_sink_crc.c | 11 ++++------
>  3 files changed, 62 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 131ff4b..8b08678 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -448,6 +448,63 @@ void igt_debug_wait_for_keypress(const char *var)
>  	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
>  }
>  
> +/**
> + * igt_interactive_debug_manual_check:
> + * @var: var lookup to to enable this wait
> + * @expected: message to be printed as expected behaviour before wait for keys Y/n
> + *
> + * Waits for a key press when run interactively and when the corresponding debug
> + * var is set in the --interactive-debug=<var> variable. Multiple vars
> + * can be specified as a comma-separated list or alternatively "all" if a wait
> + * should happen for all cases.
> + *
> + * 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.
> + *
> + * When not connected to a terminal interactive_debug is ignored
> + * and execution immediately continues. For this reason by default this function
> + * returns true. It returns false only when N/n is pressed indicating the
> + * user ins't seeing what was expected.
> + *
> + * Returns: False on N/n is pressed. True otherwise.
> + */
> +bool igt_interactive_debug_manual_check(const char *var, const char *expected)

See my other mail, but for consistency I think we should call this
igt_debug_manual_check. That way we match the prefix of
igt_debug_wait_for_keypress.

> +{
> +	struct termios oldt, newt;
> +	char key;
> +
> +	if (!isatty(STDIN_FILENO))
> +		return true;
> +
> +	if (!igt_interactive_debug)
> +		return true;
> +
> +	if (!strstr(igt_interactive_debug, var) &&
> +	    !strstr(igt_interactive_debug, "all"))
> +		return true;
> +
> +	igt_info("Is %s [Y/n]", expected);
> +
> +	tcgetattr ( STDIN_FILENO, &oldt );
> +	newt = oldt;
> +	newt.c_lflag &= ~ICANON;
> +	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
> +	key = getchar();
> +	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
> +
> +
> +	if (key) {
> +		if (key == 'n' || key == 'N') {
> +			igt_info("\n");
> +			return false;
> +		} else
> +			igt_info("\n");
> +	}
> +
> +	return true;
> +}
> +
>  #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
>  /* We just leak this on exit ... */
>  int pm_status_fd = -1;
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 0c361f2..de909a1 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
>  void igt_drop_root(void);
>  
>  void igt_debug_wait_for_keypress(const char *var);
> +bool igt_interactive_debug_manual_check(const char *var, const char *expected);
>  
>  enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 24f5ca8..fcf9c66 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -266,7 +266,7 @@ static void get_sink_crc(data_t *data, char *crc) {
>  	igt_require(file);
>  
>  	ret = fscanf(file, "%s\n", crc);
> -	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=manual\n");
> +	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
>  
>  	fclose(file);
>  
> @@ -316,12 +316,9 @@ 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");
> -	igt_assert(igt_interactive_debug || condition);
> +	int ret;
> +	ret = igt_interactive_debug_manual_check("no-crc", expected);
> +	igt_assert(ret || condition);

Imo the igt assert should be in the helper too, and the helper just has a
return type of void.
-Daniel

>  }
>  
>  static void test_crc(data_t *data)
> -- 
> 1.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] lib/igt_aux: Introduce igt_interactive_debug_manual_check.

[Rodrigo Vivi]

This is an extention of igt_debug_wait_for_keypress that also can have
customized message and return key pressed.

v2: This is actualy a v2. V1 was an extension of original
    igt_debug_wait_for_keypress but it was nacked.

v3: Make [Y/n] check inside aux function as suggested by Daniel.
    Also renaming and adding first use case along with the axu function.

v4: Simplify function name and make it assert pressed key
    is different from n/N as suggested by Daniel.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 lib/igt_aux.c            | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h            |  1 +
 tests/kms_psr_sink_crc.c |  8 ++------
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 131ff4b..788ac3f 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -448,6 +448,56 @@ void igt_debug_wait_for_keypress(const char *var)
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
 }
 
+/**
+ * igt_debug_manual_check:
+ * @var: var lookup to to enable this wait
+ * @expected: message to be printed as expected behaviour before wait for keys Y/n
+ *
+ * Waits for a key press when run interactively and when the corresponding debug
+ * var is set in the --interactive-debug=<var> variable. Multiple vars
+ * can be specified as a comma-separated list or alternatively "all" if a wait
+ * should happen for all cases.
+ *
+ * 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.
+ *
+ * When not connected to a terminal interactive_debug is ignored
+ * and execution immediately continues. For this reason by default this function
+ * returns true. It returns false only when N/n is pressed indicating the
+ * user ins't seeing what was expected.
+ *
+ * Force test fail when N/n is pressed.
+ */
+void igt_debug_manual_check(const char *var, const char *expected)
+{
+	struct termios oldt, newt;
+	char key;
+
+	if (!isatty(STDIN_FILENO))
+		return;
+
+	if (!igt_interactive_debug)
+		return;
+
+	if (!strstr(igt_interactive_debug, var) &&
+	    !strstr(igt_interactive_debug, "all"))
+		return;
+
+	igt_info("Is %s [Y/n]", expected);
+
+	tcgetattr ( STDIN_FILENO, &oldt );
+	newt = oldt;
+	newt.c_lflag &= ~ICANON;
+	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
+	key = getchar();
+	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
+
+	igt_info("\n");
+
+	igt_assert(key != 'n' && key != 'N');
+}
+
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
 /* We just leak this on exit ... */
 int pm_status_fd = -1;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 0c361f2..7a5078b 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
 void igt_drop_root(void);
 
 void igt_debug_wait_for_keypress(const char *var);
+void igt_debug_manual_check(const char *var, const char *expected);
 
 enum igt_runtime_pm_status {
 	IGT_RUNTIME_PM_STATUS_ACTIVE,
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 24f5ca8..b820efc 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -266,7 +266,7 @@ static void get_sink_crc(data_t *data, char *crc) {
 	igt_require(file);
 
 	ret = fscanf(file, "%s\n", crc);
-	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=manual\n");
+	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
 
 	fclose(file);
 
@@ -316,11 +316,7 @@ 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");
+	igt_debug_manual_check("no-crc", expected);
 	igt_assert(igt_interactive_debug || condition);
 }
 
-- 
2.1.0

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

[PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Rodrigo Vivi]

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

v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 0a56705..46e04c8 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -137,9 +137,8 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color)
 static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
 {
 	buf->bo = bo;
-	buf->stride = 4096;
+	buf->stride = 1024;
 	buf->tiling = I915_TILING_X;
-	buf->size = 4096;
 }
 
 static void fill_render(data_t *data, uint32_t handle, unsigned char color)
@@ -167,7 +166,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);
-- 
2.1.0

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

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> This will allow manual tests when crc isn't available.
> 
> v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.

buf->size isn't nonsense really, it does seem to match what we've
allocated. But it also seems unused in the rendercpy functions I've
checked. Imo you should keep that line.
-Daniel

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 0a56705..46e04c8 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -137,9 +137,8 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color)
>  static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
>  {
>  	buf->bo = bo;
> -	buf->stride = 4096;
> +	buf->stride = 1024;
>  	buf->tiling = I915_TILING_X;
> -	buf->size = 4096;
>  }
>  
>  static void fill_render(data_t *data, uint32_t handle, unsigned char color)
> @@ -167,7 +166,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);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] lib/igt_aux: Introduce igt_interactive_debug_manual_check.

[Daniel Vetter]

On Thu, Mar 19, 2015 at 06:11:45PM -0700, Rodrigo Vivi wrote:
> This is an extention of igt_debug_wait_for_keypress that also can have
> customized message and return key pressed.
> 
> v2: This is actualy a v2. V1 was an extension of original
>     igt_debug_wait_for_keypress but it was nacked.
> 
> v3: Make [Y/n] check inside aux function as suggested by Daniel.
>     Also renaming and adding first use case along with the axu function.
> 
> v4: Simplify function name and make it assert pressed key
>     is different from n/N as suggested by Daniel.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

lgtm, thanks.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  lib/igt_aux.c            | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_aux.h            |  1 +
>  tests/kms_psr_sink_crc.c |  8 ++------
>  3 files changed, 53 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 131ff4b..788ac3f 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -448,6 +448,56 @@ void igt_debug_wait_for_keypress(const char *var)
>  	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
>  }
>  
> +/**
> + * igt_debug_manual_check:
> + * @var: var lookup to to enable this wait
> + * @expected: message to be printed as expected behaviour before wait for keys Y/n
> + *
> + * Waits for a key press when run interactively and when the corresponding debug
> + * var is set in the --interactive-debug=<var> variable. Multiple vars
> + * can be specified as a comma-separated list or alternatively "all" if a wait
> + * should happen for all cases.
> + *
> + * 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.
> + *
> + * When not connected to a terminal interactive_debug is ignored
> + * and execution immediately continues. For this reason by default this function
> + * returns true. It returns false only when N/n is pressed indicating the
> + * user ins't seeing what was expected.
> + *
> + * Force test fail when N/n is pressed.
> + */
> +void igt_debug_manual_check(const char *var, const char *expected)
> +{
> +	struct termios oldt, newt;
> +	char key;
> +
> +	if (!isatty(STDIN_FILENO))
> +		return;
> +
> +	if (!igt_interactive_debug)
> +		return;
> +
> +	if (!strstr(igt_interactive_debug, var) &&
> +	    !strstr(igt_interactive_debug, "all"))
> +		return;
> +
> +	igt_info("Is %s [Y/n]", expected);
> +
> +	tcgetattr ( STDIN_FILENO, &oldt );
> +	newt = oldt;
> +	newt.c_lflag &= ~ICANON;
> +	tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
> +	key = getchar();
> +	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
> +
> +	igt_info("\n");
> +
> +	igt_assert(key != 'n' && key != 'N');
> +}
> +
>  #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
>  /* We just leak this on exit ... */
>  int pm_status_fd = -1;
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 0c361f2..7a5078b 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
>  void igt_drop_root(void);
>  
>  void igt_debug_wait_for_keypress(const char *var);
> +void igt_debug_manual_check(const char *var, const char *expected);
>  
>  enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 24f5ca8..b820efc 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -266,7 +266,7 @@ static void get_sink_crc(data_t *data, char *crc) {
>  	igt_require(file);
>  
>  	ret = fscanf(file, "%s\n", crc);
> -	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=manual\n");
> +	igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
>  
>  	fclose(file);
>  
> @@ -316,11 +316,7 @@ 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");
> +	igt_debug_manual_check("no-crc", expected);
>  	igt_assert(igt_interactive_debug || condition);
>  }
>  
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Vivi, Rodrigo]

On Fri, 2015-03-20 at 10:54 +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> > This will allow manual tests when crc isn't available.
> > 
> > v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.
> 
> buf->size isn't nonsense really, it does seem to match what we've
> allocated. But it also seems unused in the rendercpy functions I've
> checked. Imo you should keep that line

oh sure, bad phrase.... the value 4 was nonsense.
But if I increases that it fails. Without setting it works.
What do you think?

> -Daniel
> 
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 0a56705..46e04c8 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -137,9 +137,8 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color)
> >  static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
> >  {
> >  	buf->bo = bo;
> > -	buf->stride = 4096;
> > +	buf->stride = 1024;
> >  	buf->tiling = I915_TILING_X;
> > -	buf->size = 4096;
> >  }
> >  
> >  static void fill_render(data_t *data, uint32_t handle, unsigned char color)
> > @@ -167,7 +166,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);
> > -- 
> > 2.1.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Fri, Mar 20, 2015 at 05:43:40PM +0000, Vivi, Rodrigo wrote:
> On Fri, 2015-03-20 at 10:54 +0100, Daniel Vetter wrote:
> > On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> > > This will allow manual tests when crc isn't available.
> > > 
> > > v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.
> > 
> > buf->size isn't nonsense really, it does seem to match what we've
> > allocated. But it also seems unused in the rendercpy functions I've
> > checked. Imo you should keep that line
> 
> oh sure, bad phrase.... the value 4 was nonsense.
> But if I increases that it fails. Without setting it works.
> What do you think?

If just keeping that line makes the test work there's something _really_
fishy going on. If it persists after git clean -dfx and full recompile I'd
dig into it, since this really doesn't make sense.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Mon, Mar 23, 2015 at 09:35:27AM +0100, Daniel Vetter wrote:
> On Fri, Mar 20, 2015 at 05:43:40PM +0000, Vivi, Rodrigo wrote:
> > On Fri, 2015-03-20 at 10:54 +0100, Daniel Vetter wrote:
> > > On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> > > > This will allow manual tests when crc isn't available.
> > > > 
> > > > v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.
> > > 
> > > buf->size isn't nonsense really, it does seem to match what we've
> > > allocated. But it also seems unused in the rendercpy functions I've
> > > checked. Imo you should keep that line
> > 
> > oh sure, bad phrase.... the value 4 was nonsense.
> > But if I increases that it fails. Without setting it works.
> > What do you think?
> 
> If just keeping that line makes the test work there's something _really_
> fishy going on. If it persists after git clean -dfx and full recompile I'd
> dig into it, since this really doesn't make sense.

valgrined might be the best tool for a first attempt at figuring out
what's wrong.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Vivi, Rodrigo]

On Mon, 2015-03-23 at 09:36 +0100, Daniel Vetter wrote:
> On Mon, Mar 23, 2015 at 09:35:27AM +0100, Daniel Vetter wrote:
> > On Fri, Mar 20, 2015 at 05:43:40PM +0000, Vivi, Rodrigo wrote:
> > > On Fri, 2015-03-20 at 10:54 +0100, Daniel Vetter wrote:
> > > > On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> > > > > This will allow manual tests when crc isn't available.
> > > > > 
> > > > > v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.
> > > > 
> > > > buf->size isn't nonsense really, it does seem to match what we've
> > > > allocated. But it also seems unused in the rendercpy functions I've
> > > > checked. Imo you should keep that line
> > > 
> > > oh sure, bad phrase.... the value 4 was nonsense.
> > > But if I increases that it fails. Without setting it works.
> > > What do you think?
> > 
> > If just keeping that line makes the test work there's something _really_
> > fishy going on. If it persists after git clean -dfx and full recompile I'd
> > dig into it, since this really doesn't make sense.

But what are the values of size and stride you believe it should work in
a actual visible way?

> 
> valgrined might be the best tool for a first attempt at figuring out
> what's wrong.
> -Daniel

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

Re: [PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Daniel Vetter]

On Mon, Mar 23, 2015 at 09:53:44PM +0000, Vivi, Rodrigo wrote:
> On Mon, 2015-03-23 at 09:36 +0100, Daniel Vetter wrote:
> > On Mon, Mar 23, 2015 at 09:35:27AM +0100, Daniel Vetter wrote:
> > > On Fri, Mar 20, 2015 at 05:43:40PM +0000, Vivi, Rodrigo wrote:
> > > > On Fri, 2015-03-20 at 10:54 +0100, Daniel Vetter wrote:
> > > > > On Thu, Mar 19, 2015 at 06:24:24PM -0700, Rodrigo Vivi wrote:
> > > > > > This will allow manual tests when crc isn't available.
> > > > > > 
> > > > > > v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.
> > > > > 
> > > > > buf->size isn't nonsense really, it does seem to match what we've
> > > > > allocated. But it also seems unused in the rendercpy functions I've
> > > > > checked. Imo you should keep that line
> > > > 
> > > > oh sure, bad phrase.... the value 4 was nonsense.
> > > > But if I increases that it fails. Without setting it works.
> > > > What do you think?
> > > 
> > > If just keeping that line makes the test work there's something _really_
> > > fishy going on. If it persists after git clean -dfx and full recompile I'd
> > > dig into it, since this really doesn't make sense.
> 
> But what are the values of size and stride you believe it should work in
> a actual visible way?

Size should obviously match the size of the bo allocated, and stride
should be whatever you need, but necessarily must be smaller than the
buffer to make sense. I didn't read the test too closely and don't have an
edp psr machine at hand to try out what would really be needed here. Ben
has played around a lot with the rendercpy stuff, probably best to ask him
for a small favour and look into this a bit ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] tests/kms_psr_sink_crc: Make render visible to human eyes

[Rodrigo Vivi]

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

v2: Remove unused and non-sense buf->size and decrease buf->stride a bit as suggested by Daniel.

v3: Fix v2 mistake and get buf->size back with a value that makes more sense.
    TBD: to be changed for variable size depending on modified fb size on following patch

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

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 0a56705..94d74ec 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 = 64 * 4096;
 }
 
 static void fill_render(data_t *data, uint32_t handle, unsigned char color)
@@ -155,7 +155,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 	dst = gem_handle_to_libdrm_bo(data->bufmgr, data->drm_fd, "", handle);
 	igt_assert(dst);
 
-	src = drm_intel_bo_alloc(data->bufmgr, "", 4096, 4096);
+	src = drm_intel_bo_alloc(data->bufmgr, "", 64 * 4096, 4096);
 	igt_assert(src);
 
 	gem_write(data->drm_fd, src->handle, 0, buf, 4);
@@ -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);
-- 
2.1.0

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