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

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

[Stanislav Lisovskiy via igt-dev]

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;
 	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;
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Summers, Stuart via igt-dev]

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

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?

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

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 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

[igt-dev] ✗ Fi.CI.BAT: failure for igt/tests: Fix error checking in kms_atomic_transition (rev2)

[Patchwork]

== Series Details ==

Series: igt/tests: Fix error checking in kms_atomic_transition (rev2)
URL   : https://patchwork.freedesktop.org/series/56617/
State : failure

== Summary ==

Applying: igt/tests: Fix error checking in kms_atomic_transition
Using index info to reconstruct a base tree...
M	tests/kms_atomic_transition.c
Falling back to patching base and 3-way merge...
Auto-merging tests/kms_atomic_transition.c
CONFLICT (content): Merge conflict in tests/kms_atomic_transition.c
Patch failed at 0001 igt/tests: Fix error checking in kms_atomic_transition
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Lisovskiy, Stanislav via igt-dev]

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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev