Re: [igt-dev] [PATCH v2 i-g-t] tests/kms_cursor_legacy: Don't stop cursor_vs_flip early

Martin Peres
2019-04-24T10:01:25Z

On 17/04/2019 08:20, Arkadiusz Hiler wrote:
> The test is doing multiple iterations (50). Each iteration tries to
> squeeze target number of cursor updates in half a second worth of flips.
> If we don't hit the target in any given iteration we bail out early.
>
> Because of that we don't have the data on the number of iterations that
> have failed and/or succeeded any given run, which makes hunting down
> this elusive issue hard.
>
> Let's change that so we always go through all the iterations and fail at
> the end printing out the number of iterations we haven't met the target.
>
> Each failed iteration also logs how many cursor updates it has managed
> to do.
>
> Since our target is generated run-time and is also load sensitive, let's
> bump the log level of message containing our target to "info" so we can
> compare those values across both passes and failures.

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>
> v2: had more coffee and reveresed the fail condition this time
> because assert -> "fail_on"
>
> tests/kms_cursor_legacy.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 9febf6e9..5e7c4b53 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -644,7 +644,7 @@ get_cursor_updates_per_vblank(igt_display_t *display, enum pipe pipe,
> target /= 4;
> igt_require(target > 1);
>
> - igt_debug("Using a target of %d cursor updates per quarter-vblank\n", target);
> + igt_info("Using a target of %d cursor updates per quarter-vblank\n", target);
>
> return target;
> }
> @@ -1030,6 +1030,7 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
> enum pipe pipe = find_connected_pipe(display, false);
> igt_output_t *output;
> uint32_t vrefresh;
> + int fail_count;
>
> if (mode >= flip_test_atomic)
> igt_require(display->is_atomic);
> @@ -1050,6 +1051,8 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
>
> target = get_cursor_updates_per_vblank(display, pipe, &arg[0]);
>
> + fail_count = 0;
> +
> for (int i = 0; i < nloops; i++) {
> shared[0] = 0;
> igt_fork(child, 1) {
> @@ -1090,14 +1093,20 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
>
> shared[0] = 1;
> igt_waitchildren();
> - igt_assert_f(shared[0] > vrefresh*target / 2,
> - "completed %lu cursor updated in a period of %u flips, "
> - "we expect to complete approximately %lu updates, "
> - "with the threshold set at %lu\n",
> - shared[0], vrefresh / 2,
> - vrefresh*target, vrefresh*target / 2);
> + if (shared[0] <= vrefresh*target / 2) {
> + fail_count++;
> + igt_critical("completed %lu cursor updated in a period of %u flips, "
> + "we expect to complete approximately %lu updates, "
> + "with the threshold set at %lu\n",
> + shared[0], vrefresh / 2,
> + vrefresh*target, vrefresh*target / 2);
> + }
> }
>
> + igt_assert_f(fail_count == 0,
> + "Failed to meet cursor update expectations in %d out of %d iterations\n",
> + fail_count, nloops);
> +
> igt_remove_fb(display->drm_fd, &fb_info);
> igt_remove_fb(display->drm_fd, &cursor_fb);
> munmap((void *)shared, PAGE_SIZE);
>