Re: [PATCH v3 1/1] kms_cursor_crc: Enabling this test for all cursor sizes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx wrote:
> > > From: Sagar Kamble <sagar.a.kamble@xxxxxxxxx>
> > > 
> > > 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@xxxxxxxxx>
> > > ---
> > >  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@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 


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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux