public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
@ 2017-09-19 11:31 Maarten Lankhorst
  2017-09-19 11:49 ` Ville Syrjälä
  2017-09-19 23:08 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2017-09-19 11:31 UTC (permalink / raw)
  To: intel-gfx

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 <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
Cc: Marta Lofstedt <marta.lofstedt@intel.com>
---
 tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 2d32d3a91570..c3149535b838 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -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);
 }
 
-- 
2.14.1

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

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

* Re: [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
  2017-09-19 11:31 [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy Maarten Lankhorst
@ 2017-09-19 11:49 ` Ville Syrjälä
  2017-09-19 12:00   ` Maarten Lankhorst
  2017-09-20 12:58   ` Maarten Lankhorst
  2017-09-19 23:08 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 2 replies; 5+ messages in thread
From: Ville Syrjälä @ 2017-09-19 11:49 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Tue, Sep 19, 2017 at 01:31:13PM +0200, Maarten Lankhorst wrote:
> 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 <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> ---
>  tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 2d32d3a91570..c3149535b838 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -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);

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ä <ville.syrjala@linux.intel.com>

> +
> +		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);
>  }
>  
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
  2017-09-19 11:49 ` Ville Syrjälä
@ 2017-09-19 12:00   ` Maarten Lankhorst
  2017-09-20 12:58   ` Maarten Lankhorst
  1 sibling, 0 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2017-09-19 12:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 19-09-17 om 13:49 schreef Ville Syrjälä:
> On Tue, Sep 19, 2017 at 01:31:13PM +0200, Maarten Lankhorst wrote:
>> 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 <maarten.lankhorst@linux.intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
>> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
>> ---
>>  tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++----
>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> index 2d32d3a91570..c3149535b838 100644
>> --- a/tests/kms_cursor_legacy.c
>> +++ b/tests/kms_cursor_legacy.c
>> @@ -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);
> 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ä <ville.syrjala@linux.intel.com>
>
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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.BAT: failure for tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
  2017-09-19 11:31 [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy Maarten Lankhorst
  2017-09-19 11:49 ` Ville Syrjälä
@ 2017-09-19 23:08 ` Patchwork
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-09-19 23:08 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== 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 build
da197b5f3cb516aaaea72d0d60b0f5c1c81081dd igt/gem_eio: Add another variant of in-flight to avoid request coalescing

with latest DRM-Tip kernel build CI_DRM_3110
bf6ecf6d25c1 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy
  2017-09-19 11:49 ` Ville Syrjälä
  2017-09-19 12:00   ` Maarten Lankhorst
@ 2017-09-20 12:58   ` Maarten Lankhorst
  1 sibling, 0 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2017-09-20 12:58 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 19-09-17 om 13:49 schreef Ville Syrjälä:
> On Tue, Sep 19, 2017 at 01:31:13PM +0200, Maarten Lankhorst wrote:
>> 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 <maarten.lankhorst@linux.intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102259
>> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
>> ---
>>  tests/kms_cursor_legacy.c | 25 +++++++++++++++++++++----
>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> index 2d32d3a91570..c3149535b838 100644
>> --- a/tests/kms_cursor_legacy.c
>> +++ b/tests/kms_cursor_legacy.c
>> @@ -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);
> 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ä <ville.syrjala@linux.intel.com>
>
>> +
>> +		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);
>>  }
>>  
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Thanks, pushed with a limit in place to prevent overflow. :)

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

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

end of thread, other threads:[~2017-09-20 12:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-19 11:31 [PATCH i-g-t] tests/kms_cursor_legacy: Do not start collecting CRC after making FB busy Maarten Lankhorst
2017-09-19 11:49 ` Ville Syrjälä
2017-09-19 12:00   ` Maarten Lankhorst
2017-09-20 12:58   ` Maarten Lankhorst
2017-09-19 23:08 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox