On Fri, Jun 28, 2019 at 3:16 AM Welty, Brian <brian.welty@xxxxxxxxx> wrote: > On 6/26/2019 11:01 PM, Daniel Vetter wrote: > > On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: > >> On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > >>> > >>> I think with all the ttm refactoring going on I think we need to de-ttm > >>> the interface functions here a bit. With Gerd Hoffmans series you can just > >>> use a gem_bo pointer here, so what's left to do is have some extracted > >>> structure for tracking memory types. I think Brian Welty has some ideas > >>> for this, even in patch form. Would be good to keep him on cc at least for > >>> the next version. We'd need to explicitly hand in the ttm_mem_reg (or > >>> whatever the specific thing is going to be). > >> > >> I assume Gerd Hoffman's series you are referring to is this one? > >> https://www.spinics.net/lists/dri-devel/msg215056.html > > > > There's a newer one, much more complete, but yes that's the work. > > > >> I can certainly keep an eye out for Gerd's refactoring while > >> refactoring other parts of this RFC. > >> > >> I have added Brian and Gerd to the thread for awareness. > > > > btw just realized that maybe building the interfaces on top of ttm_mem_reg > > is maybe not the best. That's what you're using right now, but in a way > > that's just the ttm internal detail of how the backing storage is > > allocated. I think the structure we need to abstract away is > > ttm_mem_type_manager, without any of the actual management details. > > > > Any de-ttm refactoring should probably not spam all the cgroups folks. > So I removed cgroups list. > > As Daniel mentioned, some of us are looking at possible refactoring of TTM > for reuse in i915 driver. > Here is a brief summary of some ideas to be considered: > > 1) refactor part of ttm_mem_type_manager into a new drm_mem_type_region. > Really, should then move the array from ttm_bo_device.man[] into drm_device. > > Relevant to drm_cgroup, you could then perhaps access these stats through > drm_device and don't need the mem_stats array in drmcgrp_device_resource. > > 1a) doing this right means replacing TTM_PL_XXX memory types with new DRM > defines. But could keep the TTM ones as redefinition of (new) DRM ones. > Probably those private ones (TTM_PL_PRIV) make this difficult. > > All of the above could be eventually leveraged by the vram support being > implemented now in i915 driver. > > 2) refactor ttm_mem_reg + ttm_bus_placement into something generic for > any GEM object, maybe call it drm_gem_object_placement. > ttm_mem_reg could remain as a wrapper for TTM drivers. > This hasn't been broadly discussed with intel-gfx folks, so not sure > this fits well into i915 or not. > > Relevant to drm_cgroup, maybe this function: > drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, > struct ttm_mem_reg *new_mem) > could potentially become: > drmcgrp_mem_track_move(struct drm_gem_object *old_bo, bool evict, > struct drm_gem_object_placement *new_place) > > Though from ttm_mem_reg, you look to only be using mem_type and size. > I think Daniel is noting that ttm_mem_reg wasn't truly needed here, so > you could just pass in the mem_type and size instead. Yeah I think the relevant part of your refactoring is creating a more abstract memory type/resource thing (not the individual allocations from it which ttm calls regions and I get confused about that every time). I think that abstraction should also have a field for the total (which I think cgroups also needs, both as read-only information and starting value). ttm would that put somewhere into the ttm_mem_type_manager, i915 would put it somewhere else, cma based drivers could perhaps expose the cma heap like that (if it's exclusive to the gpu at least). > Would appreciate any feedback (positive or negative) on above.... > Perhaps this should move to a new thread? I could send out basic RFC > patches for (1) if helpful but as it touches all the TTM drivers, nice to > hear some feedback first. > Anyway, this doesn't necessarily need to block forward progress on drm_cgroup, > as refactoring into common base structures could happen incrementally. Yeah I think new dri-devel thread with a totally unpolished rfc as draft plus this as the intro would be good. That way we can ground this a bit better in actual code. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
