On 5/27/26 18:29, Natalie Vock wrote:
> This state can be reached via other means than physical moves, like PRT
> bindings. Make the name match the actual purpose of the state.
> 
> Signed-off-by: Natalie Vock <[email protected]>

That's a really good idea, I was already wondering in the original patch how to 
improve the naming but couldn't come up with something better.

Reviewed-by: Christian König <[email protected]>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 55 ++++++++++++++------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  9 +++--
>  2 files changed, 34 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 969716b3e67e4..36f6c0c36f5e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -142,7 +142,7 @@ static void amdgpu_vm_assert_locked(struct amdgpu_vm *vm)
>  static void amdgpu_vm_bo_status_init(struct amdgpu_vm_bo_status *lists)
>  {
>       INIT_LIST_HEAD(&lists->evicted);
> -     INIT_LIST_HEAD(&lists->moved);
> +     INIT_LIST_HEAD(&lists->needs_update);
>       INIT_LIST_HEAD(&lists->idle);
>  }
>  
> @@ -211,16 +211,17 @@ static void amdgpu_vm_bo_evicted(struct 
> amdgpu_vm_bo_base *vm_bo)
>       amdgpu_vm_bo_unlock_lists(vm_bo);
>  }
>  /**
> - * amdgpu_vm_bo_moved - vm_bo is moved
> + * amdgpu_vm_bo_needs_update - vm_bo needs pagetable update
>   *
> - * @vm_bo: vm_bo which is moved
> + * @vm_bo: vm_bo which is out of date
>   * @moved: true if the BO physically changed locations, i.e. all previous
>   *         mappings are invalid
>   *
> - * State for vm_bo objects meaning the underlying BO was moved but the new
> - * location not yet reflected in the page tables.
> + * State for vm_bo objects meaning the underlying BO had mapping changes 
> (move, PRT bind/unbind)
> + * but the new location is not yet reflected in the page tables.
>   */
> -static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo, bool moved)
> +static void amdgpu_vm_bo_needs_update(struct amdgpu_vm_bo_base *vm_bo,
> +                                   bool moved)
>  {
>       struct amdgpu_vm_bo_status *lists;
>       struct amdgpu_bo *bo = vm_bo->bo;
> @@ -236,7 +237,7 @@ static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base 
> *vm_bo, bool moved)
>       } else {
>               if (moved)
>                       vm_bo->moved = true;
> -             list_move(&vm_bo->vm_status, &lists->moved);
> +             list_move(&vm_bo->vm_status, &lists->needs_update);
>       }
>       amdgpu_vm_bo_unlock_lists(vm_bo);
>  }
> @@ -270,11 +271,12 @@ static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base 
> *vm_bo)
>  static void amdgpu_vm_bo_reset_state_machine(struct amdgpu_vm *vm)
>  {
>       amdgpu_vm_assert_locked(vm);
> -     list_splice_init(&vm->kernel.idle, &vm->kernel.moved);
> -     list_splice_init(&vm->always_valid.idle, &vm->always_valid.moved);
> +     list_splice_init(&vm->kernel.idle, &vm->kernel.needs_update);
> +     list_splice_init(&vm->always_valid.idle,
> +                      &vm->always_valid.needs_update);
>  
>       spin_lock(&vm->individual_lock);
> -     list_splice_init(&vm->individual.idle, &vm->individual.moved);
> +     list_splice_init(&vm->individual.idle, &vm->individual.needs_update);
>       spin_unlock(&vm->individual_lock);
>  }
>  
> @@ -428,7 +430,7 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base 
> *base,
>        */
>       if (bo->preferred_domains &
>           amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type))
> -             amdgpu_vm_bo_moved(base, true);
> +             amdgpu_vm_bo_needs_update(base, true);
>       else
>               amdgpu_vm_bo_evicted(base);
>  }
> @@ -600,7 +602,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm,
>                       return r;
>  
>               vm->update_funcs->map_table(to_amdgpu_bo_vm(bo_base->bo));
> -             amdgpu_vm_bo_moved(bo_base, false);
> +             amdgpu_vm_bo_needs_update(bo_base, false);
>       }
>  
>       /*
> @@ -617,7 +619,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm,
>               if (r)
>                       return r;
>  
> -             amdgpu_vm_bo_moved(bo_base, false);
> +             amdgpu_vm_bo_needs_update(bo_base, false);
>       }
>  
>       if (!ticket)
> @@ -637,7 +639,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm,
>               if (r)
>                       return r;
>  
> -             amdgpu_vm_bo_moved(bo_base, false);
> +             amdgpu_vm_bo_needs_update(bo_base, false);
>  
>               /* It's a bit inefficient to always jump back to the start, but
>                * we would need to re-structure the KFD for properly fixing
> @@ -971,7 +973,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>  
>       amdgpu_vm_assert_locked(vm);
>  
> -     if (list_empty(&vm->kernel.moved))
> +     if (list_empty(&vm->kernel.needs_update))
>               return 0;
>  
>       if (!drm_dev_enter(adev_to_drm(adev), &idx))
> @@ -987,7 +989,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>       if (r)
>               goto error;
>  
> -     list_for_each_entry(entry, &vm->kernel.moved, vm_status) {
> +     list_for_each_entry(entry, &vm->kernel.needs_update, vm_status) {
>               /* vm_flush_needed after updating moved PDEs */
>               flush_tlb_needed |= entry->moved;
>  
> @@ -1003,7 +1005,8 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>       if (flush_tlb_needed)
>               atomic64_inc(&vm->tlb_seq);
>  
> -     list_for_each_entry_safe(entry, tmp, &vm->kernel.moved, vm_status)
> +     list_for_each_entry_safe(entry, tmp, &vm->kernel.needs_update,
> +                              vm_status)
>               amdgpu_vm_bo_idle(entry);
>  
>  error:
> @@ -1616,7 +1619,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>       bool clear, unlock;
>       int r;
>  
> -     list_for_each_entry_safe(bo_va, tmp, &vm->always_valid.moved,
> +     list_for_each_entry_safe(bo_va, tmp, &vm->always_valid.needs_update,
>                                base.vm_status) {
>               /* Per VM BOs never need to bo cleared in the page tables */
>               r = amdgpu_vm_bo_update(adev, bo_va, NULL, false, false);
> @@ -1625,8 +1628,8 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>       }
>  
>       spin_lock(&vm->individual_lock);
> -     while (!list_empty(&vm->individual.moved)) {
> -             bo_va = list_first_entry(&vm->individual.moved,
> +     while (!list_empty(&vm->individual.needs_update)) {
> +             bo_va = list_first_entry(&vm->individual.needs_update,
>                                        typeof(*bo_va), base.vm_status);
>               resv = bo_va->base.bo->tbo.base.resv;
>               spin_unlock(&vm->individual_lock);
> @@ -1785,7 +1788,7 @@ static void amdgpu_vm_bo_insert_map(struct 
> amdgpu_device *adev,
>               amdgpu_vm_prt_get(adev);
>  
>       if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
> -             amdgpu_vm_bo_moved(&bo_va->base, false);
> +             amdgpu_vm_bo_needs_update(&bo_va->base, false);
>  
>       trace_amdgpu_vm_bo_map(bo_va, mapping);
>  }
> @@ -2094,7 +2097,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device 
> *adev,
>  
>               if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
>                   !before->bo_va->base.moved)
> -                     amdgpu_vm_bo_moved(&before->bo_va->base, false);
> +                     amdgpu_vm_bo_needs_update(&before->bo_va->base, false);
>       } else {
>               kfree(before);
>       }
> @@ -2109,7 +2112,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device 
> *adev,
>  
>               if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
>                   !after->bo_va->base.moved)
> -                     amdgpu_vm_bo_moved(&after->bo_va->base, false);
> +                     amdgpu_vm_bo_needs_update(&after->bo_va->base, false);
>       } else {
>               kfree(after);
>       }
> @@ -2283,7 +2286,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_bo *bo, bool 
> evicted)
>  
>               if (bo_base->moved)
>                       continue;
> -             amdgpu_vm_bo_moved(bo_base, true);
> +             amdgpu_vm_bo_needs_update(bo_base, true);
>       }
>  }
>  
> @@ -3077,7 +3080,7 @@ static void amdgpu_debugfs_vm_bo_status_info(struct 
> seq_file *m,
>  
>       id = 0;
>       seq_puts(m, "\tMoved BOs:\n");
> -     list_for_each_entry(base, &lists->moved, vm_status) {
> +     list_for_each_entry(base, &lists->needs_update, vm_status) {
>               if (!base->bo)
>                       continue;
>  
> @@ -3086,7 +3089,7 @@ static void amdgpu_debugfs_vm_bo_status_info(struct 
> seq_file *m,
>  
>       id = 0;
>       seq_puts(m, "\tIdle BOs:\n");
> -     list_for_each_entry(base, &lists->moved, vm_status) {
> +     list_for_each_entry(base, &lists->needs_update, vm_status) {
>               if (!base->bo)
>                       continue;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index d3f3852f1ebae..e6ad79b09042f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -212,7 +212,8 @@ struct amdgpu_vm_bo_base {
>        * protected by vm BO being reserved */
>       bool                            shared;
>  
> -     /* protected by the BO being reserved */
> +     /* if the BO was moved and all mappings are invalid
> +      * protected by the BO being reserved */
>       bool                            moved;
>  };
>  
> @@ -220,14 +221,14 @@ struct amdgpu_vm_bo_base {
>   * The following status lists contain amdgpu_vm_bo_base objects for
>   * either PD/PTs, per VM BOs or BOs with individual resv object.
>   *
> - * The state transits are: evicted -> moved -> idle
> + * The state transits are: evicted -> needs_update -> idle
>   */
>  struct amdgpu_vm_bo_status {
>       /* BOs evicted which need to move into place again */
>       struct list_head                evicted;
>  
> -     /* BOs which moved but new location hasn't been updated in the PDs/PTs 
> */
> -     struct list_head                moved;
> +     /* BOs whose mappings changed but PDs/PTs haven't been updated */
> +     struct list_head needs_update;
>  
>       /* BOs done with the state machine and need no further action */
>       struct list_head                idle;

Reply via email to