Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

["Christian König" <christian.koenig@amd.com>]

@Felix and @Philip any objections to this patch?

It is actually a bug fix for the NPA support.

Regards,
Christian.

On 9/28/26 21:08, Timur Kristóf wrote:
> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian 
> König wrote:
>> That was broken since adding the NPA support.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> 
> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>  1 file changed, 33 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>> c03327f1242d3..e8f441e018839 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));			
> \
>>  	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), 
> &(cursor)))
>>
>> +/* Return the flags used for cleared PDEs/PTES */
>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>> +					struct amdgpu_vm *vm,
>> +					unsigned int level)
>> +{
>> +	uint64_t flags;
>> +
>> +	if (adev->asic_type < CHIP_VEGA10)
>> +		return 0;
>> +
>> +	if (level != AMDGPU_VM_PTB) {
>> +		uint64_t value = 0;
>> +
>> +		flags = AMDGPU_PDE_PTE_FLAG(adev);
>> +		if (vm->is_npa)
>> +			flags |= adev->gmc.noretry_flags;
>> +		amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>> +	} else if (vm->is_npa) {
>> +		flags = adev->gmc.noretry_flags;
>> +	} else {
>> +		/* Workaround for fault priority problem on GMC9 */
>> +		flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>> +	}
>> +
>> +	return flags;
>> +}
>> +
>>  /**
>>   * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>   *
>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>  	struct amdgpu_vm_update_params params;
>>  	struct amdgpu_bo *ancestor = &vmbo->bo;
>> -	unsigned int entries;
>>  	struct amdgpu_bo *bo = &vmbo->bo;
>> -	uint64_t value = 0, flags = 0;
>> -	uint64_t addr;
>> +	unsigned int entries;
>> +	uint64_t flags;
>>  	int r, idx;
>>
>>  	/* Figure out our place in the hierarchy */
>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm, if (r)
>>  		goto exit;
>>
>> -	addr = 0;
>> -
>> -	if (adev->asic_type >= CHIP_VEGA10) {
>> -		if (level != AMDGPU_VM_PTB) {
>> -			if (vm->is_npa)
>> -				flags = adev->gmc.noretry_flags;
>> -			/* Handle leaf PDEs as PTEs */
>> -			flags |= AMDGPU_PDE_PTE_FLAG(adev);
>> -			amdgpu_gmc_get_vm_pde(adev, level,
>> -					      &value, &flags);
>> -		} else if (vm->is_npa) {
>> -			flags = adev->gmc.noretry_flags;
>> -		} else {
>> -			/* Workaround for fault priority problem on 
> GMC9 */
>> -			flags = AMDGPU_PTE_EXECUTABLE | adev-
>> gmc.init_pte_flags;
>> -		}
>> -	}
>> -
>> -	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>> -				     value, flags);
>> +	flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>> +	r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0, 
> flags);
>>  	if (r)
>>  		goto exit;
>>
>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>> amdgpu_vm_update_params *params, flags |=
>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>  		amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>
>> -	} else if (adev->asic_type >= CHIP_VEGA10 &&
>> -		   !(flags & AMDGPU_PTE_VALID) &&
>> +	} else if (!(flags & AMDGPU_PTE_VALID) &&
>>  		   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>> -
>> -		/* Workaround for fault priority problem on GMC9 and 
> GFX12,
>> -		 * EXECUTABLE for GMC9 fault priority and init_pte_flags
>> -		 * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>> -		 */
>> -		flags |= AMDGPU_PTE_EXECUTABLE | adev-
>> gmc.init_pte_flags;
>> +		flags |= amdgpu_vm_pt_clear_flags(adev, params->vm, 
> level);
>>  	}
>>
>>  	/*
> 
> 
> 
>