All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: cleanup mtype mapping
@ 2019-09-02 14:57 Christian König
       [not found] ` <20190902145732.2567-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2019-09-02 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Unify how we map the UAPI flags to the PTE hardware flags for a mapping.

Only the MTYPE is actually ASIC dependent, all other flags should be
copied over 1 to 1 and ASIC differences are handled later on.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 32 +++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h       |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h       |  7 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  6 ++-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c        | 40 +++++-------------
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c         | 16 -------
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c         | 16 -------
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c         | 18 --------
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         | 42 +++++--------------
 10 files changed, 59 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b0f0e060ded6..d90b647985d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -380,7 +380,7 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
 			AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
 	}
 
-	return amdgpu_gmc_get_pte_flags(adev, mapping_flags);
+	return amdgpu_gem_va_map_flags(adev, mapping_flags);
 }
 
 /* add_bo_to_vm - Add a BO to a VM
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index e7af35c7080d..22eab74d9998 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -528,6 +528,34 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
 }
 
+/**
+ * amdgpu_gem_va_map_flags - map GEM UAPI flags into hardware flags
+ *
+ * @adev: amdgpu_device pointer
+ * @flags: GEM UAPI flags
+ *
+ * Returns the GEM UAPI flags mapped into hardware for the ASIC.
+ */
+uint64_t amdgpu_gem_va_map_flags(struct amdgpu_device *adev, uint32_t flags)
+{
+	uint64_t pte_flag = 0;
+
+	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
+		pte_flag |= AMDGPU_PTE_EXECUTABLE;
+	if (flags & AMDGPU_VM_PAGE_READABLE)
+		pte_flag |= AMDGPU_PTE_READABLE;
+	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
+		pte_flag |= AMDGPU_PTE_WRITEABLE;
+	if (flags & AMDGPU_VM_PAGE_PRT)
+		pte_flag |= AMDGPU_PTE_PRT;
+
+	if (adev->gmc.gmc_funcs->map_mtype)
+		pte_flag |= amdgpu_gmc_map_mtype(adev,
+						 flags & AMDGPU_VM_MTYPE_MASK);
+
+	return pte_flag;
+}
+
 int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp)
 {
@@ -625,7 +653,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 
 	switch (args->operation) {
 	case AMDGPU_VA_OP_MAP:
-		va_flags = amdgpu_gmc_get_pte_flags(adev, args->flags);
+		va_flags = amdgpu_gem_va_map_flags(adev, args->flags);
 		r = amdgpu_vm_bo_map(adev, bo_va, args->va_address,
 				     args->offset_in_bo, args->map_size,
 				     va_flags);
@@ -640,7 +668,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 						args->map_size);
 		break;
 	case AMDGPU_VA_OP_REPLACE:
-		va_flags = amdgpu_gmc_get_pte_flags(adev, args->flags);
+		va_flags = amdgpu_gem_va_map_flags(adev, args->flags);
 		r = amdgpu_vm_bo_replace_map(adev, bo_va, args->va_address,
 					     args->offset_in_bo, args->map_size,
 					     va_flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
index b8ba6e27c61f..e6acba65763b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
@@ -67,6 +67,7 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp);
 int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *filp);
+uint64_t amdgpu_gem_va_map_flags(struct amdgpu_device *adev, uint32_t flags);
 int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp);
 int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index b6e1d98ef01e..d5e4574afbc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -99,9 +99,8 @@ struct amdgpu_gmc_funcs {
 				   unsigned pasid);
 	/* enable/disable PRT support */
 	void (*set_prt)(struct amdgpu_device *adev, bool enable);
-	/* set pte flags based per asic */
-	uint64_t (*get_vm_pte_flags)(struct amdgpu_device *adev,
-				     uint32_t flags);
+	/* map mtype to hardware flags */
+	uint64_t (*map_mtype)(struct amdgpu_device *adev, uint32_t flags);
 	/* get the pde for a given mc addr */
 	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
 			   u64 *dst, u64 *flags);
@@ -184,8 +183,8 @@ struct amdgpu_gmc {
 #define amdgpu_gmc_flush_gpu_tlb(adev, vmid, vmhub, type) ((adev)->gmc.gmc_funcs->flush_gpu_tlb((adev), (vmid), (vmhub), (type)))
 #define amdgpu_gmc_emit_flush_gpu_tlb(r, vmid, addr) (r)->adev->gmc.gmc_funcs->emit_flush_gpu_tlb((r), (vmid), (addr))
 #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
+#define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
 #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
-#define amdgpu_gmc_get_pte_flags(adev, flags) (adev)->gmc.gmc_funcs->get_vm_pte_flags((adev),(flags))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 501e13420786..2cb82b229802 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1571,8 +1571,10 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
 		flags &= ~AMDGPU_PTE_WRITEABLE;
 
-	flags &= ~AMDGPU_PTE_EXECUTABLE;
-	flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+	if (adev->asic_type >= CHIP_TONGA) {
+		flags &= ~AMDGPU_PTE_EXECUTABLE;
+		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+	}
 
 	if (adev->asic_type >= CHIP_NAVI10) {
 		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 46efd4d17a34..7eb0ba87fef9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -397,43 +397,23 @@ static void gmc_v10_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid
  * 1 system
  * 0 valid
  */
-static uint64_t gmc_v10_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					   uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
 
-	switch (flags & AMDGPU_VM_MTYPE_MASK) {
+static uint64_t gmc_v10_0_map_mtype(struct amdgpu_device *adev, uint32_t flags)
+{
+	switch (flags) {
 	case AMDGPU_VM_MTYPE_DEFAULT:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_NC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_WC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_WC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_WC);
 	case AMDGPU_VM_MTYPE_CC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_CC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_CC);
 	case AMDGPU_VM_MTYPE_UC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_UC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_UC);
 	default:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	}
-
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
 }
 
 static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
@@ -464,7 +444,7 @@ static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
-	.get_vm_pte_flags = gmc_v10_0_get_vm_pte_flags,
+	.map_mtype = gmc_v10_0_map_mtype,
 	.get_vm_pde = gmc_v10_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 3264548e375c..2e305b21db69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -383,21 +383,6 @@ static uint64_t gmc_v6_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
 	return pd_addr;
 }
 
-static uint64_t gmc_v6_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1159,7 +1144,6 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
 	.set_prt = gmc_v6_0_set_prt,
 	.get_vm_pde = gmc_v6_0_get_vm_pde,
-	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index cc0aa178eb2d..3b77421234a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -460,21 +460,6 @@ static void gmc_v7_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
 	amdgpu_ring_emit_wreg(ring, mmIH_VMID_0_LUT + vmid, pasid);
 }
 
-static uint64_t gmc_v7_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1354,7 +1339,6 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
 	.set_prt = gmc_v7_0_set_prt,
-	.get_vm_pte_flags = gmc_v7_0_get_vm_pte_flags,
 	.get_vm_pde = gmc_v7_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 88f3a171452f..58444aa0af05 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -683,23 +683,6 @@ static void gmc_v8_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
  * 0 valid
  */
 
-static uint64_t gmc_v8_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1722,7 +1705,6 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
 	.set_prt = gmc_v8_0_set_prt,
-	.get_vm_pte_flags = gmc_v8_0_get_vm_pte_flags,
 	.get_vm_pde = gmc_v8_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index af069a4837fa..22660e1005db 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -580,47 +580,25 @@ static void gmc_v9_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
  * 0 valid
  */
 
-static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
-						uint32_t flags)
+static uint64_t gmc_v9_0_map_mtype(struct amdgpu_device *adev, uint32_t flags)
 
 {
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-
-	switch (flags & AMDGPU_VM_MTYPE_MASK) {
+	switch (flags) {
 	case AMDGPU_VM_MTYPE_DEFAULT:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_NC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_WC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
 	case AMDGPU_VM_MTYPE_RW:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
 	case AMDGPU_VM_MTYPE_CC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_CC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_CC);
 	case AMDGPU_VM_MTYPE_UC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_UC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_UC);
 	default:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	}
-
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
 }
 
 static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
@@ -651,7 +629,7 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
-	.get_vm_pte_flags = gmc_v9_0_get_vm_pte_flags,
+	.map_mtype = gmc_v9_0_map_mtype,
 	.get_vm_pde = gmc_v9_0_get_vm_pde
 };
 
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
       [not found] ` <20190902145732.2567-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
@ 2019-09-02 14:57   ` Christian König
       [not found]     ` <20190902145732.2567-2-christian.koenig-5C7GfCeVMHo@public.gmane.org>
  2019-09-03 14:26   ` [PATCH 1/2] drm/amdgpu: cleanup mtype mapping Zeng, Oak
  1 sibling, 1 reply; 7+ messages in thread
From: Christian König @ 2019-09-02 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Move the ASIC specific code into a new callback function.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
 7 files changed, 82 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index d5e4574afbc2..d3be51ba6349 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
 	/* get the pde for a given mc addr */
 	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
 			   u64 *dst, u64 *flags);
+	/* get the pte flags to use for a BO VA mapping */
+	void (*get_vm_pte)(struct amdgpu_device *adev,
+			   struct amdgpu_bo_va_mapping *mapping,
+			   uint64_t *flags);
 };
 
 struct amdgpu_xgmi {
@@ -185,6 +189,7 @@ struct amdgpu_gmc {
 #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
 #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
 #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
+#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 2cb82b229802..b285ab25146d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
 		flags &= ~AMDGPU_PTE_WRITEABLE;
 
-	if (adev->asic_type >= CHIP_TONGA) {
-		flags &= ~AMDGPU_PTE_EXECUTABLE;
-		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
-	}
-
-	if (adev->asic_type >= CHIP_NAVI10) {
-		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
-		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
-	} else {
-		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
-		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
-	}
-
-	if ((mapping->flags & AMDGPU_PTE_PRT) &&
-	    (adev->asic_type >= CHIP_VEGA10)) {
-		flags |= AMDGPU_PTE_PRT;
-		if (adev->asic_type >= CHIP_NAVI10) {
-			flags |= AMDGPU_PTE_SNOOPED;
-			flags |= AMDGPU_PTE_LOG;
-			flags |= AMDGPU_PTE_SYSTEM;
-		}
-		flags &= ~AMDGPU_PTE_VALID;
-	}
-	if (adev->asic_type == CHIP_ARCTURUS &&
-	    !(flags & AMDGPU_PTE_SYSTEM) &&
-	    mapping->bo_va->is_xgmi)
-		flags |= AMDGPU_PTE_SNOOPED;
+	/* Apply ASIC specific mapping flags */
+	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
 
 	trace_amdgpu_vm_bo_update(mapping);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 7eb0ba87fef9..1a8d8f528b01 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
+				 struct amdgpu_bo_va_mapping *mapping,
+				 uint64_t *flags)
+{
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+
+	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
+	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
+
+	if (mapping->flags & AMDGPU_PTE_PRT) {
+		*flags |= AMDGPU_PTE_PRT;
+		*flags |= AMDGPU_PTE_SNOOPED;
+		*flags |= AMDGPU_PTE_LOG;
+		*flags |= AMDGPU_PTE_SYSTEM;
+		*flags &= ~AMDGPU_PTE_VALID;
+	}
+}
+
 static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
 	.map_mtype = gmc_v10_0_map_mtype,
-	.get_vm_pde = gmc_v10_0_get_vm_pde
+	.get_vm_pde = gmc_v10_0_get_vm_pde,
+	.get_vm_pte = gmc_v10_0_get_vm_pte
 };
 
 static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 2e305b21db69..ce591c995691 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+}
+
 static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
 					      bool value)
 {
@@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
 	.set_prt = gmc_v6_0_set_prt,
 	.get_vm_pde = gmc_v6_0_get_vm_pde,
+	.get_vm_pte = gmc_v6_0_get_vm_pte,
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 3b77421234a7..e3f53fbfcd8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+}
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
 	.set_prt = gmc_v7_0_set_prt,
-	.get_vm_pde = gmc_v7_0_get_vm_pde
+	.get_vm_pde = gmc_v7_0_get_vm_pde,
+	.get_vm_pte = gmc_v7_0_get_vm_pte
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 58444aa0af05..256d1faacada 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+}
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
 	.set_prt = gmc_v8_0_set_prt,
-	.get_vm_pde = gmc_v8_0_get_vm_pde
+	.get_vm_pde = gmc_v8_0_get_vm_pde,
+	.get_vm_pte = gmc_v8_0_get_vm_pte
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 22660e1005db..b3d2d70e84c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+
+	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
+	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
+
+	if (mapping->flags & AMDGPU_PTE_PRT) {
+		*flags |= AMDGPU_PTE_PRT;
+		*flags &= ~AMDGPU_PTE_VALID;
+	}
+
+	if (adev->asic_type == CHIP_ARCTURUS &&
+	    !(*flags & AMDGPU_PTE_SYSTEM) &&
+	    mapping->bo_va->is_xgmi)
+		*flags |= AMDGPU_PTE_SNOOPED;
+}
+
 static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
 	.map_mtype = gmc_v9_0_map_mtype,
-	.get_vm_pde = gmc_v9_0_get_vm_pde
+	.get_vm_pde = gmc_v9_0_get_vm_pde,
+	.get_vm_pte = gmc_v9_0_get_vm_pte
 };
 
 static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] drm/amdgpu: cleanup mtype mapping
       [not found] ` <20190902145732.2567-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
  2019-09-02 14:57   ` [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation Christian König
@ 2019-09-03 14:26   ` Zeng, Oak
  1 sibling, 0 replies; 7+ messages in thread
From: Zeng, Oak @ 2019-09-03 14:26 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

This is nice clean up. Acked-by: Oak Zeng <Oak.Zeng@amd.com>

Regards,
Oak

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian König
Sent: Monday, September 2, 2019 10:58 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH 1/2] drm/amdgpu: cleanup mtype mapping

Unify how we map the UAPI flags to the PTE hardware flags for a mapping.

Only the MTYPE is actually ASIC dependent, all other flags should be copied over 1 to 1 and ASIC differences are handled later on.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 32 +++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h       |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h       |  7 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  6 ++-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c        | 40 +++++-------------
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c         | 16 -------
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c         | 16 -------
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c         | 18 --------
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         | 42 +++++--------------
 10 files changed, 59 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b0f0e060ded6..d90b647985d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -380,7 +380,7 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
 			AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
 	}
 
-	return amdgpu_gmc_get_pte_flags(adev, mapping_flags);
+	return amdgpu_gem_va_map_flags(adev, mapping_flags);
 }
 
 /* add_bo_to_vm - Add a BO to a VM
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index e7af35c7080d..22eab74d9998 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -528,6 +528,34 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);  }
 
+/**
+ * amdgpu_gem_va_map_flags - map GEM UAPI flags into hardware flags
+ *
+ * @adev: amdgpu_device pointer
+ * @flags: GEM UAPI flags
+ *
+ * Returns the GEM UAPI flags mapped into hardware for the ASIC.
+ */
+uint64_t amdgpu_gem_va_map_flags(struct amdgpu_device *adev, uint32_t 
+flags) {
+	uint64_t pte_flag = 0;
+
+	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
+		pte_flag |= AMDGPU_PTE_EXECUTABLE;
+	if (flags & AMDGPU_VM_PAGE_READABLE)
+		pte_flag |= AMDGPU_PTE_READABLE;
+	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
+		pte_flag |= AMDGPU_PTE_WRITEABLE;
+	if (flags & AMDGPU_VM_PAGE_PRT)
+		pte_flag |= AMDGPU_PTE_PRT;
+
+	if (adev->gmc.gmc_funcs->map_mtype)
+		pte_flag |= amdgpu_gmc_map_mtype(adev,
+						 flags & AMDGPU_VM_MTYPE_MASK);
+
+	return pte_flag;
+}
+
 int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp)
 {
@@ -625,7 +653,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 
 	switch (args->operation) {
 	case AMDGPU_VA_OP_MAP:
-		va_flags = amdgpu_gmc_get_pte_flags(adev, args->flags);
+		va_flags = amdgpu_gem_va_map_flags(adev, args->flags);
 		r = amdgpu_vm_bo_map(adev, bo_va, args->va_address,
 				     args->offset_in_bo, args->map_size,
 				     va_flags);
@@ -640,7 +668,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 						args->map_size);
 		break;
 	case AMDGPU_VA_OP_REPLACE:
-		va_flags = amdgpu_gmc_get_pte_flags(adev, args->flags);
+		va_flags = amdgpu_gem_va_map_flags(adev, args->flags);
 		r = amdgpu_vm_bo_replace_map(adev, bo_va, args->va_address,
 					     args->offset_in_bo, args->map_size,
 					     va_flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
index b8ba6e27c61f..e6acba65763b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
@@ -67,6 +67,7 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp);
 int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *filp);
+uint64_t amdgpu_gem_va_map_flags(struct amdgpu_device *adev, uint32_t 
+flags);
 int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp);
 int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index b6e1d98ef01e..d5e4574afbc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -99,9 +99,8 @@ struct amdgpu_gmc_funcs {
 				   unsigned pasid);
 	/* enable/disable PRT support */
 	void (*set_prt)(struct amdgpu_device *adev, bool enable);
-	/* set pte flags based per asic */
-	uint64_t (*get_vm_pte_flags)(struct amdgpu_device *adev,
-				     uint32_t flags);
+	/* map mtype to hardware flags */
+	uint64_t (*map_mtype)(struct amdgpu_device *adev, uint32_t flags);
 	/* get the pde for a given mc addr */
 	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
 			   u64 *dst, u64 *flags);
@@ -184,8 +183,8 @@ struct amdgpu_gmc {
 #define amdgpu_gmc_flush_gpu_tlb(adev, vmid, vmhub, type) ((adev)->gmc.gmc_funcs->flush_gpu_tlb((adev), (vmid), (vmhub), (type)))  #define amdgpu_gmc_emit_flush_gpu_tlb(r, vmid, addr) (r)->adev->gmc.gmc_funcs->emit_flush_gpu_tlb((r), (vmid), (addr))  #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
+#define amdgpu_gmc_map_mtype(adev, flags) 
+(adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
 #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags)) -#define amdgpu_gmc_get_pte_flags(adev, flags) (adev)->gmc.gmc_funcs->get_vm_pte_flags((adev),(flags))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 501e13420786..2cb82b229802 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1571,8 +1571,10 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
 		flags &= ~AMDGPU_PTE_WRITEABLE;
 
-	flags &= ~AMDGPU_PTE_EXECUTABLE;
-	flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+	if (adev->asic_type >= CHIP_TONGA) {
+		flags &= ~AMDGPU_PTE_EXECUTABLE;
+		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+	}
 
 	if (adev->asic_type >= CHIP_NAVI10) {
 		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 46efd4d17a34..7eb0ba87fef9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -397,43 +397,23 @@ static void gmc_v10_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid
  * 1 system
  * 0 valid
  */
-static uint64_t gmc_v10_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					   uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
 
-	switch (flags & AMDGPU_VM_MTYPE_MASK) {
+static uint64_t gmc_v10_0_map_mtype(struct amdgpu_device *adev, 
+uint32_t flags) {
+	switch (flags) {
 	case AMDGPU_VM_MTYPE_DEFAULT:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_NC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_WC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_WC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_WC);
 	case AMDGPU_VM_MTYPE_CC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_CC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_CC);
 	case AMDGPU_VM_MTYPE_UC:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_UC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_UC);
 	default:
-		pte_flag |= AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_NV10(MTYPE_NC);
 	}
-
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
 }
 
 static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level, @@ -464,7 +444,7 @@ static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
-	.get_vm_pte_flags = gmc_v10_0_get_vm_pte_flags,
+	.map_mtype = gmc_v10_0_map_mtype,
 	.get_vm_pde = gmc_v10_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 3264548e375c..2e305b21db69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -383,21 +383,6 @@ static uint64_t gmc_v6_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
 	return pd_addr;
 }
 
-static uint64_t gmc_v6_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1159,7 +1144,6 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
 	.set_prt = gmc_v6_0_set_prt,
 	.get_vm_pde = gmc_v6_0_get_vm_pde,
-	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = { diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index cc0aa178eb2d..3b77421234a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -460,21 +460,6 @@ static void gmc_v7_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
 	amdgpu_ring_emit_wreg(ring, mmIH_VMID_0_LUT + vmid, pasid);  }
 
-static uint64_t gmc_v7_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1354,7 +1339,6 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
 	.set_prt = gmc_v7_0_set_prt,
-	.get_vm_pte_flags = gmc_v7_0_get_vm_pte_flags,
 	.get_vm_pde = gmc_v7_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 88f3a171452f..58444aa0af05 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -683,23 +683,6 @@ static void gmc_v8_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
  * 0 valid
  */
 
-static uint64_t gmc_v8_0_get_vm_pte_flags(struct amdgpu_device *adev,
-					  uint32_t flags)
-{
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
-}
-
 static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -1722,7 +1705,6 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
 	.set_prt = gmc_v8_0_set_prt,
-	.get_vm_pte_flags = gmc_v8_0_get_vm_pte_flags,
 	.get_vm_pde = gmc_v8_0_get_vm_pde
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index af069a4837fa..22660e1005db 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -580,47 +580,25 @@ static void gmc_v9_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
  * 0 valid
  */
 
-static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
-						uint32_t flags)
+static uint64_t gmc_v9_0_map_mtype(struct amdgpu_device *adev, uint32_t 
+flags)
 
 {
-	uint64_t pte_flag = 0;
-
-	if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
-		pte_flag |= AMDGPU_PTE_EXECUTABLE;
-	if (flags & AMDGPU_VM_PAGE_READABLE)
-		pte_flag |= AMDGPU_PTE_READABLE;
-	if (flags & AMDGPU_VM_PAGE_WRITEABLE)
-		pte_flag |= AMDGPU_PTE_WRITEABLE;
-
-	switch (flags & AMDGPU_VM_MTYPE_MASK) {
+	switch (flags) {
 	case AMDGPU_VM_MTYPE_DEFAULT:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_NC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	case AMDGPU_VM_MTYPE_WC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
 	case AMDGPU_VM_MTYPE_RW:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
 	case AMDGPU_VM_MTYPE_CC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_CC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_CC);
 	case AMDGPU_VM_MTYPE_UC:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_UC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_UC);
 	default:
-		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
-		break;
+		return AMDGPU_PTE_MTYPE_VG10(MTYPE_NC);
 	}
-
-	if (flags & AMDGPU_VM_PAGE_PRT)
-		pte_flag |= AMDGPU_PTE_PRT;
-
-	return pte_flag;
 }
 
 static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level, @@ -651,7 +629,7 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
-	.get_vm_pte_flags = gmc_v9_0_get_vm_pte_flags,
+	.map_mtype = gmc_v9_0_map_mtype,
 	.get_vm_pde = gmc_v9_0_get_vm_pde
 };
 
--
2.17.1

_______________________________________________
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

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
       [not found]     ` <20190902145732.2567-2-christian.koenig-5C7GfCeVMHo@public.gmane.org>
@ 2019-09-03 20:28       ` Kuehling, Felix
       [not found]         ` <bf6fbe9a-c568-0d5c-635c-d759924f72c1-5C7GfCeVMHo@public.gmane.org>
  2019-09-03 21:32       ` Liu, Shaoyun
  1 sibling, 1 reply; 7+ messages in thread
From: Kuehling, Felix @ 2019-09-03 20:28 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 2019-09-02 10:57 a.m., Christian König wrote:
> Move the ASIC specific code into a new callback function.

NAK. I believe the BUG_ONs you're adding will trigger with ROCm on 
Hawaii and Kaveri. See inline ...

ROCm user mode doesn't care that the page table doesn't have an 
executable bit on those chips. If the HW makes all memory executable, we 
should just ignore the flag.


>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>   7 files changed, 82 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index d5e4574afbc2..d3be51ba6349 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>   	/* get the pde for a given mc addr */
>   	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>   			   u64 *dst, u64 *flags);
> +	/* get the pte flags to use for a BO VA mapping */
> +	void (*get_vm_pte)(struct amdgpu_device *adev,
> +			   struct amdgpu_bo_va_mapping *mapping,
> +			   uint64_t *flags);
>   };
>   
>   struct amdgpu_xgmi {
> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>   #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>   #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>   #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>   
>   /**
>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 2cb82b229802..b285ab25146d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>   		flags &= ~AMDGPU_PTE_WRITEABLE;
>   
> -	if (adev->asic_type >= CHIP_TONGA) {
> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> -	}
> -
> -	if (adev->asic_type >= CHIP_NAVI10) {
> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> -	} else {
> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
> -	}
> -
> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
> -	    (adev->asic_type >= CHIP_VEGA10)) {
> -		flags |= AMDGPU_PTE_PRT;
> -		if (adev->asic_type >= CHIP_NAVI10) {
> -			flags |= AMDGPU_PTE_SNOOPED;
> -			flags |= AMDGPU_PTE_LOG;
> -			flags |= AMDGPU_PTE_SYSTEM;
> -		}
> -		flags &= ~AMDGPU_PTE_VALID;
> -	}
> -	if (adev->asic_type == CHIP_ARCTURUS &&
> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
> -	    mapping->bo_va->is_xgmi)
> -		flags |= AMDGPU_PTE_SNOOPED;
> +	/* Apply ASIC specific mapping flags */
> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>   
>   	trace_amdgpu_vm_bo_update(mapping);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 7eb0ba87fef9..1a8d8f528b01 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
> +				 struct amdgpu_bo_va_mapping *mapping,
> +				 uint64_t *flags)
> +{
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> +
> +	if (mapping->flags & AMDGPU_PTE_PRT) {
> +		*flags |= AMDGPU_PTE_PRT;
> +		*flags |= AMDGPU_PTE_SNOOPED;
> +		*flags |= AMDGPU_PTE_LOG;
> +		*flags |= AMDGPU_PTE_SYSTEM;
> +		*flags &= ~AMDGPU_PTE_VALID;
> +	}
> +}
> +
>   static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>   	.map_mtype = gmc_v10_0_map_mtype,
> -	.get_vm_pde = gmc_v10_0_get_vm_pde
> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>   };
>   
>   static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 2e305b21db69..ce591c995691 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +}
> +
>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>   					      bool value)
>   {
> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>   	.set_prt = gmc_v6_0_set_prt,
>   	.get_vm_pde = gmc_v6_0_get_vm_pde,
> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 3b77421234a7..e3f53fbfcd8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);

NAK. This is going to break ROCm on Kaveri and Hawaii.

Regards,
   Felix


> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>   	.set_prt = gmc_v7_0_set_prt,
> -	.get_vm_pde = gmc_v7_0_get_vm_pde
> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 58444aa0af05..256d1faacada 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>   	.set_prt = gmc_v8_0_set_prt,
> -	.get_vm_pde = gmc_v8_0_get_vm_pde
> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 22660e1005db..b3d2d70e84c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
> +
> +	if (mapping->flags & AMDGPU_PTE_PRT) {
> +		*flags |= AMDGPU_PTE_PRT;
> +		*flags &= ~AMDGPU_PTE_VALID;
> +	}
> +
> +	if (adev->asic_type == CHIP_ARCTURUS &&
> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
> +	    mapping->bo_va->is_xgmi)
> +		*flags |= AMDGPU_PTE_SNOOPED;
> +}
> +
>   static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>   	.map_mtype = gmc_v9_0_map_mtype,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>   };
>   
>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
       [not found]     ` <20190902145732.2567-2-christian.koenig-5C7GfCeVMHo@public.gmane.org>
  2019-09-03 20:28       ` Kuehling, Felix
@ 2019-09-03 21:32       ` Liu, Shaoyun
       [not found]         ` <0cbe1576-2ff9-8404-ea73-a0fa9c085158-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 7+ messages in thread
From: Liu, Shaoyun @ 2019-09-03 21:32 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Maybe  it's not necessary to separate the mtype from the get_vm_pte 
function , so we just need one  asic specific functions (get_vm_pte) .

What make me confusing originally is  the  the  logic  to setup the  PTE 
flag.   We first map the  UAPI flags to HW specific PTE flag , save it 
into mapping  structure  and  in amdgpu_bo_split_mapping adjust the  
flag again before set the value to HW . Is it possible we just have one 
place to set the asic specific PTE flag ,  ex, the  mapping  structure 
still keep the  UAPI flag and  all the HW specific PTE setting is in the 
last step before real set to HW ?

Regards

shaoyun.liu

On 2019-09-02 10:57 a.m., Christian König wrote:
> Move the ASIC specific code into a new callback function.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>   7 files changed, 82 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index d5e4574afbc2..d3be51ba6349 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>   	/* get the pde for a given mc addr */
>   	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>   			   u64 *dst, u64 *flags);
> +	/* get the pte flags to use for a BO VA mapping */
> +	void (*get_vm_pte)(struct amdgpu_device *adev,
> +			   struct amdgpu_bo_va_mapping *mapping,
> +			   uint64_t *flags);
>   };
>   
>   struct amdgpu_xgmi {
> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>   #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>   #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>   #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>   
>   /**
>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 2cb82b229802..b285ab25146d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>   		flags &= ~AMDGPU_PTE_WRITEABLE;
>   
> -	if (adev->asic_type >= CHIP_TONGA) {
> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> -	}
> -
> -	if (adev->asic_type >= CHIP_NAVI10) {
> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> -	} else {
> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
> -	}
> -
> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
> -	    (adev->asic_type >= CHIP_VEGA10)) {
> -		flags |= AMDGPU_PTE_PRT;
> -		if (adev->asic_type >= CHIP_NAVI10) {
> -			flags |= AMDGPU_PTE_SNOOPED;
> -			flags |= AMDGPU_PTE_LOG;
> -			flags |= AMDGPU_PTE_SYSTEM;
> -		}
> -		flags &= ~AMDGPU_PTE_VALID;
> -	}
> -	if (adev->asic_type == CHIP_ARCTURUS &&
> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
> -	    mapping->bo_va->is_xgmi)
> -		flags |= AMDGPU_PTE_SNOOPED;
> +	/* Apply ASIC specific mapping flags */
> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>   
>   	trace_amdgpu_vm_bo_update(mapping);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 7eb0ba87fef9..1a8d8f528b01 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
> +				 struct amdgpu_bo_va_mapping *mapping,
> +				 uint64_t *flags)
> +{
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> +
> +	if (mapping->flags & AMDGPU_PTE_PRT) {
> +		*flags |= AMDGPU_PTE_PRT;
> +		*flags |= AMDGPU_PTE_SNOOPED;
> +		*flags |= AMDGPU_PTE_LOG;
> +		*flags |= AMDGPU_PTE_SYSTEM;
> +		*flags &= ~AMDGPU_PTE_VALID;
> +	}
> +}
> +
>   static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>   	.map_mtype = gmc_v10_0_map_mtype,
> -	.get_vm_pde = gmc_v10_0_get_vm_pde
> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>   };
>   
>   static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 2e305b21db69..ce591c995691 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +}
> +
>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>   					      bool value)
>   {
> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>   	.set_prt = gmc_v6_0_set_prt,
>   	.get_vm_pde = gmc_v6_0_get_vm_pde,
> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 3b77421234a7..e3f53fbfcd8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>   	.set_prt = gmc_v7_0_set_prt,
> -	.get_vm_pde = gmc_v7_0_get_vm_pde
> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 58444aa0af05..256d1faacada 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
> +
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>   	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>   	.set_prt = gmc_v8_0_set_prt,
> -	.get_vm_pde = gmc_v8_0_get_vm_pde
> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>   };
>   
>   static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 22660e1005db..b3d2d70e84c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
> +				struct amdgpu_bo_va_mapping *mapping,
> +				uint64_t *flags)
> +{
> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
> +
> +	if (mapping->flags & AMDGPU_PTE_PRT) {
> +		*flags |= AMDGPU_PTE_PRT;
> +		*flags &= ~AMDGPU_PTE_VALID;
> +	}
> +
> +	if (adev->asic_type == CHIP_ARCTURUS &&
> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
> +	    mapping->bo_va->is_xgmi)
> +		*flags |= AMDGPU_PTE_SNOOPED;
> +}
> +
>   static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>   	.map_mtype = gmc_v9_0_map_mtype,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>   };
>   
>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
       [not found]         ` <bf6fbe9a-c568-0d5c-635c-d759924f72c1-5C7GfCeVMHo@public.gmane.org>
@ 2019-09-04  7:50           ` Christian König
  0 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2019-09-04  7:50 UTC (permalink / raw)
  To: Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 03.09.19 um 22:28 schrieb Kuehling, Felix:
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
> NAK. I believe the BUG_ONs you're adding will trigger with ROCm on
> Hawaii and Kaveri. See inline ...

Those are the flags as generated by the TTM code, they shouldn't contain 
the executable bit.

The mapping->flags are the one generated by ROCm.

> ROCm user mode doesn't care that the page table doesn't have an
> executable bit on those chips. If the HW makes all memory executable, we
> should just ignore the flag.

I've added this because I'm not sure how the hardware reacts when it 
sees a reserved bit set.

The alternative is to just clear the flag, because it isn't really 
supported in any way.

Christian.

>
>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>>    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>>    7 files changed, 82 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index d5e4574afbc2..d3be51ba6349 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>>    	/* get the pde for a given mc addr */
>>    	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>>    			   u64 *dst, u64 *flags);
>> +	/* get the pte flags to use for a BO VA mapping */
>> +	void (*get_vm_pte)(struct amdgpu_device *adev,
>> +			   struct amdgpu_bo_va_mapping *mapping,
>> +			   uint64_t *flags);
>>    };
>>    
>>    struct amdgpu_xgmi {
>> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>>    #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>>    #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>>    #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>>    
>>    /**
>>     * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2cb82b229802..b285ab25146d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>>    	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>>    		flags &= ~AMDGPU_PTE_WRITEABLE;
>>    
>> -	if (adev->asic_type >= CHIP_TONGA) {
>> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
>> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> -	}
>> -
>> -	if (adev->asic_type >= CHIP_NAVI10) {
>> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> -	} else {
>> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
>> -	}
>> -
>> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> -	    (adev->asic_type >= CHIP_VEGA10)) {
>> -		flags |= AMDGPU_PTE_PRT;
>> -		if (adev->asic_type >= CHIP_NAVI10) {
>> -			flags |= AMDGPU_PTE_SNOOPED;
>> -			flags |= AMDGPU_PTE_LOG;
>> -			flags |= AMDGPU_PTE_SYSTEM;
>> -		}
>> -		flags &= ~AMDGPU_PTE_VALID;
>> -	}
>> -	if (adev->asic_type == CHIP_ARCTURUS &&
>> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
>> -	    mapping->bo_va->is_xgmi)
>> -		flags |= AMDGPU_PTE_SNOOPED;
>> +	/* Apply ASIC specific mapping flags */
>> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>>    
>>    	trace_amdgpu_vm_bo_update(mapping);
>>    
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 7eb0ba87fef9..1a8d8f528b01 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
>> +				 struct amdgpu_bo_va_mapping *mapping,
>> +				 uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +		*flags |= AMDGPU_PTE_LOG;
>> +		*flags |= AMDGPU_PTE_SYSTEM;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v10_0_map_mtype,
>> -	.get_vm_pde = gmc_v10_0_get_vm_pde
>> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 2e305b21db69..ce591c995691 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>>    					      bool value)
>>    {
>> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>>    	.set_prt = gmc_v6_0_set_prt,
>>    	.get_vm_pde = gmc_v6_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 3b77421234a7..e3f53fbfcd8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> NAK. This is going to break ROCm on Kaveri and Hawaii.
>
> Regards,
>     Felix
>
>
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v7_0_set_prt,
>> -	.get_vm_pde = gmc_v7_0_get_vm_pde
>> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 58444aa0af05..256d1faacada 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v8_0_set_prt,
>> -	.get_vm_pde = gmc_v8_0_get_vm_pde
>> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +
>> +	if (adev->asic_type == CHIP_ARCTURUS &&
>> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
>> +	    mapping->bo_va->is_xgmi)
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v9_0_map_mtype,
>> -	.get_vm_pde = gmc_v9_0_get_vm_pde
>> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
       [not found]         ` <0cbe1576-2ff9-8404-ea73-a0fa9c085158-5C7GfCeVMHo@public.gmane.org>
@ 2019-09-04  7:59           ` Christian König
  0 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2019-09-04  7:59 UTC (permalink / raw)
  To: Liu, Shaoyun, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

> the  mapping  structure still keep the  UAPI flag and  all the HW specific PTE setting is in the
> last step before real set to HW ?

I thought about that approach as well, but that would unfortunately not 
work correctly.

The problem is that we have some mapping flags which don't have an 
userspace representation.

Additional to that I want to avoid leaking the UAPI defines into the VM 
subsystem.

We shouldn't have created the UAPI defines in the first place and should 
have used the hardware flags directly.

Regards,
Christian.

Am 03.09.19 um 23:32 schrieb Liu, Shaoyun:
> Maybe  it's not necessary to separate the mtype from the get_vm_pte
> function , so we just need one  asic specific functions (get_vm_pte) .
>
> What make me confusing originally is  the  the  logic  to setup the  PTE
> flag.   We first map the  UAPI flags to HW specific PTE flag , save it
> into mapping  structure  and  in amdgpu_bo_split_mapping adjust the
> flag again before set the value to HW . Is it possible we just have one
> place to set the asic specific PTE flag ,  ex, the  mapping  structure
> still keep the  UAPI flag and  all the HW specific PTE setting is in the
> last step before real set to HW ?
>
> Regards
>
> shaoyun.liu
>
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>>    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>>    7 files changed, 82 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index d5e4574afbc2..d3be51ba6349 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>>    	/* get the pde for a given mc addr */
>>    	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>>    			   u64 *dst, u64 *flags);
>> +	/* get the pte flags to use for a BO VA mapping */
>> +	void (*get_vm_pte)(struct amdgpu_device *adev,
>> +			   struct amdgpu_bo_va_mapping *mapping,
>> +			   uint64_t *flags);
>>    };
>>    
>>    struct amdgpu_xgmi {
>> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>>    #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>>    #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>>    #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>>    
>>    /**
>>     * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2cb82b229802..b285ab25146d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>>    	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>>    		flags &= ~AMDGPU_PTE_WRITEABLE;
>>    
>> -	if (adev->asic_type >= CHIP_TONGA) {
>> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
>> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> -	}
>> -
>> -	if (adev->asic_type >= CHIP_NAVI10) {
>> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> -	} else {
>> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
>> -	}
>> -
>> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> -	    (adev->asic_type >= CHIP_VEGA10)) {
>> -		flags |= AMDGPU_PTE_PRT;
>> -		if (adev->asic_type >= CHIP_NAVI10) {
>> -			flags |= AMDGPU_PTE_SNOOPED;
>> -			flags |= AMDGPU_PTE_LOG;
>> -			flags |= AMDGPU_PTE_SYSTEM;
>> -		}
>> -		flags &= ~AMDGPU_PTE_VALID;
>> -	}
>> -	if (adev->asic_type == CHIP_ARCTURUS &&
>> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
>> -	    mapping->bo_va->is_xgmi)
>> -		flags |= AMDGPU_PTE_SNOOPED;
>> +	/* Apply ASIC specific mapping flags */
>> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>>    
>>    	trace_amdgpu_vm_bo_update(mapping);
>>    
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 7eb0ba87fef9..1a8d8f528b01 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
>> +				 struct amdgpu_bo_va_mapping *mapping,
>> +				 uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +		*flags |= AMDGPU_PTE_LOG;
>> +		*flags |= AMDGPU_PTE_SYSTEM;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v10_0_map_mtype,
>> -	.get_vm_pde = gmc_v10_0_get_vm_pde
>> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 2e305b21db69..ce591c995691 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>>    					      bool value)
>>    {
>> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>>    	.set_prt = gmc_v6_0_set_prt,
>>    	.get_vm_pde = gmc_v6_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 3b77421234a7..e3f53fbfcd8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v7_0_set_prt,
>> -	.get_vm_pde = gmc_v7_0_get_vm_pde
>> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 58444aa0af05..256d1faacada 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v8_0_set_prt,
>> -	.get_vm_pde = gmc_v8_0_get_vm_pde
>> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +
>> +	if (adev->asic_type == CHIP_ARCTURUS &&
>> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
>> +	    mapping->bo_va->is_xgmi)
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v9_0_map_mtype,
>> -	.get_vm_pde = gmc_v9_0_get_vm_pde
>> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-04  7:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-02 14:57 [PATCH 1/2] drm/amdgpu: cleanup mtype mapping Christian König
     [not found] ` <20190902145732.2567-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2019-09-02 14:57   ` [PATCH 2/2] drm/amdgpu: cleanup PTE flag generation Christian König
     [not found]     ` <20190902145732.2567-2-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2019-09-03 20:28       ` Kuehling, Felix
     [not found]         ` <bf6fbe9a-c568-0d5c-635c-d759924f72c1-5C7GfCeVMHo@public.gmane.org>
2019-09-04  7:50           ` Christian König
2019-09-03 21:32       ` Liu, Shaoyun
     [not found]         ` <0cbe1576-2ff9-8404-ea73-a0fa9c085158-5C7GfCeVMHo@public.gmane.org>
2019-09-04  7:59           ` Christian König
2019-09-03 14:26   ` [PATCH 1/2] drm/amdgpu: cleanup mtype mapping Zeng, Oak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.