[igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Tue Apr 9 11:47:32 UTC 2019
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.
>> + prepare_planes(data, pipe, &blue, tiling, c, output);
>> + err = igt_display_try_commit2(&data->display,
>> COMMIT_ATOMIC);
>
> No need to do a real commit here, you can only test with:
>
> err = igt_display_try_commit_atomic(&data->display,
> DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>
>
> Also if a err different than -EINVAL is returned you should handle like
> a error.
>
We're just enabling planes in the commit, which error would you expect?
>> +
>> + for_each_plane_on_pipe(&data->display, pipe, plane)
>> + igt_plane_set_fb(plane, NULL);
>> +
>> + for (int x = 0; x < c; x++)
>> + igt_remove_fb(data->drm_fd, &data->fb[x]);
>
> This is still leaking, example:
>
> c = 3
> plane 0, plane 1 and plane 3 enabled
>
> Other leak, the framebuffer created in test_grab_crc()
>
>
Huh? hmm... Your comment doesn't make sense. Maybe you have confused
idea of what plane vs framebuffer is or something?
>> + } while (!err && c < n_planes);
>> +
>> + if(err)
>> + c--;
>> +
>> + igt_info("Testing connector %s using pipe %s with %d planes %s
>> with seed %d\n",
>> + igt_output_name(output), kmstest_pipe_name(pipe), c,
>> + info, opt.seed);
>> +
>> i = 0;
>> while (i < iterations || loop_forever) {
>> - prepare_planes(data, pipe, &blue, tiling, n_planes,
>> output);
>> + prepare_planes(data, pipe, &blue, tiling, c, output);
>>
>> igt_display_commit2(&data->display, COMMIT_ATOMIC);
>>
>> igt_pipe_crc_get_current(data->display.drm_fd, data-
>>> pipe_crc, &crc);
>>
>> + for_each_plane_on_pipe(&data->display, pipe, plane)
>> + igt_plane_set_fb(plane, NULL);
>> +
>> + for (int x = 0; x < c; x++)
>> + igt_remove_fb(data->drm_fd, &data->fb[x]);
>
> move this cleanup to function and call it here and where you test the
> number of planes that can be enabled
>
>> +
>> igt_assert_crc_equal(&data->ref_crc, &crc);
>>
>> i++;
More information about the igt-dev
mailing list