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

From: "Summers, Stuart" <stuart.summers@intel.com>
To: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>,
	"Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
Cc: "norkernel@freedesktop.org" <norkernel@freedesktop.org>,
	"Syrjala, Ville" <ville.syrjala@intel.com>,
	"Heikkila, Juha-pekka" <juha-pekka.heikkila@intel.com>,
	"Peres, Martin" <martin.peres@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t v4] igt/tests: Fix error checking in kms_atomic_transition
Date: Tue, 19 Feb 2019 21:05:12 +0000	[thread overview]
Message-ID: <690c1112b0fd97f4810cced16d442d16ca75809b.camel@intel.com> (raw)
In-Reply-To: <20190219093800.4312-1-stanislav.lisovskiy@intel.com>


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

On Tue, 2019-02-19 at 11:38 +0200, Stanislav Lisovskiy 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.
> v3: Removed redundant debug line.
> v4: Added assertion if error is not EINVAL as expected,
>     other errors except EINVAL are considered now a failures.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109225
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  tests/kms_atomic_transition.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_atomic_transition.c
> b/tests/kms_atomic_transition.c
> index 12bafe87..e1a2d4a0 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -187,6 +187,12 @@ 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 != 0)
> +
> +#define is_atomic_check_plane_size_errno(errno) \
> +		(errno == -EINVAL)
> +
>  static void setup_parms(igt_display_t *display, enum pipe pipe,
>  			const drmModeModeInfo *mode,
>  			struct igt_fb *primary_fb,
> @@ -251,8 +257,9 @@ 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);
> +		igt_assert(!is_atomic_check_failure_errno(ret));
>  
> -		if (ret == -EINVAL) {
> +		if (is_atomic_check_plane_size_errno(ret)) {
>  			if (cursor_width == sprite_width &&
>  			    cursor_height == sprite_height) {
>  				igt_assert_f(alpha,
> @@ -442,7 +449,9 @@ 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)
> +		igt_assert(!is_atomic_check_failure_errno(ret));
> +
> +		if (!is_atomic_check_plane_size_errno(ret) || pipe_obj-
> >n_planes < 3)
>  			break;
>  
>  		ret = 0;

LGTM

Reviewed-by: Stuart Summers <stuart.summers@intel.com>

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