[igt-dev] [PATCH i-g-t v7 2/2] igt/tests/kms_atomic_transition: Tolerate if can't have all planes
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Tue Apr 9 15:17:05 UTC 2019
Op 09-04-2019 om 16:08 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.
>
> v7: Removed unneeded retry logic, made plane random removal simplier.
> Great thanks to Maarten Lankhorst for suggestions.
What I mean is replacing alpha, at this snippet:
parms[i].fb = sprite_fb;
Add:
alpha = igt_plane_has_format_mod(plane, DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE))
This should get rid of the retry logic for alpha, and should probably be done as a preparation patch to simplify things.
I also think we should be able to enable at least the primary, cursor and a single overlay plane. If we can't do that we fail hard..
Because of this I don't think we should handle removing primary or cursor plane, we should probably fail in that case.
~Maarten
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
> tests/kms_atomic_transition.c | 133 ++++++++++++++++++++--------------
> 1 file changed, 79 insertions(+), 54 deletions(-)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 638fe17e..4b530e56 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -184,6 +184,9 @@ static void set_sprite_wh(igt_display_t *display, enum pipe pipe,
> plane->type == DRM_PLANE_TYPE_CURSOR)
> continue;
>
> + if (!parms[i].mask)
> + continue;
> +
> parms[i].width = w;
> parms[i].height = h;
> }
> @@ -212,9 +215,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];
> - igt_plane_t *plane;
> - uint32_t iter_mask = 3;
> + uint32_t n_overlays, overlays[n_planes];
> + igt_plane_t *plane, *primary, *cursor;
> + uint32_t iter_mask;
> + int ret = 0;
>
> do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
> if (cursor_width >= mode->hdisplay)
> @@ -224,6 +228,9 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
> if (cursor_height >= mode->vdisplay)
> cursor_height = mode->vdisplay;
>
> + n_overlays = 0;
> + iter_mask = 3;
> +
> for_each_plane_on_pipe(display, pipe, plane) {
> int i = plane->index;
>
> @@ -232,17 +239,17 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
> parms[i].width = mode->hdisplay;
> parms[i].height = mode->vdisplay;
> parms[i].mask = 1 << 0;
> + primary = plane;
> } 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;
> + cursor = plane;
> } else {
> parms[i].fb = sprite_fb;
> parms[i].mask = 1 << 2;
> -
> iter_mask |= 1 << 2;
> -
> overlays[n_overlays++] = i;
> }
> }
> @@ -278,16 +285,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,56 +299,78 @@ 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) {
> + /* retry once with XRGB format. */
> + if (alpha) {
> + alpha = false;
> + igt_info("Removed alpha\n");
> + }
> + else {
> + igt_plane_t *removed_plane = NULL;
> + igt_assert_f(n_planes > 0, "No planes left to proceed with!");
> + if (n_overlays > 0) {
> + uint32_t plane_to_remove = hars_petruska_f54_1_random_unsafe_max(n_overlays);
> + removed_plane = &display->pipes[pipe].planes[overlays[plane_to_remove]];
> + igt_plane_set_fb(removed_plane, NULL);
> + while (plane_to_remove < (n_overlays - 1)) {
> + overlays[plane_to_remove] = overlays[plane_to_remove + 1];
> + plane_to_remove++;
> + }
> + n_overlays--;
> + }
> + else {
> + if (primary) {
> + removed_plane = primary;
> + igt_plane_set_fb(primary, NULL);
> + primary = NULL;
> + }
> + else if (cursor) {
> + removed_plane = cursor;
> + igt_plane_set_fb(cursor, NULL);
> + cursor = NULL;
> + }
> + }
> + if (removed_plane) {
> + parms[removed_plane->index].mask = 0;
> + igt_info("Removed plane %d\n", removed_plane->index);
> + }
> + n_planes--;
> + igt_info("Reduced available planes to %d\n", n_planes);
> + }
> + continue;
> + }
> +
> + sprite_width = prev_w;
> + sprite_height = prev_h;
> +
> + if (!max_sprite_width)
> + max_sprite_width = true;
> + else
> + max_sprite_height = true;
> }
>
> + set_sprite_wh(display, pipe, parms, sprite_fb,
> + alpha, sprite_width, sprite_height);
> +
> igt_info("Running test on pipe %s with resolution %dx%d and sprite size %dx%d alpha %i\n",
> kmstest_pipe_name(pipe), mode->hdisplay, mode->vdisplay,
> sprite_width, sprite_height, alpha);
> @@ -436,7 +462,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> uint32_t iter_max, i;
> struct plane_parms parms[pipe_obj->n_planes];
> unsigned flags = 0;
> - int ret;
> + int ret = 0;
>
> if (fencing)
> prepare_fencing(display, pipe);
> @@ -463,7 +489,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);
> @@ -479,7 +504,6 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> * planes to fix this
> */
> while (1) {
> - wm_setup_plane(display, pipe, iter_max - 1, parms, false);
>
> if (fencing)
> igt_pipe_request_out_fence(pipe_obj);
> @@ -498,9 +522,6 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> plane->type == DRM_PLANE_TYPE_CURSOR)
> continue;
>
> - if (parms[i].width <= 512)
> - continue;
> -
> parms[i].width /= 2;
> ret = 1;
> igt_info("Reducing sprite %i to %ux%u\n", i - 1, parms[i].width, parms[i].height);
> @@ -511,6 +532,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> igt_skip("Cannot run tests without proper size sprite planes\n");
> }
>
> + wm_setup_plane(display, pipe, iter_max - 1, parms, false);
> +
> igt_display_commit2(display, COMMIT_ATOMIC);
>
> if (type == TRANSITION_AFTER_FREE) {
> @@ -525,8 +548,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);
>
More information about the igt-dev
mailing list