[PATCH i-g-t] tests/kms_vblank: Add test to ensure DRM_CAP_CRTC_IN_VBLANK_EVENT works correctly

[Daniel Vetter]

On Thu, Nov 23, 2017 at 01:26:14PM +0100, Maarten Lankhorst wrote:
> This was implemented correctly only on the atomic ioctl before, but
> it should really be working on all 3 ioctl's involved, so ensure we
> always set crtc_id correctly with a testcase.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

We seemt to completely lack these checks for the vblank ioctl and the
atomic ioctl too. Can you pls fill these gaps (probably best in kms_flip.c
and kms_atomic.c), too?

> ---
>  tests/kms_vblank.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 47fd10fb9078..004f0e6104ee 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -121,7 +121,6 @@ static void run_test(data_t *data, int fd, void (*testfunc)(data_t *, int, int))
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
>  	enum pipe p;
> -	unsigned int valid_tests = 0;
>  
>  	for_each_pipe_with_valid_output(display, p, output) {
>  		igt_hang_t hang;
> @@ -170,11 +169,60 @@ static void run_test(data_t *data, int fd, void (*testfunc)(data_t *, int, int))
>  
>  		/* cleanup what prepare_crtc() has done */
>  		cleanup_crtc(data, fd, output);
> -		valid_tests++;
>  	}
> +}
> +
> +static void crtc_id_subtest(data_t *data, int fd)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe p;
> +
> +	for_each_pipe_with_valid_output(display, p, output) {
> +		struct drm_event_vblank buf;
> +		const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
> +		unsigned crtc_id, expected_crtc_id;
> +		uint64_t val;
> +		union drm_wait_vblank vbl;
> +
> +		crtc_id = display->pipes[p].crtc_id;
> +		if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
> +			expected_crtc_id = crtc_id;
> +		else
> +			expected_crtc_id = 0;
> +
> +		data->pipe = p;
> +		prepare_crtc(data, fd, output);
> +
> +		memset(&vbl, 0, sizeof(vbl));
> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +		vbl.request.type |= pipe_id_flag;
> +		vbl.request.sequence = 1;
> +		igt_assert_eq(wait_vblank(fd, &vbl), 0);
> +
> +		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +		igt_assert_eq(buf.crtc_id, expected_crtc_id);
> +
> +		do_or_die(drmModePageFlip(fd, crtc_id,
> +					  data->primary_fb.fb_id,
> +					  DRM_MODE_PAGE_FLIP_EVENT, NULL));
> +
> +		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +		igt_assert_eq(buf.crtc_id, expected_crtc_id);
> +
> +		if (display->is_atomic) {
> +			igt_plane_t *primary = igt_output_get_plane(output, 0);
> +
> +			igt_plane_set_fb(primary, &data->primary_fb);
> +			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
>  
> -	igt_require_f(valid_tests,
> -		      "no valid crtc/connector combinations found\n");
> +			igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +			igt_assert_eq(buf.crtc_id, expected_crtc_id);
> +		}
> +
> +		cleanup_crtc(data, fd, output);
> +		return;
> +	}
>  }
>  
>  static void accuracy(data_t *data, int fd, int nchildren)
> @@ -307,8 +355,12 @@ igt_main
>  		fd = drm_open_driver(DRIVER_ANY);
>  		kmstest_set_vt_graphics_mode();
>  		igt_display_init(&data.display, fd);
> +		igt_display_require_output(&data.display);
>  	}
>  
> +	igt_subtest("crtc-id")
> +		crtc_id_subtest(&data, fd);

Either I'm stupid, or this doesn't apply on top of latest igt master.

Either way I think taking the expensive modesets out of individual tests
would be good, so that when we run entire binaries in CI, we'll benefit
from some good speedup. Which is the plan, now that machines are a bit
more stable ...
-Daniel
> +
>  	for (f = funcs; f->name; f++) {
>  		for (m = modes; m->name; m++) {
>  			if (m->flags & ~f->valid)
> -- 
> 2.15.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch