See one comment inline [Oak]
Thanks,
Oak
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Christian König
Sent: Tuesday, February 19, 2019 8:41 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: [PATCH 2/7] drm/amdgpu: rework shadow handling during PD clear
This way we only deal with the real BO in here.
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 64 ++++++++++++++++----------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 12d51d96491e..3c7b98a758c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -788,38 +788,58 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
if (r)
- goto error;
+ return r;
r = amdgpu_ttm_alloc_gart(&bo->tbo);
if (r)
return r;
+ if (bo->shadow) {
+ r = ttm_bo_validate(&bo->shadow->tbo, &bo->shadow->placement,
+ &ctx);
+ if (r)
+ return r;
+
+ r = amdgpu_ttm_alloc_gart(&bo->shadow->tbo);
+ if (r)
+ return r;
+
+ }
[Oak] I don't have the context what a shadow bo is used for, but is it possible that a shadow bo can have it own shadow? If yes, here you want to do it in a do...while loop. Also although this rework is performance better as you only need one job submission, it makes the codes harder to read. The original recursive way is easier to understand.
+
r = amdgpu_job_alloc_with_ib(adev, 64, &job);
if (r)
- goto error;
+ return r;
- addr = amdgpu_bo_gpu_offset(bo);
- if (ats_entries) {
- uint64_t ats_value;
+ while (1) {
+ addr = amdgpu_bo_gpu_offset(bo);
+ if (ats_entries) {
+ uint64_t ats_value;
- ats_value = AMDGPU_PTE_DEFAULT_ATC;
- if (level != AMDGPU_VM_PTB)
- ats_value |= AMDGPU_PDE_PTE;
+ ats_value = AMDGPU_PTE_DEFAULT_ATC;
+ if (level != AMDGPU_VM_PTB)
+ ats_value |= AMDGPU_PDE_PTE;
- amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
- ats_entries, 0, ats_value);
- addr += ats_entries * 8;
- }
+ amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
+ ats_entries, 0, ats_value);
+ addr += ats_entries * 8;
+ }
- if (entries) {
- uint64_t value = 0;
+ if (entries) {
+ uint64_t value = 0;
- /* Workaround for fault priority problem on GMC9 */
- if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10)
- value = AMDGPU_PTE_EXECUTABLE;
+ /* Workaround for fault priority problem on GMC9 */
+ if (level == AMDGPU_VM_PTB &&
+ adev->asic_type >= CHIP_VEGA10)
+ value = AMDGPU_PTE_EXECUTABLE;
+
+ amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
+ entries, 0, value);
+ }
- amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
- entries, 0, value);
+ if (bo->shadow)
+ bo = bo->shadow;
+ else
+ break;
}
amdgpu_ring_pad_ib(ring, &job->ibs[0]); @@ -838,16 +858,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
amdgpu_bo_fence(bo, fence, true);
dma_fence_put(fence);
- if (bo->shadow)
- return amdgpu_vm_clear_bo(adev, vm, bo->shadow,
- level, pte_support_ats);
-
return 0;
error_free:
amdgpu_job_free(job);
-
-error:
return r;
}
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
