2015-10-23 9:42 GMT-02:00 David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>:
> Some tests should not be run by default, due to their slow,
> and sometimes superfluous, nature.
>
> We still want to be able to run these tests though in some cases.
> Until now there's been no unified way of handling this. Remedy
> this by introducing the --with-slow-combinatorial option to
> igt_core, and use it in gem_concurrent_blit & kms_frontbuffer_tracking.
> ---
> lib/igt_core.c | 19 ++++++
> lib/igt_core.h | 1 +
> tests/gem_concurrent_blit.c | 40 ++++++++----
> tests/kms_frontbuffer_tracking.c | 135 +++++++++++++++++++++++++++------------
> 4 files changed, 142 insertions(+), 53 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 59127cafe606..ba40ce0e0ead 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -216,6 +216,7 @@ const char *igt_interactive_debug;
>
> /* subtests helpers */
> static bool list_subtests = false;
> +static bool with_slow_combinatorial = false;
> static char *run_single_subtest = NULL;
> static bool run_single_subtest_found = false;
> static const char *in_subtest = NULL;
> @@ -235,6 +236,7 @@ bool test_child;
>
> enum {
> OPT_LIST_SUBTESTS,
> + OPT_WITH_SLOW_COMBINATORIAL,
> OPT_RUN_SUBTEST,
> OPT_DESCRIPTION,
> OPT_DEBUG,
> @@ -478,6 +480,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>
> fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
> fprintf(f, " --list-subtests\n"
> + " --with-slow-combinatorial\n"
> " --run-subtest <pattern>\n"
> " --debug[=log-domain]\n"
> " --interactive-debug[=domain]\n"
> @@ -510,6 +513,7 @@ static int common_init(int *argc, char **argv,
> int c, option_index = 0, i, x;
> static struct option long_options[] = {
> {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> + {"with-slow-combinatorial", 0, 0, OPT_WITH_SLOW_COMBINATORIAL},
> {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> {"help-description", 0, 0, OPT_DESCRIPTION},
> {"debug", optional_argument, 0, OPT_DEBUG},
> @@ -617,6 +621,10 @@ static int common_init(int *argc, char **argv,
> if (!run_single_subtest)
> list_subtests = true;
> break;
> + case OPT_WITH_SLOW_COMBINATORIAL:
> + if (!run_single_subtest)
> + with_slow_combinatorial = true;
> + break;
> case OPT_RUN_SUBTEST:
> if (!list_subtests)
> run_single_subtest = strdup(optarg);
> @@ -1629,6 +1637,17 @@ void igt_skip_on_simulation(void)
> igt_require(!igt_run_in_simulation());
> }
>
> +/**
> + * igt_slow_combinatorial:
> + *
> + * This is used to define subtests that should only be listed/run
> + * when the "--with-slow-combinatorial" has been specified
> + */
> +void igt_slow_combinatorial(void)
> +{
> + igt_skip_on(!with_slow_combinatorial);
> +}
> +
> /* structured logging */
>
> /**
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 5ae09653fd55..6ddf25563275 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -680,6 +680,7 @@ bool igt_run_in_simulation(void);
> #define SLOW_QUICK(slow,quick) (igt_run_in_simulation() ? (quick) : (slow))
>
> void igt_skip_on_simulation(void);
> +void igt_slow_combinatorial(void);
>
> extern const char *igt_interactive_debug;
>
> diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c
> index 1d2d787202df..311b6829e984 100644
> --- a/tests/gem_concurrent_blit.c
> +++ b/tests/gem_concurrent_blit.c
> @@ -931,9 +931,6 @@ run_basic_modes(const struct access_mode *mode,
> struct buffers buffers;
>
> for (h = hangs; h->suffix; h++) {
> - if (!all && *h->suffix)
> - continue;
> -
> for (p = all ? pipelines : pskip; p->prefix; p++) {
> igt_fixture {
> batch = buffers_init(&buffers, mode, fd);
> @@ -941,6 +938,8 @@ run_basic_modes(const struct access_mode *mode,
>
> /* try to overwrite the source values */
> igt_subtest_f("%s-%s-overwrite-source-one%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -950,6 +949,8 @@ run_basic_modes(const struct access_mode *mode,
> }
>
> igt_subtest_f("%s-%s-overwrite-source%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -959,6 +960,8 @@ run_basic_modes(const struct access_mode *mode,
> }
>
> igt_subtest_f("%s-%s-overwrite-source-read-bcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -968,6 +971,8 @@ run_basic_modes(const struct access_mode *mode,
> }
>
> igt_subtest_f("%s-%s-overwrite-source-read-rcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> igt_require(rendercopy);
> @@ -978,6 +983,8 @@ run_basic_modes(const struct access_mode *mode,
> }
>
> igt_subtest_f("%s-%s-overwrite-source-rev%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -988,6 +995,8 @@ run_basic_modes(const struct access_mode *mode,
>
> /* try to intermix copies with GPU copies*/
> igt_subtest_f("%s-%s-intermix-rcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> igt_require(rendercopy);
> @@ -997,6 +1006,8 @@ run_basic_modes(const struct access_mode *mode,
> p->copy, h->hang);
> }
> igt_subtest_f("%s-%s-intermix-bcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> igt_require(rendercopy);
> @@ -1006,6 +1017,8 @@ run_basic_modes(const struct access_mode *mode,
> p->copy, h->hang);
> }
> igt_subtest_f("%s-%s-intermix-both%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> igt_require(rendercopy);
> @@ -1017,6 +1030,8 @@ run_basic_modes(const struct access_mode *mode,
>
> /* try to read the results before the copy completes */
> igt_subtest_f("%s-%s-early-read%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -1027,6 +1042,8 @@ run_basic_modes(const struct access_mode *mode,
>
> /* concurrent reads */
> igt_subtest_f("%s-%s-read-read-bcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -1035,6 +1052,8 @@ run_basic_modes(const struct access_mode *mode,
> p->copy, h->hang);
> }
> igt_subtest_f("%s-%s-read-read-rcs%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> igt_require(rendercopy);
> @@ -1046,6 +1065,8 @@ run_basic_modes(const struct access_mode *mode,
>
> /* and finally try to trick the kernel into loosing the pending write */
> igt_subtest_f("%s-%s-gpu-read-after-write%s%s", mode->name, p->prefix, suffix, h->suffix) {
> + if (*h->suffix)
> + igt_slow_combinatorial();
> h->require();
> p->require();
> buffers_create(&buffers, num_buffers);
> @@ -1064,13 +1085,11 @@ run_basic_modes(const struct access_mode *mode,
> static void
> run_modes(const struct access_mode *mode)
> {
> - if (all) {
> - run_basic_modes(mode, "", run_single);
> + run_basic_modes(mode, "", run_single);
>
> - igt_fork_signal_helper();
> - run_basic_modes(mode, "-interruptible", run_interruptible);
> - igt_stop_signal_helper();
> - }
> + igt_fork_signal_helper();
> + run_basic_modes(mode, "-interruptible", run_interruptible);
> + igt_stop_signal_helper();
>
> igt_fork_signal_helper();
> run_basic_modes(mode, "-forked", run_forked);
> @@ -1083,9 +1102,6 @@ igt_main
>
> igt_skip_on_simulation();
>
> - if (strstr(igt_test_name(), "all"))
> - all = true;
> -
> igt_fixture {
> fd = drm_open_driver(DRIVER_INTEL);
> devid = intel_get_drm_devid(fd);
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index d97e148c5073..6f84ef0813d9 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -47,8 +47,8 @@ IGT_TEST_DESCRIPTION("Test the Kernel's frontbuffer tracking mechanism and "
> * combinations that are somewhat redundant and don't add much value to the
> * test. For example, since we already do the offscreen testing with a single
> * pipe enabled, there's no much value in doing it again with dual pipes. If you
> - * still want to try these redundant tests, you need to use the --show-hidden
> - * option.
> + * still want to try these redundant tests, you need to use the
> + * "--with-slow-combinatorial" option.
> *
> * The most important hidden thing is the FEATURE_NONE set of tests. Whenever
> * you get a failure on any test, it is important to check whether the same test
> @@ -116,6 +116,10 @@ struct test_mode {
> } format;
>
> enum igt_draw_method method;
> +
> + /* The test is slow and/or combinatorial;
> + * skip unless otherwise specified */
> + bool slow;
> };
>
> enum flip_type {
> @@ -237,7 +241,6 @@ struct {
> bool fbc_check_last_action;
> bool no_edp;
> bool small_modes;
> - bool show_hidden;
It's not clear to me, please clarify: now the tests that were
previously completely hidden will be listed in --list-subtests and
will be shown as skipped during normal runs?
For kms_frontbuffer_tracking, hidden tests are supposed to be just for
developers who know what they are doing. I hide them behind a special
command-line switch that's not used by QA because I don't want QA
wasting time running those tests. One third of the
kms_frontbuffer_tracking hidden tests only serve the purpose of
checking whether there's a bug in kms_frontbuffer_track itself or not.
For some other hidden tests, they are there just to help better debug
in case some other non-hidden tests fail. Some other hidden tests are
100% useless and superfulous.QA should only run the non-hidden tests.
So if some non-hidden test fails, the developers can use the hidden
tests to help debugging.
Besides, the "if (t.slow)" could have been moved to
check_test_requirements(), making the code much simpler :)
> int step;
> int only_feature;
> int only_pipes;
> @@ -250,7 +253,6 @@ struct {
> .fbc_check_last_action = true,
> .no_edp = false,
> .small_modes = false,
> - .show_hidden= false,
> .step = 0,
> .only_feature = FEATURE_COUNT,
> .only_pipes = PIPE_COUNT,
> @@ -2892,9 +2894,6 @@ static int opt_handler(int option, int option_index, void *data)
> case 'm':
> opt.small_modes = true;
> break;
> - case 'i':
> - opt.show_hidden = true;
> - break;
> case 't':
> opt.step++;
> break;
> @@ -2942,7 +2941,6 @@ const char *help_str =
> " --no-fbc-action-check Don't check for the FBC last action\n"
> " --no-edp Don't use eDP monitors\n"
> " --use-small-modes Use smaller resolutions for the modes\n"
> -" --show-hidden Show hidden subtests\n"
> " --step Stop on each step so you can check the screen\n"
> " --nop-only Only run the \"nop\" feature subtests\n"
> " --fbc-only Only run the \"fbc\" feature subtests\n"
> @@ -3036,6 +3034,7 @@ static const char *format_str(enum pixel_format format)
>
> #define TEST_MODE_ITER_BEGIN(t) \
> t.format = FORMAT_DEFAULT; \
> + t.slow = false; \
> for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) { \
> for (t.pipes = 0; t.pipes < PIPE_COUNT; t.pipes++) { \
> for (t.screen = 0; t.screen < SCREEN_COUNT; t.screen++) { \
> @@ -3046,15 +3045,15 @@ static const char *format_str(enum pixel_format format)
> continue; \
> if (t.screen == SCREEN_OFFSCREEN && t.plane != PLANE_PRI) \
> continue; \
> - if (!opt.show_hidden && t.pipes == PIPE_DUAL && \
> + if (t.pipes == PIPE_DUAL && \
> t.screen == SCREEN_OFFSCREEN) \
> - continue; \
> - if ((!opt.show_hidden && opt.only_feature != FEATURE_NONE) \
> - && t.feature == FEATURE_NONE) \
> - continue; \
> - if (!opt.show_hidden && t.fbs == FBS_SHARED && \
> + t.slow = true; \
> + if (opt.only_feature != FEATURE_NONE && \
> + t.feature == FEATURE_NONE) \
> + t.slow = true; \
> + if (t.fbs == FBS_SHARED && \
> (t.plane == PLANE_CUR || t.plane == PLANE_SPR)) \
> - continue;
> + t.slow = true;
>
>
> #define TEST_MODE_ITER_END } } } } } }
> @@ -3069,7 +3068,6 @@ int main(int argc, char *argv[])
> { "no-fbc-action-check", 0, 0, 'a'},
> { "no-edp", 0, 0, 'e'},
> { "use-small-modes", 0, 0, 'm'},
> - { "show-hidden", 0, 0, 'i'},
> { "step", 0, 0, 't'},
> { "nop-only", 0, 0, 'n'},
> { "fbc-only", 0, 0, 'f'},
> @@ -3088,9 +3086,11 @@ int main(int argc, char *argv[])
> setup_environment();
>
> for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) {
> - if ((!opt.show_hidden && opt.only_feature != FEATURE_NONE)
> - && t.feature == FEATURE_NONE)
> - continue;
> + bool slow = false;
> +
> + if (opt.only_feature != FEATURE_NONE &&
> + t.feature == FEATURE_NONE)
> + slow = true;
> for (t.pipes = 0; t.pipes < PIPE_COUNT; t.pipes++) {
> t.screen = SCREEN_PRIM;
> t.plane = PLANE_PRI;
> @@ -3101,8 +3101,11 @@ int main(int argc, char *argv[])
>
> igt_subtest_f("%s-%s-rte",
> feature_str(t.feature),
> - pipes_str(t.pipes))
> + pipes_str(t.pipes)) {
> + if (slow)
> + igt_slow_combinatorial();
> rte_subtest(&t);
> + }
> }
> }
>
> @@ -3113,39 +3116,52 @@ int main(int argc, char *argv[])
> screen_str(t.screen),
> plane_str(t.plane),
> fbs_str(t.fbs),
> - igt_draw_get_method_name(t.method))
> + igt_draw_get_method_name(t.method)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> draw_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> if (t.plane != PLANE_PRI ||
> - t.screen == SCREEN_OFFSCREEN ||
> - (!opt.show_hidden && t.method != IGT_DRAW_BLT))
> + t.screen == SCREEN_OFFSCREEN)
> continue;
> + if (t.method != IGT_DRAW_BLT)
> + t.slow = true;
>
> igt_subtest_f("%s-%s-%s-%s-flip-%s",
> feature_str(t.feature),
> pipes_str(t.pipes),
> screen_str(t.screen),
> fbs_str(t.fbs),
> - igt_draw_get_method_name(t.method))
> + igt_draw_get_method_name(t.method)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> flip_subtest(&t, FLIP_PAGEFLIP);
> + }
>
> igt_subtest_f("%s-%s-%s-%s-evflip-%s",
> feature_str(t.feature),
> pipes_str(t.pipes),
> screen_str(t.screen),
> fbs_str(t.fbs),
> - igt_draw_get_method_name(t.method))
> + igt_draw_get_method_name(t.method)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> flip_subtest(&t, FLIP_PAGEFLIP_EVENT);
> + }
>
> igt_subtest_f("%s-%s-%s-%s-msflip-%s",
> feature_str(t.feature),
> pipes_str(t.pipes),
> screen_str(t.screen),
> fbs_str(t.fbs),
> - igt_draw_get_method_name(t.method))
> + igt_draw_get_method_name(t.method)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> flip_subtest(&t, FLIP_MODESET);
> + }
>
> TEST_MODE_ITER_END
>
> @@ -3159,8 +3175,11 @@ int main(int argc, char *argv[])
> igt_subtest_f("%s-%s-%s-fliptrack",
> feature_str(t.feature),
> pipes_str(t.pipes),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> fliptrack_subtest(&t, FLIP_PAGEFLIP);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> @@ -3174,16 +3193,22 @@ int main(int argc, char *argv[])
> pipes_str(t.pipes),
> screen_str(t.screen),
> plane_str(t.plane),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> move_subtest(&t);
> + }
>
> igt_subtest_f("%s-%s-%s-%s-%s-onoff",
> feature_str(t.feature),
> pipes_str(t.pipes),
> screen_str(t.screen),
> plane_str(t.plane),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> onoff_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> @@ -3197,23 +3222,30 @@ int main(int argc, char *argv[])
> pipes_str(t.pipes),
> screen_str(t.screen),
> plane_str(t.plane),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> fullscreen_plane_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> if (t.screen != SCREEN_PRIM ||
> - t.method != IGT_DRAW_BLT ||
> - (!opt.show_hidden && t.plane != PLANE_PRI) ||
> - (!opt.show_hidden && t.fbs != FBS_INDIVIDUAL))
> + t.method != IGT_DRAW_BLT)
> continue;
> + if (t.plane != PLANE_PRI ||
> + t.fbs != FBS_INDIVIDUAL)
> + t.slow = true;
>
> igt_subtest_f("%s-%s-%s-%s-multidraw",
> feature_str(t.feature),
> pipes_str(t.pipes),
> plane_str(t.plane),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> multidraw_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> @@ -3224,8 +3256,11 @@ int main(int argc, char *argv[])
> t.method != IGT_DRAW_MMAP_GTT)
> continue;
>
> - igt_subtest_f("%s-farfromfence", feature_str(t.feature))
> + igt_subtest_f("%s-farfromfence", feature_str(t.feature)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> farfromfence_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> @@ -3243,8 +3278,11 @@ int main(int argc, char *argv[])
> igt_subtest_f("%s-%s-draw-%s",
> feature_str(t.feature),
> format_str(t.format),
> - igt_draw_get_method_name(t.method))
> + igt_draw_get_method_name(t.method)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> format_draw_subtest(&t);
> + }
> }
> TEST_MODE_ITER_END
>
> @@ -3256,8 +3294,11 @@ int main(int argc, char *argv[])
> continue;
> igt_subtest_f("%s-%s-scaledprimary",
> feature_str(t.feature),
> - fbs_str(t.fbs))
> + fbs_str(t.fbs)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> scaledprimary_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> TEST_MODE_ITER_BEGIN(t)
> @@ -3268,19 +3309,31 @@ int main(int argc, char *argv[])
> t.method != IGT_DRAW_MMAP_CPU)
> continue;
>
> - igt_subtest_f("%s-modesetfrombusy", feature_str(t.feature))
> + igt_subtest_f("%s-modesetfrombusy", feature_str(t.feature)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> modesetfrombusy_subtest(&t);
> + }
>
> if (t.feature & FEATURE_FBC)
> - igt_subtest_f("%s-badstride", feature_str(t.feature))
> + igt_subtest_f("%s-badstride", feature_str(t.feature)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> badstride_subtest(&t);
> + }
>
> if (t.feature & FEATURE_PSR)
> - igt_subtest_f("%s-slowdraw", feature_str(t.feature))
> + igt_subtest_f("%s-slowdraw", feature_str(t.feature)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> slow_draw_subtest(&t);
> + }
>
> - igt_subtest_f("%s-suspend", feature_str(t.feature))
> + igt_subtest_f("%s-suspend", feature_str(t.feature)) {
> + if (t.slow)
> + igt_slow_combinatorial();
> suspend_subtest(&t);
> + }
> TEST_MODE_ITER_END
>
> igt_fixture
> --
> 2.6.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
