Re: [igt-dev] [PATCH i-g-t v6] tests/kms_atomic_transition:reduce execution time | igt-dev

quoted hunk

-----Original Message-----
From: Nidhi Gupta <redacted>
Sent: Tuesday, January 5, 2021 4:39 PM
To: igt-dev@lists.freedesktop.org
Cc: B S, Karthik <redacted>; Shankar, Uma
[off-list ref]; Gupta, Nidhi1 [off-list ref]
Subject: [PATCH i-g-t v6] tests/kms_atomic_transition:reduce execution time

v1: All the subtests are using for_each_pipe_with_valid_output function which
No need to say v1, instead describe what this change is for. Add any timing benefits
wrt execution time.

will execute on all the possible combination of pipe and output to reduce the
execution time replaced the function with for_each_pipe_with_single_output
this will loop over all the pipes but at most once.

v2: kms_atomic_transition test is taking minimum of 69.5s time to execute on CI.
To reduce the execution time this patch will add the change which will run the
test on 1 HDR plane, 1 SDR UV plane, 1 SDR Y plane and skip the rest of the
planes.

v3: combined v1 and v2 in one patch.

v4: -restricted execution of all the subtests to
    2 pipes. (Uma)
    -Modified skip_plane() function. (Uma)

v5: -added a extended flag, if it is set but the user
s/but/by

    test will be executed on all the pipes otherwise will be
    executed only on 2 pipes. (Karthik)

Signed-off-by: Nidhi Gupta <redacted>
---
 tests/kms_atomic_transition.c | 136 +++++++++++++++++++++++++++-------
 1 file changed, 110 insertions(+), 26 deletions(-)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c index
02206f0a..1d0faa67 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -50,6 +50,10 @@ int *timeline;
 pthread_t *thread;
 int *seqno;

+typedef struct {
+	bool extended;
+} data_t;
I think its good to add igt_display_t *display in this structure itself.

 static void
 run_primary_test(igt_display_t *display, enum pipe pipe, igt_output_t *output)
{ @@ -120,8 +124,35 @@ static void configure_fencing(igt_plane_t *plane)
 	igt_assert_eq(ret, 0);
 }

+static bool skip_plane(igt_display_t *display, data_t *data,
+igt_plane_t *plane) 
If you do the above, then pass just the data_t and display will come along with it.
Do it universally in the patch at all places.

{
+	int index = plane->index;
+
+	if (data->extended)
+		return false;
+
+	if (!is_i915_device(display->drm_fd))
+		return false;
+
+	if (plane->type == DRM_PLANE_TYPE_CURSOR)
+		return false;
+
+	if (intel_gen(intel_get_drm_devid(display->drm_fd)) < 11)
+		return false;
+
+	/*
+	 * Test 1 HDR plane, 1 SDR UV plane, 1 SDR Y plane.
+	 *
+	 * Kernel registers planes in the hardware Z order:
+	 * 0,1,2 HDR planes
+	 * 3,4 SDR UV planes
+	 * 5,6 SDR Y planes
+	 */
+	return index != 0 && index != 3 && index != 5; }
+
 static int
-wm_setup_plane(igt_display_t *display, enum pipe pipe,
+wm_setup_plane(igt_display_t *display, data_t *data, enum pipe pipe,
 	       uint32_t mask, struct plane_parms *parms, bool fencing)  {
 	igt_plane_t *plane;
@@ -135,6 +166,9 @@ wm_setup_plane(igt_display_t *display, enum pipe pipe,
 	for_each_plane_on_pipe(display, pipe, plane) {
 		int i = plane->index;

+		if (skip_plane(display, data, plane))
+			continue;
+
 		if (!mask || !(parms[i].mask & mask)) {
 			if (plane->values[IGT_PLANE_FB_ID]) {
 				igt_plane_set_fb(plane, NULL);
@@ -205,7 +239,8 @@ static void set_sprite_wh(igt_display_t *display, enum
pipe pipe,  #define is_atomic_check_plane_size_errno(errno) \
 		(errno == -EINVAL)

-static void setup_parms(igt_display_t *display, enum pipe pipe,
+static void setup_parms(igt_display_t *display, data_t *data,
+			enum pipe pipe,
 			const drmModeModeInfo *mode,
 			struct igt_fb *primary_fb,
 			struct igt_fb *argb_fb,
@@ -298,7 +333,7 @@ static void setup_parms(igt_display_t *display, enum
pipe pipe,
 		set_sprite_wh(display, pipe, parms, sprite_fb,
 			      alpha, sprite_width, sprite_height);

-		wm_setup_plane(display, pipe, (1 << n_planes) - 1, parms, false);
+		wm_setup_plane(display, data, pipe, (1 << n_planes) - 1, parms,
+false);
 		ret = igt_display_try_commit_atomic(display,
DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET,
NULL);
 		igt_assert(!is_atomic_check_failure_errno(ret));

@@ -437,7 +472,7 @@ static void wait_for_transition(igt_display_t *display,
enum pipe pipe, bool non
  * so test this and make sure it works.
  */
 static void
-run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t
*output,
+run_transition_test(igt_display_t *display, data_t *data, enum pipe
+pipe, igt_output_t *output,
 		enum transition_type type, bool nonblocking, bool fencing)  {
 	struct igt_fb fb, argb_fb, sprite_fb;
@@ -470,7 +505,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output

 	igt_output_set_pipe(output, pipe);

-	wm_setup_plane(display, pipe, 0, NULL, false);
+	wm_setup_plane(display, data, pipe, 0, NULL, false);

 	if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
 		igt_output_set_pipe(output, PIPE_NONE); @@ -482,7 +517,7 @@
run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output

 	igt_display_commit2(display, COMMIT_ATOMIC);

-	setup_parms(display, pipe, mode, &fb, &argb_fb, &sprite_fb, parms,
&iter_max);
+	setup_parms(display, data, pipe, mode, &fb, &argb_fb, &sprite_fb,
+parms, &iter_max);

 	/*
 	 * In some configurations the tests may not run to completion with all
@@ -490,7 +525,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output
 	 * planes to fix this
 	 */
 	while (1) {
-		wm_setup_plane(display, pipe, iter_max - 1, parms, false);
+		wm_setup_plane(display, data, pipe, iter_max - 1, parms, false);

 		if (fencing)
 			igt_pipe_request_out_fence(pipe_obj);
@@ -524,7 +559,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output
 	if (type == TRANSITION_AFTER_FREE) {
 		int fence_fd = -1;

-		wm_setup_plane(display, pipe, 0, parms, fencing);
+		wm_setup_plane(display, data, pipe, 0, parms, fencing);

 		atomic_commit(display, pipe, flags, (void *)(unsigned long)0,
fencing);
 		if (fencing) {
@@ -560,7 +595,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output

 		igt_output_set_pipe(output, pipe);

-		if (!wm_setup_plane(display, pipe, i, parms, fencing))
+		if (!wm_setup_plane(display, data, pipe, i, parms, fencing))
 			continue;

 		atomic_commit(display, pipe, flags, (void *)(unsigned long)i,
fencing); @@ -569,7 +604,7 @@ run_transition_test(igt_display_t *display,
enum pipe pipe, igt_output_t *output
 		if (type == TRANSITION_MODESET_DISABLE) {
 			igt_output_set_pipe(output, PIPE_NONE);

-			if (!wm_setup_plane(display, pipe, 0, parms, fencing))
+			if (!wm_setup_plane(display, data, pipe, 0, parms,
fencing))
 				continue;

 			atomic_commit(display, pipe, flags, (void *) 0UL, fencing);
@@ -586,7 +621,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output
 				    n_enable_planes < pipe_obj->n_planes)
 					continue;

-				if (!wm_setup_plane(display, pipe, j, parms,
fencing))
+				if (!wm_setup_plane(display, data, pipe, j, parms,
fencing))
 					continue;

 				if (type >= TRANSITION_MODESET)
@@ -595,7 +630,7 @@ run_transition_test(igt_display_t *display, enum pipe
pipe, igt_output_t *output
 				atomic_commit(display, pipe, flags, (void
*)(unsigned long) j, fencing);
 				wait_for_transition(display, pipe, nonblocking,
fencing);

-				if (!wm_setup_plane(display, pipe, i, parms,
fencing))
+				if (!wm_setup_plane(display, data, pipe, i, parms,
fencing))
 					continue;

 				if (type >= TRANSITION_MODESET)
@@ -913,7 +948,30 @@ static bool output_is_internal_panel(igt_output_t
*output)
 	}
 }

-igt_main
+static int opt_handler(int opt, int opt_index, void *_data) {
+	data_t *data = _data;
+
+	switch (opt) {
+	case 'e':
+		data->extended = true;
+		break;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+static const struct option long_opts[] = {
+	{ .name = "extended", .has_arg = false, .val = 'e', },
+	{}
+};
+
+static const char help_str[] =
+	"  --extended\t\tRun the extended tests\n";
+
+static data_t data;
+
+igt_main_args("", long_opts, help_str, opt_handler, &data)
 {
 	igt_display_t display;
 	igt_output_t *output;
@@ -935,48 +993,63 @@ igt_main
 	}

 	igt_subtest("plane-primary-toggle-with-vblank-wait")
-		for_each_pipe_with_valid_output(&display, pipe, output)
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			run_primary_test(&display, pipe, output);
+		}

 	igt_subtest_with_dynamic("plane-all-transition") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_PLANES, false, false);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_PLANES, false, false);
 		}
 	}

 	igt_subtest_with_dynamic("plane-all-transition-fencing") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_PLANES, false, true);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_PLANES, false, true);
 		}
 	}

 	igt_subtest_with_dynamic("plane-all-transition-nonblocking") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_PLANES, true, false);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_PLANES, true, false);
 		}
 	}

 	igt_subtest_with_dynamic("plane-all-transition-nonblocking-fencing") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_PLANES, true, true);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_PLANES, true, true);
 		}
 	}

 	igt_subtest_with_dynamic("plane-use-after-nonblocking-unbind") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_AFTER_FREE, true, false);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_AFTER_FREE, true, false);
 		}
 	}

 	igt_subtest_with_dynamic("plane-use-after-nonblocking-unbind-
fencing") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_AFTER_FREE, true, true);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_AFTER_FREE, true, true);
 		}
 	}

@@ -987,45 +1060,56 @@ igt_main
 	 */
 	igt_subtest_with_dynamic("plane-all-modeset-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			if (output_is_internal_panel(output))
 				continue;

 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_MODESET, false, false);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_MODESET, false, false);
 		}

 	igt_subtest_with_dynamic("plane-all-modeset-transition-fencing")
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			if (output_is_internal_panel(output))
 				continue;

 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_MODESET, false, true);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_MODESET, false, true);
 		}

 	igt_subtest_with_dynamic("plane-all-modeset-transition-internal-
panels") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			if (!output_is_internal_panel(output))
 				continue;

 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_MODESET_FAST, false, false);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_MODESET_FAST, false, false);
 		}
 	}

 	igt_subtest_with_dynamic("plane-all-modeset-transition-fencing-
internal-panels") {
 		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
 			if (!output_is_internal_panel(output))
 				continue;

 			igt_dynamic_f("%s-pipe-%s", igt_output_name(output),
kmstest_pipe_name(pipe))
-				run_transition_test(&display, pipe, output,
TRANSITION_MODESET_FAST, false, true);
+				run_transition_test(&display, &data, pipe,
output,
+TRANSITION_MODESET_FAST, false, true);
 		}
 	}

 	igt_subtest("plane-toggle-modeset-transition")
-		for_each_pipe_with_valid_output(&display, pipe, output)
-			run_transition_test(&display, pipe, output,
TRANSITION_MODESET_DISABLE, false, false);
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			if (pipe >= 2 && !data.extended)
+				break;
+			run_transition_test(&display, &data, pipe, output,
TRANSITION_MODESET_DISABLE, false, false);
+		}

 	igt_subtest_with_dynamic("modeset-transition") {
 		for (i = 1; i <= count; i++) {
--
2.26.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help