* [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed
@ 2019-04-05 14:55 Juha-Pekka Heikkila
2019-04-05 16:16 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) Patchwork
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Juha-Pekka Heikkila @ 2019-04-05 14:55 UTC (permalink / raw)
To: igt-dev; +Cc: Lisovskiy, Stanislav
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@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++) {
+ 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++;
+ prepare_planes(data, pipe, &blue, tiling, c, output);
+ err = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
+
+ 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]);
+ } 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]);
+
igt_assert_crc_equal(&data->ref_crc, &crc);
i++;
--
2.7.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 13+ messages in thread* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) 2019-04-05 14:55 [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Juha-Pekka Heikkila @ 2019-04-05 16:16 ` Patchwork 2019-04-06 14:19 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-04-05 16:16 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) URL : https://patchwork.freedesktop.org/series/58875/ State : success == Summary == CI Bug Log - changes from CI_DRM_5881 -> IGTPW_2802 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/58875/revisions/2/mbox/ Known issues ------------ Here are the changes found in IGTPW_2802 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718] #### Possible fixes #### * igt@i915_pm_rpm@module-reload: - fi-skl-6770hq: FAIL [fdo#108511] -> PASS * igt@i915_selftest@live_execlists: - fi-apl-guc: INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS * igt@i915_selftest@live_uncore: - fi-ivb-3770: DMESG-FAIL [fdo#110210] -> PASS * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724 [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511 [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720 [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210 Participating hosts (49 -> 45) ------------------------------ Missing (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bdw-samus Build changes ------------- * IGT: IGT_4931 -> IGTPW_2802 CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2802: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2802/ IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2802/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) 2019-04-05 14:55 [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Juha-Pekka Heikkila 2019-04-05 16:16 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) Patchwork @ 2019-04-06 14:19 ` Patchwork 2019-04-09 1:29 ` [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Souza, Jose 2019-05-29 13:39 ` Lisovskiy, Stanislav 3 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-04-06 14:19 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) URL : https://patchwork.freedesktop.org/series/58875/ State : success == Summary == CI Bug Log - changes from CI_DRM_5881_full -> IGTPW_2802_full ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/58875/revisions/2/mbox/ Known issues ------------ Here are the changes found in IGTPW_2802_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_busy@extended-parallel-bsd1: - shard-hsw: NOTRUN -> SKIP [fdo#109271] +29 * igt@gem_create@create-clear: - shard-snb: PASS -> INCOMPLETE [fdo#105411] * igt@gem_ctx_isolation@vecs0-s3: - shard-kbl: PASS -> INCOMPLETE [fdo#103665] * igt@gem_exec_store@cachelines-bsd1: - shard-snb: NOTRUN -> SKIP [fdo#109271] +79 * igt@gem_stolen@stolen-clear: - shard-glk: NOTRUN -> SKIP [fdo#109271] +10 * igt@kms_busy@basic-flip-e: - shard-glk: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] * igt@kms_busy@extended-modeset-hang-newfb-render-c: - shard-hsw: PASS -> DMESG-WARN [fdo#110222] +2 * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: - shard-kbl: NOTRUN -> DMESG-WARN [fdo#110222] * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b: - shard-snb: PASS -> DMESG-WARN [fdo#110222] - shard-apl: NOTRUN -> DMESG-WARN [fdo#110222] * igt@kms_busy@extended-modeset-hang-oldfb-render-f: - shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3 * igt@kms_busy@extended-pageflip-hang-newfb-render-c: - shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6 * igt@kms_ccs@pipe-c-missing-ccs-buffer: - shard-apl: NOTRUN -> SKIP [fdo#109271] +35 * igt@kms_flip@flip-vs-suspend: - shard-snb: PASS -> DMESG-WARN [fdo#102365] * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt: - shard-kbl: NOTRUN -> SKIP [fdo#109271] +40 * igt@kms_lease@atomic_implicit_crtc: - shard-apl: NOTRUN -> FAIL [fdo#110279] * igt@kms_pipe_crc_basic@read-crc-pipe-e: - shard-apl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2 * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb: - shard-kbl: NOTRUN -> FAIL [fdo#108145] * igt@kms_plane_alpha_blend@pipe-b-alpha-basic: - shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590] * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb: - shard-hsw: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2 * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom: - shard-kbl: PASS -> DMESG-FAIL [fdo#105763] * igt@testdisplay: - shard-apl: PASS -> INCOMPLETE [fdo#103927] #### Possible fixes #### * igt@i915_pm_rc6_residency@rc6-accuracy: - shard-snb: SKIP [fdo#109271] -> PASS * igt@kms_busy@extended-modeset-hang-newfb-render-b: - shard-hsw: DMESG-WARN [fdo#110222] -> PASS +1 - shard-snb: DMESG-WARN [fdo#110222] -> PASS * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic: - shard-glk: FAIL [fdo#106509] / [fdo#107409] -> PASS * igt@kms_flip@flip-vs-suspend: - shard-kbl: DMESG-WARN [fdo#108566] -> PASS * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes: - shard-kbl: INCOMPLETE [fdo#103665] -> PASS * igt@kms_plane_scaling@pipe-c-scaler-with-rotation: - shard-glk: SKIP [fdo#109271] / [fdo#109278] -> PASS * igt@kms_rotation_crc@multiplane-rotation-cropping-top: - shard-kbl: FAIL [fdo#109016] -> PASS * igt@kms_setmode@basic: - shard-kbl: FAIL [fdo#99912] -> PASS * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm: - shard-apl: FAIL [fdo#104894] -> PASS +1 [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365 [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894 [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411 [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763 [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509 [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566 [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590 [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222 [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279 [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 Participating hosts (10 -> 5) ------------------------------ Missing (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u Build changes ------------- * IGT: IGT_4931 -> IGTPW_2802 * Piglit: piglit_4509 -> None CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2802: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2802/ IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2802/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-05 14:55 [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Juha-Pekka Heikkila 2019-04-05 16:16 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) Patchwork 2019-04-06 14:19 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork @ 2019-04-09 1:29 ` Souza, Jose 2019-04-09 11:47 ` Juha-Pekka Heikkila 2019-04-09 12:47 ` Lisovskiy, Stanislav 2019-05-29 13:39 ` Lisovskiy, Stanislav 3 siblings, 2 replies; 13+ messages in thread From: Souza, Jose @ 2019-04-09 1:29 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org Cc: Lisovskiy, Stanislav [-- Attachment #1.1: Type: text/plain, Size: 6585 bytes --] 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@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. > + 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. > + > + 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() > + } 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++; [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-09 1:29 ` [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Souza, Jose @ 2019-04-09 11:47 ` Juha-Pekka Heikkila 2019-04-10 20:06 ` Souza, Jose 2019-04-11 8:35 ` Maarten Lankhorst 2019-04-09 12:47 ` Lisovskiy, Stanislav 1 sibling, 2 replies; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-04-09 11:47 UTC (permalink / raw) To: Souza, Jose, igt-dev@lists.freedesktop.org; +Cc: Lisovskiy, Stanislav 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@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++; _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-09 11:47 ` Juha-Pekka Heikkila @ 2019-04-10 20:06 ` Souza, Jose 2019-04-11 15:36 ` Juha-Pekka Heikkila 2019-04-11 8:35 ` Maarten Lankhorst 1 sibling, 1 reply; 13+ messages in thread From: Souza, Jose @ 2019-04-10 20:06 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org Cc: Lisovskiy, Stanislav [-- Attachment #1.1: Type: text/plain, Size: 8737 bytes --] On Tue, 2019-04-09 at 14:47 +0300, Juha-Pekka Heikkila wrote: > 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@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. Platforms with 3 planes will do 3 iterations with current approach while it could be done with just one and ICL will will do 5 iterations(my ICL setup support 5 planes at the same time) while it could be done in 3 iterations. Now takes this additional iterations * 4 tests per pipe * number of pipes and you have a considerable waste of CI 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? -ENOMEM is one example that I have without looking at the libdrm and kernel code. > > > > + > > > + 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? Yeah, I checked again and you are not leaking, sorry about that. > > > > + } 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++; [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-10 20:06 ` Souza, Jose @ 2019-04-11 15:36 ` Juha-Pekka Heikkila 0 siblings, 0 replies; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-04-11 15:36 UTC (permalink / raw) To: Souza, Jose, igt-dev@lists.freedesktop.org; +Cc: Lisovskiy, Stanislav On 10.4.2019 23.06, Souza, Jose wrote: > On Tue, 2019-04-09 at 14:47 +0300, Juha-Pekka Heikkila wrote: >> 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@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. > > Platforms with 3 planes will do 3 iterations with current approach > while it could be done with just one and ICL will will do 5 > iterations(my ICL setup support 5 planes at the same time) while it > could be done in 3 iterations. > > Now takes this additional iterations * 4 tests per pipe * number of > pipes and you have a considerable waste of CI time. I still don't think it to be 'considerable waste of time' when the difference is small enough to be lost in noise. On my box with this test execution time fluctuate for one subtest between 0.561s .. 1.671s as the test report itself. Though, I admit I was wrong about how long this loop takes, its just one frame, not over five frames. TBH if one actually was to care about time spent I'd go after setting those fbs to null but this patch is not about that. Then again I don't even understand why are you bothering about possible 0.09s lost in CI? > > >> >>>> + 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? > > -ENOMEM is one example that I have without looking at the libdrm and > kernel code. > You do realize we'd be wasting time on checking that? Anyway if you're into wasting time and consider we got this far and suddenly get any random error, still what's the point in why should handling this error be included here? Wouldn't it give us failure on the actual loop anyway and fail this test? Why make it intentionally more complex? >> >>>> + >>>> + 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? > > Yeah, I checked again and you are not leaking, sorry about that. > >> >>>> + } 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++; _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-09 11:47 ` Juha-Pekka Heikkila 2019-04-10 20:06 ` Souza, Jose @ 2019-04-11 8:35 ` Maarten Lankhorst 1 sibling, 0 replies; 13+ messages in thread From: Maarten Lankhorst @ 2019-04-11 8:35 UTC (permalink / raw) To: juhapekka.heikkila, Souza, Jose, igt-dev@lists.freedesktop.org Cc: Lisovskiy, Stanislav 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@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 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-09 1:29 ` [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Souza, Jose 2019-04-09 11:47 ` Juha-Pekka Heikkila @ 2019-04-09 12:47 ` Lisovskiy, Stanislav 2019-04-10 19:55 ` Souza, Jose 1 sibling, 1 reply; 13+ messages in thread From: Lisovskiy, Stanislav @ 2019-04-09 12:47 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org, Souza, Jose On Mon, 2019-04-08 at 18:29 -0700, 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@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. It is actually opposite. Platforms with high amount of planes would be slower obviously, as you will have to increment more and faster for systems with less amount of planes. Consider 0->7 and 0- >3(platforms with less amount of planes) - so where is more waste of 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: This is igt_display_TRY_commit2. > > 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. > > > + > > + 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 There should be no leak here. He uses suffle[i] for index indirection in prepare_planes like igt_plane_t *plane = igt_output_get_plane(output, suffle[i]), so that suffle[0]=0, suffle[1]=1, suffle[2]=3, meanwhile correspondent data->fb[i] remains contiguous, so that plane 0 corresponds to data->fb[0], plane 1 -> data->fb[1] and plane 3 -> data->fb[2]. So all the igt_remove_fb calls target correct data->fb[] through suffle[i] used as indirection layer. > > Other leak, the framebuffer created in test_grab_crc() > > > > + } 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++; _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-09 12:47 ` Lisovskiy, Stanislav @ 2019-04-10 19:55 ` Souza, Jose 2019-04-11 6:38 ` Lisovskiy, Stanislav 0 siblings, 1 reply; 13+ messages in thread From: Souza, Jose @ 2019-04-10 19:55 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org, Lisovskiy, Stanislav [-- Attachment #1.1: Type: text/plain, Size: 9107 bytes --] On Tue, 2019-04-09 at 13:47 +0100, Lisovskiy, Stanislav wrote: > On Mon, 2019-04-08 at 18:29 -0700, 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@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. > > It is actually opposite. Platforms with high amount of planes would > be slower obviously, as you will have to increment more and faster > for systems with less amount of planes. Consider 0->7 and 0- > > 3(platforms with less amount of planes) - so where is more waste > of time? Platforms with 3 planes will do 3 iterations with current approach while it could be done with just one and ICL will will do 5 iterations(my ICL setup support 5 planes at the same time) while it could be done in 3 iterations. Now takes this additional iterations * 4 tests per pipe * number of pipes and you have a considerable waste of CI 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: > > This is igt_display_TRY_commit2. The TRY here means that the test will not fail in case the commit is not accepted/valid, it will not call drmModeAtomicCommit() with DRM_MODE_ATOMIC_TEST_ONLY, this will save some time when the commit is valid/accepted. > > > 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. > > > > > + > > > + 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 > > There should be no leak here. He uses suffle[i] > for index indirection in prepare_planes like igt_plane_t *plane = > igt_output_get_plane(output, suffle[i]), so that suffle[0]=0, > suffle[1]=1, suffle[2]=3, meanwhile correspondent data->fb[i] > remains contiguous, so that plane 0 corresponds to data->fb[0], > plane 1 -> data->fb[1] and plane 3 -> data->fb[2]. > So all the igt_remove_fb calls target correct data->fb[] through > suffle[i] used as indirection layer. Okay > > > Other leak, the framebuffer created in test_grab_crc() > > > > > > > + } 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++; [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-10 19:55 ` Souza, Jose @ 2019-04-11 6:38 ` Lisovskiy, Stanislav 0 siblings, 0 replies; 13+ messages in thread From: Lisovskiy, Stanislav @ 2019-04-11 6:38 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org, Souza, Jose On Wed, 2019-04-10 at 12:55 -0700, Souza, Jose wrote: > On Tue, 2019-04-09 at 13:47 +0100, Lisovskiy, Stanislav wrote: > > On Mon, 2019-04-08 at 18:29 -0700, 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@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. > > > > It is actually opposite. Platforms with high amount of planes would > > be slower obviously, as you will have to increment more and faster > > for systems with less amount of planes. Consider 0->7 and 0- > > > 3(platforms with less amount of planes) - so where is more waste > > > > of time? > > Platforms with 3 planes will do 3 iterations with current approach > while it could be done with just one and ICL will will do 5 > iterations(my ICL setup support 5 planes at the same time) while it > could be done in 3 iterations. > > Now takes this additional iterations * 4 tests per pipe * number of > pipes and you have a considerable waste of CI time. That is exactly what I said. "Platforms with high amount of planes would be slower obviously". However in reality I'm currently testing it with bandwidth patches: the number of allowed planes can be quite different, based on tiling and other resources, so it is really hard to predict from which plane it is beneficial to start. For example: on my ICL setup it can sometimes tolerate 8(yeah) planes on one pipe, however if second pipe is used simultaneously, it is allowed to have only 1(!) plane. > > > > > > > + 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: > > > > This is igt_display_TRY_commit2. > > The TRY here means that the test will not fail in case the commit is > not accepted/valid, it will not call drmModeAtomicCommit() with > DRM_MODE_ATOMIC_TEST_ONLY, this will save some time when the commit > is > valid/accepted. I agree here, naming confused me - there is a try commit2 function with quite similar name.. > > > > > > 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. > > > > > > > + > > > > + 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 > > > > There should be no leak here. He uses suffle[i] > > for index indirection in prepare_planes like igt_plane_t *plane = > > igt_output_get_plane(output, suffle[i]), so that suffle[0]=0, > > suffle[1]=1, suffle[2]=3, meanwhile correspondent data->fb[i] > > remains contiguous, so that plane 0 corresponds to data->fb[0], > > plane 1 -> data->fb[1] and plane 3 -> data->fb[2]. > > So all the igt_remove_fb calls target correct data->fb[] through > > suffle[i] used as indirection layer. > > Okay > > > > > > Other leak, the framebuffer created in test_grab_crc() > > > > > > > > > > + } 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++; _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-04-05 14:55 [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Juha-Pekka Heikkila ` (2 preceding siblings ...) 2019-04-09 1:29 ` [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Souza, Jose @ 2019-05-29 13:39 ` Lisovskiy, Stanislav 2019-06-04 10:26 ` Kahola, Mika 3 siblings, 1 reply; 13+ messages in thread From: Lisovskiy, Stanislav @ 2019-05-29 13:39 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org On Fri, 2019-04-05 at 17:55 +0300, Juha-Pekka Heikkila wrote: With current failures we need this fixed. Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > 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@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++) { > + 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++; > + prepare_planes(data, pipe, &blue, tiling, c, output); > + err = igt_display_try_commit2(&data->display, > COMMIT_ATOMIC); > + > + 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]); > + } 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]); > + > igt_assert_crc_equal(&data->ref_crc, &crc); > > i++; _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed 2019-05-29 13:39 ` Lisovskiy, Stanislav @ 2019-06-04 10:26 ` Kahola, Mika 0 siblings, 0 replies; 13+ messages in thread From: Kahola, Mika @ 2019-06-04 10:26 UTC (permalink / raw) To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org, Lisovskiy, Stanislav On Wed, 2019-05-29 at 13:39 +0000, Lisovskiy, Stanislav wrote: > On Fri, 2019-04-05 at 17:55 +0300, Juha-Pekka Heikkila wrote: > > > With current failures we need this fixed. > > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Patch is now merged to IGT. Thanks for the patch and a review! Cheers, Mika > > > 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@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++) { > > + 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++; > > + prepare_planes(data, pipe, &blue, tiling, c, output); > > + err = igt_display_try_commit2(&data->display, > > COMMIT_ATOMIC); > > + > > + 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]); > > + } 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]); > > + > > igt_assert_crc_equal(&data->ref_crc, &crc); > > > > i++; > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-04 10:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-05 14:55 [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Juha-Pekka Heikkila 2019-04-05 16:16 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_multiple: Before going testing check how many planes are allowed (rev2) Patchwork 2019-04-06 14:19 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2019-04-09 1:29 ` [igt-dev] [PATCH i-g-t] tests/kms_plane_multiple: before going testing check how many planes are allowed Souza, Jose 2019-04-09 11:47 ` Juha-Pekka Heikkila 2019-04-10 20:06 ` Souza, Jose 2019-04-11 15:36 ` Juha-Pekka Heikkila 2019-04-11 8:35 ` Maarten Lankhorst 2019-04-09 12:47 ` Lisovskiy, Stanislav 2019-04-10 19:55 ` Souza, Jose 2019-04-11 6:38 ` Lisovskiy, Stanislav 2019-05-29 13:39 ` Lisovskiy, Stanislav 2019-06-04 10:26 ` Kahola, Mika
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox