[Intel-gfx] [RFC PATCH 00/42] Introduce memory region concept (including device local memory)

[Dave Airlie]

On Tue, 19 Feb 2019 at 23:32, Joonas Lahtinen
<joonas.lahtinen at linux.intel.com> wrote:
>
> + dri-devel mailing list, especially for the buddy allocator part
>
> Quoting Dave Airlie (2019-02-15 02:47:07)
> > On Fri, 15 Feb 2019 at 00:57, Matthew Auld <matthew.auld at intel.com> wrote:
> > >
> > > In preparation for upcoming devices with device local memory, introduce the
> > > concept of different memory regions, and a simple buddy allocator to manage
> > > them.
> >
> > This is missing the information on why it's not TTM.
> >
> > Nothing against extending i915 gem off into doing stuff we already
> > have examples off in tree, but before you do that it would be good to
> > have a why we can't use TTM discussion in public.
>
> Glad that you asked. It's my fault that it was not included in
> the cover letter. I anticipated the question, but was travelling
> for a couple of days at the time this was sent. I didn't want
> to write a hasty explanation and then disappear, leaving others to
> take the heat.
>
> So here goes the less-hasty version:
>
> We did an analysis on the effort needed vs benefit gained of using
> TTM when this was started initially. The conclusion was that we
> already share the interesting bits of code through core DRM, really.
>
> Re-writing the memory handling to TTM would buy us more fine-grained
> locking. But it's more a trait of rewriting the memory handling with
> the information we have learned, than rewriting it to use TTM :)
>
> And further, we've been getting rid of struct_mutex at a steady phase
> in the past years, so we have a clear path to the fine-grained locking
> already in the not-so-distant future. With all this we did not see
> much gained from converting over, as the code sharing is already
> substantial.
>
> We also wanted to have the buddy allocator instead of a for loop making
> drm_mm allocations to make sure we can keep the memory fragmentation
> at bay. The intent is to move the buddy allocator to core DRM, to the
> benefit of all the drivers, if there is interest from community. It has
> been written as a strictly separate component with that in mind.
>
> And if you take the buddy allocator out of the patch set, the rest is
> mostly just vfuncing things up to be able to have different backing
> storages for objects. We took the opportunity to move over to the more
> valgrind friendly mmap while touching things, but it's something we
> have been contemplating anyway. And yeah, loads of selftests.
>
> That's really all that needed adding, and most of it is internal to
> i915 and not to do with uAPI. This means porting over an userspace
> driver doesn't require a substantial rewrite, but adding new a few
> new IOCTLs to set the preferred backing storage placements.
>
> All the previous GEM abstractions keep applying, so we did not see
> a justification to rewrite the kernel driver and userspace drivers.
> It would have just to made things look like TTM, when we already
> have the important parts of the code shared with TTM drivers
> behind the GEM interfaces which all our drivers sit on top of.

a) you guys should be the community as well, if the buddy allocator is
useful in the core DRM get out there and try and see if anyone else
has a use case for it, like the GPU scheduler we have now (can i915
use that yet? :-)

b) however this last two paragraphs fill me with no confidence that
you've looked at TTM at all. It sounds like you took comments about
TTM made 10 years ago, and didn't update them. There should be no
major reason for a uapi change just because you adopt TTM. TTM hasn't
ever had a common uapi across drivers upstream, one was proposed
initially > 10 years ago. All the current TTM using drivers except
vmware use a GEM based API as well. TTM is an internal driver helper
for managing pools of RAM.

I'm just not sure what rebuilding a chunk of shared code inside the
i915 driver is buying you, except a transition path into divergence
from all the other discrete RAM drivers. Like the gallium aversion in
userspace, having a TTM aversion in kernel space is going to be the
wrong path, and I'd rather not have to clean it up in 5 years when you
guys eventually realise it.

The i915 GEM code get rewritten and refactored quite often and has a
bus factor of ickle, if he decided to go elsewhere, you will have a
pile of code that nobody gets, I think having a TTM backend would have
a better long term effect on your driver maintainability.

Dave.