All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-09 21:45 ` Radhakrishna Sripada
  0 siblings, 0 replies; 12+ messages in thread
From: Radhakrishna Sripada @ 2018-03-09 21:45 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Rodrigo Vivi, Daniel Vetter

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

Rework the rotate and reflect subtests by checking the
crtc supported properties against the ones that the
test is testing. Remove the hardcoded platform names in
igt_require()

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

Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
---
 lib/igt_kms.h            |  1 +
 tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 1c46186e8a9d..c306aa1f5b8d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -289,6 +289,7 @@ typedef enum {
 
 #define IGT_ROTATION_MASK \
 	(IGT_ROTATION_0 | IGT_ROTATION_90 | IGT_ROTATION_180 | IGT_ROTATION_270)
+#define IGT_REFLECT_MASK	(IGT_REFLECT_X | IGT_REFLECT_Y)
 
 typedef struct {
 	/*< private >*/
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..8f7e5d4f2dd2 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -38,6 +38,7 @@ typedef struct {
 	igt_crc_t flip_crc;
 	igt_pipe_crc_t *pipe_crc;
 	igt_rotation_t rotation;
+	int **supported_rotation;
 	int pos_x;
 	int pos_y;
 	uint32_t override_fmt;
@@ -373,13 +374,14 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 		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);
 		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+		igt_require(data->supported_rotation[pipe][plane->index] & data->rotation);
+
+		if (data->rotation & IGT_REFLECT_MASK)
+			igt_require(data->supported_rotation[pipe][plane->index] & IGT_REFLECT_MASK);
 
 		prepare_crtc(data, output, pipe, plane);
 
@@ -460,6 +462,36 @@ static void test_plane_rotation_exhaust_fences(data_t *data,
 		igt_remove_fb(fd, &fb[i]);
 }
 
+static void igt_supported_rotation_init(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	bool found;
+	int i, j, k, n_pipes = display->n_pipes;
+	int **s_rotation;
+
+	s_rotation = calloc(sizeof(int *), n_pipes);
+
+	for (i = 0; i < n_pipes; i++) {
+		s_rotation[i] = calloc(sizeof(int), (display->pipes + i)->n_planes);
+
+		for (j = 0; j < (display->pipes + i)->n_planes; j++) {
+			drmModePropertyPtr prop;
+			found = kmstest_get_property(data->gfx_fd, display->pipes[i].planes[j].drm_plane->plane_id,
+						    DRM_MODE_OBJECT_PLANE, "rotation", NULL, NULL, &prop);
+			igt_assert(found);
+			igt_assert(prop->flags & DRM_MODE_PROP_BITMASK);
+
+			for (k = 0; k < prop->count_enums; k++) {
+				s_rotation[i][j] |= 1 << (prop->enums[k].value);
+			}
+
+			drmModeFreeProperty(prop);
+		}
+	}
+
+	data->supported_rotation = s_rotation;
+}
+
 static const char *plane_test_str(unsigned plane)
 {
 	switch (plane) {
@@ -552,15 +584,13 @@ igt_main
 		igt_require_pipe_crc(data.gfx_fd);
 
 		igt_display_init(&data.display, data.gfx_fd);
+		igt_supported_rotation_init(&data);
 	}
 
 	for (subtest = subtests; subtest->rot; subtest++) {
 		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);
 		}
@@ -596,9 +626,7 @@ 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));
+			igt_require(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);
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-09 21:45 ` Radhakrishna Sripada
  0 siblings, 0 replies; 12+ messages in thread
From: Radhakrishna Sripada @ 2018-03-09 21:45 UTC (permalink / raw)
  To: igt-dev
  Cc: Anusha Srivatsa, intel-gfx, Manasi Navare, Rodrigo Vivi, Daniel Vetter

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

Rework the rotate and reflect subtests by checking the
crtc supported properties against the ones that the
test is testing. Remove the hardcoded platform names in
igt_require()

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

Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
---
 lib/igt_kms.h            |  1 +
 tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 1c46186e8a9d..c306aa1f5b8d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -289,6 +289,7 @@ typedef enum {
 
 #define IGT_ROTATION_MASK \
 	(IGT_ROTATION_0 | IGT_ROTATION_90 | IGT_ROTATION_180 | IGT_ROTATION_270)
+#define IGT_REFLECT_MASK	(IGT_REFLECT_X | IGT_REFLECT_Y)
 
 typedef struct {
 	/*< private >*/
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..8f7e5d4f2dd2 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -38,6 +38,7 @@ typedef struct {
 	igt_crc_t flip_crc;
 	igt_pipe_crc_t *pipe_crc;
 	igt_rotation_t rotation;
+	int **supported_rotation;
 	int pos_x;
 	int pos_y;
 	uint32_t override_fmt;
@@ -373,13 +374,14 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 		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);
 		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+		igt_require(data->supported_rotation[pipe][plane->index] & data->rotation);
+
+		if (data->rotation & IGT_REFLECT_MASK)
+			igt_require(data->supported_rotation[pipe][plane->index] & IGT_REFLECT_MASK);
 
 		prepare_crtc(data, output, pipe, plane);
 
@@ -460,6 +462,36 @@ static void test_plane_rotation_exhaust_fences(data_t *data,
 		igt_remove_fb(fd, &fb[i]);
 }
 
+static void igt_supported_rotation_init(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	bool found;
+	int i, j, k, n_pipes = display->n_pipes;
+	int **s_rotation;
+
+	s_rotation = calloc(sizeof(int *), n_pipes);
+
+	for (i = 0; i < n_pipes; i++) {
+		s_rotation[i] = calloc(sizeof(int), (display->pipes + i)->n_planes);
+
+		for (j = 0; j < (display->pipes + i)->n_planes; j++) {
+			drmModePropertyPtr prop;
+			found = kmstest_get_property(data->gfx_fd, display->pipes[i].planes[j].drm_plane->plane_id,
+						    DRM_MODE_OBJECT_PLANE, "rotation", NULL, NULL, &prop);
+			igt_assert(found);
+			igt_assert(prop->flags & DRM_MODE_PROP_BITMASK);
+
+			for (k = 0; k < prop->count_enums; k++) {
+				s_rotation[i][j] |= 1 << (prop->enums[k].value);
+			}
+
+			drmModeFreeProperty(prop);
+		}
+	}
+
+	data->supported_rotation = s_rotation;
+}
+
 static const char *plane_test_str(unsigned plane)
 {
 	switch (plane) {
@@ -552,15 +584,13 @@ igt_main
 		igt_require_pipe_crc(data.gfx_fd);
 
 		igt_display_init(&data.display, data.gfx_fd);
+		igt_supported_rotation_init(&data);
 	}
 
 	for (subtest = subtests; subtest->rot; subtest++) {
 		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);
 		}
@@ -596,9 +626,7 @@ 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));
+			igt_require(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);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() (rev2)
  2018-03-09 21:45 ` [igt-dev] " Radhakrishna Sripada
  (?)
@ 2018-03-09 23:29 ` Patchwork
  -1 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-03-09 23:29 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: igt-dev

== Series Details ==

Series: tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() (rev2)
URL   : https://patchwork.freedesktop.org/series/38205/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
5d71d7782a830843c7231fbd72ab3edae19b48d7 igt/gem_fd_exhaustion: Modify fs.nr_open for duration of the test

with latest DRM-Tip kernel build CI_DRM_3908
7e5ff4244a75 drm-tip: 2018y-03m-09d-22h-21m-43s UTC integration manifest

No testlist changes.

---- Known issues:

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6700k2) fdo#104108
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575

fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:504s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:491s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:491s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:483s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:405s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:574s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:583s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:290s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:518s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:468s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:425s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:469s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:466s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:587s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:428s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:527s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:536s
fi-skl-6700k2    total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:504s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:422s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:429s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:530s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:505s
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:19  time:515s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1102/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() (rev2)
  2018-03-09 21:45 ` [igt-dev] " Radhakrishna Sripada
  (?)
  (?)
@ 2018-03-10  7:02 ` Patchwork
  -1 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-03-10  7:02 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: igt-dev

== Series Details ==

Series: tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() (rev2)
URL   : https://patchwork.freedesktop.org/series/38205/
State : failure

== Summary ==

---- Possible new issues:

Test kms_color:
        Subgroup pipe-c-ctm-negative:
                pass       -> FAIL       (shard-apl)
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup flip-vs-panning-vs-hang-interruptible:
                pass       -> DMESG-WARN (shard-snb) fdo#103821
Test kms_rotation_crc:
        Subgroup primary-rotation-270:
                pass       -> FAIL       (shard-apl) fdo#105185
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-apl) fdo#99912
Test kms_vblank:
        Subgroup pipe-b-accuracy-idle:
                pass       -> FAIL       (shard-hsw) fdo#102583

fdo#103821 https://bugs.freedesktop.org/show_bug.cgi?id=103821
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583

shard-apl        total:3409 pass:1800 dwarn:1   dfail:0   fail:8   skip:1598 time:11955s
shard-hsw        total:3467 pass:1772 dwarn:1   dfail:0   fail:2   skip:1691 time:11613s
shard-snb        total:3467 pass:1364 dwarn:2   dfail:0   fail:1   skip:2100 time:6991s
Blacklisted hosts:
shard-kbl        total:3409 pass:1923 dwarn:1   dfail:0   fail:7   skip:1476 time:8814s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1102/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
  2018-03-09 21:45 ` [igt-dev] " Radhakrishna Sripada
@ 2018-03-12 11:03   ` Maarten Lankhorst
  -1 siblings, 0 replies; 12+ messages in thread
From: Maarten Lankhorst @ 2018-03-12 11:03 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev; +Cc: intel-gfx, Rodrigo Vivi, Daniel Vetter

Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>
> Rework the rotate and reflect subtests by checking the
> crtc supported properties against the ones that the
> test is testing. Remove the hardcoded platform names in
> igt_require()
>
> v2: Make use of the property enums to get the supported rotations
>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> ---
>  lib/igt_kms.h            |  1 +
>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 38 insertions(+), 9 deletions(-)

Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..

Lets keep it as it is now?
---
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..9b9da53054c1 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -42,7 +42,6 @@ typedef struct {
 	int pos_y;
 	uint32_t override_fmt;
 	uint64_t override_tiling;
-	int devid;
 } data_t;
 
 typedef struct {
@@ -358,11 +357,22 @@ static void test_single_case(data_t *data, enum pipe pipe,
 	}
 }
 
+static bool supports_rotation(igt_display_t *display, igt_plane_t *plane, uint64_t rotation)
+{
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+
+	igt_assert(!igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL));
+	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
+
+	return !igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+}
+
 static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_format)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
 	enum pipe pipe;
+	bool tested = false;
 
 	if (plane_type == DRM_PLANE_TYPE_CURSOR)
 		igt_require(display->has_cursor_plane);
@@ -373,13 +383,15 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 		igt_plane_t *plane;
 		int i, j;
 
-		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
-			continue;
+		igt_display_reset(display);
 
 		igt_output_set_pipe(output, pipe);
-
 		plane = igt_output_get_plane_type(output, plane_type);
-		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+
+		if (!supports_rotation(display, plane, data->rotation))
+			continue;
+
+		tested = true;
 
 		prepare_crtc(data, output, pipe, plane);
 
@@ -410,6 +422,8 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 			}
 		}
 	}
+
+	igt_assert_f(tested, "Rotation for this plane type not supported on any valid pipe.\n");
 }
 
 static void test_plane_rotation_exhaust_fences(data_t *data,
@@ -538,14 +552,11 @@ 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);
 
 		kmstest_set_vt_graphics_mode();
 
@@ -558,16 +569,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 +584,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 +591,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 +601,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 +615,6 @@ igt_main
 		enum pipe pipe;
 		igt_output_t *output;
 
-		igt_require(gen >= 9);
 		igt_display_require_output(&data.display);
 
 		for_each_pipe_with_valid_output(&data.display, pipe, output) {

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-12 11:03   ` Maarten Lankhorst
  0 siblings, 0 replies; 12+ messages in thread
From: Maarten Lankhorst @ 2018-03-12 11:03 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev
  Cc: Anusha Srivatsa, intel-gfx, Manasi Navare, Rodrigo Vivi, Daniel Vetter

Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>
> Rework the rotate and reflect subtests by checking the
> crtc supported properties against the ones that the
> test is testing. Remove the hardcoded platform names in
> igt_require()
>
> v2: Make use of the property enums to get the supported rotations
>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> ---
>  lib/igt_kms.h            |  1 +
>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 38 insertions(+), 9 deletions(-)

Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..

Lets keep it as it is now?
---
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..9b9da53054c1 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -42,7 +42,6 @@ typedef struct {
 	int pos_y;
 	uint32_t override_fmt;
 	uint64_t override_tiling;
-	int devid;
 } data_t;
 
 typedef struct {
@@ -358,11 +357,22 @@ static void test_single_case(data_t *data, enum pipe pipe,
 	}
 }
 
+static bool supports_rotation(igt_display_t *display, igt_plane_t *plane, uint64_t rotation)
+{
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+
+	igt_assert(!igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL));
+	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
+
+	return !igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+}
+
 static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_format)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
 	enum pipe pipe;
+	bool tested = false;
 
 	if (plane_type == DRM_PLANE_TYPE_CURSOR)
 		igt_require(display->has_cursor_plane);
@@ -373,13 +383,15 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 		igt_plane_t *plane;
 		int i, j;
 
-		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
-			continue;
+		igt_display_reset(display);
 
 		igt_output_set_pipe(output, pipe);
-
 		plane = igt_output_get_plane_type(output, plane_type);
-		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
+
+		if (!supports_rotation(display, plane, data->rotation))
+			continue;
+
+		tested = true;
 
 		prepare_crtc(data, output, pipe, plane);
 
@@ -410,6 +422,8 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 			}
 		}
 	}
+
+	igt_assert_f(tested, "Rotation for this plane type not supported on any valid pipe.\n");
 }
 
 static void test_plane_rotation_exhaust_fences(data_t *data,
@@ -538,14 +552,11 @@ 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);
 
 		kmstest_set_vt_graphics_mode();
 
@@ -558,16 +569,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 +584,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 +591,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 +601,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 +615,6 @@ igt_main
 		enum pipe pipe;
 		igt_output_t *output;
 
-		igt_require(gen >= 9);
 		igt_display_require_output(&data.display);
 
 		for_each_pipe_with_valid_output(&data.display, pipe, output) {

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
  2018-03-12 11:03   ` [igt-dev] " Maarten Lankhorst
@ 2018-03-12 14:23     ` Ville Syrjälä
  -1 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-03-12 14:23 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, igt-dev, Rodrigo Vivi, Daniel Vetter

On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >
> > Rework the rotate and reflect subtests by checking the
> > crtc supported properties against the ones that the
> > test is testing. Remove the hardcoded platform names in
> > igt_require()
> >
> > v2: Make use of the property enums to get the supported rotations
> >
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> > ---
> >  lib/igt_kms.h            |  1 +
> >  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
> >  2 files changed, 38 insertions(+), 9 deletions(-)
> 
> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..

The current code is actually wrong. chv doesn't require X-tiling for
reflection.

I think the actual hardware restrictions we have are:
gen4+: 0/180 degree rotation supported always
chv: supports reflection, except with 180 degree rotation
gen9+: 90/270 degree rotation supported with y/yf tiling
gen10+: supports reflection, except with linear fb

> 
> Lets keep it as it is now?
> ---
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 0cd5c6e52b57..9b9da53054c1 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -42,7 +42,6 @@ typedef struct {
>  	int pos_y;
>  	uint32_t override_fmt;
>  	uint64_t override_tiling;
> -	int devid;
>  } data_t;
>  
>  typedef struct {
> @@ -358,11 +357,22 @@ static void test_single_case(data_t *data, enum pipe pipe,
>  	}
>  }
>  
> +static bool supports_rotation(igt_display_t *display, igt_plane_t *plane, uint64_t rotation)
> +{
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
> +
> +	igt_assert(!igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL));
> +	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
> +
> +	return !igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +}
> +
>  static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_format)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
>  	enum pipe pipe;
> +	bool tested = false;
>  
>  	if (plane_type == DRM_PLANE_TYPE_CURSOR)
>  		igt_require(display->has_cursor_plane);
> @@ -373,13 +383,15 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  		igt_plane_t *plane;
>  		int i, j;
>  
> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> -			continue;
> +		igt_display_reset(display);
>  
>  		igt_output_set_pipe(output, pipe);
> -
>  		plane = igt_output_get_plane_type(output, plane_type);
> -		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
> +
> +		if (!supports_rotation(display, plane, data->rotation))
> +			continue;
> +
> +		tested = true;
>  
>  		prepare_crtc(data, output, pipe, plane);
>  
> @@ -410,6 +422,8 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  			}
>  		}
>  	}
> +
> +	igt_assert_f(tested, "Rotation for this plane type not supported on any valid pipe.\n");
>  }
>  
>  static void test_plane_rotation_exhaust_fences(data_t *data,
> @@ -538,14 +552,11 @@ 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);
>  
>  		kmstest_set_vt_graphics_mode();
>  
> @@ -558,16 +569,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 +584,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 +591,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 +601,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 +615,6 @@ igt_main
>  		enum pipe pipe;
>  		igt_output_t *output;
>  
> -		igt_require(gen >= 9);
>  		igt_display_require_output(&data.display);
>  
>  		for_each_pipe_with_valid_output(&data.display, pipe, output) {
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-12 14:23     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-03-12 14:23 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Manasi Navare, Anusha Srivatsa, intel-gfx, igt-dev, Rodrigo Vivi,
	Daniel Vetter

On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >
> > Rework the rotate and reflect subtests by checking the
> > crtc supported properties against the ones that the
> > test is testing. Remove the hardcoded platform names in
> > igt_require()
> >
> > v2: Make use of the property enums to get the supported rotations
> >
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> > ---
> >  lib/igt_kms.h            |  1 +
> >  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
> >  2 files changed, 38 insertions(+), 9 deletions(-)
> 
> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..

The current code is actually wrong. chv doesn't require X-tiling for
reflection.

I think the actual hardware restrictions we have are:
gen4+: 0/180 degree rotation supported always
chv: supports reflection, except with 180 degree rotation
gen9+: 90/270 degree rotation supported with y/yf tiling
gen10+: supports reflection, except with linear fb

> 
> Lets keep it as it is now?
> ---
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 0cd5c6e52b57..9b9da53054c1 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -42,7 +42,6 @@ typedef struct {
>  	int pos_y;
>  	uint32_t override_fmt;
>  	uint64_t override_tiling;
> -	int devid;
>  } data_t;
>  
>  typedef struct {
> @@ -358,11 +357,22 @@ static void test_single_case(data_t *data, enum pipe pipe,
>  	}
>  }
>  
> +static bool supports_rotation(igt_display_t *display, igt_plane_t *plane, uint64_t rotation)
> +{
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
> +
> +	igt_assert(!igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL));
> +	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
> +
> +	return !igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +}
> +
>  static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_format)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
>  	enum pipe pipe;
> +	bool tested = false;
>  
>  	if (plane_type == DRM_PLANE_TYPE_CURSOR)
>  		igt_require(display->has_cursor_plane);
> @@ -373,13 +383,15 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  		igt_plane_t *plane;
>  		int i, j;
>  
> -		if (IS_CHERRYVIEW(data->devid) && pipe != PIPE_B)
> -			continue;
> +		igt_display_reset(display);
>  
>  		igt_output_set_pipe(output, pipe);
> -
>  		plane = igt_output_get_plane_type(output, plane_type);
> -		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
> +
> +		if (!supports_rotation(display, plane, data->rotation))
> +			continue;
> +
> +		tested = true;
>  
>  		prepare_crtc(data, output, pipe, plane);
>  
> @@ -410,6 +422,8 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
>  			}
>  		}
>  	}
> +
> +	igt_assert_f(tested, "Rotation for this plane type not supported on any valid pipe.\n");
>  }
>  
>  static void test_plane_rotation_exhaust_fences(data_t *data,
> @@ -538,14 +552,11 @@ 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);
>  
>  		kmstest_set_vt_graphics_mode();
>  
> @@ -558,16 +569,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 +584,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 +591,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 +601,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 +615,6 @@ igt_main
>  		enum pipe pipe;
>  		igt_output_t *output;
>  
> -		igt_require(gen >= 9);
>  		igt_display_require_output(&data.display);
>  
>  		for_each_pipe_with_valid_output(&data.display, pipe, output) {
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
  2018-03-12 14:23     ` Ville Syrjälä
@ 2018-03-12 15:09       ` Maarten Lankhorst
  -1 siblings, 0 replies; 12+ messages in thread
From: Maarten Lankhorst @ 2018-03-12 15:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, igt-dev, Rodrigo Vivi, Daniel Vetter

Op 12-03-18 om 15:23 schreef Ville Syrjälä:
> On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
>> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
>>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>>
>>> Rework the rotate and reflect subtests by checking the
>>> crtc supported properties against the ones that the
>>> test is testing. Remove the hardcoded platform names in
>>> igt_require()
>>>
>>> v2: Make use of the property enums to get the supported rotations
>>>
>>> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
>>> ---
>>>  lib/igt_kms.h            |  1 +
>>>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
>>>  2 files changed, 38 insertions(+), 9 deletions(-)
>> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
>> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..
> The current code is actually wrong. chv doesn't require X-tiling for
> reflection.
>
> I think the actual hardware restrictions we have are:
> gen4+: 0/180 degree rotation supported always
> chv: supports reflection, except with 180 degree rotation
> gen9+: 90/270 degree rotation supported with y/yf tiling
> gen10+: supports reflection, except with linear fb
So can we leave it the way it is now?

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-12 15:09       ` Maarten Lankhorst
  0 siblings, 0 replies; 12+ messages in thread
From: Maarten Lankhorst @ 2018-03-12 15:09 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Manasi Navare, Anusha Srivatsa, intel-gfx, igt-dev, Rodrigo Vivi,
	Daniel Vetter

Op 12-03-18 om 15:23 schreef Ville Syrjälä:
> On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
>> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
>>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>>
>>> Rework the rotate and reflect subtests by checking the
>>> crtc supported properties against the ones that the
>>> test is testing. Remove the hardcoded platform names in
>>> igt_require()
>>>
>>> v2: Make use of the property enums to get the supported rotations
>>>
>>> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
>>> ---
>>>  lib/igt_kms.h            |  1 +
>>>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
>>>  2 files changed, 38 insertions(+), 9 deletions(-)
>> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
>> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..
> The current code is actually wrong. chv doesn't require X-tiling for
> reflection.
>
> I think the actual hardware restrictions we have are:
> gen4+: 0/180 degree rotation supported always
> chv: supports reflection, except with 180 degree rotation
> gen9+: 90/270 degree rotation supported with y/yf tiling
> gen10+: supports reflection, except with linear fb
So can we leave it the way it is now?

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
  2018-03-12 15:09       ` Maarten Lankhorst
@ 2018-03-12 15:22         ` Ville Syrjälä
  -1 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-03-12 15:22 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, igt-dev, Rodrigo Vivi, Daniel Vetter

On Mon, Mar 12, 2018 at 04:09:06PM +0100, Maarten Lankhorst wrote:
> Op 12-03-18 om 15:23 schreef Ville Syrjälä:
> > On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
> >> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> >>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >>>
> >>> Rework the rotate and reflect subtests by checking the
> >>> crtc supported properties against the ones that the
> >>> test is testing. Remove the hardcoded platform names in
> >>> igt_require()
> >>>
> >>> v2: Make use of the property enums to get the supported rotations
> >>>
> >>> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> >>> ---
> >>>  lib/igt_kms.h            |  1 +
> >>>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
> >>>  2 files changed, 38 insertions(+), 9 deletions(-)
> >> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
> >> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..
> > The current code is actually wrong. chv doesn't require X-tiling for
> > reflection.
> >
> > I think the actual hardware restrictions we have are:
> > gen4+: 0/180 degree rotation supported always
> > chv: supports reflection, except with 180 degree rotation
> > gen9+: 90/270 degree rotation supported with y/yf tiling
> > gen10+: supports reflection, except with linear fb
> So can we leave it the way it is now?

Maybe. I don't really know what we're trying to test here. Someone
should probably remove the bogus x-tiling check from chv at least.

Perhaps the best solution would be to make it as generic as possible
by checking the plane supported rotations, while still keeoing the
manual checks for the few exceptions I listed above. Might even be
nice to put the generic stuff into something like
igt_plane_has_rotation(). And maybe the exceptions should be there
as well?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()
@ 2018-03-12 15:22         ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-03-12 15:22 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, igt-dev, Rodrigo Vivi, Daniel Vetter

On Mon, Mar 12, 2018 at 04:09:06PM +0100, Maarten Lankhorst wrote:
> Op 12-03-18 om 15:23 schreef Ville Syrjälä:
> > On Mon, Mar 12, 2018 at 12:03:17PM +0100, Maarten Lankhorst wrote:
> >> Op 09-03-18 om 22:45 schreef Radhakrishna Sripada:
> >>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >>>
> >>> Rework the rotate and reflect subtests by checking the
> >>> crtc supported properties against the ones that the
> >>> test is testing. Remove the hardcoded platform names in
> >>> igt_require()
> >>>
> >>> v2: Make use of the property enums to get the supported rotations
> >>>
> >>> Cc: Radhakrishna Sripada <radhakrishna.sripada@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>
> >>> ---
> >>>  lib/igt_kms.h            |  1 +
> >>>  tests/kms_rotation_crc.c | 46 +++++++++++++++++++++++++++++++++++++---------
> >>>  2 files changed, 38 insertions(+), 9 deletions(-)
> >> Just let it rest, it's not worth the effort to remove this, you only end up with more complicated code..
> >> Closest I've come is below. Which will still fail because it will try to generate the wrong tilings pre-gen9..
> > The current code is actually wrong. chv doesn't require X-tiling for
> > reflection.
> >
> > I think the actual hardware restrictions we have are:
> > gen4+: 0/180 degree rotation supported always
> > chv: supports reflection, except with 180 degree rotation
> > gen9+: 90/270 degree rotation supported with y/yf tiling
> > gen10+: supports reflection, except with linear fb
> So can we leave it the way it is now?

Maybe. I don't really know what we're trying to test here. Someone
should probably remove the bogus x-tiling check from chv at least.

Perhaps the best solution would be to make it as generic as possible
by checking the plane supported rotations, while still keeoing the
manual checks for the few exceptions I listed above. Might even be
nice to put the generic stuff into something like
igt_plane_has_rotation(). And maybe the exceptions should be there
as well?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-03-12 15:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 21:45 [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() Radhakrishna Sripada
2018-03-09 21:45 ` [igt-dev] " Radhakrishna Sripada
2018-03-09 23:29 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() (rev2) Patchwork
2018-03-10  7:02 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2018-03-12 11:03 ` [PATCH i-g-t v2] tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require() Maarten Lankhorst
2018-03-12 11:03   ` [igt-dev] " Maarten Lankhorst
2018-03-12 14:23   ` Ville Syrjälä
2018-03-12 14:23     ` Ville Syrjälä
2018-03-12 15:09     ` Maarten Lankhorst
2018-03-12 15:09       ` Maarten Lankhorst
2018-03-12 15:22       ` Ville Syrjälä
2018-03-12 15:22         ` [Intel-gfx] " Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.