[PATCH 4/7] drm/amdgpu: rework TLB flushing

[Christian König]

Am 18.03.22 um 16:45 schrieb philip yang:
>
> On 2022-03-17 9:50 a.m., Christian König wrote:
>
>> [SNIP]
>> @@ -890,9 +929,20 @@ int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>   
>>   	r = vm->update_funcs->commit(&params, fence);
>>   
>> +	if (!unlocked && (!(flags & AMDGPU_PTE_VALID) || params.table_freed)) {
>> +		tlb_cb->vm = vm;
>> +		if (!*fence || dma_fence_add_callback(*fence, &tlb_cb->cb,
>> +						      amdgpu_vm_tlb_seq_cb))
>> +			amdgpu_vm_tlb_seq_cb(*fence, &tlb_cb->cb);
>> +		tlb_cb = NULL;
>> +	}
>> +
>
> Should move fence_add_callback before calling vm->update_funcs->commit?
>

No, absolutely not.  vm->update_funcs->commit is what provides the fence 
to add a callback here in the first place.

> With this fixed, patches 5-7 are Reviewed-by: Philip 
> Yang<Philip.Yang at amd.com>
>
> Need another patch to fix svm_range_map_to_gpu, remove local variable 
> table_freed and call kfd_flush_tlb after waiting for update fence done.
>

I think I've already tackled that in the follow up patches, but I'm not 
100% sure I've did it right. Please take a close look at this.

Regards,
Christian.

>>   	if (table_freed)
>>   		*table_freed = *table_freed || params.table_freed;
>>   
>> +error_free:
>> +	kfree(tlb_cb);
>> +
>>   error_unlock:
>>   	amdgpu_vm_eviction_unlock(vm);
>>   	drm_dev_exit(idx);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 1731681914f5..38a1eab1ff74 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -284,6 +284,9 @@ struct amdgpu_vm {
>>   	struct drm_sched_entity	immediate;
>>   	struct drm_sched_entity	delayed;
>>   
>> +	/* Last finished delayed update */
>> +	atomic64_t		tlb_seq;
>> +
>>   	/* Last unlocked submission to the scheduler entities */
>>   	struct dma_fence	*last_unlocked;
>>   
>> @@ -478,4 +481,16 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
>>   void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m);
>>   #endif
>>   
>> +/**
>> + * amdgpu_vm_tlb_seq - return tlb flush sequence number
>> + * @vm: the amdgpu_vm structure to query
>> + *
>> + * Returns the tlb flush sequence number which indicates that the VM TLBs needs
>> + * to be invalidated whenever the sequence number change.
>> + */
>> +static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
>> +{
>> +	return atomic64_read(&vm->tlb_seq);
>> +}
>> +
>>   #endif