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.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.
I'm ok with that. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to