[igt-dev] [PATCH i-g-t] tests/kms_render: Rewrite test to take advantage of igt_display.

Mika Kahola mika.kahola at intel.com
Wed Jan 24 11:55:55 UTC 2018


On Wed, 2018-01-24 at 11:25 +0100, Maarten Lankhorst wrote:
> This test was taking ~100s for each subtest, and both tests were the
> same,
> but required the user to pay attention. I've changed it to automated
> checking with CRC, and removed the subtests.
> 
> This reduces the runtime for the test from about 200s to 10s
> (with NV12 enabled), and checks whether pixel formats are supported
> without special casing them.
I welcome this idea to reduce test execution time. At the moment our
IGT tests takes quite much time to complete.
 
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  tests/kms_render.c | 191 ++++++++++++++++++-------------------------
> ----------
>  1 file changed, 64 insertions(+), 127 deletions(-)
> 
> diff --git a/tests/kms_render.c b/tests/kms_render.c
> index 255cf3de39cf..0b31d351dd90 100644
> --- a/tests/kms_render.c
> +++ b/tests/kms_render.c
> @@ -31,16 +31,7 @@
>  
>  #include "intel_bufmgr.h"
>  
> -drmModeRes *resources;
> -int drm_fd;
> -
> -enum test_flags {
> -	TEST_DIRECT_RENDER	= 0x01,
> -	TEST_GPU_BLIT		= 0x02,
> -};
> -
> -static int paint_fb(struct igt_fb *fb, const char *test_name,
> -		    const char *mode_format_str, const char
> *cconf_str)
> +static int paint_fb(int drm_fd, struct igt_fb *fb, const char
> *mode_format_str, const char *cconf_str)
>  {
>  	cairo_t *cr;
>  
> @@ -51,7 +42,7 @@ static int paint_fb(struct igt_fb *fb, const char
> *test_name,
>  
>  	cairo_move_to(cr, fb->width / 2, fb->height / 2);
>  	cairo_set_font_size(cr, 36);
> -	igt_cairo_printf_line(cr, align_hcenter, 10, "%s",
> test_name);
> +	igt_cairo_printf_line(cr, align_hcenter, 10, "kms_render");
>  	igt_cairo_printf_line(cr, align_hcenter, 10, "%s",
> mode_format_str);
>  	igt_cairo_printf_line(cr, align_hcenter, 10, "%s",
> cconf_str);
>  
> @@ -60,7 +51,7 @@ static int paint_fb(struct igt_fb *fb, const char
> *test_name,
>  	return 0;
>  }
>  
> -static void gpu_blit(struct igt_fb *dst_fb, struct igt_fb *src_fb)
> +static void gpu_blit(int drm_fd, struct igt_fb *dst_fb, struct
> igt_fb *src_fb)
>  {
>  	drm_intel_bo *dst_bo;
>  	drm_intel_bo *src_bo;
> @@ -101,156 +92,102 @@ static void gpu_blit(struct igt_fb *dst_fb,
> struct igt_fb *src_fb)
>  	drm_intel_bo_unreference(dst_bo);
>  }
>  
> -static int test_format(const char *test_name,
> -		       struct kmstest_connector_config *cconf,
> -		       drmModeModeInfo *mode, uint32_t format,
> -		       enum test_flags flags)
> +static void test_format(igt_display_t *display,
> +			enum pipe pipe,
> +			igt_output_t *output,
> +			igt_plane_t *primary,
> +			uint32_t format)
>  {
> -	int width;
> -	int height;
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
> +	int width = mode->hdisplay, height = mode->vdisplay, ret,
> drm_fd = display->drm_fd;
This could be split into separate lines. It would be easier to read
that way.

Otherwise the patch looks ok.

>  	struct igt_fb fb[2];
>  	char *mode_format_str;
>  	char *cconf_str;
> -	int ret;
> +	igt_pipe_crc_t *pipe_crc;
> +	igt_crc_t direct, blit;
>  
>  	ret = asprintf(&mode_format_str, "%s @ %dHz / %s",
> -		 mode->name, mode->vrefresh,
> igt_format_str(format));
> +		       mode->name, mode->vrefresh,
> igt_format_str(format));
>  	igt_assert_lt(0, ret);
> +
>  	ret = asprintf(&cconf_str, "pipe %s, encoder %s, connector
> %s",
> -		       kmstest_pipe_name(cconf->pipe),
> -		       kmstest_encoder_type_str(cconf->encoder-
> >encoder_type),
> -		       kmstest_connector_type_str(cconf->connector-
> >connector_type));
> +		       kmstest_pipe_name(pipe),
> +		       kmstest_encoder_type_str(output-
> >config.encoder->encoder_type),
> +		       output->name);
>  	igt_assert_lt(0, ret);
>  
> -	igt_info("Beginning test %s with %s on %s\n",
> -		 test_name, mode_format_str, cconf_str);
> -
> -	width = mode->hdisplay;
> -	height = mode->vdisplay;
> -
> -	if (!igt_create_fb(drm_fd, width, height, format,
> -			   LOCAL_DRM_FORMAT_MOD_NONE, &fb[0]))
> -		goto err1;
> -
> -	if (!igt_create_fb(drm_fd, width, height, format,
> -			   LOCAL_DRM_FORMAT_MOD_NONE,	&fb[1])
> )
> -		goto err2;
> -
> -	if (drmModeSetCrtc(drm_fd, cconf->crtc->crtc_id,
> fb[0].fb_id,
> -				 0, 0, &cconf->connector-
> >connector_id, 1,
> -				 mode))
> -		goto err2;
> -	do_or_die(drmModePageFlip(drm_fd, cconf->crtc->crtc_id,
> fb[0].fb_id,
> -				  0, NULL));
> -	sleep(2);
> -
> -	if (flags & TEST_DIRECT_RENDER) {
> -		paint_fb(&fb[0], test_name, mode_format_str,
> cconf_str);
> -	} else if (flags & TEST_GPU_BLIT) {
> -		paint_fb(&fb[1], test_name, mode_format_str,
> cconf_str);
> -		gpu_blit(&fb[0], &fb[1]);
> -	}
> -	sleep(5);
> +	igt_create_fb(drm_fd, width, height, format,
> +		      LOCAL_DRM_FORMAT_MOD_NONE, &fb[0]);
>  
> -	igt_info("Test %s with %s on %s: PASSED\n",
> -		 test_name, mode_format_str, cconf_str);
> -	free(mode_format_str);
> -	free(cconf_str);
> +	igt_create_fb(drm_fd, width, height, format,
> +		      LOCAL_DRM_FORMAT_MOD_NONE, &fb[1]);
>  
> -	igt_remove_fb(drm_fd, &fb[1]);
> -	igt_remove_fb(drm_fd, &fb[0]);
> +	paint_fb(drm_fd, &fb[0], mode_format_str, cconf_str);
> +	gpu_blit(drm_fd, &fb[1], &fb[0]);
>  
> -	return 0;
> +	igt_plane_set_fb(primary, &fb[0]);
> +	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
> +
> +	pipe_crc = igt_pipe_crc_new(drm_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> +	igt_pipe_crc_collect_crc(pipe_crc, &direct);
> +
> +	igt_plane_set_fb(primary, &fb[1]);
> +	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
> +	igt_pipe_crc_collect_crc(pipe_crc, &blit);
> +
> +	igt_assert_crc_equal(&direct, &blit);
> +
> +	igt_pipe_crc_free(pipe_crc);
>  
> -err2:
> -	igt_remove_fb(drm_fd, &fb[0]);
> -err1:
> -	igt_info("Test %s with %s on %s: SKIPPED\n",
> -		 test_name, mode_format_str, cconf_str);
>  	free(mode_format_str);
>  	free(cconf_str);
>  
> -	return -1;
> +	igt_remove_fb(drm_fd, &fb[1]);
> +	igt_remove_fb(drm_fd, &fb[0]);
>  }
>  
> -static void test_connector(const char *test_name,
> -			   struct kmstest_connector_config *cconf,
> -			   enum test_flags flags)
> +static void test_connector(igt_display_t *display, enum pipe pipe,
> +			   igt_output_t *output)
>  {
>  	const uint32_t *formats;
> -	int format_count;
> -	int i;
> +	igt_plane_t *primary = &display->pipes[pipe].planes[0];
> +	int format_count, i, j, tested = 0;
> +
> +	igt_display_reset(display);
> +	igt_output_set_pipe(output, pipe);
>  
>  	igt_get_all_cairo_formats(&formats, &format_count);
>  	for (i = 0; i < format_count; i++) {
> -		if (is_i915_device(drm_fd)
> -		    && intel_gen(intel_get_drm_devid(drm_fd)) < 4
> -		    && formats[i] == DRM_FORMAT_XRGB2101010) {
> -			igt_info("gen2/3 don't support 10bpc,
> skipping\n");
> -			continue;
> -		}
> -
> -		test_format(test_name,
> -			    cconf, &cconf->connector->modes[0],
> -			    formats[i], flags);
> -	}
> -}
> -
> -static int run_test(const char *test_name, enum test_flags flags)
> -{
> -	int i;
> -
> -	resources = drmModeGetResources(drm_fd);
> -	igt_assert(resources);
> -
> -	/* Find any connected displays */
> -	for (i = 0; i < resources->count_connectors; i++) {
> -		uint32_t connector_id;
> -		int j;
> -
> -		connector_id = resources->connectors[i];
> -		for (j = 0; j < resources->count_crtcs; j++) {
> -			struct kmstest_connector_config cconf;
> -
> -			if (!kmstest_get_connector_config(drm_fd,
> connector_id,
> -							   1 << j,
> &cconf))
> +		for (j = 0; j < primary->drm_plane->count_formats;
> j++) {
> +			if (primary->drm_plane->formats[j] !=
> formats[i])
>  				continue;
>  
> -			test_connector(test_name, &cconf, flags);
> -
> -			kmstest_free_connector_config(&cconf);
> +			test_format(display, pipe, output,
> +				    primary, formats[i]);
> +			tested = 1;
> +			break;
>  		}
>  	}
> -
> -	drmModeFreeResources(resources);
> -
> -	return 1;
> +	igt_require_f(tested > 0, "No valid formats could be
> tested\n");
>  }
>  
> -igt_main
> +igt_simple_main
>  {
> -	struct {
> -		enum test_flags flags;
> -		const char *name;
> -	} tests[] = {
> -		{ TEST_DIRECT_RENDER,	"direct-render" },
> -		{ TEST_GPU_BLIT,	"gpu-blit" },
> -	};
> -	int i;
> +	igt_display_t display;
> +	igt_output_t *output;
> +	enum pipe pipe;
>  
>  	igt_skip_on_simulation();
>  
> -	igt_fixture {
> -		drm_fd = drm_open_driver_master(DRIVER_ANY);
> +	display.drm_fd = drm_open_driver_master(DRIVER_ANY);
>  
> -		kmstest_set_vt_graphics_mode();
> -	}
> +	kmstest_set_vt_graphics_mode();
> +	igt_display_init(&display, display.drm_fd);
> +	igt_display_require_output(&display);
>  
> -	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> -		igt_subtest(tests[i].name)
> -			run_test(tests[i].name, tests[i].flags);
> -	}
> +	for_each_pipe_with_valid_output(&display, pipe, output)
> +		test_connector(&display, pipe, output);
>  
> -	igt_fixture
> -		close(drm_fd);
> +	igt_display_fini(&display);
> +	close(display.drm_fd);
>  }
-- 
Mika Kahola - Intel OTC



More information about the igt-dev mailing list