<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2022-03-21 5:33 a.m., Christian
      König wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:9a2ecbb9-21dd-842b-2787-7c50930acfc3@amd.com">Am
      18.03.22 um 16:45 schrieb philip yang:
      <br>
      <blockquote type="cite">
        <br>
        On 2022-03-17 9:50 a.m., Christian König wrote:
        <br>
        <br>
        <blockquote type="cite">[SNIP]
          <br>
          @@ -890,9 +929,20 @@ int amdgpu_vm_bo_update_mapping(struct
          amdgpu_device *adev,
          <br>
                  r = vm->update_funcs->commit(&params,
          fence);
          <br>
            +    if (!unlocked && (!(flags &
          AMDGPU_PTE_VALID) || params.table_freed)) {
          <br>
          +        tlb_cb->vm = vm;
          <br>
          +        if (!*fence || dma_fence_add_callback(*fence,
          &tlb_cb->cb,
          <br>
          +                              amdgpu_vm_tlb_seq_cb))
          <br>
          +            amdgpu_vm_tlb_seq_cb(*fence, &tlb_cb->cb);
          <br>
          +        tlb_cb = NULL;
          <br>
          +    }
          <br>
          +
          <br>
        </blockquote>
        <br>
        Should move fence_add_callback before calling
        vm->update_funcs->commit?
        <br>
        <br>
      </blockquote>
      <br>
      No, absolutely not.  vm->update_funcs->commit is what
      provides the fence to add a callback here in the first place.
      <br>
      <br>
      <blockquote type="cite">With this fixed, patches 5-7 are
        Reviewed-by: Philip Yang<a class="moz-txt-link-rfc2396E" href="mailto:Philip.Yang@amd.com"><Philip.Yang@amd.com></a>
        <br>
        <br>
        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.
        <br>
        <br>
      </blockquote>
      <br>
      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.<br>
    </blockquote>
    <p>Added the missing patch to patch 7.<br>
    </p>
    <p>Regards.</p>
    <p>Philip<br>
    </p>
    <blockquote type="cite" cite="mid:9a2ecbb9-21dd-842b-2787-7c50930acfc3@amd.com">Regards,
      <br>
      Christian.
      <br>
      <br>
      <blockquote type="cite">
        <blockquote type="cite">      if (table_freed)
          <br>
                    *table_freed = *table_freed || params.table_freed;
          <br>
            +error_free:
          <br>
          +    kfree(tlb_cb);
          <br>
          +
          <br>
            error_unlock:
          <br>
                amdgpu_vm_eviction_unlock(vm);
          <br>
                drm_dev_exit(idx);
          <br>
          diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
          b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
          <br>
          index 1731681914f5..38a1eab1ff74 100644
          <br>
          --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
          <br>
          +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
          <br>
          @@ -284,6 +284,9 @@ struct amdgpu_vm {
          <br>
                struct drm_sched_entity    immediate;
          <br>
                struct drm_sched_entity    delayed;
          <br>
            +    /* Last finished delayed update */
          <br>
          +    atomic64_t        tlb_seq;
          <br>
          +
          <br>
                /* Last unlocked submission to the scheduler entities */
          <br>
                struct dma_fence    *last_unlocked;
          <br>
            @@ -478,4 +481,16 @@ int amdgpu_vm_ptes_update(struct
          amdgpu_vm_update_params *params,
          <br>
            void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct
          seq_file *m);
          <br>
            #endif
          <br>
            +/**
          <br>
          + * amdgpu_vm_tlb_seq - return tlb flush sequence number
          <br>
          + * @vm: the amdgpu_vm structure to query
          <br>
          + *
          <br>
          + * Returns the tlb flush sequence number which indicates that
          the VM TLBs needs
          <br>
          + * to be invalidated whenever the sequence number change.
          <br>
          + */
          <br>
          +static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm
          *vm)
          <br>
          +{
          <br>
          +    return atomic64_read(&vm->tlb_seq);
          <br>
          +}
          <br>
          +
          <br>
            #endif
          <br>
        </blockquote>
      </blockquote>
      <br>
    </blockquote>
  </body>
</html>