[igt-dev] [PATCH i-g-t 3/3] tests/plane_lowres: Test each plane individually

Kahola, Mika mika.kahola at intel.com
Mon Apr 1 13:18:47 UTC 2019


I think in this test it would be sufficient to test only with one plane
as this test is intended to test resolution change. We do have
kms_plane_multiple test that tests all planes at once and thereofre
exercises full bandwith. Therefore, I see this as valid change to the
test.

On Fri, 2019-03-29 at 18:03 -0700, José Roberto de Souza wrote:
> ICL has some many planes per pipe that it is causing this test to
> skip due bandwidth limitation when combined with 4K displays.
> 
> The objective of this test is test the visibility of the planes when
> switching between high and low resolution, more information in the
> patch that added this test 12e34d8c909a ("tests/kms_plane_lowres:
> Plane visibility after atomic modesets").
> So it was setting all the planes the tested pipe in the bottom left
> of the display using the height of high resolution,  checking the
> visibility and then switching to the low resolution mode and checking
> again the visibility and now it is expected that all planes would be
> invisible.
> 
> So to overcome ICL bandwidth issues, here it is testing each plane
> individually.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Mika Kahola <mika.kahola at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> ---
>  lib/igt_kms.c            |  21 +--
>  lib/igt_kms.h            |   2 +-
>  tests/kms_plane_lowres.c | 274 ++++++++++++++++++-------------------
> --
>  3 files changed, 135 insertions(+), 162 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index ce9fe152..1e2415bf 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1634,27 +1634,18 @@ static void kmstest_get_crtc(int device, enum
> pipe pipe, struct kmstest_crtc *cr
>   *
>   * Asserts only if the plane's visibility state matches the status
> being passed by @visibility
>   */
> -void igt_assert_plane_visible(int fd, enum pipe pipe, bool
> visibility)
> +void igt_assert_plane_visible(int fd, enum pipe pipe, int
> plane_index, bool visibility)
>  {
>  	struct kmstest_crtc crtc;
> -	int i;
> -	bool visible;
> +	bool visible = true;
>  
>  	kmstest_get_crtc(fd, pipe, &crtc);
>  
> -	visible = true;
> -	for (i = 0; i < crtc.n_planes; i++) {
> -		if (crtc.planes[i].type == DRM_PLANE_TYPE_PRIMARY)
> -			continue;
> +	igt_assert(plane_index < crtc.n_planes);
>  
> -		if (crtc.planes[i].pos_x > crtc.width) {
> -			visible = false;
> -			break;
> -		} else if (crtc.planes[i].pos_y > crtc.height) {
> -			visible = false;
> -			break;
> -		}
> -	}
> +	if (crtc.planes[plane_index].pos_x > crtc.width ||
> +	    crtc.planes[plane_index].pos_y > crtc.height)
> +		visible = false;
>  
>  	free(crtc.planes);
>  	igt_assert_eq(visible, visibility);
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index e392e0ef..38bdc08f 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -228,7 +228,7 @@ void *kmstest_dumb_map_buffer(int fd, uint32_t
> handle, uint64_t size,
>  void kmstest_dumb_destroy(int fd, uint32_t handle);
>  void kmstest_wait_for_pageflip(int fd);
>  unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int
> flags);
> -void igt_assert_plane_visible(int fd, enum pipe pipe, bool
> visibility);
> +void igt_assert_plane_visible(int fd, enum pipe pipe, int
> plane_index, bool visibility);
>  
>  /*
>   * A small modeset API
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index 51bb7cd8..0b78573f 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -39,7 +39,8 @@ IGT_TEST_DESCRIPTION("Test atomic mode setting with
> a plane by switching between
>  typedef struct {
>  	int drm_fd;
>  	igt_display_t display;
> -	struct igt_fb *fb;
> +	struct igt_fb fb_primary;
> +	struct igt_fb fb_plane;
>  } data_t;
>  
>  static drmModeModeInfo
> @@ -64,24 +65,6 @@ get_lowres_mode(int drmfd, igt_output_t *output,
> drmModeModeInfo *mode_default)
>  	return mode;
>  }
>  
> -static void
> -test_fini(data_t *data, igt_output_t *output, enum pipe pipe)
> -{
> -	igt_plane_t *plane;
> -
> -	/* restore original mode */
> -	igt_output_override_mode(output, NULL);
> -
> -	for_each_plane_on_pipe(&data->display, pipe, plane)
> -		igt_plane_set_fb(plane, NULL);
> -
> -	/* reset the constraint on the pipe */
> -	igt_output_set_pipe(output, PIPE_ANY);
> -
> -	free(data->fb);
> -	data->fb = NULL;
> -}
> -
>  static void
>  check_mode(drmModeModeInfo *mode1, drmModeModeInfo *mode2)
>  {
> @@ -90,175 +73,160 @@ check_mode(drmModeModeInfo *mode1,
> drmModeModeInfo *mode2)
>  	igt_assert_eq(mode1->vrefresh, mode2->vrefresh);
>  }
>  
> -static drmModeModeInfo *
> -test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
> -	   igt_output_t *output)
> +/*
> + * Return false if test on this plane should be skipped
> + */
> +static bool
> +setup_plane(data_t *data, igt_plane_t *plane, drmModeModeInfo *mode,
> +	    uint64_t modifier)
>  {
> -	drmModeModeInfo *mode;
> -	int size;
> -	int i = 1, x, y;
> -	igt_plane_t *plane;
> -
> -	igt_skip_on(!igt_display_has_format_mod(&data->display,
> -						DRM_FORMAT_XRGB8888,
> -						modifier));
> -
> -	igt_output_set_pipe(output, pipe);
> -
> -	mode = igt_output_get_mode(output);
> -
> -	data->fb = calloc(data->display.pipes[pipe].n_planes,
> sizeof(struct igt_fb));
> -	igt_assert_f(data->fb, "Failed to allocate memory for %d
> FBs\n",
> -	             data->display.pipes[pipe].n_planes);
> -
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> -			    DRM_FORMAT_XRGB8888,
> -			    modifier,
> -			    0.0, 0.0, 1.0,
> -			    &data->fb[0]);
> -
> -	/* yellow sprite plane in lower left corner */
> -	for_each_plane_on_pipe(&data->display, pipe, plane) {
> -		uint64_t plane_modifier;
> -		uint32_t plane_format;
> -
> -		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> -			igt_plane_set_fb(plane, &data->fb[0]);
> -			continue;
> -		}
> -
> -		if (plane->type == DRM_PLANE_TYPE_CURSOR)
> -			size = 64;
> -		else
> -			size = SIZE;
> -
> -		x = 0;
> -		y = mode->vdisplay - size;
> -
> -		plane_format = plane->type == DRM_PLANE_TYPE_CURSOR ?
> -			DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
> -
> -		plane_modifier = plane->type == DRM_PLANE_TYPE_CURSOR ?
> -			LOCAL_DRM_FORMAT_MOD_NONE : modifier;
> +	uint64_t plane_modifier;
> +	uint32_t plane_format;
> +	int size, x, y;
> +
> +	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +		return false;
> +
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR)
> +		size = 64;
> +	else
> +		size = SIZE;
> +
> +	x = 0;
> +	y = mode->vdisplay - size;
> +
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> +		plane_format = DRM_FORMAT_ARGB8888;
> +		plane_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
> +	} else {
> +		plane_format = DRM_FORMAT_XRGB8888;
> +		plane_modifier = modifier;
> +	}
>  
> -		igt_skip_on(!igt_plane_has_format_mod(plane,
> plane_format,
> -						      plane_modifier));
> +	if (!igt_plane_has_format_mod(plane, plane_format,
> plane_modifier))
> +		return false;
>  
> -		igt_create_color_fb(data->drm_fd,
> -				    size, size,
> -				    plane_format,
> -				    plane_modifier,
> -				    1.0, 1.0, 0.0,
> -				    &data->fb[i]);
> +	igt_create_color_fb(data->drm_fd, size, size, plane_format,
> +			    plane_modifier, 1.0, 1.0, 0.0, &data-
> >fb_plane);
> +	igt_plane_set_position(plane, x, y);
> +	igt_plane_set_fb(plane, &data->fb_plane);
>  
> -		igt_plane_set_position(plane, x, y);
> -		igt_plane_set_fb(plane, &data->fb[i++]);
> -	}
> +	return true;
> +}
>  
> -	return mode;
> +static igt_plane_t *
> +primary_plane_get(igt_display_t *display, enum pipe pipe)
> +{
> +	unsigned plane_primary_id = display->pipes[pipe].plane_primary;
> +	return &display->pipes[pipe].planes[plane_primary_id];
>  }
>  
> -static void
> -test_plane_position_with_output(data_t *data, enum pipe pipe,
> +static unsigned
> +test_planes_on_pipe_with_output(data_t *data, enum pipe pipe,
>  				igt_output_t *output, uint64_t
> modifier)
>  {
> -	igt_crc_t crc_hires1, crc_hires2;
> -	igt_crc_t crc_lowres;
> -	drmModeModeInfo mode_lowres;
> -	drmModeModeInfo *mode1, *mode2, *mode3;
> -	int ret;
> +	drmModeModeInfo *mode, mode_lowres;
>  	igt_pipe_crc_t *pipe_crc;
> +	unsigned tested = 0;
> +	igt_plane_t *plane;
>  
>  	igt_info("Testing connector %s using pipe %s\n",
>  		 igt_output_name(output), kmstest_pipe_name(pipe));
>  
> -	mode1 = test_setup(data, pipe, modifier, output);
> +	igt_output_set_pipe(output, pipe);
> +	mode = igt_output_get_mode(output);
> +	mode_lowres = get_lowres_mode(data->drm_fd, output, mode);
>  
> -	mode_lowres = get_lowres_mode(data->drm_fd, output, mode1);
> +	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> +			    DRM_FORMAT_XRGB8888, modifier, 0.0, 0.0,
> 1.0,
> +			    &data->fb_primary);
> +	igt_plane_set_fb(primary_plane_get(&data->display, pipe),
> +			 &data->fb_primary);
>  
> -	ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
> -	igt_skip_on(ret != 0);
> +	pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> +				    INTEL_PIPE_CRC_SOURCE_AUTO);
>  
> -	pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> -	igt_pipe_crc_start(pipe_crc);
> -	igt_pipe_crc_get_single(pipe_crc, &crc_hires1);
> +	/* yellow sprite plane in lower left corner */
> +	for_each_plane_on_pipe(&data->display, pipe, plane) {
> +		igt_crc_t crc_hires1, crc_hires2;
> +		int r;
>  
> -	igt_assert_plane_visible(data->drm_fd, pipe, true);
> +		if (!setup_plane(data, plane, mode, modifier))
> +			continue;
>  
> -	/* switch to lower resolution */
> -	igt_output_override_mode(output, &mode_lowres);
> -	igt_output_set_pipe(output, pipe);
> +		r = igt_display_try_commit2(&data->display,
> COMMIT_ATOMIC);
> +		if (r) {
> +			igt_debug("Commit failed %i", r);
> +			continue;
> +		}
>  
> -	mode2 = igt_output_get_mode(output);
> +		igt_pipe_crc_start(pipe_crc);
> +		igt_pipe_crc_get_single(pipe_crc, &crc_hires1);
>  
> -	check_mode(&mode_lowres, mode2);
> +		igt_assert_plane_visible(data->drm_fd, pipe, plane-
> >index,
> +					 true);
>  
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc,
> &crc_lowres);
> +		/* switch to lower resolution */
> +		igt_output_override_mode(output, &mode_lowres);
> +		igt_output_set_pipe(output, pipe);
> +		check_mode(&mode_lowres, igt_output_get_mode(output));
>  
> -	igt_assert_plane_visible(data->drm_fd, pipe, false);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
> -	/* switch back to higher resolution */
> -	igt_output_override_mode(output, NULL);
> -	igt_output_set_pipe(output, pipe);
> +		igt_assert_plane_visible(data->drm_fd, pipe, plane-
> >index,
> +					 false);
>  
> -	mode3 = igt_output_get_mode(output);
> +		/* switch back to higher resolution */
> +		igt_output_override_mode(output, NULL);
> +		igt_output_set_pipe(output, pipe);
> +		check_mode(mode, igt_output_get_mode(output));
>  
> -	check_mode(mode1, mode3);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +		igt_pipe_crc_get_current(data->drm_fd, pipe_crc,
> &crc_hires2);
>  
> -	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc,
> &crc_hires2);
> +		igt_assert_plane_visible(data->drm_fd, pipe, plane-
> >index,
> +					 true);
>  
> -	igt_assert_plane_visible(data->drm_fd, pipe, true);
> +		igt_assert_crc_equal(&crc_hires1, &crc_hires2);
>  
> -	igt_assert_crc_equal(&crc_hires1, &crc_hires2);
> +		igt_pipe_crc_stop(pipe_crc);
>  
> -	igt_pipe_crc_stop(pipe_crc);
> -	igt_pipe_crc_free(pipe_crc);
> +		igt_plane_set_fb(plane, NULL);
> +		igt_remove_fb(data->drm_fd, &data->fb_plane);
> +		tested++;
> +	}
>  
> -	test_fini(data, output, pipe);
> -}
> +	igt_pipe_crc_free(pipe_crc);
>  
> -static void
> -test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
> -{
> -	igt_output_t *output;
> +	igt_plane_set_fb(primary_plane_get(&data->display, pipe),
> NULL);
> +	igt_remove_fb(data->drm_fd, &data->fb_primary);
> +	igt_output_set_pipe(output, PIPE_NONE);
>  
> -	for_each_valid_output_on_pipe(&data->display, pipe, output)
> -		test_plane_position_with_output(data, pipe, output,
> modifier);
> +	return tested;
>  }
>  
>  static void
> -run_tests_for_pipe(data_t *data, enum pipe pipe)
> +test_planes_on_pipe(data_t *data, enum pipe pipe, uint64_t modifier)
>  {
> -	igt_fixture {
> -		igt_skip_on(pipe >= data->display.n_pipes);
> -
> -		igt_display_require_output_on_pipe(&data->display,
> pipe);
> -	}
> -
> -	igt_subtest_f("pipe-%s-tiling-none",
> -		      kmstest_pipe_name(pipe))
> -		test_plane_position(data, pipe,
> LOCAL_DRM_FORMAT_MOD_NONE);
> +	igt_output_t *output;
> +	unsigned tested = 0;
>  
> -	igt_subtest_f("pipe-%s-tiling-x",
> -		      kmstest_pipe_name(pipe))
> -		test_plane_position(data, pipe,
> LOCAL_I915_FORMAT_MOD_X_TILED);
> +	igt_skip_on(pipe >= data->display.n_pipes);
> +	igt_display_require_output_on_pipe(&data->display, pipe);
> +	igt_skip_on(!igt_display_has_format_mod(&data->display,
> +						DRM_FORMAT_XRGB8888,
> modifier));
>  
> -	igt_subtest_f("pipe-%s-tiling-y",
> -		      kmstest_pipe_name(pipe))
> -		test_plane_position(data, pipe,
> LOCAL_I915_FORMAT_MOD_Y_TILED);
> +	for_each_valid_output_on_pipe(&data->display, pipe, output)
> +		tested += test_planes_on_pipe_with_output(data, pipe,
> output,
> +							  modifier);
>  
> -	igt_subtest_f("pipe-%s-tiling-yf",
> -		      kmstest_pipe_name(pipe))
> -		test_plane_position(data, pipe,
> LOCAL_I915_FORMAT_MOD_Yf_TILED);
> +	igt_assert(tested > 0);
>  }
>  
> -static data_t data;
> -
>  igt_main
>  {
> +	data_t data = {};
>  	enum pipe pipe;
>  
>  	igt_skip_on_simulation();
> @@ -273,9 +241,23 @@ igt_main
>  		igt_require(data.display.is_atomic);
>  	}
>  
> -	for_each_pipe_static(pipe)
> -		igt_subtest_group
> -			run_tests_for_pipe(&data, pipe);
> +	for_each_pipe_static(pipe) {
> +		igt_subtest_f("pipe-%s-tiling-none",
> kmstest_pipe_name(pipe))
> +			test_planes_on_pipe(&data, pipe,
> +					    LOCAL_DRM_FORMAT_MOD_NONE);
> +
> +		igt_subtest_f("pipe-%s-tiling-x",
> kmstest_pipe_name(pipe))
> +			test_planes_on_pipe(&data, pipe,
> +					    LOCAL_I915_FORMAT_MOD_X_TIL
> ED);
> +
> +		igt_subtest_f("pipe-%s-tiling-y",
> kmstest_pipe_name(pipe))
> +			test_planes_on_pipe(&data, pipe,
> +					    LOCAL_I915_FORMAT_MOD_Y_TIL
> ED);
> +
> +		igt_subtest_f("pipe-%s-tiling-yf",
> kmstest_pipe_name(pipe))
> +			test_planes_on_pipe(&data, pipe,
> +					    LOCAL_I915_FORMAT_MOD_Yf_TI
> LED);
> +	}
>  
>  	igt_fixture {
>  		igt_display_fini(&data.display);


More information about the igt-dev mailing list