[Intel-gfx] [PATCH v2 29/37] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET

[Matthew Auld]

On 30/07/2019 10:49, Daniel Vetter wrote:
> On Thu, Jun 27, 2019 at 09:56:25PM +0100, Matthew Auld wrote:
>> From: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>>
>> Add a new CPU mmap implementation that allows multiple fault handlers
>> that depends on the object's backing pages.
>>
>> Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
>> and use the zero extending behaviour of drm to differentiate between
>> them, when we inspect the flags.
>>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> 
> So I thought that the plan is to reject invalid mmaps, i.e. mmap modes
> which are not compatibale with all placement options. Given that, why do
> we need this?

We are meant to reject anything !wc for LMEM. There were some patches 
for that but I guess got lost under the radar...

> 
> - cpu mmap with all the flags still keep working, as long as the only
>    placement you select is smem.
> 
> - for lmem/stolen the only option we have is a wc mapping, either through
>    the pci bar or through the gtt. So for objects only sitting in there
>    also no problem, we can just keep using the current gtt mmap stuff (but
>    redirect it internally).
> 
> - that leaves us with objects which can move around. Only option allows is
>    WC, and the gtt mmap ioctl does that already. When the object is in smem
>    we'll need to redirect it to a cpu wc mmap, but I think we need to do
>    that anyway.

So for legacy, gtt_mmap will still go through the aperture, otherwise if 
LMEM is supported then there is no aperture, so we just wc mmap via cpu 
or LMEMBAR depending on the final object placement. And cpu_mmap still 
works if we don't care about LMEM. Hmm, so do we even need most of the 
previous patch then? ALso does that mean we also have to track the 
placement of an object in igt?

gem_mmap__wc:

if (supports_lmem(dev))
	gtt_mmap();
else
	gem_mmap(wc);

gem_mmap__wc:

if (placement_contains(obj, LMEM))
	gtt_mmap();
else
	gem_mmap(wc);

?

> 
> So not really seeing what the uapi problem is you're trying to solve here?
> 
> Can you pls explain why we need this?

The naming of gtt_mmap seemed confusing, since there is no aperture, and 
having one mmap ioctl to cover both smem and lmem seemed like a nice 
idea...also I think umd's stopped using gtt_mmap(or were told to?) but 
maybe those aren't good enough reasons.