[PATCH v2 1/1] drm/amdgpu: Update PDEs flush TLB if PTB/PDB moved

[Lazar, Lijo]

On 6/2/2022 6:54 PM, Lazar, Lijo wrote:
> 
> 
> On 6/2/2022 6:50 PM, Philip Yang wrote:
>> Flush TLBs when existing PDEs are updated because a PTB or PDB moved,
>> but avoids unnecessary TLB flushes when new PDBs or PTBs are added to
>> the page table, which commonly happens when memory is mapped for the
>> first time.
>>
>> Suggested-by: Christian König <christian.koenig at amd.com>
>> Signed-off-by: Philip Yang <Philip.Yang at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 9596c22fded6..1ea204218903 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -737,6 +737,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>>   {
>>       struct amdgpu_vm_update_params params;
>>       struct amdgpu_vm_bo_base *entry;
>> +    bool flush_tlb_needed = false;
>>       int r, idx;
>>       if (list_empty(&vm->relocated))
>> @@ -755,6 +756,9 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>>           goto error;
>>       list_for_each_entry(entry, &vm->relocated, vm_status) {
>> +        /* vm_flush_needed after updating moved PDEs */
>> +        flush_tlb_needed |= entry->moved;
> 
> That is a strange thing to do for a bool variable. Why not just assign it?
> 

Hmm.. In a loop, perhaps you meant logical OR?

Thanks,
Lijo

> Thanks,
> Lijo
> 
>> +
>>           r = amdgpu_vm_pde_update(&params, entry);
>>           if (r)
>>               goto error;
>> @@ -764,8 +768,8 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>>       if (r)
>>           goto error;
>> -    /* vm_flush_needed after updating PDEs */
>> -    atomic64_inc(&vm->tlb_seq);
>> +    if (flush_tlb_needed)
>> +        atomic64_inc(&vm->tlb_seq);
>>       while (!list_empty(&vm->relocated)) {
>>           entry = list_first_entry(&vm->relocated,
>>