Re: [igt-dev] [PATCH i-g-t 2/3] tests/kms_mmap_write_crc: Fix igt_fixture handling

Daniel Vetter
2019-02-20T23:45:03Z

On Wed, Feb 20, 2019 at 02:31:21PM +0000, Chris Wilson wrote:
> Quoting Daniel Vetter (2019-02-20 14:24:48)
> > igt_fixture within an igt_simple_test don't really work as advertised.
> >
> > Minimal fix, the testcase seems fairly questionable already ...
>
> Definitely; it was written to reproduce a specific setup and demonstrate
> an issue with forgetting to sync, or soemthing like that. Maarten
> hopefully knows whether this can be simply subsumed into one of our
> other tests. It's just the use of a cpu_hog stands out; though really it
> wants a memory_hog? for that extra bit of cache pressure. That might be
> useful to generalise in our other coherency sensitive tests.
>
> > References: https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_222/fi-icl-y/igt@kms_mmap_write_crc.html
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > tests/kms_mmap_write_crc.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
> > index 93c96ef4de08..bf2b0c2991b3 100644
> > --- a/tests/kms_mmap_write_crc.c
> > +++ b/tests/kms_mmap_write_crc.c
> > @@ -268,7 +268,8 @@ static data_t data;
> > int main(int argc, char **argv)
> > {
> > int i;
> > - igt_simple_init_parse_opts(&argc, argv, "n", NULL, NULL, opt_handler, NULL);
> > + igt_subtest_init_parse_opts(&argc, argv, "n", NULL, NULL,
> > + opt_handler, NULL);
> >
> > igt_skip_on_simulation();
> >
> > @@ -286,10 +287,12 @@ int main(int argc, char **argv)
> > fork_cpuhog_helper();
> > }
> >
> > - igt_info("Using %d rounds for the test\n", ROUNDS);
> > + igt_subtest("main") {
> > + igt_info("Using %d rounds for the test\n", ROUNDS);
> >
> > - for (i = 0; i < ROUNDS; i++)
> > - run_test(&data);
> > + for (i = 0; i < ROUNDS; i++)
> > + run_test(&data);
> > + }
>
> Ok, that was much simpler than I feared. I started by ripping out the
> parse_opts :)

Yeah occasionally it pays of to be a minimalist :-)

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks for taking a look, merged.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation