[igt-dev] [PATCH i-g-t 3/3] tests/plane_lowres: Test each plane individually
Souza, Jose
jose.souza at intel.com
Mon Apr 1 19:47:03 UTC 2019
On Mon, 2019-04-01 at 14:18 +0100, Kahola, Mika wrote:
> 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.
Now it is pretty easy to change to just one plane if we want to.
>
> 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>
Thanks for the reviews, merged.
>
> > ---
> > 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);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190401/e55bad95/attachment-0001.sig>
More information about the igt-dev
mailing list