On Wed, Sep 11, 2019 at 2:19 PM Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > Quoting Balestrieri, Francesco (2019-09-11 13:03:25) > > On 04/09/2019, 13.33, "Intel-gfx on behalf of Daniel Vetter" <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx on behalf of daniel@xxxxxxxx> wrote: > > > > On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue > > > - ret = create_mmap_offset(obj); > > > - if (ret == 0) > > > - *offset = drm_vma_node_offset_addr(&obj->base.vma_node); > > > + mmo = kzalloc(sizeof(*mmo), GFP_KERNEL); > > > > I got thrown off a bunch of times here reading the code, but I think I > > got this right now. > > > > Why exactly do we want multiple vma offsets? Yes this makes it a > > drop-in replacement for the old cpu mmap ioctl, which was a bit > > dubious design. But if we go all new here, I really wonder about why > > this is necessary. No other discrete driver needs this, they all fix > > the mmap mode for the lifetime of an object, because flushing stuff is > > as expensive as just reallocating (or at least close enough). > > > > I think us going once again our separate route here needs a lot more > > justification than just "we've accidentally ended up with uapi like > > this 10 years ago". > > That's exactly the whole point, to replace the uapi we accidentally > ended up with 10 years ago with the api that doesn't cause valgrind to > complain, is easily extensible and supports all legacy usecases which > should be a very good position to be in to support unknown future > usecases as well. Letting userspace control their mmapings is very > powerful, and we definitely do not want to be limiting their > flexibility. > > That no other driver even seems to allow multiple mmaps, and so has > not developed a desire to manage multiple vma per object does not seem > to be a reason to limit ourselves. The infrastructure all supports it; > the only thing that is at odds is the desire to force the lowest common > denominator as the defacto standard. Just because something is possible (or looks possible at first) doesn't make it good uapi. That's how we get stuff like gtt mmap on userptr bo, and then regrets. This entire thing here gets sold as "uapi cleanup for lmem". Which makes sense, except most of the uapi isn't really cleaned up at all: - We still have relocations (and we even made them more powerful by pipelining them, at a time when all our mesa drivers finally managed to move to softpin). We could ditch all the reloc stuff for gen12+ and lmem instead. Both amdgpu and nv50+ have done that. - We're still adding more uapi that's all adding variable state. Meanwhile everything else (gallium/iris, vk) move to invariant state object models, where you create stuff once with the right properties and call it a day. lmem does add quite a few new state bits here, would be a lot simpler if we could make them as invariant as possible. For placement we might need to allow changes to the priority, but not to the placement list itself. For mmap, you just select the mmap mode that you want (because once you fixing caching mode, placement list and tiling bits, there's really not a choice anymore), and that's the one you get with the single mmap offset. Instead we just hang onto all the accumulated complexity, add more, and the only cleanup I'm seeing is that we bake in the multi-mmap model to save one single line in userspace for the valgrind annotation. Which everyone has already anyway, that is not really going to go away. I think using lmem to clean up the uapi makes tons of sense, but if we bother with that it should be a real cleanup. Not just cosmetics for the one valgrind annotation line in userspace we have right now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
