On 17-07-12 04:31 AM, Christian König wrote:
> From: Christian König <[email protected]>
>
> No need to try to map them very time.

s/very/every/

Other than that this is Reviewed-by: Felix Kuehling
<[email protected]>.

Regards,
  Felix

>
> Signed-off-by: Christian König <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 87 
> ++++++++++++++++++++--------------
>  1 file changed, 51 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 1c6018b..55d1c7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -159,11 +159,17 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
>   */
>  static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>                                   int (*validate)(void *, struct amdgpu_bo *),
> -                                 void *param)
> +                                 void *param, bool use_cpu_for_update)
>  {
>       unsigned i;
>       int r;
>  
> +     if (use_cpu_for_update) {
> +             r = amdgpu_bo_kmap(parent->bo, NULL);
> +             if (r)
> +                     return r;
> +     }
> +
>       if (!parent->entries)
>               return 0;
>  
> @@ -181,7 +187,8 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt 
> *parent,
>                * Recurse into the sub directory. This is harmless because we
>                * have only a maximum of 5 layers.
>                */
> -             r = amdgpu_vm_validate_level(entry, validate, param);
> +             r = amdgpu_vm_validate_level(entry, validate, param,
> +                                          use_cpu_for_update);
>               if (r)
>                       return r;
>       }
> @@ -212,7 +219,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
> struct amdgpu_vm *vm,
>       if (num_evictions == vm->last_eviction_counter)
>               return 0;
>  
> -     return amdgpu_vm_validate_level(&vm->root, validate, param);
> +     return amdgpu_vm_validate_level(&vm->root, validate, param,
> +                                     vm->use_cpu_for_update);
>  }
>  
>  /**
> @@ -328,6 +336,14 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
> *adev,
>                       if (r)
>                               return r;
>  
> +                     if (vm->use_cpu_for_update) {
> +                             r = amdgpu_bo_kmap(pt, NULL);
> +                             if (r) {
> +                                     amdgpu_bo_unref(&pt);
> +                                     return r;
> +                             }
> +                     }
> +
>                       /* Keep a reference to the root directory to avoid
>                       * freeing them up in the wrong order.
>                       */
> @@ -1042,14 +1058,11 @@ static int amdgpu_vm_update_level(struct 
> amdgpu_device *adev,
>       shadow = parent->bo->shadow;
>  
>       if (vm->use_cpu_for_update) {
> -             r = amdgpu_bo_kmap(parent->bo, (void **)&pd_addr);
> -             if (r)
> -                     return r;
> +             pd_addr = (unsigned long)parent->bo->kptr;
>               r = amdgpu_vm_wait_pd(adev, vm, AMDGPU_FENCE_OWNER_VM);
> -             if (unlikely(r)) {
> -                     amdgpu_bo_kunmap(parent->bo);
> +             if (unlikely(r))
>                       return r;
> -             }
> +
>               params.func = amdgpu_vm_cpu_set_ptes;
>       } else {
>               if (shadow) {
> @@ -1144,28 +1157,29 @@ static int amdgpu_vm_update_level(struct 
> amdgpu_device *adev,
>                           count, incr, AMDGPU_PTE_VALID);
>       }
>  
> -     if (params.func == amdgpu_vm_cpu_set_ptes)
> -             amdgpu_bo_kunmap(parent->bo);
> -     else if (params.ib->length_dw == 0) {
> -             amdgpu_job_free(job);
> -     } else {
> -             amdgpu_ring_pad_ib(ring, params.ib);
> -             amdgpu_sync_resv(adev, &job->sync, parent->bo->tbo.resv,
> -                              AMDGPU_FENCE_OWNER_VM);
> -             if (shadow)
> -                     amdgpu_sync_resv(adev, &job->sync, shadow->tbo.resv,
> +     if (!vm->use_cpu_for_update) {
> +             if (params.ib->length_dw == 0) {
> +                     amdgpu_job_free(job);
> +             } else {
> +                     amdgpu_ring_pad_ib(ring, params.ib);
> +                     amdgpu_sync_resv(adev, &job->sync, parent->bo->tbo.resv,
>                                        AMDGPU_FENCE_OWNER_VM);
> +                     if (shadow)
> +                             amdgpu_sync_resv(adev, &job->sync,
> +                                              shadow->tbo.resv,
> +                                              AMDGPU_FENCE_OWNER_VM);
> +
> +                     WARN_ON(params.ib->length_dw > ndw);
> +                     r = amdgpu_job_submit(job, ring, &vm->entity,
> +                                     AMDGPU_FENCE_OWNER_VM, &fence);
> +                     if (r)
> +                             goto error_free;
>  
> -             WARN_ON(params.ib->length_dw > ndw);
> -             r = amdgpu_job_submit(job, ring, &vm->entity,
> -                             AMDGPU_FENCE_OWNER_VM, &fence);
> -             if (r)
> -                     goto error_free;
> -
> -             amdgpu_bo_fence(parent->bo, fence, true);
> -             dma_fence_put(vm->last_dir_update);
> -             vm->last_dir_update = dma_fence_get(fence);
> -             dma_fence_put(fence);
> +                     amdgpu_bo_fence(parent->bo, fence, true);
> +                     dma_fence_put(vm->last_dir_update);
> +                     vm->last_dir_update = dma_fence_get(fence);
> +                     dma_fence_put(fence);
> +             }
>       }
>       /*
>        * Recurse into the subdirectories. This recursion is harmless because
> @@ -1291,7 +1305,6 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>       uint64_t addr, pe_start;
>       struct amdgpu_bo *pt;
>       unsigned nptes;
> -     int r;
>       bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
>  
>  
> @@ -1310,9 +1323,7 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>  
>  
>               if (use_cpu_update) {
> -                     r = amdgpu_bo_kmap(pt, (void *)&pe_start);
> -                     if (r)
> -                             return r;
> +                     pe_start = (unsigned long)pt->kptr;
>               } else {
>                       if (pt->shadow) {
>                               pe_start = amdgpu_bo_gpu_offset(pt->shadow);
> @@ -1328,9 +1339,6 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>                            AMDGPU_GPU_PAGE_SIZE, flags);
>  
>               dst += nptes * AMDGPU_GPU_PAGE_SIZE;
> -
> -             if (use_cpu_update)
> -                     amdgpu_bo_kunmap(pt);
>       }
>  
>       return 0;
> @@ -2458,6 +2466,13 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm,
>               goto error_free_root;
>  
>       vm->last_eviction_counter = atomic64_read(&adev->num_evictions);
> +
> +     if (vm->use_cpu_for_update) {
> +             r = amdgpu_bo_kmap(vm->root.bo, NULL);
> +             if (r)
> +                     goto error_free_root;
> +     }
> +
>       amdgpu_bo_unreserve(vm->root.bo);
>  
>       return 0;

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to