Re: [PATCH 08/12] drm/amdgpu: rework synchronization of VM updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 06.01.20 um 22:38 schrieb Felix Kuehling:
On 2020-01-06 10:03 a.m., Christian König wrote:
[SNIP]
@@ -1557,7 +1557,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
        /* sync to everything except eviction fences on unmapping */

I think this comment needs to be updated. Something like "Implicitly sync to command submissions in the same VM before unmapping. Sync to moving fences before mapping."

Good point, just updated this.



[SNIP]
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 68b013be3837..0be72660db4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -44,26 +44,21 @@ static int amdgpu_vm_cpu_map_table(struct amdgpu_bo *table)
   * Returns:
   * Negativ errno, 0 for success.
   */
-static int amdgpu_vm_cpu_prepare(struct amdgpu_vm_update_params *p, void *owner,
-                 struct dma_fence *exclusive)
+static int amdgpu_vm_cpu_prepare(struct amdgpu_vm_update_params *p,
+                 struct dma_resv *resv,
+                 enum amdgpu_sync_mode sync_mode)
  {
+    struct amdgpu_sync sync;
      int r;
  -    /* Wait for any BO move to be completed */
-    if (exclusive) {
-        r = dma_fence_wait(exclusive, true);
-        if (unlikely(r))
-            return r;
-    }
-
-    /* Don't wait for submissions during page fault */
-    if (p->direct)
+    if (!resv)
          return 0;
  -    /* Wait for PT BOs to be idle. PTs share the same resv. object
-     * as the root PD BO
-     */
-    return amdgpu_bo_sync_wait(p->vm->root.base.bo, owner, true);
+    amdgpu_sync_create(&sync);
+    amdgpu_sync_resv(p->adev, &sync, resv, sync_mode, p->vm);
+    r = amdgpu_sync_wait(&sync, true);
+    amdgpu_sync_free(&sync);

This looks like it's pretty much duplicating amdgpu_bo_sync_wait. In fact, when I first wrote that function it was meant as amdgpu_sync_wait_resv (and the comment above that function still incorrectly has that name).

Question is where is the best place to put this? It doesn't really fit into amdgpu_sync.c because it uses the syncobject and doesn't implements it.

But if we give the reservation object as parameter amdgpu_object.c doesn't fits either as well.

Regards,
Christian.


Regards,
  Felix


+    return r;
  }
    /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index 3b61317c0f08..e7a383134521 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -58,9 +58,9 @@ static int amdgpu_vm_sdma_map_table(struct amdgpu_bo *table)
   * Negativ errno, 0 for success.
   */
  static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
-                  void *owner, struct dma_fence *exclusive)
+                  struct dma_resv *resv,
+                  enum amdgpu_sync_mode sync_mode)
  {
-    struct amdgpu_bo *root = p->vm->root.base.bo;
      unsigned int ndw = AMDGPU_VM_SDMA_MIN_NUM_DW;
      int r;
  @@ -70,17 +70,10 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
        p->num_dw_left = ndw;
  -    /* Wait for moves to be completed */
-    r = amdgpu_sync_fence(&p->job->sync, exclusive, false);
-    if (r)
-        return r;
-
-    /* Don't wait for any submissions during page fault handling */
-    if (p->direct)
+    if (!resv)
          return 0;
  -    return amdgpu_sync_resv(p->adev, &p->job->sync, root->tbo.base.resv,
-                AMDGPU_SYNC_NE_OWNER, owner);
+    return amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, p->vm);
  }
    /**

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux