[Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Wed Nov 1 14:23:46 UTC 2017
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 at linux.intel.com>
>>> Signed-off-by: Imre Deak <imre.deak at 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.
I'm ok with that. :)
More information about the Intel-gfx
mailing list