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.
1) 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.
2) 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
