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

[Souza, Jose]

On Mon, 2019-04-01 at 14:23 +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.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
>  tests/kms_atomic_transition.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/kms_atomic_transition.c
> b/tests/kms_atomic_transition.c
> index 638fe17e..e4dee7e2 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -214,7 +214,8 @@ static void setup_parms(igt_display_t *display,
> enum pipe pipe,
>  	uint32_t n_planes = display->pipes[pipe].n_planes;
>  	uint32_t n_overlays = 0, overlays[n_planes];

Nitpick: you don't need to initialize n_overlays here as you do right
after the retry label.

>  	igt_plane_t *plane;
> -	uint32_t iter_mask = 3;
> +	uint32_t iter_mask;
> +	int retries = n_planes - 1;
>  
>  	do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH,
> &cursor_width));
>  	if (cursor_width >= mode->hdisplay)
> @@ -224,6 +225,10 @@ 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;
> +
>  	for_each_plane_on_pipe(display, pipe, plane) {
>  		int i = plane->index;
>  
> @@ -278,7 +283,6 @@ 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;
>  
> @@ -298,12 +302,22 @@ retry:
>  		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;
> +				if (--retries >= 0) {

The 7 first loops it is going to this, on the 8 it will try for the
first time to reduce the number of planes.
Also this way we will only test without alpha in platforms with a high
number of planes.

> +					/* retry once with XRGB format.
> */
> +					if (alpha) {
> +						alpha = false;
> +					}
> +					else if (display-
> >pipes[pipe].n_planes > 0) {
> +						display-
> >pipes[pipe].n_planes--;

When you do this wm_setup_plane() will iterate in one less plane, and
not removing framebuffer from that plane, this is probably why you
needed the first patch.

> +						igt_warn("Reduced
> available planes to %d\n",
> +							    display-
> >pipes[pipe].n_planes);
> +					}
> +					n_planes = display-
> >pipes[pipe].n_planes;
> +					igt_assert_f(n_planes > 0, "No
> planes left to proceed with!");
> +					goto retry;
> +				}
> +				igt_assert_f(retries > 0,
> +				      "Cannot configure the test with
> all sprite planes enabled\n");
>  			}
>  
>  			sprite_width = prev_w;


My suggestion:

Right after "max_sprite_height = (sprite_height == mode->vdisplay);"
test if all planes support alpha by using:
igt_plane_has_format_mod(plane, plane_format, plane_modifier).

Then write a new loop testing the maximum number of planes supported
and remember to remove the framebuffer from planes.

And finally run this final loop that will find the max size for sprite
planes.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190402/a4acc5f3/attachment.sig>