public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Souza, Jose" <jose.souza@intel.com>
To: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>,
	"Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
Cc: "Syrjala, Ville" <ville.syrjala@intel.com>,
	"Peres, Martin" <martin.peres@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t v3 2/3] igt/tests/kms_atomic_transition: Tolerate if can't have all planes
Date: Tue, 2 Apr 2019 20:42:55 +0000	[thread overview]
Message-ID: <07ec8a1e0346dd5cf0683e8e592e380b32566154.camel@intel.com> (raw)
In-Reply-To: <20190401112305.6832-3-stanislav.lisovskiy@intel.com>


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

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@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.



[-- 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

  reply	other threads:[~2019-04-02 20:42 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-01 11:23 [igt-dev] [PATCH i-g-t v3 0/3] kms_atomic_transition improvements Stanislav Lisovskiy
2019-04-01 11:23 ` [igt-dev] [PATCH i-g-t v3 1/3] igt/tests/kms_atomic_transition: Skip transition, if no changes done Stanislav Lisovskiy
2019-04-01 23:04   ` Souza, Jose
2019-04-02  6:59     ` Lisovskiy, Stanislav
2019-04-01 11:23 ` [igt-dev] [PATCH i-g-t v3 2/3] igt/tests/kms_atomic_transition: Tolerate if can't have all planes Stanislav Lisovskiy
2019-04-02 20:42   ` Souza, Jose [this message]
2019-04-03  8:08     ` Lisovskiy, Stanislav
2019-04-01 11:23 ` [igt-dev] [PATCH i-g-t v3 3/3] igt/tests/kms_atomic_transition: Remove redundant code Stanislav Lisovskiy
2019-04-01 12:07 ` [igt-dev] ✗ Fi.CI.BAT: failure for kms_atomic_transition improvements (rev3) Patchwork
2019-04-01 13:42 ` [igt-dev] ✗ Fi.CI.BAT: failure for kms_atomic_transition improvements (rev4) Patchwork
2019-04-02  8:59 ` [igt-dev] ✓ Fi.CI.BAT: success for kms_atomic_transition improvements (rev5) Patchwork
2019-04-02 13:50 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07ec8a1e0346dd5cf0683e8e592e380b32566154.camel@intel.com \
    --to=jose.souza@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=martin.peres@intel.com \
    --cc=stanislav.lisovskiy@intel.com \
    --cc=ville.syrjala@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox