On Mon, Oct 17, 2016 at 02:28:37PM +0300, Mika Kahola wrote: > + for (int i = 0; i < iterations; i++) { > + igt_info("%d/%d: Testing connector %s using pipe %s with %d planes\n", > + i+1, iterations, igt_output_name(output), > + kmstest_pipe_name(pipe), max_planes); > + > + test_init(data, pipe); > + > + test_grab_crc(data, output, pipe, &blue, tiling, > + &test.reference_crc); > + > + test_planes(data, pipe, &blue, tiling, max_planes, output); > + > + if (test_atomic) { > + igt_display_commit_atomic(&data->display, > + DRM_MODE_PAGE_FLIP_EVENT, > + &data->display); > + } else > + igt_display_commit2(&data->display, COMMIT_LEGACY); > + > + igt_pipe_crc_start(data->pipe_crc); > + n = igt_pipe_crc_get_crcs(data->pipe_crc, 1, &crc); > + igt_assert_eq(n, 1); > + igt_pipe_crc_stop(data->pipe_crc); Comment on testing method here: With atomic we don't just require that the result looks good at the end, but also that _every_ frame is perfect. That means you need a slightly different test sequence: 1. Enable crc capture. 2. Create a new atomic state (randomized, whatever) which should in the end still result in the same screen contents using the punchout box trick. This depends upon the exact subtest. 3. Commit the state from step 2. 4. Wait to make sure the atomic commit has completed. If you do an async commit, that means waiting for the flip_event to get signalled (didn't see code for that anywhere). 5. Fetch all the crc values (if there's not a bug in your code or in the kernel it should be just 1) and make sure they are _all_ the right value. Your code here only grabs 1 crc after the atomic commit completed, which means if there's tearing or underruns you'll miss them. Which means it won't actually validate the crucial feature for which we've created atomic! 6. Go back to 2. 7. After enough loops, stop crc capturing. Note that this is the loop for ALLOW_MODESET==false atomic commits, i.e. where every atomic commit should take at most 1 vblank interval. If any of the commits take longer than that, there's a bug in either the kernel or your testcase. Note that crc_start alone has a few vblank waits (due to crc bugs on some platforms) which will break this. Cheers, Daniel > + > + igt_assert_crc_equal(&test.reference_crc, crc); > + > + test_fini(data, output, max_planes); > + } > +} -- 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