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@xxxxxxxxxxxxxxxxxx>
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@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
