On Wed, Nov 01, 2017 at 12:32:37PM +0100, Maarten Lankhorst wrote:
> Op 31-10-17 om 14:44 schreef Imre Deak:
> > Doing modeset on internal panels may have a considerable overhead due to
> > the panel specific power sequencing delays. To avoid long test runtimes
> > limit the runtime of each subtest. Randomize the plane/pipe combinations
> > to preserve the test coverage on such panels at least over multiple test
> > runs.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.d...@intel.com>
> > ---
> > tests/kms_atomic_transition.c | 175
> > ++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 150 insertions(+), 25 deletions(-)
> >
> > diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> > index 4c295125..ac67fc3a 100644
> > --- a/tests/kms_atomic_transition.c
> > +++ b/tests/kms_atomic_transition.c
> > @@ -39,6 +39,14 @@
> > #define DRM_CAP_CURSOR_HEIGHT 0x9
> > #endif
> >
> > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > +
> > +struct test_config {
> > + igt_display_t *display;
> > + bool user_seed;
> > + int seed;
> > +};
> > +
> > struct plane_parms {
> > struct igt_fb *fb;
> > uint32_t width, height;
> > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t
> > *display, enum pipe pipe, bool non
> > }
> > }
> >
> > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > +static void shuffle_array(uint32_t *array, int size, int seed)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < size; i++) {
> > + int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > +
> > + igt_swap(array[i], array[j]);
> > + }
> > +}
> I wouldn't worry about predictibility of the random number generator..
>
> int j = i + rand() % (size - i) is good enough and easier to read. :)
Chris already suggested igt_permute_array(), will use that.
>
> I think the struct test_config can be killed too, since it goes unused
> in shuffle_array, nothing in the test uses it...
Oops, some kind of left-over from an earlier version. Thanks for spotting
it.
> > +static void init_combination_array(uint32_t *array, int size, int seed)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < size; i++)
> > + array[i] = i;
> > +
> > + shuffle_array(array, size, seed);
> > +}
> > +
> > /*
> > * 1. Set primary plane to a known fb.
> > * 2. Make sure getcrtc returns the correct fb id.
> > @@ -411,19 +441,27 @@ static void wait_for_transition(igt_display_t
> > *display, enum pipe pipe, bool non
> > * so test this and make sure it works.
> > */
> > static void
> > -run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t
> > *output,
> > - enum transition_type type, bool nonblocking, bool fencing)
> > +run_transition_test(struct test_config *test_config, enum pipe pipe,
> > + igt_output_t *output, enum transition_type type,
> > + bool nonblocking, bool fencing)
> > {
> > struct igt_fb fb, argb_fb, sprite_fb;
> > drmModeModeInfo *mode, override_mode;
> > + igt_display_t *display = test_config->display;
> > igt_plane_t *plane;
> > igt_pipe_t *pipe_obj = &display->pipes[pipe];
> > uint32_t iter_max = 1 << pipe_obj->n_planes, i;
> > + uint32_t *plane_combinations;
> > + struct timespec start = { };
> > struct plane_parms parms[pipe_obj->n_planes];
> > bool skip_test = false;
> > unsigned flags = 0;
> > int ret;
> >
> > + plane_combinations = malloc(sizeof(*plane_combinations) * iter_max);
> > + igt_assert(plane_combinations);
> > + init_combination_array(plane_combinations, iter_max, test_config->seed);
> It would be cleaner to have a separate test for transition_modeset.
> The rest should be run to completion and don't need shuffling, since
> in normal cases, they'll never hit a timeout.
Do you mean type == TRANSITION_MODESET? There are already separate
subtests for that. Yes, can disable the timeout and shuffling for the
rest.
> So make a separate test for that, and perhaps also add a flag for
> disabling the timeout.
Ok.
>
> > if (fencing)
> > prepare_fencing(display, pipe);
> > else
> > @@ -527,39 +565,59 @@ run_transition_test(igt_display_t *display, enum pipe
> > pipe, igt_output_t *output
> > goto cleanup;
> > }
> >
> > + igt_nsec_elapsed(&start);
> > +
> > for (i = 0; i < iter_max; i++) {
> > igt_output_set_pipe(output, pipe);
> >
> > - wm_setup_plane(display, pipe, i, parms, fencing);
> > + wm_setup_plane(display, pipe, plane_combinations[i], parms,
> > + fencing);
> >
> > - atomic_commit(display, pipe, flags, (void *)(unsigned long)i,
> > fencing);
> > + atomic_commit(display, pipe, flags,
> > + (void *)(unsigned long)plane_combinations[i],
> > + fencing);
> > wait_for_transition(display, pipe, nonblocking, fencing);
> >
> > if (type == TRANSITION_MODESET_DISABLE) {
> > + if (igt_nsec_elapsed(&start) >= MAX_SUBTEST_DURATION_NS)
> > + goto cleanup;
> > +
> > igt_output_set_pipe(output, PIPE_NONE);
> >
> > wm_setup_plane(display, pipe, 0, parms, fencing);
> >
> > atomic_commit(display, pipe, flags, (void *) 0UL,
> > fencing);
> > wait_for_transition(display, pipe, nonblocking,
> > fencing);
> > +
> > } else {
> > uint32_t j;
> >
> > /* i -> i+1 will be done when i increases, can be
> > skipped here */
> > for (j = iter_max - 1; j > i + 1; j--) {
> > - wm_setup_plane(display, pipe, j, parms,
> > fencing);
> > + if (igt_nsec_elapsed(&start) >=
> > MAX_SUBTEST_DURATION_NS)
> > + goto cleanup;
> > +
> > + wm_setup_plane(display, pipe,
> > + plane_combinations[j], parms,
> > + fencing);
> >
> > if (type == TRANSITION_MODESET)
> > igt_output_override_mode(output,
> > &override_mode);
> >
> > - atomic_commit(display, pipe, flags, (void
> > *)(unsigned long) j, fencing);
> > + atomic_commit(display, pipe, flags,
> > + (void *)(unsigned
> > long)plane_combinations[j],
> > + fencing);
> > wait_for_transition(display, pipe, nonblocking,
> > fencing);
> >
> > - wm_setup_plane(display, pipe, i, parms,
> > fencing);
> > + wm_setup_plane(display, pipe,
> > + plane_combinations[i], parms,
> > + fencing);
> > if (type == TRANSITION_MODESET)
> > igt_output_override_mode(output, NULL);
> >
> > - atomic_commit(display, pipe, flags, (void
> > *)(unsigned long) i, fencing);
> > + atomic_commit(display, pipe, flags,
> > + (void *)(unsigned
> > long)plane_combinations[i],
> > + fencing);
> > wait_for_transition(display, pipe, nonblocking,
> > fencing);
> > }
> > }
> > @@ -579,6 +637,9 @@ cleanup:
> > igt_remove_fb(display->drm_fd, &fb);
> > igt_remove_fb(display->drm_fd, &argb_fb);
> > igt_remove_fb(display->drm_fd, &sprite_fb);
> > +
> > + free(plane_combinations);
> > +
> > if (skip_test)
> > igt_skip("Atomic nonblocking modesets are not supported.\n");
> > }
> > @@ -696,16 +757,24 @@ static void collect_crcs_mask(igt_pipe_crc_t
> > **pipe_crcs, unsigned mask, igt_crc
> > }
> > }
> >
> > -static void run_modeset_tests(igt_display_t *display, int howmany, bool
> > nonblocking, bool fencing)
> > +static void run_modeset_tests(struct test_config *test_config, int howmany,
> > + bool nonblocking, bool fencing)
> > {
> > + igt_display_t *display = test_config->display;
> > struct igt_fb fbs[2];
> > int i, j;
> > unsigned iter_max = 1 << display->n_pipes;
> > + uint32_t *pipe_combinations;
> > + struct timespec start = { };
> > igt_pipe_crc_t *pipe_crcs[IGT_MAX_PIPES] = { 0 };
> > igt_output_t *output;
> > unsigned width = 0, height = 0;
> > bool skip_test = false;
> >
> > + pipe_combinations = malloc(sizeof(*pipe_combinations) * iter_max);
> > + igt_assert(pipe_combinations);
> > + init_combination_array(pipe_combinations, iter_max, test_config->seed);
>
> Kill all the changes to run_modeset_tests too please? The randomness
> here goes unused, and I would rather have it run all the modesetting
> combinations. I can understand plane transitions taking too long, but
> the modeset tests are typically very short.
IIUC with 3 pipes it's 27 iterations, so with 2 full modesets per iteration it
can take ~1 minute using slow panels. The same with 4 pipes would take ~4
minutes.
> > for_each_connected_output(display, output) {
> > drmModeModeInfo *mode = igt_output_get_mode(output);
> >
> > @@ -757,6 +826,8 @@ static void run_modeset_tests(igt_display_t *display,
> > int howmany, bool nonblock
> >
> > igt_display_commit2(display, COMMIT_ATOMIC);
> >
> > + igt_nsec_elapsed(&start);
> > +
> > for (i = 0; i < iter_max; i++) {
> > igt_crc_t crcs[5][IGT_MAX_PIPES];
> > unsigned event_mask;
> > @@ -773,6 +844,9 @@ static void run_modeset_tests(igt_display_t *display,
> > int howmany, bool nonblock
> > collect_crcs_mask(pipe_crcs, i, crcs[0]);
> >
> > for (j = iter_max - 1; j > i + 1; j--) {
> > + if (igt_nsec_elapsed(&start) >= MAX_SUBTEST_DURATION_NS)
> > + goto cleanup;
> > +
> > if (hweight32(j) > howmany)
> > continue;
> >
> > @@ -828,13 +902,18 @@ cleanup:
> > igt_remove_fb(display->drm_fd, &fbs[1]);
> > igt_remove_fb(display->drm_fd, &fbs[0]);
> >
> > + free(pipe_combinations);
> > +
> > if (skip_test)
> > igt_skip("Atomic nonblocking modesets are not supported.\n");
> >
> > }
> >
> > -static void run_modeset_transition(igt_display_t *display, int
> > requested_outputs, bool nonblocking, bool fencing)
> > +static void run_modeset_transition(struct test_config *test_config,
> > + int requested_outputs, bool nonblocking,
> > + bool fencing)
> > {
> > + igt_display_t *display = test_config->display;
> > igt_output_t *outputs[IGT_MAX_PIPES] = {};
> > int num_outputs = 0;
> > enum pipe pipe;
> > @@ -861,16 +940,41 @@ static void run_modeset_transition(igt_display_t
> > *display, int requested_outputs
> > "Should have at least %i outputs, found %i\n",
> > requested_outputs, num_outputs);
> >
> > - run_modeset_tests(display, requested_outputs, nonblocking, fencing);
> > + run_modeset_tests(test_config, requested_outputs, nonblocking, fencing);
> > }
> >
> > -igt_main
> > +static int opt_handler(int option, int option_index, void *handler_data)
> > {
> > + struct test_config *test_config = handler_data;
> > +
> > + switch (option) {
> > + case 's':
> > + test_config->user_seed = true;
> > + test_config->seed = strtol(optarg, NULL, 0);
> > + break;
> > + default:
> > + igt_assert(false);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + const char *help_str =
> > + " --seed Seed for random number generator\n";
> > + struct option long_options[] = {
> > + { "seed", required_argument, NULL, 's' },
> > + { },
> > + };
> > + struct test_config test_config = { };
> > igt_display_t display;
> > igt_output_t *output;
> > enum pipe pipe;
> > int i;
> >
> > + igt_subtest_init_parse_opts(&argc, argv, "", long_options, help_str,
> > + opt_handler, &test_config);
> > igt_skip_on_simulation();
> >
> > igt_fixture {
> > @@ -883,6 +987,15 @@ igt_main
> > igt_require(display.is_atomic);
> >
> > igt_display_require_output(&display);
> > +
> > + test_config.display = &display;
> > + if (!test_config.user_seed)
> > + test_config.seed = time(NULL);
> > +
> > + srand(test_config.seed);
> > +
> > + igt_info("Running tests randomized with seed %d\n",
> > + test_config.seed);
> > }
> >
> > igt_subtest("plane-primary-toggle-with-vblank-wait")
> > @@ -891,55 +1004,67 @@ igt_main
> >
> > igt_subtest("plane-all-transition")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_PLANES, false, false);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_PLANES, false, false);
> >
> > igt_subtest("plane-all-transition-fencing")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_PLANES, false, true);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_PLANES, false, true);
> >
> > igt_subtest("plane-all-transition-nonblocking")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_PLANES, true, false);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_PLANES, true, false);
> >
> > igt_subtest("plane-all-transition-nonblocking-fencing")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_PLANES, true, true);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_PLANES, true, true);
> >
> > igt_subtest("plane-use-after-nonblocking-unbind")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_AFTER_FREE, true, false);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_AFTER_FREE, true, false);
> >
> > igt_subtest("plane-use-after-nonblocking-unbind-fencing")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_AFTER_FREE, true, true);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_AFTER_FREE, true, true);
> >
> > igt_subtest("plane-all-modeset-transition")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_MODESET, false, false);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_MODESET, false, false);
> >
> > igt_subtest("plane-all-modeset-transition-fencing")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_MODESET, false, true);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_MODESET, false, true);
> >
> > igt_subtest("plane-toggle-modeset-transition")
> > for_each_pipe_with_valid_output(&display, pipe, output)
> > - run_transition_test(&display, pipe, output,
> > TRANSITION_MODESET_DISABLE, false, false);
> > + run_transition_test(&test_config, pipe, output,
> > + TRANSITION_MODESET_DISABLE,
> > + false, false);
> >
> > for (i = 1; i <= IGT_MAX_PIPES; i++) {
> > igt_subtest_f("%ix-modeset-transitions", i)
> > - run_modeset_transition(&display, i, false, false);
> > + run_modeset_transition(&test_config, i, false, false);
> >
> > igt_subtest_f("%ix-modeset-transitions-nonblocking", i)
> > - run_modeset_transition(&display, i, true, false);
> > + run_modeset_transition(&test_config, i, true, false);
> >
> > igt_subtest_f("%ix-modeset-transitions-fencing", i)
> > - run_modeset_transition(&display, i, false, true);
> > + run_modeset_transition(&test_config, i, false, true);
> >
> > igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i)
> > - run_modeset_transition(&display, i, true, true);
> > + run_modeset_transition(&test_config, i, true, true);
> > }
> >
> > igt_fixture {
> > igt_display_fini(&display);
> > }
> > +
> > + igt_exit();
> > }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
