[PATCH 1/2] drm/amdgpu: increase fragmentation size for Vega10 v2

[PATCH 1/2] drm/amdgpu: increase fragmentation size for Vega10 v2

[Christian König]

From: Christian König <christian.koenig@amd.com>

The fragment bits work differently for Vega10 compared to previous generations.

Increase the fragment size to 2MB for now to better handle that.

v2: handle the hardware setup as well

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  | 5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h   | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 4 +++-
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 4 +++-
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 7a8da32..fc77844 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -588,8 +588,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 		dev_info.virtual_address_offset = AMDGPU_VA_RESERVED_SIZE;
 		dev_info.virtual_address_max = (uint64_t)adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
 		dev_info.virtual_address_alignment = max((int)PAGE_SIZE, AMDGPU_GPU_PAGE_SIZE);
-		dev_info.pte_fragment_size = (1 << AMDGPU_LOG2_PAGES_PER_FRAG) *
-					     AMDGPU_GPU_PAGE_SIZE;
+		dev_info.pte_fragment_size =
+			(1 << AMDGPU_LOG2_PAGES_PER_FRAG(adev)) *
+			AMDGPU_GPU_PAGE_SIZE;
 		dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE;
 
 		dev_info.cu_active_number = adev->gfx.cu_info.number;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 55d1c7f..a3dbebe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1381,8 +1381,9 @@ static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params	*params,
 	 */
 
 	/* SI and newer are optimized for 64KB */
-	uint64_t frag_flags = AMDGPU_PTE_FRAG(AMDGPU_LOG2_PAGES_PER_FRAG);
-	uint64_t frag_align = 1 << AMDGPU_LOG2_PAGES_PER_FRAG;
+	unsigned pages_per_frag = AMDGPU_LOG2_PAGES_PER_FRAG(params->adev);
+	uint64_t frag_flags = AMDGPU_PTE_FRAG(pages_per_frag);
+	uint64_t frag_align = 1 << pages_per_frag;
 
 	uint64_t frag_start = ALIGN(start, frag_align);
 	uint64_t frag_end = end & ~(frag_align - 1);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 3441ec5..c4f5d1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -51,7 +51,9 @@ struct amdgpu_bo_list_entry;
 #define AMDGPU_VM_PTB_ALIGN_SIZE   32768
 
 /* LOG2 number of continuous pages for the fragment field */
-#define AMDGPU_LOG2_PAGES_PER_FRAG 4
+#define AMDGPU_LOG2_PAGES_PER_FRAG(adev) \
+	((adev)->asic_type < CHIP_VEGA10 ? 4 : \
+	 (adev)->vm_manager.block_size)
 
 #define AMDGPU_PTE_VALID	(1ULL << 0)
 #define AMDGPU_PTE_SYSTEM	(1ULL << 1)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index 008ad3d..408723e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -129,7 +129,7 @@ static void gfxhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
 	/* Setup L2 cache */
 	tmp = RREG32_SOC15(GC, 0, mmVM_L2_CNTL);
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 1);
-	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_FRAGMENT_PROCESSING, 0);
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_FRAGMENT_PROCESSING, 1);
 	/* XXX for emulation, Refer to closed source code.*/
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, L2_PDE0_CACHE_TAG_GENERATION_MODE,
 			    0);
@@ -144,6 +144,8 @@ static void gfxhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
 	WREG32_SOC15(GC, 0, mmVM_L2_CNTL2, tmp);
 
 	tmp = mmVM_L2_CNTL3_DEFAULT;
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12);
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
 	WREG32_SOC15(GC, 0, mmVM_L2_CNTL3, tmp);
 
 	tmp = mmVM_L2_CNTL4_DEFAULT;
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index 96f1628..ad8def3 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -143,7 +143,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
 	/* Setup L2 cache */
 	tmp = RREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL);
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 1);
-	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_FRAGMENT_PROCESSING, 0);
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_FRAGMENT_PROCESSING, 1);
 	/* XXX for emulation, Refer to closed source code.*/
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, L2_PDE0_CACHE_TAG_GENERATION_MODE,
 			    0);
@@ -158,6 +158,8 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
 	WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL2, tmp);
 
 	tmp = mmVM_L2_CNTL3_DEFAULT;
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, BANK_SELECT, 12);
+	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL3, L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
 	WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL3, tmp);
 
 	tmp = mmVM_L2_CNTL4_DEFAULT;
-- 
2.7.4

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

[PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Christian König]

From: Christian König <christian.koenig@amd.com>

The hardware can use huge pages to map 2MB of address space with only one PDE.

v2: few cleanups and rebased
v3: skip PT updates if we are using the PDE
v4: rebased, added support for CPU based updates

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119 +++++++++++++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
 2 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a3dbebe..62d97f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 
 			entry->bo = pt;
 			entry->addr = 0;
+			entry->huge_page = false;
 		}
 
 		if (level < adev->vm_manager.num_level) {
@@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
 
 		pt = amdgpu_bo_gpu_offset(bo);
 		pt = amdgpu_gart_get_vm_pde(adev, pt);
-		if (parent->entries[pt_idx].addr == pt)
+		if (parent->entries[pt_idx].addr == pt ||
+		    parent->entries[pt_idx].huge_page)
 			continue;
 
 		parent->entries[pt_idx].addr = pt;
@@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
 }
 
 /**
- * amdgpu_vm_find_pt - find the page table for an address
+ * amdgpu_vm_find_entry - find the entry for an address
  *
  * @p: see amdgpu_pte_update_params definition
  * @addr: virtual address in question
+ * @entry: resulting entry or NULL
+ * @parent: parent entry
  *
- * Find the page table BO for a virtual address, return NULL when none found.
+ * Find the vm_pt entry and it's parent for the given address.
  */
-static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
-					  uint64_t addr)
+void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p, uint64_t addr,
+			 struct amdgpu_vm_pt **entry,
+			 struct amdgpu_vm_pt **parent)
 {
-	struct amdgpu_vm_pt *entry = &p->vm->root;
 	unsigned idx, level = p->adev->vm_manager.num_level;
 
-	while (entry->entries) {
+	*parent = NULL;
+	*entry = &p->vm->root;
+	while ((*entry)->entries) {
 		idx = addr >> (p->adev->vm_manager.block_size * level--);
-		idx %= amdgpu_bo_size(entry->bo) / 8;
-		entry = &entry->entries[idx];
+		idx %= amdgpu_bo_size((*entry)->bo) / 8;
+		*parent = *entry;
+		*entry = &(*entry)->entries[idx];
 	}
 
 	if (level)
-		return NULL;
+		*entry = NULL;
+}
+
+/**
+ * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
+ *
+ * @p: see amdgpu_pte_update_params definition
+ * @entry: vm_pt entry to check
+ * @parent: parent entry
+ * @nptes: number of PTEs updated with this operation
+ * @dst: destination address where the PTEs should point to
+ * @flags: access flags fro the PTEs
+ *
+ * Check if we can update the PD with a huge page.
+ */
+static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
+				       struct amdgpu_vm_pt *entry,
+				       struct amdgpu_vm_pt *parent,
+				       unsigned nptes, uint64_t dst,
+				       uint64_t flags)
+{
+	bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
+	uint64_t pd_addr, pde;
+	int r;
 
-	return entry->bo;
+	/* In the case of a mixed PT the PDE must point to it*/
+	if (p->adev->asic_type < CHIP_VEGA10 ||
+	    nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
+	    p->func != amdgpu_vm_do_set_ptes ||
+	    !(flags & AMDGPU_PTE_VALID)) {
+
+		dst = amdgpu_bo_gpu_offset(entry->bo);
+		dst = amdgpu_gart_get_vm_pde(p->adev, dst);
+		flags = AMDGPU_PTE_VALID;
+	} else {
+		flags |= AMDGPU_PDE_PTE;
+	}
+
+	if (entry->addr == dst &&
+	    entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
+		return 0;
+
+	entry->addr = dst;
+	entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
+
+	if (use_cpu_update) {
+		r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
+		if (r)
+			return r;
+
+		pde = pd_addr + (entry - parent->entries) * 8;
+		amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
+	} else {
+		if (parent->bo->shadow) {
+			pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
+			pde = pd_addr + (entry - parent->entries) * 8;
+			amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
+		}
+		pd_addr = amdgpu_bo_gpu_offset(parent->bo);
+		pde = pd_addr + (entry - parent->entries) * 8;
+		amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
+	}
+
+	return 0;
 }
 
 /**
@@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
 	struct amdgpu_bo *pt;
 	unsigned nptes;
 	bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
-
+	int r;
 
 	/* walk over the address space and update the page tables */
-	for (addr = start; addr < end; addr += nptes) {
-		pt = amdgpu_vm_get_pt(params, addr);
-		if (!pt) {
-			pr_err("PT not found, aborting update_ptes\n");
-			return -EINVAL;
-		}
+	for (addr = start; addr < end; addr += nptes,
+	     dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
+		struct amdgpu_vm_pt *entry, *parent;
+
+		amdgpu_vm_get_entry(params, addr, &entry, &parent);
+		if (!entry)
+			return -ENOENT;
 
 		if ((addr & ~mask) == (end & ~mask))
 			nptes = end - addr;
 		else
 			nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
 
+		r = amdgpu_vm_handle_huge_pages(params, entry, parent,
+						nptes, dst, flags);
+		if (r)
+			return r;
 
+		if (entry->huge_page)
+			continue;
+
+		pt = entry->bo;
 		if (use_cpu_update) {
 			pe_start = (unsigned long)pt->kptr;
 		} else {
@@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
 		pe_start += (addr & mask) * 8;
 		params->func(params, pe_start, dst, nptes,
 			     AMDGPU_GPU_PAGE_SIZE, flags);
-
-		dst += nptes * AMDGPU_GPU_PAGE_SIZE;
 	}
 
 	return 0;
@@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 	/* padding, etc. */
 	ndw = 64;
 
+	/* one PDE write for each huge page */
+	ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
+
 	if (src) {
 		/* only copy commands needed */
 		ndw += ncmds * 7;
@@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 
 error_free:
 	amdgpu_job_free(job);
+	amdgpu_vm_invalidate_level(&vm->root);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index c4f5d1f..34d9174 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
 /* TILED for VEGA10, reserved for older ASICs  */
 #define AMDGPU_PTE_PRT		(1ULL << 51)
 
+/* PDE is handled as PTE for VEGA10 */
+#define AMDGPU_PDE_PTE		(1ULL << 54)
+
 /* VEGA10 only */
 #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
 #define AMDGPU_PTE_MTYPE_MASK	AMDGPU_PTE_MTYPE(3ULL)
@@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
 struct amdgpu_vm_pt {
 	struct amdgpu_bo	*bo;
 	uint64_t		addr;
+	bool			huge_page;
 
 	/* array of page tables, one for each directory entry */
 	struct amdgpu_vm_pt	*entries;
-- 
2.7.4

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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[StDenis, Tom]

In amdgpu_vm_get_entry() if the bo size is less than 8 you'll get a divide by zero.  Are there mechanisms to prevent this?  Maybe add a BUG() there?

Tom
________________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <deathsimple@vodafone.de>
Sent: Monday, July 17, 2017 17:02
To: amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix
Subject: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

From: Christian König <christian.koenig@amd.com>

The hardware can use huge pages to map 2MB of address space with only one PDE.

v2: few cleanups and rebased
v3: skip PT updates if we are using the PDE
v4: rebased, added support for CPU based updates

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119 +++++++++++++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
 2 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a3dbebe..62d97f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,

                        entry->bo = pt;
                        entry->addr = 0;
+                       entry->huge_page = false;
                }

                if (level < adev->vm_manager.num_level) {
@@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,

                pt = amdgpu_bo_gpu_offset(bo);
                pt = amdgpu_gart_get_vm_pde(adev, pt);
-               if (parent->entries[pt_idx].addr == pt)
+               if (parent->entries[pt_idx].addr == pt ||
+                   parent->entries[pt_idx].huge_page)
                        continue;

                parent->entries[pt_idx].addr = pt;
@@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
 }

 /**
- * amdgpu_vm_find_pt - find the page table for an address
+ * amdgpu_vm_find_entry - find the entry for an address
  *
  * @p: see amdgpu_pte_update_params definition
  * @addr: virtual address in question
+ * @entry: resulting entry or NULL
+ * @parent: parent entry
  *
- * Find the page table BO for a virtual address, return NULL when none found.
+ * Find the vm_pt entry and it's parent for the given address.
  */
-static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
-                                         uint64_t addr)
+void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p, uint64_t addr,
+                        struct amdgpu_vm_pt **entry,
+                        struct amdgpu_vm_pt **parent)
 {
-       struct amdgpu_vm_pt *entry = &p->vm->root;
        unsigned idx, level = p->adev->vm_manager.num_level;

-       while (entry->entries) {
+       *parent = NULL;
+       *entry = &p->vm->root;
+       while ((*entry)->entries) {
                idx = addr >> (p->adev->vm_manager.block_size * level--);
-               idx %= amdgpu_bo_size(entry->bo) / 8;
-               entry = &entry->entries[idx];
+               idx %= amdgpu_bo_size((*entry)->bo) / 8;
+               *parent = *entry;
+               *entry = &(*entry)->entries[idx];
        }

        if (level)
-               return NULL;
+               *entry = NULL;
+}
+
+/**
+ * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
+ *
+ * @p: see amdgpu_pte_update_params definition
+ * @entry: vm_pt entry to check
+ * @parent: parent entry
+ * @nptes: number of PTEs updated with this operation
+ * @dst: destination address where the PTEs should point to
+ * @flags: access flags fro the PTEs
+ *
+ * Check if we can update the PD with a huge page.
+ */
+static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
+                                      struct amdgpu_vm_pt *entry,
+                                      struct amdgpu_vm_pt *parent,
+                                      unsigned nptes, uint64_t dst,
+                                      uint64_t flags)
+{
+       bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
+       uint64_t pd_addr, pde;
+       int r;

-       return entry->bo;
+       /* In the case of a mixed PT the PDE must point to it*/
+       if (p->adev->asic_type < CHIP_VEGA10 ||
+           nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
+           p->func != amdgpu_vm_do_set_ptes ||
+           !(flags & AMDGPU_PTE_VALID)) {
+
+               dst = amdgpu_bo_gpu_offset(entry->bo);
+               dst = amdgpu_gart_get_vm_pde(p->adev, dst);
+               flags = AMDGPU_PTE_VALID;
+       } else {
+               flags |= AMDGPU_PDE_PTE;
+       }
+
+       if (entry->addr == dst &&
+           entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
+               return 0;
+
+       entry->addr = dst;
+       entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
+
+       if (use_cpu_update) {
+               r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
+               if (r)
+                       return r;
+
+               pde = pd_addr + (entry - parent->entries) * 8;
+               amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
+       } else {
+               if (parent->bo->shadow) {
+                       pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
+                       pde = pd_addr + (entry - parent->entries) * 8;
+                       amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
+               }
+               pd_addr = amdgpu_bo_gpu_offset(parent->bo);
+               pde = pd_addr + (entry - parent->entries) * 8;
+               amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
+       }
+
+       return 0;
 }

 /**
@@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
        struct amdgpu_bo *pt;
        unsigned nptes;
        bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
-
+       int r;

        /* walk over the address space and update the page tables */
-       for (addr = start; addr < end; addr += nptes) {
-               pt = amdgpu_vm_get_pt(params, addr);
-               if (!pt) {
-                       pr_err("PT not found, aborting update_ptes\n");
-                       return -EINVAL;
-               }
+       for (addr = start; addr < end; addr += nptes,
+            dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
+               struct amdgpu_vm_pt *entry, *parent;
+
+               amdgpu_vm_get_entry(params, addr, &entry, &parent);
+               if (!entry)
+                       return -ENOENT;

                if ((addr & ~mask) == (end & ~mask))
                        nptes = end - addr;
                else
                        nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);

+               r = amdgpu_vm_handle_huge_pages(params, entry, parent,
+                                               nptes, dst, flags);
+               if (r)
+                       return r;

+               if (entry->huge_page)
+                       continue;
+
+               pt = entry->bo;
                if (use_cpu_update) {
                        pe_start = (unsigned long)pt->kptr;
                } else {
@@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
                pe_start += (addr & mask) * 8;
                params->func(params, pe_start, dst, nptes,
                             AMDGPU_GPU_PAGE_SIZE, flags);
-
-               dst += nptes * AMDGPU_GPU_PAGE_SIZE;
        }

        return 0;
@@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
        /* padding, etc. */
        ndw = 64;

+       /* one PDE write for each huge page */
+       ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
+
        if (src) {
                /* only copy commands needed */
                ndw += ncmds * 7;
@@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,

 error_free:
        amdgpu_job_free(job);
+       amdgpu_vm_invalidate_level(&vm->root);
        return r;
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index c4f5d1f..34d9174 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
 /* TILED for VEGA10, reserved for older ASICs  */
 #define AMDGPU_PTE_PRT         (1ULL << 51)

+/* PDE is handled as PTE for VEGA10 */
+#define AMDGPU_PDE_PTE         (1ULL << 54)
+
 /* VEGA10 only */
 #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
 #define AMDGPU_PTE_MTYPE_MASK  AMDGPU_PTE_MTYPE(3ULL)
@@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
 struct amdgpu_vm_pt {
        struct amdgpu_bo        *bo;
        uint64_t                addr;
+       bool                    huge_page;

        /* array of page tables, one for each directory entry */
        struct amdgpu_vm_pt     *entries;
--
2.7.4

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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Christian König]

The minimum BO size is 4K, so that can never happen.

Christian.

Am 17.07.2017 um 23:21 schrieb StDenis, Tom:
> In amdgpu_vm_get_entry() if the bo size is less than 8 you'll get a divide by zero.  Are there mechanisms to prevent this?  Maybe add a BUG() there?
>
> Tom
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <deathsimple@vodafone.de>
> Sent: Monday, July 17, 2017 17:02
> To: amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix
> Subject: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4
>
> From: Christian König <christian.koenig@amd.com>
>
> The hardware can use huge pages to map 2MB of address space with only one PDE.
>
> v2: few cleanups and rebased
> v3: skip PT updates if we are using the PDE
> v4: rebased, added support for CPU based updates
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119 +++++++++++++++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
>   2 files changed, 103 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a3dbebe..62d97f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>
>                          entry->bo = pt;
>                          entry->addr = 0;
> +                       entry->huge_page = false;
>                  }
>
>                  if (level < adev->vm_manager.num_level) {
> @@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>
>                  pt = amdgpu_bo_gpu_offset(bo);
>                  pt = amdgpu_gart_get_vm_pde(adev, pt);
> -               if (parent->entries[pt_idx].addr == pt)
> +               if (parent->entries[pt_idx].addr == pt ||
> +                   parent->entries[pt_idx].huge_page)
>                          continue;
>
>                  parent->entries[pt_idx].addr = pt;
> @@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>   }
>
>   /**
> - * amdgpu_vm_find_pt - find the page table for an address
> + * amdgpu_vm_find_entry - find the entry for an address
>    *
>    * @p: see amdgpu_pte_update_params definition
>    * @addr: virtual address in question
> + * @entry: resulting entry or NULL
> + * @parent: parent entry
>    *
> - * Find the page table BO for a virtual address, return NULL when none found.
> + * Find the vm_pt entry and it's parent for the given address.
>    */
> -static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
> -                                         uint64_t addr)
> +void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p, uint64_t addr,
> +                        struct amdgpu_vm_pt **entry,
> +                        struct amdgpu_vm_pt **parent)
>   {
> -       struct amdgpu_vm_pt *entry = &p->vm->root;
>          unsigned idx, level = p->adev->vm_manager.num_level;
>
> -       while (entry->entries) {
> +       *parent = NULL;
> +       *entry = &p->vm->root;
> +       while ((*entry)->entries) {
>                  idx = addr >> (p->adev->vm_manager.block_size * level--);
> -               idx %= amdgpu_bo_size(entry->bo) / 8;
> -               entry = &entry->entries[idx];
> +               idx %= amdgpu_bo_size((*entry)->bo) / 8;
> +               *parent = *entry;
> +               *entry = &(*entry)->entries[idx];
>          }
>
>          if (level)
> -               return NULL;
> +               *entry = NULL;
> +}
> +
> +/**
> + * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
> + *
> + * @p: see amdgpu_pte_update_params definition
> + * @entry: vm_pt entry to check
> + * @parent: parent entry
> + * @nptes: number of PTEs updated with this operation
> + * @dst: destination address where the PTEs should point to
> + * @flags: access flags fro the PTEs
> + *
> + * Check if we can update the PD with a huge page.
> + */
> +static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
> +                                      struct amdgpu_vm_pt *entry,
> +                                      struct amdgpu_vm_pt *parent,
> +                                      unsigned nptes, uint64_t dst,
> +                                      uint64_t flags)
> +{
> +       bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
> +       uint64_t pd_addr, pde;
> +       int r;
>
> -       return entry->bo;
> +       /* In the case of a mixed PT the PDE must point to it*/
> +       if (p->adev->asic_type < CHIP_VEGA10 ||
> +           nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
> +           p->func != amdgpu_vm_do_set_ptes ||
> +           !(flags & AMDGPU_PTE_VALID)) {
> +
> +               dst = amdgpu_bo_gpu_offset(entry->bo);
> +               dst = amdgpu_gart_get_vm_pde(p->adev, dst);
> +               flags = AMDGPU_PTE_VALID;
> +       } else {
> +               flags |= AMDGPU_PDE_PTE;
> +       }
> +
> +       if (entry->addr == dst &&
> +           entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
> +               return 0;
> +
> +       entry->addr = dst;
> +       entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
> +
> +       if (use_cpu_update) {
> +               r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
> +               if (r)
> +                       return r;
> +
> +               pde = pd_addr + (entry - parent->entries) * 8;
> +               amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
> +       } else {
> +               if (parent->bo->shadow) {
> +                       pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
> +                       pde = pd_addr + (entry - parent->entries) * 8;
> +                       amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
> +               }
> +               pd_addr = amdgpu_bo_gpu_offset(parent->bo);
> +               pde = pd_addr + (entry - parent->entries) * 8;
> +               amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
> +       }
> +
> +       return 0;
>   }
>
>   /**
> @@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>          struct amdgpu_bo *pt;
>          unsigned nptes;
>          bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
> -
> +       int r;
>
>          /* walk over the address space and update the page tables */
> -       for (addr = start; addr < end; addr += nptes) {
> -               pt = amdgpu_vm_get_pt(params, addr);
> -               if (!pt) {
> -                       pr_err("PT not found, aborting update_ptes\n");
> -                       return -EINVAL;
> -               }
> +       for (addr = start; addr < end; addr += nptes,
> +            dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
> +               struct amdgpu_vm_pt *entry, *parent;
> +
> +               amdgpu_vm_get_entry(params, addr, &entry, &parent);
> +               if (!entry)
> +                       return -ENOENT;
>
>                  if ((addr & ~mask) == (end & ~mask))
>                          nptes = end - addr;
>                  else
>                          nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
>
> +               r = amdgpu_vm_handle_huge_pages(params, entry, parent,
> +                                               nptes, dst, flags);
> +               if (r)
> +                       return r;
>
> +               if (entry->huge_page)
> +                       continue;
> +
> +               pt = entry->bo;
>                  if (use_cpu_update) {
>                          pe_start = (unsigned long)pt->kptr;
>                  } else {
> @@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>                  pe_start += (addr & mask) * 8;
>                  params->func(params, pe_start, dst, nptes,
>                               AMDGPU_GPU_PAGE_SIZE, flags);
> -
> -               dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>          }
>
>          return 0;
> @@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>          /* padding, etc. */
>          ndw = 64;
>
> +       /* one PDE write for each huge page */
> +       ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
> +
>          if (src) {
>                  /* only copy commands needed */
>                  ndw += ncmds * 7;
> @@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>
>   error_free:
>          amdgpu_job_free(job);
> +       amdgpu_vm_invalidate_level(&vm->root);
>          return r;
>   }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index c4f5d1f..34d9174 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
>   /* TILED for VEGA10, reserved for older ASICs  */
>   #define AMDGPU_PTE_PRT         (1ULL << 51)
>
> +/* PDE is handled as PTE for VEGA10 */
> +#define AMDGPU_PDE_PTE         (1ULL << 54)
> +
>   /* VEGA10 only */
>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>   #define AMDGPU_PTE_MTYPE_MASK  AMDGPU_PTE_MTYPE(3ULL)
> @@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
>   struct amdgpu_vm_pt {
>          struct amdgpu_bo        *bo;
>          uint64_t                addr;
> +       bool                    huge_page;
>
>          /* array of page tables, one for each directory entry */
>          struct amdgpu_vm_pt     *entries;
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Felix Kuehling]

On 17-07-17 05:02 PM, Christian König wrote:
> +	if (p->adev->asic_type < CHIP_VEGA10 ||
> +	    nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
> +	    p->func != amdgpu_vm_do_set_ptes ||
> +	    !(flags & AMDGPU_PTE_VALID)) {

Because of this condition, I think this still won't work correctly for
cpu page table updates. p->func will be amdgpu_vm_cpu_set_ptes.

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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Christian König]

Am 17.07.2017 um 23:30 schrieb Felix Kuehling:
> On 17-07-17 05:02 PM, Christian König wrote:
>> +	if (p->adev->asic_type < CHIP_VEGA10 ||
>> +	    nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
>> +	    p->func != amdgpu_vm_do_set_ptes ||
>> +	    !(flags & AMDGPU_PTE_VALID)) {
> Because of this condition, I think this still won't work correctly for
> cpu page table updates. p->func will be amdgpu_vm_cpu_set_ptes.

Good point.

This is totally untested anyway, because of lack of hardware access at 
the moment.

Just wanted to point you to the bits I've changed for testing it.

Christian.

>
> Regards,
>    Felix
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[zhoucm1]

Still holding on? I thought this patch was pushed in earlier with my RB.

Regards,
David Zhou
On 2017年07月18日 05:02, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> The hardware can use huge pages to map 2MB of address space with only one PDE.
>
> v2: few cleanups and rebased
> v3: skip PT updates if we are using the PDE
> v4: rebased, added support for CPU based updates
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119 +++++++++++++++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
>   2 files changed, 103 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a3dbebe..62d97f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   
>   			entry->bo = pt;
>   			entry->addr = 0;
> +			entry->huge_page = false;
>   		}
>   
>   		if (level < adev->vm_manager.num_level) {
> @@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>   
>   		pt = amdgpu_bo_gpu_offset(bo);
>   		pt = amdgpu_gart_get_vm_pde(adev, pt);
> -		if (parent->entries[pt_idx].addr == pt)
> +		if (parent->entries[pt_idx].addr == pt ||
> +		    parent->entries[pt_idx].huge_page)
>   			continue;
>   
>   		parent->entries[pt_idx].addr = pt;
> @@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>   }
>   
>   /**
> - * amdgpu_vm_find_pt - find the page table for an address
> + * amdgpu_vm_find_entry - find the entry for an address
>    *
>    * @p: see amdgpu_pte_update_params definition
>    * @addr: virtual address in question
> + * @entry: resulting entry or NULL
> + * @parent: parent entry
>    *
> - * Find the page table BO for a virtual address, return NULL when none found.
> + * Find the vm_pt entry and it's parent for the given address.
>    */
> -static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
> -					  uint64_t addr)
> +void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p, uint64_t addr,
> +			 struct amdgpu_vm_pt **entry,
> +			 struct amdgpu_vm_pt **parent)
>   {
> -	struct amdgpu_vm_pt *entry = &p->vm->root;
>   	unsigned idx, level = p->adev->vm_manager.num_level;
>   
> -	while (entry->entries) {
> +	*parent = NULL;
> +	*entry = &p->vm->root;
> +	while ((*entry)->entries) {
>   		idx = addr >> (p->adev->vm_manager.block_size * level--);
> -		idx %= amdgpu_bo_size(entry->bo) / 8;
> -		entry = &entry->entries[idx];
> +		idx %= amdgpu_bo_size((*entry)->bo) / 8;
> +		*parent = *entry;
> +		*entry = &(*entry)->entries[idx];
>   	}
>   
>   	if (level)
> -		return NULL;
> +		*entry = NULL;
> +}
> +
> +/**
> + * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
> + *
> + * @p: see amdgpu_pte_update_params definition
> + * @entry: vm_pt entry to check
> + * @parent: parent entry
> + * @nptes: number of PTEs updated with this operation
> + * @dst: destination address where the PTEs should point to
> + * @flags: access flags fro the PTEs
> + *
> + * Check if we can update the PD with a huge page.
> + */
> +static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
> +				       struct amdgpu_vm_pt *entry,
> +				       struct amdgpu_vm_pt *parent,
> +				       unsigned nptes, uint64_t dst,
> +				       uint64_t flags)
> +{
> +	bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
> +	uint64_t pd_addr, pde;
> +	int r;
>   
> -	return entry->bo;
> +	/* In the case of a mixed PT the PDE must point to it*/
> +	if (p->adev->asic_type < CHIP_VEGA10 ||
> +	    nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
> +	    p->func != amdgpu_vm_do_set_ptes ||
> +	    !(flags & AMDGPU_PTE_VALID)) {
> +
> +		dst = amdgpu_bo_gpu_offset(entry->bo);
> +		dst = amdgpu_gart_get_vm_pde(p->adev, dst);
> +		flags = AMDGPU_PTE_VALID;
> +	} else {
> +		flags |= AMDGPU_PDE_PTE;
> +	}
> +
> +	if (entry->addr == dst &&
> +	    entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
> +		return 0;
> +
> +	entry->addr = dst;
> +	entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
> +
> +	if (use_cpu_update) {
> +		r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
> +		if (r)
> +			return r;
> +
> +		pde = pd_addr + (entry - parent->entries) * 8;
> +		amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
> +	} else {
> +		if (parent->bo->shadow) {
> +			pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
> +			pde = pd_addr + (entry - parent->entries) * 8;
> +			amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
> +		}
> +		pd_addr = amdgpu_bo_gpu_offset(parent->bo);
> +		pde = pd_addr + (entry - parent->entries) * 8;
> +		amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
> +	}
> +
> +	return 0;
>   }
>   
>   /**
> @@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>   	struct amdgpu_bo *pt;
>   	unsigned nptes;
>   	bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
> -
> +	int r;
>   
>   	/* walk over the address space and update the page tables */
> -	for (addr = start; addr < end; addr += nptes) {
> -		pt = amdgpu_vm_get_pt(params, addr);
> -		if (!pt) {
> -			pr_err("PT not found, aborting update_ptes\n");
> -			return -EINVAL;
> -		}
> +	for (addr = start; addr < end; addr += nptes,
> +	     dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
> +		struct amdgpu_vm_pt *entry, *parent;
> +
> +		amdgpu_vm_get_entry(params, addr, &entry, &parent);
> +		if (!entry)
> +			return -ENOENT;
>   
>   		if ((addr & ~mask) == (end & ~mask))
>   			nptes = end - addr;
>   		else
>   			nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
>   
> +		r = amdgpu_vm_handle_huge_pages(params, entry, parent,
> +						nptes, dst, flags);
> +		if (r)
> +			return r;
>   
> +		if (entry->huge_page)
> +			continue;
> +
> +		pt = entry->bo;
>   		if (use_cpu_update) {
>   			pe_start = (unsigned long)pt->kptr;
>   		} else {
> @@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>   		pe_start += (addr & mask) * 8;
>   		params->func(params, pe_start, dst, nptes,
>   			     AMDGPU_GPU_PAGE_SIZE, flags);
> -
> -		dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>   	}
>   
>   	return 0;
> @@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   	/* padding, etc. */
>   	ndw = 64;
>   
> +	/* one PDE write for each huge page */
> +	ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
> +
>   	if (src) {
>   		/* only copy commands needed */
>   		ndw += ncmds * 7;
> @@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   
>   error_free:
>   	amdgpu_job_free(job);
> +	amdgpu_vm_invalidate_level(&vm->root);
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index c4f5d1f..34d9174 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
>   /* TILED for VEGA10, reserved for older ASICs  */
>   #define AMDGPU_PTE_PRT		(1ULL << 51)
>   
> +/* PDE is handled as PTE for VEGA10 */
> +#define AMDGPU_PDE_PTE		(1ULL << 54)
> +
>   /* VEGA10 only */
>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>   #define AMDGPU_PTE_MTYPE_MASK	AMDGPU_PTE_MTYPE(3ULL)
> @@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
>   struct amdgpu_vm_pt {
>   	struct amdgpu_bo	*bo;
>   	uint64_t		addr;
> +	bool			huge_page;
>   
>   	/* array of page tables, one for each directory entry */
>   	struct amdgpu_vm_pt	*entries;

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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Christian König]

Yeah, I mean it looks good from the software side but we still don't see 
the hardware react as it should.

It doesn't seem to hurt anything, so I'm torn apart between pushing it 
and completely fixing it later on or wait till we have figured 
everything out.

Felix what is your opinion on that?

Regards,
Christian.

Am 18.07.2017 um 04:24 schrieb zhoucm1:
> Still holding on? I thought this patch was pushed in earlier with my RB.
>
> Regards,
> David Zhou
> On 2017年07月18日 05:02, Christian König wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> The hardware can use huge pages to map 2MB of address space with only 
>> one PDE.
>>
>> v2: few cleanups and rebased
>> v3: skip PT updates if we are using the PDE
>> v4: rebased, added support for CPU based updates
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119 
>> +++++++++++++++++++++++++++------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
>>   2 files changed, 103 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index a3dbebe..62d97f5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct 
>> amdgpu_device *adev,
>>                 entry->bo = pt;
>>               entry->addr = 0;
>> +            entry->huge_page = false;
>>           }
>>             if (level < adev->vm_manager.num_level) {
>> @@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct 
>> amdgpu_device *adev,
>>             pt = amdgpu_bo_gpu_offset(bo);
>>           pt = amdgpu_gart_get_vm_pde(adev, pt);
>> -        if (parent->entries[pt_idx].addr == pt)
>> +        if (parent->entries[pt_idx].addr == pt ||
>> +            parent->entries[pt_idx].huge_page)
>>               continue;
>>             parent->entries[pt_idx].addr = pt;
>> @@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct 
>> amdgpu_device *adev,
>>   }
>>     /**
>> - * amdgpu_vm_find_pt - find the page table for an address
>> + * amdgpu_vm_find_entry - find the entry for an address
>>    *
>>    * @p: see amdgpu_pte_update_params definition
>>    * @addr: virtual address in question
>> + * @entry: resulting entry or NULL
>> + * @parent: parent entry
>>    *
>> - * Find the page table BO for a virtual address, return NULL when 
>> none found.
>> + * Find the vm_pt entry and it's parent for the given address.
>>    */
>> -static struct amdgpu_bo *amdgpu_vm_get_pt(struct 
>> amdgpu_pte_update_params *p,
>> -                      uint64_t addr)
>> +void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p, 
>> uint64_t addr,
>> +             struct amdgpu_vm_pt **entry,
>> +             struct amdgpu_vm_pt **parent)
>>   {
>> -    struct amdgpu_vm_pt *entry = &p->vm->root;
>>       unsigned idx, level = p->adev->vm_manager.num_level;
>>   -    while (entry->entries) {
>> +    *parent = NULL;
>> +    *entry = &p->vm->root;
>> +    while ((*entry)->entries) {
>>           idx = addr >> (p->adev->vm_manager.block_size * level--);
>> -        idx %= amdgpu_bo_size(entry->bo) / 8;
>> -        entry = &entry->entries[idx];
>> +        idx %= amdgpu_bo_size((*entry)->bo) / 8;
>> +        *parent = *entry;
>> +        *entry = &(*entry)->entries[idx];
>>       }
>>         if (level)
>> -        return NULL;
>> +        *entry = NULL;
>> +}
>> +
>> +/**
>> + * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
>> + *
>> + * @p: see amdgpu_pte_update_params definition
>> + * @entry: vm_pt entry to check
>> + * @parent: parent entry
>> + * @nptes: number of PTEs updated with this operation
>> + * @dst: destination address where the PTEs should point to
>> + * @flags: access flags fro the PTEs
>> + *
>> + * Check if we can update the PD with a huge page.
>> + */
>> +static int amdgpu_vm_handle_huge_pages(struct 
>> amdgpu_pte_update_params *p,
>> +                       struct amdgpu_vm_pt *entry,
>> +                       struct amdgpu_vm_pt *parent,
>> +                       unsigned nptes, uint64_t dst,
>> +                       uint64_t flags)
>> +{
>> +    bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
>> +    uint64_t pd_addr, pde;
>> +    int r;
>>   -    return entry->bo;
>> +    /* In the case of a mixed PT the PDE must point to it*/
>> +    if (p->adev->asic_type < CHIP_VEGA10 ||
>> +        nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
>> +        p->func != amdgpu_vm_do_set_ptes ||
>> +        !(flags & AMDGPU_PTE_VALID)) {
>> +
>> +        dst = amdgpu_bo_gpu_offset(entry->bo);
>> +        dst = amdgpu_gart_get_vm_pde(p->adev, dst);
>> +        flags = AMDGPU_PTE_VALID;
>> +    } else {
>> +        flags |= AMDGPU_PDE_PTE;
>> +    }
>> +
>> +    if (entry->addr == dst &&
>> +        entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
>> +        return 0;
>> +
>> +    entry->addr = dst;
>> +    entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
>> +
>> +    if (use_cpu_update) {
>> +        r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
>> +        if (r)
>> +            return r;
>> +
>> +        pde = pd_addr + (entry - parent->entries) * 8;
>> +        amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
>> +    } else {
>> +        if (parent->bo->shadow) {
>> +            pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
>> +            pde = pd_addr + (entry - parent->entries) * 8;
>> +            amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>> +        }
>> +        pd_addr = amdgpu_bo_gpu_offset(parent->bo);
>> +        pde = pd_addr + (entry - parent->entries) * 8;
>> +        amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>> +    }
>> +
>> +    return 0;
>>   }
>>     /**
>> @@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct 
>> amdgpu_pte_update_params *params,
>>       struct amdgpu_bo *pt;
>>       unsigned nptes;
>>       bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
>> -
>> +    int r;
>>         /* walk over the address space and update the page tables */
>> -    for (addr = start; addr < end; addr += nptes) {
>> -        pt = amdgpu_vm_get_pt(params, addr);
>> -        if (!pt) {
>> -            pr_err("PT not found, aborting update_ptes\n");
>> -            return -EINVAL;
>> -        }
>> +    for (addr = start; addr < end; addr += nptes,
>> +         dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
>> +        struct amdgpu_vm_pt *entry, *parent;
>> +
>> +        amdgpu_vm_get_entry(params, addr, &entry, &parent);
>> +        if (!entry)
>> +            return -ENOENT;
>>             if ((addr & ~mask) == (end & ~mask))
>>               nptes = end - addr;
>>           else
>>               nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
>>   +        r = amdgpu_vm_handle_huge_pages(params, entry, parent,
>> +                        nptes, dst, flags);
>> +        if (r)
>> +            return r;
>>   +        if (entry->huge_page)
>> +            continue;
>> +
>> +        pt = entry->bo;
>>           if (use_cpu_update) {
>>               pe_start = (unsigned long)pt->kptr;
>>           } else {
>> @@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct 
>> amdgpu_pte_update_params *params,
>>           pe_start += (addr & mask) * 8;
>>           params->func(params, pe_start, dst, nptes,
>>                    AMDGPU_GPU_PAGE_SIZE, flags);
>> -
>> -        dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>>       }
>>         return 0;
>> @@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct 
>> amdgpu_device *adev,
>>       /* padding, etc. */
>>       ndw = 64;
>>   +    /* one PDE write for each huge page */
>> +    ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
>> +
>>       if (src) {
>>           /* only copy commands needed */
>>           ndw += ncmds * 7;
>> @@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
>> amdgpu_device *adev,
>>     error_free:
>>       amdgpu_job_free(job);
>> +    amdgpu_vm_invalidate_level(&vm->root);
>>       return r;
>>   }
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index c4f5d1f..34d9174 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
>>   /* TILED for VEGA10, reserved for older ASICs  */
>>   #define AMDGPU_PTE_PRT        (1ULL << 51)
>>   +/* PDE is handled as PTE for VEGA10 */
>> +#define AMDGPU_PDE_PTE        (1ULL << 54)
>> +
>>   /* VEGA10 only */
>>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>>   #define AMDGPU_PTE_MTYPE_MASK    AMDGPU_PTE_MTYPE(3ULL)
>> @@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
>>   struct amdgpu_vm_pt {
>>       struct amdgpu_bo    *bo;
>>       uint64_t        addr;
>> +    bool            huge_page;
>>         /* array of page tables, one for each directory entry */
>>       struct amdgpu_vm_pt    *entries;
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Felix Kuehling]

If you submit the code, it allows more people to experiment with it.

Regards,
  Felix


On 17-07-18 09:54 AM, Christian König wrote:
> Yeah, I mean it looks good from the software side but we still don't
> see the hardware react as it should.
>
> It doesn't seem to hurt anything, so I'm torn apart between pushing it
> and completely fixing it later on or wait till we have figured
> everything out.
>
> Felix what is your opinion on that?
>
> Regards,
> Christian.
>
> Am 18.07.2017 um 04:24 schrieb zhoucm1:
>> Still holding on? I thought this patch was pushed in earlier with my RB.
>>
>> Regards,
>> David Zhou
>> On 2017年07月18日 05:02, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> The hardware can use huge pages to map 2MB of address space with
>>> only one PDE.
>>>
>>> v2: few cleanups and rebased
>>> v3: skip PT updates if we are using the PDE
>>> v4: rebased, added support for CPU based updates
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119
>>> +++++++++++++++++++++++++++------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
>>>   2 files changed, 103 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index a3dbebe..62d97f5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>>                 entry->bo = pt;
>>>               entry->addr = 0;
>>> +            entry->huge_page = false;
>>>           }
>>>             if (level < adev->vm_manager.num_level) {
>>> @@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct
>>> amdgpu_device *adev,
>>>             pt = amdgpu_bo_gpu_offset(bo);
>>>           pt = amdgpu_gart_get_vm_pde(adev, pt);
>>> -        if (parent->entries[pt_idx].addr == pt)
>>> +        if (parent->entries[pt_idx].addr == pt ||
>>> +            parent->entries[pt_idx].huge_page)
>>>               continue;
>>>             parent->entries[pt_idx].addr = pt;
>>> @@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct
>>> amdgpu_device *adev,
>>>   }
>>>     /**
>>> - * amdgpu_vm_find_pt - find the page table for an address
>>> + * amdgpu_vm_find_entry - find the entry for an address
>>>    *
>>>    * @p: see amdgpu_pte_update_params definition
>>>    * @addr: virtual address in question
>>> + * @entry: resulting entry or NULL
>>> + * @parent: parent entry
>>>    *
>>> - * Find the page table BO for a virtual address, return NULL when
>>> none found.
>>> + * Find the vm_pt entry and it's parent for the given address.
>>>    */
>>> -static struct amdgpu_bo *amdgpu_vm_get_pt(struct
>>> amdgpu_pte_update_params *p,
>>> -                      uint64_t addr)
>>> +void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p,
>>> uint64_t addr,
>>> +             struct amdgpu_vm_pt **entry,
>>> +             struct amdgpu_vm_pt **parent)
>>>   {
>>> -    struct amdgpu_vm_pt *entry = &p->vm->root;
>>>       unsigned idx, level = p->adev->vm_manager.num_level;
>>>   -    while (entry->entries) {
>>> +    *parent = NULL;
>>> +    *entry = &p->vm->root;
>>> +    while ((*entry)->entries) {
>>>           idx = addr >> (p->adev->vm_manager.block_size * level--);
>>> -        idx %= amdgpu_bo_size(entry->bo) / 8;
>>> -        entry = &entry->entries[idx];
>>> +        idx %= amdgpu_bo_size((*entry)->bo) / 8;
>>> +        *parent = *entry;
>>> +        *entry = &(*entry)->entries[idx];
>>>       }
>>>         if (level)
>>> -        return NULL;
>>> +        *entry = NULL;
>>> +}
>>> +
>>> +/**
>>> + * amdgpu_vm_handle_huge_pages - handle updating the PD with huge
>>> pages
>>> + *
>>> + * @p: see amdgpu_pte_update_params definition
>>> + * @entry: vm_pt entry to check
>>> + * @parent: parent entry
>>> + * @nptes: number of PTEs updated with this operation
>>> + * @dst: destination address where the PTEs should point to
>>> + * @flags: access flags fro the PTEs
>>> + *
>>> + * Check if we can update the PD with a huge page.
>>> + */
>>> +static int amdgpu_vm_handle_huge_pages(struct
>>> amdgpu_pte_update_params *p,
>>> +                       struct amdgpu_vm_pt *entry,
>>> +                       struct amdgpu_vm_pt *parent,
>>> +                       unsigned nptes, uint64_t dst,
>>> +                       uint64_t flags)
>>> +{
>>> +    bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
>>> +    uint64_t pd_addr, pde;
>>> +    int r;
>>>   -    return entry->bo;
>>> +    /* In the case of a mixed PT the PDE must point to it*/
>>> +    if (p->adev->asic_type < CHIP_VEGA10 ||
>>> +        nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
>>> +        p->func != amdgpu_vm_do_set_ptes ||
>>> +        !(flags & AMDGPU_PTE_VALID)) {
>>> +
>>> +        dst = amdgpu_bo_gpu_offset(entry->bo);
>>> +        dst = amdgpu_gart_get_vm_pde(p->adev, dst);
>>> +        flags = AMDGPU_PTE_VALID;
>>> +    } else {
>>> +        flags |= AMDGPU_PDE_PTE;
>>> +    }
>>> +
>>> +    if (entry->addr == dst &&
>>> +        entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
>>> +        return 0;
>>> +
>>> +    entry->addr = dst;
>>> +    entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
>>> +
>>> +    if (use_cpu_update) {
>>> +        r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
>>> +        if (r)
>>> +            return r;
>>> +
>>> +        pde = pd_addr + (entry - parent->entries) * 8;
>>> +        amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
>>> +    } else {
>>> +        if (parent->bo->shadow) {
>>> +            pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
>>> +            pde = pd_addr + (entry - parent->entries) * 8;
>>> +            amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>>> +        }
>>> +        pd_addr = amdgpu_bo_gpu_offset(parent->bo);
>>> +        pde = pd_addr + (entry - parent->entries) * 8;
>>> +        amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>>> +    }
>>> +
>>> +    return 0;
>>>   }
>>>     /**
>>> @@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct
>>> amdgpu_pte_update_params *params,
>>>       struct amdgpu_bo *pt;
>>>       unsigned nptes;
>>>       bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
>>> -
>>> +    int r;
>>>         /* walk over the address space and update the page tables */
>>> -    for (addr = start; addr < end; addr += nptes) {
>>> -        pt = amdgpu_vm_get_pt(params, addr);
>>> -        if (!pt) {
>>> -            pr_err("PT not found, aborting update_ptes\n");
>>> -            return -EINVAL;
>>> -        }
>>> +    for (addr = start; addr < end; addr += nptes,
>>> +         dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
>>> +        struct amdgpu_vm_pt *entry, *parent;
>>> +
>>> +        amdgpu_vm_get_entry(params, addr, &entry, &parent);
>>> +        if (!entry)
>>> +            return -ENOENT;
>>>             if ((addr & ~mask) == (end & ~mask))
>>>               nptes = end - addr;
>>>           else
>>>               nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
>>>   +        r = amdgpu_vm_handle_huge_pages(params, entry, parent,
>>> +                        nptes, dst, flags);
>>> +        if (r)
>>> +            return r;
>>>   +        if (entry->huge_page)
>>> +            continue;
>>> +
>>> +        pt = entry->bo;
>>>           if (use_cpu_update) {
>>>               pe_start = (unsigned long)pt->kptr;
>>>           } else {
>>> @@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct
>>> amdgpu_pte_update_params *params,
>>>           pe_start += (addr & mask) * 8;
>>>           params->func(params, pe_start, dst, nptes,
>>>                    AMDGPU_GPU_PAGE_SIZE, flags);
>>> -
>>> -        dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>>>       }
>>>         return 0;
>>> @@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct
>>> amdgpu_device *adev,
>>>       /* padding, etc. */
>>>       ndw = 64;
>>>   +    /* one PDE write for each huge page */
>>> +    ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
>>> +
>>>       if (src) {
>>>           /* only copy commands needed */
>>>           ndw += ncmds * 7;
>>> @@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct
>>> amdgpu_device *adev,
>>>     error_free:
>>>       amdgpu_job_free(job);
>>> +    amdgpu_vm_invalidate_level(&vm->root);
>>>       return r;
>>>   }
>>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> index c4f5d1f..34d9174 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> @@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
>>>   /* TILED for VEGA10, reserved for older ASICs  */
>>>   #define AMDGPU_PTE_PRT        (1ULL << 51)
>>>   +/* PDE is handled as PTE for VEGA10 */
>>> +#define AMDGPU_PDE_PTE        (1ULL << 54)
>>> +
>>>   /* VEGA10 only */
>>>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>>>   #define AMDGPU_PTE_MTYPE_MASK    AMDGPU_PTE_MTYPE(3ULL)
>>> @@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
>>>   struct amdgpu_vm_pt {
>>>       struct amdgpu_bo    *bo;
>>>       uint64_t        addr;
>>> +    bool            huge_page;
>>>         /* array of page tables, one for each directory entry */
>>>       struct amdgpu_vm_pt    *entries;
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

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

Re: [PATCH 2/2] drm/amdgpu: enable huge page handling in the VM v4

[Christian König]

Ok, in this case let's commit it.

I've fixed the one liner regarding CPU based updates and send it to the 
list once more.

Please give me an rb or even tested-by on that version and then I'm 
going to commit it.

Thanks,
Christian.

Am 18.07.2017 um 20:56 schrieb Felix Kuehling:
> If you submit the code, it allows more people to experiment with it.
>
> Regards,
>    Felix
>
>
> On 17-07-18 09:54 AM, Christian König wrote:
>> Yeah, I mean it looks good from the software side but we still don't
>> see the hardware react as it should.
>>
>> It doesn't seem to hurt anything, so I'm torn apart between pushing it
>> and completely fixing it later on or wait till we have figured
>> everything out.
>>
>> Felix what is your opinion on that?
>>
>> Regards,
>> Christian.
>>
>> Am 18.07.2017 um 04:24 schrieb zhoucm1:
>>> Still holding on? I thought this patch was pushed in earlier with my RB.
>>>
>>> Regards,
>>> David Zhou
>>> On 2017年07月18日 05:02, Christian König wrote:
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> The hardware can use huge pages to map 2MB of address space with
>>>> only one PDE.
>>>>
>>>> v2: few cleanups and rebased
>>>> v3: skip PT updates if we are using the PDE
>>>> v4: rebased, added support for CPU based updates
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 119
>>>> +++++++++++++++++++++++++++------
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   4 ++
>>>>    2 files changed, 103 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index a3dbebe..62d97f5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -351,6 +351,7 @@ static int amdgpu_vm_alloc_levels(struct
>>>> amdgpu_device *adev,
>>>>                  entry->bo = pt;
>>>>                entry->addr = 0;
>>>> +            entry->huge_page = false;
>>>>            }
>>>>              if (level < adev->vm_manager.num_level) {
>>>> @@ -1116,7 +1117,8 @@ static int amdgpu_vm_update_level(struct
>>>> amdgpu_device *adev,
>>>>              pt = amdgpu_bo_gpu_offset(bo);
>>>>            pt = amdgpu_gart_get_vm_pde(adev, pt);
>>>> -        if (parent->entries[pt_idx].addr == pt)
>>>> +        if (parent->entries[pt_idx].addr == pt ||
>>>> +            parent->entries[pt_idx].huge_page)
>>>>                continue;
>>>>              parent->entries[pt_idx].addr = pt;
>>>> @@ -1257,29 +1259,95 @@ int amdgpu_vm_update_directories(struct
>>>> amdgpu_device *adev,
>>>>    }
>>>>      /**
>>>> - * amdgpu_vm_find_pt - find the page table for an address
>>>> + * amdgpu_vm_find_entry - find the entry for an address
>>>>     *
>>>>     * @p: see amdgpu_pte_update_params definition
>>>>     * @addr: virtual address in question
>>>> + * @entry: resulting entry or NULL
>>>> + * @parent: parent entry
>>>>     *
>>>> - * Find the page table BO for a virtual address, return NULL when
>>>> none found.
>>>> + * Find the vm_pt entry and it's parent for the given address.
>>>>     */
>>>> -static struct amdgpu_bo *amdgpu_vm_get_pt(struct
>>>> amdgpu_pte_update_params *p,
>>>> -                      uint64_t addr)
>>>> +void amdgpu_vm_get_entry(struct amdgpu_pte_update_params *p,
>>>> uint64_t addr,
>>>> +             struct amdgpu_vm_pt **entry,
>>>> +             struct amdgpu_vm_pt **parent)
>>>>    {
>>>> -    struct amdgpu_vm_pt *entry = &p->vm->root;
>>>>        unsigned idx, level = p->adev->vm_manager.num_level;
>>>>    -    while (entry->entries) {
>>>> +    *parent = NULL;
>>>> +    *entry = &p->vm->root;
>>>> +    while ((*entry)->entries) {
>>>>            idx = addr >> (p->adev->vm_manager.block_size * level--);
>>>> -        idx %= amdgpu_bo_size(entry->bo) / 8;
>>>> -        entry = &entry->entries[idx];
>>>> +        idx %= amdgpu_bo_size((*entry)->bo) / 8;
>>>> +        *parent = *entry;
>>>> +        *entry = &(*entry)->entries[idx];
>>>>        }
>>>>          if (level)
>>>> -        return NULL;
>>>> +        *entry = NULL;
>>>> +}
>>>> +
>>>> +/**
>>>> + * amdgpu_vm_handle_huge_pages - handle updating the PD with huge
>>>> pages
>>>> + *
>>>> + * @p: see amdgpu_pte_update_params definition
>>>> + * @entry: vm_pt entry to check
>>>> + * @parent: parent entry
>>>> + * @nptes: number of PTEs updated with this operation
>>>> + * @dst: destination address where the PTEs should point to
>>>> + * @flags: access flags fro the PTEs
>>>> + *
>>>> + * Check if we can update the PD with a huge page.
>>>> + */
>>>> +static int amdgpu_vm_handle_huge_pages(struct
>>>> amdgpu_pte_update_params *p,
>>>> +                       struct amdgpu_vm_pt *entry,
>>>> +                       struct amdgpu_vm_pt *parent,
>>>> +                       unsigned nptes, uint64_t dst,
>>>> +                       uint64_t flags)
>>>> +{
>>>> +    bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
>>>> +    uint64_t pd_addr, pde;
>>>> +    int r;
>>>>    -    return entry->bo;
>>>> +    /* In the case of a mixed PT the PDE must point to it*/
>>>> +    if (p->adev->asic_type < CHIP_VEGA10 ||
>>>> +        nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
>>>> +        p->func != amdgpu_vm_do_set_ptes ||
>>>> +        !(flags & AMDGPU_PTE_VALID)) {
>>>> +
>>>> +        dst = amdgpu_bo_gpu_offset(entry->bo);
>>>> +        dst = amdgpu_gart_get_vm_pde(p->adev, dst);
>>>> +        flags = AMDGPU_PTE_VALID;
>>>> +    } else {
>>>> +        flags |= AMDGPU_PDE_PTE;
>>>> +    }
>>>> +
>>>> +    if (entry->addr == dst &&
>>>> +        entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
>>>> +        return 0;
>>>> +
>>>> +    entry->addr = dst;
>>>> +    entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
>>>> +
>>>> +    if (use_cpu_update) {
>>>> +        r = amdgpu_bo_kmap(parent->bo, (void *)&pd_addr);
>>>> +        if (r)
>>>> +            return r;
>>>> +
>>>> +        pde = pd_addr + (entry - parent->entries) * 8;
>>>> +        amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
>>>> +    } else {
>>>> +        if (parent->bo->shadow) {
>>>> +            pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
>>>> +            pde = pd_addr + (entry - parent->entries) * 8;
>>>> +            amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>>>> +        }
>>>> +        pd_addr = amdgpu_bo_gpu_offset(parent->bo);
>>>> +        pde = pd_addr + (entry - parent->entries) * 8;
>>>> +        amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>>>> +    }
>>>> +
>>>> +    return 0;
>>>>    }
>>>>      /**
>>>> @@ -1306,22 +1374,31 @@ static int amdgpu_vm_update_ptes(struct
>>>> amdgpu_pte_update_params *params,
>>>>        struct amdgpu_bo *pt;
>>>>        unsigned nptes;
>>>>        bool use_cpu_update = (params->func == amdgpu_vm_cpu_set_ptes);
>>>> -
>>>> +    int r;
>>>>          /* walk over the address space and update the page tables */
>>>> -    for (addr = start; addr < end; addr += nptes) {
>>>> -        pt = amdgpu_vm_get_pt(params, addr);
>>>> -        if (!pt) {
>>>> -            pr_err("PT not found, aborting update_ptes\n");
>>>> -            return -EINVAL;
>>>> -        }
>>>> +    for (addr = start; addr < end; addr += nptes,
>>>> +         dst += nptes * AMDGPU_GPU_PAGE_SIZE) {
>>>> +        struct amdgpu_vm_pt *entry, *parent;
>>>> +
>>>> +        amdgpu_vm_get_entry(params, addr, &entry, &parent);
>>>> +        if (!entry)
>>>> +            return -ENOENT;
>>>>              if ((addr & ~mask) == (end & ~mask))
>>>>                nptes = end - addr;
>>>>            else
>>>>                nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
>>>>    +        r = amdgpu_vm_handle_huge_pages(params, entry, parent,
>>>> +                        nptes, dst, flags);
>>>> +        if (r)
>>>> +            return r;
>>>>    +        if (entry->huge_page)
>>>> +            continue;
>>>> +
>>>> +        pt = entry->bo;
>>>>            if (use_cpu_update) {
>>>>                pe_start = (unsigned long)pt->kptr;
>>>>            } else {
>>>> @@ -1337,8 +1414,6 @@ static int amdgpu_vm_update_ptes(struct
>>>> amdgpu_pte_update_params *params,
>>>>            pe_start += (addr & mask) * 8;
>>>>            params->func(params, pe_start, dst, nptes,
>>>>                     AMDGPU_GPU_PAGE_SIZE, flags);
>>>> -
>>>> -        dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>>>>        }
>>>>          return 0;
>>>> @@ -1490,6 +1565,9 @@ static int amdgpu_vm_bo_update_mapping(struct
>>>> amdgpu_device *adev,
>>>>        /* padding, etc. */
>>>>        ndw = 64;
>>>>    +    /* one PDE write for each huge page */
>>>> +    ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 7;
>>>> +
>>>>        if (src) {
>>>>            /* only copy commands needed */
>>>>            ndw += ncmds * 7;
>>>> @@ -1569,6 +1647,7 @@ static int amdgpu_vm_bo_update_mapping(struct
>>>> amdgpu_device *adev,
>>>>      error_free:
>>>>        amdgpu_job_free(job);
>>>> +    amdgpu_vm_invalidate_level(&vm->root);
>>>>        return r;
>>>>    }
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> index c4f5d1f..34d9174 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> @@ -70,6 +70,9 @@ struct amdgpu_bo_list_entry;
>>>>    /* TILED for VEGA10, reserved for older ASICs  */
>>>>    #define AMDGPU_PTE_PRT        (1ULL << 51)
>>>>    +/* PDE is handled as PTE for VEGA10 */
>>>> +#define AMDGPU_PDE_PTE        (1ULL << 54)
>>>> +
>>>>    /* VEGA10 only */
>>>>    #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>>>>    #define AMDGPU_PTE_MTYPE_MASK    AMDGPU_PTE_MTYPE(3ULL)
>>>> @@ -100,6 +103,7 @@ struct amdgpu_bo_list_entry;
>>>>    struct amdgpu_vm_pt {
>>>>        struct amdgpu_bo    *bo;
>>>>        uint64_t        addr;
>>>> +    bool            huge_page;
>>>>          /* array of page tables, one for each directory entry */
>>>>        struct amdgpu_vm_pt    *entries;
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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