[Intel-gfx] [PATCH i-g-t v3 1/7] lib/igt_kms: Add forcing TEST_ONLY for atomic commits

[Maarten Lankhorst]

Op 01-02-17 om 14:18 schreef Mika Kahola:
> Add an option to force atomic commits to do commits with TEST_ONLY flag
> first before doing the actual commit.
>
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
>  lib/igt_kms.c | 18 +++++++++++++++++-
>  lib/igt_kms.h |  1 +
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 4ba6316..c513ef8 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2453,7 +2453,23 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
>  		igt_atomic_prepare_connector_commit(output, req);
>  	}
>  
> -	ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
> +	if (display->force_test_atomic &&
> +	    !(flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
> +		unsigned int test_flags = flags & ~DRM_MODE_PAGE_FLIP_EVENT;
> +		int test_ret;
> +
> +		test_flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> +
> +		test_ret = drmModeAtomicCommit(display->drm_fd, req, test_flags, user_data);
> +		ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
> +
> +		if (test_ret)
> +			igt_assert_eq(test_ret, ret);
> +		else
> +			igt_assert(ret != -EINVAL);
> +	} else
> +		ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
> +
>  	drmModeAtomicFree(req);
>  	return ret;
>  
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 2562618..e45fc21 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -338,6 +338,7 @@ struct igt_display {
>  	igt_pipe_t *pipes;
>  	bool has_cursor_plane;
>  	bool is_atomic;
> +	bool force_test_atomic;
>  };
>  
>  void igt_display_init(igt_display_t *display, int drm_fd);

Sorry, didn't notice it before but force_test_atomic is a big behavioral switch that
might force other tests to potentially fail. If this a subtest fails and the flag is
not explictly cleared. This is also why you need to set it separately even if false.

I really wish there was a igt_subtest_exit_handler, I could remove a lot of boilerplate in the KMS tests with that.

My personal wishlist for it:
- Complete all sw_sync fences
- Remove all allocated pipe_crc's, so next test won't fail due to fd already being assigned.
- Clean igt_display_t with a synchronous commit.
- Clear any pending drm events without blocking.
- Remove any leftover framebuffers allocated in the subtest. This may fail if fb's become
  invalid, like in the kms_rmfb test.

Be careful with dereferencing fb's, the previous pointer may not be valid any more if it
was allocated on the stack. It should probably require an extra memory allocation during
fb alloc to keep track.

This would allow us to kill most of the teardown boilerplate in igt kms tests, and increases
reliability when a subtest fails, the next subtest is a lot more likely to succeed.

~Maarten