[igt-dev] [PATCH i-g-t v1] igt/tests: kms_atomic_transition improvements.
Daniel Vetter
daniel at ffwll.ch
Fri Mar 29 08:55:37 UTC 2019
On Thu, Mar 28, 2019 at 05:17:28PM +0200, Stanislav Lisovskiy wrote:
> Fixes:
> - 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.
> - Removed unneeded overlays variable, changed the parms
> initialization cycle that it initializes also iter_mask
> and parms[i].mask at the same time, fullfiling the same
> requirements(i.e always use primary, cursor and one sprite
> plane, for the rest parms[i].mask is randomized).
> - While fixing used amount of planes, discovered that if
> wm_setup_plane is called with 0 planes(might happen during
> main testing cycle, as parms[i].mask can be 0 due to randomization)
> 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 need to skip this step.
Sounds like 3 patches crammed into one. Can you pls split up?
Thanks, Daniel
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
> tests/kms_atomic_transition.c | 85 +++++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 38 deletions(-)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 18f73317..a75e98b5 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)
> @@ -206,9 +212,10 @@ 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;
> 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)
> @@ -217,6 +224,9 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
> do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height));
> if (cursor_height >= mode->vdisplay)
> cursor_height = mode->vdisplay;
> +retry:
> + n_overlays = 0;
> + iter_mask = 0;
>
> for_each_plane_on_pipe(display, pipe, plane) {
> int i = plane->index;
> @@ -225,36 +235,21 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
> parms[i].fb = primary_fb;
> parms[i].width = mode->hdisplay;
> parms[i].height = mode->vdisplay;
> - parms[i].mask = 1 << 0;
> } else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> parms[i].fb = argb_fb;
> parms[i].width = cursor_width;
> 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;
> + n_overlays++;
> }
> - }
> -
> - if (n_overlays >= 2) {
> - uint32_t i;
> -
> - /*
> - * Create 2 groups for overlays, make sure 1 plane is put
> - * in each then spread the rest out.
> - */
> - iter_mask |= 1 << 3;
> - parms[overlays[n_overlays - 1]].mask = 1 << 3;
> -
> - for (i = 1; i < n_overlays - 1; i++) {
> - int val = hars_petruska_f54_1_random_unsafe_max(2);
> -
> - parms[overlays[i]].mask = 1 << (2 + val);
> + iter_mask |= 1 << i;
> + if (i <= 2) {
> + /* always leave one plane as in original algorithm */
> + parms[i].mask = 1 << i;
> + }
> + else {
> + parms[i].mask = hars_petruska_f54_1_random_unsafe_max(2) << i;
> }
> }
>
> @@ -272,7 +267,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;
>
> @@ -292,12 +286,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) {
> + /* retry once with XRGB format. */
> + if (alpha) {
> + alpha = false;
> + }
> + else if (display->pipes[pipe].n_planes > 0) {
> + display->pipes[pipe].n_planes--;
> + igt_info("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;
> @@ -544,7 +548,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 +557,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 +574,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 +583,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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the igt-dev
mailing list