[igt-dev] [PATCH i-g-t] lib/i915/gem_mman: Add support for GEM_MMAP_OFFSET ioctl

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-02-10 10:31:40)
> 
> On 05/12/2019 12:40, Chris Wilson wrote:
> > Quoting Zbigniew Kempczyński (2019-12-05 12:31:13)
> >> With introduction of new kernel ioctl we need to cover this in
> >> the IGT's. Patch adds mmap functions appropriate for this.
> >>
> >> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> >> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
> >> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > Do we really have people outside of gem_mmap_wc.c and gem_mmap_offset.c
> > that care whether their WC/WB mmapping is provided by MMAP_OFFSET_IOCTL
> > or MMAP_IOCTL. They just expect it to work, I would have thought --
> > basically just gem_mmap__device_coherent (and maybe the alternative would
> > be gem_mmap__cache_coherent, don't quote me on that :)
> > 
> > It's not a huge deal, I think it falls under overengineering that will
> > simply lead to confusion of "when do I use one and not the other?"
> 
> What shall we do with call sites which used to do:
> 
>    gem_mmap__gtt()
>    get_set_domain(GTT)
> 
> And now do:
> 
>    gem_mmap__device_coherent()
>    gem_set_domain(GTT)
> 
> ?

For the large part it's immaterial. The subtly is only when mixing GEM
operations on the same buffer within the same critical section.

> Sounds like we need a helper to either lets the caller know which 
> flavour of mmap was used so correct flushing can be done, or a 
> synchronous helper where possible sounds much easier:
> 
>    gem_mmap__device_coherent_sync() - does the correct set_domain internally

gem_mmap__device_coherent_domain() to return the domain and sync to roll
up the common actions into one (though I'm not fond of hiding subtle
details in helpers when the tests are meant to be exercising those
subtle details :(
-Chris