[PATCH 3/5] drm/amdgpu: stop adding VM updates fences to the resv obj
Felix Kuehling
felix.kuehling at amd.com
Thu Dec 5 16:43:18 UTC 2019
On 2019-12-05 8:39 a.m., Christian König wrote:
> Don't add the VM update fences to the resv object and remove
> the handling to stop implicitely syncing to them.
>
> Ongoing updates prevent page tables from being evicted and we manually
> block for all updates to complete before releasing PDs and PTS.
>
> This way we can do updates even without the resv obj locked.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 6 ++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 30 ++++++++++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 11 +++++---
> 4 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index f1e5fbef54d8..ae8bc766215c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -243,10 +243,8 @@ int amdgpu_sync_resv(struct amdgpu_device *adev,
> /* VM updates are only interesting
> * for other VM updates and moves.
Thanks for updating the commit description. I think this comment should
also be updated because "other VM updates" fences are no longer in the
resv. Something like this: VM updates only sync with moves but not with
user command submissions or KFD evictions fences. With that fixed, this
patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
> */
> - if ((owner != AMDGPU_FENCE_OWNER_UNDEFINED) &&
> - (fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED) &&
> - ((owner == AMDGPU_FENCE_OWNER_VM) !=
> - (fence_owner == AMDGPU_FENCE_OWNER_VM)))
> + if (owner == AMDGPU_FENCE_OWNER_VM &&
> + fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED)
> continue;
>
> /* Ignore fence from the same owner and explicit one as
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a22bd57129d1..0d700e8154c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -562,8 +562,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
> {
> entry->priority = 0;
> entry->tv.bo = &vm->root.base.bo->tbo;
> - /* One for the VM updates, one for TTM and one for the CS job */
> - entry->tv.num_shared = 3;
> + /* One for TTM and one for the CS job */
> + entry->tv.num_shared = 2;
> entry->user_pages = NULL;
> list_add(&entry->tv.head, validated);
> }
> @@ -2522,6 +2522,11 @@ bool amdgpu_vm_evictable(struct amdgpu_bo *bo)
> if (!dma_resv_test_signaled_rcu(bo->tbo.base.resv, true))
> return false;
>
> + /* Don't evict VM page tables while they are updated */
> + if (!dma_fence_is_signaled(bo_base->vm->last_direct) ||
> + !dma_fence_is_signaled(bo_base->vm->last_delayed))
> + return false;
> +
> return true;
> }
>
> @@ -2687,8 +2692,16 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
> */
> long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
> {
> - return dma_resv_wait_timeout_rcu(vm->root.base.bo->tbo.base.resv,
> - true, true, timeout);
> + timeout = dma_resv_wait_timeout_rcu(vm->root.base.bo->tbo.base.resv,
> + true, true, timeout);
> + if (timeout <= 0)
> + return timeout;
> +
> + timeout = dma_fence_wait_timeout(vm->last_direct, true, timeout);
> + if (timeout <= 0)
> + return timeout;
> +
> + return dma_fence_wait_timeout(vm->last_delayed, true, timeout);
> }
>
> /**
> @@ -2757,6 +2770,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> else
> vm->update_funcs = &amdgpu_vm_sdma_funcs;
> vm->last_update = NULL;
> + vm->last_direct = dma_fence_get_stub();
> + vm->last_delayed = dma_fence_get_stub();
>
> amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
> if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
> @@ -2807,6 +2822,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> vm->root.base.bo = NULL;
>
> error_free_delayed:
> + dma_fence_put(vm->last_direct);
> + dma_fence_put(vm->last_delayed);
> drm_sched_entity_destroy(&vm->delayed);
>
> error_free_direct:
> @@ -3007,6 +3024,11 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> vm->pasid = 0;
> }
>
> + dma_fence_wait(vm->last_direct, false);
> + dma_fence_put(vm->last_direct);
> + dma_fence_wait(vm->last_delayed, false);
> + dma_fence_put(vm->last_delayed);
> +
> list_for_each_entry_safe(mapping, tmp, &vm->freed, list) {
> if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
> amdgpu_vm_prt_fini(adev, vm);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index db561765453b..d93ea9ad879e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -269,6 +269,10 @@ struct amdgpu_vm {
> struct drm_sched_entity direct;
> struct drm_sched_entity delayed;
>
> + /* Last submission to the scheduler entities */
> + struct dma_fence *last_direct;
> + struct dma_fence *last_delayed;
> +
> unsigned int pasid;
> /* dedicated to vm */
> struct amdgpu_vmid *reserved_vmid[AMDGPU_MAX_VMHUBS];
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index 50f487666977..19b7f80758f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -95,11 +95,10 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
> static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
> struct dma_fence **fence)
> {
> - struct amdgpu_bo *root = p->vm->root.base.bo;
> struct amdgpu_ib *ib = p->job->ibs;
> struct drm_sched_entity *entity;
> + struct dma_fence *f, *tmp;
> struct amdgpu_ring *ring;
> - struct dma_fence *f;
> int r;
>
> entity = p->direct ? &p->vm->direct : &p->vm->delayed;
> @@ -112,7 +111,13 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
> if (r)
> goto error;
>
> - amdgpu_bo_fence(root, f, true);
> + tmp = dma_fence_get(f);
> + if (p->direct)
> + swap(p->vm->last_direct, tmp);
> + else
> + swap(p->vm->last_delayed, tmp);
> + dma_fence_put(tmp);
> +
> if (fence && !p->direct)
> swap(*fence, f);
> dma_fence_put(f);
More information about the amd-gfx
mailing list