[igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed

[Maarten Lankhorst]

Op 09-04-2019 om 13:47 schreef Juha-Pekka Heikkila:
> On 9.4.2019 4.29, Souza, Jose wrote:
>> On Fri, 2019-04-05 at 17:55 +0300, Juha-Pekka Heikkila wrote:
>>> before start testing try out how many planes kernel will
>>> allow simultaneously to be used.
>>>
>>> v2 (Jose Souza, Daniel Vetter): Fix resource leaks. Randomize
>>> used planes.
>>>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>>> ---
>>>   tests/kms_plane_multiple.c | 88
>>> +++++++++++++++++++++++++++++++++++++---------
>>>   1 file changed, 71 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
>>> index bfaeede..f2707dc 100644
>>> --- a/tests/kms_plane_multiple.c
>>> +++ b/tests/kms_plane_multiple.c
>>> @@ -80,16 +80,6 @@ static void test_fini(data_t *data, igt_output_t
>>> *output, int n_planes)
>>>   {
>>>       igt_pipe_crc_stop(data->pipe_crc);
>>>   -    for (int i = 0; i < n_planes; i++) {
>>> -        igt_plane_t *plane = data->plane[i];
>>> -        if (!plane)
>>> -            continue;
>>> -        if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>>> -            continue;
>>> -        igt_plane_set_fb(plane, NULL);
>>> -        data->plane[i] = NULL;
>>> -    }
>>> -
>>>       /* reset the constraint on the pipe */
>>>       igt_output_set_pipe(output, PIPE_ANY);
>>>   @@ -99,7 +89,8 @@ static void test_fini(data_t *data, igt_output_t
>>> *output, int n_planes)
>>>       free(data->plane);
>>>       data->plane = NULL;
>>>   -    igt_remove_fb(data->drm_fd, data->fb);
>>> +    free(data->fb);
>>> +    data->fb = NULL;
>>>         igt_display_reset(&data->display);
>>>   }
>>> @@ -195,6 +186,7 @@ prepare_planes(data_t *data, enum pipe pipe_id,
>>> color_t *color,
>>>       int *y;
>>>       int *size;
>>>       int i;
>>> +    int* suffle;
>>>         igt_output_set_pipe(output, pipe_id);
>>>       primary = igt_output_get_plane_type(output,
>>> DRM_PLANE_TYPE_PRIMARY);
>>> @@ -206,6 +198,34 @@ prepare_planes(data_t *data, enum pipe pipe_id,
>>> color_t *color,
>>>       igt_assert_f(y, "Failed to allocate %ld bytes for variable
>>> y\n", (long int) (pipe->n_planes * sizeof(*y)));
>>>       size = malloc(pipe->n_planes * sizeof(*size));
>>>       igt_assert_f(size, "Failed to allocate %ld bytes for variable
>>> size\n", (long int) (pipe->n_planes * sizeof(*size)));
>>> +    suffle = malloc(pipe->n_planes * sizeof(*suffle));
>>> +    igt_assert_f(suffle, "Failed to allocate %ld bytes for variable
>>> size\n", (long int) (pipe->n_planes * sizeof(*suffle)));
>>> +
>>> +    for (i = 0; i < pipe->n_planes; i++)
>>> +        suffle[i] = i;
>>> +
>>> +    /*
>>> +     * suffle table for planes. using rand() should keep it
>>> +     * 'randomized in expected way'
>>> +     */
>>> +    for (i = 0; i < 256; i++) {
>>
>> Not beautiful at all
>>
>>> +        int n, m;
>>> +        int a, b;
>>> +
>>> +        n = rand() % (pipe->n_planes-1);
>>> +        m = rand() % (pipe->n_planes-1);
>>> +
>>> +        /*
>>> +         * keep primary plane at its place for test's sake.
>>> +         */
>>> +        if(n == primary->index || m == primary->index)
>>> +            continue;
>>> +
>>> +        a = suffle[n];
>>> +        b = suffle[m];
>>> +        suffle[n] = b;
>>> +        suffle[m] = a;
>>> +    }
>>>         mode = igt_output_get_mode(output);
>>>   @@ -213,7 +233,11 @@ prepare_planes(data_t *data, enum pipe pipe_id,
>>> color_t *color,
>>>       x[primary->index] = 0;
>>>       y[primary->index] = 0;
>>>       for (i = 0; i < max_planes; i++) {
>>> -        igt_plane_t *plane = igt_output_get_plane(output, i);
>>> +        /*
>>> +         * Here is made assumption primary plane will have
>>> +         * index zero.
>>> +         */
>>> +        igt_plane_t *plane = igt_output_get_plane(output,
>>> suffle[i]);
>>>           uint32_t plane_format;
>>>           uint64_t plane_tiling;
>>>   @@ -251,6 +275,10 @@ prepare_planes(data_t *data, enum pipe pipe_id,
>>> color_t *color,
>>>       create_fb_for_mode_position(data, output, mode, color, x, y,
>>>                       size, size, tiling, max_planes);
>>>       igt_plane_set_fb(data->plane[primary->index], &data-
>>>> fb[primary->index]);
>>> +    free((void*)x);
>>> +    free((void*)y);
>>> +    free((void*)size);
>>> +    free((void*)suffle);
>>>   }
>>>     static void
>>> @@ -260,7 +288,9 @@ test_plane_position_with_output(data_t *data,
>>> enum pipe pipe,
>>>   {
>>>       color_t blue  = { 0.0f, 0.0f, 1.0f };
>>>       igt_crc_t crc;
>>> +    igt_plane_t *plane;
>>>       int i;
>>> +    int err, c = 0;
>>>       int iterations = opt.iterations < 1 ? 1 : opt.iterations;
>>>       bool loop_forever;
>>>       char info[256];
>>> @@ -274,22 +304,46 @@ test_plane_position_with_output(data_t *data,
>>> enum pipe pipe,
>>>               iterations, iterations > 1 ? "iterations" :
>>> "iteration");
>>>       }
>>>   -    igt_info("Testing connector %s using pipe %s with %d planes %s
>>> with seed %d\n",
>>> -         igt_output_name(output), kmstest_pipe_name(pipe),
>>> n_planes,
>>> -         info, opt.seed);
>>> -
>>>       test_init(data, pipe, n_planes);
>>>         test_grab_crc(data, output, pipe, &blue, tiling);
>>>   +    /*
>>> +     * Find out how many planes are allowed simultaneously
>>> +     */
>>> +    do {
>>> +        c++;
>>
>> Adding one plane at time is a waste of CI time in platforms without a
>> high number of planes(anything besides ICL), better start with max and
>> decreasing until commit is accepted.
>>
>
> I'd rather not try to hammer in configurations which may not work, that's not what we're testing here. Time anyways is less than 6 frames per loop so I don't think we are really wasting time. 


This is the whole point of atomic though. With the try_commit_atomic(TEST_ONLY | ALLOW_MODESET) mentioned by Jose you're not wasting any time in finding a configuration that works.. We don't need to commit it to HW, only have to test in sw. :)

~Maarten