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

[Tvrtko Ursulin]

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?

Regards,

Tvrtko