On 17-07-12 04:31 AM, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> We don't have any update fence in that case, so the need
> for flushing isn't detected atomatically.
s/atomatically/automatically/
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ff5de3a..bf7696a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -421,7 +421,7 @@ static int amdgpu_vm_grab_reserved_vmid_locked(struct amdgpu_vm *vm,
> struct dma_fence *updates = sync->last_vm_update;
> int r = 0;
> struct dma_fence *flushed, *tmp;
> - bool needs_flush = false;
> + bool needs_flush = vm->use_cpu_for_update;
I think this will result in unnecessary flushing. To avoid that you'd
probably have to define a new fence timeline for CPU pagetable updates
so that the code below can check which updates have already been
flushed. Fences on that timeline would be signalled as soon as they are
created.
Anyway, this fixes correctness first. Optimization can come later.
With the typo above fixed, this is Reviewed-by: Felix Kuehling
<Felix.Kuehling at amd.com>
Regards,
Felix
>
> flushed = id->flushed_updates;
> if ((amdgpu_vm_had_gpu_reset(adev, id)) ||
> @@ -542,11 +542,11 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
> }
> kfree(fences);
>
> - job->vm_needs_flush = false;
> + job->vm_needs_flush = vm->use_cpu_for_update;
> /* Check if we can use a VMID already assigned to this VM */
> list_for_each_entry_reverse(id, &id_mgr->ids_lru, list) {
> struct dma_fence *flushed;
> - bool needs_flush = false;
> + bool needs_flush = vm->use_cpu_for_update;
>
> /* Check all the prerequisites to using this VMID */
> if (amdgpu_vm_had_gpu_reset(adev, id))
