Re: [PATCH i-g-t] igt/kms-crc-rotation: Fix flip subtests to follow kms guidelines.

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jan 09, 2018 at 10:08:29PM -0800, Anusha Srivatsa wrote:
> Fix flip subtest that used plaform names
> in igt_require() instead of testing if rotation
> property is supported on given combination of
> rotation/flips.
> 
> Suggested-by: Daniel Vetter
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  tests/kms_rotation_crc.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 799cf11..63f3b5b 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -394,9 +394,6 @@ static void __test_plane_rotation(data_t *data, int plane_type, bool test_bad_fo
>  		igt_plane_t *plane;
>  		int i;
>  
> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> -			continue;
> -
>  		igt_output_set_pipe(output, pipe);
>  
>  		plane = igt_output_get_plane_type(output, plane_type);
> @@ -837,9 +834,7 @@ igt_main
>  			      tiling_test_str(reflect_x->tiling),
>  			      rot_test_str(reflect_x->rot),
>  			      flip_test_str(reflect_x->flips)) {
> -			igt_require(gen >= 10 ||
> -				    (IS_CHERRYVIEW(data.devid) && reflect_x->rot == IGT_ROTATION_0
> -				     && reflect_x->tiling == LOCAL_I915_FORMAT_MOD_X_TILED));

This isn't what I meant, and I also just realized that the rotation
subtests also hardcodes this.

Instead what you need to do in both cases is to read the possible values
for the ROTATION property, and skip if not everything you need is
supported.

The other subtest check that needs to be fixed is

			igt_require(!(subtest->rot &
				    (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
				    gen >= 9);


It's convoluted, but essentially what this does is skip 90/270 rotation
tests on pre-gen9.

Part of the checks are there already, see

	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));

But this isn't complete - we only check whether the property exists, not
whether it supports all the different rotation/reflect modes we want to
test.

> +			igt_require(subtest->flips);

This check here simply skips all the tests where we don't flip, which is
probably not quite what you had in mind I think. Did you test this and
compare results before/after on a machine where this test actually runs?
-Daniel

>  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
>  			data.override_tiling = reflect_x->tiling;
>  			data.flips = reflect_x->flips;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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