[igt-dev] [PATCH i-g-t] i915/gem_exec_big: Add a single shot test

[Daniel Vetter]

On Wed, Feb 13, 2019 at 11:15 AM Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-02-13 10:11:27)
> > On Tue, Feb 12, 2019 at 10:43:41PM +0000, Chris Wilson wrote:
> > > CI complains that the exhaustive test of trying every size up to the
> > > limit is too slow, so add a simple test that tries to submit one
> > > extreme batch buffer and check all the relocations land.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105555
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > ---
> > >  tests/i915/gem_exec_big.c    | 70 ++++++++++++++++++++++++++++++------
> > >  tests/intel-ci/blacklist.txt |  1 +
> > >  2 files changed, 60 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
> > > index a15672f66..6d7041cf4 100644
> > > --- a/tests/i915/gem_exec_big.c
> > > +++ b/tests/i915/gem_exec_big.c
> > > @@ -71,7 +71,7 @@ static void exec1(int fd, uint32_t handle, uint64_t reloc_ofs, unsigned flags, c
> > >       gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
> > >       gem_exec[0].alignment = 0;
> > >       gem_exec[0].offset = 0;
> > > -     gem_exec[0].flags = 0;
> > > +     gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > >       gem_exec[0].rsvd1 = 0;
> > >       gem_exec[0].rsvd2 = 0;
> > >
> > > @@ -154,12 +154,11 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
> > >       gem_exec[0].handle = handle;
> > >       gem_exec[0].relocation_count = nreloc;
> > >       gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
> > > +     gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > >
> > >       memset(&execbuf, 0, sizeof(execbuf));
> > >       execbuf.buffers_ptr = to_user_pointer(gem_exec);
> > >       execbuf.buffer_count = 1;
> > > -     execbuf.batch_start_offset = 0;
> > > -     execbuf.batch_len = 8;
> > >       execbuf.flags = flags;
> > >
> > >       /* Avoid hitting slowpaths in the reloc processing which might yield a
> > > @@ -197,16 +196,10 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
> > >  #undef reloc_ofs
> > >  }
> > >
> > > -igt_simple_main
> > > +static void exhaustive(int fd)
> > >  {
> > >       uint32_t batch[2] = {MI_BATCH_BUFFER_END};
> > >       uint64_t batch_size, max, ggtt_max, reloc_ofs;
> > > -     int fd;
> > > -
> > > -     fd = drm_open_driver(DRIVER_INTEL);
> > > -     igt_require_gem(fd);
> > > -
> > > -     use_64bit_relocs = intel_gen(intel_get_drm_devid(fd)) >= 8;
> > >
> > >       max = 3 * gem_aperture_size(fd) / 4;
> > >       ggtt_max = 3 * gem_global_aperture_size(fd) / 4;
> > > @@ -258,6 +251,61 @@ igt_simple_main
> > >               else
> > >                       batch_size *= 2;
> > >       }
> > > +}
> > > +
> > > +static void single(int i915)
> > > +{
> > > +     const uint32_t bbe = MI_BATCH_BUFFER_END;
> > > +     uint64_t batch_size, limit;
> > > +     uint32_t handle;
> > > +     void *ptr;
> > > +
> > > +     batch_size = (intel_get_avail_ram_mb() - 4) << 20; /* internal slack */
> > > +     limit = gem_aperture_size(i915) - (256 << 10); /* low pages reserved */
> > > +     if (!gem_uses_full_ppgtt(i915))
> > > +             limit = 3 * limit / 4;
> > > +
> > > +     batch_size = min(batch_size, limit);
> > > +     batch_size = ALIGN(batch_size, 4096);
> > > +     igt_info("Submitting a %'"PRId64"MiB batch, %saperture size %'"PRId64"MiB\n",
> > > +              batch_size >> 20,
> > > +              gem_uses_full_ppgtt(i915) ? "" : "shared ",
> > > +              gem_aperture_size(i915) >> 20);
> > > +     intel_require_memory(1, batch_size, CHECK_RAM);
> > > +
> > > +     handle = gem_create(i915, batch_size);
> > > +     gem_write(i915, handle, 0, &bbe, sizeof(bbe));
> > > +
> > > +     if (!FORCE_PREAD_PWRITE && gem_has_llc(i915))
> > > +             ptr = __gem_mmap__cpu(i915, handle, 0, batch_size, PROT_READ);
> > > +     else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(i915))
> > > +             ptr = __gem_mmap__wc(i915, handle, 0, batch_size, PROT_READ);
> > > +     else
> > > +             ptr = NULL;
> > > +
> > > +     execN(i915, handle, batch_size, 0, ptr);
> > > +
> > > +     if (ptr)
> > > +             munmap(ptr, batch_size);
> > > +}
> > > +
> > > +igt_main
> > > +{
> > > +     int i915 = -1;
> > > +
> > > +     igt_fixture {
> > > +             i915 = drm_open_driver(DRIVER_INTEL);
> > > +             igt_require_gem(i915);
> > > +
> > > +             use_64bit_relocs = intel_gen(intel_get_drm_devid(i915)) >= 8;
> > > +     }
> > > +
> > > +     igt_subtest("single")
> > > +             single(i915);
> > > +
> > > +     igt_subtest("exhaustive")
> > > +             exhaustive(i915);
> >
> > Do we still need this one? CI time isn't an endless resource (as much as
> > we'd want to), neither is our ability to maintain everything. And if all
> > we get is timeouts in CI I think there's better uses for that machine
> > time. And we do use all the CI machine time, so anytime you take away 10
> > minutes, it's 10 minutes of not running some other testcase.
>
> It's not for CI and not run in CI. CI is not the be all and end all of
> testing. We still have to manually find test cases for CI to run...

It's run in drmtip runs afaict. That's time shared with a ton of other
runs we do, so yeah, more time spent here means less time spent
somewhere else. "Adding even more tests" when CI folks seem to say
"already takes too long" just seems like the wrong direction.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch