[Intel-gfx] [PATCH igt] igt/kms_setmode: Test that the vblank interval matches the dotclock

[Daniel Vetter]

On Mon, Oct 24, 2016 at 11:08:31AM +0100, Chris Wilson wrote:
> As we allow userspace to set the dotclock, we should try to respect it!
> Userpsace will try to set its frametimings based upon the dotclock, so
> ideally it should match the measured vblank interval.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

Ack on the testcase. And I think with this one we can make the kms_flip
ones a bit more lenient in what they accept. At least I think it'd be
useful to afford a bit more imprecision in exchange for more test coverage
on shittier hw.
-Daniel

> ---
>  tests/kms_setmode.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index df958f0..966ad93 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -71,6 +71,7 @@ enum test_flags {
>  	TEST_SINGLE_CRTC_CLONE		= 0x04,
>  	TEST_EXCLUSIVE_CRTC_CLONE	= 0x08,
>  	TEST_STEALING			= 0x10,
> +	TEST_TIMINGS			= 0x20,
>  };
>  
>  struct test_config {
> @@ -413,6 +414,75 @@ static int test_stealing(int fd, struct crtc_config *crtc, uint32_t *ids)
>  	return ret;
>  }
>  
> +static double frame_time(const drmModeModeInfo *kmode)
> +{
> +	return 1000.0 * kmode->htotal * kmode->vtotal / kmode->clock;
> +}
> +
> +static void check_timings(int crtc_idx, const drmModeModeInfo *kmode)
> +{
> +#define CALIBRATE_TS_STEPS 120 /* ~2s has to be less than 128! */

Because speed matters, maybe just 50?

> +	drmVBlank wait;
> +	igt_stats_t stats;
> +	uint32_t last_seq;
> +	uint64_t last_timestamp;
> +	double expected;
> +	double mean;
> +	double stddev;
> +	int n;
> +
> +	memset(&wait, 0, sizeof(wait));
> +	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> +	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_NEXTONMISS;
> +	do_or_die(drmWaitVBlank(drm_fd, &wait));
> +
> +	last_seq = wait.reply.sequence;
> +	last_timestamp = wait.reply.tval_sec;
> +	last_timestamp *= 1000000;
> +	last_timestamp += wait.reply.tval_usec;
> +
> +	memset(&wait, 0, sizeof(wait));
> +	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> +	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
> +	wait.request.sequence = last_seq;
> +	for (n = 0; n < CALIBRATE_TS_STEPS; n++) {
> +		++wait.request.sequence;
> +		do_or_die(drmWaitVBlank(drm_fd, &wait));
> +	}
> +
> +	igt_stats_init_with_size(&stats, CALIBRATE_TS_STEPS);
> +	for (n = 0; n < CALIBRATE_TS_STEPS; n++) {
> +		struct drm_event_vblank ev;
> +		uint64_t now;
> +
> +		igt_assert(read(drm_fd, &ev, sizeof(ev)) == sizeof(ev));
> +		igt_assert_eq(ev.sequence, last_seq + 1);
> +
> +		now = ev.tv_sec;
> +		now *= 1000000;
> +		now += ev.tv_usec;
> +
> +		igt_stats_push(&stats, now - last_timestamp);
> +
> +		last_timestamp = now;
> +		last_seq = ev.sequence;
> +	}
> +
> +	expected = frame_time(kmode);
> +
> +	mean = igt_stats_get_mean(&stats);
> +	stddev = igt_stats_get_std_deviation(&stats);
> +
> +	igt_info("Expected frametime: %.0fus; measured %.1fus +- %.3fus accuracy %.2f%%\n",
> +		 expected, mean, stddev, 100 * 6 * stddev / mean);
> +	igt_assert(6 * stddev / mean < 0.005); /* 99% accuracy within 0.5% */
> +
> +	igt_assert_f(fabs(mean - expected) < 2*stddev,
> +		      "vblank interval differs from modeline! expected %.1fus, measured %1.fus +- %.3fus, difference %.1fus (%.1f sigma)\n",
> +		      expected, mean, stddev,
> +		      fabs(mean - expected), fabs(mean - expected) / stddev);
> +}
> +
>  static void test_crtc_config(const struct test_config *tconf,
>  			     struct crtc_config *crtcs, int crtc_count)
>  {
> @@ -475,8 +545,8 @@ static void test_crtc_config(const struct test_config *tconf,
>  
>  	igt_assert(config_failed == !!(tconf->flags & TEST_INVALID));
>  
> -	if (ret == 0 && connector_connected && !(tconf->flags & TEST_INVALID))
> -		sleep(5);
> +	if (ret == 0 && connector_connected && tconf->flags & TEST_TIMINGS)
> +		check_timings(crtcs[0].crtc_idx, &crtcs[0].mode);
>  
>  	for (i = 0; i < crtc_count; i++) {
>  		if (crtcs[i].fb_info.fb_id) {
> @@ -732,6 +802,7 @@ int main(int argc, char **argv)
>  		enum test_flags flags;
>  		const char *name;
>  	} tests[] = {
> +		{ TEST_TIMINGS, "basic" },

If you want this in BAT, it needs to be manually added to the list. And
maybe give it a better name, like timings or basic-timings.
-Daniel

>  		{ TEST_CLONE | TEST_SINGLE_CRTC_CLONE,
>  					"basic-clone-single-crtc" },
>  		{ TEST_INVALID | TEST_CLONE | TEST_SINGLE_CRTC_CLONE,
> -- 
> 2.10.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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