* [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
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
` (2 more replies)
0 siblings, 3 replies; 8+ 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] 8+ 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] [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:58 ` Patchwork 2018-07-23 20:23 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2018-07-24 2:52 ` [Intel-gfx] [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases Dhinakaran Pandiyan 2 siblings, 0 replies; 8+ 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] 8+ 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] [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: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 ` Patchwork 2018-07-24 2:52 ` [Intel-gfx] [igt-dev] [PATCH i-g-t v7] tests/kms_rotation_crc: Move platform checks to one place for non exhaust fence cases Dhinakaran Pandiyan 2 siblings, 0 replies; 8+ 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] 8+ 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-23 18:25 [igt-dev] [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: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 ` Dhinakaran Pandiyan 2018-07-24 3:26 ` Dhinakaran Pandiyan 2 siblings, 1 reply; 8+ 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] 8+ 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 ` [Intel-gfx] [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 3:26 ` Dhinakaran Pandiyan 2018-07-27 20:43 ` Radhakrishna Sripada 0 siblings, 1 reply; 8+ 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] 8+ 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 ` Dhinakaran Pandiyan @ 2018-07-27 20:43 ` Radhakrishna Sripada 2018-07-27 22:06 ` Dhinakaran Pandiyan 0 siblings, 1 reply; 8+ 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] 8+ 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 2018-07-27 22:26 ` Radhakrishna Sripada 0 siblings, 1 reply; 8+ 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] 8+ 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 0 siblings, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2018-07-27 22:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-23 18:25 [igt-dev] [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: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 ` [Intel-gfx] [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 3:26 ` Dhinakaran Pandiyan 2018-07-27 20:43 ` Radhakrishna Sripada 2018-07-27 22:06 ` Dhinakaran Pandiyan 2018-07-27 22:26 ` Radhakrishna Sripada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).