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

["Souza, Jose" <jose.souza@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7528 bytes --]

On Thu, 2019-04-04 at 15:13 +0300, Stanislav Lisovskiy wrote:
> 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.
> 
> 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..e6d55ca0 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)) {

This will not guarantee that x(planes_to_remove) planes will be
removed.

> +				parms[i].fb = NULL;
> +				parms[i].mask = 0;
> +				planes_to_remove--;
> +				igt_warn("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_warn("Removed alpha\n");
> +			}
> +			else {
> +				igt_assert_f(n_planes > 1, "No planes
> left to proceed with!");
> +				n_planes--;
> +				igt_warn("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);
>  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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