Op 01-11-17 om 13:55 schreef Imre Deak: > 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. I'm ok with that. :) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
