[igt-dev] [PATCH i-g-t v6 0/2] kms_atomic_transition improvements

[igt-dev] [PATCH i-g-t v6 0/2] kms_atomic_transition improvements

[Stanislav Lisovskiy]

Fixed run_transition_test issue, which might happen when no changes
are done and wait_transition is called and made setup_parms function
tolerate if kernel can't afford having all sprite planes enabled
depending on current mode. Remove redundant condition nesting and code.

Stanislav Lisovskiy (2):
  igt/tests/kms_atomic_transition: Skip transition, if no changes done
  igt/tests/kms_atomic_transition: Tolerate if can't have all planes

 tests/kms_atomic_transition.c | 146 ++++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 58 deletions(-)

-- 
2.17.1

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

[igt-dev] [PATCH i-g-t v6 1/2] igt/tests/kms_atomic_transition: Skip transition, if no changes done

[Stanislav Lisovskiy]

While fixing used amount of planes, discovered that if
wm_setup_plane is called with specific parameter that
gives parms[i].mask & mask == 0 for all used planes,
then subsequent wait_transition fails in assertion on fd_completed.
So added return value to wm_setup_plane, which would allow to
determine, if we really need to wait for any transitions.

v2: Fixed commit message, to properly describe a reasoning for
    wm_setup_plane changes.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 tests/kms_atomic_transition.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 18f73317..638fe17e 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -118,11 +118,12 @@ static void configure_fencing(igt_plane_t *plane)
 	igt_assert_eq(ret, 0);
 }
 
-static void
+static int
 wm_setup_plane(igt_display_t *display, enum pipe pipe,
 	       uint32_t mask, struct plane_parms *parms, bool fencing)
 {
 	igt_plane_t *plane;
+	int planes_set_up = 0;
 
 	/*
 	* Make sure these buffers are suited for display use
@@ -133,8 +134,10 @@ wm_setup_plane(igt_display_t *display, enum pipe pipe,
 		int i = plane->index;
 
 		if (!mask || !(parms[i].mask & mask)) {
-			if (plane->values[IGT_PLANE_FB_ID])
+			if (plane->values[IGT_PLANE_FB_ID]) {
 				igt_plane_set_fb(plane, NULL);
+				planes_set_up++;
+			}
 			continue;
 		}
 
@@ -144,7 +147,10 @@ wm_setup_plane(igt_display_t *display, enum pipe pipe,
 		igt_plane_set_fb(plane, parms[i].fb);
 		igt_fb_set_size(parms[i].fb, plane, parms[i].width, parms[i].height);
 		igt_plane_set_size(plane, parms[i].width, parms[i].height);
+
+		planes_set_up++;
 	}
+	return planes_set_up;
 }
 
 static void ev_page_flip(int fd, unsigned seq, unsigned tv_sec, unsigned tv_usec, void *user_data)
@@ -544,7 +550,8 @@ 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, i, parms, fencing);
+		if (!wm_setup_plane(display, pipe, i, parms, fencing))
+			continue;
 
 		atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
 		wait_for_transition(display, pipe, nonblocking, fencing);
@@ -552,7 +559,8 @@ 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);
 
-			wm_setup_plane(display, pipe, 0, parms, fencing);
+			if (!wm_setup_plane(display, pipe, 0, parms, fencing))
+				continue;
 
 			atomic_commit(display, pipe, flags, (void *) 0UL, fencing);
 			wait_for_transition(display, pipe, nonblocking, fencing);
@@ -568,7 +576,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 				    n_enable_planes < pipe_obj->n_planes)
 					continue;
 
-				wm_setup_plane(display, pipe, j, parms, fencing);
+				if (!wm_setup_plane(display, pipe, j, parms, fencing))
+					continue;
 
 				if (type >= TRANSITION_MODESET)
 					igt_output_override_mode(output, &override_mode);
@@ -576,7 +585,9 @@ 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);
 
-				wm_setup_plane(display, pipe, i, parms, fencing);
+				if (!wm_setup_plane(display, pipe, i, parms, fencing))
+					continue;
+
 				if (type >= TRANSITION_MODESET)
 					igt_output_override_mode(output, NULL);
 
-- 
2.17.1

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

[igt-dev] [PATCH i-g-t v6 2/2] igt/tests/kms_atomic_transition: Tolerate if can't have all planes

[Stanislav Lisovskiy]

With some upcoming changes i915 might not allow
all sprite planes enabled, depending on available
bandwidth limitation. Thus the test need to decrement
amount of planes and try again, instead of panicking.

v2: Removed excessive nested conditions, making code a bit
    more readable(hopefully).

v3: Stopped using global n_planes variable as it might cause
    resources being unreleased.
    Using now parms[i].mask as a way to determine if plane
    has to be included into commit.

v4: Removed unneeded n_overlays initialization.

v5: Randomize which of sprite planes to remove if hitting
    resource limits.

v6: Replace igt_warn with igt_info, to make IGT tests happier.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 tests/kms_atomic_transition.c | 123 ++++++++++++++++++++--------------
 1 file changed, 71 insertions(+), 52 deletions(-)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 638fe17e..a37f1d46 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -212,9 +212,12 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 	unsigned sprite_width, sprite_height, prev_w, prev_h;
 	bool max_sprite_width, max_sprite_height, alpha = true;
 	uint32_t n_planes = display->pipes[pipe].n_planes;
-	uint32_t n_overlays = 0, overlays[n_planes];
+	uint32_t n_overlays, overlays[n_planes];
 	igt_plane_t *plane;
-	uint32_t iter_mask = 3;
+	uint32_t iter_mask;
+	int retries = n_planes - 1;
+	int ret = 0;
+	uint32_t planes_to_remove;
 
 	do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
 	if (cursor_width >= mode->hdisplay)
@@ -224,6 +227,11 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 	if (cursor_height >= mode->vdisplay)
 		cursor_height = mode->vdisplay;
 
+retry:
+	n_overlays = 0;
+	iter_mask = 3;
+	planes_to_remove = display->pipes[pipe].n_planes - n_planes;
+
 	for_each_plane_on_pipe(display, pipe, plane) {
 		int i = plane->index;
 
@@ -238,12 +246,30 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 			parms[i].height = cursor_height;
 			parms[i].mask = 1 << 1;
 		} else {
-			parms[i].fb = sprite_fb;
-			parms[i].mask = 1 << 2;
-
-			iter_mask |= 1 << 2;
-
-			overlays[n_overlays++] = i;
+			/* Randomize if we remove that sprite plane or not */
+			uint32_t remove = hars_petruska_f54_1_random_unsafe_max(2);
+			/*
+			 * Figure out how much sprite planes left:
+			 * planes left = overall amount - 1 primary - 1 cursor - sprite planes used
+			 */
+			int planes_left = display->pipes[pipe].n_planes - 2 - n_overlays;
+			/*
+			 * Remove the plane if remove is set and we have sprite planes to remove.
+			 * If we have left same or less amount of planes than we need
+			 * to remove then no randomization, just remove.
+			 */
+			if ((remove && planes_to_remove > 0) || (planes_to_remove >= planes_left)) {
+				parms[i].fb = NULL;
+				parms[i].mask = 0;
+				planes_to_remove--;
+				igt_info("Removed plane %d\n", i);
+			}
+			else {
+				parms[i].fb = sprite_fb;
+				parms[i].mask = 1 << 2;
+				iter_mask |= 1 << 2;
+				overlays[n_overlays++] = i;
+			}
 		}
 	}
 
@@ -278,16 +304,13 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 	 * Pre gen9 not all sizes are supported, find the biggest possible
 	 * size that can be enabled on all sprite planes.
 	 */
-retry:
 	prev_w = sprite_width = cursor_width;
 	prev_h = sprite_height = cursor_height;
 
 	max_sprite_width = (sprite_width == mode->hdisplay);
 	max_sprite_height = (sprite_height == mode->vdisplay);
 
-	while (1) {
-		int ret;
-
+	while (!max_sprite_width && !max_sprite_height) {
 		set_sprite_wh(display, pipe, parms, sprite_fb,
 			      alpha, sprite_width, sprite_height);
 
@@ -295,54 +318,49 @@ retry:
 		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));
 
-		if (is_atomic_check_plane_size_errno(ret)) {
-			if (cursor_width == sprite_width &&
-			    cursor_height == sprite_height) {
-				igt_assert_f(alpha,
-					      "Cannot configure the test with all sprite planes enabled\n");
-
-				/* retry once with XRGB format. */
-				alpha = false;
-				goto retry;
-			}
-
-			sprite_width = prev_w;
-			sprite_height = prev_h;
-
-			if (max_sprite_width && max_sprite_height) {
-				set_sprite_wh(display, pipe, parms, sprite_fb,
-					      alpha, sprite_width, sprite_height);
-				break;
-			}
-
-			if (!max_sprite_width)
-				max_sprite_width = true;
-			else
-				max_sprite_height = true;
-		} else {
+		if (!is_atomic_check_plane_size_errno(ret)) {
 			prev_w = sprite_width;
 			prev_h = sprite_height;
-		}
-
-		if (!max_sprite_width) {
-			sprite_width *= 2;
-
+    
+			sprite_width *= max_sprite_width ? 1 : 2;
 			if (sprite_width >= mode->hdisplay) {
 				max_sprite_width = true;
-
 				sprite_width = mode->hdisplay;
 			}
-		} else if (!max_sprite_height) {
-			sprite_height *= 2;
 
+			sprite_height *= max_sprite_height ? 1 : 2;
 			if (sprite_height >= mode->vdisplay) {
 				max_sprite_height = true;
-
 				sprite_height = mode->vdisplay;
 			}
-		} else
-			/* Max sized sprites for all! */
-			break;
+			continue;
+		}
+
+		if (cursor_width == sprite_width &&
+		    cursor_height == sprite_height) {
+			igt_assert_f(retries > 0,
+				      "Cannot configure the test with all sprite planes enabled\n");
+			--retries;
+			/* retry once with XRGB format. */
+			if (alpha) {
+				alpha = false;
+				igt_info("Removed alpha\n");
+			}
+			else {
+				igt_assert_f(n_planes > 1, "No planes left to proceed with!");
+				n_planes--;
+				igt_info("Reduced available planes to %d\n", n_planes);
+			}
+			goto retry;
+		}
+
+		sprite_width = prev_w;
+		sprite_height = prev_h;
+
+		if (!max_sprite_width)
+			max_sprite_width = true;
+		else
+			max_sprite_height = true;
 	}
 
 	igt_info("Running test on pipe %s with resolution %dx%d and sprite size %dx%d alpha %i\n",
@@ -463,7 +481,6 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 	if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
 		igt_output_set_pipe(output, PIPE_NONE);
-
 		igt_display_commit2(display, COMMIT_ATOMIC);
 
 		igt_output_set_pipe(output, pipe);
@@ -525,8 +542,10 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 		}
 
 		/* force planes to be part of commit */
-		for_each_plane_on_pipe(display, pipe, plane)
-			igt_plane_set_position(plane, 0, 0);
+		for_each_plane_on_pipe(display, pipe, plane) {
+			if (parms[plane->index].mask)
+				igt_plane_set_position(plane, 0, 0);
+		}
 
 		igt_display_commit2(display, COMMIT_ATOMIC);
 
-- 
2.17.1

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

Re: [igt-dev] [PATCH i-g-t v6 2/2] igt/tests/kms_atomic_transition: Tolerate if can't have all planes

[Maarten Lankhorst]

Op 08-04-2019 om 12:18 schreef Stanislav Lisovskiy:
> With some upcoming changes i915 might not allow
> all sprite planes enabled, depending on available
> bandwidth limitation. Thus the test need to decrement
> amount of planes and try again, instead of panicking.
>
> v2: Removed excessive nested conditions, making code a bit
>     more readable(hopefully).
>
> v3: Stopped using global n_planes variable as it might cause
>     resources being unreleased.
>     Using now parms[i].mask as a way to determine if plane
>     has to be included into commit.
>
> v4: Removed unneeded n_overlays initialization.
>
> v5: Randomize which of sprite planes to remove if hitting
>     resource limits.
>
> v6: Replace igt_warn with igt_info, to make IGT tests happier.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  tests/kms_atomic_transition.c | 123 ++++++++++++++++++++--------------
>  1 file changed, 71 insertions(+), 52 deletions(-)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 638fe17e..a37f1d46 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -212,9 +212,12 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
>  	unsigned sprite_width, sprite_height, prev_w, prev_h;
>  	bool max_sprite_width, max_sprite_height, alpha = true;
>  	uint32_t n_planes = display->pipes[pipe].n_planes;
> -	uint32_t n_overlays = 0, overlays[n_planes];
> +	uint32_t n_overlays, overlays[n_planes];
>  	igt_plane_t *plane;
> -	uint32_t iter_mask = 3;
> +	uint32_t iter_mask;
> +	int retries = n_planes - 1;
> +	int ret = 0;
> +	uint32_t planes_to_remove;
>  
>  	do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
>  	if (cursor_width >= mode->hdisplay)
> @@ -224,6 +227,11 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
>  	if (cursor_height >= mode->vdisplay)
>  		cursor_height = mode->vdisplay;
>  
> +retry:
> +	n_overlays = 0;
> +	iter_mask = 3;
> +	planes_to_remove = display->pipes[pipe].n_planes - n_planes;
> +
>  	for_each_plane_on_pipe(display, pipe, plane) {
>  		int i = plane->index;
>  
> @@ -238,12 +246,30 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
>  			parms[i].height = cursor_height;
>  			parms[i].mask = 1 << 1;
>  		} else {
> -			parms[i].fb = sprite_fb;
> -			parms[i].mask = 1 << 2;
> -
> -			iter_mask |= 1 << 2;
> -
> -			overlays[n_overlays++] = i;
> +			/* Randomize if we remove that sprite plane or not */
> +			uint32_t remove = hars_petruska_f54_1_random_unsafe_max(2);
> +			/*
> +			 * Figure out how much sprite planes left:
> +			 * planes left = overall amount - 1 primary - 1 cursor - sprite planes used
> +			 */
> +			int planes_left = display->pipes[pipe].n_planes - 2 - n_overlays;
> +			/*
> +			 * Remove the plane if remove is set and we have sprite planes to remove.
> +			 * If we have left same or less amount of planes than we need
> +			 * to remove then no randomization, just remove.
> +			 */
> +			if ((remove && planes_to_remove > 0) || (planes_to_remove >= planes_left)) {
> +				parms[i].fb = NULL;
> +				parms[i].mask = 0;
> +				planes_to_remove--;
> +				igt_info("Removed plane %d\n", i);
> +			}
> +			else {
> +				parms[i].fb = sprite_fb;
> +				parms[i].mask = 1 << 2;
> +				iter_mask |= 1 << 2;
> +				overlays[n_overlays++] = i;
> +			}
>  		}
>  	}
>  
> @@ -278,16 +304,13 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
>  	 * Pre gen9 not all sizes are supported, find the biggest possible
>  	 * size that can be enabled on all sprite planes.
>  	 */
> -retry:
>  	prev_w = sprite_width = cursor_width;
>  	prev_h = sprite_height = cursor_height;
>  
>  	max_sprite_width = (sprite_width == mode->hdisplay);
>  	max_sprite_height = (sprite_height == mode->vdisplay);
>  
> -	while (1) {
> -		int ret;
> -
> +	while (!max_sprite_width && !max_sprite_height) {
>  		set_sprite_wh(display, pipe, parms, sprite_fb,
>  			      alpha, sprite_width, sprite_height);
>  
> @@ -295,54 +318,49 @@ retry:
>  		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));
>  
> -		if (is_atomic_check_plane_size_errno(ret)) {
> -			if (cursor_width == sprite_width &&
> -			    cursor_height == sprite_height) {
> -				igt_assert_f(alpha,
> -					      "Cannot configure the test with all sprite planes enabled\n");
> -
> -				/* retry once with XRGB format. */
> -				alpha = false;
> -				goto retry;
> -			}
> -
> -			sprite_width = prev_w;
> -			sprite_height = prev_h;
> -
> -			if (max_sprite_width && max_sprite_height) {
> -				set_sprite_wh(display, pipe, parms, sprite_fb,
> -					      alpha, sprite_width, sprite_height);
> -				break;
> -			}
> -
> -			if (!max_sprite_width)
> -				max_sprite_width = true;
> -			else
> -				max_sprite_height = true;
> -		} else {
> +		if (!is_atomic_check_plane_size_errno(ret)) {
>  			prev_w = sprite_width;
>  			prev_h = sprite_height;
> -		}
> -
> -		if (!max_sprite_width) {
> -			sprite_width *= 2;
> -
> +    
> +			sprite_width *= max_sprite_width ? 1 : 2;
>  			if (sprite_width >= mode->hdisplay) {
>  				max_sprite_width = true;
> -
>  				sprite_width = mode->hdisplay;
>  			}
> -		} else if (!max_sprite_height) {
> -			sprite_height *= 2;
>  
> +			sprite_height *= max_sprite_height ? 1 : 2;
>  			if (sprite_height >= mode->vdisplay) {
>  				max_sprite_height = true;
> -
>  				sprite_height = mode->vdisplay;
>  			}
> -		} else
> -			/* Max sized sprites for all! */
> -			break;
> +			continue;
> +		}
> +
> +		if (cursor_width == sprite_width &&
> +		    cursor_height == sprite_height) {
> +			igt_assert_f(retries > 0,
> +				      "Cannot configure the test with all sprite planes enabled\n");
> +			--retries;
> +			/* retry once with XRGB format. */
> +			if (alpha) {
> +				alpha = false;
> +				igt_info("Removed alpha\n");
> +			}
> +			else {
> +				igt_assert_f(n_planes > 1, "No planes left to proceed with!");
> +				n_planes--;
> +				igt_info("Reduced available planes to %d\n", n_planes);
> +			}
> +			goto retry;

You've removed the alpha fallback? It should be removed, now that igt_plane_has_format_mod exists;
even so, I would do it in a separate preparation patch, otherwise earlier platforms may start failing.

We could remove 1 plane at a time in this block here, and then do goto retry; to the place where it was before
the patch. I think this would remove the need for a lot of double work in the early part of setup_parms, and
makes the test slightly more deterministic. We would for example remove plane 5 first, and then not enable it
again until the next subtest. And then disable more and more on top of that, instead of retrying completely.

You don't even need to loop more than once, when encountering an enabled overlay plane, set
if (rand() % (++planes_encountered_so_far)) plane_to_remove = plane;
and disable the selected plane at the end.

This way plane 1 has a 100% chance of being removed, if we encounter plane 2 it's 50/50 for both, plane 3 has 1/3
chance of being removed, 2/3 for one of the previous, etc.

~Maarten

> +		}
> +
> +		sprite_width = prev_w;
> +		sprite_height = prev_h;
> +
> +		if (!max_sprite_width)
> +			max_sprite_width = true;
> +		else
> +			max_sprite_height = true;
>  	}
>  
>  	igt_info("Running test on pipe %s with resolution %dx%d and sprite size %dx%d alpha %i\n",
> @@ -463,7 +481,6 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>  
>  	if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
>  		igt_output_set_pipe(output, PIPE_NONE);
> -
>  		igt_display_commit2(display, COMMIT_ATOMIC);
>  
>  		igt_output_set_pipe(output, pipe);
> @@ -525,8 +542,10 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>  		}
>  
>  		/* force planes to be part of commit */
> -		for_each_plane_on_pipe(display, pipe, plane)
> -			igt_plane_set_position(plane, 0, 0);
> +		for_each_plane_on_pipe(display, pipe, plane) {
> +			if (parms[plane->index].mask)
> +				igt_plane_set_position(plane, 0, 0);
> +		}
>  
>  		igt_display_commit2(display, COMMIT_ATOMIC);
>  


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

Re: [igt-dev] [PATCH i-g-t v6 2/2] igt/tests/kms_atomic_transition: Tolerate if can't have all planes

[Lisovskiy, Stanislav]

On Tue, 2019-04-09 at 13:23 +0200, Maarten Lankhorst wrote:
> Op 08-04-2019 om 12:18 schreef Stanislav Lisovskiy:
> > With some upcoming changes i915 might not allow
> > all sprite planes enabled, depending on available
> > bandwidth limitation. Thus the test need to decrement
> > amount of planes and try again, instead of panicking.
> > 
> > v2: Removed excessive nested conditions, making code a bit
> >     more readable(hopefully).
> > 
> > v3: Stopped using global n_planes variable as it might cause
> >     resources being unreleased.
> >     Using now parms[i].mask as a way to determine if plane
> >     has to be included into commit.
> > 
> > v4: Removed unneeded n_overlays initialization.
> > 
> > v5: Randomize which of sprite planes to remove if hitting
> >     resource limits.
> > 
> > v6: Replace igt_warn with igt_info, to make IGT tests happier.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  tests/kms_atomic_transition.c | 123 ++++++++++++++++++++--------
> > ------
> >  1 file changed, 71 insertions(+), 52 deletions(-)
> > 


> > +
> > +		if (cursor_width == sprite_width &&
> > +		    cursor_height == sprite_height) {
> > +			igt_assert_f(retries > 0,
> > +				      "Cannot configure the test with
> > all sprite planes enabled\n");
> > +			--retries;
> > +			/* retry once with XRGB format. */
> > +			if (alpha) {
> > +				alpha = false;
> > +				igt_info("Removed alpha\n");
> > +			}
> > +			else {
> > +				igt_assert_f(n_planes > 1, "No planes
> > left to proceed with!");
> > +				n_planes--;
> > +				igt_info("Reduced available planes to
> > %d\n", n_planes);
> > +			}
> > +			goto retry;
> 
> You've removed the alpha fallback? It should be removed, now that
> igt_plane_has_format_mod exists;
> even so, I would do it in a separate preparation patch, otherwise
> earlier platforms may start failing.

No I haven't. I made it so, that it first tries to remove alpha and 
only if this was not enough(alpha == false), start to play with planes.
I agree igt_plane_has_format_mod could be helpful to save at least
one
iteration here.

> 
> We could remove 1 plane at a time in this block here, and then do
> goto retry; to the place where it was before
> the patch. I think this would remove the need for a lot of double
> work in the early part of setup_parms, and
> makes the test slightly more deterministic. We would for example
> remove plane 5 first, and then not enable it
> again until the next subtest. And then disable more and more on top
> of that, instead of retrying completely.

I agree, this could be great idea. So that I actually don't even
go back but just disable one random plain and keep going. 

> 
> You don't even need to loop more than once, when encountering an
> enabled overlay plane, set
> if (rand() % (++planes_encountered_so_far)) plane_to_remove = plane;
> and disable the selected plane at the end.
> 
> This way plane 1 has a 100% chance of being removed, if we encounter
> plane 2 it's 50/50 for both, plane 3 has 1/3
> chance of being removed, 2/3 for one of the previous, etc.

Also true! Will do that.

-Stanislav

> 
> ~Maarten
> 
> > +		}
> > +
> > +		sprite_width = prev_w;
> > +		sprite_height = prev_h;
> > +
> > +		if (!max_sprite_width)
> > +			max_sprite_width = true;
> > +		else
> > +			max_sprite_height = true;
> >  	}
> >  
> >  	igt_info("Running test on pipe %s with resolution %dx%d and
> > sprite size %dx%d alpha %i\n",
> > @@ -463,7 +481,6 @@ run_transition_test(igt_display_t *display,
> > enum pipe pipe, igt_output_t *output
> >  
> >  	if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
> >  		igt_output_set_pipe(output, PIPE_NONE);
> > -
> >  		igt_display_commit2(display, COMMIT_ATOMIC);
> >  
> >  		igt_output_set_pipe(output, pipe);
> > @@ -525,8 +542,10 @@ run_transition_test(igt_display_t *display,
> > enum pipe pipe, igt_output_t *output
> >  		}
> >  
> >  		/* force planes to be part of commit */
> > -		for_each_plane_on_pipe(display, pipe, plane)
> > -			igt_plane_set_position(plane, 0, 0);
> > +		for_each_plane_on_pipe(display, pipe, plane) {
> > +			if (parms[plane->index].mask)
> > +				igt_plane_set_position(plane, 0, 0);
> > +		}
> >  
> >  		igt_display_commit2(display, COMMIT_ATOMIC);
> >  
> 
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for kms_atomic_transition improvements (rev12)

[Patchwork]

== Series Details ==

Series: kms_atomic_transition improvements (rev12)
URL   : https://patchwork.freedesktop.org/series/58728/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5887 -> IGTPW_2809
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2809 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2809, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58728/revisions/12/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-skl-6600u:       PASS -> FAIL

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@basic-read:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_selftest@live_hangcheck:
    - fi-bxt-dsi:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-c:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - fi-glk-dsi:         PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-ilk-650:         INCOMPLETE [fdo#109723] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u3}:        FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-y:           DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108569]

  
  {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#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109723]: https://bugs.freedesktop.org/show_bug.cgi?id=109723
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (50 -> 44)
------------------------------

  Additional (1): fi-snb-2600 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-elk-e7500 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4932 -> IGTPW_2809

  CI_DRM_5887: 360bc4546928ec1c9324ddd3991a01e89476a16c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2809: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2809/
  IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[igt-dev] ✗ Fi.CI.BAT: failure for kms_atomic_transition improvements (rev13)

[Patchwork]

== Series Details ==

Series: kms_atomic_transition improvements (rev13)
URL   : https://patchwork.freedesktop.org/series/58728/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5887 -> IGTPW_2810
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2810 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2810, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58728/revisions/13/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-skl-6600u:       PASS -> FAIL

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@basic-read:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_selftest@live_hangcheck:
    - fi-bxt-dsi:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-c:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  
#### Possible fixes ####

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u3}:        FAIL [fdo#103167] -> PASS
    - fi-icl-u2:          FAIL [fdo#103167] -> 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#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210


Participating hosts (50 -> 44)
------------------------------

  Additional (1): fi-snb-2600 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-dsi fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4932 -> IGTPW_2810

  CI_DRM_5887: 360bc4546928ec1c9324ddd3991a01e89476a16c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2810: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2810/
  IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[igt-dev] ✓ Fi.CI.BAT: success for kms_atomic_transition improvements (rev14)

[Patchwork]

== Series Details ==

Series: kms_atomic_transition improvements (rev14)
URL   : https://patchwork.freedesktop.org/series/58728/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5887 -> IGTPW_2811
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58728/revisions/14/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-y:           PASS -> INCOMPLETE [fdo#110246]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]

  * igt@gem_workarounds@basic-read:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_selftest@live_hangcheck:
    - fi-bxt-dsi:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-c:
    - fi-snb-2600:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-ilk-650:         INCOMPLETE [fdo#109723] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u3}:        FAIL [fdo#103167] -> 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#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109723]: https://bugs.freedesktop.org/show_bug.cgi?id=109723
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246


Participating hosts (50 -> 41)
------------------------------

  Additional (1): fi-snb-2600 
  Missing    (10): fi-kbl-soraka fi-ilk-m540 fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510 fi-bdw-samus fi-byt-clapper fi-skl-6600u 


Build changes
-------------

    * IGT: IGT_4932 -> IGTPW_2811

  CI_DRM_5887: 360bc4546928ec1c9324ddd3991a01e89476a16c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2811: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2811/
  IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for kms_atomic_transition improvements (rev14)

[Patchwork]

== Series Details ==

Series: kms_atomic_transition improvements (rev14)
URL   : https://patchwork.freedesktop.org/series/58728/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5887_full -> IGTPW_2811_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58728/revisions/14/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@gem_tiled_swapping@non-threaded:
    - shard-snb:          PASS -> FAIL [fdo#108686]

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_atomic_transition@6x-modeset-transitions:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-apl:          PASS -> FAIL [fdo#103167]
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +12

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +29

  * igt@kms_frontbuffer_tracking@fbcpsr-shrfb-scaledprimary:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +12

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-b-ts-continuation-modeset:
    - shard-apl:          PASS -> FAIL [fdo#104894]

  * igt@kms_vblank@pipe-c-ts-continuation-modeset:
    - shard-kbl:          PASS -> FAIL [fdo#104894] +1

  * igt@prime_nv_api@i915_self_import:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +7

  
#### Possible fixes ####

  * igt@kms_color@pipe-a-ctm-max:
    - shard-kbl:          FAIL [fdo#108147] -> PASS
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-hsw:          FAIL [fdo#103355] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-plflip-blt:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1
    - shard-kbl:          FAIL [fdo#104894] -> PASS +1

  
#### Warnings ####

  * igt@kms_psr@cursor_render:
    - shard-snb:          SKIP [fdo#109271] -> INCOMPLETE [fdo#105411]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [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#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_4932 -> IGTPW_2811
    * Piglit: piglit_4509 -> None

  CI_DRM_5887: 360bc4546928ec1c9324ddd3991a01e89476a16c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2811: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2811/
  IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ 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_2811/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev