All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-23 18:25 ` Radhakrishna Sripada
  0 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-23 18:25 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Daniel Vetter

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)
v4: Include the cherryview exception for reflect subtest(Maarten)
v5: Rebase and move the check from CNL to ICL for reflect-x case
v6: Fix the CI regression
v7: rebase

[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: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: 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 | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 6cb5858adb0f..f20b8a6d4ba1 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 {
@@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
 		igt_plane_set_position(plane, data->pos_x, data->pos_y);
 }
 
+static void igt_check_rotation(data_t *data)
+{
+	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
+		igt_require(data->gen >= 9);
+	if (data->rotation & IGT_REFLECT_X)
+		igt_require(data->gen >= 11 ||
+			    (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_ROTATION_0)));
+	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,
@@ -352,15 +364,18 @@ 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);
 
+		if (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_REFLECT_X) &&
+		    pipe != kmstest_pipe_to_index('B'))
+			continue;
+
 		plane = igt_output_get_plane_type(output, plane_type);
 		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
 
@@ -521,14 +536,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();
 
@@ -541,16 +555,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;
@@ -560,7 +570,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);
@@ -568,7 +577,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);
@@ -579,9 +587,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);
@@ -596,7 +601,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) {
-- 
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] 14+ messages in thread

* [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-23 18:25 ` Radhakrishna Sripada
  0 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-23 18:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Anusha Srivatsa, intel-gfx, Daniel Vetter

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)
v4: Include the cherryview exception for reflect subtest(Maarten)
v5: Rebase and move the check from CNL to ICL for reflect-x case
v6: Fix the CI regression
v7: rebase

[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: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: 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 | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 6cb5858adb0f..f20b8a6d4ba1 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 {
@@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
 		igt_plane_set_position(plane, data->pos_x, data->pos_y);
 }
 
+static void igt_check_rotation(data_t *data)
+{
+	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
+		igt_require(data->gen >= 9);
+	if (data->rotation & IGT_REFLECT_X)
+		igt_require(data->gen >= 11 ||
+			    (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_ROTATION_0)));
+	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,
@@ -352,15 +364,18 @@ 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);
 
+		if (IS_CHERRYVIEW(data->devid) && (data->rotation & IGT_REFLECT_X) &&
+		    pipe != kmstest_pipe_to_index('B'))
+			continue;
+
 		plane = igt_output_get_plane_type(output, plane_type);
 		igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
 
@@ -521,14 +536,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();
 
@@ -541,16 +555,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;
@@ -560,7 +570,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);
@@ -568,7 +577,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);
@@ -579,9 +587,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);
@@ -596,7 +601,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) {
-- 
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] 14+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases (rev5)
  2018-07-23 18:25 ` [igt-dev] " Radhakrishna Sripada
  (?)
@ 2018-07-23 18:58 ` Patchwork
  -1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-23 18:58 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: igt-dev

== Series Details ==

Series: tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases (rev5)
URL   : https://patchwork.freedesktop.org/series/40553/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4521 -> IGTPW_1632 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/40553/revisions/5/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1632 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106248, fdo#106725)

    igt@gem_pwrite@basic:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
      fi-glk-j4005:       PASS -> FAIL (fdo#106765)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000, fdo#106097)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#106765 https://bugs.freedesktop.org/show_bug.cgi?id=106765


== Participating hosts (47 -> 43) ==

  Additional (1): fi-bsw-kefka 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4570 -> IGTPW_1632

  CI_DRM_4521: a4ebbd84c682fd30edbde6ac0e48d150d4c5c066 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1632: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1632/
  IGT_4570: 65cdccdc7bcbb791d791aeeeecb784a382110a3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases (rev5)
  2018-07-23 18:25 ` [igt-dev] " Radhakrishna Sripada
  (?)
  (?)
@ 2018-07-23 20:23 ` Patchwork
  -1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-07-23 20:23 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: igt-dev

== Series Details ==

Series: tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases (rev5)
URL   : https://patchwork.freedesktop.org/series/40553/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4570_full -> IGTPW_1632_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1632_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1632_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/40553/revisions/5/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1632_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          PASS -> SKIP

    igt@kms_atomic_interruptible@legacy-pageflip:
      shard-snb:          SKIP -> PASS +1

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

  Here are the changes found in IGTPW_1632_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-vebox:
      shard-glk:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#100368)
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105189)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@perf_pmu@busy-idle-check-all-vcs0:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@gem_mmap_gtt@coherency:
      shard-glk:          FAIL (fdo#100587) -> SKIP +1

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-snb:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@perf_pmu@multi-client-vcs1:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@prime_vgem@coherency-gtt:
      shard-apl:          FAIL (fdo#100587) -> SKIP +1

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#100587 https://bugs.freedesktop.org/show_bug.cgi?id=100587
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4570 -> IGTPW_1632
    * Linux: CI_DRM_4519 -> CI_DRM_4521

  CI_DRM_4519: f14c0ec8fe9acce6fd1be84766f854ab8874eb33 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4521: a4ebbd84c682fd30edbde6ac0e48d150d4c5c066 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1632: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1632/
  IGT_4570: 65cdccdc7bcbb791d791aeeeecb784a382110a3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
  2018-07-23 18:25 ` [igt-dev] " Radhakrishna Sripada
@ 2018-07-24  2:52   ` Dhinakaran Pandiyan
  -1 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-24  2:52 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev; +Cc: Daniel Vetter, intel-gfx

On Mon, 2018-07-23 at 11:25 -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)
> v4: Include the cherryview exception for reflect subtest(Maarten)
> v5: Rebase and move the check from CNL to ICL for reflect-x case
> v6: Fix the CI regression
> v7: rebase
> 
> [1]: https://patchwork.freedesktop.org/patch/209647/
> 

Oh well, I wrote my comments below and then read this link. Please add
new test requirements in separate patches. Only have the code movement
here.

> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: 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 | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 6cb5858adb0f..f20b8a6d4ba1 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 {
> @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> igt_output_t *output,
>  		igt_plane_set_position(plane, data->pos_x, data-
> >pos_y);
>  }
>  
> +static void igt_check_rotation(data_t *data)
> +{
> +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> +		igt_require(data->gen >= 9);
> +	if (data->rotation & IGT_REFLECT_X)
> +		igt_require(data->gen >= 11 ||

This check used to be igt_require(gen >= 10

> +			    (IS_CHERRYVIEW(data->devid) && (data-
> >rotation & IGT_ROTATION_0)));

There was also a check for tiling format 
-                                   (IS_CHERRYVIEW(data.devid) &&
reflect_x->rot == IGT_ROTATION_0
-                                    && reflect_x->tiling ==
LOCAL_I915_FORMAT_MOD_X_TILED));


> +	if (data->rotation & IGT_ROTATION_180)
> +		igt_require(data->gen >= 4);

Doesn't look like this requirement was is in the test earlier.

> +}
> +
>  static void test_single_case(data_t *data, enum pipe pipe,
>  			     igt_output_t *output, igt_plane_t
> *plane,
>  			     enum rectangle_type rect,
> @@ -352,15 +364,18 @@ 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);
>  
> +		if (IS_CHERRYVIEW(data->devid) && (data->rotation &
> IGT_REFLECT_X) &&
> +		    pipe != kmstest_pipe_to_index('B'))
> +			continue;
> +

Why do this? 

>  		plane = igt_output_get_plane_type(output,
> plane_type);
>  		igt_require(igt_plane_has_prop(plane,
> IGT_PLANE_ROTATION));
>  
> @@ -521,14 +536,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();
>  
> @@ -541,16 +555,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;
> @@ -560,7 +570,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);
> @@ -568,7 +577,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);
> @@ -579,9 +587,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);
> @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-24  2:52   ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-24  2:52 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev; +Cc: Daniel Vetter, intel-gfx

On Mon, 2018-07-23 at 11:25 -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)
> v4: Include the cherryview exception for reflect subtest(Maarten)
> v5: Rebase and move the check from CNL to ICL for reflect-x case
> v6: Fix the CI regression
> v7: rebase
> 
> [1]: https://patchwork.freedesktop.org/patch/209647/
> 

Oh well, I wrote my comments below and then read this link. Please add
new test requirements in separate patches. Only have the code movement
here.

> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: 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 | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 6cb5858adb0f..f20b8a6d4ba1 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 {
> @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> igt_output_t *output,
>  		igt_plane_set_position(plane, data->pos_x, data-
> >pos_y);
>  }
>  
> +static void igt_check_rotation(data_t *data)
> +{
> +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> +		igt_require(data->gen >= 9);
> +	if (data->rotation & IGT_REFLECT_X)
> +		igt_require(data->gen >= 11 ||

This check used to be igt_require(gen >= 10

> +			    (IS_CHERRYVIEW(data->devid) && (data-
> >rotation & IGT_ROTATION_0)));

There was also a check for tiling format 
-                                   (IS_CHERRYVIEW(data.devid) &&
reflect_x->rot == IGT_ROTATION_0
-                                    && reflect_x->tiling ==
LOCAL_I915_FORMAT_MOD_X_TILED));


> +	if (data->rotation & IGT_ROTATION_180)
> +		igt_require(data->gen >= 4);

Doesn't look like this requirement was is in the test earlier.

> +}
> +
>  static void test_single_case(data_t *data, enum pipe pipe,
>  			     igt_output_t *output, igt_plane_t
> *plane,
>  			     enum rectangle_type rect,
> @@ -352,15 +364,18 @@ 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);
>  
> +		if (IS_CHERRYVIEW(data->devid) && (data->rotation &
> IGT_REFLECT_X) &&
> +		    pipe != kmstest_pipe_to_index('B'))
> +			continue;
> +

Why do this? 

>  		plane = igt_output_get_plane_type(output,
> plane_type);
>  		igt_require(igt_plane_has_prop(plane,
> IGT_PLANE_ROTATION));
>  
> @@ -521,14 +536,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();
>  
> @@ -541,16 +555,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;
> @@ -560,7 +570,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);
> @@ -568,7 +577,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);
> @@ -579,9 +587,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);
> @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
  2018-07-24  2:52   ` [Intel-gfx] " Dhinakaran Pandiyan
@ 2018-07-24  3:26     ` Dhinakaran Pandiyan
  -1 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-24  3:26 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev; +Cc: Daniel Vetter, intel-gfx

On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-07-23 at 11:25 -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)
> > v4: Include the cherryview exception for reflect subtest(Maarten)
> > v5: Rebase and move the check from CNL to ICL for reflect-x case
> > v6: Fix the CI regression
> > v7: rebase
> > 
> > [1]: https://patchwork.freedesktop.org/patch/209647/
> > 
> Oh well, I wrote my comments below and then read this link. Please
> add
> new test requirements in separate patches. Only have the code
> movement
> here.
> 

Quoting Ville from [1] above

"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?"

If I am reading this correctly, this patch should have retained the
plane property checks in addition to exceptions you have added.

-DK

> > 
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: 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 | 35 ++++++++++++++++++++---------------
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > igt_output_t *output,
> >  		igt_plane_set_position(plane, data->pos_x, data-
> > > 
> > > pos_y);
> >  }
> >  
> > +static void igt_check_rotation(data_t *data)
> > +{
> > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > +		igt_require(data->gen >= 9);
> > +	if (data->rotation & IGT_REFLECT_X)
> > +		igt_require(data->gen >= 11 ||
> This check used to be igt_require(gen >= 10
> 
> > 
> > +			    (IS_CHERRYVIEW(data->devid) && (data-
> > > 
> > > rotation & IGT_ROTATION_0)));
> There was also a check for tiling format 
> -                                   (IS_CHERRYVIEW(data.devid) &&
> reflect_x->rot == IGT_ROTATION_0
> -                                    && reflect_x->tiling ==
> LOCAL_I915_FORMAT_MOD_X_TILED));
> 
> 
> > 
> > +	if (data->rotation & IGT_ROTATION_180)
> > +		igt_require(data->gen >= 4);
> Doesn't look like this requirement was is in the test earlier.
> 
> > 
> > +}
> > +
> >  static void test_single_case(data_t *data, enum pipe pipe,
> >  			     igt_output_t *output, igt_plane_t
> > *plane,
> >  			     enum rectangle_type rect,
> > @@ -352,15 +364,18 @@ 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);
> >  
> > +		if (IS_CHERRYVIEW(data->devid) && (data->rotation
> > &
> > IGT_REFLECT_X) &&
> > +		    pipe != kmstest_pipe_to_index('B'))
> > +			continue;
> > +
> Why do this? 
> 
> > 
> >  		plane = igt_output_get_plane_type(output,
> > plane_type);
> >  		igt_require(igt_plane_has_prop(plane,
> > IGT_PLANE_ROTATION));
> >  
> > @@ -521,14 +536,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();
> >  
> > @@ -541,16 +555,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;
> > @@ -560,7 +570,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);
> > @@ -568,7 +577,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);
> > @@ -579,9 +587,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);
> > @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-24  3:26     ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-24  3:26 UTC (permalink / raw)
  To: Radhakrishna Sripada, igt-dev; +Cc: Daniel Vetter, intel-gfx

On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-07-23 at 11:25 -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)
> > v4: Include the cherryview exception for reflect subtest(Maarten)
> > v5: Rebase and move the check from CNL to ICL for reflect-x case
> > v6: Fix the CI regression
> > v7: rebase
> > 
> > [1]: https://patchwork.freedesktop.org/patch/209647/
> > 
> Oh well, I wrote my comments below and then read this link. Please
> add
> new test requirements in separate patches. Only have the code
> movement
> here.
> 

Quoting Ville from [1] above

"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?"

If I am reading this correctly, this patch should have retained the
plane property checks in addition to exceptions you have added.

-DK

> > 
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: 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 | 35 ++++++++++++++++++++---------------
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > igt_output_t *output,
> >  		igt_plane_set_position(plane, data->pos_x, data-
> > > 
> > > pos_y);
> >  }
> >  
> > +static void igt_check_rotation(data_t *data)
> > +{
> > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > +		igt_require(data->gen >= 9);
> > +	if (data->rotation & IGT_REFLECT_X)
> > +		igt_require(data->gen >= 11 ||
> This check used to be igt_require(gen >= 10
> 
> > 
> > +			    (IS_CHERRYVIEW(data->devid) && (data-
> > > 
> > > rotation & IGT_ROTATION_0)));
> There was also a check for tiling format 
> -                                   (IS_CHERRYVIEW(data.devid) &&
> reflect_x->rot == IGT_ROTATION_0
> -                                    && reflect_x->tiling ==
> LOCAL_I915_FORMAT_MOD_X_TILED));
> 
> 
> > 
> > +	if (data->rotation & IGT_ROTATION_180)
> > +		igt_require(data->gen >= 4);
> Doesn't look like this requirement was is in the test earlier.
> 
> > 
> > +}
> > +
> >  static void test_single_case(data_t *data, enum pipe pipe,
> >  			     igt_output_t *output, igt_plane_t
> > *plane,
> >  			     enum rectangle_type rect,
> > @@ -352,15 +364,18 @@ 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);
> >  
> > +		if (IS_CHERRYVIEW(data->devid) && (data->rotation
> > &
> > IGT_REFLECT_X) &&
> > +		    pipe != kmstest_pipe_to_index('B'))
> > +			continue;
> > +
> Why do this? 
> 
> > 
> >  		plane = igt_output_get_plane_type(output,
> > plane_type);
> >  		igt_require(igt_plane_has_prop(plane,
> > IGT_PLANE_ROTATION));
> >  
> > @@ -521,14 +536,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();
> >  
> > @@ -541,16 +555,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;
> > @@ -560,7 +570,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);
> > @@ -568,7 +577,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);
> > @@ -579,9 +587,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);
> > @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
  2018-07-24  3:26     ` [Intel-gfx] " Dhinakaran Pandiyan
@ 2018-07-27 20:43       ` Radhakrishna Sripada
  -1 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-27 20:43 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, igt-dev, Daniel Vetter

On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > On Mon, 2018-07-23 at 11:25 -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)
> > > v4: Include the cherryview exception for reflect subtest(Maarten)
> > > v5: Rebase and move the check from CNL to ICL for reflect-x case
> > > v6: Fix the CI regression
> > > v7: rebase
> > > 
> > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > 
> > Oh well, I wrote my comments below and then read this link. Please
> > add
> > new test requirements in separate patches. Only have the code
> > movement
> > here.
> > 
> 
> Quoting Ville from [1] above
> 
> "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?"
> 
> If I am reading this correctly, this patch should have retained the
> plane property checks in addition to exceptions you have added.

Based on the feedback received from patch v2 [1], we moved back to using
the platform checks instead of using the connector property.


[1] https://patchwork.freedesktop.org/patch/209647/
> 
> -DK
> 
> > > 
> > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: 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 | 35 ++++++++++++++++++++---------------
> > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > igt_output_t *output,
> > >  		igt_plane_set_position(plane, data->pos_x, data-
> > > > 
> > > > pos_y);
> > >  }
> > >  
> > > +static void igt_check_rotation(data_t *data)
> > > +{
> > > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > +		igt_require(data->gen >= 9);
> > > +	if (data->rotation & IGT_REFLECT_X)
> > > +		igt_require(data->gen >= 11 ||
> > This check used to be igt_require(gen >= 10

Will move it back to gen10.
> > 
> > > 
> > > +			    (IS_CHERRYVIEW(data->devid) && (data-
> > > > 
> > > > rotation & IGT_ROTATION_0)));
> > There was also a check for tiling format 
> > -                                   (IS_CHERRYVIEW(data.devid) &&
> > reflect_x->rot == IGT_ROTATION_0
> > -                                    && reflect_x->tiling ==
> > LOCAL_I915_FORMAT_MOD_X_TILED));
> > 
> > 
> > > 
> > > +	if (data->rotation & IGT_ROTATION_180)
> > > +		igt_require(data->gen >= 4);
> > Doesn't look like this requirement was is in the test earlier.
> > 
This is the requirement present in the kernel and got included here
on suggestion from here[1].

> > > 
> > > +}
> > > +
> > >  static void test_single_case(data_t *data, enum pipe pipe,
> > >  			     igt_output_t *output, igt_plane_t
> > > *plane,
> > >  			     enum rectangle_type rect,
> > > @@ -352,15 +364,18 @@ 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);
> > >  
> > > +		if (IS_CHERRYVIEW(data->devid) && (data->rotation
> > > &
> > > IGT_REFLECT_X) &&
> > > +		    pipe != kmstest_pipe_to_index('B'))
> > > +			continue;
> > > +
> > Why do this? 
> > 
On braswell reflectx is supported only on Pipe B and the test case
needs to be skipped for pipes A,C.


Regards,
Radhhakrishna(RK) Sripada
> > > 
> > >  		plane = igt_output_get_plane_type(output,
> > > plane_type);
> > >  		igt_require(igt_plane_has_prop(plane,
> > > IGT_PLANE_ROTATION));
> > >  
> > > @@ -521,14 +536,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();
> > >  
> > > @@ -541,16 +555,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;
> > > @@ -560,7 +570,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);
> > > @@ -568,7 +577,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);
> > > @@ -579,9 +587,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);
> > > @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-27 20:43       ` Radhakrishna Sripada
  0 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-27 20:43 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Anusha Srivatsa, intel-gfx, igt-dev, Daniel Vetter

On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > On Mon, 2018-07-23 at 11:25 -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)
> > > v4: Include the cherryview exception for reflect subtest(Maarten)
> > > v5: Rebase and move the check from CNL to ICL for reflect-x case
> > > v6: Fix the CI regression
> > > v7: rebase
> > > 
> > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > 
> > Oh well, I wrote my comments below and then read this link. Please
> > add
> > new test requirements in separate patches. Only have the code
> > movement
> > here.
> > 
> 
> Quoting Ville from [1] above
> 
> "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?"
> 
> If I am reading this correctly, this patch should have retained the
> plane property checks in addition to exceptions you have added.

Based on the feedback received from patch v2 [1], we moved back to using
the platform checks instead of using the connector property.


[1] https://patchwork.freedesktop.org/patch/209647/
> 
> -DK
> 
> > > 
> > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: 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 | 35 ++++++++++++++++++++---------------
> > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > igt_output_t *output,
> > >  		igt_plane_set_position(plane, data->pos_x, data-
> > > > 
> > > > pos_y);
> > >  }
> > >  
> > > +static void igt_check_rotation(data_t *data)
> > > +{
> > > +	if (data->rotation & (IGT_ROTATION_90 | IGT_ROTATION_270))
> > > +		igt_require(data->gen >= 9);
> > > +	if (data->rotation & IGT_REFLECT_X)
> > > +		igt_require(data->gen >= 11 ||
> > This check used to be igt_require(gen >= 10

Will move it back to gen10.
> > 
> > > 
> > > +			    (IS_CHERRYVIEW(data->devid) && (data-
> > > > 
> > > > rotation & IGT_ROTATION_0)));
> > There was also a check for tiling format 
> > -                                   (IS_CHERRYVIEW(data.devid) &&
> > reflect_x->rot == IGT_ROTATION_0
> > -                                    && reflect_x->tiling ==
> > LOCAL_I915_FORMAT_MOD_X_TILED));
> > 
> > 
> > > 
> > > +	if (data->rotation & IGT_ROTATION_180)
> > > +		igt_require(data->gen >= 4);
> > Doesn't look like this requirement was is in the test earlier.
> > 
This is the requirement present in the kernel and got included here
on suggestion from here[1].

> > > 
> > > +}
> > > +
> > >  static void test_single_case(data_t *data, enum pipe pipe,
> > >  			     igt_output_t *output, igt_plane_t
> > > *plane,
> > >  			     enum rectangle_type rect,
> > > @@ -352,15 +364,18 @@ 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);
> > >  
> > > +		if (IS_CHERRYVIEW(data->devid) && (data->rotation
> > > &
> > > IGT_REFLECT_X) &&
> > > +		    pipe != kmstest_pipe_to_index('B'))
> > > +			continue;
> > > +
> > Why do this? 
> > 
On braswell reflectx is supported only on Pipe B and the test case
needs to be skipped for pipes A,C.


Regards,
Radhhakrishna(RK) Sripada
> > > 
> > >  		plane = igt_output_get_plane_type(output,
> > > plane_type);
> > >  		igt_require(igt_plane_has_prop(plane,
> > > IGT_PLANE_ROTATION));
> > >  
> > > @@ -521,14 +536,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();
> > >  
> > > @@ -541,16 +555,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;
> > > @@ -560,7 +570,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);
> > > @@ -568,7 +577,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);
> > > @@ -579,9 +587,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);
> > > @@ -596,7 +601,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) {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
  2018-07-27 20:43       ` Radhakrishna Sripada
@ 2018-07-27 22:06         ` Dhinakaran Pandiyan
  -1 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-27 22:06 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx, igt-dev, Daniel Vetter

On Fri, 2018-07-27 at 13:43 -0700, Radhakrishna Sripada wrote:
> On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Mon, 2018-07-23 at 11:25 -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)
> > > > v4: Include the cherryview exception for reflect
> > > > subtest(Maarten)
> > > > v5: Rebase and move the check from CNL to ICL for reflect-x
> > > > case
> > > > v6: Fix the CI regression
> > > > v7: rebase
> > > > 
> > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > > 
> > > Oh well, I wrote my comments below and then read this link.
> > > Please
> > > add
> > > new test requirements in separate patches. Only have the code
> > > movement
> > > here.
> > > 
> > Quoting Ville from [1] above
> > 
> > "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?"
> > 
> > If I am reading this correctly, this patch should have retained the
> > plane property checks in addition to exceptions you have added.
> Based on the feedback received from patch v2 [1], we moved back to
> using
> the platform checks instead of using the connector property.
> 

Okay, I am not reading the comment, specifically "make it as generic as
possible by checking the plane supported rotations", the way you are.
Check with Ville.

Anyway, if you want to add new platform checks, I do not recommend
adding all of them in a single patch.


> 
> [1] https://patchwork.freedesktop.org/patch/209647/
> > 
> > 
> > -DK
> > 
> > > 
> > > > 
> > > > 
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: 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 | 35 ++++++++++++++++++++----------
> > > > -----
> > > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_rotation_crc.c
> > > > b/tests/kms_rotation_crc.c
> > > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > > igt_output_t *output,
> > > >  		igt_plane_set_position(plane, data->pos_x,
> > > > data-
> > > > > 
> > > > > 
> > > > > pos_y);
> > > >  }
> > > >  
> > > > +static void igt_check_rotation(data_t *data)
> > > > +{
> > > > +	if (data->rotation & (IGT_ROTATION_90 |
> > > > IGT_ROTATION_270))
> > > > +		igt_require(data->gen >= 9);
> > > > +	if (data->rotation & IGT_REFLECT_X)
> > > > +		igt_require(data->gen >= 11 ||
> > > This check used to be igt_require(gen >= 10
> Will move it back to gen10.
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +			    (IS_CHERRYVIEW(data->devid) &&
> > > > (data-
> > > > > 
> > > > > 
> > > > > rotation & IGT_ROTATION_0)));
> > > There was also a check for tiling format 
> > > -                                   (IS_CHERRYVIEW(data.devid) &&
> > > reflect_x->rot == IGT_ROTATION_0
> > > -                                    && reflect_x->tiling ==
> > > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > 
> > > 
> > > > 
> > > > 
> > > > +	if (data->rotation & IGT_ROTATION_180)
> > > > +		igt_require(data->gen >= 4);
> > > Doesn't look like this requirement was is in the test earlier.
> > > 
> This is the requirement present in the kernel and got included here
> on suggestion from here[1].
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +}
> > > > +
> > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > >  			     igt_output_t *output, igt_plane_t
> > > > *plane,
> > > >  			     enum rectangle_type rect,
> > > > @@ -352,15 +364,18 @@ 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);
> > > >  
> > > > +		if (IS_CHERRYVIEW(data->devid) && (data-
> > > > >rotation
> > > > &
> > > > IGT_REFLECT_X) &&
> > > > +		    pipe != kmstest_pipe_to_index('B'))
> > > > +			continue;
> > > > +
> > > Why do this? 
> > > 
> On braswell reflectx is supported only on Pipe B and the test case
> needs to be skipped for pipes A,C.
> 
> 
> Regards,
> Radhhakrishna(RK) Sripada
> > 
> > > 
> > > > 
> > > > 
> > > >  		plane = igt_output_get_plane_type(output,
> > > > plane_type);
> > > >  		igt_require(igt_plane_has_prop(plane,
> > > > IGT_PLANE_ROTATION));
> > > >  
> > > > @@ -521,14 +536,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();
> > > >  
> > > > @@ -541,16 +555,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;
> > > > @@ -560,7 +570,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);
> > > > @@ -568,7 +577,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);
> > > > @@ -579,9 +587,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);
> > > > @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-27 22:06         ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 14+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-27 22:06 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: Anusha Srivatsa, intel-gfx, igt-dev, Daniel Vetter

On Fri, 2018-07-27 at 13:43 -0700, Radhakrishna Sripada wrote:
> On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Mon, 2018-07-23 at 11:25 -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)
> > > > v4: Include the cherryview exception for reflect
> > > > subtest(Maarten)
> > > > v5: Rebase and move the check from CNL to ICL for reflect-x
> > > > case
> > > > v6: Fix the CI regression
> > > > v7: rebase
> > > > 
> > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > > 
> > > Oh well, I wrote my comments below and then read this link.
> > > Please
> > > add
> > > new test requirements in separate patches. Only have the code
> > > movement
> > > here.
> > > 
> > Quoting Ville from [1] above
> > 
> > "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?"
> > 
> > If I am reading this correctly, this patch should have retained the
> > plane property checks in addition to exceptions you have added.
> Based on the feedback received from patch v2 [1], we moved back to
> using
> the platform checks instead of using the connector property.
> 

Okay, I am not reading the comment, specifically "make it as generic as
possible by checking the plane supported rotations", the way you are.
Check with Ville.

Anyway, if you want to add new platform checks, I do not recommend
adding all of them in a single patch.


> 
> [1] https://patchwork.freedesktop.org/patch/209647/
> > 
> > 
> > -DK
> > 
> > > 
> > > > 
> > > > 
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: 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 | 35 ++++++++++++++++++++----------
> > > > -----
> > > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_rotation_crc.c
> > > > b/tests/kms_rotation_crc.c
> > > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > > igt_output_t *output,
> > > >  		igt_plane_set_position(plane, data->pos_x,
> > > > data-
> > > > > 
> > > > > 
> > > > > pos_y);
> > > >  }
> > > >  
> > > > +static void igt_check_rotation(data_t *data)
> > > > +{
> > > > +	if (data->rotation & (IGT_ROTATION_90 |
> > > > IGT_ROTATION_270))
> > > > +		igt_require(data->gen >= 9);
> > > > +	if (data->rotation & IGT_REFLECT_X)
> > > > +		igt_require(data->gen >= 11 ||
> > > This check used to be igt_require(gen >= 10
> Will move it back to gen10.
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > +			    (IS_CHERRYVIEW(data->devid) &&
> > > > (data-
> > > > > 
> > > > > 
> > > > > rotation & IGT_ROTATION_0)));
> > > There was also a check for tiling format 
> > > -                                   (IS_CHERRYVIEW(data.devid) &&
> > > reflect_x->rot == IGT_ROTATION_0
> > > -                                    && reflect_x->tiling ==
> > > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > 
> > > 
> > > > 
> > > > 
> > > > +	if (data->rotation & IGT_ROTATION_180)
> > > > +		igt_require(data->gen >= 4);
> > > Doesn't look like this requirement was is in the test earlier.
> > > 
> This is the requirement present in the kernel and got included here
> on suggestion from here[1].
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +}
> > > > +
> > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > >  			     igt_output_t *output, igt_plane_t
> > > > *plane,
> > > >  			     enum rectangle_type rect,
> > > > @@ -352,15 +364,18 @@ 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);
> > > >  
> > > > +		if (IS_CHERRYVIEW(data->devid) && (data-
> > > > >rotation
> > > > &
> > > > IGT_REFLECT_X) &&
> > > > +		    pipe != kmstest_pipe_to_index('B'))
> > > > +			continue;
> > > > +
> > > Why do this? 
> > > 
> On braswell reflectx is supported only on Pipe B and the test case
> needs to be skipped for pipes A,C.
> 
> 
> Regards,
> Radhhakrishna(RK) Sripada
> > 
> > > 
> > > > 
> > > > 
> > > >  		plane = igt_output_get_plane_type(output,
> > > > plane_type);
> > > >  		igt_require(igt_plane_has_prop(plane,
> > > > IGT_PLANE_ROTATION));
> > > >  
> > > > @@ -521,14 +536,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();
> > > >  
> > > > @@ -541,16 +555,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;
> > > > @@ -560,7 +570,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);
> > > > @@ -568,7 +577,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);
> > > > @@ -579,9 +587,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);
> > > > @@ -596,7 +601,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) {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
  2018-07-27 22:06         ` Dhinakaran Pandiyan
@ 2018-07-27 22:26           ` Radhakrishna Sripada
  -1 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-27 22:26 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, igt-dev, Daniel Vetter

On Fri, Jul 27, 2018 at 03:06:19PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-07-27 at 13:43 -0700, Radhakrishna Sripada wrote:
> > On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > > > 
> > > > On Mon, 2018-07-23 at 11:25 -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)
> > > > > v4: Include the cherryview exception for reflect
> > > > > subtest(Maarten)
> > > > > v5: Rebase and move the check from CNL to ICL for reflect-x
> > > > > case
> > > > > v6: Fix the CI regression
> > > > > v7: rebase
> > > > > 
> > > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > > > 
> > > > Oh well, I wrote my comments below and then read this link.
> > > > Please
> > > > add
> > > > new test requirements in separate patches. Only have the code
> > > > movement
> > > > here.
> > > > 
> > > Quoting Ville from [1] above
> > > 
> > > "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?"
> > > 
> > > If I am reading this correctly, this patch should have retained the
> > > plane property checks in addition to exceptions you have added.
> > Based on the feedback received from patch v2 [1], we moved back to
> > using
> > the platform checks instead of using the connector property.
> > 
> 
> Okay, I am not reading the comment, specifically "make it as generic as
> possible by checking the plane supported rotations", the way you are.
> Check with Ville.
Based on that feedback incorporated the changes in v3[2].
> 
> Anyway, if you want to add new platform checks, I do not recommend
> adding all of them in a single patch.
No new checks were added apart from the ones already supported by the driver.

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

-RK
> > > 
> > > 
> > > -DK
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: 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 | 35 ++++++++++++++++++++----------
> > > > > -----
> > > > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/tests/kms_rotation_crc.c
> > > > > b/tests/kms_rotation_crc.c
> > > > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > > > igt_output_t *output,
> > > > >  		igt_plane_set_position(plane, data->pos_x,
> > > > > data-
> > > > > > 
> > > > > > 
> > > > > > pos_y);
> > > > >  }
> > > > >  
> > > > > +static void igt_check_rotation(data_t *data)
> > > > > +{
> > > > > +	if (data->rotation & (IGT_ROTATION_90 |
> > > > > IGT_ROTATION_270))
> > > > > +		igt_require(data->gen >= 9);
> > > > > +	if (data->rotation & IGT_REFLECT_X)
> > > > > +		igt_require(data->gen >= 11 ||
> > > > This check used to be igt_require(gen >= 10
> > Will move it back to gen10.
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +			    (IS_CHERRYVIEW(data->devid) &&
> > > > > (data-
> > > > > > 
> > > > > > 
> > > > > > rotation & IGT_ROTATION_0)));
> > > > There was also a check for tiling format 
> > > > -                                   (IS_CHERRYVIEW(data.devid) &&
> > > > reflect_x->rot == IGT_ROTATION_0
> > > > -                                    && reflect_x->tiling ==
> > > > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +	if (data->rotation & IGT_ROTATION_180)
> > > > > +		igt_require(data->gen >= 4);
> > > > Doesn't look like this requirement was is in the test earlier.
> > > > 
> > This is the requirement present in the kernel and got included here
> > on suggestion from here[1].
> > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +}
> > > > > +
> > > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > > >  			     igt_output_t *output, igt_plane_t
> > > > > *plane,
> > > > >  			     enum rectangle_type rect,
> > > > > @@ -352,15 +364,18 @@ 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);
> > > > >  
> > > > > +		if (IS_CHERRYVIEW(data->devid) && (data-
> > > > > >rotation
> > > > > &
> > > > > IGT_REFLECT_X) &&
> > > > > +		    pipe != kmstest_pipe_to_index('B'))
> > > > > +			continue;
> > > > > +
> > > > Why do this? 
> > > > 
> > On braswell reflectx is supported only on Pipe B and the test case
> > needs to be skipped for pipes A,C.
> > 
> > 
> > Regards,
> > Radhhakrishna(RK) Sripada
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > >  		plane = igt_output_get_plane_type(output,
> > > > > plane_type);
> > > > >  		igt_require(igt_plane_has_prop(plane,
> > > > > IGT_PLANE_ROTATION));
> > > > >  
> > > > > @@ -521,14 +536,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();
> > > > >  
> > > > > @@ -541,16 +555,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;
> > > > > @@ -560,7 +570,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);
> > > > > @@ -568,7 +577,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);
> > > > > @@ -579,9 +587,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);
> > > > > @@ -596,7 +601,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) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases
@ 2018-07-27 22:26           ` Radhakrishna Sripada
  0 siblings, 0 replies; 14+ messages in thread
From: Radhakrishna Sripada @ 2018-07-27 22:26 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Anusha Srivatsa, intel-gfx, igt-dev, Daniel Vetter

On Fri, Jul 27, 2018 at 03:06:19PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-07-27 at 13:43 -0700, Radhakrishna Sripada wrote:
> > On Mon, Jul 23, 2018 at 08:26:06PM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Mon, 2018-07-23 at 19:52 -0700, Dhinakaran Pandiyan wrote:
> > > > 
> > > > On Mon, 2018-07-23 at 11:25 -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)
> > > > > v4: Include the cherryview exception for reflect
> > > > > subtest(Maarten)
> > > > > v5: Rebase and move the check from CNL to ICL for reflect-x
> > > > > case
> > > > > v6: Fix the CI regression
> > > > > v7: rebase
> > > > > 
> > > > > [1]: https://patchwork.freedesktop.org/patch/209647/
> > > > > 
> > > > Oh well, I wrote my comments below and then read this link.
> > > > Please
> > > > add
> > > > new test requirements in separate patches. Only have the code
> > > > movement
> > > > here.
> > > > 
> > > Quoting Ville from [1] above
> > > 
> > > "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?"
> > > 
> > > If I am reading this correctly, this patch should have retained the
> > > plane property checks in addition to exceptions you have added.
> > Based on the feedback received from patch v2 [1], we moved back to
> > using
> > the platform checks instead of using the connector property.
> > 
> 
> Okay, I am not reading the comment, specifically "make it as generic as
> possible by checking the plane supported rotations", the way you are.
> Check with Ville.
Based on that feedback incorporated the changes in v3[2].
> 
> Anyway, if you want to add new platform checks, I do not recommend
> adding all of them in a single patch.
No new checks were added apart from the ones already supported by the driver.

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

-RK
> > > 
> > > 
> > > -DK
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: 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 | 35 ++++++++++++++++++++----------
> > > > > -----
> > > > >  1 file changed, 20 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/tests/kms_rotation_crc.c
> > > > > b/tests/kms_rotation_crc.c
> > > > > index 6cb5858adb0f..f20b8a6d4ba1 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 {
> > > > > @@ -284,6 +285,17 @@ static void prepare_fbs(data_t *data,
> > > > > igt_output_t *output,
> > > > >  		igt_plane_set_position(plane, data->pos_x,
> > > > > data-
> > > > > > 
> > > > > > 
> > > > > > pos_y);
> > > > >  }
> > > > >  
> > > > > +static void igt_check_rotation(data_t *data)
> > > > > +{
> > > > > +	if (data->rotation & (IGT_ROTATION_90 |
> > > > > IGT_ROTATION_270))
> > > > > +		igt_require(data->gen >= 9);
> > > > > +	if (data->rotation & IGT_REFLECT_X)
> > > > > +		igt_require(data->gen >= 11 ||
> > > > This check used to be igt_require(gen >= 10
> > Will move it back to gen10.
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +			    (IS_CHERRYVIEW(data->devid) &&
> > > > > (data-
> > > > > > 
> > > > > > 
> > > > > > rotation & IGT_ROTATION_0)));
> > > > There was also a check for tiling format 
> > > > -                                   (IS_CHERRYVIEW(data.devid) &&
> > > > reflect_x->rot == IGT_ROTATION_0
> > > > -                                    && reflect_x->tiling ==
> > > > LOCAL_I915_FORMAT_MOD_X_TILED));
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +	if (data->rotation & IGT_ROTATION_180)
> > > > > +		igt_require(data->gen >= 4);
> > > > Doesn't look like this requirement was is in the test earlier.
> > > > 
> > This is the requirement present in the kernel and got included here
> > on suggestion from here[1].
> > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +}
> > > > > +
> > > > >  static void test_single_case(data_t *data, enum pipe pipe,
> > > > >  			     igt_output_t *output, igt_plane_t
> > > > > *plane,
> > > > >  			     enum rectangle_type rect,
> > > > > @@ -352,15 +364,18 @@ 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);
> > > > >  
> > > > > +		if (IS_CHERRYVIEW(data->devid) && (data-
> > > > > >rotation
> > > > > &
> > > > > IGT_REFLECT_X) &&
> > > > > +		    pipe != kmstest_pipe_to_index('B'))
> > > > > +			continue;
> > > > > +
> > > > Why do this? 
> > > > 
> > On braswell reflectx is supported only on Pipe B and the test case
> > needs to be skipped for pipes A,C.
> > 
> > 
> > Regards,
> > Radhhakrishna(RK) Sripada
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > >  		plane = igt_output_get_plane_type(output,
> > > > > plane_type);
> > > > >  		igt_require(igt_plane_has_prop(plane,
> > > > > IGT_PLANE_ROTATION));
> > > > >  
> > > > > @@ -521,14 +536,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();
> > > > >  
> > > > > @@ -541,16 +555,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;
> > > > > @@ -560,7 +570,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);
> > > > > @@ -568,7 +577,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);
> > > > > @@ -579,9 +587,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);
> > > > > @@ -596,7 +601,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) {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2018-07-27 22:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-23 18:25 [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases Radhakrishna Sripada
2018-07-23 18:25 ` [igt-dev] " Radhakrishna Sripada
2018-07-23 18:58 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases (rev5) Patchwork
2018-07-23 20:23 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2018-07-24  2:52 ` [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases Dhinakaran Pandiyan
2018-07-24  2:52   ` [Intel-gfx] " Dhinakaran Pandiyan
2018-07-24  3:26   ` Dhinakaran Pandiyan
2018-07-24  3:26     ` [Intel-gfx] " Dhinakaran Pandiyan
2018-07-27 20:43     ` Radhakrishna Sripada
2018-07-27 20:43       ` Radhakrishna Sripada
2018-07-27 22:06       ` Dhinakaran Pandiyan
2018-07-27 22:06         ` Dhinakaran Pandiyan
2018-07-27 22:26         ` Radhakrishna Sripada
2018-07-27 22:26           ` Radhakrishna Sripada

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.