From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com,
"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
"Matthew Auld" <matthew.william.auld@gmail.com>,
"König Christian" <Christian.Koenig@amd.com>
Subject: [Intel-gfx] [RFC PATCH] drm/ttm: Add a private member to the struct ttm_resource
Date: Fri, 10 Sep 2021 15:15:12 +0200 [thread overview]
Message-ID: <20210910131512.161655-1-thomas.hellstrom@linux.intel.com> (raw)
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
WARNING: multiple messages have this Message-ID (diff)
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com,
"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
"Matthew Auld" <matthew.william.auld@gmail.com>,
"König Christian" <Christian.Koenig@amd.com>
Subject: [RFC PATCH] drm/ttm: Add a private member to the struct ttm_resource
Date: Fri, 10 Sep 2021 15:15:12 +0200 [thread overview]
Message-ID: <20210910131512.161655-1-thomas.hellstrom@linux.intel.com> (raw)
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
next reply other threads:[~2021-09-10 13:15 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-10 13:15 Thomas Hellström [this message]
2021-09-10 13:15 ` [RFC PATCH] drm/ttm: Add a private member to the struct ttm_resource Thomas Hellström
2021-09-10 13:25 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2021-09-10 13:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-10 14:40 ` [Intel-gfx] [RFC PATCH] " Christian König
2021-09-10 14:40 ` Christian König
2021-09-10 15:30 ` [Intel-gfx] " Thomas Hellström
2021-09-10 15:30 ` Thomas Hellström
2021-09-10 17:03 ` [Intel-gfx] " Christian König
2021-09-10 17:03 ` Christian König
2021-09-11 6:07 ` [Intel-gfx] " Thomas Hellström
2021-09-11 6:07 ` Thomas Hellström
2021-09-13 6:17 ` [Intel-gfx] " Christian König
2021-09-13 6:17 ` Christian König
2021-09-13 9:36 ` [Intel-gfx] " Thomas Hellström
2021-09-13 9:36 ` Thomas Hellström
2021-09-13 9:41 ` [Intel-gfx] " Christian König
2021-09-13 9:41 ` Christian König
2021-09-13 10:16 ` [Intel-gfx] " Thomas Hellström
2021-09-13 10:16 ` Thomas Hellström
2021-09-13 12:41 ` [Intel-gfx] " Thomas Hellström
2021-09-13 12:41 ` Thomas Hellström
2021-09-14 7:40 ` [Intel-gfx] " Christian König
2021-09-14 7:40 ` Christian König
2021-09-14 8:27 ` [Intel-gfx] " Thomas Hellström
2021-09-14 8:27 ` Thomas Hellström
2021-09-14 8:53 ` [Intel-gfx] " Christian König
2021-09-14 8:53 ` Christian König
2021-09-14 10:38 ` [Intel-gfx] " Thomas Hellström
2021-09-14 10:38 ` Thomas Hellström
2021-09-14 14:07 ` [Intel-gfx] " Daniel Vetter
2021-09-14 14:07 ` Daniel Vetter
2021-09-14 15:30 ` [Intel-gfx] " Thomas Hellström
2021-09-14 15:30 ` Thomas Hellström
2021-09-10 15:12 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210910131512.161655-1-thomas.hellstrom@linux.intel.com \
--to=thomas.hellstrom@linux.intel.com \
--cc=Christian.Koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.auld@intel.com \
--cc=matthew.william.auld@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.