Skip to content

Commit

Permalink
drm/ttm: rework bulk move handling v5
Browse files Browse the repository at this point in the history
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
v4: fix bugs, handle pin/unpin as well
v5: improve 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>
Link: https://patchwork.freedesktop.org/patch/msgid/20220321132601.2161-5-christian.koenig@amd.com
  • Loading branch information
Christian König committed Mar 29, 2022
1 parent 7842cf6 commit fee2ede
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 126 deletions.
1 change: 0 additions & 1 deletion drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

/*
Expand Down
72 changes: 10 additions & 62 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*
Expand All @@ -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;
}

/**
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_ttm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
61 changes: 52 additions & 9 deletions drivers/gpu/drm/ttm/ttm_bo.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,55 @@ 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.
* ttm_lru_bulk_move_tail() should be used to move all resources to the tail of
* their LRU list.
*/
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,
Expand Down Expand Up @@ -316,6 +355,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);
Expand Down Expand Up @@ -352,7 +392,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);
Expand Down Expand Up @@ -644,7 +684,8 @@ void ttm_bo_pin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
WARN_ON_ONCE(!kref_read(&bo->kref));
++bo->pin_count;
if (!(bo->pin_count++) && bo->bulk_move && bo->resource)
ttm_lru_bulk_move_del(bo->bulk_move, bo->resource);
}
EXPORT_SYMBOL(ttm_bo_pin);

Expand All @@ -658,10 +699,11 @@ void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
WARN_ON_ONCE(!kref_read(&bo->kref));
if (bo->pin_count)
--bo->pin_count;
else
WARN_ON_ONCE(true);
if (WARN_ON_ONCE(!bo->pin_count))
return;

if (!(--bo->pin_count) && bo->bulk_move && bo->resource)
ttm_lru_bulk_move_add(bo->bulk_move, bo->resource);
}
EXPORT_SYMBOL(ttm_bo_unpin);

Expand Down Expand Up @@ -906,6 +948,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);
Expand Down
Loading

0 comments on commit fee2ede

Please sign in to comment.