tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
PATCH 1/1 tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
Collecting CRC may force a modeset, which is a bad idea after we just forced a hang. The hang is intended to make sure the page flip doesn't complete before the cursor, making sure that works.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
Cc: Marta Lofstedt <[email protected]>
--- tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
tests/kms_cursor_legacy.c +21 -4
@@ -1334,7 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_pipe_crc_t *pipe_crc;igt_pipe_t *pipe_connected = &display->pipes[pipe];igt_plane_t *plane_primary =igt_pipe_get_plane_type (pipe_connected,DRM_PLANE_TYPE_PRIMARY ); -igt_crc_t crcs[3 ]; +igt_crc_t crcs[2 ];if (atomic)igt_require (display->is_atomic);
@@ -1348,7 +1348,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_display_commit2 (display, display->is_atomic ?COMMIT_ATOMIC :COMMIT_LEGACY ); - pipe_crc =igt_pipe_crc_new (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO ); + pipe_crc =igt_pipe_crc_new_nonblock (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO );set_cursor_on_pipe (display, pipe, &cursor_fb);igt_display_commit2 (display,COMMIT_UNIVERSAL );
@@ -1371,9 +1371,17 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); +/* +* We must enable CRC collecting here since this may force +* a modeset, and this loop is timing sensitive. +*/ +igt_pipe_crc_start (pipe_crc); +/* Disable cursor, and immediately queue a flip. Check if resulting crc is correct. */ for (int i =1 ; i >=0 ; i--) {uint32_t *busy; +igt_crc_t *received_crcs =NULL ; +int ncrcs; busy =make_fb_busy (display->drm_fd, &fb_info[1 ]);
@@ -1384,7 +1392,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_assert_eq (get_vblank (display->drm_fd, pipe,0 ), vblank_start); -igt_pipe_crc_collect_crc (pipe_crc, &crcs[2 ]); + ncrcs =igt_pipe_crc_get_crcs (pipe_crc,8 , &received_crcs);finish_fb_busy (busy);
@@ -1397,13 +1405,22 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); -igt_assert_crc_equal (&crcs[i], &crcs[2 ]); +igt_assert (ncrcs >0 ); + +igt_assert_crc_equal (&crcs[i], &received_crcs[ncrcs -1 ]); +free (received_crcs); }do_cleanup_display (display);igt_remove_fb (display->drm_fd, &fb_info[1 ]);igt_remove_fb (display->drm_fd, &fb_info[0 ]);igt_remove_fb (display->drm_fd, &cursor_fb); + +/* +* igt_pipe_crc_stop() may force a modeset for workarounds, call +* it after do_cleanup_display since we disable the display anyway. +*/ +igt_pipe_crc_stop (pipe_crc);igt_pipe_crc_free (pipe_crc); }
Signature · 6 lines
2.14.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 61 lines · Maarten Lankhorst
Collecting CRC may force a modeset, which is a bad idea after we just forced a hang. The hang is intended to make sure the page flip doesn't complete before the cursor, making sure that works.
Signed-off-by Maarten Lankhorst <[email protected]>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
Cc: Marta Lofstedt <[email protected]>--- tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)tests/kms_cursor_legacy.c +11
-3@@ -1334,7 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_pipe_crc_t *pipe_crc;igt_pipe_t *pipe_connected = &display->pipes[pipe];igt_plane_t *plane_primary =igt_pipe_get_plane_type (pipe_connected,DRM_PLANE_TYPE_PRIMARY ); -igt_crc_t crcs[3 ]; +igt_crc_t crcs[2 ];if (atomic)igt_require (display->is_atomic);@@ -1348,7 +1348,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_display_commit2 (display, display->is_atomic ?COMMIT_ATOMIC :COMMIT_LEGACY ); - pipe_crc =igt_pipe_crc_new (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO ); + pipe_crc =igt_pipe_crc_new_nonblock (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO );set_cursor_on_pipe (display, pipe, &cursor_fb);igt_display_commit2 (display,COMMIT_UNIVERSAL );@@ -1371,9 +1371,17 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); +/* +* We must enable CRC collecting here since this may force +* a modeset, and this loop is timing sensitive. +*/ +igt_pipe_crc_start (pipe_crc); +/* Disable cursor, and immediately queue a flip. Check if resulting crc is correct. */ for (int i =1 ; i >=0 ; i--) {uint32_t *busy; +igt_crc_t *received_crcs =NULL ; +int ncrcs; busy =make_fb_busy (display->drm_fd, &fb_info[1 ]);@@ -1384,7 +1392,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_assert_eq (get_vblank (display->drm_fd, pipe,0 ), vblank_start); -igt_pipe_crc_collect_crc (pipe_crc, &crcs[2 ]); + ncrcs =igt_pipe_crc_get_crcs (pipe_crc,8 , &received_crcs);finish_fb_busy (busy);
tests/kms_cursor_legacy.c static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic) · line 1408
Quoted 5 lines
tests/kms_cursor_legacy.c +1
-1@@ -1397,13 +1405,22 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); -igt_assert_crc_equal (&crcs[i], &crcs[2 ]); +igt_assert (ncrcs >0 );
Should we perhaps assign some arbitrary upper limit on the crcs we get? If the kernel already buffered 8 or more crcs, we may get a crc mismatch even though the problem is that the test blocked for too long and we're now looking at a stale crc rather than the current crc being wrong. Might make it slightly easier to diagnose the failure correctly.
Anyways
Quoted 25 lines
tests/kms_cursor_legacy.c +9
-0+ +igt_assert_crc_equal (&crcs[i], &received_crcs[ncrcs -1 ]); +free (received_crcs); }do_cleanup_display (display);igt_remove_fb (display->drm_fd, &fb_info[1 ]);igt_remove_fb (display->drm_fd, &fb_info[0 ]);igt_remove_fb (display->drm_fd, &cursor_fb); + +/* +* igt_pipe_crc_stop() may force a modeset for workarounds, call +* it after do_cleanup_display since we disable the display anyway. +*/ +igt_pipe_crc_stop (pipe_crc);igt_pipe_crc_free (pipe_crc); }Signature · 6 lines
2.14.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 19-09-17 om 13:49 schreef Ville Syrjälä:
Quoted 73 lines · Maarten Lankhorst
Quoted 61 lines
Collecting CRC may force a modeset, which is a bad idea after we just forced a hang. The hang is intended to make sure the page flip doesn't complete before the cursor, making sure that works.
Signed-off-by Maarten Lankhorst <[email protected]>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
Cc: Marta Lofstedt <[email protected]>--- tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)tests/kms_cursor_legacy.c +11
-3@@ -1334,7 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_pipe_crc_t *pipe_crc;igt_pipe_t *pipe_connected = &display->pipes[pipe];igt_plane_t *plane_primary =igt_pipe_get_plane_type (pipe_connected,DRM_PLANE_TYPE_PRIMARY ); -igt_crc_t crcs[3 ]; +igt_crc_t crcs[2 ];if (atomic)igt_require (display->is_atomic);@@ -1348,7 +1348,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_display_commit2 (display, display->is_atomic ?COMMIT_ATOMIC :COMMIT_LEGACY ); - pipe_crc =igt_pipe_crc_new (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO ); + pipe_crc =igt_pipe_crc_new_nonblock (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO );set_cursor_on_pipe (display, pipe, &cursor_fb);igt_display_commit2 (display,COMMIT_UNIVERSAL );@@ -1371,9 +1371,17 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); +/* +* We must enable CRC collecting here since this may force +* a modeset, and this loop is timing sensitive. +*/ +igt_pipe_crc_start (pipe_crc); +/* Disable cursor, and immediately queue a flip. Check if resulting crc is correct. */ for (int i =1 ; i >=0 ; i--) {uint32_t *busy; +igt_crc_t *received_crcs =NULL ; +int ncrcs; busy =make_fb_busy (display->drm_fd, &fb_info[1 ]);@@ -1384,7 +1392,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_assert_eq (get_vblank (display->drm_fd, pipe,0 ), vblank_start); -igt_pipe_crc_collect_crc (pipe_crc, &crcs[2 ]); + ncrcs =igt_pipe_crc_get_crcs (pipe_crc,8 , &received_crcs);finish_fb_busy (busy);tests/kms_cursor_legacy.c static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic) · line 1408
Quoted 5 lines
tests/kms_cursor_legacy.c +1
-1@@ -1397,13 +1405,22 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); -igt_assert_crc_equal (&crcs[i], &crcs[2 ]); +igt_assert (ncrcs >0 );Should we perhaps assign some arbitrary upper limit on the crcs we get? If the kernel already buffered 8 or more crcs, we may get a crc mismatch even though the problem is that the test blocked for too long and we're now looking at a stale crc rather than the current crc being wrong. Might make it slightly easier to diagnose the failure correctly.
Quoted 3 lines
Anyways
Reviewed-by Ville Syrjälä <[email protected]>
I figured 8 is enough, ack if I change it to igt_assert(ncrcs > 0 && ncrcs < 8), so we know we don't overflow?
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 19-09-17 om 13:49 schreef Ville Syrjälä:
Quoted 94 lines · Maarten Lankhorst
Quoted 61 lines
Collecting CRC may force a modeset, which is a bad idea after we just forced a hang. The hang is intended to make sure the page flip doesn't complete before the cursor, making sure that works.
Signed-off-by Maarten Lankhorst <[email protected]>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
Cc: Marta Lofstedt <[email protected]>--- tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)tests/kms_cursor_legacy.c +11
-3@@ -1334,7 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_pipe_crc_t *pipe_crc;igt_pipe_t *pipe_connected = &display->pipes[pipe];igt_plane_t *plane_primary =igt_pipe_get_plane_type (pipe_connected,DRM_PLANE_TYPE_PRIMARY ); -igt_crc_t crcs[3 ]; +igt_crc_t crcs[2 ];if (atomic)igt_require (display->is_atomic);@@ -1348,7 +1348,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_display_commit2 (display, display->is_atomic ?COMMIT_ATOMIC :COMMIT_LEGACY ); - pipe_crc =igt_pipe_crc_new (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO ); + pipe_crc =igt_pipe_crc_new_nonblock (display->drm_fd, pipe,INTEL_PIPE_CRC_SOURCE_AUTO );set_cursor_on_pipe (display, pipe, &cursor_fb);igt_display_commit2 (display,COMMIT_UNIVERSAL );@@ -1371,9 +1371,17 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); +/* +* We must enable CRC collecting here since this may force +* a modeset, and this loop is timing sensitive. +*/ +igt_pipe_crc_start (pipe_crc); +/* Disable cursor, and immediately queue a flip. Check if resulting crc is correct. */ for (int i =1 ; i >=0 ; i--) {uint32_t *busy; +igt_crc_t *received_crcs =NULL ; +int ncrcs; busy =make_fb_busy (display->drm_fd, &fb_info[1 ]);@@ -1384,7 +1392,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_assert_eq (get_vblank (display->drm_fd, pipe,0 ), vblank_start); -igt_pipe_crc_collect_crc (pipe_crc, &crcs[2 ]); + ncrcs =igt_pipe_crc_get_crcs (pipe_crc,8 , &received_crcs);finish_fb_busy (busy);tests/kms_cursor_legacy.c static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic) · line 1408
Quoted 5 lines
tests/kms_cursor_legacy.c +1
-1@@ -1397,13 +1405,22 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)igt_plane_set_fb (plane_primary, &fb_info[0 ]);igt_display_commit2 (display,COMMIT_UNIVERSAL ); -igt_assert_crc_equal (&crcs[i], &crcs[2 ]); +igt_assert (ncrcs >0 );Should we perhaps assign some arbitrary upper limit on the crcs we get? If the kernel already buffered 8 or more crcs, we may get a crc mismatch even though the problem is that the test blocked for too long and we're now looking at a stale crc rather than the current crc being wrong. Might make it slightly easier to diagnose the failure correctly.
Anyways
Reviewed-by Ville Syrjälä <[email protected]>Quoted 18 lines
tests/kms_cursor_legacy.c +9
-0+ +igt_assert_crc_equal (&crcs[i], &received_crcs[ncrcs -1 ]); +free (received_crcs); }do_cleanup_display (display);igt_remove_fb (display->drm_fd, &fb_info[1 ]);igt_remove_fb (display->drm_fd, &fb_info[0 ]);igt_remove_fb (display->drm_fd, &cursor_fb); + +/* +* igt_pipe_crc_stop() may force a modeset for workarounds, call +* it after do_cleanup_display since we disable the display anyway. +*/ +igt_pipe_crc_stop (pipe_crc);igt_pipe_crc_free (pipe_crc); }
Quoted 7 lines
Quoted 7 lines
Signature · 6 lines
2.14.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks, pushed with a limit in place to prevent overflow. :)
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.BAT: failure for tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
In reply to Maarten Lankhorst== Series Details ==
== Series Details ==
Series: tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
URL : https://patchwork.freedesktop.org/series/30582/
State : failure
== Summary ==
IGT patchset tested on top of latest successful buildda197b5f3cb516aaaea72d0d60b0f5c1c81081dd igt/gem_eio: Add another variant of in-flight to avoid request coalescing
with latest DRM-Tip kernel build CI_DRM_3110bf6ecf6d25c1 drm-tip: 2017y-09m-19d-17h-23m-04s UTC integration manifest
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> INCOMPLETE (fi-cfl-s)
Test kms_frontbuffer_tracking:
Subgroup basic:
dmesg-warn -> PASS (fi-kbl-7500u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
incomplete -> PASS (fi-kbl-7500u)fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:445s fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:470s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:419s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:524s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:278s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:516s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:505s fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:503s fi-cfl-s total:118 pass:97 dwarn:0 dfail:0 fail:0 skip:20 fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:427s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:567s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:425s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:408s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:437s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:490s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:464s fi-kbl-7500u total:289 pass:263 dwarn:1 dfail:0 fail:1 skip:24 time:462s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:586s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:545s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:451s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:746s fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:490s fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:478s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:568s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:418s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_228/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx