[igt-dev] [PATCH i-g-t 2/2] lib/i915/gem_mman: mmap to the CPU instead of the GTT in some tests

[Chris Wilson]

Quoting Summers, Stuart (2019-11-19 18:58:31)
> On Tue, 2019-11-19 at 18:25 +0000, Chris Wilson wrote:
> > Quoting Stuart Summers (2019-11-19 18:16:19)
> > > Do not limit to the GTT for tests which are not specifically
> > > testing
> > > capability in the GTT.
> > > 
> > > Signed-off-by: Stuart Summers <stuart.summers at intel.com>
> > > ---
> > >  lib/i915/gem_mman.c            | 19 +++++++++++++++++++
> > >  lib/i915/gem_mman.h            |  1 +
> > >  tests/i915/gem_ctx_shared.c    | 20 +++++---------------
> > >  tests/i915/gem_exec_schedule.c |  3 +--
> > >  4 files changed, 26 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> > > index a0e34aef..d37840f7 100644
> > > --- a/lib/i915/gem_mman.c
> > > +++ b/lib/i915/gem_mman.c
> > > @@ -40,6 +40,25 @@
> > >  #define VG(x) do {} while (0)
> > >  #endif
> > >  
> > > +void *gem_mmap_host(int fd, uint32_t handle, uint64_t size,
> > > unsigned prot)
> > > +{
> > > +       void *ptr;
> > > +       uint32_t domain;
> > > +
> > > +       if (gem_has_mappable_ggtt(fd)) {
> > > +               ptr = gem_mmap__gtt(fd, handle, size, prot);
> > > +               domain = I915_GEM_DOMAIN_GTT;
> > > +       } else {
> > > +               ptr = gem_mmap__cpu(fd, handle, 0, size, prot);
> > > +               domain = I915_GEM_DOMAIN_CPU;
> > > +       }
> > > +
> > > +       gem_set_domain(fd, handle, /* no write hazard lies! */
> > > +                      domain, domain);
> > 
> > And be very careful putting the set-domain in here; as the lies are
> > integral to the test and not suitable for libraries. Domain handling
> > is
> > sadly of paramount importance for most tests.
> 
> Hm.. so maybe a little more in the kernel to safeguard here. I was
> thinking of doing that anyway. Then we can adopt the tests as needed.

/* no write hazard lies! */

is all about how we are circumventing the kernel's opinion on correct
behaviour to get at the behaviour we require ;)

> And for these, maybe the right way to go is to use the new mmap_offset
> Zbigniew is implementing. The IOCTL posted by Abdiel already has a WC
> case for that.

Yes. Pretty much all of these should be using gem_mmap__wc(), and
whether that is implemented using mmap_offset_ioctl or mmap_ioctl is
immaterial. The CPUs / platforms that cannot use WC are immaterial to
most of these tests (these test in particular are only applicable to
recent gen), but for a select few tests we will need to make sure that
they are still usable with gem_mmap__gtt.

So a possible gem_mmap__device_coherent() {
	ptr = gem_mmap__offset(.type = WC);
	if (ptr)
		return ptr;

	ptr = gem_mmap__wc();
	if (ptr)
		return ptr;

	return gem_mmap__gt();
}
Names and details left to the reader. We did decide that library
functions like gem_mmap__wc() could use either mmap_offset_ioctl or
mmap_ioctl as they saw fit; and tests that required exact ioctls would
call those ioctls explicitly.
-Chris