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
