[igt-dev] [PATCH i-g-t v5 2/2] igt/tests/kms_atomic_transition: Tolerate if can't have all planes
Lisovskiy, Stanislav
stanislav.lisovskiy at intel.com
Fri Apr 5 06:57:34 UTC 2019
On Thu, 2019-04-04 at 17:59 -0700, Souza, Jose wrote:
> 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 at 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.
There are two cases:
1) If planes to remove is less than planes left, it means that we
still have some freedom for randomization which planes has to be
removed. Lets say we need to remove 3, but we have for 5, then we can
randomize which 3 out of 5 we can remove. So in that case we use
remove variable as a random factor to decide whether plane has to be
removed or not.
2) If planes to remove is equal(or more) than planes left, it means
that we need to remove anyway. Lets say we need to remove 3 and we
have 3. Obviously we need to remove all of those.
If you still see that there is a flaw in this algorithm, can you please
explain?
>
> > + 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);
> >
More information about the igt-dev
mailing list