[PATCH 2/6] drm/amdgpu: let amdgpu_vm_clear_bo figure out ats status v2

[Zeng, Oak]

Making some codes into functions would help people to understand. At least for me the codes is not very easy to understand. See inline comments. But those suggested new functions are only used in this clear_bo function, so it is your call to do it or not. Anyway this patch is Reviewed-by: Oak Zeng <Oak.Zeng at amd.com>

Regards,
Oak

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Christian König
Sent: Tuesday, February 26, 2019 7:47 AM
To: amd-gfx at lists.freedesktop.org
Subject: [PATCH 2/6] drm/amdgpu: let amdgpu_vm_clear_bo figure out ats status v2

Instead of providing it from outside figure out the ats status in the function itself from the data structures.

v2: simplify finding the right level

Signed-off-by: Christian König <christian.koenig at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 51 ++++++++++++++------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1613305610dd..362436f4e856 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -747,8 +747,6 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
  * @adev: amdgpu_device pointer
  * @vm: VM to clear BO from
  * @bo: BO to clear
- * @level: level this BO is at
- * @pte_support_ats: indicate ATS support from PTE
  *
  * Root PD needs to be reserved when calling this.
  *
@@ -756,10 +754,11 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
  * 0 on success, errno otherwise.
  */
 static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
-			      struct amdgpu_vm *vm, struct amdgpu_bo *bo,
-			      unsigned level, bool pte_support_ats)
+			      struct amdgpu_vm *vm,
+			      struct amdgpu_bo *bo)
 {
 	struct ttm_operation_ctx ctx = { true, false };
+	unsigned level = adev->vm_manager.root_level;
 	struct dma_fence *fence = NULL;
 	unsigned entries, ats_entries;
 	struct amdgpu_ring *ring;
@@ -768,17 +767,31 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
 	int r;
 
 	entries = amdgpu_bo_size(bo) / 8;
+	if (vm->pte_support_ats) {
+		ats_entries = amdgpu_vm_level_shift(adev, level);
+		ats_entries += AMDGPU_GPU_PAGE_SHIFT;
+		ats_entries = AMDGPU_GMC_HOLE_START >> ats_entries;
 [Oak] This can be make a function or macro: GET_MAX_ATS_ENTRIES(level)
-	if (pte_support_ats) {
-		if (level == adev->vm_manager.root_level) {
-			ats_entries = amdgpu_vm_level_shift(adev, level);
-			ats_entries += AMDGPU_GPU_PAGE_SHIFT;
-			ats_entries = AMDGPU_GMC_HOLE_START >> ats_entries;
+		if (!bo->parent) {
 			ats_entries = min(ats_entries, entries);
 			entries -= ats_entries;
 		} else {
-			ats_entries = entries;
-			entries = 0;
+			struct amdgpu_bo *ancestor = bo;
+			struct amdgpu_vm_pt *pt;
+
+			do {
+				++level;
+				ancestor = ancestor->parent;
+			} while (ancestor);
+
+			pt = container_of(ancestor->vm_bo, struct amdgpu_vm_pt,
+					  base);
[Oak]: above 10 lines can be make a function: amdgpu_vm_get_bo_level_and_top_level_pt(struct amdgpu_bo * bo, unsigned *level, amdgpu_vm **pt)
+			if ((pt - vm->root.entries) >= ats_entries) {
+				ats_entries = 0;
+			} else {
+				ats_entries = entries;
+				entries = 0;
+			}
 		}
 	} else {
 		ats_entries = 0;
@@ -908,7 +921,6 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,  {
 	struct amdgpu_vm_pt_cursor cursor;
 	struct amdgpu_bo *pt;
-	bool ats = false;
 	uint64_t eaddr;
 	int r;
 
@@ -918,9 +930,6 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 
 	eaddr = saddr + size - 1;
 
-	if (vm->pte_support_ats)
-		ats = saddr < AMDGPU_GMC_HOLE_START;
-
 	saddr /= AMDGPU_GPU_PAGE_SIZE;
 	eaddr /= AMDGPU_GPU_PAGE_SIZE;
 
@@ -969,7 +978,7 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 
 		amdgpu_vm_bo_base_init(&entry->base, vm, pt);
 
-		r = amdgpu_vm_clear_bo(adev, vm, pt, cursor.level, ats);
+		r = amdgpu_vm_clear_bo(adev, vm, pt);
 		if (r)
 			goto error_free_pt;
 	}
@@ -3044,9 +3053,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 
 	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
 
-	r = amdgpu_vm_clear_bo(adev, vm, root,
-			       adev->vm_manager.root_level,
-			       vm->pte_support_ats);
+	r = amdgpu_vm_clear_bo(adev, vm, root);
 	if (r)
 		goto error_unreserve;
 
@@ -3141,9 +3148,8 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, uns
 	 * changing any other state, in case it fails.
 	 */
 	if (pte_support_ats != vm->pte_support_ats) {
-		r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
-			       adev->vm_manager.root_level,
-			       pte_support_ats);
+		vm->pte_support_ats = pte_support_ats;
+		r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo);
 		if (r)
 			goto free_idr;
 	}
@@ -3151,7 +3157,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, uns
 	/* Update VM state */
 	vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
 				    AMDGPU_VM_USE_CPU_FOR_COMPUTE);
-	vm->pte_support_ats = pte_support_ats;
 	DRM_DEBUG_DRIVER("VM update mode is %s\n",
 			 vm->use_cpu_for_update ? "CPU" : "SDMA");
 	WARN_ONCE((vm->use_cpu_for_update && !amdgpu_gmc_vram_full_visible(&adev->gmc)),
--
2.17.1

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