[igt-dev] [PATCH i-g-t v3 0/2] tests/kms_flip: Binary mode optimizations

[igt-dev] [PATCH i-g-t v3 0/2] tests/kms_flip: Binary mode optimizations

[Mika Kahola]

kms_flip in binary mode takes considerable amount of time to complete.
These couple of patches introduces optimizations to the test. The first
patch sets the subtest duration to be a configurable command line parameter.
The second patch suggests the change in execution order so that all basic
subtests are executed first and after that move on to execute 2X tests.

The binary mode execution time on GLK platform reduced from 1189 seconds
down to 536 seconds (2X tests skipped as test platform had only 1 display
connected)

For VIZ-14324

v2: Execute subtest once by default
v3: For jitter copmutation, increase default timeout for subtests to
    3 seconds (Ville)

Mika Kahola (2):
  tests/kms_flip: Set duration for subtest from command line
  tests/kms_flip: Change 2x tests execution order

 tests/kms_flip.c | 131 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 86 insertions(+), 45 deletions(-)

-- 
2.7.4

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Mika Kahola]

To reduce the execution time of kms_flip test on CI, let's move subtest
duration parameter as command line option. The default subtest duration
is 3 seconds for test that require jitter computation and for the rest
of the subtests are run only once.

v2: Run each subtest only once (default action)
v3: Reduces default timeout for tests that require jitter computation (Ville)

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 tests/kms_flip.c | 108 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 33 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 393d690..4cd1951 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -82,6 +82,8 @@
 #define DRM_CAP_TIMESTAMP_MONOTONIC 6
 #endif
 
+#define MAX_DURATION            60
+
 drmModeRes *resources;
 int drm_fd;
 static drm_intel_bufmgr *bufmgr;
@@ -95,6 +97,13 @@ static drmModeConnector *last_connector;
 
 uint32_t *fb_ptr;
 
+/* Command line parameters. */
+struct {
+	int duration;
+} opt = {
+	.duration = 0,
+};
+
 struct type_name {
 	int type;
 	const char *name;
@@ -1521,56 +1530,90 @@ static void test_nonblocking_read(int in)
 	close(fd);
 }
 
+static int opt_handler(int option, int option_index, void *input)
+{
+	switch (option) {
+	case 'd':
+		opt.duration = strtol(optarg, NULL, 0);
+
+		if (opt.duration > MAX_DURATION) {
+			igt_debug("limiting test duration from %ds to %ds\n",
+				  opt.duration, MAX_DURATION);
+			opt.duration = MAX_DURATION;
+		}
+
+		if (opt.duration < 0) {
+			igt_debug("limiting test duration from %ds to %ds\n",
+				  opt.duration, 0);
+			opt.duration = 0;
+		}
+		break;
+	default:
+		igt_assert(false);
+	}
+
+	return 0;
+}
+
+const char *help_str =
+	"  --duration test duration in seconds (default 0s)\n";
+
 int main(int argc, char **argv)
 {
+	struct option long_options[] = {
+		{ "duration", required_argument, NULL, 'd'},
+		{ 0, 0, 0, 0 }
+	};
+
 	struct {
 		int duration;
 		int flags;
 		const char *name;
 	} tests[] = {
-		{ 30, TEST_VBLANK | TEST_CHECK_TS, "wf_vblank-ts-check" },
-		{ 30, TEST_VBLANK | TEST_VBLANK_BLOCK | TEST_CHECK_TS,
+		{ 3, TEST_VBLANK | TEST_CHECK_TS, "wf_vblank-ts-check" },
+		{ 3, TEST_VBLANK | TEST_VBLANK_BLOCK | TEST_CHECK_TS,
 					"blocking-wf_vblank" },
-		{ 30,  TEST_VBLANK | TEST_VBLANK_ABSOLUTE,
+		{ 3, TEST_VBLANK | TEST_VBLANK_ABSOLUTE,
 					"absolute-wf_vblank" },
-		{ 30,  TEST_VBLANK | TEST_VBLANK_BLOCK | TEST_VBLANK_ABSOLUTE,
+		{ 3, TEST_VBLANK | TEST_VBLANK_BLOCK | TEST_VBLANK_ABSOLUTE,
 					"blocking-absolute-wf_vblank" },
-		{ 10, TEST_FLIP | TEST_BASIC, "plain-flip" },
-		{ 30, TEST_FLIP | TEST_EBUSY , "busy-flip" },
-		{ 30, TEST_FLIP | TEST_FENCE_STRESS , "flip-vs-fences" },
-		{ 30, TEST_FLIP | TEST_CHECK_TS, "plain-flip-ts-check" },
-		{ 30, TEST_FLIP | TEST_CHECK_TS | TEST_FB_RECREATE,
+		{ 3, TEST_FLIP | TEST_BASIC, "plain-flip" },
+		{ 3, TEST_FLIP | TEST_EBUSY , "busy-flip" },
+		{ 3, TEST_FLIP | TEST_FENCE_STRESS , "flip-vs-fences" },
+		{ 3, TEST_FLIP | TEST_CHECK_TS, "plain-flip-ts-check" },
+		{ 3, TEST_FLIP | TEST_CHECK_TS | TEST_FB_RECREATE,
 			"plain-flip-fb-recreate" },
-		{ 30, TEST_FLIP | TEST_RMFB | TEST_MODESET , "flip-vs-rmfb" },
-		{ 20, TEST_FLIP | TEST_DPMS | TEST_EINVAL | TEST_BASIC, "flip-vs-dpms" },
-		{ 30,  TEST_FLIP | TEST_PAN, "flip-vs-panning" },
-		{ 20, TEST_FLIP | TEST_MODESET | TEST_EINVAL | TEST_BASIC, "flip-vs-modeset" },
-		{ 30,  TEST_FLIP | TEST_VBLANK_EXPIRED_SEQ,
+		{ 3, TEST_FLIP | TEST_RMFB | TEST_MODESET , "flip-vs-rmfb" },
+		{ 3, TEST_FLIP | TEST_DPMS | TEST_EINVAL | TEST_BASIC, "flip-vs-dpms" },
+		{ 3, TEST_FLIP | TEST_PAN, "flip-vs-panning" },
+		{ 3, TEST_FLIP | TEST_MODESET | TEST_EINVAL | TEST_BASIC, "flip-vs-modeset" },
+		{ 3, TEST_FLIP | TEST_VBLANK_EXPIRED_SEQ,
 					"flip-vs-expired-vblank" },
 
-		{ 30, TEST_FLIP | TEST_VBLANK | TEST_VBLANK_ABSOLUTE |
-		      TEST_CHECK_TS, "flip-vs-absolute-wf_vblank" },
-		{ 10, TEST_FLIP | TEST_VBLANK | TEST_CHECK_TS | TEST_BASIC,
+		{ 3, TEST_FLIP | TEST_VBLANK | TEST_VBLANK_ABSOLUTE |
+		  TEST_CHECK_TS, "flip-vs-absolute-wf_vblank" },
+		{ 3, TEST_FLIP | TEST_VBLANK | TEST_CHECK_TS | TEST_BASIC,
 					"flip-vs-wf_vblank" },
-		{ 30, TEST_FLIP | TEST_VBLANK | TEST_VBLANK_BLOCK |
-			TEST_CHECK_TS, "flip-vs-blocking-wf-vblank" },
-		{ 30, TEST_FLIP | TEST_MODESET | TEST_HANG | TEST_NOEVENT, "flip-vs-modeset-vs-hang" },
-		{ 30, TEST_FLIP | TEST_PAN | TEST_HANG, "flip-vs-panning-vs-hang" },
-		{ 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, "flip-vs-bad-tiling" },
+		{ 3, TEST_FLIP | TEST_VBLANK | TEST_VBLANK_BLOCK |
+		  TEST_CHECK_TS, "flip-vs-blocking-wf-vblank" },
+		{ 3, TEST_FLIP | TEST_MODESET | TEST_HANG | TEST_NOEVENT, "flip-vs-modeset-vs-hang" },
+		{ 3, TEST_FLIP | TEST_PAN | TEST_HANG, "flip-vs-panning-vs-hang" },
+		{ 3, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, "flip-vs-bad-tiling" },
 
-		{ 1, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP,
+		{ 0, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP,
 					"flip-vs-dpms-off-vs-modeset" },
-		{ 1, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP | TEST_SINGLE_BUFFER,
+		{ 0, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP | TEST_SINGLE_BUFFER,
 					"single-buffer-flip-vs-dpms-off-vs-modeset" },
-		{ 30, TEST_FLIP | TEST_NO_2X_OUTPUT | TEST_DPMS_OFF_OTHERS , "dpms-off-confusion" },
+		{ 0, TEST_FLIP | TEST_NO_2X_OUTPUT | TEST_DPMS_OFF_OTHERS , "dpms-off-confusion" },
 		{ 0, TEST_ENOENT | TEST_NOEVENT, "nonexisting-fb" },
-		{ 10, TEST_DPMS_OFF | TEST_DPMS | TEST_VBLANK_RACE, "dpms-vs-vblank-race" },
-		{ 10, TEST_MODESET | TEST_VBLANK_RACE, "modeset-vs-vblank-race" },
+		{ 3, TEST_DPMS_OFF | TEST_DPMS | TEST_VBLANK_RACE, "dpms-vs-vblank-race" },
+		{ 3, TEST_MODESET | TEST_VBLANK_RACE, "modeset-vs-vblank-race" },
 		{ 0, TEST_BO_TOOBIG | TEST_NO_2X_OUTPUT, "bo-too-big" },
 	};
 	int i;
 
-	igt_subtest_init(argc, argv);
+	igt_subtest_init_parse_opts(&argc, argv, "", long_options, help_str,
+				    opt_handler, NULL);
 
 	igt_fixture {
 		drm_fd = drm_open_driver_master(DRIVER_ANY);
@@ -1595,7 +1638,7 @@ int main(int argc, char **argv)
 		igt_subtest_f("%s%s",
 			      tests[i].flags & TEST_BASIC ? "basic-" : "",
 			      tests[i].name)
-			run_test(tests[i].duration, tests[i].flags);
+			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
 
 		if (tests[i].flags & TEST_NO_2X_OUTPUT)
 			continue;
@@ -1605,7 +1648,7 @@ int main(int argc, char **argv)
 			continue;
 
 		igt_subtest_f( "2x-%s", tests[i].name)
-			run_pair(tests[i].duration, tests[i].flags);
+			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
 	}
 
 	igt_fork_signal_helper();
@@ -1617,7 +1660,7 @@ int main(int argc, char **argv)
 			continue;
 
 		igt_subtest_f( "%s-interruptible", tests[i].name)
-			run_test(tests[i].duration, tests[i].flags);
+			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
 
 		if (tests[i].flags & TEST_NO_2X_OUTPUT)
 			continue;
@@ -1627,7 +1670,7 @@ int main(int argc, char **argv)
 			continue;
 
 		igt_subtest_f( "2x-%s-interruptible", tests[i].name)
-			run_pair(tests[i].duration, tests[i].flags);
+			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
 	}
 	igt_stop_signal_helper();
 
@@ -1635,6 +1678,5 @@ int main(int argc, char **argv)
 	 * Let drm_fd leak, since it's needed by the dpms restore
 	 * exit_handler and igt_exit() won't return.
 	 */
-
 	igt_exit();
 }
-- 
2.7.4

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 2/2] tests/kms_flip: Change 2x tests execution order

[Mika Kahola]

In order to optimize execution of kms_flip binary mode tests, let's change
the execution order so that 2x tests will be executed after basic tests.

v2: update commit message (Petri)
v3: Change subtest execution order

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 tests/kms_flip.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 4cd1951..6217d67 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1640,19 +1640,6 @@ int main(int argc, char **argv)
 			      tests[i].name)
 			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
 
-		if (tests[i].flags & TEST_NO_2X_OUTPUT)
-			continue;
-
-		/* code doesn't disable all crtcs, so skip rpm tests */
-		if (tests[i].flags & TEST_RPM)
-			continue;
-
-		igt_subtest_f( "2x-%s", tests[i].name)
-			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
-	}
-
-	igt_fork_signal_helper();
-	for (i = 0; i < sizeof(tests) / sizeof (tests[0]); i++) {
 		/* relative blocking vblank waits that get constantly interrupt
 		 * take forver. So don't do them. */
 		if ((tests[i].flags & TEST_VBLANK_BLOCK) &&
@@ -1661,7 +1648,10 @@ int main(int argc, char **argv)
 
 		igt_subtest_f( "%s-interruptible", tests[i].name)
 			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
+	}
 
+	igt_fork_signal_helper();
+	for (i = 0; i < sizeof(tests) / sizeof (tests[0]); i++) {
 		if (tests[i].flags & TEST_NO_2X_OUTPUT)
 			continue;
 
@@ -1669,6 +1659,15 @@ int main(int argc, char **argv)
 		if (tests[i].flags & TEST_RPM)
 			continue;
 
+		igt_subtest_f( "2x-%s", tests[i].name)
+			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
+
+		/* relative blocking vblank waits that get constantly interrupt
+		 * take forver. So don't do them. */
+		if ((tests[i].flags & TEST_VBLANK_BLOCK) &&
+		    !(tests[i].flags & TEST_VBLANK_ABSOLUTE))
+			continue;
+
 		igt_subtest_f( "2x-%s-interruptible", tests[i].name)
 			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
 	}
-- 
2.7.4

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Chris Wilson]

Quoting Mika Kahola (2018-08-09 11:12:33)
> To reduce the execution time of kms_flip test on CI, let's move subtest
> duration parameter as command line option. The default subtest duration
> is 3 seconds for test that require jitter computation and for the rest
> of the subtests are run only once.

I disagree with having tests that change between invocations. It's far
too easy for one to make a mistake and not configure the test correctly,
completely fairly to reproduce whatever bug one was seeking.

If you think both durations are interesting for different aspects, test
both.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for tests/kms_flip: Binary mode optimizations (rev4)

[Patchwork]

== Series Details ==

Series: tests/kms_flip: Binary mode optimizations (rev4)
URL   : https://patchwork.freedesktop.org/series/44619/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4636 -> IGTPW_1696 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44619/revisions/4/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1696:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_suspend@basic-s3:
      {fi-kbl-soraka}:    NOTRUN -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in IGTPW_1696 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         PASS -> DMESG-FAIL (fdo#107174)
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)
      {fi-icl-u}:         NOTRUN -> INCOMPLETE (fdo#107399)

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      {fi-icl-u}:         NOTRUN -> DMESG-WARN (fdo#107382) +4

    {igt@kms_psr@primary_page_flip}:
      {fi-icl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)

    
  {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#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399


== Participating hosts (50 -> 49) ==

  Additional (3): fi-kbl-soraka fi-kbl-7560u fi-icl-u 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4590 -> IGTPW_1696

  CI_DRM_4636: 084bb2fb549650b6da80976c9bc594779ce342b4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1696: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1696/
  IGT_4590: e6ddaca7a8ea9d3d27f0ecaa36b357cc02e2df3b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1696/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Mika Kahola]

On Thu, 2018-08-09 at 11:20 +0100, Chris Wilson wrote:
> Quoting Mika Kahola (2018-08-09 11:12:33)
> > 
> > To reduce the execution time of kms_flip test on CI, let's move
> > subtest
> > duration parameter as command line option. The default subtest
> > duration
> > is 3 seconds for test that require jitter computation and for the
> > rest
> > of the subtests are run only once.
> I disagree with having tests that change between invocations. It's
> far
> too easy for one to make a mistake and not configure the test
> correctly,
> completely fairly to reproduce whatever bug one was seeking.
> 
So, should I remove the option of setting the duration for the
subtests? Originally, we have timeouts ranging from 0 to 30 seconds,
depending on a test. Based on my tests on GLK, I found out that tests
that require some jitter computation yields the similar results if we
run it for 3 or 30 seconds. When running less than 3 seconds there are
differences in jitter results. That's why I suggested that 3 seconds
would be a sweetspot for the subtests.

Should I just use 3 seconds timeout for all subtests and discard the
possibility to set the timeout? 

> If you think both durations are interesting for different aspects,
> test
> both.
> -Chris
-- 
Mika Kahola - Intel OTC

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for tests/kms_flip: Binary mode optimizations (rev4)

[Patchwork]

== Series Details ==

Series: tests/kms_flip: Binary mode optimizations (rev4)
URL   : https://patchwork.freedesktop.org/series/44619/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4590_full -> IGTPW_1696_full =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44619/revisions/4/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1696_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_sync@basic-each:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm:
      shard-kbl:          PASS -> FAIL (fdo#106539)
      shard-apl:          PASS -> FAIL (fdo#106539)
      shard-glk:          PASS -> FAIL (fdo#106539)
      shard-hsw:          PASS -> FAIL (fdo#106539)

    
    ==== Possible fixes ====

    igt@gem_cs_tlb@blt:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_workarounds@suspend-resume:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm:
      shard-hsw:          FAIL (fdo#106539) -> PASS +7

    igt@pm_rpm@cursor-dpms:
      shard-glk:          WARN -> PASS

    igt@pm_rpm@modeset-lpsp-stress:
      shard-hsw:          FAIL (fdo#106539) -> SKIP

    
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106539 https://bugs.freedesktop.org/show_bug.cgi?id=106539


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4590 -> IGTPW_1696
    * Linux: CI_DRM_4635 -> CI_DRM_4636

  CI_DRM_4635: a9f34f7e3bab765e2b1320a1b154a76be38602a7 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4636: 084bb2fb549650b6da80976c9bc594779ce342b4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1696: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1696/
  IGT_4590: e6ddaca7a8ea9d3d27f0ecaa36b357cc02e2df3b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1696/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 2/2] tests/kms_flip: Change 2x tests execution order

[Maarten Lankhorst]

Op 09-08-18 om 12:12 schreef Mika Kahola:
> In order to optimize execution of kms_flip binary mode tests, let's change
> the execution order so that 2x tests will be executed after basic tests.
>
> v2: update commit message (Petri)
> v3: Change subtest execution order
>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  tests/kms_flip.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 4cd1951..6217d67 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1640,19 +1640,6 @@ int main(int argc, char **argv)
>  			      tests[i].name)
>  			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
>  
> -		if (tests[i].flags & TEST_NO_2X_OUTPUT)
> -			continue;
> -
> -		/* code doesn't disable all crtcs, so skip rpm tests */
> -		if (tests[i].flags & TEST_RPM)
> -			continue;
> -
> -		igt_subtest_f( "2x-%s", tests[i].name)
> -			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
> -	}
> -
> -	igt_fork_signal_helper();
> -	for (i = 0; i < sizeof(tests) / sizeof (tests[0]); i++) {
>  		/* relative blocking vblank waits that get constantly interrupt
>  		 * take forver. So don't do them. */
>  		if ((tests[i].flags & TEST_VBLANK_BLOCK) &&
> @@ -1661,7 +1648,10 @@ int main(int argc, char **argv)
>  
>  		igt_subtest_f( "%s-interruptible", tests[i].name)
>  			run_test(max(opt.duration, tests[i].duration), tests[i].flags);
> +	}
>  
> +	igt_fork_signal_helper();
Hmm, this forces all the 2x tests as interruptible?
> +	for (i = 0; i < sizeof(tests) / sizeof (tests[0]); i++) {
>  		if (tests[i].flags & TEST_NO_2X_OUTPUT)
>  			continue;
>  
> @@ -1669,6 +1659,15 @@ int main(int argc, char **argv)
>  		if (tests[i].flags & TEST_RPM)
>  			continue;
>  
> +		igt_subtest_f( "2x-%s", tests[i].name)
> +			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
> +
> +		/* relative blocking vblank waits that get constantly interrupt
> +		 * take forver. So don't do them. */
> +		if ((tests[i].flags & TEST_VBLANK_BLOCK) &&
> +		    !(tests[i].flags & TEST_VBLANK_ABSOLUTE))
> +			continue;
> +
>  		igt_subtest_f( "2x-%s-interruptible", tests[i].name) {
put igt_fork_signal_helper() here
>  			run_pair(max(opt.duration, tests[i].duration), tests[i].flags);
And stop it after.
>  	}

Yeah doing the 2x tests isn't optimal in that order and causes a lot of modesets.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Maarten Lankhorst]

Op 09-08-18 om 12:12 schreef Mika Kahola:
> To reduce the execution time of kms_flip test on CI, let's move subtest
> duration parameter as command line option. The default subtest duration
> is 3 seconds for test that require jitter computation and for the rest
> of the subtests are run only once.
>
> v2: Run each subtest only once (default action)
> v3: Reduces default timeout for tests that require jitter computation (Ville)
>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
I think this is the wrong approach. What we really want to do is kill tests from kms_flip
instead and move them to separate places.

Killing off all the interruptible tests would save 50% of the time. So all we have to do is
making sure that we have tests that test the missing ioctl's in in kms_atomic_interruptible,
and we would save 50% of the time.

After that we have to get rid of the unrelated tests to make the code more readable..
nonexisting-fb, dpms-vs-vblank-race, modeset-vs-vblank-race and bo-too-big would be
better off in a separate testcase.

Same for bad-tiling, but I already had a patch for that.
https://patchwork.freedesktop.org/patch/259361/

This would clean up kms_flip a lot, and make the test more readable.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Kahola, Mika]

On Thu, 2018-11-08 at 12:47 +0100, Maarten Lankhorst wrote:
> Op 09-08-18 om 12:12 schreef Mika Kahola:
> > To reduce the execution time of kms_flip test on CI, let's move
> > subtest
> > duration parameter as command line option. The default subtest
> > duration
> > is 3 seconds for test that require jitter computation and for the
> > rest
> > of the subtests are run only once.
> > 
> > v2: Run each subtest only once (default action)
> > v3: Reduces default timeout for tests that require jitter
> > computation (Ville)
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> 
> I think this is the wrong approach. What we really want to do is kill
> tests from kms_flip
> instead and move them to separate places.
> 
> Killing off all the interruptible tests would save 50% of the time.
> So all we have to do is
> making sure that we have tests that test the missing ioctl's in in
> kms_atomic_interruptible,
> and we would save 50% of the time.
kms_atomic_interruptible would probably be more logical place for
interrupt tests. I agree that it would reduce the execution time of the
kms_flip test but in the end of the day we still need to run those
interruptible tests and therefore we would end up increasing the
execution time of the kms_atomic_interruptible test.

Do you feel that we could have overall reduction in test execution if
we move these interruptible tests out from kms_flip? Maybe kms_flip
readability would improve and therefore worth the effort?
> 
> After that we have to get rid of the unrelated tests to make the code
> more readable..
> nonexisting-fb, dpms-vs-vblank-race, modeset-vs-vblank-race and bo-
> too-big would be
> better off in a separate testcase.
> 
> Same for bad-tiling, but I already had a patch for that.
> https://patchwork.freedesktop.org/patch/259361/
> 
> This would clean up kms_flip a lot, and make the test more readable.
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Maarten Lankhorst]

Op 08-11-18 om 13:45 schreef Kahola, Mika:
> On Thu, 2018-11-08 at 12:47 +0100, Maarten Lankhorst wrote:
>> Op 09-08-18 om 12:12 schreef Mika Kahola:
>>> To reduce the execution time of kms_flip test on CI, let's move
>>> subtest
>>> duration parameter as command line option. The default subtest
>>> duration
>>> is 3 seconds for test that require jitter computation and for the
>>> rest
>>> of the subtests are run only once.
>>>
>>> v2: Run each subtest only once (default action)
>>> v3: Reduces default timeout for tests that require jitter
>>> computation (Ville)
>>>
>>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> I think this is the wrong approach. What we really want to do is kill
>> tests from kms_flip
>> instead and move them to separate places.
>>
>> Killing off all the interruptible tests would save 50% of the time.
>> So all we have to do is
>> making sure that we have tests that test the missing ioctl's in in
>> kms_atomic_interruptible,
>> and we would save 50% of the time.
> kms_atomic_interruptible would probably be more logical place for
> interrupt tests. I agree that it would reduce the execution time of the
> kms_flip test but in the end of the day we still need to run those
> interruptible tests and therefore we would end up increasing the
> execution time of the kms_atomic_interruptible test.
>
> Do you feel that we could have overall reduction in test execution if
> we move these interruptible tests out from kms_flip? Maybe kms_flip
> readability would improve and therefore worth the effort?
The kms_flip ones just double the execution time of the tests, and don't result in more coverage.

Testing the various ioctl's in a controlled fashion would result in better coverage and lower execution time.
Just look at how kms_atomic_interruptible is structure, it should be easy to add more tests to it and actually
guarantees that the ioctl being tested is interrupted.

I definitely hope that removing those tests increase readability, because of all the unrelated API test special
cases being removed, and only actual flip related tests remaining.

~Maarten
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/kms_flip: Set duration for subtest from command line

[Kahola, Mika]

On Thu, 2018-11-08 at 14:11 +0100, Maarten Lankhorst wrote:
> Op 08-11-18 om 13:45 schreef Kahola, Mika:
> > On Thu, 2018-11-08 at 12:47 +0100, Maarten Lankhorst wrote:
> > > Op 09-08-18 om 12:12 schreef Mika Kahola:
> > > > To reduce the execution time of kms_flip test on CI, let's move
> > > > subtest
> > > > duration parameter as command line option. The default subtest
> > > > duration
> > > > is 3 seconds for test that require jitter computation and for
> > > > the
> > > > rest
> > > > of the subtests are run only once.
> > > > 
> > > > v2: Run each subtest only once (default action)
> > > > v3: Reduces default timeout for tests that require jitter
> > > > computation (Ville)
> > > > 
> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > 
> > > I think this is the wrong approach. What we really want to do is
> > > kill
> > > tests from kms_flip
> > > instead and move them to separate places.
> > > 
> > > Killing off all the interruptible tests would save 50% of the
> > > time.
> > > So all we have to do is
> > > making sure that we have tests that test the missing ioctl's in
> > > in
> > > kms_atomic_interruptible,
> > > and we would save 50% of the time.
> > 
> > kms_atomic_interruptible would probably be more logical place for
> > interrupt tests. I agree that it would reduce the execution time of
> > the
> > kms_flip test but in the end of the day we still need to run those
> > interruptible tests and therefore we would end up increasing the
> > execution time of the kms_atomic_interruptible test.
> > 
> > Do you feel that we could have overall reduction in test execution
> > if
> > we move these interruptible tests out from kms_flip? Maybe kms_flip
> > readability would improve and therefore worth the effort?
> 
> The kms_flip ones just double the execution time of the tests, and
> don't result in more coverage.
> 
> Testing the various ioctl's in a controlled fashion would result in
> better coverage and lower execution time.
> Just look at how kms_atomic_interruptible is structure, it should be
> easy to add more tests to it and actually
> guarantees that the ioctl being tested is interrupted.
> 
> I definitely hope that removing those tests increase readability,
> because of all the unrelated API test special
> cases being removed, and only actual flip related tests remaining.
Ok. I prepare a patch that removes interruptible tests from kms_flip.
Let's see how kms_flip looks after that change. I also look into adding
the removed tests to kms_atomic_interruptible. 
 
> 
> ~Maarten
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev