All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] kms_cursor_crc/Enabling this test for variable cursor width and heights
@ 2014-02-24 15:37 sagar.a.kamble
  2014-02-24 15:48 ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: sagar.a.kamble @ 2014-02-24 15:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sagar Kamble

From: Sagar Kamble <sagar.a.kamble@intel.com>

Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 tests/kms_cursor_crc.c | 51 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 38aa1ab..4458248 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -34,6 +34,8 @@
 #include "igt_debugfs.h"
 #include "igt_kms.h"
 
+int	w = 0, h = 0;
+
 enum cursor_type {
 	WHITE_VISIBLE,
 	WHITE_INVISIBLE,
@@ -170,7 +172,7 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
 
 	/* enable cursor */
 	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id,
-				    data->fb[cursor_type].gem_handle, 64, 64) == 0);
+				    data->fb[cursor_type].gem_handle, w, h) == 0);
 
 	if (onscreen) {
 		/* cursor onscreen, crc should match, except when white visible cursor is used */
@@ -180,32 +182,32 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
 		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 - (w - 2), right + (w - 2), top     , bottom     );
+		do_test(test_data, left     , right     , top - (w - 2), bottom + (w - 2));
+		do_test(test_data, left - (w - 2), right + (w - 2), top - (w - 2), bottom + (w - 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 - (w - 1), right + (w - 1), top     , bottom     );
+		do_test(test_data, left     , right     , top - (w - 1), bottom + (w - 1));
+		do_test(test_data, left - (w - 1), right + (w - 1), top - (w - 1), bottom + (w - 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 - w, right + w, top     , bottom     );
+		do_test(test_data, left     , right     , top - w, bottom + w);
+		do_test(test_data, left - w, right + w, top - w, bottom + w);
 
 		/* 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 - (w + 1), right + (w + 1), top     , bottom     );
+		do_test(test_data, left     , right     , top - (w + 1), bottom + (w + 1));
+		do_test(test_data, left - (w + 1), right + (w + 1), top - (w + 1), bottom + (w + 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 - (w + 2), right + (w + 2), top     , bottom     );
+		do_test(test_data, left     , right     , top - (w + 2), bottom + (w + 2));
+		do_test(test_data, left - (w + 2), right + (w + 2), top - (w + 2), bottom + (w + 2));
 
 		/* fully outside by a lot of extra pixels */
 		do_test(test_data, left - 512, right + 512, top      , bottom      );
@@ -251,9 +253,9 @@ static bool prepare_crtc(test_data_t *test_data, uint32_t connector_id)
 
 	/* x/y position where the cursor is still fully visible */
 	test_data->left = 0;
-	test_data->right = connector.config.default_mode.hdisplay - 64;
+	test_data->right = connector.config.default_mode.hdisplay - w;
 	test_data->top = 0;
-	test_data->bottom = connector.config.default_mode.vdisplay - 64;
+	test_data->bottom = connector.config.default_mode.vdisplay - h;
 
 	/* make sure cursor is disabled */
 	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id, 0, 0, 0) == 0);
@@ -314,20 +316,29 @@ static void create_cursor_fb(data_t *data,
 {
 	cairo_t *cr;
 
-	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, 64, 64,
+	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, w, h,
 						      DRM_FORMAT_ARGB8888, false,
 						      &data->fb[cursor_type]);
 	igt_assert(data->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, w, h, r, g, b, a);
 	igt_assert(cairo_status(cr) == 0);
 }
 
 igt_main
 {
 	data_t data = {};
+	int	c;
+	char	args[10];
+
+	printf("\nEnter Width and Height of Cursor Plane (64x64, 128x128):");
+	scanf("%s", args);
+	if (sscanf(args, "%dx%d", &w, &h) != 2) {
+		printf("\nInvalid arguments");
+		return -1;
+	}
 
 	igt_skip_on_simulation();
 
-- 
1.8.5

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

* Re: [PATCH 1/1] kms_cursor_crc/Enabling this test for variable cursor width and heights
  2014-02-24 15:37 [PATCH 1/1] kms_cursor_crc/Enabling this test for variable cursor width and heights sagar.a.kamble
@ 2014-02-24 15:48 ` Ville Syrjälä
  2014-03-05 14:05   ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2014-02-24 15:48 UTC (permalink / raw)
  To: sagar.a.kamble; +Cc: intel-gfx

On Mon, Feb 24, 2014 at 09:07:05PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  tests/kms_cursor_crc.c | 51 ++++++++++++++++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 38aa1ab..4458248 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -34,6 +34,8 @@
>  #include "igt_debugfs.h"
>  #include "igt_kms.h"
>  
> +int	w = 0, h = 0;
> +
>  enum cursor_type {
>  	WHITE_VISIBLE,
>  	WHITE_INVISIBLE,
> @@ -170,7 +172,7 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
>  
>  	/* enable cursor */
>  	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id,
> -				    data->fb[cursor_type].gem_handle, 64, 64) == 0);
> +				    data->fb[cursor_type].gem_handle, w, h) == 0);
>  
>  	if (onscreen) {
>  		/* cursor onscreen, crc should match, except when white visible cursor is used */
> @@ -180,32 +182,32 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
>  		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 - (w - 2), right + (w - 2), top     , bottom     );
> +		do_test(test_data, left     , right     , top - (w - 2), bottom + (w - 2));
> +		do_test(test_data, left - (w - 2), right + (w - 2), top - (w - 2), bottom + (w - 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 - (w - 1), right + (w - 1), top     , bottom     );
> +		do_test(test_data, left     , right     , top - (w - 1), bottom + (w - 1));
> +		do_test(test_data, left - (w - 1), right + (w - 1), top - (w - 1), bottom + (w - 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 - w, right + w, top     , bottom     );
> +		do_test(test_data, left     , right     , top - w, bottom + w);
> +		do_test(test_data, left - w, right + w, top - w, bottom + w);
>  
>  		/* 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 - (w + 1), right + (w + 1), top     , bottom     );
> +		do_test(test_data, left     , right     , top - (w + 1), bottom + (w + 1));
> +		do_test(test_data, left - (w + 1), right + (w + 1), top - (w + 1), bottom + (w + 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 - (w + 2), right + (w + 2), top     , bottom     );
> +		do_test(test_data, left     , right     , top - (w + 2), bottom + (w + 2));
> +		do_test(test_data, left - (w + 2), right + (w + 2), top - (w + 2), bottom + (w + 2));
>  
>  		/* fully outside by a lot of extra pixels */
>  		do_test(test_data, left - 512, right + 512, top      , bottom      );
> @@ -251,9 +253,9 @@ static bool prepare_crtc(test_data_t *test_data, uint32_t connector_id)
>  
>  	/* x/y position where the cursor is still fully visible */
>  	test_data->left = 0;
> -	test_data->right = connector.config.default_mode.hdisplay - 64;
> +	test_data->right = connector.config.default_mode.hdisplay - w;
>  	test_data->top = 0;
> -	test_data->bottom = connector.config.default_mode.vdisplay - 64;
> +	test_data->bottom = connector.config.default_mode.vdisplay - h;
>  
>  	/* make sure cursor is disabled */
>  	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id, 0, 0, 0) == 0);
> @@ -314,20 +316,29 @@ static void create_cursor_fb(data_t *data,
>  {
>  	cairo_t *cr;
>  
> -	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, 64, 64,
> +	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, w, h,
>  						      DRM_FORMAT_ARGB8888, false,
>  						      &data->fb[cursor_type]);
>  	igt_assert(data->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, w, h, r, g, b, a);
>  	igt_assert(cairo_status(cr) == 0);
>  }
>  
>  igt_main
>  {
>  	data_t data = {};
> +	int	c;
> +	char	args[10];
> +
> +	printf("\nEnter Width and Height of Cursor Plane (64x64, 128x128):");
> +	scanf("%s", args);
> +	if (sscanf(args, "%dx%d", &w, &h) != 2) {
> +		printf("\nInvalid arguments");
> +		return -1;
> +	}

These tests are meant to run without user interaction, so reading from
stdin isn't something we want to be doing.

What you should do is add more subtests for the different cursors sizes.
And you should make the 128x128 and 256x256 subtests skip if the kernel
refuses the specific cursor size. By using igt_subtest_f() you can avoid
a bunch of duplicated code for iterating the subtests.

>  
>  	igt_skip_on_simulation();
>  
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH 1/1] kms_cursor_crc/Enabling this test for variable cursor width and heights
  2014-02-24 15:48 ` Ville Syrjälä
@ 2014-03-05 14:05   ` Daniel Vetter
  2014-03-08 18:49     ` [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors sagar.a.kamble
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2014-03-05 14:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, sagar.a.kamble

On Mon, Feb 24, 2014 at 05:48:16PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 24, 2014 at 09:07:05PM +0530, sagar.a.kamble@intel.com wrote:
> > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > 
> > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > ---
> >  tests/kms_cursor_crc.c | 51 ++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 31 insertions(+), 20 deletions(-)
> > 
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 38aa1ab..4458248 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -34,6 +34,8 @@
> >  #include "igt_debugfs.h"
> >  #include "igt_kms.h"
> >  
> > +int	w = 0, h = 0;
> > +
> >  enum cursor_type {
> >  	WHITE_VISIBLE,
> >  	WHITE_INVISIBLE,
> > @@ -170,7 +172,7 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> >  
> >  	/* enable cursor */
> >  	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id,
> > -				    data->fb[cursor_type].gem_handle, 64, 64) == 0);
> > +				    data->fb[cursor_type].gem_handle, w, h) == 0);
> >  
> >  	if (onscreen) {
> >  		/* cursor onscreen, crc should match, except when white visible cursor is used */
> > @@ -180,32 +182,32 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> >  		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 - (w - 2), right + (w - 2), top     , bottom     );
> > +		do_test(test_data, left     , right     , top - (w - 2), bottom + (w - 2));
> > +		do_test(test_data, left - (w - 2), right + (w - 2), top - (w - 2), bottom + (w - 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 - (w - 1), right + (w - 1), top     , bottom     );
> > +		do_test(test_data, left     , right     , top - (w - 1), bottom + (w - 1));
> > +		do_test(test_data, left - (w - 1), right + (w - 1), top - (w - 1), bottom + (w - 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 - w, right + w, top     , bottom     );
> > +		do_test(test_data, left     , right     , top - w, bottom + w);
> > +		do_test(test_data, left - w, right + w, top - w, bottom + w);
> >  
> >  		/* 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 - (w + 1), right + (w + 1), top     , bottom     );
> > +		do_test(test_data, left     , right     , top - (w + 1), bottom + (w + 1));
> > +		do_test(test_data, left - (w + 1), right + (w + 1), top - (w + 1), bottom + (w + 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 - (w + 2), right + (w + 2), top     , bottom     );
> > +		do_test(test_data, left     , right     , top - (w + 2), bottom + (w + 2));
> > +		do_test(test_data, left - (w + 2), right + (w + 2), top - (w + 2), bottom + (w + 2));
> >  
> >  		/* fully outside by a lot of extra pixels */
> >  		do_test(test_data, left - 512, right + 512, top      , bottom      );
> > @@ -251,9 +253,9 @@ static bool prepare_crtc(test_data_t *test_data, uint32_t connector_id)
> >  
> >  	/* x/y position where the cursor is still fully visible */
> >  	test_data->left = 0;
> > -	test_data->right = connector.config.default_mode.hdisplay - 64;
> > +	test_data->right = connector.config.default_mode.hdisplay - w;
> >  	test_data->top = 0;
> > -	test_data->bottom = connector.config.default_mode.vdisplay - 64;
> > +	test_data->bottom = connector.config.default_mode.vdisplay - h;
> >  
> >  	/* make sure cursor is disabled */
> >  	igt_assert(drmModeSetCursor(data->drm_fd, test_data->crtc_id, 0, 0, 0) == 0);
> > @@ -314,20 +316,29 @@ static void create_cursor_fb(data_t *data,
> >  {
> >  	cairo_t *cr;
> >  
> > -	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, 64, 64,
> > +	data->fb_id[cursor_type] = kmstest_create_fb2(data->drm_fd, w, h,
> >  						      DRM_FORMAT_ARGB8888, false,
> >  						      &data->fb[cursor_type]);
> >  	igt_assert(data->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, w, h, r, g, b, a);
> >  	igt_assert(cairo_status(cr) == 0);
> >  }
> >  
> >  igt_main
> >  {
> >  	data_t data = {};
> > +	int	c;
> > +	char	args[10];
> > +
> > +	printf("\nEnter Width and Height of Cursor Plane (64x64, 128x128):");
> > +	scanf("%s", args);
> > +	if (sscanf(args, "%dx%d", &w, &h) != 2) {
> > +		printf("\nInvalid arguments");
> > +		return -1;
> > +	}
> 
> These tests are meant to run without user interaction, so reading from
> stdin isn't something we want to be doing.
> 
> What you should do is add more subtests for the different cursors sizes.
> And you should make the 128x128 and 256x256 subtests skip if the kernel
> refuses the specific cursor size. By using igt_subtest_f() you can avoid
> a bunch of duplicated code for iterating the subtests.

Agree with Ville here, we want subtests for all the cursor sizes we
support and to correctly skip them if the kernel doesn't accept a cursor
of a given size. Once that's all ready your cursor sizes patch is imo good
to go.

Please don't forget to add a Testcase: tag to your kernel patch
referencing your new (sub)tests here.

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

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

* [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors
  2014-03-05 14:05   ` Daniel Vetter
@ 2014-03-08 18:49     ` sagar.a.kamble
  2014-03-17  8:05       ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: sagar.a.kamble @ 2014-03-08 18:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sagar Kamble

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(-)

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);
-- 
1.8.5

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

* Re: [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors
  2014-03-08 18:49     ` [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors sagar.a.kamble
@ 2014-03-17  8:05       ` Daniel Vetter
  2014-03-18 10:29         ` [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes sagar.a.kamble
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2014-03-17  8:05 UTC (permalink / raw)
  To: Sagar Arun Kamble; +Cc: intel-gfx

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes
  2014-03-17  8:05       ` Daniel Vetter
@ 2014-03-18 10:29         ` sagar.a.kamble
  2014-03-20 16:18           ` Imre Deak
  0 siblings, 1 reply; 9+ messages in thread
From: sagar.a.kamble @ 2014-03-18 10:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sagar Kamble

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.

v3: Restructuring test enumeration using drmGetCap. [Daniel's review comments]

Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 80 insertions(+), 51 deletions(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index f98fbdb..9ddf9b4 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -32,6 +32,13 @@
 #include "igt_debugfs.h"
 #include "igt_kms.h"
 
+#ifndef DRM_CAP_CURSOR_WIDTH
+#define DRM_CAP_CURSOR_WIDTH 0x8
+#endif
+#ifndef DRM_CAP_CURSOR_HEIGHT
+#define DRM_CAP_CURSOR_HEIGHT 0x9
+#endif
+
 enum cursor_type {
 	WHITE_VISIBLE,
 	WHITE_INVISIBLE,
@@ -124,7 +131,7 @@ static void cursor_disable(test_data_t *test_data)
 }
 
 static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
-		     bool onscreen)
+		     bool onscreen, int cursor_w, int cursor_h)
 {
 	int left = test_data->left;
 	int right = test_data->right;
@@ -135,43 +142,43 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
 
 	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);
 
 		/* 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);
@@ -180,7 +187,8 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
 	cursor_disable(test_data);
 }
 
-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,
+			 int cursor_w, int cursor_h)
 {
 	drmModeModeInfo *mode;
 	data_t *data = test_data->data;
@@ -219,9 +227,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);
@@ -249,7 +257,8 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
 	igt_output_set_pipe(output, PIPE_ANY);
 }
 
-static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
+static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen,
+		     int cursor_w, int cursor_h)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
@@ -259,13 +268,12 @@ static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
 	};
 	int valid_tests = 0;
 
-
 	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_w, cursor_h))
 				continue;
 
 			valid_tests++;
@@ -274,8 +282,7 @@ 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);
-
+			test_crc(&test_data, cursor_type, onscreen, cursor_w, cursor_h);
 
 			fprintf(stdout, "\n%s on pipe %c, connector %s: PASSED\n\n",
 				igt_subtest_name(), pipe_name(test_data.pipe),
@@ -291,31 +298,72 @@ 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(data_t *data, int cursor_max_size)
+{
+	int cursor_size;
+	char c_size[5];
+	for (cursor_size = 64; cursor_size <= cursor_max_size; cursor_size *= 2)
+	{
+		igt_require(cursor_max_size >= cursor_size);
+		sprintf(c_size, "%d", cursor_size);
+
+		/* Creating cursor framebuffers */
+		create_cursor_fb(data, WHITE_VISIBLE, 1.0, 1.0, 1.0, 1.0, cursor_size, cursor_size);
+		create_cursor_fb(data, WHITE_INVISIBLE, 1.0, 1.0, 1.0, 0.0, cursor_size, cursor_size);
+		create_cursor_fb(data, BLACK_VISIBLE, 0.0, 0.0, 0.0, 1.0, cursor_size, cursor_size);
+		create_cursor_fb(data, BLACK_INVISIBLE, 0.0, 0.0, 0.0, 0.0, cursor_size, cursor_size);
+
+		/* Using created cursor FBs to test cursor support */
+		igt_subtest_f("white-visible-cursor-%s-onscreen", c_size)
+			run_test(data, WHITE_VISIBLE, true, cursor_size, cursor_size);
+		igt_subtest_f("white-invisible-cursor-%s-offscreen", c_size)
+			run_test(data, WHITE_INVISIBLE, false, cursor_size, cursor_size);
+                igt_subtest_f("black-visible-cursor-%s-onscreen", c_size)
+                        run_test(data, BLACK_VISIBLE, true, cursor_size, cursor_size);
+                igt_subtest_f("black-invisible-cursor-%s-offscreen", c_size)
+                        run_test(data, BLACK_INVISIBLE, false, cursor_size, cursor_size);
+	}
+
+}
+
 igt_main
 {
 	data_t data = {};
+	int cursor_max_size, ret;
+	uint64_t cursor_width, cursor_height;
 
 	igt_skip_on_simulation();
 
 	igt_fixture {
 		data.drm_fd = drm_open_any();
 
+		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width);
+		igt_assert(ret == 0);
+		/* Not making use of cursor_height since it is same as width, still reading */
+		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height);
+		igt_assert(ret == 0);
+
+		fprintf(stdout, "%d, %d\n", cursor_width, cursor_height);
+		/* We assume width and height are same so max is assigned width */
+		cursor_max_size = (int)cursor_width;
+
 		igt_set_vt_graphics_mode();
 
 		igt_debugfs_init(&data.debugfs);
@@ -325,28 +373,9 @@ igt_main
 		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);
 	}
 
-	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, cursor_max_size);
 
 	igt_fixture {
 		free(data.pipe_crc);
-- 
1.8.5

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

* Re: [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes
  2014-03-18 10:29         ` [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes sagar.a.kamble
@ 2014-03-20 16:18           ` Imre Deak
  2014-03-20 16:33             ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Imre Deak @ 2014-03-20 16:18 UTC (permalink / raw)
  To: sagar.a.kamble; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 12560 bytes --]

On Tue, 2014-03-18 at 15:59 +0530, 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.
> 
> v3: Restructuring test enumeration using drmGetCap. [Daniel's review comments]
> 
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 80 insertions(+), 51 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index f98fbdb..9ddf9b4 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -32,6 +32,13 @@
>  #include "igt_debugfs.h"
>  #include "igt_kms.h"
>  
> +#ifndef DRM_CAP_CURSOR_WIDTH
> +#define DRM_CAP_CURSOR_WIDTH 0x8
> +#endif
> +#ifndef DRM_CAP_CURSOR_HEIGHT
> +#define DRM_CAP_CURSOR_HEIGHT 0x9
> +#endif
> +
>  enum cursor_type {
>  	WHITE_VISIBLE,
>  	WHITE_INVISIBLE,
> @@ -124,7 +131,7 @@ static void cursor_disable(test_data_t *test_data)
>  }
>  
>  static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> -		     bool onscreen)
> +		     bool onscreen, int cursor_w, int cursor_h)
>  {
>  	int left = test_data->left;
>  	int right = test_data->right;
> @@ -135,43 +142,43 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
>  
>  	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);
>  
>  		/* 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));

Nitpick: indent error, also the same on some of the following lines.

>  	} 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);
> @@ -180,7 +187,8 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
>  	cursor_disable(test_data);
>  }
>  
> -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,
> +			 int cursor_w, int cursor_h)
>  {
>  	drmModeModeInfo *mode;
>  	data_t *data = test_data->data;
> @@ -219,9 +227,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);
> @@ -249,7 +257,8 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
>  	igt_output_set_pipe(output, PIPE_ANY);
>  }
>  
> -static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
> +static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen,
> +		     int cursor_w, int cursor_h)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
> @@ -259,13 +268,12 @@ static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
>  	};
>  	int valid_tests = 0;
>  
> -
>  	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_w, cursor_h))
>  				continue;
>  
>  			valid_tests++;
> @@ -274,8 +282,7 @@ 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);
> -
> +			test_crc(&test_data, cursor_type, onscreen, cursor_w, cursor_h);
>  
>  			fprintf(stdout, "\n%s on pipe %c, connector %s: PASSED\n\n",
>  				igt_subtest_name(), pipe_name(test_data.pipe),
> @@ -291,31 +298,72 @@ 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(data_t *data, int cursor_max_size)
> +{
> +	int cursor_size;
> +	char c_size[5];
> +	for (cursor_size = 64; cursor_size <= cursor_max_size; cursor_size *= 2)
> +	{
> +		igt_require(cursor_max_size >= cursor_size);
> +		sprintf(c_size, "%d", cursor_size);
> +
> +		/* Creating cursor framebuffers */
> +		create_cursor_fb(data, WHITE_VISIBLE, 1.0, 1.0, 1.0, 1.0, cursor_size, cursor_size);
> +		create_cursor_fb(data, WHITE_INVISIBLE, 1.0, 1.0, 1.0, 0.0, cursor_size, cursor_size);
> +		create_cursor_fb(data, BLACK_VISIBLE, 0.0, 0.0, 0.0, 1.0, cursor_size, cursor_size);
> +		create_cursor_fb(data, BLACK_INVISIBLE, 0.0, 0.0, 0.0, 0.0, cursor_size, cursor_size);
> +
> +		/* Using created cursor FBs to test cursor support */
> +		igt_subtest_f("white-visible-cursor-%s-onscreen", c_size)
> +			run_test(data, WHITE_VISIBLE, true, cursor_size, cursor_size);
> +		igt_subtest_f("white-invisible-cursor-%s-offscreen", c_size)
> +			run_test(data, WHITE_INVISIBLE, false, cursor_size, cursor_size);
> +                igt_subtest_f("black-visible-cursor-%s-onscreen", c_size)
> +                        run_test(data, BLACK_VISIBLE, true, cursor_size, cursor_size);
> +                igt_subtest_f("black-invisible-cursor-%s-offscreen", c_size)
> +                        run_test(data, BLACK_INVISIBLE, false, cursor_size, cursor_size);
> +	}
> +
> +}
> +
>  igt_main
>  {
>  	data_t data = {};
> +	int cursor_max_size, ret;
> +	uint64_t cursor_width, cursor_height;
>  
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
>  		data.drm_fd = drm_open_any();
>  
> +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width);
> +		igt_assert(ret == 0);
> +		/* Not making use of cursor_height since it is same as width, still reading */
> +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height);
> +		igt_assert(ret == 0);
> +
> +		fprintf(stdout, "%d, %d\n", cursor_width, cursor_height);
> +		/* We assume width and height are same so max is assigned width */

I would add here an explicit assert about the above. Other than this and
the formatting issue above it looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		cursor_max_size = (int)cursor_width;
> +
>  		igt_set_vt_graphics_mode();
>  
>  		igt_debugfs_init(&data.debugfs);
> @@ -325,28 +373,9 @@ igt_main
>  		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);
>  	}
>  
> -	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, cursor_max_size);
>  
>  	igt_fixture {
>  		free(data.pipe_crc);


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* Re: [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes
  2014-03-20 16:18           ` Imre Deak
@ 2014-03-20 16:33             ` Daniel Vetter
  2014-03-20 17:04               ` Sagar Arun Kamble
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2014-03-20 16:33 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, sagar.a.kamble

On Thu, Mar 20, 2014 at 06:18:05PM +0200, Imre Deak wrote:
> On Tue, 2014-03-18 at 15:59 +0530, 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.
> > 
> > v3: Restructuring test enumeration using drmGetCap. [Daniel's review comments]
> > 
> > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > ---
> >  tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 80 insertions(+), 51 deletions(-)
> > 
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index f98fbdb..9ddf9b4 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -32,6 +32,13 @@
> >  #include "igt_debugfs.h"
> >  #include "igt_kms.h"
> >  
> > +#ifndef DRM_CAP_CURSOR_WIDTH
> > +#define DRM_CAP_CURSOR_WIDTH 0x8
> > +#endif
> > +#ifndef DRM_CAP_CURSOR_HEIGHT
> > +#define DRM_CAP_CURSOR_HEIGHT 0x9
> > +#endif
> > +
> >  enum cursor_type {
> >  	WHITE_VISIBLE,
> >  	WHITE_INVISIBLE,
> > @@ -124,7 +131,7 @@ static void cursor_disable(test_data_t *test_data)
> >  }
> >  
> >  static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> > -		     bool onscreen)
> > +		     bool onscreen, int cursor_w, int cursor_h)
> >  {
> >  	int left = test_data->left;
> >  	int right = test_data->right;
> > @@ -135,43 +142,43 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> >  
> >  	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);
> >  
> >  		/* 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));
> 
> Nitpick: indent error, also the same on some of the following lines.
> 
> >  	} 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);
> > @@ -180,7 +187,8 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> >  	cursor_disable(test_data);
> >  }
> >  
> > -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,
> > +			 int cursor_w, int cursor_h)
> >  {
> >  	drmModeModeInfo *mode;
> >  	data_t *data = test_data->data;
> > @@ -219,9 +227,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);
> > @@ -249,7 +257,8 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
> >  	igt_output_set_pipe(output, PIPE_ANY);
> >  }
> >  
> > -static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
> > +static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen,
> > +		     int cursor_w, int cursor_h)
> >  {
> >  	igt_display_t *display = &data->display;
> >  	igt_output_t *output;
> > @@ -259,13 +268,12 @@ static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
> >  	};
> >  	int valid_tests = 0;
> >  
> > -
> >  	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_w, cursor_h))
> >  				continue;
> >  
> >  			valid_tests++;
> > @@ -274,8 +282,7 @@ 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);
> > -
> > +			test_crc(&test_data, cursor_type, onscreen, cursor_w, cursor_h);
> >  
> >  			fprintf(stdout, "\n%s on pipe %c, connector %s: PASSED\n\n",
> >  				igt_subtest_name(), pipe_name(test_data.pipe),
> > @@ -291,31 +298,72 @@ 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(data_t *data, int cursor_max_size)
> > +{
> > +	int cursor_size;
> > +	char c_size[5];
> > +	for (cursor_size = 64; cursor_size <= cursor_max_size; cursor_size *= 2)
> > +	{
> > +		igt_require(cursor_max_size >= cursor_size);
> > +		sprintf(c_size, "%d", cursor_size);
> > +
> > +		/* Creating cursor framebuffers */
> > +		create_cursor_fb(data, WHITE_VISIBLE, 1.0, 1.0, 1.0, 1.0, cursor_size, cursor_size);
> > +		create_cursor_fb(data, WHITE_INVISIBLE, 1.0, 1.0, 1.0, 0.0, cursor_size, cursor_size);
> > +		create_cursor_fb(data, BLACK_VISIBLE, 0.0, 0.0, 0.0, 1.0, cursor_size, cursor_size);
> > +		create_cursor_fb(data, BLACK_INVISIBLE, 0.0, 0.0, 0.0, 0.0, cursor_size, cursor_size);
> > +
> > +		/* Using created cursor FBs to test cursor support */
> > +		igt_subtest_f("white-visible-cursor-%s-onscreen", c_size)
> > +			run_test(data, WHITE_VISIBLE, true, cursor_size, cursor_size);
> > +		igt_subtest_f("white-invisible-cursor-%s-offscreen", c_size)
> > +			run_test(data, WHITE_INVISIBLE, false, cursor_size, cursor_size);
> > +                igt_subtest_f("black-visible-cursor-%s-onscreen", c_size)
> > +                        run_test(data, BLACK_VISIBLE, true, cursor_size, cursor_size);
> > +                igt_subtest_f("black-invisible-cursor-%s-offscreen", c_size)
> > +                        run_test(data, BLACK_INVISIBLE, false, cursor_size, cursor_size);
> > +	}
> > +
> > +}
> > +
> >  igt_main
> >  {
> >  	data_t data = {};
> > +	int cursor_max_size, ret;
> > +	uint64_t cursor_width, cursor_height;
> >  
> >  	igt_skip_on_simulation();
> >  
> >  	igt_fixture {
> >  		data.drm_fd = drm_open_any();
> >  
> > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width);
> > +		igt_assert(ret == 0);
> > +		/* Not making use of cursor_height since it is same as width, still reading */
> > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height);
> > +		igt_assert(ret == 0);
> > +
> > +		fprintf(stdout, "%d, %d\n", cursor_width, cursor_height);
> > +		/* We assume width and height are same so max is assigned width */
> 
> I would add here an explicit assert about the above. Other than this and
> the formatting issue above it looks ok:

Both fixed up in a follow-up patch, this one here in really tidy code
using igt_assert_cmpint.

> Reviewed-by: Imre Deak <imre.deak@intel.com>

Merged, thanks for patch&review.
-Daniel

> 
> > +		cursor_max_size = (int)cursor_width;
> > +
> >  		igt_set_vt_graphics_mode();
> >  
> >  		igt_debugfs_init(&data.debugfs);
> > @@ -325,28 +373,9 @@ igt_main
> >  		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);
> >  	}
> >  
> > -	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, cursor_max_size);
> >  
> >  	igt_fixture {
> >  		free(data.pipe_crc);
> 



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


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

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

* Re: [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes
  2014-03-20 16:33             ` Daniel Vetter
@ 2014-03-20 17:04               ` Sagar Arun Kamble
  0 siblings, 0 replies; 9+ messages in thread
From: Sagar Arun Kamble @ 2014-03-20 17:04 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Thanks Daniel.
On Thu, 2014-03-20 at 17:33 +0100, Daniel Vetter wrote:
> On Thu, Mar 20, 2014 at 06:18:05PM +0200, Imre Deak wrote:
> > On Tue, 2014-03-18 at 15:59 +0530, 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.
> > > 
> > > v3: Restructuring test enumeration using drmGetCap. [Daniel's review comments]
> > > 
> > > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > > ---
> > >  tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++++++++-------------------
> > >  1 file changed, 80 insertions(+), 51 deletions(-)
> > > 
> > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > index f98fbdb..9ddf9b4 100644
> > > --- a/tests/kms_cursor_crc.c
> > > +++ b/tests/kms_cursor_crc.c
> > > @@ -32,6 +32,13 @@
> > >  #include "igt_debugfs.h"
> > >  #include "igt_kms.h"
> > >  
> > > +#ifndef DRM_CAP_CURSOR_WIDTH
> > > +#define DRM_CAP_CURSOR_WIDTH 0x8
> > > +#endif
> > > +#ifndef DRM_CAP_CURSOR_HEIGHT
> > > +#define DRM_CAP_CURSOR_HEIGHT 0x9
> > > +#endif
> > > +
> > >  enum cursor_type {
> > >  	WHITE_VISIBLE,
> > >  	WHITE_INVISIBLE,
> > > @@ -124,7 +131,7 @@ static void cursor_disable(test_data_t *test_data)
> > >  }
> > >  
> > >  static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> > > -		     bool onscreen)
> > > +		     bool onscreen, int cursor_w, int cursor_h)
> > >  {
> > >  	int left = test_data->left;
> > >  	int right = test_data->right;
> > > @@ -135,43 +142,43 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> > >  
> > >  	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);
> > >  
> > >  		/* 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));
> > 
> > Nitpick: indent error, also the same on some of the following lines.
> > 
> > >  	} 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);
> > > @@ -180,7 +187,8 @@ static void test_crc(test_data_t *test_data, enum cursor_type cursor_type,
> > >  	cursor_disable(test_data);
> > >  }
> > >  
> > > -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,
> > > +			 int cursor_w, int cursor_h)
> > >  {
> > >  	drmModeModeInfo *mode;
> > >  	data_t *data = test_data->data;
> > > @@ -219,9 +227,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);
> > > @@ -249,7 +257,8 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
> > >  	igt_output_set_pipe(output, PIPE_ANY);
> > >  }
> > >  
> > > -static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
> > > +static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen,
> > > +		     int cursor_w, int cursor_h)
> > >  {
> > >  	igt_display_t *display = &data->display;
> > >  	igt_output_t *output;
> > > @@ -259,13 +268,12 @@ static void run_test(data_t *data, enum cursor_type cursor_type, bool onscreen)
> > >  	};
> > >  	int valid_tests = 0;
> > >  
> > > -
> > >  	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_w, cursor_h))
> > >  				continue;
> > >  
> > >  			valid_tests++;
> > > @@ -274,8 +282,7 @@ 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);
> > > -
> > > +			test_crc(&test_data, cursor_type, onscreen, cursor_w, cursor_h);
> > >  
> > >  			fprintf(stdout, "\n%s on pipe %c, connector %s: PASSED\n\n",
> > >  				igt_subtest_name(), pipe_name(test_data.pipe),
> > > @@ -291,31 +298,72 @@ 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(data_t *data, int cursor_max_size)
> > > +{
> > > +	int cursor_size;
> > > +	char c_size[5];
> > > +	for (cursor_size = 64; cursor_size <= cursor_max_size; cursor_size *= 2)
> > > +	{
> > > +		igt_require(cursor_max_size >= cursor_size);
> > > +		sprintf(c_size, "%d", cursor_size);
> > > +
> > > +		/* Creating cursor framebuffers */
> > > +		create_cursor_fb(data, WHITE_VISIBLE, 1.0, 1.0, 1.0, 1.0, cursor_size, cursor_size);
> > > +		create_cursor_fb(data, WHITE_INVISIBLE, 1.0, 1.0, 1.0, 0.0, cursor_size, cursor_size);
> > > +		create_cursor_fb(data, BLACK_VISIBLE, 0.0, 0.0, 0.0, 1.0, cursor_size, cursor_size);
> > > +		create_cursor_fb(data, BLACK_INVISIBLE, 0.0, 0.0, 0.0, 0.0, cursor_size, cursor_size);
> > > +
> > > +		/* Using created cursor FBs to test cursor support */
> > > +		igt_subtest_f("white-visible-cursor-%s-onscreen", c_size)
> > > +			run_test(data, WHITE_VISIBLE, true, cursor_size, cursor_size);
> > > +		igt_subtest_f("white-invisible-cursor-%s-offscreen", c_size)
> > > +			run_test(data, WHITE_INVISIBLE, false, cursor_size, cursor_size);
> > > +                igt_subtest_f("black-visible-cursor-%s-onscreen", c_size)
> > > +                        run_test(data, BLACK_VISIBLE, true, cursor_size, cursor_size);
> > > +                igt_subtest_f("black-invisible-cursor-%s-offscreen", c_size)
> > > +                        run_test(data, BLACK_INVISIBLE, false, cursor_size, cursor_size);
> > > +	}
> > > +
> > > +}
> > > +
> > >  igt_main
> > >  {
> > >  	data_t data = {};
> > > +	int cursor_max_size, ret;
> > > +	uint64_t cursor_width, cursor_height;
> > >  
> > >  	igt_skip_on_simulation();
> > >  
> > >  	igt_fixture {
> > >  		data.drm_fd = drm_open_any();
> > >  
> > > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width);
> > > +		igt_assert(ret == 0);
> > > +		/* Not making use of cursor_height since it is same as width, still reading */
> > > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height);
> > > +		igt_assert(ret == 0);
> > > +
> > > +		fprintf(stdout, "%d, %d\n", cursor_width, cursor_height);
> > > +		/* We assume width and height are same so max is assigned width */
> > 
> > I would add here an explicit assert about the above. Other than this and
> > the formatting issue above it looks ok:
> 
> Both fixed up in a follow-up patch, this one here in really tidy code
> using igt_assert_cmpint.
> 
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> Merged, thanks for patch&review.
> -Daniel
> 
> > 
> > > +		cursor_max_size = (int)cursor_width;
> > > +
> > >  		igt_set_vt_graphics_mode();
> > >  
> > >  		igt_debugfs_init(&data.debugfs);
> > > @@ -325,28 +373,9 @@ igt_main
> > >  		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);
> > >  	}
> > >  
> > > -	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, cursor_max_size);
> > >  
> > >  	igt_fixture {
> > >  		free(data.pipe_crc);
> > 
> 
> 
> 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 

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

end of thread, other threads:[~2014-03-20 17:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-24 15:37 [PATCH 1/1] kms_cursor_crc/Enabling this test for variable cursor width and heights sagar.a.kamble
2014-02-24 15:48 ` Ville Syrjälä
2014-03-05 14:05   ` Daniel Vetter
2014-03-08 18:49     ` [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors sagar.a.kamble
2014-03-17  8:05       ` Daniel Vetter
2014-03-18 10:29         ` [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes sagar.a.kamble
2014-03-20 16:18           ` Imre Deak
2014-03-20 16:33             ` Daniel Vetter
2014-03-20 17:04               ` Sagar Arun Kamble

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.