Re: [igt-dev] [PATCH i-g-t v2] igt/tests: Fix error checking in kms_atomic_transition

Lisovskiy, Stanislav via igt-dev
2019-02-15T16:54:23Z

On Fri, 2019-02-15 at 08:09 -0800, Summers, Stuart wrote:
> On Fri, 2019-02-15 at 17:28 +0200, Stanislav Lisovskiy via igt-dev
> wrote:
> > From: Stanislav Lisovskiy <stanislav.lisovskiy@gmail.com>
> >
> > There is no guarantee that error return value will be
> > always EINVAL, made a check more general as it can be
> > ERANGE, ENOSPC, EINVAL and probably others, which all
> > mean the same in context of this test case: i.e this sprite
> > size is not valid.
> >
> > v2: Added macro to make check look a bit nicer.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109225
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> > tests/kms_atomic_transition.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/kms_atomic_transition.c
> > b/tests/kms_atomic_transition.c
> > index 12bafe87..32bff1e9 100644
> > --- a/tests/kms_atomic_transition.c
> > +++ b/tests/kms_atomic_transition.c
> > @@ -187,6 +187,11 @@ static void set_sprite_wh(igt_display_t
> > *display, enum pipe pipe,
> > LOCAL_DRM_FORMAT_MOD_NONE, sprite_fb);
> > }
> >
> > +#define is_atomic_check_failure_errno(errno) \
> > + (errno == -EINVAL || \
> > + errno == -ERANGE || \
> > + errno == -ENOSPC)
> > +
> > static void setup_parms(igt_display_t *display, enum pipe pipe,
> > const drmModeModeInfo *mode,
> > struct igt_fb *primary_fb,
> > @@ -252,7 +257,7 @@ retry:
> > wm_setup_plane(display, pipe, (1 << n_planes) - 1,
> > parms, false);
> > ret = igt_display_try_commit_atomic(display,
> > DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> >
> > - if (ret == -EINVAL) {
> > + if (is_atomic_check_failure_errno(ret)) {
> > if (cursor_width == sprite_width &&
> > cursor_height == sprite_height) {
> > igt_assert_f(alpha,
> > @@ -391,6 +396,7 @@ run_transition_test(igt_display_t *display,
> > enum
> > pipe pipe, igt_output_t *output
> > igt_plane_t *plane;
> > igt_pipe_t *pipe_obj = &display->pipes[pipe];
> > uint32_t iter_max = 1 << pipe_obj->n_planes, i;
> > + iter_max = 1 << 3;
>
> Why this added assignment?

Oooh, thank you so much. I've put it here, when I was debugging and
didn't remove it!..
>
> > struct plane_parms parms[pipe_obj->n_planes];
> > unsigned flags = 0;
> > int ret;
> > @@ -442,7 +448,7 @@ run_transition_test(igt_display_t *display,
> > enum
> > pipe pipe, igt_output_t *output
> > igt_pipe_request_out_fence(pipe_obj);
> >
> > ret = igt_display_try_commit_atomic(display,
> > DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > - if (ret != -EINVAL || pipe_obj->n_planes < 3)
> > + if (!is_atomic_check_failure_errno(ret) ||
> > pipe_obj-
> > > n_planes < 3)
> >
> > break;
> >
> > ret = 0;
--
Best Regards,

Lisovskiy Stanislav