[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