On 17-07-13 09:25 AM, Christian König wrote:
> From: Christian König <[email protected]>
>
> That should reduce our command submission overhead quite a bit.
> Signed-off-by: Christian König <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 55d1c7f..3f5863e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -173,6 +173,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt 
> *parent,
>       if (!parent->entries)
>               return 0;
>  
> +     parent->needs_update = true;

This means that after amdgpu_vm_validate_pt_bos, all vm_pts will have
needs_update set, even if the last eviction didn't evict any page
tables. That will create unnecessary overhead in amdgpu_vm_update_level.
You could probably add another check in the loop below for whether
validate actually moved any child page tables.

Regards,
  Felix

>       for (i = 0; i <= parent->last_entry_used; ++i) {
>               struct amdgpu_vm_pt *entry = &parent->entries[i];
>  
> @@ -351,6 +352,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
> *adev,
>  
>                       entry->bo = pt;
>                       entry->addr = 0;
> +                     parent->needs_update = true;
>               }
>  
>               if (level < adev->vm_manager.num_level) {
> @@ -361,6 +363,8 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
> *adev,
>                                                  sub_eaddr, level);
>                       if (r)
>                               return r;
> +
> +                     parent->needs_update |= entry->needs_update;
>               }
>       }
>  
> @@ -1050,7 +1054,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
> *adev,
>  
>       int r;
>  
> -     if (!parent->entries)
> +     if (!parent->entries || !parent->needs_update)
>               return 0;
>  
>       memset(&params, 0, sizeof(params));
> @@ -1196,6 +1200,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
> *adev,
>                       return r;
>       }
>  
> +     parent->needs_update = false;
>       return 0;
>  
>  error_free:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 3441ec5..c3dfd3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -102,6 +102,7 @@ struct amdgpu_vm_pt {
>       /* array of page tables, one for each directory entry */
>       struct amdgpu_vm_pt     *entries;
>       unsigned                last_entry_used;
> +     bool                    needs_update;
>  };
>  
>  struct amdgpu_vm {

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

Reply via email to