Re: [PATCH i-g-t] tests/kms_plane_multiple: CRC based atomic correctness test

[Mika Kahola <mika.kahola@intel.com>]

On Mon, 2016-10-17 at 16:30 +0200, Daniel Vetter wrote:
> 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_FL
> > IP_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!
That's true, I missed that one. I'll make modification on the test
sequence and spin another round of this test.

Cheers,
Mika

> 
> 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);
> > +	}
> > +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx