Ville Syrjälä
2021-Mar-19 15:01 UTC
[Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:> From: Lyude Paul <lyude at redhat.com> > > Currently we just assume that every cursor size up to data->cursor_max_w/h will > be supported by the driver, and check for support of nonsquare cursors by > checking if we're running on u815 and if so, which variant of intel hardware > we're running on. This isn't really ideal as we're about to enable 32x32 cursor > size tests for nouveau, and Intel hardware doesn't support cursor sizes that > small. > > So, fix this by removing has_nonsquare_cursors() and replacing it with a more > generic require_cursor_size() function which checks whether or not the driver > we're using supports a given cursor size by attempting a test-only atomic > commit. > > Signed-off-by: Lyude Paul <lyude at redhat.com> > Cc: Martin Peres <martin.peres at free.fr> > Cc: Ben Skeggs <bskeggs at redhat.com> > Cc: Jeremy Cline <jcline at redhat.com> > --- > tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++----------------- > 1 file changed, 76 insertions(+), 55 deletions(-) > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c > index 3541ea06..b9c05472 100644 > --- a/tests/kms_cursor_crc.c > +++ b/tests/kms_cursor_crc.c > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int cur_w, int cur_h) > igt_put_cairo_ctx(cr); > } > > -static bool has_nonsquare_cursors(data_t *data) > +static void require_cursor_size(data_t *data, int w, int h) > { > - uint32_t devid; > + igt_fb_t primary_fb; > + drmModeModeInfo *mode; > + igt_display_t *display = &data->display; > + igt_output_t *output = data->output; > + igt_plane_t *primary, *cursor; > + int ret; > > - if (!is_i915_device(data->drm_fd)) > - return false; > + igt_output_set_pipe(output, data->pipe); > > - devid = intel_get_drm_devid(data->drm_fd); > + mode = igt_output_get_mode(output); > + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY); > + cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR); > > - /* > - * Test non-square cursors a bit on the platforms > - * that support such things. > - */ > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G) > - return true; > + /* Create temporary primary fb for testing */ > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, DRM_FORMAT_XRGB8888, > + LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb)); > > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)) > - return false; > + igt_plane_set_fb(primary, &primary_fb); > + igt_plane_set_fb(cursor, &data->fb); > + igt_plane_set_size(cursor, w, h); > + igt_fb_set_size(&data->fb, cursor, w, h); > + > + /* Test if the kernel supports the given cursor size or not */ > + ret = igt_display_try_commit_atomic(display, > + DRM_MODE_ATOMIC_TEST_ONLY | > + DRM_MODE_ATOMIC_ALLOW_MODESET, > + NULL);Would be better to not depend on atomic. We have platforms that don't expose it yet.> + > + igt_plane_set_fb(primary, NULL); > + igt_plane_set_fb(cursor, NULL); > + > + igt_remove_fb(data->drm_fd, &primary_fb); > + igt_output_set_pipe(output, PIPE_NONE); > > - return intel_gen(devid) >= 7; > + igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", w, h); > } > > static void test_cursor_size(data_t *data) > @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum pipe pipe) > create_cursor_fb(data, w, h); > } > > - /* Using created cursor FBs to test cursor support */ > - igt_describe("Check if a given-size cursor is well-positioned inside the screen."); > - igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", kmstest_pipe_name(pipe), w, h) > - run_test(data, test_crc_onscreen, w, h); > - > - igt_describe("Check if a given-size cursor is well-positioned outside the screen."); > - igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", kmstest_pipe_name(pipe), w, h) > - run_test(data, test_crc_offscreen, w, h); > - > - igt_describe("Check the smooth and pixel-by-pixel given-size cursor movements on" > - "horizontal, vertical and diagonal."); > - igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", kmstest_pipe_name(pipe), w, h) > - run_test(data, test_crc_sliding, w, h); > - > - igt_describe("Check random placement of a cursor with given size."); > - igt_subtest_f("pipe-%s-cursor-%dx%d-random", kmstest_pipe_name(pipe), w, h) > - run_test(data, test_crc_random, w, h); > - > - igt_describe("Check the rapid update of given-size cursor movements."); > - igt_subtest_f("pipe-%s-cursor-%dx%d-rapid-movement", kmstest_pipe_name(pipe), w, h) { > - run_test(data, test_rapid_movement, w, h); > + igt_subtest_group { > + igt_fixture > + require_cursor_size(data, w, h); > + > + /* Using created cursor FBs to test cursor support */ > + igt_describe("Check if a given-size cursor is well-positioned inside the " > + "screen."); > + igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_onscreen, w, h); > + > + igt_describe("Check if a given-size cursor is well-positioned outside the " > + "screen."); > + igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_offscreen, w, h); > + > + igt_describe("Check the smooth and pixel-by-pixel given-size cursor " > + "movements on horizontal, vertical and diagonal."); > + igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_sliding, w, h); > + > + igt_describe("Check random placement of a cursor with given size."); > + igt_subtest_f("pipe-%s-cursor-%dx%d-random", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_random, w, h); > + > + igt_describe("Check the rapid update of given-size cursor movements."); > + igt_subtest_f("pipe-%s-cursor-%dx%d-rapid-movement", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_rapid_movement, w, h); > } > > igt_fixture > @@ -730,27 +753,25 @@ static void run_tests_on_pipe(data_t *data, enum pipe pipe) > */ > h /= 3; > > - igt_fixture { > - if (has_nonsquare_cursors(data)) > - create_cursor_fb(data, w, h); > - } > + igt_fixture > + create_cursor_fb(data, w, h); > > - /* Using created cursor FBs to test cursor support */ > - igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", kmstest_pipe_name(pipe), w, h) { > - igt_require(has_nonsquare_cursors(data)); > - run_test(data, test_crc_onscreen, w, h); > - } > - igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", kmstest_pipe_name(pipe), w, h) { > - igt_require(has_nonsquare_cursors(data)); > - run_test(data, test_crc_offscreen, w, h); > - } > - igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", kmstest_pipe_name(pipe), w, h) { > - igt_require(has_nonsquare_cursors(data)); > - run_test(data, test_crc_sliding, w, h); > - } > - igt_subtest_f("pipe-%s-cursor-%dx%d-random", kmstest_pipe_name(pipe), w, h) { > - igt_require(has_nonsquare_cursors(data)); > - run_test(data, test_crc_random, w, h); > + igt_subtest_group { > + igt_fixture > + require_cursor_size(data, w, h); > + > + /* Using created cursor FBs to test cursor support */ > + igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_onscreen, w, h); > + > + igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_offscreen, w, h); > + > + igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_sliding, w, h); > + > + igt_subtest_f("pipe-%s-cursor-%dx%d-random", kmstest_pipe_name(pipe), w, h) > + run_test(data, test_crc_random, w, h); > } > > igt_fixture > -- > 2.29.2 > > _______________________________________________ > igt-dev mailing list > igt-dev at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev-- Ville Syrj?l? Intel
Lyude Paul
2021-Mar-19 17:40 UTC
[Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, 2021-03-19 at 17:01 +0200, Ville Syrj?l? wrote:> On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote: > > From: Lyude Paul <lyude at redhat.com> > > > > Currently we just assume that every cursor size up to data->cursor_max_w/h > > will > > be supported by the driver, and check for support of nonsquare cursors by > > checking if we're running on u815 and if so, which variant of intel hardware > > we're running on. This isn't really ideal as we're about to enable 32x32 > > cursor > > size tests for nouveau, and Intel hardware doesn't support cursor sizes that > > small. > > > > So, fix this by removing has_nonsquare_cursors() and replacing it with a > > more > > generic require_cursor_size() function which checks whether or not the > > driver > > we're using supports a given cursor size by attempting a test-only atomic > > commit. > > > > Signed-off-by: Lyude Paul <lyude at redhat.com> > > Cc: Martin Peres <martin.peres at free.fr> > > Cc: Ben Skeggs <bskeggs at redhat.com> > > Cc: Jeremy Cline <jcline at redhat.com> > > --- > > ?tests/kms_cursor_crc.c | 131 ++++++++++++++++++++++++----------------- > > ?1 file changed, 76 insertions(+), 55 deletions(-) > > > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c > > index 3541ea06..b9c05472 100644 > > --- a/tests/kms_cursor_crc.c > > +++ b/tests/kms_cursor_crc.c > > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int cur_w, > > int cur_h) > > ????????igt_put_cairo_ctx(cr); > > ?} > > ? > > -static bool has_nonsquare_cursors(data_t *data) > > +static void require_cursor_size(data_t *data, int w, int h) > > ?{ > > -???????uint32_t devid; > > +???????igt_fb_t primary_fb; > > +???????drmModeModeInfo *mode; > > +???????igt_display_t *display = &data->display; > > +???????igt_output_t *output = data->output; > > +???????igt_plane_t *primary, *cursor; > > +???????int ret; > > ? > > -???????if (!is_i915_device(data->drm_fd)) > > -???????????????return false; > > +???????igt_output_set_pipe(output, data->pipe); > > ? > > -???????devid = intel_get_drm_devid(data->drm_fd); > > +???????mode = igt_output_get_mode(output); > > +???????primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY); > > +???????cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR); > > ? > > -???????/* > > -??????? * Test non-square cursors a bit on the platforms > > -??????? * that support such things. > > -??????? */ > > -???????if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G) > > -???????????????return true; > > +???????/* Create temporary primary fb for testing */ > > +???????igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode- > > >vdisplay, DRM_FORMAT_XRGB8888, > > +??????????????????????????????? LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb)); > > ? > > -???????if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)) > > -???????????????return false; > > +???????igt_plane_set_fb(primary, &primary_fb); > > +???????igt_plane_set_fb(cursor, &data->fb); > > +???????igt_plane_set_size(cursor, w, h); > > +???????igt_fb_set_size(&data->fb, cursor, w, h); > > + > > +???????/* Test if the kernel supports the given cursor size or not */ > > +???????ret = igt_display_try_commit_atomic(display, > > +?????????????????????????????????????????? DRM_MODE_ATOMIC_TEST_ONLY | > > +?????????????????????????????????????????? DRM_MODE_ATOMIC_ALLOW_MODESET, > > +?????????????????????????????????????????? NULL); > > Would be better to not depend on atomic. We have platforms > that don't expose it yet.Do you have any other ideas how we could probe for this then? it seems like the only alternative would be to add intel-specific checks to fix that, or add some ioctl for querying the minimum cursor size (which sounds preferable imo). would the latter work for you, or do you have another idea?> > > + > > +???????igt_plane_set_fb(primary, NULL); > > +???????igt_plane_set_fb(cursor, NULL); > > + > > +???????igt_remove_fb(data->drm_fd, &primary_fb); > > +???????igt_output_set_pipe(output, PIPE_NONE); > > ? > > -???????return intel_gen(devid) >= 7; > > +???????igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", w, > > h); > > ?} > > ? > > ?static void test_cursor_size(data_t *data) > > @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum pipe > > pipe) > > ????????????????????????create_cursor_fb(data, w, h); > > ????????????????} > > ? > > -???????????????/* Using created cursor FBs to test cursor support */ > > -???????????????igt_describe("Check if a given-size cursor is well- > > positioned inside the screen."); > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", > > kmstest_pipe_name(pipe), w, h) > > -???????????????????????run_test(data, test_crc_onscreen, w, h); > > - > > -???????????????igt_describe("Check if a given-size cursor is well- > > positioned outside the screen."); > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", > > kmstest_pipe_name(pipe), w, h) > > -???????????????????????run_test(data, test_crc_offscreen, w, h); > > - > > -???????????????igt_describe("Check the smooth and pixel-by-pixel given-size > > cursor movements on" > > -??????????????????????????? "horizontal, vertical and diagonal."); > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", > > kmstest_pipe_name(pipe), w, h) > > -???????????????????????run_test(data, test_crc_sliding, w, h); > > - > > -???????????????igt_describe("Check random placement of a cursor with given > > size."); > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-random", > > kmstest_pipe_name(pipe), w, h) > > -???????????????????????run_test(data, test_crc_random, w, h); > > - > > -???????????????igt_describe("Check the rapid update of given-size cursor > > movements."); > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-rapid-movement", > > kmstest_pipe_name(pipe), w, h) { > > -???????????????????????run_test(data, test_rapid_movement, w, h); > > +???????????????igt_subtest_group { > > +???????????????????????igt_fixture > > +???????????????????????????????require_cursor_size(data, w, h); > > + > > +???????????????????????/* Using created cursor FBs to test cursor support > > */ > > +???????????????????????igt_describe("Check if a given-size cursor is well- > > positioned inside the " > > +??????????????????????????????????? "screen."); > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_onscreen, w, h); > > + > > +???????????????????????igt_describe("Check if a given-size cursor is well- > > positioned outside the " > > +??????????????????????????????????? "screen."); > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_offscreen, w, h); > > + > > +???????????????????????igt_describe("Check the smooth and pixel-by-pixel > > given-size cursor " > > +??????????????????????????????????? "movements on horizontal, vertical and > > diagonal."); > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_sliding, w, h); > > + > > +???????????????????????igt_describe("Check random placement of a cursor > > with given size."); > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-random", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_random, w, h); > > + > > +???????????????????????igt_describe("Check the rapid update of given-size > > cursor movements."); > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-rapid-movement", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_rapid_movement, w, h); > > ????????????????} > > ? > > ????????????????igt_fixture > > @@ -730,27 +753,25 @@ static void run_tests_on_pipe(data_t *data, enum pipe > > pipe) > > ???????????????? */ > > ????????????????h /= 3; > > ? > > -???????????????igt_fixture { > > -???????????????????????if (has_nonsquare_cursors(data)) > > -???????????????????????????????create_cursor_fb(data, w, h); > > -???????????????} > > +???????????????igt_fixture > > +???????????????????????create_cursor_fb(data, w, h); > > ? > > -???????????????/* Using created cursor FBs to test cursor support */ > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", > > kmstest_pipe_name(pipe), w, h) { > > -???????????????????????igt_require(has_nonsquare_cursors(data)); > > -???????????????????????run_test(data, test_crc_onscreen, w, h); > > -???????????????} > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", > > kmstest_pipe_name(pipe), w, h) { > > -???????????????????????igt_require(has_nonsquare_cursors(data)); > > -???????????????????????run_test(data, test_crc_offscreen, w, h); > > -???????????????} > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", > > kmstest_pipe_name(pipe), w, h) { > > -???????????????????????igt_require(has_nonsquare_cursors(data)); > > -???????????????????????run_test(data, test_crc_sliding, w, h); > > -???????????????} > > -???????????????igt_subtest_f("pipe-%s-cursor-%dx%d-random", > > kmstest_pipe_name(pipe), w, h) { > > -???????????????????????igt_require(has_nonsquare_cursors(data)); > > -???????????????????????run_test(data, test_crc_random, w, h); > > +???????????????igt_subtest_group { > > +???????????????????????igt_fixture > > +???????????????????????????????require_cursor_size(data, w, h); > > + > > +???????????????????????/* Using created cursor FBs to test cursor support > > */ > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_onscreen, w, h); > > + > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_offscreen, w, h); > > + > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-sliding", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_sliding, w, h); > > + > > +???????????????????????igt_subtest_f("pipe-%s-cursor-%dx%d-random", > > kmstest_pipe_name(pipe), w, h) > > +???????????????????????????????run_test(data, test_crc_random, w, h); > > ????????????????} > > ? > > ????????????????igt_fixture > > -- > > 2.29.2 > > > > _______________________________________________ > > igt-dev mailing list > > igt-dev at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/igt-dev >-- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!