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

[Daniel Vetter <daniel@ffwll.ch>]

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
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev