Re: [PATCH] drm/amdgpu : remove unused code

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 04.03.24 um 10:16 schrieb jesse.zhang@amd.com:
> From: Jesse Zhang <jesse.zhang@amd.com>
>
> Remove the unused function - amdgpu_vm_pt_is_root_clean
> and remove the impossible condition
>
> v1: entries == 0 is not possible any more,
>     so this condition could probably be removed (Felix)
>
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h    |  2 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 61 ++++++-----------------
>   2 files changed, 16 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 7f95039bb37d..047ec1930d12 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -537,8 +537,6 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			int level, bool immediate, struct amdgpu_bo_vm **vmbo,
>   			int32_t xcp_id);
>   void amdgpu_vm_pt_free_root(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> -bool amdgpu_vm_pt_is_root_clean(struct amdgpu_device *adev,
> -				struct amdgpu_vm *vm);
>   
>   int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params,
>   			 struct amdgpu_vm_bo_base *entry);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> index 8bce4da67131..7ecddb77b3ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -367,6 +367,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	struct amdgpu_bo *bo = &vmbo->bo;
>   	uint64_t addr;
>   	int r, idx;
> +	uint64_t value = 0, flags = 0;

Please don't initialize the values here.

Rather move that before the if (... >= VEGA10) below.

With that fixed feel free to add Reviewed-by: Christian König 
<christian.koenig@amd.com>

Regards,
Christian.

>   
>   	/* Figure out our place in the hierarchy */
>   	if (ancestor->parent) {
> @@ -409,27 +410,24 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   
>   	addr = 0;
>   
> -	if (entries) {
> -		uint64_t value = 0, flags = 0;
> -
> -		if (adev->asic_type >= CHIP_VEGA10) {
> -			if (level != AMDGPU_VM_PTB) {
> -				/* Handle leaf PDEs as PTEs */
> -				flags |= AMDGPU_PDE_PTE;
> -				amdgpu_gmc_get_vm_pde(adev, level,
> -						      &value, &flags);
> -			} else {
> -				/* Workaround for fault priority problem on GMC9 */
> -				flags = AMDGPU_PTE_EXECUTABLE;
> -			}
> -		}
>   
> -		r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
> -					     value, flags);
> -		if (r)
> -			goto exit;
> +	if (adev->asic_type >= CHIP_VEGA10) {
> +		if (level != AMDGPU_VM_PTB) {
> +			/* Handle leaf PDEs as PTEs */
> +			flags |= AMDGPU_PDE_PTE;
> +			amdgpu_gmc_get_vm_pde(adev, level,
> +					      &value, &flags);
> +		} else {
> +			/* Workaround for fault priority problem on GMC9 */
> +			flags = AMDGPU_PTE_EXECUTABLE;
> +		}
>   	}
>   
> +	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
> +				     value, flags);
> +	if (r)
> +		goto exit;
> +
>   	r = vm->update_funcs->commit(&params, NULL);
>   exit:
>   	drm_dev_exit(idx);
> @@ -673,33 +671,6 @@ void amdgpu_vm_pt_free_root(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	amdgpu_vm_pt_free_dfs(adev, vm, NULL, false);
>   }
>   
> -/**
> - * amdgpu_vm_pt_is_root_clean - check if a root PD is clean
> - *
> - * @adev: amdgpu_device pointer
> - * @vm: the VM to check
> - *
> - * Check all entries of the root PD, if any subsequent PDs are allocated,
> - * it means there are page table creating and filling, and is no a clean
> - * VM
> - *
> - * Returns:
> - *	0 if this VM is clean
> - */
> -bool amdgpu_vm_pt_is_root_clean(struct amdgpu_device *adev,
> -				struct amdgpu_vm *vm)
> -{
> -	enum amdgpu_vm_level root = adev->vm_manager.root_level;
> -	unsigned int entries = amdgpu_vm_pt_num_entries(adev, root);
> -	unsigned int i = 0;
> -
> -	for (i = 0; i < entries; i++) {
> -		if (to_amdgpu_bo_vm(vm->root.bo)->entries[i].bo)
> -			return false;
> -	}
> -	return true;
> -}
> -
>   /**
>    * amdgpu_vm_pde_update - update a single level in the hierarchy
>    *