Re: [PATCH 1/3] drm/ttm: Add range validation function
Jerome,
Please see comments inline.
Jerome Glisse wrote:
Add a function to validate buffer in a given range of
memory. This is needed by some hw for some of their
buffer (scanout buffer, cursor ...).
Signed-off-by: Jerome Glisse jgli...@redhat.com
---
drivers/gpu/drm/ttm/ttm_bo.c| 305
++-
include/drm/ttm/ttm_bo_api.h|5 +
include/drm/ttm/ttm_bo_driver.h |1 +
3 files changed, 310 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 87c0625..6b0e7e8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -247,7 +247,6 @@ EXPORT_SYMBOL(ttm_bo_unreserve);
/*
* Call bo-mutex locked.
*/
-
Whitespace.
static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
{
struct ttm_bo_device *bdev = bo-bdev;
@@ -418,6 +417,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo, bool remove_all)
kref_put(bo-list_kref, ttm_bo_ref_bug);
}
if (bo-mem.mm_node) {
+ bo-mem.mm_node-private = NULL;
drm_mm_put_block(bo-mem.mm_node);
bo-mem.mm_node = NULL;
}
@@ -610,6 +610,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
unsigned mem_type,
spin_lock(glob-lru_lock);
if (evict_mem.mm_node) {
+ evict_mem.mm_node-private = NULL;
drm_mm_put_block(evict_mem.mm_node);
evict_mem.mm_node = NULL;
}
@@ -826,6 +827,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
mem-mm_node = node;
mem-mem_type = mem_type;
mem-placement = cur_flags;
+ node-private = bo;
return 0;
}
I'd like a big warning here somewhere, because since the pointer to the
bo is not refcounted, it may
never be dereferenced outside of the lru spinlock, because as soon as
you release the lru spinlock, someone
else may destroy the buffer. The current usage is valid AFAICT. If you
choose to refcount it, use bo::list_kref.
@@ -856,6 +858,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
if (ret == 0 mem-mm_node) {
mem-placement = cur_flags;
+ mem-mm_node-private = bo;
return 0;
}
@@ -868,6 +871,173 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_mem_space);
+static unsigned long ttm_bo_free_size_if_evicted(struct ttm_buffer_object
*bo)
+{
+ struct ttm_mem_type_manager *man = bo-bdev-man[bo-mem.mem_type];
+ struct drm_mm_node *node;
+ unsigned long size;
+
+ size = bo-mem.mm_node-size;
+ if (bo-mem.mm_node-ml_entry.prev != man-manager.ml_entry) {
+ node = list_entry(bo-mem.mm_node-ml_entry.prev,
+ struct drm_mm_node, ml_entry);
+ if (node-free)
+ size += node-size;
+ }
+ if (bo-mem.mm_node-ml_entry.next != man-manager.ml_entry) {
+ node = list_entry(bo-mem.mm_node-ml_entry.next,
+ struct drm_mm_node, ml_entry);
+ if (node-free)
+ size += node-size;
+ }
+ return size;
+}
+
+static void ttm_manager_evict_first(struct ttm_buffer_object *bo)
+{
+ struct ttm_mem_type_manager *man = bo-bdev-man[bo-mem.mem_type];
+ unsigned long free_size_bo, free_size_bo_first;
+ struct ttm_buffer_object *bo_first;
+
+ /* BO is not on lru list, don't add it */
+ if (!list_empty(bo-lru))
+ return;
+ bo_first = list_first_entry(man-lru, struct ttm_buffer_object, lru);
+ free_size_bo = ttm_bo_free_size_if_evicted(bo);
+ free_size_bo_first = ttm_bo_free_size_if_evicted(bo_first);
+ if (free_size_bo free_size_bo_first) {
+ list_del_init(bo-lru);
+ list_add(bo-lru, man-lru);
+ }
+}
+
Hmm, Can you explain why we'd ever *not* want to put bo first on the lru
list?
I mean , bo_first might not even be in the correct range? That would
also save the size computations.
+static int ttm_manager_space_range(struct ttm_buffer_object *bo,
+ uint32_t mem_type,
+ struct ttm_mem_reg *mem,
+ unsigned long start, unsigned long end,
+ bool interruptible, bool no_wait)
+{
+ struct ttm_bo_global *glob = bo-glob;
+ struct drm_mm_node *entry;
+ struct ttm_mem_type_manager *man = bo-bdev-man[mem_type];
+ struct drm_mm *mm = man-manager;
+ unsigned size = end - start;
+ struct ttm_buffer_object *tbo;
+ unsigned wasted;
+ int ret;
+
+ mem-mm_node = NULL;
+ ret =
[PATCH 1/3] drm/ttm: Add range validation function
Add a function to validate buffer in a given range of
memory. This is needed by some hw for some of their
buffer (scanout buffer, cursor ...).
Signed-off-by: Jerome Glisse jgli...@redhat.com
---
drivers/gpu/drm/ttm/ttm_bo.c| 305 ++-
include/drm/ttm/ttm_bo_api.h|5 +
include/drm/ttm/ttm_bo_driver.h |1 +
3 files changed, 310 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 87c0625..6b0e7e8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -247,7 +247,6 @@ EXPORT_SYMBOL(ttm_bo_unreserve);
/*
* Call bo-mutex locked.
*/
-
static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
{
struct ttm_bo_device *bdev = bo-bdev;
@@ -418,6 +417,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo, bool remove_all)
kref_put(bo-list_kref, ttm_bo_ref_bug);
}
if (bo-mem.mm_node) {
+ bo-mem.mm_node-private = NULL;
drm_mm_put_block(bo-mem.mm_node);
bo-mem.mm_node = NULL;
}
@@ -610,6 +610,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
unsigned mem_type,
spin_lock(glob-lru_lock);
if (evict_mem.mm_node) {
+ evict_mem.mm_node-private = NULL;
drm_mm_put_block(evict_mem.mm_node);
evict_mem.mm_node = NULL;
}
@@ -826,6 +827,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
mem-mm_node = node;
mem-mem_type = mem_type;
mem-placement = cur_flags;
+ node-private = bo;
return 0;
}
@@ -856,6 +858,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
if (ret == 0 mem-mm_node) {
mem-placement = cur_flags;
+ mem-mm_node-private = bo;
return 0;
}
@@ -868,6 +871,173 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_mem_space);
+static unsigned long ttm_bo_free_size_if_evicted(struct ttm_buffer_object *bo)
+{
+ struct ttm_mem_type_manager *man = bo-bdev-man[bo-mem.mem_type];
+ struct drm_mm_node *node;
+ unsigned long size;
+
+ size = bo-mem.mm_node-size;
+ if (bo-mem.mm_node-ml_entry.prev != man-manager.ml_entry) {
+ node = list_entry(bo-mem.mm_node-ml_entry.prev,
+ struct drm_mm_node, ml_entry);
+ if (node-free)
+ size += node-size;
+ }
+ if (bo-mem.mm_node-ml_entry.next != man-manager.ml_entry) {
+ node = list_entry(bo-mem.mm_node-ml_entry.next,
+ struct drm_mm_node, ml_entry);
+ if (node-free)
+ size += node-size;
+ }
+ return size;
+}
+
+static void ttm_manager_evict_first(struct ttm_buffer_object *bo)
+{
+ struct ttm_mem_type_manager *man = bo-bdev-man[bo-mem.mem_type];
+ unsigned long free_size_bo, free_size_bo_first;
+ struct ttm_buffer_object *bo_first;
+
+ /* BO is not on lru list, don't add it */
+ if (!list_empty(bo-lru))
+ return;
+ bo_first = list_first_entry(man-lru, struct ttm_buffer_object, lru);
+ free_size_bo = ttm_bo_free_size_if_evicted(bo);
+ free_size_bo_first = ttm_bo_free_size_if_evicted(bo_first);
+ if (free_size_bo free_size_bo_first) {
+ list_del_init(bo-lru);
+ list_add(bo-lru, man-lru);
+ }
+}
+
+static int ttm_manager_space_range(struct ttm_buffer_object *bo,
+ uint32_t mem_type,
+ struct ttm_mem_reg *mem,
+ unsigned long start, unsigned long end,
+ bool interruptible, bool no_wait)
+{
+ struct ttm_bo_global *glob = bo-glob;
+ struct drm_mm_node *entry;
+ struct ttm_mem_type_manager *man = bo-bdev-man[mem_type];
+ struct drm_mm *mm = man-manager;
+ unsigned size = end - start;
+ struct ttm_buffer_object *tbo;
+ unsigned wasted;
+ int ret;
+
+ mem-mm_node = NULL;
+ ret = drm_mm_pre_get(man-manager);
+ if (unlikely(ret))
+ return ret;
+ spin_lock(glob-lru_lock);
+ list_for_each_entry(entry, mm-ml_entry, ml_entry) {
+ wasted = 0;
+ if (mem-page_alignment) {
+ unsigned tmp = entry-start % mem-page_alignment;
+ if (tmp)
+ wasted += mem-page_alignment - tmp;
+ }
+ if (entry-start end (entry-start+entry-size) start) {
+ if (!entry-free) {
+
