diff mbox

[i-g-t,v3] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases

Message ID 20180322221055.711-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sripada, Radhakrishna March 22, 2018, 10:10 p.m. UTC
From: Anusha Srivatsa <anusha.srivatsa@intel.com>

Cleanup the testcases by moving the platform checks to a single function.

The earlier version of the path is posted here [1]

v2: Make use of the property enums to get the supported rotations
v3: Move hardcodings to a single function(Ville)

[1]: https://patchwork.freedesktop.org/patch/209647/

Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Mika Kahola March 28, 2018, 8:26 a.m. UTC | #1
On Thu, 2018-03-22 at 15:10 -0700, Radhakrishna Sripada wrote:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> 
> Cleanup the testcases by moving the platform checks to a single
> function.
> 
> The earlier version of the path is posted here [1]
> 
> v2: Make use of the property enums to get the supported rotations
> v3: Move hardcodings to a single function(Ville)
> 
> [1]: https://patchwork.freedesktop.org/patch/209647/
> 
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>

Tested-by: Mika Kahola <mika.kahola@intel.com>

> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 0cd5c6e52b57..60fb9012e14e 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -43,6 +43,7 @@ typedef struct {
>  	uint32_t override_fmt;
>  	uint64_t override_tiling;
>  	int devid;
> +	int gen;
>  } data_t;
>  
>  typedef struct {
> @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
>  	igt_assert(drmHandleEvent(fd, &evctx) == 0);
>  }
>  
> +static void igt_check_rotation(data_t *data)
> +{
> +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> +		igt_require(data->gen >= 9);
> +	else if (data->rotation & IGT_REFLECT_X)
> +		igt_require(data->gen >= 10 ||
> +			    (IS_CHERRYVIEW(data->devid) && (data-
> >rotation & IGT_ROTATION_0)));
> +	else if (data->rotation & IGT_ROTATION_180)
> +		igt_require(data->gen >= 4);
> +}
> +
>  static void test_single_case(data_t *data, enum pipe pipe,
>  			     igt_output_t *output, igt_plane_t
> *plane,
>  			     enum rectangle_type rect,
> @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data,
> int plane_type, bool test_bad_form
>  
>  	igt_display_require_output(display);
>  
> +	igt_check_rotation(data);
> +
>  	for_each_pipe_with_valid_output(display, pipe, output) {
>  		igt_plane_t *plane;
>  		int i, j;
>  
> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> -			continue;
> -
>  		igt_output_set_pipe(output, pipe);
>  
>  		plane = igt_output_get_plane_type(output,
> plane_type);
> @@ -538,14 +549,13 @@ igt_main
>  	};
>  
>  	data_t data = {};
> -	int gen = 0;
>  
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
>  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
>  		data.devid = intel_get_drm_devid(data.gfx_fd);
> -		gen = intel_gen(data.devid);
> +		data.gen = intel_gen(data.devid);
>  
>  		kmstest_set_vt_graphics_mode();
>  
> @@ -558,16 +568,12 @@ igt_main
>  		igt_subtest_f("%s-rotation-%s",
>  			      plane_test_str(subtest->plane),
>  			      rot_test_str(subtest->rot)) {
> -			igt_require(!(subtest->rot &
> -				    (IGT_ROTATION_90 |
> IGT_ROTATION_270)) ||
> -				    gen >= 9);
>  			data.rotation = subtest->rot;
>  			test_plane_rotation(&data, subtest->plane,
> false);
>  		}
>  	}
>  
>  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.pos_x = 100,
>  		data.pos_y = 0;
> @@ -577,7 +583,6 @@ igt_main
>  	data.pos_y = 0;
>  
>  	igt_subtest_f("bad-pixel-format") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.override_fmt = DRM_FORMAT_RGB565;
>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> true);
> @@ -585,7 +590,6 @@ igt_main
>  	data.override_fmt = 0;
>  
>  	igt_subtest_f("bad-tiling") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.override_tiling =
> LOCAL_I915_FORMAT_MOD_X_TILED;
>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> true);
> @@ -596,9 +600,6 @@ igt_main
>  		igt_subtest_f("primary-%s-reflect-x-%s",
>  			      tiling_test_str(reflect_x->tiling),
>  			      rot_test_str(reflect_x->rot)) {
> -			igt_require(gen >= 10 ||
> -				    (IS_CHERRYVIEW(data.devid) &&
> reflect_x->rot == IGT_ROTATION_0
> -				     && reflect_x->tiling ==
> LOCAL_I915_FORMAT_MOD_X_TILED));
>  			data.rotation = (IGT_REFLECT_X | reflect_x-
> >rot);
>  			data.override_tiling = reflect_x->tiling;
>  			test_plane_rotation(&data,
> DRM_PLANE_TYPE_PRIMARY, false);
> @@ -613,7 +614,7 @@ igt_main
>  		enum pipe pipe;
>  		igt_output_t *output;
>  
> -		igt_require(gen >= 9);
> +		igt_require(data.gen >= 9);
>  		igt_display_require_output(&data.display);
>  
>  		for_each_pipe_with_valid_output(&data.display, pipe,
> output) {
Maarten Lankhorst March 28, 2018, 8:29 a.m. UTC | #2
Op 22-03-18 om 23:10 schreef Radhakrishna Sripada:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>
> Cleanup the testcases by moving the platform checks to a single function.
>
> The earlier version of the path is posted here [1]
>
> v2: Make use of the property enums to get the supported rotations
> v3: Move hardcodings to a single function(Ville)
>
> [1]: https://patchwork.freedesktop.org/patch/209647/
>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 0cd5c6e52b57..60fb9012e14e 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -43,6 +43,7 @@ typedef struct {
>  	uint32_t override_fmt;
>  	uint64_t override_tiling;
>  	int devid;
> +	int gen;
>  } data_t;
>  
>  typedef struct {
> @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
>  	igt_assert(drmHandleEvent(fd, &evctx) == 0);
>  }
>  
> +static void igt_check_rotation(data_t *data)
> +{
> +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> +		igt_require(data->gen >= 9);
> +	else if (data->rotation & IGT_REFLECT_X)
> +		igt_require(data->gen >= 10 ||
> +			    (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_ROTATION_0)));
> +	else if (data->rotation & IGT_ROTATION_180)
> +		igt_require(data->gen >= 4);
> +}
This still won't work as intended on CHV as it only has reflection on PIPE_B, for X tiled fb's.


>  static void test_single_case(data_t *data, enum pipe pipe,
>  			     igt_output_t *output, igt_plane_t *plane,
>  			     enum rectangle_type rect,
> @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  
>  	igt_display_require_output(display);
>  
> +	igt_check_rotation(data);
> +
>  	for_each_pipe_with_valid_output(display, pipe, output) {
>  		igt_plane_t *plane;
>  		int i, j;
>  
> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> -			continue;
> -
>  		igt_output_set_pipe(output, pipe);
>  
>  		plane = igt_output_get_plane_type(output, plane_type);
> @@ -538,14 +549,13 @@ igt_main
>  	};
>  
>  	data_t data = {};
> -	int gen = 0;
>  
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
>  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
>  		data.devid = intel_get_drm_devid(data.gfx_fd);
> -		gen = intel_gen(data.devid);
> +		data.gen = intel_gen(data.devid);
>  
>  		kmstest_set_vt_graphics_mode();
>  
> @@ -558,16 +568,12 @@ igt_main
>  		igt_subtest_f("%s-rotation-%s",
>  			      plane_test_str(subtest->plane),
>  			      rot_test_str(subtest->rot)) {
> -			igt_require(!(subtest->rot &
> -				    (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
> -				    gen >= 9);
>  			data.rotation = subtest->rot;
>  			test_plane_rotation(&data, subtest->plane, false);
>  		}
>  	}
>  
>  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.pos_x = 100,
>  		data.pos_y = 0;
> @@ -577,7 +583,6 @@ igt_main
>  	data.pos_y = 0;
>  
>  	igt_subtest_f("bad-pixel-format") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.override_fmt = DRM_FORMAT_RGB565;
>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
> @@ -585,7 +590,6 @@ igt_main
>  	data.override_fmt = 0;
>  
>  	igt_subtest_f("bad-tiling") {
> -		igt_require(gen >= 9);
>  		data.rotation = IGT_ROTATION_90;
>  		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
> @@ -596,9 +600,6 @@ igt_main
>  		igt_subtest_f("primary-%s-reflect-x-%s",
>  			      tiling_test_str(reflect_x->tiling),
>  			      rot_test_str(reflect_x->rot)) {
> -			igt_require(gen >= 10 ||
> -				    (IS_CHERRYVIEW(data.devid) && reflect_x->rot == IGT_ROTATION_0
> -				     && reflect_x->tiling == LOCAL_I915_FORMAT_MOD_X_TILED));
>  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
>  			data.override_tiling = reflect_x->tiling;
>  			test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, false);
> @@ -613,7 +614,7 @@ igt_main
>  		enum pipe pipe;
>  		igt_output_t *output;
>  
> -		igt_require(gen >= 9);
> +		igt_require(data.gen >= 9);
>  		igt_display_require_output(&data.display);
>  
>  		for_each_pipe_with_valid_output(&data.display, pipe, output) {
Ville Syrjälä March 28, 2018, 10:35 a.m. UTC | #3
On Wed, Mar 28, 2018 at 10:29:15AM +0200, Maarten Lankhorst wrote:
> Op 22-03-18 om 23:10 schreef Radhakrishna Sripada:
> > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >
> > Cleanup the testcases by moving the platform checks to a single function.
> >
> > The earlier version of the path is posted here [1]
> >
> > v2: Make use of the property enums to get the supported rotations
> > v3: Move hardcodings to a single function(Ville)
> >
> > [1]: https://patchwork.freedesktop.org/patch/209647/
> >
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index 0cd5c6e52b57..60fb9012e14e 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -43,6 +43,7 @@ typedef struct {
> >  	uint32_t override_fmt;
> >  	uint64_t override_tiling;
> >  	int devid;
> > +	int gen;
> >  } data_t;
> >  
> >  typedef struct {
> > @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
> >  	igt_assert(drmHandleEvent(fd, &evctx) == 0);
> >  }
> >  
> > +static void igt_check_rotation(data_t *data)
> > +{
> > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > +		igt_require(data->gen >= 9);
> > +	else if (data->rotation & IGT_REFLECT_X)
> > +		igt_require(data->gen >= 10 ||
> > +			    (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_ROTATION_0)));
> > +	else if (data->rotation & IGT_ROTATION_180)
> > +		igt_require(data->gen >= 4);
> > +}
> This still won't work as intended on CHV as it only has reflection on PIPE_B, for X tiled fb's.

s/X tiled//

> 
> 
> >  static void test_single_case(data_t *data, enum pipe pipe,
> >  			     igt_output_t *output, igt_plane_t *plane,
> >  			     enum rectangle_type rect,
> > @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
> >  
> >  	igt_display_require_output(display);
> >  
> > +	igt_check_rotation(data);
> > +
> >  	for_each_pipe_with_valid_output(display, pipe, output) {
> >  		igt_plane_t *plane;
> >  		int i, j;
> >  
> > -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> > -			continue;
> > -
> >  		igt_output_set_pipe(output, pipe);
> >  
> >  		plane = igt_output_get_plane_type(output, plane_type);
> > @@ -538,14 +549,13 @@ igt_main
> >  	};
> >  
> >  	data_t data = {};
> > -	int gen = 0;
> >  
> >  	igt_skip_on_simulation();
> >  
> >  	igt_fixture {
> >  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
> >  		data.devid = intel_get_drm_devid(data.gfx_fd);
> > -		gen = intel_gen(data.devid);
> > +		data.gen = intel_gen(data.devid);
> >  
> >  		kmstest_set_vt_graphics_mode();
> >  
> > @@ -558,16 +568,12 @@ igt_main
> >  		igt_subtest_f("%s-rotation-%s",
> >  			      plane_test_str(subtest->plane),
> >  			      rot_test_str(subtest->rot)) {
> > -			igt_require(!(subtest->rot &
> > -				    (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
> > -				    gen >= 9);
> >  			data.rotation = subtest->rot;
> >  			test_plane_rotation(&data, subtest->plane, false);
> >  		}
> >  	}
> >  
> >  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> > -		igt_require(gen >= 9);
> >  		data.rotation = IGT_ROTATION_90;
> >  		data.pos_x = 100,
> >  		data.pos_y = 0;
> > @@ -577,7 +583,6 @@ igt_main
> >  	data.pos_y = 0;
> >  
> >  	igt_subtest_f("bad-pixel-format") {
> > -		igt_require(gen >= 9);
> >  		data.rotation = IGT_ROTATION_90;
> >  		data.override_fmt = DRM_FORMAT_RGB565;
> >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
> > @@ -585,7 +590,6 @@ igt_main
> >  	data.override_fmt = 0;
> >  
> >  	igt_subtest_f("bad-tiling") {
> > -		igt_require(gen >= 9);
> >  		data.rotation = IGT_ROTATION_90;
> >  		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
> > @@ -596,9 +600,6 @@ igt_main
> >  		igt_subtest_f("primary-%s-reflect-x-%s",
> >  			      tiling_test_str(reflect_x->tiling),
> >  			      rot_test_str(reflect_x->rot)) {
> > -			igt_require(gen >= 10 ||
> > -				    (IS_CHERRYVIEW(data.devid) && reflect_x->rot == IGT_ROTATION_0
> > -				     && reflect_x->tiling == LOCAL_I915_FORMAT_MOD_X_TILED));
> >  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
> >  			data.override_tiling = reflect_x->tiling;
> >  			test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, false);
> > @@ -613,7 +614,7 @@ igt_main
> >  		enum pipe pipe;
> >  		igt_output_t *output;
> >  
> > -		igt_require(gen >= 9);
> > +		igt_require(data.gen >= 9);
> >  		igt_display_require_output(&data.display);
> >  
> >  		for_each_pipe_with_valid_output(&data.display, pipe, output) {
>
Srivatsa, Anusha March 28, 2018, 5:18 p.m. UTC | #4
The rework looks good to me.

Thanks Mika for testing this.

>-----Original Message-----

>From: Kahola, Mika

>Sent: Wednesday, March 28, 2018 1:26 AM

>To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; igt-

>dev@lists.freedesktop.org

>Cc: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha

><anusha.srivatsa@intel.com>; Ville Syrjälä <ville.syrjala@linux.intel.com>; Vetter,

>Daniel <daniel.vetter@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;

>Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Navare, Manasi D

><manasi.d.navare@intel.com>

>Subject: Re: [PATCH i-g-t v3] tests/kms_rotation_crc: Move platform checks to

>one place for non exhaust fence cases

>

>On Thu, 2018-03-22 at 15:10 -0700, Radhakrishna Sripada wrote:

>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>

>>

>> Cleanup the testcases by moving the platform checks to a single

>> function.

>>

>> The earlier version of the path is posted here [1]

>>

>> v2: Make use of the property enums to get the supported rotations

>> v3: Move hardcodings to a single function(Ville)

>>

>> [1]: https://patchwork.freedesktop.org/patch/209647/

>>

>> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

>> Cc: Daniel Vetter <daniel.vetter@intel.com>

>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

>> Cc: Mika Kahola <mika.kahola@intel.com>

>> Cc: Manasi Navare <manasi.d.navare@intel.com>

>

>Tested-by: Mika Kahola <mika.kahola@intel.com>

Reviewed-by: Anusha Srivatsa <Anusha.srivatsa@intel.com>

>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>

>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

>> ---

>>  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------

>>  1 file changed, 16 insertions(+), 15 deletions(-)

>>

>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index

>> 0cd5c6e52b57..60fb9012e14e 100644

>> --- a/tests/kms_rotation_crc.c

>> +++ b/tests/kms_rotation_crc.c

>> @@ -43,6 +43,7 @@ typedef struct {

>>  	uint32_t override_fmt;

>>  	uint64_t override_tiling;

>>  	int devid;

>> +	int gen;

>>  } data_t;

>>

>>  typedef struct {

>> @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)

>>  	igt_assert(drmHandleEvent(fd, &evctx) == 0);

>>  }

>>

>> +static void igt_check_rotation(data_t *data) {

>> +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))

>> +		igt_require(data->gen >= 9);

>> +	else if (data->rotation & IGT_REFLECT_X)

>> +		igt_require(data->gen >= 10 ||

>> +			    (IS_CHERRYVIEW(data->devid) && (data-

>> >rotation & IGT_ROTATION_0)));

>> +	else if (data->rotation & IGT_ROTATION_180)

>> +		igt_require(data->gen >= 4);

>> +}

>> +

>>  static void test_single_case(data_t *data, enum pipe pipe,

>>  			     igt_output_t *output, igt_plane_t *plane,

>>  			     enum rectangle_type rect,

>> @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data,

>> int plane_type, bool test_bad_form

>>

>>  	igt_display_require_output(display);

>>

>> +	igt_check_rotation(data);

>> +

>>  	for_each_pipe_with_valid_output(display, pipe, output) {

>>  		igt_plane_t *plane;

>>  		int i, j;

>>

>> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)

>> -			continue;

>> -

>>  		igt_output_set_pipe(output, pipe);

>>

>>  		plane = igt_output_get_plane_type(output, plane_type); @@ -

>538,14

>> +549,13 @@ igt_main

>>  	};

>>

>>  	data_t data = {};

>> -	int gen = 0;

>>

>>  	igt_skip_on_simulation();

>>

>>  	igt_fixture {

>>  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);

>>  		data.devid = intel_get_drm_devid(data.gfx_fd);

>> -		gen = intel_gen(data.devid);

>> +		data.gen = intel_gen(data.devid);

>>

>>  		kmstest_set_vt_graphics_mode();

>>

>> @@ -558,16 +568,12 @@ igt_main

>>  		igt_subtest_f("%s-rotation-%s",

>>  			      plane_test_str(subtest->plane),

>>  			      rot_test_str(subtest->rot)) {

>> -			igt_require(!(subtest->rot &

>> -				    (IGT_ROTATION_90 |

>> IGT_ROTATION_270)) ||

>> -				    gen >= 9);

>>  			data.rotation = subtest->rot;

>>  			test_plane_rotation(&data, subtest->plane, false);

>>  		}

>>  	}

>>

>>  	igt_subtest_f("sprite-rotation-90-pos-100-0") {

>> -		igt_require(gen >= 9);

>>  		data.rotation = IGT_ROTATION_90;

>>  		data.pos_x = 100,

>>  		data.pos_y = 0;

>> @@ -577,7 +583,6 @@ igt_main

>>  	data.pos_y = 0;

>>

>>  	igt_subtest_f("bad-pixel-format") {

>> -		igt_require(gen >= 9);

>>  		data.rotation = IGT_ROTATION_90;

>>  		data.override_fmt = DRM_FORMAT_RGB565;

>>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);

>@@ -585,7

>> +590,6 @@ igt_main

>>  	data.override_fmt = 0;

>>

>>  	igt_subtest_f("bad-tiling") {

>> -		igt_require(gen >= 9);

>>  		data.rotation = IGT_ROTATION_90;

>>  		data.override_tiling =

>> LOCAL_I915_FORMAT_MOD_X_TILED;

>>  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);

>@@ -596,9

>> +600,6 @@ igt_main

>>  		igt_subtest_f("primary-%s-reflect-x-%s",

>>  			      tiling_test_str(reflect_x->tiling),

>>  			      rot_test_str(reflect_x->rot)) {

>> -			igt_require(gen >= 10 ||

>> -				    (IS_CHERRYVIEW(data.devid) &&

>> reflect_x->rot == IGT_ROTATION_0

>> -				     && reflect_x->tiling ==

>> LOCAL_I915_FORMAT_MOD_X_TILED));

>>  			data.rotation = (IGT_REFLECT_X | reflect_x-

>> >rot);

>>  			data.override_tiling = reflect_x->tiling;

>>  			test_plane_rotation(&data,

>> DRM_PLANE_TYPE_PRIMARY, false);

>> @@ -613,7 +614,7 @@ igt_main

>>  		enum pipe pipe;

>>  		igt_output_t *output;

>>

>> -		igt_require(gen >= 9);

>> +		igt_require(data.gen >= 9);

>>  		igt_display_require_output(&data.display);

>>

>>  		for_each_pipe_with_valid_output(&data.display, pipe,

>> output) {

>--

>Mika Kahola - Intel OTC
Sripada, Radhakrishna March 29, 2018, 6:19 a.m. UTC | #5
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Wednesday, March 28, 2018 3:36 AM
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; igt-
> dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Srivatsa, Anusha
> <anusha.srivatsa@intel.com>; Vetter, Daniel <daniel.vetter@intel.com>; Vivi,
> Rodrigo <rodrigo.vivi@intel.com>; Kahola, Mika <mika.kahola@intel.com>;
> Navare, Manasi D <manasi.d.navare@intel.com>
> Subject: Re: [PATCH i-g-t v3] tests/kms_rotation_crc: Move platform checks
> to one place for non exhaust fence cases
> 
> On Wed, Mar 28, 2018 at 10:29:15AM +0200, Maarten Lankhorst wrote:
> > Op 22-03-18 om 23:10 schreef Radhakrishna Sripada:
> > > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > >
> > > Cleanup the testcases by moving the platform checks to a single function.
> > >
> > > The earlier version of the path is posted here [1]
> > >
> > > v2: Make use of the property enums to get the supported rotations
> > > v3: Move hardcodings to a single function(Ville)
> > >
> > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > >
> > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
> > >  1 file changed, 16 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > index 0cd5c6e52b57..60fb9012e14e 100644
> > > --- a/tests/kms_rotation_crc.c
> > > +++ b/tests/kms_rotation_crc.c
> > > @@ -43,6 +43,7 @@ typedef struct {
> > >  	uint32_t override_fmt;
> > >  	uint64_t override_tiling;
> > >  	int devid;
> > > +	int gen;
> > >  } data_t;
> > >
> > >  typedef struct {
> > > @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
> > >  	igt_assert(drmHandleEvent(fd, &evctx) == 0);  }
> > >
> > > +static void igt_check_rotation(data_t *data) {
> > > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > +		igt_require(data->gen >= 9);
> > > +	else if (data->rotation & IGT_REFLECT_X)
> > > +		igt_require(data->gen >= 10 ||
> > > +			    (IS_CHERRYVIEW(data->devid) && (data->rotation
> & IGT_ROTATION_0)));
> > > +	else if (data->rotation & IGT_ROTATION_180)
> > > +		igt_require(data->gen >= 4);
> > > +}
> > This still won't work as intended on CHV as it only has reflection on PIPE_B,
> for X tiled fb's.
> 
> s/X tiled//
So should we hack the reflect subtest to partially run for a single pipe and skip for the other pipes on Chv? If so is there a non-murky way to do it?

Thanks,
Radhakrishna

> 
> >
> >
> > >  static void test_single_case(data_t *data, enum pipe pipe,
> > >  			     igt_output_t *output, igt_plane_t *plane,
> > >  			     enum rectangle_type rect,
> > > @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data,
> > > int plane_type, bool test_bad_form
> > >
> > >  	igt_display_require_output(display);
> > >
> > > +	igt_check_rotation(data);
> > > +
> > >  	for_each_pipe_with_valid_output(display, pipe, output) {
> > >  		igt_plane_t *plane;
> > >  		int i, j;
> > >
> > > -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> > > -			continue;
> > > -
> > >  		igt_output_set_pipe(output, pipe);
> > >
> > >  		plane = igt_output_get_plane_type(output, plane_type); @@
> -538,14
> > > +549,13 @@ igt_main
> > >  	};
> > >
> > >  	data_t data = {};
> > > -	int gen = 0;
> > >
> > >  	igt_skip_on_simulation();
> > >
> > >  	igt_fixture {
> > >  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
> > >  		data.devid = intel_get_drm_devid(data.gfx_fd);
> > > -		gen = intel_gen(data.devid);
> > > +		data.gen = intel_gen(data.devid);
> > >
> > >  		kmstest_set_vt_graphics_mode();
> > >
> > > @@ -558,16 +568,12 @@ igt_main
> > >  		igt_subtest_f("%s-rotation-%s",
> > >  			      plane_test_str(subtest->plane),
> > >  			      rot_test_str(subtest->rot)) {
> > > -			igt_require(!(subtest->rot &
> > > -				    (IGT_ROTATION_90 | IGT_ROTATION_270))
> ||
> > > -				    gen >= 9);
> > >  			data.rotation = subtest->rot;
> > >  			test_plane_rotation(&data, subtest->plane, false);
> > >  		}
> > >  	}
> > >
> > >  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> > > -		igt_require(gen >= 9);
> > >  		data.rotation = IGT_ROTATION_90;
> > >  		data.pos_x = 100,
> > >  		data.pos_y = 0;
> > > @@ -577,7 +583,6 @@ igt_main
> > >  	data.pos_y = 0;
> > >
> > >  	igt_subtest_f("bad-pixel-format") {
> > > -		igt_require(gen >= 9);
> > >  		data.rotation = IGT_ROTATION_90;
> > >  		data.override_fmt = DRM_FORMAT_RGB565;
> > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> true); @@
> > > -585,7 +590,6 @@ igt_main
> > >  	data.override_fmt = 0;
> > >
> > >  	igt_subtest_f("bad-tiling") {
> > > -		igt_require(gen >= 9);
> > >  		data.rotation = IGT_ROTATION_90;
> > >  		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> true); @@
> > > -596,9 +600,6 @@ igt_main
> > >  		igt_subtest_f("primary-%s-reflect-x-%s",
> > >  			      tiling_test_str(reflect_x->tiling),
> > >  			      rot_test_str(reflect_x->rot)) {
> > > -			igt_require(gen >= 10 ||
> > > -				    (IS_CHERRYVIEW(data.devid) && reflect_x-
> >rot == IGT_ROTATION_0
> > > -				     && reflect_x->tiling ==
> LOCAL_I915_FORMAT_MOD_X_TILED));
> > >  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
> > >  			data.override_tiling = reflect_x->tiling;
> > >  			test_plane_rotation(&data,
> DRM_PLANE_TYPE_PRIMARY, false); @@
> > > -613,7 +614,7 @@ igt_main
> > >  		enum pipe pipe;
> > >  		igt_output_t *output;
> > >
> > > -		igt_require(gen >= 9);
> > > +		igt_require(data.gen >= 9);
> > >  		igt_display_require_output(&data.display);
> > >
> > >  		for_each_pipe_with_valid_output(&data.display, pipe,
> output) {
> >
> 
> --
> Ville Syrjälä
> Intel OTC
Ville Syrjälä April 6, 2018, 3:56 p.m. UTC | #6
On Thu, Mar 29, 2018 at 06:19:40AM +0000, Sripada, Radhakrishna wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Wednesday, March 28, 2018 3:36 AM
> > To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; igt-
> > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Srivatsa, Anusha
> > <anusha.srivatsa@intel.com>; Vetter, Daniel <daniel.vetter@intel.com>; Vivi,
> > Rodrigo <rodrigo.vivi@intel.com>; Kahola, Mika <mika.kahola@intel.com>;
> > Navare, Manasi D <manasi.d.navare@intel.com>
> > Subject: Re: [PATCH i-g-t v3] tests/kms_rotation_crc: Move platform checks
> > to one place for non exhaust fence cases
> > 
> > On Wed, Mar 28, 2018 at 10:29:15AM +0200, Maarten Lankhorst wrote:
> > > Op 22-03-18 om 23:10 schreef Radhakrishna Sripada:
> > > > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > >
> > > > Cleanup the testcases by moving the platform checks to a single function.
> > > >
> > > > The earlier version of the path is posted here [1]
> > > >
> > > > v2: Make use of the property enums to get the supported rotations
> > > > v3: Move hardcodings to a single function(Ville)
> > > >
> > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > >
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > ---
> > > >  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
> > > >  1 file changed, 16 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > > index 0cd5c6e52b57..60fb9012e14e 100644
> > > > --- a/tests/kms_rotation_crc.c
> > > > +++ b/tests/kms_rotation_crc.c
> > > > @@ -43,6 +43,7 @@ typedef struct {
> > > >  	uint32_t override_fmt;
> > > >  	uint64_t override_tiling;
> > > >  	int devid;
> > > > +	int gen;
> > > >  } data_t;
> > > >
> > > >  typedef struct {
> > > > @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
> > > >  	igt_assert(drmHandleEvent(fd, &evctx) == 0);  }
> > > >
> > > > +static void igt_check_rotation(data_t *data) {
> > > > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > > +		igt_require(data->gen >= 9);
> > > > +	else if (data->rotation & IGT_REFLECT_X)
> > > > +		igt_require(data->gen >= 10 ||
> > > > +			    (IS_CHERRYVIEW(data->devid) && (data->rotation
> > & IGT_ROTATION_0)));
> > > > +	else if (data->rotation & IGT_ROTATION_180)
> > > > +		igt_require(data->gen >= 4);
> > > > +}
> > > This still won't work as intended on CHV as it only has reflection on PIPE_B,
> > for X tiled fb's.
> > 
> > s/X tiled//
> So should we hack the reflect subtest to partially run for a single pipe and skip for the other pipes on Chv? If so is there a non-murky way to do it?

At least skipping for pipe A/C should happen automagically if we ask the
plane what it supports. The pipe B case still needs to be special cased
though.

> 
> Thanks,
> Radhakrishna
> 
> > 
> > >
> > >
> > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > >  			     igt_output_t *output, igt_plane_t *plane,
> > > >  			     enum rectangle_type rect,
> > > > @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data,
> > > > int plane_type, bool test_bad_form
> > > >
> > > >  	igt_display_require_output(display);
> > > >
> > > > +	igt_check_rotation(data);
> > > > +
> > > >  	for_each_pipe_with_valid_output(display, pipe, output) {
> > > >  		igt_plane_t *plane;
> > > >  		int i, j;
> > > >
> > > > -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> > > > -			continue;
> > > > -
> > > >  		igt_output_set_pipe(output, pipe);
> > > >
> > > >  		plane = igt_output_get_plane_type(output, plane_type); @@
> > -538,14
> > > > +549,13 @@ igt_main
> > > >  	};
> > > >
> > > >  	data_t data = {};
> > > > -	int gen = 0;
> > > >
> > > >  	igt_skip_on_simulation();
> > > >
> > > >  	igt_fixture {
> > > >  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
> > > >  		data.devid = intel_get_drm_devid(data.gfx_fd);
> > > > -		gen = intel_gen(data.devid);
> > > > +		data.gen = intel_gen(data.devid);
> > > >
> > > >  		kmstest_set_vt_graphics_mode();
> > > >
> > > > @@ -558,16 +568,12 @@ igt_main
> > > >  		igt_subtest_f("%s-rotation-%s",
> > > >  			      plane_test_str(subtest->plane),
> > > >  			      rot_test_str(subtest->rot)) {
> > > > -			igt_require(!(subtest->rot &
> > > > -				    (IGT_ROTATION_90 | IGT_ROTATION_270))
> > ||
> > > > -				    gen >= 9);
> > > >  			data.rotation = subtest->rot;
> > > >  			test_plane_rotation(&data, subtest->plane, false);
> > > >  		}
> > > >  	}
> > > >
> > > >  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> > > > -		igt_require(gen >= 9);
> > > >  		data.rotation = IGT_ROTATION_90;
> > > >  		data.pos_x = 100,
> > > >  		data.pos_y = 0;
> > > > @@ -577,7 +583,6 @@ igt_main
> > > >  	data.pos_y = 0;
> > > >
> > > >  	igt_subtest_f("bad-pixel-format") {
> > > > -		igt_require(gen >= 9);
> > > >  		data.rotation = IGT_ROTATION_90;
> > > >  		data.override_fmt = DRM_FORMAT_RGB565;
> > > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> > true); @@
> > > > -585,7 +590,6 @@ igt_main
> > > >  	data.override_fmt = 0;
> > > >
> > > >  	igt_subtest_f("bad-tiling") {
> > > > -		igt_require(gen >= 9);
> > > >  		data.rotation = IGT_ROTATION_90;
> > > >  		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> > > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> > true); @@
> > > > -596,9 +600,6 @@ igt_main
> > > >  		igt_subtest_f("primary-%s-reflect-x-%s",
> > > >  			      tiling_test_str(reflect_x->tiling),
> > > >  			      rot_test_str(reflect_x->rot)) {
> > > > -			igt_require(gen >= 10 ||
> > > > -				    (IS_CHERRYVIEW(data.devid) && reflect_x-
> > >rot == IGT_ROTATION_0
> > > > -				     && reflect_x->tiling ==
> > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > >  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
> > > >  			data.override_tiling = reflect_x->tiling;
> > > >  			test_plane_rotation(&data,
> > DRM_PLANE_TYPE_PRIMARY, false); @@
> > > > -613,7 +614,7 @@ igt_main
> > > >  		enum pipe pipe;
> > > >  		igt_output_t *output;
> > > >
> > > > -		igt_require(gen >= 9);
> > > > +		igt_require(data.gen >= 9);
> > > >  		igt_display_require_output(&data.display);
> > > >
> > > >  		for_each_pipe_with_valid_output(&data.display, pipe,
> > output) {
> > >
> > 
> > --
> > Ville Syrjälä
> > Intel OTC
Sripada, Radhakrishna April 12, 2018, 11:06 p.m. UTC | #7
On Fri, Apr 06, 2018 at 06:56:22PM +0300, Ville Syrjälä wrote:
> On Thu, Mar 29, 2018 at 06:19:40AM +0000, Sripada, Radhakrishna wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > > Sent: Wednesday, March 28, 2018 3:36 AM
> > > To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; igt-
> > > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Srivatsa, Anusha
> > > <anusha.srivatsa@intel.com>; Vetter, Daniel <daniel.vetter@intel.com>; Vivi,
> > > Rodrigo <rodrigo.vivi@intel.com>; Kahola, Mika <mika.kahola@intel.com>;
> > > Navare, Manasi D <manasi.d.navare@intel.com>
> > > Subject: Re: [PATCH i-g-t v3] tests/kms_rotation_crc: Move platform checks
> > > to one place for non exhaust fence cases
> > > 
> > > On Wed, Mar 28, 2018 at 10:29:15AM +0200, Maarten Lankhorst wrote:
> > > > Op 22-03-18 om 23:10 schreef Radhakrishna Sripada:
> > > > > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > >
> > > > > Cleanup the testcases by moving the platform checks to a single function.
> > > > >
> > > > > The earlier version of the path is posted here [1]
> > > > >
> > > > > v2: Make use of the property enums to get the supported rotations
> > > > > v3: Move hardcodings to a single function(Ville)
> > > > >
> > > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > > >
> > > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > ---
> > > > >  tests/kms_rotation_crc.c | 31 ++++++++++++++++---------------
> > > > >  1 file changed, 16 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > > > index 0cd5c6e52b57..60fb9012e14e 100644
> > > > > --- a/tests/kms_rotation_crc.c
> > > > > +++ b/tests/kms_rotation_crc.c
> > > > > @@ -43,6 +43,7 @@ typedef struct {
> > > > >  	uint32_t override_fmt;
> > > > >  	uint64_t override_tiling;
> > > > >  	int devid;
> > > > > +	int gen;
> > > > >  } data_t;
> > > > >
> > > > >  typedef struct {
> > > > > @@ -301,6 +302,17 @@ static void wait_for_pageflip(int fd)
> > > > >  	igt_assert(drmHandleEvent(fd, &evctx) == 0);  }
> > > > >
> > > > > +static void igt_check_rotation(data_t *data) {
> > > > > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > > > +		igt_require(data->gen >= 9);
> > > > > +	else if (data->rotation & IGT_REFLECT_X)
> > > > > +		igt_require(data->gen >= 10 ||
> > > > > +			    (IS_CHERRYVIEW(data->devid) && (data->rotation
> > > & IGT_ROTATION_0)));
> > > > > +	else if (data->rotation & IGT_ROTATION_180)
> > > > > +		igt_require(data->gen >= 4);
> > > > > +}
> > > > This still won't work as intended on CHV as it only has reflection on PIPE_B,
> > > for X tiled fb's.
> > > 
> > > s/X tiled//
> > So should we hack the reflect subtest to partially run for a single pipe and skip for the other pipes on Chv? If so is there a non-murky way to do it?
> 
> At least skipping for pipe A/C should happen automagically if we ask the
> plane what it supports. The pipe B case still needs to be special cased
> though.
> 

I sent an updated patch [1] making the changes. Is it ok?

[1] https://patchwork.freedesktop.org/patch/214698/

> > 
> > Thanks,
> > Radhakrishna
> > 
> > > 
> > > >
> > > >
> > > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > > >  			     igt_output_t *output, igt_plane_t *plane,
> > > > >  			     enum rectangle_type rect,
> > > > > @@ -369,13 +381,12 @@ static void test_plane_rotation(data_t *data,
> > > > > int plane_type, bool test_bad_form
> > > > >
> > > > >  	igt_display_require_output(display);
> > > > >
> > > > > +	igt_check_rotation(data);
> > > > > +
> > > > >  	for_each_pipe_with_valid_output(display, pipe, output) {
> > > > >  		igt_plane_t *plane;
> > > > >  		int i, j;
> > > > >
> > > > > -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> > > > > -			continue;
> > > > > -
> > > > >  		igt_output_set_pipe(output, pipe);
> > > > >
> > > > >  		plane = igt_output_get_plane_type(output, plane_type); @@
> > > -538,14
> > > > > +549,13 @@ igt_main
> > > > >  	};
> > > > >
> > > > >  	data_t data = {};
> > > > > -	int gen = 0;
> > > > >
> > > > >  	igt_skip_on_simulation();
> > > > >
> > > > >  	igt_fixture {
> > > > >  		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
> > > > >  		data.devid = intel_get_drm_devid(data.gfx_fd);
> > > > > -		gen = intel_gen(data.devid);
> > > > > +		data.gen = intel_gen(data.devid);
> > > > >
> > > > >  		kmstest_set_vt_graphics_mode();
> > > > >
> > > > > @@ -558,16 +568,12 @@ igt_main
> > > > >  		igt_subtest_f("%s-rotation-%s",
> > > > >  			      plane_test_str(subtest->plane),
> > > > >  			      rot_test_str(subtest->rot)) {
> > > > > -			igt_require(!(subtest->rot &
> > > > > -				    (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > ||
> > > > > -				    gen >= 9);
> > > > >  			data.rotation = subtest->rot;
> > > > >  			test_plane_rotation(&data, subtest->plane, false);
> > > > >  		}
> > > > >  	}
> > > > >
> > > > >  	igt_subtest_f("sprite-rotation-90-pos-100-0") {
> > > > > -		igt_require(gen >= 9);
> > > > >  		data.rotation = IGT_ROTATION_90;
> > > > >  		data.pos_x = 100,
> > > > >  		data.pos_y = 0;
> > > > > @@ -577,7 +583,6 @@ igt_main
> > > > >  	data.pos_y = 0;
> > > > >
> > > > >  	igt_subtest_f("bad-pixel-format") {
> > > > > -		igt_require(gen >= 9);
> > > > >  		data.rotation = IGT_ROTATION_90;
> > > > >  		data.override_fmt = DRM_FORMAT_RGB565;
> > > > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> > > true); @@
> > > > > -585,7 +590,6 @@ igt_main
> > > > >  	data.override_fmt = 0;
> > > > >
> > > > >  	igt_subtest_f("bad-tiling") {
> > > > > -		igt_require(gen >= 9);
> > > > >  		data.rotation = IGT_ROTATION_90;
> > > > >  		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> > > > >  		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY,
> > > true); @@
> > > > > -596,9 +600,6 @@ igt_main
> > > > >  		igt_subtest_f("primary-%s-reflect-x-%s",
> > > > >  			      tiling_test_str(reflect_x->tiling),
> > > > >  			      rot_test_str(reflect_x->rot)) {
> > > > > -			igt_require(gen >= 10 ||
> > > > > -				    (IS_CHERRYVIEW(data.devid) && reflect_x-
> > > >rot == IGT_ROTATION_0
> > > > > -				     && reflect_x->tiling ==
> > > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > > >  			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
> > > > >  			data.override_tiling = reflect_x->tiling;
> > > > >  			test_plane_rotation(&data,
> > > DRM_PLANE_TYPE_PRIMARY, false); @@
> > > > > -613,7 +614,7 @@ igt_main
> > > > >  		enum pipe pipe;
> > > > >  		igt_output_t *output;
> > > > >
> > > > > -		igt_require(gen >= 9);
> > > > > +		igt_require(data.gen >= 9);
> > > > >  		igt_display_require_output(&data.display);
> > > > >
> > > > >  		for_each_pipe_with_valid_output(&data.display, pipe,
> > > output) {
> > > >
> > > 
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..60fb9012e14e 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -43,6 +43,7 @@  typedef struct {
 	uint32_t override_fmt;
 	uint64_t override_tiling;
 	int devid;
+	int gen;
 } data_t;
 
 typedef struct {
@@ -301,6 +302,17 @@  static void wait_for_pageflip(int fd)
 	igt_assert(drmHandleEvent(fd, &evctx) == 0);
 }
 
+static void igt_check_rotation(data_t *data)
+{
+	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
+		igt_require(data->gen >= 9);
+	else if (data->rotation & IGT_REFLECT_X)
+		igt_require(data->gen >= 10 ||
+			    (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_ROTATION_0)));
+	else if (data->rotation & IGT_ROTATION_180)
+		igt_require(data->gen >= 4);
+}
+
 static void test_single_case(data_t *data, enum pipe pipe,
 			     igt_output_t *output, igt_plane_t *plane,
 			     enum rectangle_type rect,
@@ -369,13 +381,12 @@  static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 
 	igt_display_require_output(display);
 
+	igt_check_rotation(data);
+
 	for_each_pipe_with_valid_output(display, pipe, output) {
 		igt_plane_t *plane;
 		int i, j;
 
-		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
-			continue;
-
 		igt_output_set_pipe(output, pipe);
 
 		plane = igt_output_get_plane_type(output, plane_type);
@@ -538,14 +549,13 @@  igt_main
 	};
 
 	data_t data = {};
-	int gen = 0;
 
 	igt_skip_on_simulation();
 
 	igt_fixture {
 		data.gfx_fd = drm_open_driver_master(DRIVER_INTEL);
 		data.devid = intel_get_drm_devid(data.gfx_fd);
-		gen = intel_gen(data.devid);
+		data.gen = intel_gen(data.devid);
 
 		kmstest_set_vt_graphics_mode();
 
@@ -558,16 +568,12 @@  igt_main
 		igt_subtest_f("%s-rotation-%s",
 			      plane_test_str(subtest->plane),
 			      rot_test_str(subtest->rot)) {
-			igt_require(!(subtest->rot &
-				    (IGT_ROTATION_90 | IGT_ROTATION_270)) ||
-				    gen >= 9);
 			data.rotation = subtest->rot;
 			test_plane_rotation(&data, subtest->plane, false);
 		}
 	}
 
 	igt_subtest_f("sprite-rotation-90-pos-100-0") {
-		igt_require(gen >= 9);
 		data.rotation = IGT_ROTATION_90;
 		data.pos_x = 100,
 		data.pos_y = 0;
@@ -577,7 +583,6 @@  igt_main
 	data.pos_y = 0;
 
 	igt_subtest_f("bad-pixel-format") {
-		igt_require(gen >= 9);
 		data.rotation = IGT_ROTATION_90;
 		data.override_fmt = DRM_FORMAT_RGB565;
 		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
@@ -585,7 +590,6 @@  igt_main
 	data.override_fmt = 0;
 
 	igt_subtest_f("bad-tiling") {
-		igt_require(gen >= 9);
 		data.rotation = IGT_ROTATION_90;
 		data.override_tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
 		test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, true);
@@ -596,9 +600,6 @@  igt_main
 		igt_subtest_f("primary-%s-reflect-x-%s",
 			      tiling_test_str(reflect_x->tiling),
 			      rot_test_str(reflect_x->rot)) {
-			igt_require(gen >= 10 ||
-				    (IS_CHERRYVIEW(data.devid) && reflect_x->rot == IGT_ROTATION_0
-				     && reflect_x->tiling == LOCAL_I915_FORMAT_MOD_X_TILED));
 			data.rotation = (IGT_REFLECT_X | reflect_x->rot);
 			data.override_tiling = reflect_x->tiling;
 			test_plane_rotation(&data, DRM_PLANE_TYPE_PRIMARY, false);
@@ -613,7 +614,7 @@  igt_main
 		enum pipe pipe;
 		igt_output_t *output;
 
-		igt_require(gen >= 9);
+		igt_require(data.gen >= 9);
 		igt_display_require_output(&data.display);
 
 		for_each_pipe_with_valid_output(&data.display, pipe, output) {