[RFC PATCH] drm/ttm: Add a private member to the struct ttm_resource

* [RFC PATCH] drm/ttm: Add a private member to the struct ttm_resource
@ 2021-09-10 13:15 ` Thomas Hellström
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Hellström @ 2021-09-10 13:15 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: maarten.lankhorst, matthew.auld, Thomas Hellström,
	Matthew Auld, König Christian

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.

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;
 };
 
 /**
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread