Am 10.09.21 um 15:15 schrieb Thomas Hellström:
Both the provider (resource manager) and the consumer (the TTM driver) want to subclass struct ttm_resource. Since this is left for the resource manager, we need to provide a private pointer for the TTM driver.
Provide a struct ttm_resource_private for the driver to subclass for data with the same lifetime as the struct ttm_resource: In the i915 case it will, for example, be an sg-table and radix tree into the LMEM /VRAM pages that currently are awkwardly attached to the GEM object.
Provide an ops structure for associated ops (Which is only destroy() ATM) It might seem pointless to provide a separate ops structure, but Linus has previously made it clear that that's the norm.
After careful audit one could perhaps also on a per-driver basis replace the delete_mem_notify() TTM driver callback with the above destroy function.
Well this is a really big NAK to this approach.
If you need to attach some additional information to the resource then implement your own resource manager like everybody else does.
Regards, Christian.
Cc: Matthew Auld matthew.william.auld@gmail.com Cc: König Christian Christian.Koenig@amd.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
drivers/gpu/drm/ttm/ttm_resource.c | 10 +++++++--- include/drm/ttm/ttm_resource.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 2431717376e7..973e7c50bfed 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -57,13 +57,17 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo, void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res) { struct ttm_resource_manager *man;
- struct ttm_resource *resource = *res;
- if (!*res)
- if (!resource) return;
- man = ttm_manager_type(bo->bdev, (*res)->mem_type);
- man->func->free(man, *res); *res = NULL;
- if (resource->priv)
resource->priv->ops.destroy(resource->priv);- man = ttm_manager_type(bo->bdev, resource->mem_type);
- man->func->free(man, resource); } EXPORT_SYMBOL(ttm_resource_free);
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index 140b6b9a8bbe..5a22c9a29c05 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -44,6 +44,7 @@ struct dma_buf_map; struct io_mapping; struct sg_table; struct scatterlist; +struct ttm_resource_private;
struct ttm_resource_manager_func { /** @@ -153,6 +154,32 @@ struct ttm_bus_placement { enum ttm_caching caching; };
+/**
- struct ttm_resource_private_ops - Operations for a struct
- ttm_resource_private
- Not much benefit to keep this as a separate struct with only a single member,
- but keeping a separate ops struct is the norm.
- */
+struct ttm_resource_private_ops {
- /**
* destroy() - Callback to destroy the private data* @priv - The private data to destroy*/- void (*destroy) (struct ttm_resource_private *priv);
+};
+/**
- struct ttm_resource_private - TTM driver private data
- @ops: Pointer to struct ttm_resource_private_ops with associated operations
- Intended to be subclassed to hold, for example cached data sharing the
- lifetime with a struct ttm_resource.
- */
+struct ttm_resource_private {
- const struct ttm_resource_private_ops ops;
+};
- /**
- struct ttm_resource
@@ -171,6 +198,7 @@ struct ttm_resource { uint32_t mem_type; uint32_t placement; struct ttm_bus_placement bus;
struct ttm_resource_private *priv; };
/**