Am 13.09.21 um 14:41 schrieb Thomas Hellström:
[SNIP]
Let's say you have a struct ttm_object_vram and a struct
ttm_object_gtt, both subclassing drm_gem_object. Then I'd say a
driver would want to subclass those to attach identical data,
extend functionality and provide a single i915_gem_object to the
rest of the driver, which couldn't care less whether it's vram or
gtt? Wouldn't you say having separate struct ttm_object_vram and a
struct ttm_object_gtt in this case would be awkward?. We *want* to
allow common handling.
Yeah, but that's a bad idea. This is like diamond inheritance in C++.
When you need the same functionality in different backends you
implement that as separate object and then add a parent class.
It's the exact same situation here. With struct ttm_resource you
let *different* implementation flavours subclass it, which makes it
awkward for the driver to extend the functionality in a common way
by subclassing, unless the driver only uses a single implementation.
Well the driver should use separate implementations for their
different domains as much as possible.
Hmm, Now you lost me a bit. Are you saying that the way we do dynamic
backends in the struct ttm_buffer_object to facilitate driver
subclassing is a bad idea or that the RFC with backpointer is a bad
idea?
Or if you mean diamond inheritance is bad, yes that's basically my point.
That diamond inheritance is a bad idea. What I don't understand is why
you need that in the first place?
Information that you attach to a resource are specific to the domain
where the resource is allocated from. So why do you want to attach the
same information to a resources from different domains?
Looking at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wikipedia.org%2Fwiki%2FMultiple_inheritance%23%2Fmedia%2FFile%3ADiamond_inheritance.svg&data=04%7C01%7Cchristian.koenig%40amd.com%7Cece4bd8aab644feacc1808d976b3ca56%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637671336950757656%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=LPMnfvC1px0bW8o420vP72oBbkm1v76A%2B0PDUw7urQY%3D&reserved=0
1)
A would be the struct ttm_resource itself,
D would be struct i915_resource,
B would be struct ttm_range_mgr_node,
C would be struct i915_ttm_buddy_resource
And we need to resolve the ambiguity using the awkward union
construct, iff we need to derive from both B and C.
Struct ttm_buffer_object and struct ttm_tt instead have B) and C)
being dynamic backends of A) or a single type derived from A) Hence
the problem doesn't exist for these types.
So the question from last email remains, if ditching this RFC, can we
have B) and C) implemented by helpers that can be used from D) and
that don't derive from A?
Well we already have that in the form of drm_mm. I mean the
ttm_range_manager is just a relatively small glue code which implements
the TTMs resource interface using the drm_mm object and a spinlock. IIRC
that less than 200 lines of code.
So you should already have the necessary helpers and just need to
implement the resource manager as far as I can see.
I mean I reused the ttm_range_manager_node in for amdgpu_gtt_mgr and
could potentially reuse a bit more of the ttm_range_manager code. But I
don't see that as much of an issue, the extra functionality there is
just minimal.
Regards,
Christian.
Thanks,
Thomas
