On Mon, Jan 08, 2018 at 11:23:14AM +0100, Maarten Lankhorst wrote:
> There's no need to test this more than once. Add a NOHANG
> flag which can be used to specify that a subtest can not
> be run when hanging. If it's set on either the subtest or
> the mode, the -hang test for this combination will not be
> generated.
>
> Changes since v1:
> - Merge the patch that renamed HANG to NOHANG.
> - Rebase after 'reorganize subtests by type'.
> - Allow subtests to specify NOHANG too.
> Changes since v2:
> - Mark accuracy test with NOHANG, gpu reset disables interrupts,
> causing the test to fail.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> tests/kms_flip.c | 10 +---------
> tests/kms_vblank.c | 25 +++++++++++++++++++++++--
> 2 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 7689e65b521a..50c16b0debbf 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -72,7 +72,7 @@
> #define TEST_SUSPEND (1 << 26)
> #define TEST_TS_CONT (1 << 27)
> #define TEST_BO_TOOBIG (1 << 28)
> -#define TEST_HANG_ONCE (1 << 29)
> +
> #define TEST_BASIC (1 << 30)
>
> #define EVENT_FLIP (1 << 0)
> @@ -1071,13 +1071,8 @@ static unsigned int wait_for_events(struct test_output *o)
> static unsigned event_loop(struct test_output *o, unsigned duration_ms)
> {
> unsigned long start, end;
> - igt_hang_t hang;
> int count = 0;
>
> - memset(&hang, 0, sizeof(hang));
> - if (o->flags & TEST_HANG_ONCE)
> - hang = hang_gpu(drm_fd);
> -
> start = gettime_us();
>
> while (1) {
> @@ -1097,8 +1092,6 @@ static unsigned event_loop(struct test_output *o, unsigned duration_ms)
>
> end = gettime_us();
>
> - unhang_gpu(drm_fd, hang);
> -
> /* Flush any remaining events */
> if (o->pending_events)
> wait_for_events(o);
> @@ -1565,7 +1558,6 @@ int main(int argc, char **argv)
> TEST_CHECK_TS, "flip-vs-blocking-wf-vblank" },
> { 30, TEST_FLIP | TEST_MODESET | TEST_HANG | TEST_NOEVENT, "flip-vs-modeset-vs-hang" },
> { 30, TEST_FLIP | TEST_PAN | TEST_HANG, "flip-vs-panning-vs-hang" },
> - { 30, TEST_VBLANK | TEST_HANG_ONCE, "vblank-vs-hang" },
> { 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, "flip-vs-bad-tiling" },
>
> { 1, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP,
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index e51e96c7f061..c56b82033b75 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -54,6 +54,7 @@ typedef struct {
> #define IDLE 1
> #define BUSY 2
> #define FORKED 4
> +#define NOHANG 8
> } data_t;
>
> static double elapsed(const struct timespec *start,
> @@ -119,6 +120,7 @@ static void run_test(data_t *data, int fd, void (*testfunc)(data_t *, int, int))
> data->flags & FORKED ? sysconf(_SC_NPROCESSORS_ONLN) : 1;
> igt_display_t *display = &data->display;
> igt_output_t *output = data->output;
> + igt_hang_t hang;
>
> prepare_crtc(data, fd, output);
>
> @@ -126,6 +128,9 @@ static void run_test(data_t *data, int fd, void (*testfunc)(data_t *, int, int))
> igt_subtest_name(), kmstest_pipe_name(data->pipe),
> igt_output_name(output), nchildren);
>
> + if (!(data->flags & NOHANG))
> + hang = igt_hang_ring(display->drm_fd, I915_EXEC_DEFAULT);
> +
> if (data->flags & BUSY) {
> union drm_wait_vblank vbl;
>
> @@ -148,6 +153,9 @@ static void run_test(data_t *data, int fd, void (*testfunc)(data_t *, int, int))
>
> igt_assert(poll(&(struct pollfd){fd, POLLIN}, 1, 0) == 0);
>
> + if (!(data->flags & NOHANG))
> + igt_post_hang_ring(fd, hang);
> +
> igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
> igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
>
> @@ -316,7 +324,7 @@ igt_main
> void (*func)(data_t *, int, int);
> unsigned int valid;
> } funcs[] = {
> - { "accuracy", accuracy, IDLE },
> + { "accuracy", accuracy, IDLE | NOHANG },
Maybe a comment here that gpu hangs can block the vblank query for too
long and so affect accuracy? Documenting the reasons for why we have
NOHANG is always good.
> { "query", vblank_query, IDLE | FORKED | BUSY },
> { "wait", vblank_wait, IDLE | FORKED | BUSY },
> { }
> @@ -354,12 +362,25 @@ igt_main
>
> for (f = funcs; f->name; f++) {
> for (m = modes; m->name; m++) {
> - if (m->flags & ~f->valid)
> + if (m->flags & ~(f->valid | NOHANG))
> continue;
>
> igt_subtest_f("pipe-%s-%s-%s",
> kmstest_pipe_name(data.pipe),
> f->name, m->name) {
> + for_each_valid_output_on_pipe(&data.display, data.pipe, data.output) {
> + data.flags = m->flags | NOHANG;
> + run_test(&data, fd, f->func);
> + }
> + }
> +
> + /* Skip the -hang version if NOHANG flag is set */
> + if (f->valid & NOHANG || m->flags & NOHANG)
> + continue;
> +
> + igt_subtest_f("pipe-%s-%s-%s-hang",
> + kmstest_pipe_name(data.pipe),
> + f->name, m->name) {
> for_each_valid_output_on_pipe(&data.display, data.pipe, data.output) {
> data.flags = m->flags;
> run_test(&data, fd, f->func);
Imo extract this into a helper, it nests way too deeply. But bikeshed for
another patch.
With the NOHANG comment added:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
