Hi Chris, On Friday, November 1, 2019 11:02:45 AM CET Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-10-31 15:28:54) > > Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable > > addresses for !ppgtt") introduced filtering of addresses possibly > > occupied by other users of shared GTT. Unfortunately, that filtering > > doesn't distinguish between actually occupied addresses and otherwise > > invalid softpin offsets. As soon as incorrect GTT alignment is assumed > > when running on future backends with possibly larger minimum page > > sizes, a half of calculated offsets to be tested will be incorrectly > > detected as occupied by other users and silently skipped instead of > > reported as a problem. That will significantly distort the intended > > test pattern. > > > > Filter out failing addresses only if not reported as invalid. > > > > v2: Skip unavailable addresses only when not running on full PPGTT. > > v3: Replace the not on full PPGTT requirement for skipping with error > > code checking. > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > --- > > tests/i915/gem_exec_reloc.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c > > index 5f59fe99..423fed8b 100644 > > --- a/tests/i915/gem_exec_reloc.c > > +++ b/tests/i915/gem_exec_reloc.c > > @@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags) > > uint64_t gtt_size = gem_aperture_size(fd); > > const uint32_t bbe = MI_BATCH_BUFFER_END; > > igt_spin_t *spin = NULL; > > - int count, n; > > + int count, n, err; > > > > igt_require(gem_has_softpin(fd)); > > > > @@ -542,8 +542,11 @@ static void basic_range(int fd, unsigned flags) > > gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe)); > > execbuf.buffers_ptr = to_user_pointer(&obj[n]); > > execbuf.buffer_count = 1; > > - if (__gem_execbuf(fd, &execbuf)) > > + err = __gem_execbuf(fd, &execbuf); > > + if (err) { > > > + igt_assert(err != -EINVAL); > > I've been thinking about this and I think the right approach is > > /* Iff using a shared GTT, some of it may be reserved */ > igt_assert_eq(err, -ENOSPC); Thanks for your help, I'll follow your approach. Shouldn't we also use the trick with invalid reloc here to save request emission? Thanks, Janusz > > > continue; > > + } > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx