[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:47:23)
> 
> On 10/02/2020 10:37, Chris Wilson wrote:
> > 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 :(
> 
> So gem_mmap__device_coherent_domain calls the set_domain, not just 
> returns the needed domain?

I would say returns the domain, with _sync being the helper that mmaps
+ set-domain in one.
-Chris