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.lankhorst@xxxxxxxxxxxxxxx> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
