* [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
@ 2022-02-22 10:38 Jouni Högander
2022-02-22 11:16 ` Petri Latvala
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jouni Högander @ 2022-02-22 10:38 UTC (permalink / raw)
To: igt-dev
Add new testcases which are moving cursor and overlay planes
without setting any damage areas.
Cursor testcase is reproducing following bug:
https://gitlab.freedesktop.org/drm/intel/-/issues/5077
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
tests/i915/kms_psr2_sf.c | 144 ++++++++++++++++++++++++++++++++++++---
1 file changed, 136 insertions(+), 8 deletions(-)
diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
index 36f8a786..b9990f87 100644
--- a/tests/i915/kms_psr2_sf.c
+++ b/tests/i915/kms_psr2_sf.c
@@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2 selective fetch by sending multiple"
enum operations {
PLANE_UPDATE,
PLANE_UPDATE_CONTINUOUS,
+ PLANE_MOVE_DAMAGED,
PLANE_MOVE,
OVERLAY_PRIM_UPDATE
};
@@ -51,7 +52,9 @@ enum plane_move_postion {
POS_TOP_LEFT,
POS_TOP_RIGHT,
POS_BOTTOM_LEFT,
- POS_BOTTOM_RIGHT
+ POS_BOTTOM_RIGHT,
+ POS_BOTTOM_LEFT_NEGATIVE,
+ POS_TOP_RIGHT_NEGATIVE,
};
typedef struct {
@@ -74,6 +77,7 @@ typedef struct {
igt_plane_t *test_plane;
cairo_t *cr;
uint32_t screen_changes;
+ int cur_x, cur_y;
} data_t;
static const char *op_str(enum operations op)
@@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
static const char * const name[] = {
[PLANE_UPDATE] = "plane-update",
[PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
- [PLANE_MOVE] = "plane-move",
+ [PLANE_MOVE] = "plane-move-without-damage",
+ [PLANE_MOVE_DAMAGED] = "plane-move",
[OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
};
@@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t *data, igt_fb_t *fb, uint32_t h,
}
}
-static void plane_move_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
+static void plane_move_damaged_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
uint32_t v)
{
int x = 0, y = 0;
@@ -260,8 +265,8 @@ static void prepare(data_t *data)
data->fb_continuous = &data->fb_overlay;
- if (data->op == PLANE_MOVE) {
- plane_move_setup_square(data, &data->fb_test,
+ if (data->op == PLANE_MOVE_DAMAGED) {
+ plane_move_damaged_setup_square(data, &data->fb_test,
data->mode->hdisplay/2,
data->mode->vdisplay/2);
@@ -273,6 +278,7 @@ static void prepare(data_t *data)
igt_plane_set_fb(sprite, &data->fb_overlay);
data->test_plane = sprite;
+
break;
case DRM_PLANE_TYPE_PRIMARY:
@@ -335,6 +341,8 @@ static void prepare(data_t *data)
igt_assert(false);
}
+ data->cur_x = data->cur_y = 0;
+
igt_plane_set_fb(primary, &data->fb_primary);
igt_display_commit2(&data->display, COMMIT_ATOMIC);
@@ -378,7 +386,7 @@ static void plane_update_expected_output(int plane_type, int box_count,
manual(expected);
}
-static void plane_move_expected_output(enum plane_move_postion pos)
+static void plane_move_damaged_expected_output(enum plane_move_postion pos)
{
char expected[64] = {};
@@ -406,6 +414,42 @@ static void plane_move_expected_output(enum plane_move_postion pos)
manual(expected);
}
+static void plane_move_expected_output(enum plane_move_postion pos)
+{
+ char expected[73] = {};
+
+ switch (pos) {
+ case POS_TOP_LEFT:
+ sprintf(expected,
+ "screen Green with Blue box on top left corner");
+ break;
+ case POS_TOP_RIGHT:
+ sprintf(expected,
+ "screen Green with Blue box on top right corner");
+ break;
+ case POS_BOTTOM_LEFT:
+ sprintf(expected,
+ "screen Green with Blue box on bottom left corner");
+ break;
+ case POS_BOTTOM_RIGHT:
+ sprintf(expected,
+ "screen Green with Blue box on bottom right corner");
+ break;
+ case POS_BOTTOM_LEFT_NEGATIVE:
+ sprintf(expected,
+ "screen Green with Blue box on bottom left corner (partly exceeding area)");
+ break;
+ case POS_TOP_RIGHT_NEGATIVE:
+ sprintf(expected,
+ "screen Green with Blue box on top right corner (partly exceeding area)");
+ break;
+ default:
+ igt_assert(false);
+ }
+
+ manual(expected);
+}
+
static void overlay_prim_update_expected_output(int box_count)
{
char expected[64] = {};
@@ -421,6 +465,9 @@ static void overlay_prim_update_expected_output(int box_count)
static void expected_output(data_t *data)
{
switch (data->op) {
+ case PLANE_MOVE_DAMAGED:
+ plane_move_damaged_expected_output(data->pos);
+ break;
case PLANE_MOVE:
plane_move_expected_output(data->pos);
break;
@@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
expected_output(data);
}
+static void plane_move(data_t *data)
+{
+ int target_x, target_y;
+
+ switch (data->pos) {
+ case POS_TOP_LEFT:
+ target_x = 0;
+ target_y = 0;
+ break;
+ case POS_TOP_RIGHT:
+ target_x = data->mode->hdisplay - data->fb_test.width;
+ target_y = 0;
+ break;
+ case POS_TOP_RIGHT_NEGATIVE:
+ target_x = data->mode->hdisplay - data->fb_test.width;
+ target_y = -data->fb_test.width / 2;
+ break;
+ case POS_BOTTOM_LEFT:
+ target_x = 0;
+ target_y = data->mode->vdisplay - data->fb_test.height;
+ break;
+ case POS_BOTTOM_LEFT_NEGATIVE:
+ target_x = -data->fb_test.width / 2;
+ target_y = data->mode->vdisplay - data->fb_test.height;
+ break;
+ case POS_BOTTOM_RIGHT:
+ target_x = data->mode->hdisplay - data->fb_test.width;
+ target_y = data->mode->vdisplay - data->fb_test.height;
+ break;
+ default:
+ igt_assert(false);
+ }
+
+ while (data->cur_x != target_x || data->cur_y != target_y) {
+ if (data->cur_x < target_x)
+ data->cur_x += min(target_x - data->cur_x, 20);
+ else if (data->cur_x > target_x)
+ data->cur_x -= min(data->cur_x - target_x, 20);
+
+ if (data->cur_y < target_y)
+ data->cur_y += min(target_y - data->cur_y, 20);
+ else if (data->cur_y > target_y)
+ data->cur_y -= min(data->cur_y - target_y, 20);
+
+ igt_plane_set_position(data->test_plane, data->cur_x, data->cur_y);
+ igt_display_commit2(&data->display, COMMIT_ATOMIC);
+ }
+
+ igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
+
+ expected_output(data);
+}
+
static void damaged_plane_update(data_t *data)
{
igt_plane_t *test_plane = data->test_plane;
@@ -526,6 +626,8 @@ static void damaged_plane_update(data_t *data)
static void run(data_t *data)
{
+ int i;
+
igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
data->screen_changes = 0;
@@ -542,9 +644,15 @@ static void run(data_t *data)
damaged_plane_update(data);
}
break;
- case PLANE_MOVE:
+ case PLANE_MOVE_DAMAGED:
damaged_plane_move(data);
break;
+ case PLANE_MOVE:
+ for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
+ data->pos = i;
+ plane_move(data);
+ }
+ break;
default:
igt_assert(false);
}
@@ -654,10 +762,20 @@ igt_main
cleanup(&data);
}
- /* Only for overlay plane */
data.op = PLANE_MOVE;
/* Verify overlay plane move selective fetch */
igt_describe("Test that selective fetch works on moving overlay plane");
+ igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
+ data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
+ prepare(&data);
+ run(&data);
+ cleanup(&data);
+ }
+
+ /* Only for overlay plane */
+ data.op = PLANE_MOVE_DAMAGED;
+ /* Verify overlay plane move selective fetch */
+ igt_describe("Test that selective fetch works on moving overlay plane");
igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
data.pos = i;
@@ -668,6 +786,16 @@ igt_main
}
}
+ data.op = PLANE_MOVE;
+ /* Verify overlay plane move selective fetch */
+ igt_describe("Test that selective fetch works on moving overlay plane");
+ igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
+ data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
+ prepare(&data);
+ run(&data);
+ cleanup(&data);
+ }
+
/* Verify primary plane selective fetch with overplay plane blended */
data.op = OVERLAY_PRIM_UPDATE;
igt_describe("Test that selective fetch works on primary plane "
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-22 10:38 [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane Jouni Högander
@ 2022-02-22 11:16 ` Petri Latvala
2022-02-22 11:48 ` Hogander, Jouni
2022-02-22 11:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2022-02-22 16:38 ` [igt-dev] [PATCH i-g-t] " Souza, Jose
2 siblings, 1 reply; 7+ messages in thread
From: Petri Latvala @ 2022-02-22 11:16 UTC (permalink / raw)
To: Jouni Högander; +Cc: igt-dev
On Tue, Feb 22, 2022 at 12:38:41PM +0200, Jouni Högander wrote:
> Add new testcases which are moving cursor and overlay planes
> without setting any damage areas.
>
> Cursor testcase is reproducing following bug:
>
> https://gitlab.freedesktop.org/drm/intel/-/issues/5077
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
> tests/i915/kms_psr2_sf.c | 144 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 136 insertions(+), 8 deletions(-)
>
> diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
> index 36f8a786..b9990f87 100644
> --- a/tests/i915/kms_psr2_sf.c
> +++ b/tests/i915/kms_psr2_sf.c
> @@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2 selective fetch by sending multiple"
> enum operations {
> PLANE_UPDATE,
> PLANE_UPDATE_CONTINUOUS,
> + PLANE_MOVE_DAMAGED,
> PLANE_MOVE,
> OVERLAY_PRIM_UPDATE
> };
> @@ -51,7 +52,9 @@ enum plane_move_postion {
> POS_TOP_LEFT,
> POS_TOP_RIGHT,
> POS_BOTTOM_LEFT,
> - POS_BOTTOM_RIGHT
> + POS_BOTTOM_RIGHT,
> + POS_BOTTOM_LEFT_NEGATIVE,
> + POS_TOP_RIGHT_NEGATIVE,
> };
>
> typedef struct {
> @@ -74,6 +77,7 @@ typedef struct {
> igt_plane_t *test_plane;
> cairo_t *cr;
> uint32_t screen_changes;
> + int cur_x, cur_y;
> } data_t;
>
> static const char *op_str(enum operations op)
> @@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
> static const char * const name[] = {
> [PLANE_UPDATE] = "plane-update",
> [PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
> - [PLANE_MOVE] = "plane-move",
> + [PLANE_MOVE] = "plane-move-without-damage",
> + [PLANE_MOVE_DAMAGED] = "plane-move",
> [OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> };
>
> @@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t *data, igt_fb_t *fb, uint32_t h,
> }
> }
>
> -static void plane_move_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
> +static void plane_move_damaged_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
> uint32_t v)
> {
> int x = 0, y = 0;
> @@ -260,8 +265,8 @@ static void prepare(data_t *data)
>
> data->fb_continuous = &data->fb_overlay;
>
> - if (data->op == PLANE_MOVE) {
> - plane_move_setup_square(data, &data->fb_test,
> + if (data->op == PLANE_MOVE_DAMAGED) {
> + plane_move_damaged_setup_square(data, &data->fb_test,
> data->mode->hdisplay/2,
> data->mode->vdisplay/2);
>
> @@ -273,6 +278,7 @@ static void prepare(data_t *data)
>
> igt_plane_set_fb(sprite, &data->fb_overlay);
> data->test_plane = sprite;
> +
> break;
>
> case DRM_PLANE_TYPE_PRIMARY:
> @@ -335,6 +341,8 @@ static void prepare(data_t *data)
> igt_assert(false);
> }
>
> + data->cur_x = data->cur_y = 0;
> +
> igt_plane_set_fb(primary, &data->fb_primary);
>
> igt_display_commit2(&data->display, COMMIT_ATOMIC);
> @@ -378,7 +386,7 @@ static void plane_update_expected_output(int plane_type, int box_count,
> manual(expected);
> }
>
> -static void plane_move_expected_output(enum plane_move_postion pos)
> +static void plane_move_damaged_expected_output(enum plane_move_postion pos)
> {
> char expected[64] = {};
>
> @@ -406,6 +414,42 @@ static void plane_move_expected_output(enum plane_move_postion pos)
> manual(expected);
> }
>
> +static void plane_move_expected_output(enum plane_move_postion pos)
> +{
> + char expected[73] = {};
> +
> + switch (pos) {
> + case POS_TOP_LEFT:
> + sprintf(expected,
> + "screen Green with Blue box on top left corner");
> + break;
> + case POS_TOP_RIGHT:
> + sprintf(expected,
> + "screen Green with Blue box on top right corner");
> + break;
> + case POS_BOTTOM_LEFT:
> + sprintf(expected,
> + "screen Green with Blue box on bottom left corner");
> + break;
> + case POS_BOTTOM_RIGHT:
> + sprintf(expected,
> + "screen Green with Blue box on bottom right corner");
> + break;
> + case POS_BOTTOM_LEFT_NEGATIVE:
> + sprintf(expected,
> + "screen Green with Blue box on bottom left corner (partly exceeding area)");
> + break;
> + case POS_TOP_RIGHT_NEGATIVE:
> + sprintf(expected,
> + "screen Green with Blue box on top right corner (partly exceeding area)");
> + break;
> + default:
> + igt_assert(false);
> + }
> +
> + manual(expected);
> +}
> +
> static void overlay_prim_update_expected_output(int box_count)
> {
> char expected[64] = {};
> @@ -421,6 +465,9 @@ static void overlay_prim_update_expected_output(int box_count)
> static void expected_output(data_t *data)
> {
> switch (data->op) {
> + case PLANE_MOVE_DAMAGED:
> + plane_move_damaged_expected_output(data->pos);
> + break;
> case PLANE_MOVE:
> plane_move_expected_output(data->pos);
> break;
> @@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
> expected_output(data);
> }
>
> +static void plane_move(data_t *data)
> +{
> + int target_x, target_y;
> +
> + switch (data->pos) {
> + case POS_TOP_LEFT:
> + target_x = 0;
> + target_y = 0;
> + break;
> + case POS_TOP_RIGHT:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = 0;
> + break;
> + case POS_TOP_RIGHT_NEGATIVE:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = -data->fb_test.width / 2;
> + break;
> + case POS_BOTTOM_LEFT:
> + target_x = 0;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + case POS_BOTTOM_LEFT_NEGATIVE:
> + target_x = -data->fb_test.width / 2;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + case POS_BOTTOM_RIGHT:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + default:
> + igt_assert(false);
> + }
> +
> + while (data->cur_x != target_x || data->cur_y != target_y) {
> + if (data->cur_x < target_x)
> + data->cur_x += min(target_x - data->cur_x, 20);
> + else if (data->cur_x > target_x)
> + data->cur_x -= min(data->cur_x - target_x, 20);
> +
> + if (data->cur_y < target_y)
> + data->cur_y += min(target_y - data->cur_y, 20);
> + else if (data->cur_y > target_y)
> + data->cur_y -= min(data->cur_y - target_y, 20);
> +
> + igt_plane_set_position(data->test_plane, data->cur_x, data->cur_y);
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> + }
> +
> + igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> + expected_output(data);
> +}
> +
> static void damaged_plane_update(data_t *data)
> {
> igt_plane_t *test_plane = data->test_plane;
> @@ -526,6 +626,8 @@ static void damaged_plane_update(data_t *data)
>
> static void run(data_t *data)
> {
> + int i;
> +
> igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
>
> data->screen_changes = 0;
> @@ -542,9 +644,15 @@ static void run(data_t *data)
> damaged_plane_update(data);
> }
> break;
> - case PLANE_MOVE:
> + case PLANE_MOVE_DAMAGED:
> damaged_plane_move(data);
> break;
> + case PLANE_MOVE:
> + for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
> + data->pos = i;
> + plane_move(data);
> + }
> + break;
> default:
> igt_assert(false);
> }
> @@ -654,10 +762,20 @@ igt_main
> cleanup(&data);
> }
>
> - /* Only for overlay plane */
> data.op = PLANE_MOVE;
> /* Verify overlay plane move selective fetch */
> igt_describe("Test that selective fetch works on moving overlay plane");
> + igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> + data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> + prepare(&data);
> + run(&data);
> + cleanup(&data);
> + }
> +
> + /* Only for overlay plane */
> + data.op = PLANE_MOVE_DAMAGED;
> + /* Verify overlay plane move selective fetch */
> + igt_describe("Test that selective fetch works on moving overlay plane");
> igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
> for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> data.pos = i;
> @@ -668,6 +786,16 @@ igt_main
> }
> }
>
> + data.op = PLANE_MOVE;
> + /* Verify overlay plane move selective fetch */
> + igt_describe("Test that selective fetch works on moving overlay plane");
> + igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
> + data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> + prepare(&data);
> + run(&data);
> + cleanup(&data);
> + }
Same text on three igt_describes and subtest names sound like some of
them are copypaste errors.
--
Petri Latvala
> +
> /* Verify primary plane selective fetch with overplay plane blended */
> data.op = OVERLAY_PRIM_UPDATE;
> igt_describe("Test that selective fetch works on primary plane "
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-22 10:38 [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane Jouni Högander
2022-02-22 11:16 ` Petri Latvala
@ 2022-02-22 11:27 ` Patchwork
2022-02-22 16:38 ` [igt-dev] [PATCH i-g-t] " Souza, Jose
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2022-02-22 11:27 UTC (permalink / raw)
To: Jouni Högander; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 8062 bytes --]
== Series Details ==
Series: tests/kms_psr2_sf: Add new testcases for moving plane
URL : https://patchwork.freedesktop.org/series/100559/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11264 -> IGTPW_6672
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_6672 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_6672, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/index.html
Participating hosts (44 -> 41)
------------------------------
Additional (2): bat-dg2-8 fi-bdw-5557u
Missing (5): fi-kbl-soraka shard-tglu fi-bsw-cyan shard-rkl shard-dg1
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_6672:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@hangcheck:
- fi-rkl-guc: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-rkl-guc/igt@i915_selftest@live@hangcheck.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-rkl-guc/igt@i915_selftest@live@hangcheck.html
Known issues
------------
Here are the changes found in IGTPW_6672 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s3@smem:
- fi-bdw-5557u: NOTRUN -> [INCOMPLETE][3] ([i915#146])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3@smem.html
* igt@gem_huc_copy@huc-copy:
- fi-skl-6600u: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@verify-random:
- fi-skl-6600u: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html
* igt@i915_selftest@live@execlists:
- fi-bsw-n3050: [PASS][6] -> [INCOMPLETE][7] ([i915#2940])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
* igt@kms_chamelium@vga-edid-read:
- fi-skl-6600u: NOTRUN -> [SKIP][8] ([fdo#109271] / [fdo#111827]) +8 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-skl-6600u: NOTRUN -> [SKIP][9] ([fdo#109271]) +21 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-skl-6600u: NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#533])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
* igt@runner@aborted:
- fi-bsw-n3050: NOTRUN -> [FAIL][11] ([fdo#109271] / [i915#1436] / [i915#2722] / [i915#3428] / [i915#4312])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-bsw-n3050/igt@runner@aborted.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3@smem:
- fi-skl-6600u: [INCOMPLETE][12] ([i915#4547]) -> [PASS][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html
* igt@i915_selftest@live@gt_pm:
- fi-tgl-1115g4: [DMESG-FAIL][14] ([i915#3987]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@gt_timelines:
- {fi-tgl-dsi}: [INCOMPLETE][16] -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-tgl-dsi/igt@i915_selftest@live@gt_timelines.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-tgl-dsi/igt@i915_selftest@live@gt_timelines.html
* igt@i915_selftest@live@hangcheck:
- bat-dg1-6: [DMESG-FAIL][18] ([i915#4494] / [i915#4957]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
* igt@kms_frontbuffer_tracking@basic:
- fi-cml-u2: [DMESG-WARN][20] ([i915#4269]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
#### Warnings ####
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][22] ([i915#3303]) -> [INCOMPLETE][23] ([i915#4785])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
[i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
[i915#1759]: https://gitlab.freedesktop.org/drm/intel/issues/1759
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
[i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
[i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
[i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
[i915#3987]: https://gitlab.freedesktop.org/drm/intel/issues/3987
[i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
[i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
[i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
[i915#5137]: https://gitlab.freedesktop.org/drm/intel/issues/5137
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_6349 -> IGTPW_6672
CI-20190529: 20190529
CI_DRM_11264: 3b562232559bd940e63f977619548e5cc70da985 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_6672: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/index.html
IGT_6349: 0513032006f385f34fcd094c810b93f31cbed09d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
== Testlist changes ==
+igt@kms_psr2_sf@cursor-plane-move-without-damage-sf
+igt@kms_psr2_sf@overlay-plane-move-without-damage-sf
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6672/index.html
[-- Attachment #2: Type: text/html, Size: 8975 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-22 11:16 ` Petri Latvala
@ 2022-02-22 11:48 ` Hogander, Jouni
0 siblings, 0 replies; 7+ messages in thread
From: Hogander, Jouni @ 2022-02-22 11:48 UTC (permalink / raw)
To: Latvala, Petri; +Cc: igt-dev
On Tue, 2022-02-22 at 13:16 +0200, Petri Latvala wrote:
> On Tue, Feb 22, 2022 at 12:38:41PM +0200, Jouni Högander wrote:
> > Add new testcases which are moving cursor and overlay planes
> > without setting any damage areas.
> >
> > Cursor testcase is reproducing following bug:
> >
> > https://gitlab.freedesktop.org/drm/intel/-/issues/5077
> >
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> > tests/i915/kms_psr2_sf.c | 144
> > ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 136 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
> > index 36f8a786..b9990f87 100644
> > --- a/tests/i915/kms_psr2_sf.c
> > +++ b/tests/i915/kms_psr2_sf.c
> > @@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2
> > selective fetch by sending multiple"
> > enum operations {
> > PLANE_UPDATE,
> > PLANE_UPDATE_CONTINUOUS,
> > + PLANE_MOVE_DAMAGED,
> > PLANE_MOVE,
> > OVERLAY_PRIM_UPDATE
> > };
> > @@ -51,7 +52,9 @@ enum plane_move_postion {
> > POS_TOP_LEFT,
> > POS_TOP_RIGHT,
> > POS_BOTTOM_LEFT,
> > - POS_BOTTOM_RIGHT
> > + POS_BOTTOM_RIGHT,
> > + POS_BOTTOM_LEFT_NEGATIVE,
> > + POS_TOP_RIGHT_NEGATIVE,
> > };
> >
> > typedef struct {
> > @@ -74,6 +77,7 @@ typedef struct {
> > igt_plane_t *test_plane;
> > cairo_t *cr;
> > uint32_t screen_changes;
> > + int cur_x, cur_y;
> > } data_t;
> >
> > static const char *op_str(enum operations op)
> > @@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
> > static const char * const name[] = {
> > [PLANE_UPDATE] = "plane-update",
> > [PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
> > - [PLANE_MOVE] = "plane-move",
> > + [PLANE_MOVE] = "plane-move-without-damage",
> > + [PLANE_MOVE_DAMAGED] = "plane-move",
> > [OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> > };
> >
> > @@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t
> > *data, igt_fb_t *fb, uint32_t h,
> > }
> > }
> >
> > -static void plane_move_setup_square(data_t *data, igt_fb_t *fb,
> > uint32_t h,
> > +static void plane_move_damaged_setup_square(data_t *data, igt_fb_t
> > *fb, uint32_t h,
> > uint32_t v)
> > {
> > int x = 0, y = 0;
> > @@ -260,8 +265,8 @@ static void prepare(data_t *data)
> >
> > data->fb_continuous = &data->fb_overlay;
> >
> > - if (data->op == PLANE_MOVE) {
> > - plane_move_setup_square(data, &data->fb_test,
> > + if (data->op == PLANE_MOVE_DAMAGED) {
> > + plane_move_damaged_setup_square(data, &data-
> > >fb_test,
> > data->mode->hdisplay/2,
> > data->mode->vdisplay/2);
> >
> > @@ -273,6 +278,7 @@ static void prepare(data_t *data)
> >
> > igt_plane_set_fb(sprite, &data->fb_overlay);
> > data->test_plane = sprite;
> > +
> > break;
> >
> > case DRM_PLANE_TYPE_PRIMARY:
> > @@ -335,6 +341,8 @@ static void prepare(data_t *data)
> > igt_assert(false);
> > }
> >
> > + data->cur_x = data->cur_y = 0;
> > +
> > igt_plane_set_fb(primary, &data->fb_primary);
> >
> > igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > @@ -378,7 +386,7 @@ static void plane_update_expected_output(int
> > plane_type, int box_count,
> > manual(expected);
> > }
> >
> > -static void plane_move_expected_output(enum plane_move_postion
> > pos)
> > +static void plane_move_damaged_expected_output(enum
> > plane_move_postion pos)
> > {
> > char expected[64] = {};
> >
> > @@ -406,6 +414,42 @@ static void plane_move_expected_output(enum
> > plane_move_postion pos)
> > manual(expected);
> > }
> >
> > +static void plane_move_expected_output(enum plane_move_postion
> > pos)
> > +{
> > + char expected[73] = {};
> > +
> > + switch (pos) {
> > + case POS_TOP_LEFT:
> > + sprintf(expected,
> > + "screen Green with Blue box on top left
> > corner");
> > + break;
> > + case POS_TOP_RIGHT:
> > + sprintf(expected,
> > + "screen Green with Blue box on top right
> > corner");
> > + break;
> > + case POS_BOTTOM_LEFT:
> > + sprintf(expected,
> > + "screen Green with Blue box on bottom left
> > corner");
> > + break;
> > + case POS_BOTTOM_RIGHT:
> > + sprintf(expected,
> > + "screen Green with Blue box on bottom right
> > corner");
> > + break;
> > + case POS_BOTTOM_LEFT_NEGATIVE:
> > + sprintf(expected,
> > + "screen Green with Blue box on bottom left
> > corner (partly exceeding area)");
> > + break;
> > + case POS_TOP_RIGHT_NEGATIVE:
> > + sprintf(expected,
> > + "screen Green with Blue box on top right corner
> > (partly exceeding area)");
> > + break;
> > + default:
> > + igt_assert(false);
> > + }
> > +
> > + manual(expected);
> > +}
> > +
> > static void overlay_prim_update_expected_output(int box_count)
> > {
> > char expected[64] = {};
> > @@ -421,6 +465,9 @@ static void
> > overlay_prim_update_expected_output(int box_count)
> > static void expected_output(data_t *data)
> > {
> > switch (data->op) {
> > + case PLANE_MOVE_DAMAGED:
> > + plane_move_damaged_expected_output(data->pos);
> > + break;
> > case PLANE_MOVE:
> > plane_move_expected_output(data->pos);
> > break;
> > @@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
> > expected_output(data);
> > }
> >
> > +static void plane_move(data_t *data)
> > +{
> > + int target_x, target_y;
> > +
> > + switch (data->pos) {
> > + case POS_TOP_LEFT:
> > + target_x = 0;
> > + target_y = 0;
> > + break;
> > + case POS_TOP_RIGHT:
> > + target_x = data->mode->hdisplay - data->fb_test.width;
> > + target_y = 0;
> > + break;
> > + case POS_TOP_RIGHT_NEGATIVE:
> > + target_x = data->mode->hdisplay - data->fb_test.width;
> > + target_y = -data->fb_test.width / 2;
> > + break;
> > + case POS_BOTTOM_LEFT:
> > + target_x = 0;
> > + target_y = data->mode->vdisplay - data->fb_test.height;
> > + break;
> > + case POS_BOTTOM_LEFT_NEGATIVE:
> > + target_x = -data->fb_test.width / 2;
> > + target_y = data->mode->vdisplay - data->fb_test.height;
> > + break;
> > + case POS_BOTTOM_RIGHT:
> > + target_x = data->mode->hdisplay - data->fb_test.width;
> > + target_y = data->mode->vdisplay - data->fb_test.height;
> > + break;
> > + default:
> > + igt_assert(false);
> > + }
> > +
> > + while (data->cur_x != target_x || data->cur_y != target_y) {
> > + if (data->cur_x < target_x)
> > + data->cur_x += min(target_x - data->cur_x, 20);
> > + else if (data->cur_x > target_x)
> > + data->cur_x -= min(data->cur_x - target_x, 20);
> > +
> > + if (data->cur_y < target_y)
> > + data->cur_y += min(target_y - data->cur_y, 20);
> > + else if (data->cur_y > target_y)
> > + data->cur_y -= min(data->cur_y - target_y, 20);
> > +
> > + igt_plane_set_position(data->test_plane, data->cur_x,
> > data->cur_y);
> > + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > + }
> > +
> > + igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > +
> > + expected_output(data);
> > +}
> > +
> > static void damaged_plane_update(data_t *data)
> > {
> > igt_plane_t *test_plane = data->test_plane;
> > @@ -526,6 +626,8 @@ static void damaged_plane_update(data_t *data)
> >
> > static void run(data_t *data)
> > {
> > + int i;
> > +
> > igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> >
> > data->screen_changes = 0;
> > @@ -542,9 +644,15 @@ static void run(data_t *data)
> > damaged_plane_update(data);
> > }
> > break;
> > - case PLANE_MOVE:
> > + case PLANE_MOVE_DAMAGED:
> > damaged_plane_move(data);
> > break;
> > + case PLANE_MOVE:
> > + for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE;
> > i++) {
> > + data->pos = i;
> > + plane_move(data);
> > + }
> > + break;
> > default:
> > igt_assert(false);
> > }
> > @@ -654,10 +762,20 @@ igt_main
> > cleanup(&data);
> > }
> >
> > - /* Only for overlay plane */
> > data.op = PLANE_MOVE;
> > /* Verify overlay plane move selective fetch */
> > igt_describe("Test that selective fetch works on moving overlay
> > plane");
> > + igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> > + data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> > + prepare(&data);
> > + run(&data);
> > + cleanup(&data);
> > + }
> > +
> > + /* Only for overlay plane */
> > + data.op = PLANE_MOVE_DAMAGED;
> > + /* Verify overlay plane move selective fetch */
> > + igt_describe("Test that selective fetch works on moving overlay
> > plane");
> > igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
> > for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> > data.pos = i;
> > @@ -668,6 +786,16 @@ igt_main
> > }
> > }
> >
> > + data.op = PLANE_MOVE;
> > + /* Verify overlay plane move selective fetch */
> > + igt_describe("Test that selective fetch works on moving overlay
> > plane");
> > + igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
> > + data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> > + prepare(&data);
> > + run(&data);
> > + cleanup(&data);
> > + }
>
> Same text on three igt_describes and subtest names sound like some of
> them are copypaste errors.
>
>
Thank you for pointing this out. I will fix it before sending next
version.
BR,
Jouni Högander
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-22 10:38 [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane Jouni Högander
2022-02-22 11:16 ` Petri Latvala
2022-02-22 11:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2022-02-22 16:38 ` Souza, Jose
2022-02-23 7:39 ` Hogander, Jouni
2 siblings, 1 reply; 7+ messages in thread
From: Souza, Jose @ 2022-02-22 16:38 UTC (permalink / raw)
To: igt-dev, Hogander, Jouni
On Tue, 2022-02-22 at 12:38 +0200, Jouni Högander wrote:
> Add new testcases which are moving cursor and overlay planes
> without setting any damage areas.
>
> Cursor testcase is reproducing following bug:
>
> https://gitlab.freedesktop.org/drm/intel/-/issues/5077
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
> tests/i915/kms_psr2_sf.c | 144 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 136 insertions(+), 8 deletions(-)
>
> diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
> index 36f8a786..b9990f87 100644
> --- a/tests/i915/kms_psr2_sf.c
> +++ b/tests/i915/kms_psr2_sf.c
> @@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2 selective fetch by sending multiple"
> enum operations {
> PLANE_UPDATE,
> PLANE_UPDATE_CONTINUOUS,
> + PLANE_MOVE_DAMAGED,
Missing more explanation in the commit message, please split each change in a single patch.
I see here a patch for this new PLANE_MOVE_DAMAGED and another adding the negative positions...
Anyhow, I don't think we need PLANE_MOVE_DAMAGED.
One of the differences between it and PLANE_MOVE is set_clip(&data->plane_move_clip) that is wrong, the damaged property is based on the plane
coordinates.
> PLANE_MOVE,
> OVERLAY_PRIM_UPDATE
> };
> @@ -51,7 +52,9 @@ enum plane_move_postion {
> POS_TOP_LEFT,
> POS_TOP_RIGHT,
> POS_BOTTOM_LEFT,
> - POS_BOTTOM_RIGHT
> + POS_BOTTOM_RIGHT,
> + POS_BOTTOM_LEFT_NEGATIVE,
> + POS_TOP_RIGHT_NEGATIVE,
> };
>
> typedef struct {
> @@ -74,6 +77,7 @@ typedef struct {
> igt_plane_t *test_plane;
> cairo_t *cr;
> uint32_t screen_changes;
> + int cur_x, cur_y;
> } data_t;
>
> static const char *op_str(enum operations op)
> @@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
> static const char * const name[] = {
> [PLANE_UPDATE] = "plane-update",
> [PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
> - [PLANE_MOVE] = "plane-move",
> + [PLANE_MOVE] = "plane-move-without-damage",
> + [PLANE_MOVE_DAMAGED] = "plane-move",
> [OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> };
>
> @@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t *data, igt_fb_t *fb, uint32_t h,
> }
> }
>
> -static void plane_move_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
> +static void plane_move_damaged_setup_square(data_t *data, igt_fb_t *fb, uint32_t h,
> uint32_t v)
> {
> int x = 0, y = 0;
> @@ -260,8 +265,8 @@ static void prepare(data_t *data)
>
> data->fb_continuous = &data->fb_overlay;
>
> - if (data->op == PLANE_MOVE) {
> - plane_move_setup_square(data, &data->fb_test,
> + if (data->op == PLANE_MOVE_DAMAGED) {
> + plane_move_damaged_setup_square(data, &data->fb_test,
> data->mode->hdisplay/2,
> data->mode->vdisplay/2);
>
> @@ -273,6 +278,7 @@ static void prepare(data_t *data)
>
> igt_plane_set_fb(sprite, &data->fb_overlay);
> data->test_plane = sprite;
> +
> break;
>
> case DRM_PLANE_TYPE_PRIMARY:
> @@ -335,6 +341,8 @@ static void prepare(data_t *data)
> igt_assert(false);
> }
>
> + data->cur_x = data->cur_y = 0;
> +
> igt_plane_set_fb(primary, &data->fb_primary);
>
> igt_display_commit2(&data->display, COMMIT_ATOMIC);
> @@ -378,7 +386,7 @@ static void plane_update_expected_output(int plane_type, int box_count,
> manual(expected);
> }
>
> -static void plane_move_expected_output(enum plane_move_postion pos)
> +static void plane_move_damaged_expected_output(enum plane_move_postion pos)
> {
> char expected[64] = {};
>
> @@ -406,6 +414,42 @@ static void plane_move_expected_output(enum plane_move_postion pos)
> manual(expected);
> }
>
> +static void plane_move_expected_output(enum plane_move_postion pos)
> +{
> + char expected[73] = {};
> +
> + switch (pos) {
> + case POS_TOP_LEFT:
> + sprintf(expected,
> + "screen Green with Blue box on top left corner");
> + break;
> + case POS_TOP_RIGHT:
> + sprintf(expected,
> + "screen Green with Blue box on top right corner");
> + break;
> + case POS_BOTTOM_LEFT:
> + sprintf(expected,
> + "screen Green with Blue box on bottom left corner");
> + break;
> + case POS_BOTTOM_RIGHT:
> + sprintf(expected,
> + "screen Green with Blue box on bottom right corner");
> + break;
> + case POS_BOTTOM_LEFT_NEGATIVE:
> + sprintf(expected,
> + "screen Green with Blue box on bottom left corner (partly exceeding area)");
> + break;
> + case POS_TOP_RIGHT_NEGATIVE:
> + sprintf(expected,
> + "screen Green with Blue box on top right corner (partly exceeding area)");
> + break;
> + default:
> + igt_assert(false);
> + }
> +
> + manual(expected);
> +}
Another unexpected changed, you changed the visual output of a current subtest, why?
> +
> static void overlay_prim_update_expected_output(int box_count)
> {
> char expected[64] = {};
> @@ -421,6 +465,9 @@ static void overlay_prim_update_expected_output(int box_count)
> static void expected_output(data_t *data)
> {
> switch (data->op) {
> + case PLANE_MOVE_DAMAGED:
> + plane_move_damaged_expected_output(data->pos);
> + break;
> case PLANE_MOVE:
> plane_move_expected_output(data->pos);
> break;
> @@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
> expected_output(data);
> }
>
> +static void plane_move(data_t *data)
> +{
> + int target_x, target_y;
> +
> + switch (data->pos) {
> + case POS_TOP_LEFT:
> + target_x = 0;
> + target_y = 0;
> + break;
> + case POS_TOP_RIGHT:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = 0;
> + break;
> + case POS_TOP_RIGHT_NEGATIVE:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = -data->fb_test.width / 2;
> + break;
> + case POS_BOTTOM_LEFT:
> + target_x = 0;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + case POS_BOTTOM_LEFT_NEGATIVE:
> + target_x = -data->fb_test.width / 2;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + case POS_BOTTOM_RIGHT:
> + target_x = data->mode->hdisplay - data->fb_test.width;
> + target_y = data->mode->vdisplay - data->fb_test.height;
> + break;
> + default:
> + igt_assert(false);
> + }
> +
> + while (data->cur_x != target_x || data->cur_y != target_y) {
> + if (data->cur_x < target_x)
> + data->cur_x += min(target_x - data->cur_x, 20);
> + else if (data->cur_x > target_x)
> + data->cur_x -= min(data->cur_x - target_x, 20);
> +
> + if (data->cur_y < target_y)
> + data->cur_y += min(target_y - data->cur_y, 20);
> + else if (data->cur_y > target_y)
> + data->cur_y -= min(data->cur_y - target_y, 20);
> +
> + igt_plane_set_position(data->test_plane, data->cur_x, data->cur_y);
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
what is this? why do atomic commits in loop until reach target?
another change that if necessary should go to another patch as this is changing a current subtest.
> + }
> +
> + igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> + expected_output(data);
> +}
> +
> static void damaged_plane_update(data_t *data)
> {
> igt_plane_t *test_plane = data->test_plane;
> @@ -526,6 +626,8 @@ static void damaged_plane_update(data_t *data)
>
> static void run(data_t *data)
> {
> + int i;
> +
> igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
>
> data->screen_changes = 0;
> @@ -542,9 +644,15 @@ static void run(data_t *data)
> damaged_plane_update(data);
> }
> break;
> - case PLANE_MOVE:
> + case PLANE_MOVE_DAMAGED:
> damaged_plane_move(data);
> break;
> + case PLANE_MOVE:
> + for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
> + data->pos = i;
> + plane_move(data);
Another unexpected change.
> + }
> + break;
> default:
> igt_assert(false);
> }
> @@ -654,10 +762,20 @@ igt_main
> cleanup(&data);
> }
>
> - /* Only for overlay plane */
> data.op = PLANE_MOVE;
> /* Verify overlay plane move selective fetch */
> igt_describe("Test that selective fetch works on moving overlay plane");
overlay != cursor
> + igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> + data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> + prepare(&data);
> + run(&data);
> + cleanup(&data);
> + }
> +
> + /* Only for overlay plane */
> + data.op = PLANE_MOVE_DAMAGED;
> + /* Verify overlay plane move selective fetch */
> + igt_describe("Test that selective fetch works on moving overlay plane");
> igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
> for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> data.pos = i;
> @@ -668,6 +786,16 @@ igt_main
> }
> }
>
> + data.op = PLANE_MOVE;
> + /* Verify overlay plane move selective fetch */
> + igt_describe("Test that selective fetch works on moving overlay plane");
> + igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
> + data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> + prepare(&data);
> + run(&data);
> + cleanup(&data);
> + }
> +
> /* Verify primary plane selective fetch with overplay plane blended */
> data.op = OVERLAY_PRIM_UPDATE;
> igt_describe("Test that selective fetch works on primary plane "
There is a few code style errors.
Other thing that you need to pay attention is the patch diff, saw some new line being added to a function that is not being changed.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-22 16:38 ` [igt-dev] [PATCH i-g-t] " Souza, Jose
@ 2022-02-23 7:39 ` Hogander, Jouni
2022-02-23 13:44 ` Hogander, Jouni
0 siblings, 1 reply; 7+ messages in thread
From: Hogander, Jouni @ 2022-02-23 7:39 UTC (permalink / raw)
To: igt-dev, Souza, Jose
On Tue, 2022-02-22 at 16:38 +0000, Souza, Jose wrote:
> On Tue, 2022-02-22 at 12:38 +0200, Jouni Högander wrote:
> > Add new testcases which are moving cursor and overlay planes
> > without setting any damage areas.
> >
> > Cursor testcase is reproducing following bug:
> >
> > https://gitlab.freedesktop.org/drm/intel/-/issues/5077
> >
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> > tests/i915/kms_psr2_sf.c | 144
> > ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 136 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
> > index 36f8a786..b9990f87 100644
> > --- a/tests/i915/kms_psr2_sf.c
> > +++ b/tests/i915/kms_psr2_sf.c
> > @@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2
> > selective fetch by sending multiple"
> > enum operations {
> > PLANE_UPDATE,
> > PLANE_UPDATE_CONTINUOUS,
> > +PLANE_MOVE_DAMAGED,
>
> Missing more explanation in the commit message, please split each
> change in a single patch.
> I see here a patch for this new PLANE_MOVE_DAMAGED and another adding
> the negative positions...
>
> Anyhow, I don't think we need PLANE_MOVE_DAMAGED.
> One of the differences between it and PLANE_MOVE is set_clip(&data-
> >plane_move_clip) that is wrong, the damaged property is based on the
> plane
> coordinates.
Obviously more explanation is needed in commit message. I will improve
it.
I wanted to keep original testcase as it is. Add a new testcase which
is targeted to trigger the screen flicker reported here:
https://gitlab.freedesktop.org/drm/intel/-/issues/5077
Current testcase is blinking screen and switching moving plane color
between blue and white when plane moves. This is because one plane move
is done on each iteration here:
for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
data.pos = i;
data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
prepare(&data);
run(&data);
cleanup(&data);
}
Additinally blinking/switching color on each iteration due to
prepare/cleanup. Doing this makes it harder to see possible glitches.
The testcase I added is doing:
prepare(&data);
for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
run(@data);
}
cleanup(&data);
This is moving cursor (or overlay plane) smoothly around screen and
possible glitches are easy to observe.
Damaged property I removed completely from this new testcase. It's not
used by psr selective fetch area calculation at all when plane moves.
I named original testcase enum as PLANE_MOVE_DAMAGED and use PLANE_MOVE
for the new one.
>
> > PLANE_MOVE,
> > OVERLAY_PRIM_UPDATE
> > };
> > @@ -51,7 +52,9 @@ enum plane_move_postion {
> > POS_TOP_LEFT,
> > POS_TOP_RIGHT,
> > POS_BOTTOM_LEFT,
> > -POS_BOTTOM_RIGHT
> > +POS_BOTTOM_RIGHT,
> > +POS_BOTTOM_LEFT_NEGATIVE,
> > +POS_TOP_RIGHT_NEGATIVE,
> > };
> >
> > typedef struct {
> > @@ -74,6 +77,7 @@ typedef struct {
> > igt_plane_t *test_plane;
> > cairo_t *cr;
> > uint32_t screen_changes;
> > +int cur_x, cur_y;
> > } data_t;
> >
> > static const char *op_str(enum operations op)
> > @@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
> > static const char * const name[] = {
> > [PLANE_UPDATE] = "plane-update",
> > [PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
> > -[PLANE_MOVE] = "plane-move",
> > +[PLANE_MOVE] = "plane-move-without-damage",
> > +[PLANE_MOVE_DAMAGED] = "plane-move",
> > [OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> > };
> >
> > @@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t
> > *data, igt_fb_t *fb, uint32_t h,
> > }
> > }
> >
> > -static void plane_move_setup_square(data_t *data, igt_fb_t *fb,
> > uint32_t h,
> > +static void plane_move_damaged_setup_square(data_t *data, igt_fb_t
> > *fb, uint32_t h,
> > uint32_t v)
> > {
> > int x = 0, y = 0;
> > @@ -260,8 +265,8 @@ static void prepare(data_t *data)
> >
> > data->fb_continuous = &data->fb_overlay;
> >
> > -if (data->op == PLANE_MOVE) {
> > -plane_move_setup_square(data, &data->fb_test,
> > +if (data->op == PLANE_MOVE_DAMAGED) {
> > +plane_move_damaged_setup_square(data, &data->fb_test,
> > data->mode->hdisplay/2,
> > data->mode->vdisplay/2);
> >
> > @@ -273,6 +278,7 @@ static void prepare(data_t *data)
> >
> > igt_plane_set_fb(sprite, &data->fb_overlay);
> > data->test_plane = sprite;
> > +
> > break;
> >
> > case DRM_PLANE_TYPE_PRIMARY:
> > @@ -335,6 +341,8 @@ static void prepare(data_t *data)
> > igt_assert(false);
> > }
> >
> > +data->cur_x = data->cur_y = 0;
> > +
> > igt_plane_set_fb(primary, &data->fb_primary);
> >
> > igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > @@ -378,7 +386,7 @@ static void plane_update_expected_output(int
> > plane_type, int box_count,
> > manual(expected);
> > }
> >
> > -static void plane_move_expected_output(enum plane_move_postion
> > pos)
> > +static void plane_move_damaged_expected_output(enum
> > plane_move_postion pos)
> > {
> > char expected[64] = {};
> >
> > @@ -406,6 +414,42 @@ static void plane_move_expected_output(enum
> > plane_move_postion pos)
> > manual(expected);
> > }
> >
> > +static void plane_move_expected_output(enum plane_move_postion
> > pos)
> > +{
> > +char expected[73] = {};
> > +
> > +switch (pos) {
> > +case POS_TOP_LEFT:
> > +sprintf(expected,
> > +"screen Green with Blue box on top left corner");
> > +break;
> > +case POS_TOP_RIGHT:
> > +sprintf(expected,
> > +"screen Green with Blue box on top right corner");
> > +break;
> > +case POS_BOTTOM_LEFT:
> > +sprintf(expected,
> > +"screen Green with Blue box on bottom left corner");
> > +break;
> > +case POS_BOTTOM_RIGHT:
> > +sprintf(expected,
> > +"screen Green with Blue box on bottom right corner");
> > +break;
> > +case POS_BOTTOM_LEFT_NEGATIVE:
> > +sprintf(expected,
> > +"screen Green with Blue box on bottom left corner (partly
> > exceeding area)");
> > +break;
> > +case POS_TOP_RIGHT_NEGATIVE:
> > +sprintf(expected,
> > +"screen Green with Blue box on top right corner (partly exceeding
> > area)");
> > +break;
> > +default:
> > +igt_assert(false);
> > +}
> > +
> > +manual(expected);
> > +}
>
> Another unexpected changed, you changed the visual output of a
> current subtest, why?
No. Old one is now plane_move_damaged_expected_output and new is
plane_move_expected_output.
>
> > +
> > static void overlay_prim_update_expected_output(int box_count)
> > {
> > char expected[64] = {};
> > @@ -421,6 +465,9 @@ static void
> > overlay_prim_update_expected_output(int box_count)
> > static void expected_output(data_t *data)
> > {
> > switch (data->op) {
> > +case PLANE_MOVE_DAMAGED:
> > +plane_move_damaged_expected_output(data->pos);
> > +break;
> > case PLANE_MOVE:
> > plane_move_expected_output(data->pos);
> > break;
> > @@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
> > expected_output(data);
> > }
> >
> > +static void plane_move(data_t *data)
> > +{
> > +int target_x, target_y;
> > +
> > +switch (data->pos) {
> > +case POS_TOP_LEFT:
> > +target_x = 0;
> > +target_y = 0;
> > +break;
> > +case POS_TOP_RIGHT:
> > +target_x = data->mode->hdisplay - data->fb_test.width;
> > +target_y = 0;
> > +break;
> > +case POS_TOP_RIGHT_NEGATIVE:
> > +target_x = data->mode->hdisplay - data->fb_test.width;
> > +target_y = -data->fb_test.width / 2;
> > +break;plane_move_damaged_expected_output
> > +case POS_BOTTOM_LEFT:
> > +target_x = 0;
> > +target_y = data->mode->vdisplay - data->fb_test.height;
> > +break;
> > +case POS_BOTTOM_LEFT_NEGATIVE:
> > + target_x = -data->fb_test.width / 2;
> > + target_y = data->mode->vdisplay - data->fb_test.height;
> > +break;
> > +case POS_BOTTOM_RIGHT:
> > + target_x = data->mode->hdisplay - data->fb_test.width;
> > + target_y = data->mode->vdisplay - data->fb_test.height;
> > +break;
> > +default:
> > +igt_assert(false);
> > +}
> > +
> > +while (data->cur_x != target_x || data->cur_y != target_y) {
> > +if (data->cur_x < target_x)
> > +data->cur_x += min(target_x - data->cur_x, 20);
> > +else if (data->cur_x > target_x)
> > +data->cur_x -= min(data->cur_x - target_x, 20);
> > +
> > +if (data->cur_y < target_y)
> > +data->cur_y += min(target_y - data->cur_y, 20);
> > +else if (data->cur_y > target_y)
> > +data->cur_y -= min(data->cur_y - target_y, 20);
> > +
> > +igt_plane_set_position(data->test_plane, data->cur_x, data-
> > >cur_y);
> > +igt_display_commit2(&data->display, COMMIT_ATOMIC);
>
> what is this? why do atomic commits in loop until reach target?
> another change that if necessary should go to another patch as this
> is changing a current subtest.
I'm moving plane here step by step to target position. This is the
"run" function for the testcase I described above. I need to do it like
this to reproduce the glitch this new testcase is targeted to reveal.
If I just move it directly to target position (i.e. x < or y < 0) the
glitch doesn't reproduce. See also my explanation above.
>
> > +}
> > +
> > +igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > +
> > +expected_output(data);
> > +}
> > +
> > static void damaged_plane_update(data_t *data)
> > {
> > igt_plane_t *test_plane = data->test_plane;
> > @@ -526,6 +626,8 @@ static void damaged_plane_update(data_t *data)
> >
> > static void run(data_t *data)
> > {
> > +int i;
> > +
> > igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> >
> > data->screen_changes = 0;
> > @@ -542,9 +644,15 @@ static void run(data_t *data)
> > damaged_plane_update(data);
> > }
> > break;
> > -case PLANE_MOVE:
> > +case PLANE_MOVE_DAMAGED:
> > damaged_plane_move(data);
> > break;
> > +case PLANE_MOVE:
> > +for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
> > +data->pos = i;
> > +plane_move(data);
>
> Another unexpected change.
I will try to split the patch as you suggested. PLANE_MOVE_DAMAGED here
is the original testcase. New is PLANE_MOVE. I choose to change enum
naming because original testcase is also switching moving plane content
and using damage property for that (PLANE_MOVE_DAMAGED). New testcase
is just moving the plane (PLANE_MOVE).
>
> > +}
> > +break;
> > default:
> > igt_assert(false);
> > }
> > @@ -654,10 +762,20 @@ igt_main
> > cleanup(&data);
> > }
> >
> > -/* Only for overlay plane */
> > data.op = PLANE_MOVE;
> > /* Verify overlay plane move selective fetch */
> > igt_describe("Test that selective fetch works on moving overlay
> > plane");
>
> overlay != cursor
Oops, one more copy paste error. I will fix it.
>
> > +igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> > +data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> > +prepare(&data);
> > +run(&data);
> > +cleanup(&data);
> > +}
> > +
> > +/* Only for overlay plane */
> > +data.op = PLANE_MOVE_DAMAGED;
> > +/* Verify overlay plane move selective fetch */
> > +igt_describe("Test that selective fetch works on moving overlay
> > plane");
> > igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
> > for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> > data.pos = i;
> > @@ -668,6 +786,16 @@ igt_main
> > }
> > }
> >
> > +data.op = PLANE_MOVE;
> > +/* Verify overlay plane move selective fetch */
> > +igt_describe("Test that selective fetch works on moving overlay
> > plane");
> > +igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
> > +data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> > +prepare(&data);
> > +run(&data);
> > +cleanup(&data);
> > +}
> > +
> > /* Verify primary plane selective fetch with overplay plane
> > blended */
> > data.op = OVERLAY_PRIM_UPDATE;
> > igt_describe("Test that selective fetch works on primary plane "
>
> There is a few code style errors.
>
Ok, I will fix these as well.
> Other thing that you need to pay attention is the patch diff, saw
> some new line being added to a function that is not being changed.
>
Yes, I will drop those.
BR,
Jouni Högander
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane
2022-02-23 7:39 ` Hogander, Jouni
@ 2022-02-23 13:44 ` Hogander, Jouni
0 siblings, 0 replies; 7+ messages in thread
From: Hogander, Jouni @ 2022-02-23 13:44 UTC (permalink / raw)
To: igt-dev, Souza, Jose
On Wed, 2022-02-23 at 07:39 +0000, Hogander, Jouni wrote:
> On Tue, 2022-02-22 at 16:38 +0000, Souza, Jose wrote:
> > On Tue, 2022-02-22 at 12:38 +0200, Jouni Högander wrote:
> > > Add new testcases which are moving cursor and overlay planes
> > > without setting any damage areas.
> > >
> > > Cursor testcase is reproducing following bug:
> > >
> > > https://gitlab.freedesktop.org/drm/intel/-/issues/5077
> > >
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > > tests/i915/kms_psr2_sf.c | 144
> > > ++++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 136 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/tests/i915/kms_psr2_sf.c b/tests/i915/kms_psr2_sf.c
> > > index 36f8a786..b9990f87 100644
> > > --- a/tests/i915/kms_psr2_sf.c
> > > +++ b/tests/i915/kms_psr2_sf.c
> > > @@ -43,6 +43,7 @@ IGT_TEST_DESCRIPTION("Tests to varify PSR2
> > > selective fetch by sending multiple"
> > > enum operations {
> > > PLANE_UPDATE,
> > > PLANE_UPDATE_CONTINUOUS,
> > > +PLANE_MOVE_DAMAGED,
> >
> > Missing more explanation in the commit message, please split each
> > change in a single patch.
> > I see here a patch for this new PLANE_MOVE_DAMAGED and another
> > adding
> > the negative positions...
> >
> > Anyhow, I don't think we need PLANE_MOVE_DAMAGED.
> > One of the differences between it and PLANE_MOVE is set_clip(&data-
> > > plane_move_clip) that is wrong, the damaged property is based on
> > > the
> > plane
> > coordinates.
>
> Obviously more explanation is needed in commit message. I will
> improve
> it.
>
> I wanted to keep original testcase as it is. Add a new testcase which
> is targeted to trigger the screen flicker reported here:
>
> https://gitlab.freedesktop.org/drm/intel/-/issues/5077
>
> Current testcase is blinking screen and switching moving plane color
> between blue and white when plane moves. This is because one plane
> move
> is done on each iteration here:
>
> for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> data.pos = i;
> data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> prepare(&data);
> run(&data);
> cleanup(&data);
> }
>
> Additinally blinking/switching color on each iteration due to
> prepare/cleanup. Doing this makes it harder to see possible glitches.
> The testcase I added is doing:
>
> prepare(&data);
> for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
> run(@data);
> }
> cleanup(&data);
>
> This is moving cursor (or overlay plane) smoothly around screen and
> possible glitches are easy to observe.
>
> Damaged property I removed completely from this new testcase. It's
> not
> used by psr selective fetch area calculation at all when plane moves.
>
> I named original testcase enum as PLANE_MOVE_DAMAGED and use
> PLANE_MOVE
> for the new one.
>
> > > PLANE_MOVE,
> > > OVERLAY_PRIM_UPDATE
> > > };
> > > @@ -51,7 +52,9 @@ enum plane_move_postion {
> > > POS_TOP_LEFT,
> > > POS_TOP_RIGHT,
> > > POS_BOTTOM_LEFT,
> > > -POS_BOTTOM_RIGHT
> > > +POS_BOTTOM_RIGHT,
> > > +POS_BOTTOM_LEFT_NEGATIVE,
> > > +POS_TOP_RIGHT_NEGATIVE,
> > > };
> > >
> > > typedef struct {
> > > @@ -74,6 +77,7 @@ typedef struct {
> > > igt_plane_t *test_plane;
> > > cairo_t *cr;
> > > uint32_t screen_changes;
> > > +int cur_x, cur_y;
> > > } data_t;
> > >
> > > static const char *op_str(enum operations op)
> > > @@ -81,7 +85,8 @@ static const char *op_str(enum operations op)
> > > static const char * const name[] = {
> > > [PLANE_UPDATE] = "plane-update",
> > > [PLANE_UPDATE_CONTINUOUS] = "plane-update-continuous",
> > > -[PLANE_MOVE] = "plane-move",
> > > +[PLANE_MOVE] = "plane-move-without-damage",
> > > +[PLANE_MOVE_DAMAGED] = "plane-move",
> > > [OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> > > };
> > >
> > > @@ -187,7 +192,7 @@ static void plane_update_setup_squares(data_t
> > > *data, igt_fb_t *fb, uint32_t h,
> > > }
> > > }
> > >
> > > -static void plane_move_setup_square(data_t *data, igt_fb_t *fb,
> > > uint32_t h,
> > > +static void plane_move_damaged_setup_square(data_t *data,
> > > igt_fb_t
> > > *fb, uint32_t h,
> > > uint32_t v)
> > > {
> > > int x = 0, y = 0;
> > > @@ -260,8 +265,8 @@ static void prepare(data_t *data)
> > >
> > > data->fb_continuous = &data->fb_overlay;
> > >
> > > -if (data->op == PLANE_MOVE) {
> > > -plane_move_setup_square(data, &data->fb_test,
> > > +if (data->op == PLANE_MOVE_DAMAGED) {
> > > +plane_move_damaged_setup_square(data, &data->fb_test,
> > > data->mode->hdisplay/2,
> > > data->mode->vdisplay/2);
> > >
> > > @@ -273,6 +278,7 @@ static void prepare(data_t *data)
> > >
> > > igt_plane_set_fb(sprite, &data->fb_overlay);
> > > data->test_plane = sprite;
> > > +
> > > break;
> > >
> > > case DRM_PLANE_TYPE_PRIMARY:
> > > @@ -335,6 +341,8 @@ static void prepare(data_t *data)
> > > igt_assert(false);
> > > }
> > >
> > > +data->cur_x = data->cur_y = 0;
> > > +
> > > igt_plane_set_fb(primary, &data->fb_primary);
> > >
> > > igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > @@ -378,7 +386,7 @@ static void plane_update_expected_output(int
> > > plane_type, int box_count,
> > > manual(expected);
> > > }
> > >
> > > -static void plane_move_expected_output(enum plane_move_postion
> > > pos)
> > > +static void plane_move_damaged_expected_output(enum
> > > plane_move_postion pos)
> > > {
> > > char expected[64] = {};
> > >
> > > @@ -406,6 +414,42 @@ static void plane_move_expected_output(enum
> > > plane_move_postion pos)
> > > manual(expected);
> > > }
> > >
> > > +static void plane_move_expected_output(enum plane_move_postion
> > > pos)
> > > +{
> > > +char expected[73] = {};
> > > +
> > > +switch (pos) {
> > > +case POS_TOP_LEFT:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on top left corner");
> > > +break;
> > > +case POS_TOP_RIGHT:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on top right corner");
> > > +break;
> > > +case POS_BOTTOM_LEFT:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on bottom left corner");
> > > +break;
> > > +case POS_BOTTOM_RIGHT:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on bottom right corner");
> > > +break;
> > > +case POS_BOTTOM_LEFT_NEGATIVE:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on bottom left corner (partly
> > > exceeding area)");
> > > +break;
> > > +case POS_TOP_RIGHT_NEGATIVE:
> > > +sprintf(expected,
> > > +"screen Green with Blue box on top right corner (partly
> > > exceeding
> > > area)");
> > > +break;
> > > +default:
> > > +igt_assert(false);
> > > +}
> > > +
> > > +manual(expected);
> > > +}
> >
> > Another unexpected changed, you changed the visual output of a
> > current subtest, why?
>
> No. Old one is now plane_move_damaged_expected_output and new is
> plane_move_expected_output.
>
> > > +
> > > static void overlay_prim_update_expected_output(int box_count)
> > > {
> > > char expected[64] = {};
> > > @@ -421,6 +465,9 @@ static void
> > > overlay_prim_update_expected_output(int box_count)
> > > static void expected_output(data_t *data)
> > > {
> > > switch (data->op) {
> > > +case PLANE_MOVE_DAMAGED:
> > > +plane_move_damaged_expected_output(data->pos);
> > > +break;
> > > case PLANE_MOVE:
> > > plane_move_expected_output(data->pos);
> > > break;
> > > @@ -487,6 +534,59 @@ static void damaged_plane_move(data_t *data)
> > > expected_output(data);
> > > }
> > >
> > > +static void plane_move(data_t *data)
> > > +{
> > > +int target_x, target_y;
> > > +
> > > +switch (data->pos) {
> > > +case POS_TOP_LEFT:
> > > +target_x = 0;
> > > +target_y = 0;
> > > +break;
> > > +case POS_TOP_RIGHT:
> > > +target_x = data->mode->hdisplay - data->fb_test.width;
> > > +target_y = 0;
> > > +break;
> > > +case POS_TOP_RIGHT_NEGATIVE:
> > > +target_x = data->mode->hdisplay - data->fb_test.width;
> > > +target_y = -data->fb_test.width / 2;
> > > +break;plane_move_damaged_expected_output
> > > +case POS_BOTTOM_LEFT:
> > > +target_x = 0;
> > > +target_y = data->mode->vdisplay - data->fb_test.height;
> > > +break;
> > > +case POS_BOTTOM_LEFT_NEGATIVE:
> > > + target_x = -data->fb_test.width / 2;
> > > + target_y = data->mode->vdisplay - data->fb_test.height;
> > > +break;
> > > +case POS_BOTTOM_RIGHT:
> > > + target_x = data->mode->hdisplay - data->fb_test.width;
> > > + target_y = data->mode->vdisplay - data->fb_test.height;
> > > +break;
> > > +default:
> > > +igt_assert(false);
> > > +}
> > > +
> > > +while (data->cur_x != target_x || data->cur_y != target_y) {
> > > +if (data->cur_x < target_x)
> > > +data->cur_x += min(target_x - data->cur_x, 20);
> > > +else if (data->cur_x > target_x)
> > > +data->cur_x -= min(data->cur_x - target_x, 20);
> > > +
> > > +if (data->cur_y < target_y)
> > > +data->cur_y += min(target_y - data->cur_y, 20);
> > > +else if (data->cur_y > target_y)
> > > +data->cur_y -= min(data->cur_y - target_y, 20);
> > > +
> > > +igt_plane_set_position(data->test_plane, data->cur_x, data-
> > > > cur_y);
> > > +igt_display_commit2(&data->display, COMMIT_ATOMIC);
> >
> > what is this? why do atomic commits in loop until reach target?
> > another change that if necessary should go to another patch as this
> > is changing a current subtest.
>
> I'm moving plane here step by step to target position. This is the
> "run" function for the testcase I described above. I need to do it
> like
> this to reproduce the glitch this new testcase is targeted to reveal.
> If I just move it directly to target position (i.e. x < or y < 0) the
> glitch doesn't reproduce. See also my explanation above.
>
> > > +}
> > > +
> > > +igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > > +
> > > +expected_output(data);
> > > +}
> > > +
> > > static void damaged_plane_update(data_t *data)
> > > {
> > > igt_plane_t *test_plane = data->test_plane;
> > > @@ -526,6 +626,8 @@ static void damaged_plane_update(data_t
> > > *data)
> > >
> > > static void run(data_t *data)
> > > {
> > > +int i;
> > > +
> > > igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > >
> > > data->screen_changes = 0;
> > > @@ -542,9 +644,15 @@ static void run(data_t *data)
> > > damaged_plane_update(data);
> > > }
> > > break;
> > > -case PLANE_MOVE:
> > > +case PLANE_MOVE_DAMAGED:
> > > damaged_plane_move(data);
> > > break;
> > > +case PLANE_MOVE:
> > > +for (i = POS_TOP_LEFT; i <= POS_TOP_RIGHT_NEGATIVE; i++) {
> > > +data->pos = i;
> > > +plane_move(data);
> >
> > Another unexpected change.
>
> I will try to split the patch as you suggested. PLANE_MOVE_DAMAGED
> here
> is the original testcase. New is PLANE_MOVE. I choose to change enum
> naming because original testcase is also switching moving plane
> content
> and using damage property for that (PLANE_MOVE_DAMAGED). New testcase
> is just moving the plane (PLANE_MOVE).
I still didn't split the patch. To me it looks much cleaner now as I
choose to keep PLANE_MOVE as it is and added new enum
PLANE_MOVE_NO_UPDATE. Please check version 2 and let's split if you
still think it's necessary.
>
> > > +}
> > > +break;
> > > default:
> > > igt_assert(false);
> > > }
> > > @@ -654,10 +762,20 @@ igt_main
> > > cleanup(&data);
> > > }
> > >
> > > -/* Only for overlay plane */
> > > data.op = PLANE_MOVE;
> > > /* Verify overlay plane move selective fetch */
> > > igt_describe("Test that selective fetch works on moving overlay
> > > plane");
> >
> > overlay != cursor
>
> Oops, one more copy paste error. I will fix it.
>
> > > +igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> > > +data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> > > +prepare(&data);
> > > +run(&data);
> > > +cleanup(&data);
> > > +}
> > > +
> > > +/* Only for overlay plane */
> > > +data.op = PLANE_MOVE_DAMAGED;
> > > +/* Verify overlay plane move selective fetch */
> > > +igt_describe("Test that selective fetch works on moving overlay
> > > plane");
> > > igt_subtest_f("%s-sf-dmg-area", op_str(data.op)) {
> > > for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> > > data.pos = i;
> > > @@ -668,6 +786,16 @@ igt_main
> > > }
> > > }
> > >
> > > +data.op = PLANE_MOVE;
> > > +/* Verify overlay plane move selective fetch */
> > > +igt_describe("Test that selective fetch works on moving overlay
> > > plane");
> > > +igt_subtest_f("overlay-%s-sf", op_str(data.op)) {
> > > +data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> > > +prepare(&data);
> > > +run(&data);
> > > +cleanup(&data);
> > > +}
> > > +
> > > /* Verify primary plane selective fetch with overplay plane
> > > blended */
> > > data.op = OVERLAY_PRIM_UPDATE;
> > > igt_describe("Test that selective fetch works on primary plane "
> >
> > There is a few code style errors.
> >
>
> Ok, I will fix these as well.
>
> > Other thing that you need to pay attention is the patch diff, saw
> > some new line being added to a function that is not being changed.
> >
>
> Yes, I will drop those.
>
> BR,
>
> Jouni Högander
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-23 13:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 10:38 [igt-dev] [PATCH i-g-t] tests/kms_psr2_sf: Add new testcases for moving plane Jouni Högander
2022-02-22 11:16 ` Petri Latvala
2022-02-22 11:48 ` Hogander, Jouni
2022-02-22 11:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2022-02-22 16:38 ` [igt-dev] [PATCH i-g-t] " Souza, Jose
2022-02-23 7:39 ` Hogander, Jouni
2022-02-23 13:44 ` Hogander, Jouni
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.