Re: [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.y...@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