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

[Daniel Vetter]

On Tue, Jul 30, 2019 at 6:22 PM Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Jul 30, 2019 at 03:28:11PM +0100, Matthew Auld wrote:
> > 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);
> >
> > ?
>
> Well if you want cpu wc mmaps, then just allocate it as smem ... we might
> need a new gem_mmap__lmem I guess to exercise all the possible ways to get
> at stuff in lmem (including when it migrates around underneath us while we
> access it through the mmap). I wouldn't try too hard to smash all these
> use/testcases into one.

Chatted a lot with Joonas today, and realized I missread outright what
this does. Looking at the end result I think it's all nicely aligned
with other (discrete/ttm) drivers, so all good from that point of
view. Still not sure whether it's really a good idea to do this fairly
minor uapi cleanup tied in with lmem. But I guess we committed to that
now, so welp ...
-Daniel

> > > 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.
>
> We stopped using gtt mmap because for many cases cpu WC mmap is faster.
>
> Wrt having a clean slate: Not sure why this would benefit us, we just
> diverge a bit more from how this works on !lmem, so a bit more complexity
> (not much) everywhere for not much gain.
>
> I'm also not sure whether there will be a whole lot of uses of such a
> magic LMEMBAR wc mapping. It's probably slow for the exact same reasons
> gtt mmap is slow.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch