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

From: Daniel Vetter <daniel@ffwll.ch>
To: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: igt-dev@lists.freedesktop.org, juhapekka.heikkila@intel.com,
	ville.syrjala@intel.com, martin.peres@intel.com
Subject: Re: [igt-dev] [PATCH i-g-t] igt/tests: Fix error checking in kms_atomic_transition
Date: Wed, 13 Feb 2019 21:41:31 +0100	[thread overview]
Message-ID: <20190213204131.GM23159@phenom.ffwll.local> (raw)
In-Reply-To: <20190213161817.18758-1-stanislav.lisovskiy@intel.com>

On Wed, Feb 13, 2019 at 06:18:17PM +0200, Stanislav Lisovskiy via igt-dev wrote:
> 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.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109225
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Kernel atomic spec is pretty clear that it's either EINVAL or ERANGE, and
nothing else. In the docs we even limit to EINVAL (scroll down to
atomic_check):

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html?highlight=drm_mode_config_funcs#c.drm_mode_config_funcs

Would be great if you can submit a kernel patch to add the ERANGE. And
change this one here to only accept ERANGE and EINVAL as "sprite doesn't
work".
-Daniel

> ---
>  tests/kms_atomic_transition.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index ec5d25de..58bf6280 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -282,7 +282,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 (ret != 0) {
>  			if (cursor_width == sprite_width &&
>  			    cursor_height == sprite_height) {
>  				igt_assert_f(alpha,
> @@ -472,7 +472,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 (ret == 0 || 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev