On 5/30/21 6:51 PM, Christian König wrote:
Hi Thomas,
Am 29.05.21 um 17:48 schrieb Thomas Hellström (Intel):
Hi, Christian,
On 4/30/21 11:25 AM, Christian König wrote:
Start with the range manager to make the resource object the base class for the allocated nodes.
While at it cleanup a lot of the code around that.
Could you briefly describe the design thoughts around this. While it's clear to me that we want separately allocated struct ttm_resource objects, it's not clear why the visibility of those are pushed down the interfaces to the range managers?
Why do you think the visibility is pushed to the range manger?
In addition to the need for a separately allocated struct ttm_resource, when looking at TTM-ify i915 I've come across a couple of problems.
- People have started abusing the range manager interface to attach
device private data to the mm_node, or probably really to the struct ttm_resource. That makes it very unclear what the input needed for the managers really are. For examle what members of the bo does the range manager really use and why? Same for the struct ttm_resource. I think in a perfect world, the interface to these range managers should be a struct ttm_placement as input and as output an opaque mm node and perhaps a way to convert that mm node to something useful like a range or a scatter-gather table.
Well I don't see that as an abuse. The backend allocation are completely driver specific and the range manager is just implementing some common ground for drm_mm based backends.
- But that doesn't really address the problem of drivers wanting to
attach device private data to a struct ttm_resource, which at some point caused someone to add a bo to the manager interface. The novueau driver attaches a "kind" member to the mm node that it pulls out of the bo; The i915 driver would want to cache an st table and a radix tree to cache index-to-pfn maps.
Driver specific backends should inherit either from the range manager when they want to implement a drm_mm based backend or from ttm_resource directly.
Hmm, OK so in our case a driver that needs a driver-specific struct ttm_resource, but still wants to be able to allocate either from drm_mm or from the buddy would then either have to re-implement the TTM drm_mm allocator or live with a pretty awkward construct?
struct i915_ttm_resource { union { struct ttm_resource res; struct ttm_range_mgr_node range_node; // Let's hope the struct ttm_resource remains the first member. struct i915_buddy_node buddy_node; }; struct i915_private_stuff common_for_all_backends; };
/Thomas