* [PATCH 1/9] drm/ttm: add common accounting to the resource mgr v3
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 8:40 ` [PATCH 2/9] drm/ttm: move the LRU into resource handling v3 Christian König
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
It makes sense to have this in the common manager for debugging and
accounting of how much resources are used.
v2: cleanup kerneldoc a bit
v3: drop the atomic, update counter under lock instead
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
include/drm/ttm/ttm_resource.h | 11 +++++++++--
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 68344c90549b..7f9ec64ebaba 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res)
{
+ struct ttm_resource_manager *man;
+
res->start = 0;
res->num_pages = PFN_UP(bo->base.size);
res->mem_type = place->mem_type;
@@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+
+ man = ttm_manager_type(bo->bdev, place->mem_type);
+ spin_lock(&bo->bdev->lru_lock);
+ man->usage += bo->base.size;
+ spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
+ spin_lock(&man->bdev->lru_lock);
+ man->usage -= res->bo->base.size;
+ spin_unlock(&man->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_fini);
@@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
spin_lock_init(&man->move_lock);
man->bdev = bdev;
man->size = p_size;
+ man->usage = 0;
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
INIT_LIST_HEAD(&man->lru[i]);
@@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
}
EXPORT_SYMBOL(ttm_resource_manager_evict_all);
+/**
+ * ttm_resource_manager_usage
+ *
+ * @man: A memory manager object.
+ *
+ * Return how many resources are currently used.
+ */
+uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
+{
+ uint64_t usage;
+
+ spin_lock(&man->bdev->lru_lock);
+ usage = man->usage;
+ spin_unlock(&man->bdev->lru_lock);
+ return usage;
+}
+EXPORT_SYMBOL(ttm_resource_manager_usage);
+
/**
* ttm_resource_manager_debug
*
@@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
drm_printf(p, " use_type: %d\n", man->use_type);
drm_printf(p, " use_tt: %d\n", man->use_tt);
drm_printf(p, " size: %llu\n", man->size);
+ drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
if (man->func->debug)
man->func->debug(man, p);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 69eea9d6399b..5516d6340aa7 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -27,6 +27,7 @@
#include <linux/types.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
#include <drm/drm_print.h>
@@ -130,10 +131,15 @@ struct ttm_resource_manager {
struct dma_fence *move;
/*
- * Protected by the global->lru_lock.
+ * Protected by the bdev->lru_lock.
*/
-
struct list_head lru[TTM_MAX_BO_PRIORITY];
+
+ /**
+ * @usage: How much of the resources are used, protected by the
+ * bdev->lru_lock.
+ */
+ uint64_t usage;
};
/**
@@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
int ttm_resource_manager_evict_all(struct ttm_device *bdev,
struct ttm_resource_manager *man);
+uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
void ttm_resource_manager_debug(struct ttm_resource_manager *man,
struct drm_printer *p);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/9] drm/ttm: move the LRU into resource handling v3
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
2022-02-09 8:40 ` [PATCH 1/9] drm/ttm: add common accounting to the resource mgr v3 Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 10:09 ` Matthew Auld
2022-02-09 8:40 ` [PATCH 3/9] drm/ttm: add resource iterator v2 Christian König
` (6 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
This way we finally fix the problem that new resource are
not immediately evict-able after allocation.
That has caused numerous problems including OOM on GDS handling
and not being able to use TTM as general resource manager.
v2: stop assuming in ttm_resource_fini that res->bo is still valid.
v3: cleanup kerneldoc, add more lockdep annotation
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 115 ++--------------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c | 64 ++++++-------
drivers/gpu/drm/ttm/ttm_resource.c | 122 +++++++++++++++++++++++-
include/drm/ttm/ttm_bo_api.h | 16 ----
include/drm/ttm/ttm_bo_driver.h | 29 +-----
include/drm/ttm/ttm_resource.h | 35 +++++++
9 files changed, 197 insertions(+), 195 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b37fc7d7d2c7..f2ce5a0defd9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (vm->bulk_moveable) {
spin_lock(&adev->mman.bdev.lru_lock);
- ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
+ ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
spin_unlock(&adev->mman.bdev.lru_lock);
return;
}
- memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
+ ttm_lru_bulk_move_init(&vm->lru_bulk_move);
spin_lock(&adev->mman.bdev.lru_lock);
list_for_each_entry(bo_base, &vm->idle, vm_status) {
@@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (!bo->parent)
continue;
- ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
- &vm->lru_bulk_move);
+ ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
if (shadow)
ttm_bo_move_to_lru_tail(&shadow->tbo,
- shadow->tbo.resource,
&vm->lru_bulk_move);
}
spin_unlock(&adev->mman.bdev.lru_lock);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index de3fe79b665a..582e8dc9bc8c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -849,7 +849,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
bo->priority = I915_TTM_PRIO_NO_PAGES;
}
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index db3dc7ef5382..cb0fa932d495 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,108 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
}
}
-static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
-{
- struct ttm_device *bdev = bo->bdev;
-
- list_move_tail(&bo->lru, &bdev->pinned);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-}
-
-static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
-{
- struct ttm_device *bdev = bo->bdev;
-
- list_del_init(&bo->lru);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-}
-
-static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
- struct ttm_buffer_object *bo)
-{
- if (!pos->first)
- pos->first = bo;
- pos->last = bo;
-}
-
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk)
{
- struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
-
- if (!bo->deleted)
- dma_resv_assert_held(bo->base.resv);
-
- if (bo->pin_count) {
- ttm_bo_move_to_pinned(bo);
- return;
- }
-
- if (!mem)
- return;
-
- man = ttm_manager_type(bdev, mem->mem_type);
- list_move_tail(&bo->lru, &man->lru[bo->priority]);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-
- if (bulk && !bo->pin_count) {
- switch (bo->resource->mem_type) {
- case TTM_PL_TT:
- ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
- break;
+ dma_resv_assert_held(bo->base.resv);
- case TTM_PL_VRAM:
- ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
- break;
- }
- }
+ if (bo->resource)
+ ttm_resource_move_to_lru_tail(bo->resource, bulk);
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
-{
- unsigned i;
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-}
-EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
-
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
@@ -344,7 +252,6 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
return ret;
}
- ttm_bo_move_to_pinned(bo);
list_del_init(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
ttm_bo_cleanup_memtype_use(bo);
@@ -445,7 +352,7 @@ static void ttm_bo_release(struct kref *kref)
*/
if (bo->pin_count) {
bo->pin_count = 0;
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_resource_move_to_lru_tail(bo->resource, NULL);
}
kref_init(&bo->kref);
@@ -458,7 +365,6 @@ static void ttm_bo_release(struct kref *kref)
}
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_del_from_lru(bo);
list_del(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
@@ -673,15 +579,17 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource *res;
bool locked = false;
unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(bo, &man->lru[i], lru) {
+ list_for_each_entry(res, &man->lru[i], lru) {
bool busy;
+ bo = res->bo;
if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
&locked, &busy)) {
if (busy && !busy_bo && ticket !=
@@ -699,7 +607,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
}
/* If the inner loop terminated early, we have our candidate */
- if (&bo->lru != &man->lru[i])
+ if (&res->lru != &man->lru[i])
break;
bo = NULL;
@@ -875,9 +783,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
}
error:
- if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_mem_space);
@@ -971,7 +876,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
kref_init(&bo->kref);
- INIT_LIST_HEAD(&bo->lru);
INIT_LIST_HEAD(&bo->ddestroy);
bo->bdev = bdev;
bo->type = type;
@@ -1021,8 +925,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
return ret;
}
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init_reserved);
@@ -1123,7 +1025,6 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
return ret == -EBUSY ? -ENOSPC : ret;
}
- ttm_bo_move_to_pinned(bo);
/* TODO: Cleanup the locking */
spin_unlock(&bo->bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 544a84fa6589..0163e92b57af 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -231,7 +231,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
atomic_inc(&ttm_glob.bo_count);
INIT_LIST_HEAD(&fbo->base.ddestroy);
- INIT_LIST_HEAD(&fbo->base.lru);
fbo->base.moving = NULL;
drm_vma_node_reset(&fbo->base.base.vma_node);
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index be24bb6cefd0..ba35887147ba 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -144,6 +144,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
{
struct ttm_resource_manager *man;
struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
unsigned i, j;
int ret;
@@ -154,8 +155,11 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
continue;
for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(bo, &man->lru[j], lru) {
- uint32_t num_pages = PFN_UP(bo->base.size);
+ list_for_each_entry(res, &man->lru[j], lru) {
+ uint32_t num_pages;
+
+ bo = res->bo;
+ num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
@@ -259,49 +263,45 @@ void ttm_device_fini(struct ttm_device *bdev)
}
EXPORT_SYMBOL(ttm_device_fini);
-void ttm_device_clear_dma_mappings(struct ttm_device *bdev)
+static void ttm_device_clear_lru_dma_mappings(struct ttm_device *bdev,
+ struct list_head *list)
{
- struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
- unsigned int i, j;
+ struct ttm_resource *res;
spin_lock(&bdev->lru_lock);
- while (!list_empty(&bdev->pinned)) {
- bo = list_first_entry(&bdev->pinned, struct ttm_buffer_object, lru);
+ while ((res = list_first_entry_or_null(list, typeof(*res), lru))) {
+ struct ttm_buffer_object *bo = res->bo;
+
/* Take ref against racing releases once lru_lock is unlocked */
- if (ttm_bo_get_unless_zero(bo)) {
- list_del_init(&bo->lru);
- spin_unlock(&bdev->lru_lock);
+ if (!ttm_bo_get_unless_zero(bo))
+ continue;
- if (bo->ttm)
- ttm_tt_unpopulate(bo->bdev, bo->ttm);
+ list_del_init(&res->lru);
+ spin_unlock(&bdev->lru_lock);
- ttm_bo_put(bo);
- spin_lock(&bdev->lru_lock);
- }
+ if (bo->ttm)
+ ttm_tt_unpopulate(bo->bdev, bo->ttm);
+
+ ttm_bo_put(bo);
+ spin_lock(&bdev->lru_lock);
}
+ spin_unlock(&bdev->lru_lock);
+}
+
+void ttm_device_clear_dma_mappings(struct ttm_device *bdev)
+{
+ struct ttm_resource_manager *man;
+ unsigned int i, j;
+
+ ttm_device_clear_lru_dma_mappings(bdev, &bdev->pinned);
for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) {
man = ttm_manager_type(bdev, i);
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- while (!list_empty(&man->lru[j])) {
- bo = list_first_entry(&man->lru[j], struct ttm_buffer_object, lru);
- if (ttm_bo_get_unless_zero(bo)) {
- list_del_init(&bo->lru);
- spin_unlock(&bdev->lru_lock);
-
- if (bo->ttm)
- ttm_tt_unpopulate(bo->bdev, bo->ttm);
-
- ttm_bo_put(bo);
- spin_lock(&bdev->lru_lock);
- }
- }
- }
+ for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j)
+ ttm_device_clear_lru_dma_mappings(bdev, &man->lru[j]);
}
- spin_unlock(&bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_device_clear_dma_mappings);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 7f9ec64ebaba..c124db5039f0 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -29,6 +29,110 @@
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_bo_driver.h>
+/**
+ * ttm_lru_bulk_move_init - initialize a bulk move structure
+ * @bulk: the structure to init
+ *
+ * For now just memset the structure to zero.
+ */
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
+{
+ memset(bulk, 0, sizeof(*bulk));
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_init);
+
+/**
+ * ttm_lru_bulk_move_tail - bulk move range of resources to the LRU tail.
+ *
+ * @bulk: bulk move structure
+ *
+ * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
+ * resource order never changes. Should be called with &ttm_device.lru_lock held.
+ */
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
+{
+ unsigned i;
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
+
+/* Record a resource position in a bulk move structure */
+static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
+ struct ttm_resource *res)
+{
+ if (!pos->first)
+ pos->first = res;
+ pos->last = res;
+}
+
+/* Move a resource to the LRU tail and track the bulk position */
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk)
+{
+ struct ttm_buffer_object *bo = res->bo;
+ struct ttm_device *bdev = bo->bdev;
+ struct ttm_resource_manager *man;
+
+ lockdep_assert_held(&bo->bdev->lru_lock);
+
+ if (bo->pin_count) {
+ list_move_tail(&res->lru, &bdev->pinned);
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(res->bo);
+ return;
+ }
+
+ man = ttm_manager_type(bdev, res->mem_type);
+ list_move_tail(&res->lru, &man->lru[bo->priority]);
+
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(bo);
+
+ if (!bulk)
+ return;
+
+ switch (res->mem_type) {
+ case TTM_PL_TT:
+ ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
+ break;
+
+ case TTM_PL_VRAM:
+ ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
+ break;
+ }
+}
+
/**
* ttm_resource_init - resource object constructure
* @bo: buffer object this resources is allocated for
@@ -52,10 +156,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+ INIT_LIST_HEAD(&res->lru);
man = ttm_manager_type(bo->bdev, place->mem_type);
spin_lock(&bo->bdev->lru_lock);
man->usage += bo->base.size;
+ ttm_resource_move_to_lru_tail(res, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -66,15 +172,21 @@ EXPORT_SYMBOL(ttm_resource_init);
* @res: the resource to clean up
*
* Should be used by resource manager backends to clean up the TTM resource
- * objects before freeing the underlying structure. Counterpart of
- * &ttm_resource_init
+ * objects before freeing the underlying structure. Makes sure the resource is
+ * removed from the LRU before destruction.
+ * Counterpart of &ttm_resource_init.
*/
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- spin_lock(&man->bdev->lru_lock);
- man->usage -= res->bo->base.size;
- spin_unlock(&man->bdev->lru_lock);
+ struct ttm_device *bdev = man->bdev;
+
+ spin_lock(&bdev->lru_lock);
+ list_del_init(&res->lru);
+ if (res->bo && bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(res->bo);
+ man->usage -= res->num_pages << PAGE_SHIFT;
+ spin_unlock(&bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_fini);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c17b2df9178b..3da77fc54552 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -55,8 +55,6 @@ struct ttm_placement;
struct ttm_place;
-struct ttm_lru_bulk_move;
-
/**
* enum ttm_bo_type
*
@@ -94,7 +92,6 @@ struct ttm_tt;
* @ttm: TTM structure holding system pages.
* @evicted: Whether the object was evicted without user-space knowing.
* @deleted: True if the object is only a zombie and already deleted.
- * @lru: List head for the lru list.
* @ddestroy: List head for the delayed destroy list.
* @swap: List head for swap LRU list.
* @moving: Fence set when BO is moving
@@ -143,7 +140,6 @@ struct ttm_buffer_object {
* Members protected by the bdev::lru_lock.
*/
- struct list_head lru;
struct list_head ddestroy;
/**
@@ -295,7 +291,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* ttm_bo_move_to_lru_tail
*
* @bo: The buffer object.
- * @mem: Resource object.
* @bulk: optional bulk move structure to remember BO positions
*
* Move this BO to the tail of all lru lists used to lookup and reserve an
@@ -303,19 +298,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* held, and is used to make a BO less likely to be considered for eviction.
*/
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk);
-/**
- * ttm_bo_bulk_move_lru_tail
- *
- * @bulk: bulk move structure
- *
- * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
- * BO order never changes. Should be called with ttm_global::lru_lock held.
- */
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
-
/**
* ttm_bo_lock_delayed_workqueue
*
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 5f087575194b..6c7352e13708 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -45,33 +45,6 @@
#include "ttm_tt.h"
#include "ttm_pool.h"
-/**
- * struct ttm_lru_bulk_move_pos
- *
- * @first: first BO in the bulk move range
- * @last: last BO in the bulk move range
- *
- * Positions for a lru bulk move.
- */
-struct ttm_lru_bulk_move_pos {
- struct ttm_buffer_object *first;
- struct ttm_buffer_object *last;
-};
-
-/**
- * struct ttm_lru_bulk_move
- *
- * @tt: first/last lru entry for BOs in the TT domain
- * @vram: first/last lru entry for BOs in the VRAM domain
- * @swap: first/last lru entry for BOs on the swap list
- *
- * Helper structure for bulk moves on the LRU list.
- */
-struct ttm_lru_bulk_move {
- struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
- struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
-};
-
/*
* ttm_bo.c
*/
@@ -182,7 +155,7 @@ static inline void
ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
{
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 5516d6340aa7..9bc5c0280a09 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -26,10 +26,12 @@
#define _TTM_RESOURCE_H_
#include <linux/types.h>
+#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
+
#include <drm/drm_print.h>
#include <drm/ttm/ttm_caching.h>
#include <drm/ttm/ttm_kmap_iter.h>
@@ -179,6 +181,33 @@ struct ttm_resource {
uint32_t placement;
struct ttm_bus_placement bus;
struct ttm_buffer_object *bo;
+ struct list_head lru;
+};
+
+/**
+ * struct ttm_lru_bulk_move_pos
+ *
+ * @first: first res in the bulk move range
+ * @last: last res in the bulk move range
+ *
+ * Positions for a lru bulk move.
+ */
+struct ttm_lru_bulk_move_pos {
+ struct ttm_resource *first;
+ struct ttm_resource *last;
+};
+
+/**
+ * struct ttm_lru_bulk_move
+ *
+ * @tt: first/last lru entry for resources in the TT domain
+ * @vram: first/last lru entry for resources in the VRAM domain
+ *
+ * Helper structure for bulk moves on the LRU list.
+ */
+struct ttm_lru_bulk_move {
+ struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
+ struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
};
/**
@@ -267,6 +296,12 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
man->move = NULL;
}
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
+
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk);
+
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/9] drm/ttm: move the LRU into resource handling v3
2022-02-09 8:40 ` [PATCH 2/9] drm/ttm: move the LRU into resource handling v3 Christian König
@ 2022-02-09 10:09 ` Matthew Auld
2022-02-09 12:08 ` Christian König
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Auld @ 2022-02-09 10:09 UTC (permalink / raw)
To: Christian König
Cc: Thomas Hellström, ML dri-devel, ray.huang, amd-gfx list,
Daniel Vetter
On Wed, 9 Feb 2022 at 08:41, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> This way we finally fix the problem that new resource are
> not immediately evict-able after allocation.
>
> That has caused numerous problems including OOM on GDS handling
> and not being able to use TTM as general resource manager.
>
> v2: stop assuming in ttm_resource_fini that res->bo is still valid.
> v3: cleanup kerneldoc, add more lockdep annotation
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 115 ++--------------------
> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
> drivers/gpu/drm/ttm/ttm_device.c | 64 ++++++-------
> drivers/gpu/drm/ttm/ttm_resource.c | 122 +++++++++++++++++++++++-
> include/drm/ttm/ttm_bo_api.h | 16 ----
> include/drm/ttm/ttm_bo_driver.h | 29 +-----
> include/drm/ttm/ttm_resource.h | 35 +++++++
> 9 files changed, 197 insertions(+), 195 deletions(-)
<snip>
> /**
> * ttm_resource_init - resource object constructure
> * @bo: buffer object this resources is allocated for
> @@ -52,10 +156,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> res->bus.is_iomem = false;
> res->bus.caching = ttm_cached;
> res->bo = bo;
> + INIT_LIST_HEAD(&res->lru);
>
> man = ttm_manager_type(bo->bdev, place->mem_type);
> spin_lock(&bo->bdev->lru_lock);
> man->usage += bo->base.size;
> + ttm_resource_move_to_lru_tail(res, NULL);
> spin_unlock(&bo->bdev->lru_lock);
> }
> EXPORT_SYMBOL(ttm_resource_init);
> @@ -66,15 +172,21 @@ EXPORT_SYMBOL(ttm_resource_init);
> * @res: the resource to clean up
> *
> * Should be used by resource manager backends to clean up the TTM resource
> - * objects before freeing the underlying structure. Counterpart of
> - * &ttm_resource_init
> + * objects before freeing the underlying structure. Makes sure the resource is
> + * removed from the LRU before destruction.
> + * Counterpart of &ttm_resource_init.
> */
> void ttm_resource_fini(struct ttm_resource_manager *man,
> struct ttm_resource *res)
> {
> - spin_lock(&man->bdev->lru_lock);
> - man->usage -= res->bo->base.size;
> - spin_unlock(&man->bdev->lru_lock);
> + struct ttm_device *bdev = man->bdev;
> +
> + spin_lock(&bdev->lru_lock);
> + list_del_init(&res->lru);
> + if (res->bo && bdev->funcs->del_from_lru_notify)
> + bdev->funcs->del_from_lru_notify(res->bo);
> + man->usage -= res->num_pages << PAGE_SHIFT;
Above we are using the bo->base.size for tracking usage, but here we
are using num_pages. Is it guaranteed that bo->base.size is always
page aligned? Do we need some kind of WARN_ON()? Perhaps also sanity
checking that usage == 0 when tearing down the man?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/9] drm/ttm: move the LRU into resource handling v3
2022-02-09 10:09 ` Matthew Auld
@ 2022-02-09 12:08 ` Christian König
0 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 12:08 UTC (permalink / raw)
To: Matthew Auld
Cc: Thomas Hellström, ML dri-devel, ray.huang, amd-gfx list,
Daniel Vetter
Am 09.02.22 um 11:09 schrieb Matthew Auld:
> On Wed, 9 Feb 2022 at 08:41, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> This way we finally fix the problem that new resource are
>> not immediately evict-able after allocation.
>>
>> That has caused numerous problems including OOM on GDS handling
>> and not being able to use TTM as general resource manager.
>>
>> v2: stop assuming in ttm_resource_fini that res->bo is still valid.
>> v3: cleanup kerneldoc, add more lockdep annotation
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
>> drivers/gpu/drm/ttm/ttm_bo.c | 115 ++--------------------
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
>> drivers/gpu/drm/ttm/ttm_device.c | 64 ++++++-------
>> drivers/gpu/drm/ttm/ttm_resource.c | 122 +++++++++++++++++++++++-
>> include/drm/ttm/ttm_bo_api.h | 16 ----
>> include/drm/ttm/ttm_bo_driver.h | 29 +-----
>> include/drm/ttm/ttm_resource.h | 35 +++++++
>> 9 files changed, 197 insertions(+), 195 deletions(-)
> <snip>
>
>> /**
>> * ttm_resource_init - resource object constructure
>> * @bo: buffer object this resources is allocated for
>> @@ -52,10 +156,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>> res->bus.is_iomem = false;
>> res->bus.caching = ttm_cached;
>> res->bo = bo;
>> + INIT_LIST_HEAD(&res->lru);
>>
>> man = ttm_manager_type(bo->bdev, place->mem_type);
>> spin_lock(&bo->bdev->lru_lock);
>> man->usage += bo->base.size;
>> + ttm_resource_move_to_lru_tail(res, NULL);
>> spin_unlock(&bo->bdev->lru_lock);
>> }
>> EXPORT_SYMBOL(ttm_resource_init);
>> @@ -66,15 +172,21 @@ EXPORT_SYMBOL(ttm_resource_init);
>> * @res: the resource to clean up
>> *
>> * Should be used by resource manager backends to clean up the TTM resource
>> - * objects before freeing the underlying structure. Counterpart of
>> - * &ttm_resource_init
>> + * objects before freeing the underlying structure. Makes sure the resource is
>> + * removed from the LRU before destruction.
>> + * Counterpart of &ttm_resource_init.
>> */
>> void ttm_resource_fini(struct ttm_resource_manager *man,
>> struct ttm_resource *res)
>> {
>> - spin_lock(&man->bdev->lru_lock);
>> - man->usage -= res->bo->base.size;
>> - spin_unlock(&man->bdev->lru_lock);
>> + struct ttm_device *bdev = man->bdev;
>> +
>> + spin_lock(&bdev->lru_lock);
>> + list_del_init(&res->lru);
>> + if (res->bo && bdev->funcs->del_from_lru_notify)
>> + bdev->funcs->del_from_lru_notify(res->bo);
>> + man->usage -= res->num_pages << PAGE_SHIFT;
> Above we are using the bo->base.size for tracking usage, but here we
> are using num_pages. Is it guaranteed that bo->base.size is always
> page aligned? Do we need some kind of WARN_ON()? Perhaps also sanity
> checking that usage == 0 when tearing down the man?
Good point, going to add that and consistently using res->num_pages for
both.
Thanks,
Christian.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/9] drm/ttm: add resource iterator v2
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
2022-02-09 8:40 ` [PATCH 1/9] drm/ttm: add common accounting to the resource mgr v3 Christian König
2022-02-09 8:40 ` [PATCH 2/9] drm/ttm: move the LRU into resource handling v3 Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 8:40 ` [PATCH 4/9] drm/radeon: remove resource accounting Christian König
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
Instead of duplicating that at different places add an iterator over all
the resources in a resource manager.
v2: add lockdep annotation and kerneldoc
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/ttm/ttm_bo.c | 41 ++++++++++---------------
drivers/gpu/drm/ttm/ttm_device.c | 26 +++++++---------
drivers/gpu/drm/ttm/ttm_resource.c | 49 ++++++++++++++++++++++++++++++
include/drm/ttm/ttm_resource.h | 23 ++++++++++++++
4 files changed, 99 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb0fa932d495..599be3dda8a9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -579,38 +579,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource_cursor cursor;
struct ttm_resource *res;
bool locked = false;
- unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(res, &man->lru[i], lru) {
- bool busy;
-
- bo = res->bo;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
- &locked, &busy)) {
- if (busy && !busy_bo && ticket !=
- dma_resv_locking_ctx(bo->base.resv))
- busy_bo = bo;
- continue;
- }
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
- break;
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
+ &locked, &busy)) {
+ if (busy && !busy_bo && ticket !=
+ dma_resv_locking_ctx(bo->base.resv))
+ busy_bo = res->bo;
+ continue;
}
- /* If the inner loop terminated early, we have our candidate */
- if (&res->lru != &man->lru[i])
- break;
-
- bo = NULL;
+ if (!ttm_bo_get_unless_zero(res->bo)) {
+ if (locked)
+ dma_resv_unlock(res->bo->base.resv);
+ continue;
+ }
+ bo = res->bo;
}
if (!bo) {
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index ba35887147ba..a0562ab386f5 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout);
int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
gfp_t gfp_flags)
{
+ struct ttm_resource_cursor cursor;
struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
struct ttm_resource *res;
- unsigned i, j;
+ unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
@@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(res, &man->lru[j], lru) {
- uint32_t num_pages;
-
- bo = res->bo;
- num_pages = PFN_UP(bo->base.size);
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ struct ttm_buffer_object *bo = res->bo;
+ uint32_t num_pages = PFN_UP(bo->base.size);
- ret = ttm_bo_swapout(bo, ctx, gfp_flags);
- /* ttm_bo_swapout has dropped the lru_lock */
- if (!ret)
- return num_pages;
- if (ret != -EBUSY)
- return ret;
- }
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped the lru_lock */
+ if (!ret)
+ return num_pages;
+ if (ret != -EBUSY)
+ return ret;
}
}
spin_unlock(&bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index c124db5039f0..e3301f6277ba 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -385,6 +385,55 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
}
EXPORT_SYMBOL(ttm_resource_manager_debug);
+/**
+ * ttm_resource_manager_first
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the first resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor)
+{
+ struct ttm_resource *res;
+
+ lockdep_assert_held(&man->bdev->lru_lock);
+
+ for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
+ ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
+/**
+ * ttm_resource_manager_next
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the next resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res)
+{
+ lockdep_assert_held(&man->bdev->lru_lock);
+
+ list_for_each_entry_continue(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ for (; cursor->priority < TTM_MAX_BO_PRIORITY; ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
static void ttm_kmap_iter_iomap_map_local(struct ttm_kmap_iter *iter,
struct dma_buf_map *dmap,
pgoff_t i)
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 9bc5c0280a09..cd3a6ed0c82c 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -184,6 +184,17 @@ struct ttm_resource {
struct list_head lru;
};
+/**
+ * struct ttm_resource_cursor
+ *
+ * @priority: the current priority
+ *
+ * Cursor to iterate over the resources in a manager.
+ */
+struct ttm_resource_cursor {
+ unsigned int priority;
+};
+
/**
* struct ttm_lru_bulk_move_pos
*
@@ -328,6 +339,18 @@ uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
void ttm_resource_manager_debug(struct ttm_resource_manager *man,
struct drm_printer *p);
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor);
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res);
+
+#define ttm_resource_manager_for_each_res(man, cursor, res) \
+ for (res = ttm_resource_manager_first(man, cursor); res; \
+ res = ttm_resource_manager_next(man, cursor, res))
+
struct ttm_kmap_iter *
ttm_kmap_iter_iomap_init(struct ttm_kmap_iter_iomap *iter_io,
struct io_mapping *iomap,
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/9] drm/radeon: remove resource accounting
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (2 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 3/9] drm/ttm: add resource iterator v2 Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 8:40 ` [PATCH 5/9] drm/amdgpu: remove GTT accounting Christian König
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
Use the one provided by TTM instead.
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/radeon/radeon.h | 2 --
drivers/gpu/drm/radeon/radeon_kms.c | 7 ++++--
drivers/gpu/drm/radeon/radeon_object.c | 30 +++-----------------------
drivers/gpu/drm/radeon/radeon_object.h | 1 -
drivers/gpu/drm/radeon/radeon_ttm.c | 18 ++--------------
5 files changed, 10 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 895776c421d4..08f83bf2c330 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2462,8 +2462,6 @@ struct radeon_device {
struct radeon_vm_manager vm_manager;
struct mutex gpu_clock_mutex;
/* memory stats */
- atomic64_t vram_usage;
- atomic64_t gtt_usage;
atomic64_t num_bytes_moved;
atomic_t gpu_reset_counter;
/* ACPI interface */
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 11ad210919c8..965161b8565b 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct drm_radeon_info *info = data;
struct radeon_mode_info *minfo = &rdev->mode_info;
uint32_t *value, value_tmp, *value_ptr, value_size;
+ struct ttm_resource_manager *man;
uint64_t value64;
struct drm_crtc *crtc;
int i, found;
@@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
case RADEON_INFO_VRAM_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->vram_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_GTT_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->gtt_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_ACTIVE_CU_COUNT:
if (rdev->family >= CHIP_BONAIRE)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 56ede9d63b12..c9bbed2a25ad 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo);
* function are calling it.
*/
-static void radeon_update_memory_usage(struct ttm_buffer_object *bo,
- unsigned int mem_type, int sign)
-{
- struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
- switch (mem_type) {
- case TTM_PL_TT:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->gtt_usage);
- else
- atomic64_sub(bo->base.size, &rdev->gtt_usage);
- break;
- case TTM_PL_VRAM:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->vram_usage);
- else
- atomic64_sub(bo->base.size, &rdev->vram_usage);
- break;
- }
-}
-
static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct radeon_bo *bo;
@@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev)
static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev)
{
u64 real_vram_size = rdev->mc.real_vram_size;
- u64 vram_usage = atomic64_read(&rdev->vram_usage);
+ struct ttm_resource_manager *man =
+ ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ u64 vram_usage = ttm_resource_manager_usage(man);
/* This function is based on the current VRAM usage.
*
@@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
}
void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem)
{
struct radeon_bo *rbo;
- radeon_update_memory_usage(bo, old_type, -1);
- if (new_mem)
- radeon_update_memory_usage(bo, new_mem->mem_type, 1);
-
if (!radeon_ttm_bo_is_radeon_bo(bo))
return;
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 1afc7992ef91..0b64e202577b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -161,7 +161,6 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
bool force_drop);
extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem);
extern vm_fault_t radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 0d1283cdc8fb..ae09a91a486a 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -199,7 +199,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
struct ttm_resource *old_mem = bo->resource;
struct radeon_device *rdev;
struct radeon_bo *rbo;
- int r, old_type;
+ int r;
if (new_mem->mem_type == TTM_PL_TT) {
r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem);
@@ -216,9 +216,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
if (WARN_ON_ONCE(rbo->tbo.pin_count > 0))
return -EINVAL;
- /* Save old type for statistics update */
- old_type = old_mem->mem_type;
-
rdev = radeon_get_rdev(bo->bdev);
if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
ttm_bo_move_null(bo, new_mem);
@@ -264,7 +261,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
out:
/* update statistics */
atomic64_add(bo->base.size, &rdev->num_bytes_moved);
- radeon_bo_move_notify(bo, old_type, new_mem);
+ radeon_bo_move_notify(bo, new_mem);
return 0;
}
@@ -679,16 +676,6 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
}
-static void
-radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
-{
- unsigned int old_type = TTM_PL_SYSTEM;
-
- if (bo->resource)
- old_type = bo->resource->mem_type;
- radeon_bo_move_notify(bo, old_type, NULL);
-}
-
static struct ttm_device_funcs radeon_bo_driver = {
.ttm_tt_create = &radeon_ttm_tt_create,
.ttm_tt_populate = &radeon_ttm_tt_populate,
@@ -697,7 +684,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &radeon_evict_flags,
.move = &radeon_bo_move,
- .delete_mem_notify = &radeon_bo_delete_mem_notify,
.io_mem_reserve = &radeon_ttm_io_mem_reserve,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/9] drm/amdgpu: remove GTT accounting
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (3 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 4/9] drm/radeon: remove resource accounting Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 21:36 ` Felix Kuehling
2022-02-09 8:40 ` [PATCH 6/9] drm/amdgpu: remove VRAM accounting Christian König
` (3 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
3 files changed, 15 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index e0c7fbe01d93..3bcd27ae379d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
struct ttm_resource_manager *man;
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
- return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
+ return sysfs_emit(buf, "%llu\n", man->size);
}
/**
@@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
+ struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
- return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
@@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
struct amdgpu_gtt_node *node;
int r;
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
- atomic64_add_return(num_pages, &mgr->used) > man->size) {
- atomic64_sub(num_pages, &mgr->used);
- return -ENOSPC;
- }
-
node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
- if (!node) {
- r = -ENOMEM;
- goto err_out;
- }
+ if (!node)
+ return -ENOMEM;
node->tbo = tbo;
ttm_resource_init(tbo, place, &node->base.base);
+ if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
+ ttm_resource_manager_usage(man) > man->size) {
+ r = -ENOSPC;
+ goto err_free;
+ }
if (place->lpfn) {
spin_lock(&mgr->lock);
@@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
err_free:
ttm_resource_fini(man, &node->base.base);
kfree(node);
-
-err_out:
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(num_pages, &mgr->used);
-
return r;
}
@@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
drm_mm_remove_node(&node->base.mm_nodes[0]);
spin_unlock(&mgr->lock);
- if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(res->num_pages, &mgr->used);
-
ttm_resource_fini(man, res);
kfree(node);
}
-/**
- * amdgpu_gtt_mgr_usage - return usage of GTT domain
- *
- * @mgr: amdgpu_gtt_mgr pointer
- *
- * Return how many bytes are used in the GTT domain
- */
-uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
-{
- return atomic64_read(&mgr->used) * PAGE_SIZE;
-}
-
/**
* amdgpu_gtt_mgr_recover - re-init gart
*
@@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
- man->size, atomic64_read(&mgr->used));
}
static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
@@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
man->use_tt = true;
man->func = &amdgpu_gtt_mgr_func;
- ttm_resource_manager_init(man, &adev->mman.bdev,
- gtt_size >> PAGE_SHIFT);
+ ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
drm_mm_init(&mgr->mm, start, size);
spin_lock_init(&mgr->lock);
- atomic64_set(&mgr->used, 0);
ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
ttm_resource_manager_set_used(man, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1ebb91db2274..9ff4aced5da7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GTT_USAGE:
- ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
+ ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GDS_CONFIG: {
struct drm_amdgpu_info_gds gds_info;
@@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
case AMDGPU_INFO_MEMORY: {
struct drm_amdgpu_memory_info mem;
struct ttm_resource_manager *gtt_man =
- ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
+ &adev->mman.gtt_mgr.manager;
+
memset(&mem, 0, sizeof(mem));
mem.vram.total_heap_size = adev->gmc.real_vram_size;
mem.vram.usable_heap_size = adev->gmc.real_vram_size -
@@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
mem.gtt.total_heap_size *= PAGE_SIZE;
mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
atomic64_read(&adev->gart_pin_size);
- mem.gtt.heap_usage =
- amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
+ mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
return copy_to_user(out, &mem,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index f8f48be16d80..120b69ec9885 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
struct ttm_resource_manager manager;
struct drm_mm mm;
spinlock_t lock;
- atomic64_t used;
};
struct amdgpu_preempt_mgr {
@@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
-uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/9] drm/amdgpu: remove GTT accounting
2022-02-09 8:40 ` [PATCH 5/9] drm/amdgpu: remove GTT accounting Christian König
@ 2022-02-09 21:36 ` Felix Kuehling
0 siblings, 0 replies; 15+ messages in thread
From: Felix Kuehling @ 2022-02-09 21:36 UTC (permalink / raw)
To: Christian König, ray.huang, daniel, thomas.hellstrom
Cc: dri-devel, amd-gfx
On 2022-02-09 03:40, Christian König wrote:
> This is provided by TTM now.
>
> Also switch man->size to bytes instead of pages and fix the double
> printing of size and usage in debugfs.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
Should amdgpu_preempt_mgr also be updated?
Regards,
Felix
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
> 3 files changed, 15 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index e0c7fbe01d93..3bcd27ae379d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
> struct ttm_resource_manager *man;
>
> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
> + return sysfs_emit(buf, "%llu\n", man->size);
> }
>
> /**
> @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
>
> - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
> @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> struct amdgpu_gtt_node *node;
> int r;
>
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> - atomic64_add_return(num_pages, &mgr->used) > man->size) {
> - atomic64_sub(num_pages, &mgr->used);
> - return -ENOSPC;
> - }
> -
> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> - if (!node) {
> - r = -ENOMEM;
> - goto err_out;
> - }
> + if (!node)
> + return -ENOMEM;
>
> node->tbo = tbo;
> ttm_resource_init(tbo, place, &node->base.base);
> + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> + ttm_resource_manager_usage(man) > man->size) {
> + r = -ENOSPC;
> + goto err_free;
> + }
>
> if (place->lpfn) {
> spin_lock(&mgr->lock);
> @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> err_free:
> ttm_resource_fini(man, &node->base.base);
> kfree(node);
> -
> -err_out:
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(num_pages, &mgr->used);
> -
> return r;
> }
>
> @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
> drm_mm_remove_node(&node->base.mm_nodes[0]);
> spin_unlock(&mgr->lock);
>
> - if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(res->num_pages, &mgr->used);
> -
> ttm_resource_fini(man, res);
> kfree(node);
> }
>
> -/**
> - * amdgpu_gtt_mgr_usage - return usage of GTT domain
> - *
> - * @mgr: amdgpu_gtt_mgr pointer
> - *
> - * Return how many bytes are used in the GTT domain
> - */
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
> -{
> - return atomic64_read(&mgr->used) * PAGE_SIZE;
> -}
> -
> /**
> * amdgpu_gtt_mgr_recover - re-init gart
> *
> @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
> spin_lock(&mgr->lock);
> drm_mm_print(&mgr->mm, printer);
> spin_unlock(&mgr->lock);
> -
> - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
> - man->size, atomic64_read(&mgr->used));
> }
>
> static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
> @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
> man->use_tt = true;
> man->func = &amdgpu_gtt_mgr_func;
>
> - ttm_resource_manager_init(man, &adev->mman.bdev,
> - gtt_size >> PAGE_SHIFT);
> + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
>
> start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
> drm_mm_init(&mgr->mm, start, size);
> spin_lock_init(&mgr->lock);
> - atomic64_set(&mgr->used, 0);
>
> ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
> ttm_resource_manager_set_used(man, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1ebb91db2274..9ff4aced5da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GTT_USAGE:
> - ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GDS_CONFIG: {
> struct drm_amdgpu_info_gds gds_info;
> @@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> case AMDGPU_INFO_MEMORY: {
> struct drm_amdgpu_memory_info mem;
> struct ttm_resource_manager *gtt_man =
> - ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> + &adev->mman.gtt_mgr.manager;
> +
> memset(&mem, 0, sizeof(mem));
> mem.vram.total_heap_size = adev->gmc.real_vram_size;
> mem.vram.usable_heap_size = adev->gmc.real_vram_size -
> @@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> mem.gtt.total_heap_size *= PAGE_SIZE;
> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
> atomic64_read(&adev->gart_pin_size);
> - mem.gtt.heap_usage =
> - amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
> mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
>
> return copy_to_user(out, &mem,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index f8f48be16d80..120b69ec9885 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
> struct ttm_resource_manager manager;
> struct drm_mm mm;
> spinlock_t lock;
> - atomic64_t used;
> };
>
> struct amdgpu_preempt_mgr {
> @@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
> void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>
> bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
> int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>
> uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/9] drm/amdgpu: remove VRAM accounting
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (4 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 5/9] drm/amdgpu: remove GTT accounting Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 9:53 ` Matthew Auld
2022-02-09 8:40 ` [PATCH 7/9] drm/amdgpu: drop amdgpu_gtt_node v2 Christian König
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 58 +++++++-------------
6 files changed, 31 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index e8440d306496..025748e9c772 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -314,7 +314,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
}
total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size);
- used_vram = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ used_vram = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
spin_lock(&adev->mm_stats.lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 9ff4aced5da7..0beab961b18b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VRAM_USAGE:
- ui64 = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ ui64 = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VIS_VRAM_USAGE:
ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
@@ -717,6 +717,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct drm_amdgpu_memory_info mem;
struct ttm_resource_manager *gtt_man =
&adev->mman.gtt_mgr.manager;
+ struct ttm_resource_manager *vram_man =
+ &adev->mman.vram_mgr.manager;
memset(&mem, 0, sizeof(mem));
mem.vram.total_heap_size = adev->gmc.real_vram_size;
@@ -724,7 +726,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
atomic64_read(&adev->vram_pin_size) -
AMDGPU_VM_RESERVED_VRAM;
mem.vram.heap_usage =
- amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ ttm_resource_manager_usage(vram_man);
mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4;
mem.cpu_accessible_vram.total_heap_size =
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d178fbec7048..5859ed0552a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1884,7 +1884,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
size = adev->gmc.real_vram_size;
else
size = adev->gmc.visible_vram_size;
- man->size = size >> PAGE_SHIFT;
+ man->size = size;
adev->mman.buffer_funcs_enabled = enable;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 120b69ec9885..cbee84a77331 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -44,7 +44,6 @@ struct amdgpu_vram_mgr {
spinlock_t lock;
struct list_head reservations_pending;
struct list_head reserved_pages;
- atomic64_t usage;
atomic64_t vis_usage;
};
@@ -127,7 +126,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
void amdgpu_vram_mgr_free_sgt(struct device *dev,
enum dma_data_direction dir,
struct sg_table *sgt);
-uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr);
uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr);
int amdgpu_vram_mgr_reserve_range(struct amdgpu_vram_mgr *mgr,
uint64_t start, uint64_t size);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 07bc0f504713..3a25dd220786 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -575,8 +575,10 @@ static int amdgpu_virt_write_vf2pf_data(struct amdgpu_device *adev)
vf2pf_info->driver_cert = 0;
vf2pf_info->os_info.all = 0;
- vf2pf_info->fb_usage = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr) >> 20;
- vf2pf_info->fb_vis_usage = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
+ vf2pf_info->fb_usage =
+ ttm_resource_manager_usage(&adev->mman.vram_mgr.manager) >> 20;
+ vf2pf_info->fb_vis_usage =
+ amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
vf2pf_info->fb_size = adev->gmc.real_vram_size >> 20;
vf2pf_info->fb_vis_size = adev->gmc.visible_vram_size >> 20;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 7442095f089c..e50fe25fbcb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -96,9 +96,9 @@ static ssize_t amdgpu_mem_info_vram_used_show(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
+ struct ttm_resource_manager *man = &adev->mman.vram_mgr.manager;
- return sysfs_emit(buf, "%llu\n",
- amdgpu_vram_mgr_usage(&adev->mman.vram_mgr));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
/**
@@ -253,7 +253,9 @@ static void amdgpu_vram_mgr_do_reserve(struct ttm_resource_manager *man)
vis_usage = amdgpu_vram_mgr_vis_size(adev, &rsv->mm_node);
atomic64_add(vis_usage, &mgr->vis_usage);
- atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &mgr->usage);
+ spin_lock(&man->bdev->lru_lock);
+ man->usage += rsv->mm_node.size << PAGE_SHIFT;
+ spin_unlock(&man->bdev->lru_lock);
list_move(&rsv->node, &mgr->reserved_pages);
}
}
@@ -378,19 +380,13 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
lpfn = place->lpfn;
if (!lpfn)
- lpfn = man->size;
+ lpfn = man->size >> PAGE_SHIFT;
max_bytes = adev->gmc.mc_vram_size;
if (tbo->type != ttm_bo_type_kernel)
max_bytes -= AMDGPU_VM_RESERVED_VRAM;
- /* bail out quickly if there's likely not enough VRAM for this BO */
mem_bytes = tbo->base.size;
- if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
- r = -ENOSPC;
- goto error_sub;
- }
-
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
@@ -408,13 +404,17 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
GFP_KERNEL | __GFP_ZERO);
- if (!node) {
- r = -ENOMEM;
- goto error_sub;
- }
+ if (!node)
+ return -ENOMEM;
ttm_resource_init(tbo, place, &node->base);
+ /* bail out quickly if there's likely not enough VRAM for this BO */
+ if (ttm_resource_manager_usage(man) > max_bytes) {
+ r = -ENOSPC;
+ goto error_fini;
+ }
+
mode = DRM_MM_INSERT_BEST;
if (place->flags & TTM_PL_FLAG_TOPDOWN)
mode = DRM_MM_INSERT_HIGH;
@@ -472,11 +472,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
while (i--)
drm_mm_remove_node(&node->mm_nodes[i]);
spin_unlock(&mgr->lock);
+error_fini:
ttm_resource_fini(man, &node->base);
kvfree(node);
-error_sub:
- atomic64_sub(mem_bytes, &mgr->usage);
return r;
}
@@ -494,7 +493,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
struct amdgpu_device *adev = to_amdgpu_device(mgr);
- uint64_t usage = 0, vis_usage = 0;
+ uint64_t vis_usage = 0;
unsigned i, pages;
spin_lock(&mgr->lock);
@@ -503,13 +502,11 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct drm_mm_node *mm = &node->mm_nodes[i];
drm_mm_remove_node(mm);
- usage += mm->size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, mm);
}
amdgpu_vram_mgr_do_reserve(man);
spin_unlock(&mgr->lock);
- atomic64_sub(usage, &mgr->usage);
atomic64_sub(vis_usage, &mgr->vis_usage);
ttm_resource_fini(man, res);
@@ -627,18 +624,6 @@ void amdgpu_vram_mgr_free_sgt(struct device *dev,
kfree(sgt);
}
-/**
- * amdgpu_vram_mgr_usage - how many bytes are used in this domain
- *
- * @mgr: amdgpu_vram_mgr pointer
- *
- * Returns how many bytes are used in this domain.
- */
-uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr)
-{
- return atomic64_read(&mgr->usage);
-}
-
/**
* amdgpu_vram_mgr_vis_usage - how many bytes are used in the visible part
*
@@ -664,13 +649,12 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
{
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
+ drm_printf(printer, " vis usage:%llu\n",
+ amdgpu_vram_mgr_vis_usage(mgr));
+
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, ram usage:%lluMB, vis usage:%lluMB\n",
- man->size, amdgpu_vram_mgr_usage(mgr) >> 20,
- amdgpu_vram_mgr_vis_usage(mgr) >> 20);
}
static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
@@ -692,11 +676,11 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
struct ttm_resource_manager *man = &mgr->manager;
ttm_resource_manager_init(man, &adev->mman.bdev,
- adev->gmc.real_vram_size >> PAGE_SHIFT);
+ adev->gmc.real_vram_size);
man->func = &amdgpu_vram_mgr_func;
- drm_mm_init(&mgr->mm, 0, man->size);
+ drm_mm_init(&mgr->mm, 0, man->size >> PAGE_SHIFT);
spin_lock_init(&mgr->lock);
INIT_LIST_HEAD(&mgr->reservations_pending);
INIT_LIST_HEAD(&mgr->reserved_pages);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/9] drm/amdgpu: remove VRAM accounting
2022-02-09 8:40 ` [PATCH 6/9] drm/amdgpu: remove VRAM accounting Christian König
@ 2022-02-09 9:53 ` Matthew Auld
2022-02-09 12:09 ` Christian König
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Auld @ 2022-02-09 9:53 UTC (permalink / raw)
To: Christian König
Cc: Thomas Hellström, ML dri-devel, ray.huang, amd-gfx list,
Daniel Vetter
On Wed, 9 Feb 2022 at 08:41, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> This is provided by TTM now.
>
> Also switch man->size to bytes instead of pages and fix the double
> printing of size and usage in debugfs.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 58 +++++++-------------
> 6 files changed, 31 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index e8440d306496..025748e9c772 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -314,7 +314,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
> }
>
> total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size);
> - used_vram = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
> + used_vram = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
> free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
>
> spin_lock(&adev->mm_stats.lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 9ff4aced5da7..0beab961b18b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_VRAM_USAGE:
> - ui64 = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
> + ui64 = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_VIS_VRAM_USAGE:
> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
> @@ -717,6 +717,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> struct drm_amdgpu_memory_info mem;
> struct ttm_resource_manager *gtt_man =
> &adev->mman.gtt_mgr.manager;
> + struct ttm_resource_manager *vram_man =
> + &adev->mman.vram_mgr.manager;
>
> memset(&mem, 0, sizeof(mem));
> mem.vram.total_heap_size = adev->gmc.real_vram_size;
> @@ -724,7 +726,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> atomic64_read(&adev->vram_pin_size) -
> AMDGPU_VM_RESERVED_VRAM;
> mem.vram.heap_usage =
> - amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
> + ttm_resource_manager_usage(vram_man);
> mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4;
>
> mem.cpu_accessible_vram.total_heap_size =
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index d178fbec7048..5859ed0552a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1884,7 +1884,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
> size = adev->gmc.real_vram_size;
> else
> size = adev->gmc.visible_vram_size;
> - man->size = size >> PAGE_SHIFT;
> + man->size = size;
> adev->mman.buffer_funcs_enabled = enable;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 120b69ec9885..cbee84a77331 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -44,7 +44,6 @@ struct amdgpu_vram_mgr {
> spinlock_t lock;
> struct list_head reservations_pending;
> struct list_head reserved_pages;
> - atomic64_t usage;
> atomic64_t vis_usage;
> };
>
> @@ -127,7 +126,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
> void amdgpu_vram_mgr_free_sgt(struct device *dev,
> enum dma_data_direction dir,
> struct sg_table *sgt);
> -uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr);
> uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr);
> int amdgpu_vram_mgr_reserve_range(struct amdgpu_vram_mgr *mgr,
> uint64_t start, uint64_t size);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 07bc0f504713..3a25dd220786 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -575,8 +575,10 @@ static int amdgpu_virt_write_vf2pf_data(struct amdgpu_device *adev)
> vf2pf_info->driver_cert = 0;
> vf2pf_info->os_info.all = 0;
>
> - vf2pf_info->fb_usage = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr) >> 20;
> - vf2pf_info->fb_vis_usage = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
> + vf2pf_info->fb_usage =
> + ttm_resource_manager_usage(&adev->mman.vram_mgr.manager) >> 20;
> + vf2pf_info->fb_vis_usage =
> + amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
> vf2pf_info->fb_size = adev->gmc.real_vram_size >> 20;
> vf2pf_info->fb_vis_size = adev->gmc.visible_vram_size >> 20;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 7442095f089c..e50fe25fbcb8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -96,9 +96,9 @@ static ssize_t amdgpu_mem_info_vram_used_show(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct ttm_resource_manager *man = &adev->mman.vram_mgr.manager;
>
> - return sysfs_emit(buf, "%llu\n",
> - amdgpu_vram_mgr_usage(&adev->mman.vram_mgr));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> /**
> @@ -253,7 +253,9 @@ static void amdgpu_vram_mgr_do_reserve(struct ttm_resource_manager *man)
>
> vis_usage = amdgpu_vram_mgr_vis_size(adev, &rsv->mm_node);
> atomic64_add(vis_usage, &mgr->vis_usage);
> - atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &mgr->usage);
> + spin_lock(&man->bdev->lru_lock);
> + man->usage += rsv->mm_node.size << PAGE_SHIFT;
> + spin_unlock(&man->bdev->lru_lock);
> list_move(&rsv->node, &mgr->reserved_pages);
> }
> }
> @@ -378,19 +380,13 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>
> lpfn = place->lpfn;
> if (!lpfn)
> - lpfn = man->size;
> + lpfn = man->size >> PAGE_SHIFT;
>
> max_bytes = adev->gmc.mc_vram_size;
> if (tbo->type != ttm_bo_type_kernel)
> max_bytes -= AMDGPU_VM_RESERVED_VRAM;
>
> - /* bail out quickly if there's likely not enough VRAM for this BO */
> mem_bytes = tbo->base.size;
> - if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
> - r = -ENOSPC;
> - goto error_sub;
> - }
> -
> if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
> pages_per_node = ~0ul;
> num_nodes = 1;
> @@ -408,13 +404,17 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>
> node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
> GFP_KERNEL | __GFP_ZERO);
> - if (!node) {
> - r = -ENOMEM;
> - goto error_sub;
> - }
> + if (!node)
> + return -ENOMEM;
>
> ttm_resource_init(tbo, place, &node->base);
>
> + /* bail out quickly if there's likely not enough VRAM for this BO */
> + if (ttm_resource_manager_usage(man) > max_bytes) {
> + r = -ENOSPC;
> + goto error_fini;
> + }
> +
> mode = DRM_MM_INSERT_BEST;
> if (place->flags & TTM_PL_FLAG_TOPDOWN)
> mode = DRM_MM_INSERT_HIGH;
> @@ -472,11 +472,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> while (i--)
> drm_mm_remove_node(&node->mm_nodes[i]);
> spin_unlock(&mgr->lock);
> +error_fini:
> ttm_resource_fini(man, &node->base);
> kvfree(node);
>
> -error_sub:
> - atomic64_sub(mem_bytes, &mgr->usage);
> return r;
> }
>
> @@ -494,7 +493,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
> struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
> struct amdgpu_device *adev = to_amdgpu_device(mgr);
> - uint64_t usage = 0, vis_usage = 0;
> + uint64_t vis_usage = 0;
> unsigned i, pages;
>
> spin_lock(&mgr->lock);
> @@ -503,13 +502,11 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
> struct drm_mm_node *mm = &node->mm_nodes[i];
>
> drm_mm_remove_node(mm);
> - usage += mm->size << PAGE_SHIFT;
> vis_usage += amdgpu_vram_mgr_vis_size(adev, mm);
> }
> amdgpu_vram_mgr_do_reserve(man);
> spin_unlock(&mgr->lock);
>
> - atomic64_sub(usage, &mgr->usage);
> atomic64_sub(vis_usage, &mgr->vis_usage);
>
> ttm_resource_fini(man, res);
> @@ -627,18 +624,6 @@ void amdgpu_vram_mgr_free_sgt(struct device *dev,
> kfree(sgt);
> }
>
> -/**
> - * amdgpu_vram_mgr_usage - how many bytes are used in this domain
> - *
> - * @mgr: amdgpu_vram_mgr pointer
> - *
> - * Returns how many bytes are used in this domain.
> - */
> -uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr)
> -{
> - return atomic64_read(&mgr->usage);
> -}
> -
> /**
> * amdgpu_vram_mgr_vis_usage - how many bytes are used in the visible part
> *
> @@ -664,13 +649,12 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
> {
> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
>
> + drm_printf(printer, " vis usage:%llu\n",
> + amdgpu_vram_mgr_vis_usage(mgr));
> +
> spin_lock(&mgr->lock);
> drm_mm_print(&mgr->mm, printer);
> spin_unlock(&mgr->lock);
> -
> - drm_printf(printer, "man size:%llu pages, ram usage:%lluMB, vis usage:%lluMB\n",
> - man->size, amdgpu_vram_mgr_usage(mgr) >> 20,
> - amdgpu_vram_mgr_vis_usage(mgr) >> 20);
> }
>
> static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
> @@ -692,11 +676,11 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
> struct ttm_resource_manager *man = &mgr->manager;
>
> ttm_resource_manager_init(man, &adev->mman.bdev,
> - adev->gmc.real_vram_size >> PAGE_SHIFT);
> + adev->gmc.real_vram_size);
The kernel-doc for ttm_resource_manager_init() says that size is in
page units, should we tweak that to say that it's driver defined or
something?
Also amdgpu_bo_validate_size() looks like it is still expecting page
units for man->size? Perhaps I'm looking at the wrong tree.
>
> man->func = &amdgpu_vram_mgr_func;
>
> - drm_mm_init(&mgr->mm, 0, man->size);
> + drm_mm_init(&mgr->mm, 0, man->size >> PAGE_SHIFT);
> spin_lock_init(&mgr->lock);
> INIT_LIST_HEAD(&mgr->reservations_pending);
> INIT_LIST_HEAD(&mgr->reserved_pages);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/9] drm/amdgpu: remove VRAM accounting
2022-02-09 9:53 ` Matthew Auld
@ 2022-02-09 12:09 ` Christian König
0 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 12:09 UTC (permalink / raw)
To: Matthew Auld
Cc: Thomas Hellström, ML dri-devel, ray.huang, amd-gfx list,
Daniel Vetter
Am 09.02.22 um 10:53 schrieb Matthew Auld:
> On Wed, 9 Feb 2022 at 08:41, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> This is provided by TTM now.
>>
>> Also switch man->size to bytes instead of pages and fix the double
>> printing of size and usage in debugfs.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 58 +++++++-------------
>> 6 files changed, 31 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index e8440d306496..025748e9c772 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -314,7 +314,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
>> }
>>
>> total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size);
>> - used_vram = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
>> + used_vram = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
>> free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
>>
>> spin_lock(&adev->mm_stats.lock);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 9ff4aced5da7..0beab961b18b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
>> case AMDGPU_INFO_VRAM_USAGE:
>> - ui64 = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
>> + ui64 = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
>> case AMDGPU_INFO_VIS_VRAM_USAGE:
>> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
>> @@ -717,6 +717,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> struct drm_amdgpu_memory_info mem;
>> struct ttm_resource_manager *gtt_man =
>> &adev->mman.gtt_mgr.manager;
>> + struct ttm_resource_manager *vram_man =
>> + &adev->mman.vram_mgr.manager;
>>
>> memset(&mem, 0, sizeof(mem));
>> mem.vram.total_heap_size = adev->gmc.real_vram_size;
>> @@ -724,7 +726,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> atomic64_read(&adev->vram_pin_size) -
>> AMDGPU_VM_RESERVED_VRAM;
>> mem.vram.heap_usage =
>> - amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
>> + ttm_resource_manager_usage(vram_man);
>> mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4;
>>
>> mem.cpu_accessible_vram.total_heap_size =
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index d178fbec7048..5859ed0552a4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1884,7 +1884,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
>> size = adev->gmc.real_vram_size;
>> else
>> size = adev->gmc.visible_vram_size;
>> - man->size = size >> PAGE_SHIFT;
>> + man->size = size;
>> adev->mman.buffer_funcs_enabled = enable;
>> }
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 120b69ec9885..cbee84a77331 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -44,7 +44,6 @@ struct amdgpu_vram_mgr {
>> spinlock_t lock;
>> struct list_head reservations_pending;
>> struct list_head reserved_pages;
>> - atomic64_t usage;
>> atomic64_t vis_usage;
>> };
>>
>> @@ -127,7 +126,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
>> void amdgpu_vram_mgr_free_sgt(struct device *dev,
>> enum dma_data_direction dir,
>> struct sg_table *sgt);
>> -uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr);
>> uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr);
>> int amdgpu_vram_mgr_reserve_range(struct amdgpu_vram_mgr *mgr,
>> uint64_t start, uint64_t size);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> index 07bc0f504713..3a25dd220786 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> @@ -575,8 +575,10 @@ static int amdgpu_virt_write_vf2pf_data(struct amdgpu_device *adev)
>> vf2pf_info->driver_cert = 0;
>> vf2pf_info->os_info.all = 0;
>>
>> - vf2pf_info->fb_usage = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr) >> 20;
>> - vf2pf_info->fb_vis_usage = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
>> + vf2pf_info->fb_usage =
>> + ttm_resource_manager_usage(&adev->mman.vram_mgr.manager) >> 20;
>> + vf2pf_info->fb_vis_usage =
>> + amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
>> vf2pf_info->fb_size = adev->gmc.real_vram_size >> 20;
>> vf2pf_info->fb_vis_size = adev->gmc.visible_vram_size >> 20;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index 7442095f089c..e50fe25fbcb8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -96,9 +96,9 @@ static ssize_t amdgpu_mem_info_vram_used_show(struct device *dev,
>> {
>> struct drm_device *ddev = dev_get_drvdata(dev);
>> struct amdgpu_device *adev = drm_to_adev(ddev);
>> + struct ttm_resource_manager *man = &adev->mman.vram_mgr.manager;
>>
>> - return sysfs_emit(buf, "%llu\n",
>> - amdgpu_vram_mgr_usage(&adev->mman.vram_mgr));
>> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
>> }
>>
>> /**
>> @@ -253,7 +253,9 @@ static void amdgpu_vram_mgr_do_reserve(struct ttm_resource_manager *man)
>>
>> vis_usage = amdgpu_vram_mgr_vis_size(adev, &rsv->mm_node);
>> atomic64_add(vis_usage, &mgr->vis_usage);
>> - atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &mgr->usage);
>> + spin_lock(&man->bdev->lru_lock);
>> + man->usage += rsv->mm_node.size << PAGE_SHIFT;
>> + spin_unlock(&man->bdev->lru_lock);
>> list_move(&rsv->node, &mgr->reserved_pages);
>> }
>> }
>> @@ -378,19 +380,13 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>>
>> lpfn = place->lpfn;
>> if (!lpfn)
>> - lpfn = man->size;
>> + lpfn = man->size >> PAGE_SHIFT;
>>
>> max_bytes = adev->gmc.mc_vram_size;
>> if (tbo->type != ttm_bo_type_kernel)
>> max_bytes -= AMDGPU_VM_RESERVED_VRAM;
>>
>> - /* bail out quickly if there's likely not enough VRAM for this BO */
>> mem_bytes = tbo->base.size;
>> - if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
>> - r = -ENOSPC;
>> - goto error_sub;
>> - }
>> -
>> if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>> pages_per_node = ~0ul;
>> num_nodes = 1;
>> @@ -408,13 +404,17 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>>
>> node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
>> GFP_KERNEL | __GFP_ZERO);
>> - if (!node) {
>> - r = -ENOMEM;
>> - goto error_sub;
>> - }
>> + if (!node)
>> + return -ENOMEM;
>>
>> ttm_resource_init(tbo, place, &node->base);
>>
>> + /* bail out quickly if there's likely not enough VRAM for this BO */
>> + if (ttm_resource_manager_usage(man) > max_bytes) {
>> + r = -ENOSPC;
>> + goto error_fini;
>> + }
>> +
>> mode = DRM_MM_INSERT_BEST;
>> if (place->flags & TTM_PL_FLAG_TOPDOWN)
>> mode = DRM_MM_INSERT_HIGH;
>> @@ -472,11 +472,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>> while (i--)
>> drm_mm_remove_node(&node->mm_nodes[i]);
>> spin_unlock(&mgr->lock);
>> +error_fini:
>> ttm_resource_fini(man, &node->base);
>> kvfree(node);
>>
>> -error_sub:
>> - atomic64_sub(mem_bytes, &mgr->usage);
>> return r;
>> }
>>
>> @@ -494,7 +493,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
>> struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
>> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
>> struct amdgpu_device *adev = to_amdgpu_device(mgr);
>> - uint64_t usage = 0, vis_usage = 0;
>> + uint64_t vis_usage = 0;
>> unsigned i, pages;
>>
>> spin_lock(&mgr->lock);
>> @@ -503,13 +502,11 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
>> struct drm_mm_node *mm = &node->mm_nodes[i];
>>
>> drm_mm_remove_node(mm);
>> - usage += mm->size << PAGE_SHIFT;
>> vis_usage += amdgpu_vram_mgr_vis_size(adev, mm);
>> }
>> amdgpu_vram_mgr_do_reserve(man);
>> spin_unlock(&mgr->lock);
>>
>> - atomic64_sub(usage, &mgr->usage);
>> atomic64_sub(vis_usage, &mgr->vis_usage);
>>
>> ttm_resource_fini(man, res);
>> @@ -627,18 +624,6 @@ void amdgpu_vram_mgr_free_sgt(struct device *dev,
>> kfree(sgt);
>> }
>>
>> -/**
>> - * amdgpu_vram_mgr_usage - how many bytes are used in this domain
>> - *
>> - * @mgr: amdgpu_vram_mgr pointer
>> - *
>> - * Returns how many bytes are used in this domain.
>> - */
>> -uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr)
>> -{
>> - return atomic64_read(&mgr->usage);
>> -}
>> -
>> /**
>> * amdgpu_vram_mgr_vis_usage - how many bytes are used in the visible part
>> *
>> @@ -664,13 +649,12 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
>> {
>> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
>>
>> + drm_printf(printer, " vis usage:%llu\n",
>> + amdgpu_vram_mgr_vis_usage(mgr));
>> +
>> spin_lock(&mgr->lock);
>> drm_mm_print(&mgr->mm, printer);
>> spin_unlock(&mgr->lock);
>> -
>> - drm_printf(printer, "man size:%llu pages, ram usage:%lluMB, vis usage:%lluMB\n",
>> - man->size, amdgpu_vram_mgr_usage(mgr) >> 20,
>> - amdgpu_vram_mgr_vis_usage(mgr) >> 20);
>> }
>>
>> static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
>> @@ -692,11 +676,11 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
>> struct ttm_resource_manager *man = &mgr->manager;
>>
>> ttm_resource_manager_init(man, &adev->mman.bdev,
>> - adev->gmc.real_vram_size >> PAGE_SHIFT);
>> + adev->gmc.real_vram_size);
> The kernel-doc for ttm_resource_manager_init() says that size is in
> page units, should we tweak that to say that it's driver defined or
> something?
Good point.
> Also amdgpu_bo_validate_size() looks like it is still expecting page
> units for man->size? Perhaps I'm looking at the wrong tree.
Looks like I missed that one. Thanks for the review, going to fix that
one as well.
Regards,
Christian.
>
>> man->func = &amdgpu_vram_mgr_func;
>>
>> - drm_mm_init(&mgr->mm, 0, man->size);
>> + drm_mm_init(&mgr->mm, 0, man->size >> PAGE_SHIFT);
>> spin_lock_init(&mgr->lock);
>> INIT_LIST_HEAD(&mgr->reservations_pending);
>> INIT_LIST_HEAD(&mgr->reserved_pages);
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/9] drm/amdgpu: drop amdgpu_gtt_node v2
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (5 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 6/9] drm/amdgpu: remove VRAM accounting Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 8:40 ` [PATCH 8/9] drm/ttm: allow bulk moves for all domains Christian König
2022-02-09 8:40 ` [PATCH 9/9] drm/ttm: rework bulk move handling v2 Christian König
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
We have the BO pointer in the base structure now as well.
v2: add lockdep and kerneldoc
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 ++++++++-------------
include/drm/ttm/ttm_resource.h | 8 ++++
2 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 3bcd27ae379d..68494b959116 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -26,23 +26,12 @@
#include "amdgpu.h"
-struct amdgpu_gtt_node {
- struct ttm_buffer_object *tbo;
- struct ttm_range_mgr_node base;
-};
-
static inline struct amdgpu_gtt_mgr *
to_gtt_mgr(struct ttm_resource_manager *man)
{
return container_of(man, struct amdgpu_gtt_mgr, manager);
}
-static inline struct amdgpu_gtt_node *
-to_amdgpu_gtt_node(struct ttm_resource *res)
-{
- return container_of(res, struct amdgpu_gtt_node, base.base);
-}
-
/**
* DOC: mem_info_gtt_total
*
@@ -106,9 +95,9 @@ const struct attribute_group amdgpu_gtt_mgr_attr_group = {
*/
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
- return drm_mm_node_allocated(&node->base.mm_nodes[0]);
+ return drm_mm_node_allocated(&node->mm_nodes[0]);
}
/**
@@ -128,15 +117,14 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
uint32_t num_pages = PFN_UP(tbo->base.size);
- struct amdgpu_gtt_node *node;
+ struct ttm_range_mgr_node *node;
int r;
- node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
+ node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL);
if (!node)
return -ENOMEM;
- node->tbo = tbo;
- ttm_resource_init(tbo, place, &node->base.base);
+ ttm_resource_init(tbo, place, &node->base);
if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
ttm_resource_manager_usage(man) > man->size) {
r = -ENOSPC;
@@ -145,8 +133,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (place->lpfn) {
spin_lock(&mgr->lock);
- r = drm_mm_insert_node_in_range(&mgr->mm,
- &node->base.mm_nodes[0],
+ r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0],
num_pages, tbo->page_alignment,
0, place->fpfn, place->lpfn,
DRM_MM_INSERT_BEST);
@@ -154,18 +141,18 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (unlikely(r))
goto err_free;
- node->base.base.start = node->base.mm_nodes[0].start;
+ node->base.start = node->mm_nodes[0].start;
} else {
- node->base.mm_nodes[0].start = 0;
- node->base.mm_nodes[0].size = node->base.base.num_pages;
- node->base.base.start = AMDGPU_BO_INVALID_OFFSET;
+ node->mm_nodes[0].start = 0;
+ node->mm_nodes[0].size = node->base.num_pages;
+ node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
- *res = &node->base.base;
+ *res = &node->base;
return 0;
err_free:
- ttm_resource_fini(man, &node->base.base);
+ ttm_resource_fini(man, &node->base);
kfree(node);
return r;
}
@@ -181,12 +168,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
spin_lock(&mgr->lock);
- if (drm_mm_node_allocated(&node->base.mm_nodes[0]))
- drm_mm_remove_node(&node->base.mm_nodes[0]);
+ if (drm_mm_node_allocated(&node->mm_nodes[0]))
+ drm_mm_remove_node(&node->mm_nodes[0]);
spin_unlock(&mgr->lock);
ttm_resource_fini(man, res);
@@ -202,7 +189,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
*/
int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
{
- struct amdgpu_gtt_node *node;
+ struct ttm_range_mgr_node *node;
struct drm_mm_node *mm_node;
struct amdgpu_device *adev;
int r = 0;
@@ -210,8 +197,8 @@ int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
spin_lock(&mgr->lock);
drm_mm_for_each_node(mm_node, &mgr->mm) {
- node = container_of(mm_node, typeof(*node), base.mm_nodes[0]);
- r = amdgpu_ttm_recover_gart(node->tbo);
+ node = container_of(mm_node, typeof(*node), mm_nodes[0]);
+ r = amdgpu_ttm_recover_gart(node->base.bo);
if (r)
break;
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index cd3a6ed0c82c..cc452a7aa016 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -347,6 +347,14 @@ ttm_resource_manager_next(struct ttm_resource_manager *man,
struct ttm_resource_cursor *cursor,
struct ttm_resource *res);
+/**
+ * ttm_resource_manager_for_each_res - iterate over all resources
+ * @man: the resource manager
+ * @cursor: struct ttm_resource_cursor for the current position
+ * @res: the current resource
+ *
+ * Iterate over all the evictable resources in a resource manager.
+ */
#define ttm_resource_manager_for_each_res(man, cursor, res) \
for (res = ttm_resource_manager_first(man, cursor); res; \
res = ttm_resource_manager_next(man, cursor, res))
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/9] drm/ttm: allow bulk moves for all domains
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (6 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 7/9] drm/amdgpu: drop amdgpu_gtt_node v2 Christian König
@ 2022-02-09 8:40 ` Christian König
2022-02-09 8:40 ` [PATCH 9/9] drm/ttm: rework bulk move handling v2 Christian König
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
Not just TT and VRAM.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/ttm/ttm_resource.c | 52 +++++++++---------------------
include/drm/ttm/ttm_device.h | 2 --
include/drm/ttm/ttm_resource.h | 4 +--
3 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index e3301f6277ba..5e732a509b4b 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -51,38 +51,24 @@ EXPORT_SYMBOL(ttm_lru_bulk_move_init);
*/
void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
{
- unsigned i;
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
- struct ttm_resource_manager *man;
+ unsigned i, j;
- if (!pos->first)
- continue;
+ for (i = 0; i < TTM_NUM_MEM_TYPES; ++i) {
+ for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->pos[i][j];
+ struct ttm_resource_manager *man;
- lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
- dma_resv_assert_held(pos->first->bo->base.resv);
- dma_resv_assert_held(pos->last->bo->base.resv);
+ if (!pos->first)
+ continue;
- man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
- struct ttm_resource_manager *man;
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
- if (!pos->first)
- continue;
-
- lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
- dma_resv_assert_held(pos->first->bo->base.resv);
- dma_resv_assert_held(pos->last->bo->base.resv);
-
- man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
+ man = ttm_manager_type(pos->first->bo->bdev, i);
+ list_bulk_move_tail(&man->lru[j], &pos->first->lru,
+ &pos->last->lru);
+ }
}
}
EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
@@ -122,15 +108,7 @@ void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
if (!bulk)
return;
- switch (res->mem_type) {
- case TTM_PL_TT:
- ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
- break;
-
- case TTM_PL_VRAM:
- ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
- break;
- }
+ ttm_lru_bulk_move_set_pos(&bulk->pos[res->mem_type][bo->priority], res);
}
/**
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 0a4ddec78d8f..425150f35fbe 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -30,8 +30,6 @@
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_pool.h>
-#define TTM_NUM_MEM_TYPES 8
-
struct ttm_device;
struct ttm_placement;
struct ttm_buffer_object;
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index cc452a7aa016..5f93a16acfd6 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -37,6 +37,7 @@
#include <drm/ttm/ttm_kmap_iter.h>
#define TTM_MAX_BO_PRIORITY 4U
+#define TTM_NUM_MEM_TYPES 8
struct ttm_device;
struct ttm_resource_manager;
@@ -217,8 +218,7 @@ struct ttm_lru_bulk_move_pos {
* Helper structure for bulk moves on the LRU list.
*/
struct ttm_lru_bulk_move {
- struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
- struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
+ struct ttm_lru_bulk_move_pos pos[TTM_NUM_MEM_TYPES][TTM_MAX_BO_PRIORITY];
};
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 9/9] drm/ttm: rework bulk move handling v2
2022-02-09 8:40 drm/ttm: moving the LRU into the resource Christian König
` (7 preceding siblings ...)
2022-02-09 8:40 ` [PATCH 8/9] drm/ttm: allow bulk moves for all domains Christian König
@ 2022-02-09 8:40 ` Christian König
8 siblings, 0 replies; 15+ messages in thread
From: Christian König @ 2022-02-09 8:40 UTC (permalink / raw)
To: ray.huang, daniel, thomas.hellstrom; +Cc: amd-gfx, dri-devel
Instead of providing the bulk move structure for each LRU update set
this as property of the BO. This should avoid costly bulk move rebuilds
with some games under RADV.
v2: some name polishing, add a few more kerneldoc words.
v3: add some lockdep
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 72 +++-----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 47 +++++++++++--
drivers/gpu/drm/ttm/ttm_resource.c | 87 ++++++++++++++++++-------
include/drm/ttm/ttm_bo_api.h | 16 ++---
include/drm/ttm/ttm_bo_driver.h | 2 +-
include/drm/ttm/ttm_device.h | 9 ---
include/drm/ttm/ttm_resource.h | 9 ++-
10 files changed, 128 insertions(+), 120 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 5859ed0552a4..57ac118fc266 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1498,7 +1498,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
.access_memory = &amdgpu_ttm_access_memory,
- .del_from_lru_notify = &amdgpu_vm_del_from_lru_notify
};
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f2ce5a0defd9..28f5e8b21a99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -375,7 +375,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
return;
- vm->bulk_moveable = false;
+ ttm_bo_set_bulk_move(&bo->tbo, &vm->lru_bulk_move);
if (bo->tbo.type == ttm_bo_type_kernel && bo->parent)
amdgpu_vm_bo_relocated(base);
else
@@ -637,36 +637,6 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
list_add(&entry->tv.head, validated);
}
-/**
- * amdgpu_vm_del_from_lru_notify - update bulk_moveable flag
- *
- * @bo: BO which was removed from the LRU
- *
- * Make sure the bulk_moveable flag is updated when a BO is removed from the
- * LRU.
- */
-void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
-{
- struct amdgpu_bo *abo;
- struct amdgpu_vm_bo_base *bo_base;
-
- if (!amdgpu_bo_is_amdgpu_bo(bo))
- return;
-
- if (bo->pin_count)
- return;
-
- abo = ttm_to_amdgpu_bo(bo);
- if (!abo->parent)
- return;
- for (bo_base = abo->vm_bo; bo_base; bo_base = bo_base->next) {
- struct amdgpu_vm *vm = bo_base->vm;
-
- if (abo->tbo.base.resv == vm->root.bo->tbo.base.resv)
- vm->bulk_moveable = false;
- }
-
-}
/**
* amdgpu_vm_move_to_lru_tail - move all BOs to the end of LRU
*
@@ -679,33 +649,9 @@ void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
{
- struct amdgpu_vm_bo_base *bo_base;
-
- if (vm->bulk_moveable) {
- spin_lock(&adev->mman.bdev.lru_lock);
- ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
- spin_unlock(&adev->mman.bdev.lru_lock);
- return;
- }
-
- ttm_lru_bulk_move_init(&vm->lru_bulk_move);
-
spin_lock(&adev->mman.bdev.lru_lock);
- list_for_each_entry(bo_base, &vm->idle, vm_status) {
- struct amdgpu_bo *bo = bo_base->bo;
- struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
-
- if (!bo->parent)
- continue;
-
- ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
- if (shadow)
- ttm_bo_move_to_lru_tail(&shadow->tbo,
- &vm->lru_bulk_move);
- }
+ ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
spin_unlock(&adev->mman.bdev.lru_lock);
-
- vm->bulk_moveable = true;
}
/**
@@ -728,8 +674,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_vm_bo_base *bo_base, *tmp;
int r;
- vm->bulk_moveable &= list_empty(&vm->evicted);
-
list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
struct amdgpu_bo *bo = bo_base->bo;
struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
@@ -1047,10 +991,16 @@ static void amdgpu_vm_free_table(struct amdgpu_vm_bo_base *entry)
if (!entry->bo)
return;
+
shadow = amdgpu_bo_shadowed(entry->bo);
+ if (shadow) {
+ ttm_bo_set_bulk_move(&shadow->tbo, NULL);
+ amdgpu_bo_unref(&shadow);
+ }
+
+ ttm_bo_set_bulk_move(&entry->bo->tbo, NULL);
entry->bo->vm_bo = NULL;
list_del(&entry->vm_status);
- amdgpu_bo_unref(&shadow);
amdgpu_bo_unref(&entry->bo);
}
@@ -1070,8 +1020,6 @@ static void amdgpu_vm_free_pts(struct amdgpu_device *adev,
struct amdgpu_vm_pt_cursor cursor;
struct amdgpu_vm_bo_base *entry;
- vm->bulk_moveable = false;
-
for_each_amdgpu_vm_pt_dfs_safe(adev, vm, start, cursor, entry)
amdgpu_vm_free_table(entry);
@@ -2651,7 +2599,7 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
if (bo) {
if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
- vm->bulk_moveable = false;
+ ttm_bo_set_bulk_move(&bo->tbo, NULL);
for (base = &bo_va->base.bo->vm_bo; *base;
base = &(*base)->next) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 85fcfb8c5efd..4d236682a118 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -317,8 +317,6 @@ struct amdgpu_vm {
/* Store positions of group of BOs */
struct ttm_lru_bulk_move lru_bulk_move;
- /* mark whether can do the bulk move */
- bool bulk_moveable;
/* Flag to indicate if VM is used for compute */
bool is_compute_context;
};
@@ -454,7 +452,6 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm);
-void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo);
void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 582e8dc9bc8c..6fc192082d8c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -849,7 +849,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
bo->priority = I915_TTM_PRIO_NO_PAGES;
}
- ttm_bo_move_to_lru_tail(bo, NULL);
+ ttm_bo_move_to_lru_tail(bo);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 599be3dda8a9..d445a6c534d7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,16 +69,53 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
}
}
-void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_lru_bulk_move *bulk)
+/**
+ * ttm_bo_move_to_lru_tail
+ *
+ * @bo: The buffer object.
+ *
+ * Move this BO to the tail of all lru lists used to lookup and reserve an
+ * object. This function must be called with struct ttm_global::lru_lock
+ * held, and is used to make a BO less likely to be considered for eviction.
+ */
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
if (bo->resource)
- ttm_resource_move_to_lru_tail(bo->resource, bulk);
+ ttm_resource_move_to_lru_tail(bo->resource);
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
+/**
+ * ttm_bo_set_bulk_move - update BOs bulk move object
+ *
+ * @bo: The buffer object.
+ *
+ * Update the BOs bulk move object, making sure that resources are added/removed
+ * as well. A bulk move allows to move many resource on the LRU at once,
+ * resulting in much less overhead of maintaining the LRU.
+ * The only requirement is that the resources stay together on the LRU and are
+ * never separated. This is enforces by setting the bulk_move structure on a BO.
+ */
+void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
+ struct ttm_lru_bulk_move *bulk)
+{
+ dma_resv_assert_held(bo->base.resv);
+
+ if (bo->bulk_move == bulk)
+ return;
+
+ spin_lock(&bo->bdev->lru_lock);
+ if (bo->bulk_move && bo->resource)
+ ttm_lru_bulk_move_del(bo->bulk_move, bo->resource);
+ bo->bulk_move = bulk;
+ if (bo->bulk_move && bo->resource)
+ ttm_lru_bulk_move_add(bo->bulk_move, bo->resource);
+ spin_unlock(&bo->bdev->lru_lock);
+}
+EXPORT_SYMBOL(ttm_bo_set_bulk_move);
+
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
@@ -316,6 +353,7 @@ static void ttm_bo_release(struct kref *kref)
int ret;
WARN_ON_ONCE(bo->pin_count);
+ WARN_ON_ONCE(bo->bulk_move);
if (!bo->deleted) {
ret = ttm_bo_individualize_resv(bo);
@@ -352,7 +390,7 @@ static void ttm_bo_release(struct kref *kref)
*/
if (bo->pin_count) {
bo->pin_count = 0;
- ttm_resource_move_to_lru_tail(bo->resource, NULL);
+ ttm_resource_move_to_lru_tail(bo->resource);
}
kref_init(&bo->kref);
@@ -874,6 +912,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
bo->moving = NULL;
bo->pin_count = 0;
bo->sg = sg;
+ bo->bulk_move = NULL;
if (resv) {
bo->base.resv = resv;
dma_resv_assert_held(bo->base.resv);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 5e732a509b4b..13771f177fc1 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -73,42 +73,77 @@ void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
}
EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
-/* Record a resource position in a bulk move structure */
-static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
- struct ttm_resource *res)
+/* Return the bulk move pos object for this resource */
+static struct ttm_lru_bulk_move_pos *
+ttm_lru_bulk_move_pos(struct ttm_lru_bulk_move *bulk, struct ttm_resource *res)
{
- if (!pos->first)
+ return &bulk->pos[res->mem_type][res->bo->priority];
+}
+
+/* Move the resource to the tail of the bulk move range */
+static void ttm_lru_bulk_move_pos_tail(struct ttm_lru_bulk_move_pos *pos,
+ struct ttm_resource *res)
+{
+ if (pos->last != res) {
+ list_move(&res->lru, &pos->last->lru);
+ pos->last = res;
+ }
+}
+
+/* Add the resource to a bulk_move cursor */
+void ttm_lru_bulk_move_add(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res)
+{
+ struct ttm_lru_bulk_move_pos *pos = ttm_lru_bulk_move_pos(bulk, res);
+
+ if (!pos->first) {
pos->first = res;
- pos->last = res;
+ pos->last = res;
+ } else {
+ ttm_lru_bulk_move_pos_tail(pos, res);
+ }
+}
+
+/* Remove the resource from a bulk_move range */
+void ttm_lru_bulk_move_del(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res)
+{
+ struct ttm_lru_bulk_move_pos *pos = ttm_lru_bulk_move_pos(bulk, res);
+
+ if (unlikely(pos->first == res && pos->last == res)) {
+ pos->first = NULL;
+ pos->last = NULL;
+ } else if (pos->first == res) {
+ pos->first = list_next_entry(res, lru);
+ } else if (pos->last == res) {
+ pos->last = list_prev_entry(res, lru);
+ } else {
+ list_move(&res->lru, &pos->last->lru);
+ }
}
-/* Move a resource to the LRU tail and track the bulk position */
-void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
- struct ttm_lru_bulk_move *bulk)
+/* Move a resource to the LRU or bulk tail */
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res)
{
struct ttm_buffer_object *bo = res->bo;
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
lockdep_assert_held(&bo->bdev->lru_lock);
if (bo->pin_count) {
list_move_tail(&res->lru, &bdev->pinned);
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(res->bo);
- return;
- }
- man = ttm_manager_type(bdev, res->mem_type);
- list_move_tail(&res->lru, &man->lru[bo->priority]);
+ } else if (bo->bulk_move) {
+ struct ttm_lru_bulk_move_pos *pos =
+ ttm_lru_bulk_move_pos(bo->bulk_move, res);
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-
- if (!bulk)
- return;
+ ttm_lru_bulk_move_pos_tail(pos, res);
+ } else {
+ struct ttm_resource_manager *man;
- ttm_lru_bulk_move_set_pos(&bulk->pos[res->mem_type][bo->priority], res);
+ man = ttm_manager_type(bdev, res->mem_type);
+ list_move_tail(&res->lru, &man->lru[bo->priority]);
+ }
}
/**
@@ -139,7 +174,7 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
man = ttm_manager_type(bo->bdev, place->mem_type);
spin_lock(&bo->bdev->lru_lock);
man->usage += bo->base.size;
- ttm_resource_move_to_lru_tail(res, NULL);
+ ttm_resource_move_to_lru_tail(res);
spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -161,8 +196,6 @@ void ttm_resource_fini(struct ttm_resource_manager *man,
spin_lock(&bdev->lru_lock);
list_del_init(&res->lru);
- if (res->bo && bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(res->bo);
man->usage -= res->num_pages << PAGE_SHIFT;
spin_unlock(&bdev->lru_lock);
}
@@ -185,6 +218,12 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
if (!*res)
return;
+ if (bo->bulk_move) {
+ spin_lock(&bo->bdev->lru_lock);
+ ttm_lru_bulk_move_del(bo->bulk_move, *res);
+ spin_unlock(&bo->bdev->lru_lock);
+ }
+
man = ttm_manager_type(bo->bdev, (*res)->mem_type);
man->func->free(man, *res);
*res = NULL;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 3da77fc54552..a6d335895685 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -135,6 +135,7 @@ struct ttm_buffer_object {
struct ttm_resource *resource;
struct ttm_tt *ttm;
bool deleted;
+ struct ttm_lru_bulk_move *bulk_move;
/**
* Members protected by the bdev::lru_lock.
@@ -287,18 +288,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
*/
void ttm_bo_put(struct ttm_buffer_object *bo);
-/**
- * ttm_bo_move_to_lru_tail
- *
- * @bo: The buffer object.
- * @bulk: optional bulk move structure to remember BO positions
- *
- * Move this BO to the tail of all lru lists used to lookup and reserve an
- * object. This function must be called with struct ttm_global::lru_lock
- * held, and is used to make a BO less likely to be considered for eviction.
- */
-void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_lru_bulk_move *bulk);
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
+void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
+ struct ttm_lru_bulk_move *bulk);
/**
* ttm_bo_lock_delayed_workqueue
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 6c7352e13708..059a595e14e5 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -155,7 +155,7 @@ static inline void
ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
{
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, NULL);
+ ttm_bo_move_to_lru_tail(bo);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 425150f35fbe..95b3c04b1ab9 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -198,15 +198,6 @@ struct ttm_device_funcs {
int (*access_memory)(struct ttm_buffer_object *bo, unsigned long offset,
void *buf, int len, int write);
- /**
- * struct ttm_bo_driver member del_from_lru_notify
- *
- * @bo: the buffer object deleted from lru
- *
- * notify driver that a BO was deleted from LRU.
- */
- void (*del_from_lru_notify)(struct ttm_buffer_object *bo);
-
/**
* Notify the driver that we're about to release a BO
*
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 5f93a16acfd6..b45b3ddc558d 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -202,7 +202,7 @@ struct ttm_resource_cursor {
* @first: first res in the bulk move range
* @last: last res in the bulk move range
*
- * Positions for a lru bulk move.
+ * Range of resources for a lru bulk move.
*/
struct ttm_lru_bulk_move_pos {
struct ttm_resource *first;
@@ -308,10 +308,13 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
}
void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
+void ttm_lru_bulk_move_add(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res);
+void ttm_lru_bulk_move_del(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res);
void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
-void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
- struct ttm_lru_bulk_move *bulk);
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res);
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread