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 <[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(-)
>>
>> 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ä <[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
