diff mbox

[v2,1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors

Message ID 1394304560-19511-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com March 8, 2014, 6:49 p.m. UTC 
From: Sagar Kamble <sagar.a.kamble@intel.com>

v1: Added 128x128 and 256x256 cursor size support.

v2: Refined the test to use igt_subtest_f and automate enumeration.

Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 lib/igt_kms.c          |  13 ++-
 tests/kms_cursor_crc.c | 219 +++++++++++++++++++++++++++++++++++--------------
 2 files changed, 164 insertions(+), 68 deletions(-)

Comments

Daniel Vetter March 17, 2014, 8:05 a.m. UTC | #1 
On Sat, Mar 8, 2014 at 7:49 PM,  <sagar.a.kamble@intel.com> wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
>
> v1: Added 128x128 and 256x256 cursor size support.
>
> v2: Refined the test to use igt_subtest_f and automate enumeration.
>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Hm, do you have an update of this patch to use the drm ioctl to query
the max cursor size and correctly skip 128x128 and higher?

Also adding the additional modes into an enum looks a bit strange. I'm
it would be simpler to explicit pass the cursor size and have an outer
loop over all of them, i.e.

for (cursor_size = 64 ; cursor_size <= 256; cursor_size *= 2) {

igt_require(cusror_max >= cursor_size);

/* lists of all the existing subtests with a
igt_subtest_f("subtest-%s", cursor_size) */
}

Adding all the indirections with the enum switches makes the code imo
harder to read, and for testcases we want to aim for simple
straightforward code.
-Daniel
diff mbox

Patch

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d611c2b..a0e5b7e 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1253,7 +1253,7 @@  static int igt_output_commit(igt_output_t *output)
 {
 	igt_display_t *display = output->display;
 	igt_pipe_t *pipe;
-	int i;
+	int i, ret;
 
 	pipe = igt_output_get_driving_pipe(output);
 	if (pipe->need_set_crtc) {
@@ -1310,7 +1310,6 @@  static int igt_output_commit(igt_output_t *output)
 	if (pipe->need_set_cursor) {
 		igt_plane_t *cursor;
 		uint32_t gem_handle, crtc_id;
-		int ret;
 
 		cursor = igt_pipe_get_plane(pipe, IGT_PLANE_CURSOR);
 		crtc_id = output->config.crtc->crtc_id;
@@ -1338,8 +1337,6 @@  static int igt_output_commit(igt_output_t *output)
 					       0, 0, 0);
 		}
 
-		igt_assert(ret == 0);
-
 		pipe->need_set_cursor = false;
 		cursor->fb_changed = false;
 	}
@@ -1355,12 +1352,12 @@  static int igt_output_commit(igt_output_t *output)
 		pipe->need_wait_for_vblank = false;
 	}
 
-	return 0;
+	return ret;
 }
 
 int igt_display_commit(igt_display_t *display)
 {
-	int i;
+	int i, ret;
 
 	LOG_INDENT(display, "commit");
 
@@ -1372,7 +1369,7 @@  int igt_display_commit(igt_display_t *display)
 		if (!output->valid)
 			continue;
 
-		igt_output_commit(output);
+		ret = igt_output_commit(output);
 	}
 
 	LOG_UNINDENT(display);
@@ -1380,7 +1377,7 @@  int igt_display_commit(igt_display_t *display)
 	if (getenv("IGT_DISPLAY_WAIT_AT_COMMIT"))
 		igt_wait_for_keypress();
 
-	return 0;
+	return ret;
 }
 
 const char *igt_output_name(igt_output_t *output)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index f98fbdb..0c2f16e 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -33,10 +33,18 @@ 
 #include "igt_kms.h"
 
 enum cursor_type {
-	WHITE_VISIBLE,
-	WHITE_INVISIBLE,
-	BLACK_VISIBLE,
-	BLACK_INVISIBLE,
+	WHITE_VISIBLE_64,
+	WHITE_INVISIBLE_64,
+	BLACK_VISIBLE_64,
+	BLACK_INVISIBLE_64,
+	WHITE_VISIBLE_128,
+        WHITE_INVISIBLE_128,
+        BLACK_VISIBLE_128,
+        BLACK_INVISIBLE_128,
+	WHITE_VISIBLE_256,
+        WHITE_INVISIBLE_256,
+        BLACK_VISIBLE_256,
+        BLACK_INVISIBLE_256,
 	NUM_CURSOR_TYPES,
 };
 
@@ -99,7 +107,7 @@  static void do_test(test_data_t *test_data,
 	do_single_test(test_data, left, bottom);
 }
 
-static void cursor_enable(test_data_t *test_data, enum cursor_type cursor_type)
+static bool cursor_enable(test_data_t *test_data, enum cursor_type cursor_type)
 {
 	data_t *data = test_data->data;
 	igt_display_t *display = &data->display;
@@ -108,7 +116,12 @@  static void cursor_enable(test_data_t *test_data, enum cursor_type cursor_type)
 
 	cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
 	igt_plane_set_fb(cursor, &data->fb[cursor_type]);
-	igt_display_commit(display);
+	if (!igt_display_commit(display)) {
+		return true;
+	}
+
+	fprintf(stdout,  "Cursor size not supported\n");
+	return false;
 }
 
 static void cursor_disable(test_data_t *test_data)
@@ -123,64 +136,67 @@  static void cursor_disable(test_data_t *test_data)
 	igt_display_commit(display);
 }
 
-static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
-		     bool onscreen)
+static bool test_crc(test_data_t *test_data, enum cursor_type cursor_type,
+		     bool onscreen, int cursor_w, int cursor_h)
 {
 	int left = test_data->left;
 	int right = test_data->right;
 	int top = test_data->top;
 	int bottom = test_data->bottom;
 
-	cursor_enable(test_data, cursor_type);
+	if (!cursor_enable(test_data, cursor_type))
+		return false;
 
 	if (onscreen) {
 		/* cursor onscreen, crc should match, except when white visible cursor is used */
-		test_data->crc_must_match = cursor_type != WHITE_VISIBLE;
+		test_data->crc_must_match = (cursor_type != WHITE_VISIBLE_64 && cursor_type != WHITE_VISIBLE_128 && cursor_type != WHITE_VISIBLE_256);
 
 		/* fully inside  */
 		do_test(test_data, left, right, top, bottom);
 
 		/* 2 pixels inside */
-		do_test(test_data, left - 62, right + 62, top     , bottom     );
-		do_test(test_data, left     , right     , top - 62, bottom + 62);
-		do_test(test_data, left - 62, right + 62, top - 62, bottom + 62);
+		do_test(test_data, left - (cursor_w-2), right + (cursor_w-2), top               , bottom               );
+		do_test(test_data, left               , right               , top - (cursor_h-2), bottom + (cursor_h-2));
+		do_test(test_data, left - (cursor_w-2), right + (cursor_w-2), top - (cursor_h-2), bottom + (cursor_h-2));
 
 		/* 1 pixel inside */
-		do_test(test_data, left - 63, right + 63, top     , bottom     );
-		do_test(test_data, left     , right     , top - 63, bottom + 63);
-		do_test(test_data, left - 63, right + 63, top - 63, bottom + 63);
+		do_test(test_data, left - (cursor_w-1), right + (cursor_w-1), top               , bottom               );
+                do_test(test_data, left               , right               , top - (cursor_h-1), bottom + (cursor_h-1));
+                do_test(test_data, left - (cursor_w-1), right + (cursor_w-1), top - (cursor_h-1), bottom + (cursor_h-1));
 	} else {
 		/* cursor offscreen, crc should always match */
 		test_data->crc_must_match = true;
 
 		/* fully outside */
-		do_test(test_data, left - 64, right + 64, top     , bottom     );
-		do_test(test_data, left     , right     , top - 64, bottom + 64);
-		do_test(test_data, left - 64, right + 64, top - 64, bottom + 64);
+		do_test(test_data, left - (cursor_w), right + (cursor_w), top             , bottom             );
+                do_test(test_data, left             , right             , top - (cursor_h), bottom + (cursor_h));
+                do_test(test_data, left - (cursor_w), right + (cursor_w), top - (cursor_h), bottom + (cursor_h));
 
 		/* fully outside by 1 extra pixels */
-		do_test(test_data, left - 65, right + 65, top     , bottom     );
-		do_test(test_data, left     , right     , top - 65, bottom + 65);
-		do_test(test_data, left - 65, right + 65, top - 65, bottom + 65);
+		do_test(test_data, left - (cursor_w+1), right + (cursor_w+1), top               , bottom               );
+                do_test(test_data, left               , right               , top - (cursor_h+1), bottom + (cursor_h+1));
+                do_test(test_data, left - (cursor_w+1), right + (cursor_w+1), top - (cursor_h+1), bottom + (cursor_h+1));
 
 		/* fully outside by 2 extra pixels */
-		do_test(test_data, left - 66, right + 66, top     , bottom     );
-		do_test(test_data, left     , right     , top - 66, bottom + 66);
-		do_test(test_data, left - 66, right + 66, top - 66, bottom + 66);
+		do_test(test_data, left - (cursor_w+2), right + (cursor_w+2), top               , bottom               );
+                do_test(test_data, left               , right               , top - (cursor_h+2), bottom + (cursor_h+2));
+                do_test(test_data, left - (cursor_w+2), right + (cursor_w+2), top - (cursor_h+2), bottom + (cursor_h+2));
 
 		/* fully outside by a lot of extra pixels */
-		do_test(test_data, left - 512, right + 512, top      , bottom      );
-		do_test(test_data, left      , right      , top - 512, bottom + 512);
-		do_test(test_data, left - 512, right + 512, top - 512, bottom + 512);
+		do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top                 , bottom                 );
+		do_test(test_data, left                 , right                 , top - (cursor_h+512), bottom + (cursor_h+512));
+		do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512));
 
 		/* go nuts */
 		do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
 	}
 
 	cursor_disable(test_data);
+	return true;
 }
 
-static bool prepare_crtc(test_data_t *test_data, igt_output_t *output)
+static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
+			enum cursor_type cursor_type, int cursor_w, int cursor_h)
 {
 	drmModeModeInfo *mode;
 	data_t *data = test_data->data;
@@ -210,7 +226,7 @@  static bool prepare_crtc(test_data_t *test_data, igt_output_t *output)
 
 	pipe_crc = create_crc(data, test_data->pipe);
 	if (!pipe_crc) {
-		printf("auto crc not supported on this connector with pipe %i\n",
+		fprintf(stdout, "auto crc not supported on this connector with pipe %i\n",
 		       test_data->pipe);
 		return false;
 	}
@@ -219,9 +235,9 @@  static bool prepare_crtc(test_data_t *test_data, igt_output_t *output)
 
 	/* x/y position where the cursor is still fully visible */
 	test_data->left = 0;
-	test_data->right = mode->hdisplay - 64;
+	test_data->right = mode->hdisplay - cursor_w;
 	test_data->top = 0;
-	test_data->bottom = mode->vdisplay - 64;
+	test_data->bottom = mode->vdisplay - cursor_h;
 
 	/* make sure cursor is disabled */
 	cursor_disable(test_data);
@@ -258,14 +274,43 @@  static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
 		.data = data,
 	};
 	int valid_tests = 0;
-
+	int cursor_w, cursor_h;
+
+	switch(cursor_type)
+        {
+                case WHITE_VISIBLE_64:
+                case WHITE_INVISIBLE_64:
+                case BLACK_VISIBLE_64:
+                case BLACK_INVISIBLE_64:
+                        cursor_w = 64;
+                        cursor_h = 64;
+                        break;
+		case WHITE_VISIBLE_128:
+                case WHITE_INVISIBLE_128:
+                case BLACK_VISIBLE_128:
+                case BLACK_INVISIBLE_128:
+                        cursor_w = 128;
+                        cursor_h = 128;
+                        break;
+		case WHITE_VISIBLE_256:
+                case WHITE_INVISIBLE_256:
+                case BLACK_VISIBLE_256:
+                case BLACK_INVISIBLE_256:
+                        cursor_w = 256;
+                        cursor_h = 256;
+                        break;
+		default:
+			cursor_w = 64;
+                        cursor_h = 64;
+                        break;
+	}
 
 	for_each_connected_output(display, output) {
 		test_data.output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
 			test_data.pipe = p;
 
-			if (!prepare_crtc(&test_data, output))
+			if (!prepare_crtc(&test_data, output, cursor_type, cursor_w, cursor_h))
 				continue;
 
 			valid_tests++;
@@ -274,13 +319,17 @@  static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
 				igt_subtest_name(), pipe_name(test_data.pipe),
 				igt_output_name(output));
 
-			test_crc(&test_data, cursor_type, onscreen);
+			if (!test_crc(&test_data, cursor_type, onscreen, cursor_w, cursor_h)) {
+				fprintf(stdout, "Test not started. Cursor size is not supported\n");
+				goto exit;
+			}
 
 
 			fprintf(stdout, "\n%s on pipe %c, connector %s: PASSED\n\n",
 				igt_subtest_name(), pipe_name(test_data.pipe),
 				igt_output_name(output));
 
+exit:
 			/* cleanup what prepare_crtc() has done */
 			cleanup_crtc(&test_data, output);
 		}
@@ -291,25 +340,87 @@  static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
 
 static void create_cursor_fb(data_t *data,
 			     enum cursor_type cursor_type,
-			     double r, double g, double b, double a)
+			     double r, double g, double b, double a,
+			     int cur_w, int cur_h)
 {
 	cairo_t *cr;
 	uint32_t fb_id[NUM_CURSOR_TYPES];
 
-	fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, 64, 64,
+	fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, cur_w, cur_h,
 						DRM_FORMAT_ARGB8888, false,
 						&data->fb[cursor_type]);
 	igt_assert(fb_id[cursor_type]);
 
 	cr = kmstest_get_cairo_ctx(data->drm_fd,
 				   &data->fb[cursor_type]);
-	kmstest_paint_color_alpha(cr, 0, 0, 64, 64, r, g, b, a);
+	kmstest_paint_color_alpha(cr, 0, 0, cur_w, cur_h, r, g, b, a);
 	igt_assert(cairo_status(cr) == 0);
 }
 
+static void run_test_generic(struct data_t *data)
+{
+	int cur;
+	char *test_name, *cur_color;
+	for (cur = 0; cur < NUM_CURSOR_TYPES; cur++)
+	{
+		switch(cur)
+		{
+			case WHITE_VISIBLE_64:
+			case WHITE_INVISIBLE_64:
+			case BLACK_VISIBLE_64:
+			case BLACK_INVISIBLE_64:
+				test_name = "64x64";
+				break;
+			case WHITE_VISIBLE_128:
+			case WHITE_INVISIBLE_128:
+			case BLACK_VISIBLE_128:
+			case BLACK_INVISIBLE_128:
+				test_name = "128x128";
+				break;
+			case WHITE_VISIBLE_256:
+			case WHITE_INVISIBLE_256:
+			case BLACK_VISIBLE_256:
+			case BLACK_INVISIBLE_256:
+				test_name = "256x256";
+				break;
+		}
+
+		switch(cur)
+		{
+			case WHITE_VISIBLE_64:
+			case WHITE_VISIBLE_128:
+			case WHITE_VISIBLE_256:
+				cur_color = "white-visible";
+				break;
+			case WHITE_INVISIBLE_64:
+			case WHITE_INVISIBLE_128:
+			case WHITE_INVISIBLE_256:
+				cur_color = "white-invisible";
+				break;
+			case BLACK_VISIBLE_64:
+			case BLACK_VISIBLE_128:
+			case BLACK_VISIBLE_256:
+				cur_color = "black-visible";
+				break;
+			case BLACK_INVISIBLE_64:
+			case BLACK_INVISIBLE_128:
+			case BLACK_INVISIBLE_256:
+				cur_color = "black-invisible";
+				break;
+		}
+
+		igt_subtest_f("cursor-%s-onscreen-%s", cur_color, test_name)
+			run_test(data, WHITE_VISIBLE_64 + cur, true);
+		igt_subtest_f("cursor-%s-offscreen-%s", cur_color, test_name)
+			run_test(data, WHITE_VISIBLE_64 + cur, false);
+	}
+
+}
+
 igt_main
 {
 	data_t data = {};
+	int i = 1, cur = 0;
 
 	igt_skip_on_simulation();
 
@@ -323,30 +434,18 @@  igt_main
 
 		igt_display_init(&data.display, data.drm_fd);
 		data.pipe_crc = calloc(igt_display_get_n_pipes(&data.display),
-				       sizeof(data.pipe_crc[0]));
-
-		create_cursor_fb(&data, WHITE_VISIBLE, 1.0, 1.0, 1.0, 1.0);
-		create_cursor_fb(&data, WHITE_INVISIBLE, 1.0, 1.0, 1.0, 0.0);
-		create_cursor_fb(&data, BLACK_VISIBLE, 0.0, 0.0, 0.0, 1.0);
-		create_cursor_fb(&data, BLACK_INVISIBLE, 0.0, 0.0, 0.0, 0.0);
+					sizeof(data.pipe_crc[0]));
+
+		for(; i < 5; i += i) {
+			create_cursor_fb(&data, WHITE_VISIBLE_64 + cur, 1.0, 1.0, 1.0, 1.0, (64 * i), (64 * i));
+			create_cursor_fb(&data, WHITE_INVISIBLE_64 + cur, 1.0, 1.0, 1.0, 0.0, (64 * i), (64 * i));
+			create_cursor_fb(&data, BLACK_VISIBLE_64 + cur, 0.0, 0.0, 0.0, 1.0, (64 * i), (64 * i));
+			create_cursor_fb(&data, BLACK_INVISIBLE_64 + cur, 0.0, 0.0, 0.0, 0.0, (64 * i), (64 * i));
+			cur += 4;
+		}
 	}
 
-	igt_subtest("cursor-white-visible-onscreen")
-		run_test(&data, WHITE_VISIBLE, true);
-	igt_subtest("cursor-white-visible-offscreen")
-		run_test(&data, WHITE_VISIBLE, false);
-	igt_subtest("cursor-white-invisible-onscreen")
-		run_test(&data, WHITE_INVISIBLE, true);
-	igt_subtest("cursor-white-invisible-offscreen")
-		run_test(&data, WHITE_INVISIBLE, false);
-	igt_subtest("cursor-black-visible-onscreen")
-		run_test(&data, BLACK_VISIBLE, true);
-	igt_subtest("cursor-black-visible-offscreen")
-		run_test(&data, BLACK_VISIBLE, false);
-	igt_subtest("cursor-black-invisible-onscreen")
-		run_test(&data, BLACK_INVISIBLE, true);
-	igt_subtest("cursor-black-invisible-offscreen")
-		run_test(&data, BLACK_INVISIBLE, false);
+	run_test_generic(&data);
 
 	igt_fixture {
 		free(data.pipe_crc);