Quoting Daniel Vetter (2019-02-13 13:02:29) > On Wed, Feb 13, 2019 at 11:15 AM Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> > > > > --- > > > > 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. Which is why I'm replacing it with a slimmer variant. > > > > 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. But doesn't mean the old test is worthless; far from it. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx