[PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Zeng, Oak]

Add definition of all supported mtypes. The RW mtype
is recently introduced for arcturus. Also add definition
for the cachable/snoopable bit, which will be used later
in this series.

Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
 include/uapi/drm/amdgpu_drm.h          | 4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 2eda3a8..7a77477 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
 #define AMDGPU_PTE_MTYPE_VG10(a)	((uint64_t)(a) << 57)
 #define AMDGPU_PTE_MTYPE_VG10_MASK	AMDGPU_PTE_MTYPE_VG10(3ULL)
 
-#define AMDGPU_MTYPE_NC 0
-#define AMDGPU_MTYPE_CC 2
+enum amdgpu_mtype {
+	AMDGPU_MTYPE_NC = 0,
+	AMDGPU_MTYPE_WC = 1,
+	AMDGPU_MTYPE_CC = 2,
+	AMDGPU_MTYPE_UC = 3,
+	AMDGPU_MTYPE_RW = 4,
+};
 
 #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
                                 | AMDGPU_PTE_SNOOPED    \
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index ca97b68..2889663 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
 #define AMDGPU_VM_MTYPE_CC		(3 << 5)
 /* Use UC MTYPE instead of default MTYPE */
 #define AMDGPU_VM_MTYPE_UC		(4 << 5)
+/* Use RW MTYPE instead of default MTYPE */
+#define AMDGPU_VM_MTYPE_RW		(5 << 5)
+/* Cacheable/snoopable */
+#define AMDGPU_VM_PAGE_SNOOPED		(1 << 9)
 
 struct drm_amdgpu_gem_va {
 	/** GEM object handle */
-- 
2.7.4

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

[PATCH 2/5] drm/amdgpu: Support new arcturus mtype

[Zeng, Oak]

Arcturus repurposed mtype WC to RW. Modify gmc functions
to support the new mtype

Change-Id: Idc338e5386a57020f45262025e2664ab4ba9f291
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c     | 61 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/include/vega10_enum.h |  7 ++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index a2aa35e..89064d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -599,6 +599,50 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
 	return pte_flag;
 }
 
+static uint64_t gmc_v9_0_arcturus_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 (adev->gmc.zfb_size > 0)
+		pte_flag |= AMDGPU_PTE_SYSTEM;
+
+	switch (flags & AMDGPU_VM_MTYPE_MASK) {
+	case AMDGPU_VM_MTYPE_DEFAULT:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
+		break;
+	case AMDGPU_VM_MTYPE_NC:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
+		break;
+	case AMDGPU_VM_MTYPE_RW:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_RW);
+		break;
+	case AMDGPU_VM_MTYPE_CC:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_CC);
+		break;
+	case AMDGPU_VM_MTYPE_UC:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_UC);
+		break;
+	default:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
+		break;
+	}
+
+	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,
 				uint64_t *addr, uint64_t *flags)
 {
@@ -631,9 +675,24 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.get_vm_pde = gmc_v9_0_get_vm_pde
 };
 
+static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_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_arcturus_get_vm_pte_flags,
+	.get_vm_pde = gmc_v9_0_get_vm_pde
+};
+
 static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
 {
-	adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
+	switch (adev->asic_type) {
+	case CHIP_ARCTURUS:
+		adev->gmc.gmc_funcs = &gmc_v9_0_arcturus_gmc_funcs;
+		break;
+	default:
+		adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
+		break;
+	}
 }
 
 static void gmc_v9_0_set_umc_funcs(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/include/vega10_enum.h b/drivers/gpu/drm/amd/include/vega10_enum.h
index c14ba65..0d6d97d 100644
--- a/drivers/gpu/drm/amd/include/vega10_enum.h
+++ b/drivers/gpu/drm/amd/include/vega10_enum.h
@@ -1041,6 +1041,13 @@ MTYPE_CC                                 = 0x00000002,
 MTYPE_UC                                 = 0x00000003,
 } MTYPE;
 
+typedef enum MTYPE_ARCTURUS {
+MTYPE_ARCTURUS_NC                                 = 0x00000000,
+MTYPE_ARCTURUS_RW                                 = 0x00000001,
+MTYPE_ARCTURUS_CC                                 = 0x00000002,
+MTYPE_ARCTURUS_UC                                 = 0x00000003,
+} MTYPE_ARCTURUS;
+
 /*
  * RMI_CID enum
  */
-- 
2.7.4

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

[PATCH 3/5] drm/amdkfd: Postpone memory mapping flags calculation to mapping time

[Zeng, Oak]

Some mapping flags are decided by memory mapping destination which is not
know at memory object allocation time. So it is reasonable to decide memory
mapping flags at mapping time, instead of alloc time. Record memory allocation
flags during allocation time and calculate mapping flags during memory mapping
time.

Also made the memory mapping flage calculation to be asic-specific, because
different asic can have different mapping scheme.

The new memory mapping flag is decided by both memory allocation flags and
whether the memory mapping is to a remote device.

This is preparation work to introduce more sophisticated mapping scheme.

Change-Id: I98e7c47d850585ad7f0a9e11617c92b7a9aced3b
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h       |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 16 +++++-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h          |  4 ++++
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c           | 20 +++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c            | 19 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c            | 20 +++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c            | 21 ++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c            | 24 ++++++++++++++++++++++--
 8 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index e519df3..026475a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -58,6 +58,7 @@ struct kgd_mem {
 	uint64_t va;
 
 	uint32_t mapping_flags;
+	uint32_t alloc_flags;
 
 	atomic_t invalid;
 	struct amdkfd_process_info *process_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 44a52b0..f91ef48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1081,7 +1081,6 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 	int byte_align;
 	u32 domain, alloc_domain;
 	u64 alloc_flags;
-	uint32_t mapping_flags;
 	int ret;
 
 	/*
@@ -1143,16 +1142,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 			adev->asic_type != CHIP_VEGAM) ?
 			VI_BO_SIZE_ALIGN : 1;
 
-	mapping_flags = AMDGPU_VM_PAGE_READABLE;
-	if (flags & ALLOC_MEM_FLAGS_WRITABLE)
-		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-	if (flags & ALLOC_MEM_FLAGS_EXECUTABLE)
-		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
-	if (flags & ALLOC_MEM_FLAGS_COHERENT)
-		mapping_flags |= AMDGPU_VM_MTYPE_UC;
-	else
-		mapping_flags |= AMDGPU_VM_MTYPE_NC;
-	(*mem)->mapping_flags = mapping_flags;
+	(*mem)->alloc_flags = flags;
 
 	amdgpu_sync_create(&(*mem)->sync);
 
@@ -1298,6 +1288,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
+	struct amdgpu_device *bo_adev;
 	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
 	int ret;
 	struct amdgpu_bo *bo;
@@ -1315,6 +1306,9 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 		return -EINVAL;
 	}
 
+	bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	mem->mapping_flags = amdgpu_gmc_get_vm_mapping_flags(adev, mem->alloc_flags, bo_adev != adev);
+
 	/* Make sure restore is not running concurrently. Since we
 	 * don't map invalid userptr BOs, we rely on the next restore
 	 * worker to do the mapping
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 071145a..6bd0c28 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -105,6 +105,9 @@ 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 per asic vm mapping flags */
+	uint32_t (*get_vm_mapping_flags)(struct amdgpu_device *adev,
+			uint32_t alloc_flags, bool remote_mapping);
 };
 
 struct amdgpu_xgmi {
@@ -185,6 +188,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_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))
+#define amdgpu_gmc_get_vm_mapping_flags(adev, alloc_flags, remote_mapping) ((adev)->gmc.gmc_funcs->get_vm_mapping_flags((adev), (alloc_flags), (remote_mapping)))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4e3ac10..7e420e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -415,12 +415,30 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static uint32_t gmc_v10_0_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+	else
+		mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+	return mapping_flags;
+}
+
 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,
-	.get_vm_pde = gmc_v10_0_get_vm_pde
+	.get_vm_pde = gmc_v10_0_get_vm_pde,
+	.get_vm_mapping_flags = gmc_v10_0_get_vm_mapping_flags
 };
 
 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 b06d876..2b2af6a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -404,6 +404,22 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static uint32_t gmc_v6_0_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+	else
+		mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+	return mapping_flags;
+}
 static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
 					      bool value)
 {
@@ -1157,7 +1173,8 @@ 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
+	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags,
+	.get_vm_mapping_flags = gmc_v6_0_get_vm_mapping_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 75aa333..619862e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -481,6 +481,23 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static uint32_t gmc_v7_0_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+	else
+		mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+	return mapping_flags;
+}
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1353,7 +1370,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
 	.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
+	.get_vm_pde = gmc_v7_0_get_vm_pde,
+	.get_vm_mapping_flags = gmc_v7_0_get_vm_mapping_flags
 };
 
 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 8bf2ba3..d2cecb30 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -706,6 +706,24 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static uint32_t gmc_v8_0_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+	else
+		mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+	return mapping_flags;
+}
+
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1721,7 +1739,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
 	.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
+	.get_vm_pde = gmc_v8_0_get_vm_pde,
+	.get_vm_mapping_flags = gmc_v8_0_get_vm_mapping_flags
 };
 
 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 89064d9..adad7c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -667,12 +667,31 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static uint32_t gmc_v9_0_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+	else
+		mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+	return mapping_flags;
+}
+
+
 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,
-	.get_vm_pde = gmc_v9_0_get_vm_pde
+	.get_vm_pde = gmc_v9_0_get_vm_pde,
+	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
 };
 
 static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
@@ -680,7 +699,8 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
 	.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_arcturus_get_vm_pte_flags,
-	.get_vm_pde = gmc_v9_0_get_vm_pde
+	.get_vm_pde = gmc_v9_0_get_vm_pde,
+	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
 };
 
 static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
-- 
2.7.4

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

[PATCH 4/5] drm/amdgpu: Support snooped PTE flag

[Zeng, Oak]

Set snooped PTE flag according to mapping flag

Change-Id: I799f68ec7a5a1abf32075f5ef31051641a0b3736
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index adad7c4..8beefaf 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -639,6 +639,9 @@ static uint64_t gmc_v9_0_arcturus_get_vm_pte_flags(struct amdgpu_device *adev,
 	if (flags & AMDGPU_VM_PAGE_PRT)
 		pte_flag |= AMDGPU_PTE_PRT;
 
+	if (flags & AMDGPU_VM_PAGE_SNOOPED)
+		pte_flag |= AMDGPU_PTE_SNOOPED;
+
 	return pte_flag;
 }
 
-- 
2.7.4

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

[PATCH 5/5] amd/amdgpu: Introduce new page mapping scheme for arcturus

[Zeng, Oak]

The new memory mapping scheme is:
Fine-grain coherency: local CC and remote UC, with snoop.
Coarse-grain coherency: local RW and remote UC, with snoop.

Change-Id: I6a071249f953cbed813bfd953b6a2e0826f54f86
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c |  1 +
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 2b2af6a..51dae7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -420,6 +420,7 @@ static uint32_t gmc_v6_0_get_vm_mapping_flags(struct amdgpu_device *adev,
 
 	return mapping_flags;
 }
+
 static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
 					      bool value)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 8beefaf..718b388 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -687,6 +687,27 @@ static uint32_t gmc_v9_0_get_vm_mapping_flags(struct amdgpu_device *adev,
 	return mapping_flags;
 }
 
+static uint32_t gmc_v9_0_arcturus_get_vm_mapping_flags(struct amdgpu_device *adev,
+				uint32_t alloc_flags, bool remote_mapping)
+{
+	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
+
+	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
+	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
+	if ((alloc_flags & ALLOC_MEM_FLAGS_VRAM) && !remote_mapping) {
+		if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
+			mapping_flags |= AMDGPU_VM_MTYPE_CC;
+		else
+			mapping_flags |= AMDGPU_VM_MTYPE_RW;
+	} else {
+		mapping_flags |= AMDGPU_VM_MTYPE_UC;
+		mapping_flags |= AMDGPU_VM_PAGE_SNOOPED;
+	}
+
+	return mapping_flags;
+}
 
 static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
@@ -703,7 +724,7 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
 	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
 	.get_vm_pte_flags = gmc_v9_0_arcturus_get_vm_pte_flags,
 	.get_vm_pde = gmc_v9_0_get_vm_pde,
-	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
+	.get_vm_mapping_flags = gmc_v9_0_arcturus_get_vm_mapping_flags
 };
 
 static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
-- 
2.7.4

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

Re: [PATCH 2/5] drm/amdgpu: Support new arcturus mtype

[Kuehling, Felix]

On 2019-08-06 22:31, Zeng, Oak wrote:
> Arcturus repurposed mtype WC to RW. Modify gmc functions
> to support the new mtype
>
> Change-Id: Idc338e5386a57020f45262025e2664ab4ba9f291
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c     | 61 ++++++++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/include/vega10_enum.h |  7 ++++
>   2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index a2aa35e..89064d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -599,6 +599,50 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
>   	return pte_flag;
>   }
>   
> +static uint64_t gmc_v9_0_arcturus_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 (adev->gmc.zfb_size > 0)
> +		pte_flag |= AMDGPU_PTE_SYSTEM;
> +
> +	switch (flags & AMDGPU_VM_MTYPE_MASK) {
> +	case AMDGPU_VM_MTYPE_DEFAULT:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +		break;
> +	case AMDGPU_VM_MTYPE_NC:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +		break;
> +	case AMDGPU_VM_MTYPE_RW:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_RW);
> +		break;
> +	case AMDGPU_VM_MTYPE_CC:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_CC);
> +		break;
> +	case AMDGPU_VM_MTYPE_UC:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_UC);
> +		break;
> +	default:
> +		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +		break;
> +	}
> +
> +	if (flags & AMDGPU_VM_PAGE_PRT)
> +		pte_flag |= AMDGPU_PTE_PRT;
> +
> +	return pte_flag;
> +}

Adding a whole new function for Arcturus is overkill. Just add MTYPE_RW 
to the existing function.

         switch(flags & AMDGPU_VM_MTYPE_MASK) {
         ...
         case AMDGPU_VM_MTYPE_WC:
                 pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
                 break;
         case AMDGPU_VM_MTYPE_RW:
                 pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
                 break;
         ...
         }

The fact that WC isn't valid on Arcturus and RW isn't valid on other 
Vega GPUs could be handled in higher level code.


> +
> +
>   static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   				uint64_t *addr, uint64_t *flags)
>   {
> @@ -631,9 +675,24 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>   	.get_vm_pde = gmc_v9_0_get_vm_pde
>   };
>   
> +static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_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_arcturus_get_vm_pte_flags,
> +	.get_vm_pde = gmc_v9_0_get_vm_pde
> +};
> +
>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
>   {
> -	adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
> +	switch (adev->asic_type) {
> +	case CHIP_ARCTURUS:
> +		adev->gmc.gmc_funcs = &gmc_v9_0_arcturus_gmc_funcs;
> +		break;
> +	default:
> +		adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
> +		break;
> +	}
>   }
>   
>   static void gmc_v9_0_set_umc_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/include/vega10_enum.h b/drivers/gpu/drm/amd/include/vega10_enum.h
> index c14ba65..0d6d97d 100644
> --- a/drivers/gpu/drm/amd/include/vega10_enum.h
> +++ b/drivers/gpu/drm/amd/include/vega10_enum.h
> @@ -1041,6 +1041,13 @@ MTYPE_CC                                 = 0x00000002,
>   MTYPE_UC                                 = 0x00000003,
>   } MTYPE;
>   
> +typedef enum MTYPE_ARCTURUS {
> +MTYPE_ARCTURUS_NC                                 = 0x00000000,
> +MTYPE_ARCTURUS_RW                                 = 0x00000001,
> +MTYPE_ARCTURUS_CC                                 = 0x00000002,
> +MTYPE_ARCTURUS_UC                                 = 0x00000003,
> +} MTYPE_ARCTURUS;

This is overkill. Most of the MTYPEs are the same on Arcturus. You can 
just define MTYPE_RW as an alias of MTYPE_WC:

typedef enum MTYPE {
MTYPE_NC                                 = 0x00000000,
MTYPE_WC                                 = 0x00000001,
MTYPE_RW                                 = 0x00000001, /* Arcturus */
MTYPE_CC                                 = 0x00000002,
MTYPE_UC                                 = 0x00000003,
} MTYPE;

Regards,
   Felix


> +
>   /*
>    * RMI_CID enum
>    */
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdkfd: Postpone memory mapping flags calculation to mapping time

[Kuehling, Felix]

On 2019-08-06 22:31, Zeng, Oak wrote:
> Some mapping flags are decided by memory mapping destination which is not
> know at memory object allocation time. So it is reasonable to decide memory
> mapping flags at mapping time, instead of alloc time. Record memory allocation
> flags during allocation time and calculate mapping flags during memory mapping
> time.
>
> Also made the memory mapping flage calculation to be asic-specific, because
> different asic can have different mapping scheme.
>
> The new memory mapping flag is decided by both memory allocation flags and
> whether the memory mapping is to a remote device.
>
> This is preparation work to introduce more sophisticated mapping scheme.
>
> Change-Id: I98e7c47d850585ad7f0a9e11617c92b7a9aced3b
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h       |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 16 +++++-----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h          |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c           | 20 +++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c            | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c            | 20 +++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c            | 21 ++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c            | 24 ++++++++++++++++++++++--
>   8 files changed, 108 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index e519df3..026475a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -58,6 +58,7 @@ struct kgd_mem {
>   	uint64_t va;
>   
>   	uint32_t mapping_flags;
> +	uint32_t alloc_flags;
>   
>   	atomic_t invalid;
>   	struct amdkfd_process_info *process_info;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 44a52b0..f91ef48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1081,7 +1081,6 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   	int byte_align;
>   	u32 domain, alloc_domain;
>   	u64 alloc_flags;
> -	uint32_t mapping_flags;
>   	int ret;
>   
>   	/*
> @@ -1143,16 +1142,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   			adev->asic_type != CHIP_VEGAM) ?
>   			VI_BO_SIZE_ALIGN : 1;
>   
> -	mapping_flags = AMDGPU_VM_PAGE_READABLE;
> -	if (flags & ALLOC_MEM_FLAGS_WRITABLE)
> -		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> -	if (flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> -		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> -	if (flags & ALLOC_MEM_FLAGS_COHERENT)
> -		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> -	else
> -		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> -	(*mem)->mapping_flags = mapping_flags;
> +	(*mem)->alloc_flags = flags;
>   
>   	amdgpu_sync_create(&(*mem)->sync);
>   
> @@ -1298,6 +1288,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
>   {
>   	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +	struct amdgpu_device *bo_adev;
>   	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>   	int ret;
>   	struct amdgpu_bo *bo;
> @@ -1315,6 +1306,9 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   		return -EINVAL;
>   	}
>   
> +	bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	mem->mapping_flags = amdgpu_gmc_get_vm_mapping_flags(adev, mem->alloc_flags, bo_adev != adev);
> +
>   	/* Make sure restore is not running concurrently. Since we
>   	 * don't map invalid userptr BOs, we rely on the next restore
>   	 * worker to do the mapping
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 071145a..6bd0c28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -105,6 +105,9 @@ 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 per asic vm mapping flags */
> +	uint32_t (*get_vm_mapping_flags)(struct amdgpu_device *adev,
> +			uint32_t alloc_flags, bool remote_mapping);

You'll probably need more than just a boolean for remote mappings. The 
type of mapping may also depend on whether the mapping uses XGMI or 
PCIe. (PCIe P2P isn't supported upstream yet, but it is on ROCm release 
branches). Therefore it's probably better to pass in the two devices and 
let the get_vm_mapping_flags function figure out the details, including 
whether both devices are in the same XGMI hive.

Regards,
   Felix


>   };
>   
>   struct amdgpu_xgmi {
> @@ -185,6 +188,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_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))
> +#define amdgpu_gmc_get_vm_mapping_flags(adev, alloc_flags, remote_mapping) ((adev)->gmc.gmc_funcs->get_vm_mapping_flags((adev), (alloc_flags), (remote_mapping)))
>   
>   /**
>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 4e3ac10..7e420e0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -415,12 +415,30 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static uint32_t gmc_v10_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
>   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,
> -	.get_vm_pde = gmc_v10_0_get_vm_pde
> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v10_0_get_vm_mapping_flags
>   };
>   
>   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 b06d876..2b2af6a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -404,6 +404,22 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v6_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>   					      bool value)
>   {
> @@ -1157,7 +1173,8 @@ 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
> +	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags,
> +	.get_vm_mapping_flags = gmc_v6_0_get_vm_mapping_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 75aa333..619862e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -481,6 +481,23 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v7_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1353,7 +1370,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>   	.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
> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v7_0_get_vm_mapping_flags
>   };
>   
>   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 8bf2ba3..d2cecb30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -706,6 +706,24 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v8_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1721,7 +1739,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>   	.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
> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v8_0_get_vm_mapping_flags
>   };
>   
>   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 89064d9..adad7c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -667,12 +667,31 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static uint32_t gmc_v9_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
> +
>   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,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
>   };
>   
>   static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
> @@ -680,7 +699,8 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
>   	.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_arcturus_get_vm_pte_flags,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
>   };
>   
>   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

Re: [PATCH 5/5] amd/amdgpu: Introduce new page mapping scheme for arcturus

[Kuehling, Felix]

On 2019-08-06 22:31, Zeng, Oak wrote:
> The new memory mapping scheme is:
> Fine-grain coherency: local CC and remote UC, with snoop.
> Coarse-grain coherency: local RW and remote UC, with snoop.
>
> Change-Id: I6a071249f953cbed813bfd953b6a2e0826f54f86
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c |  1 +
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 23 ++++++++++++++++++++++-
>   2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 2b2af6a..51dae7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -420,6 +420,7 @@ static uint32_t gmc_v6_0_get_vm_mapping_flags(struct amdgpu_device *adev,
>   
>   	return mapping_flags;
>   }
> +
>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>   					      bool value)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 8beefaf..718b388 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -687,6 +687,27 @@ static uint32_t gmc_v9_0_get_vm_mapping_flags(struct amdgpu_device *adev,
>   	return mapping_flags;
>   }
>   
> +static uint32_t gmc_v9_0_arcturus_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping)
> +{
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if ((alloc_flags & ALLOC_MEM_FLAGS_VRAM) && !remote_mapping) {
> +		if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +			mapping_flags |= AMDGPU_VM_MTYPE_CC;
> +		else
> +			mapping_flags |= AMDGPU_VM_MTYPE_RW;
> +	} else {
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +		mapping_flags |= AMDGPU_VM_PAGE_SNOOPED;

This seems incorrect. It treats all system memory the same as remote 
VRAM as uncached on the GPU.

Regards,
   Felix

> +	}
> +
> +	return mapping_flags;
> +}
>   
>   static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
> @@ -703,7 +724,7 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>   	.get_vm_pte_flags = gmc_v9_0_arcturus_get_vm_pte_flags,
>   	.get_vm_pde = gmc_v9_0_get_vm_pde,
> -	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
> +	.get_vm_mapping_flags = gmc_v9_0_arcturus_get_vm_mapping_flags
>   };
>   
>   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

Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Christian König]

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype
> is recently introduced for arcturus. Also add definition
> for the cachable/snoopable bit, which will be used later
> in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)	((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK	AMDGPU_PTE_MTYPE_VG10(3ULL)
>   
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +	AMDGPU_MTYPE_NC = 0,
> +	AMDGPU_MTYPE_WC = 1,
> +	AMDGPU_MTYPE_CC = 2,
> +	AMDGPU_MTYPE_UC = 3,
> +	AMDGPU_MTYPE_RW = 4,
> +};
>   
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC		(3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC		(4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW		(5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED		(1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is 
allowed to be aware of.

Christian.

>   
>   struct drm_amdgpu_gem_va {
>   	/** GEM object handle */

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

RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Zeng, Oak]

Hi Christian,

My understanding of the snoop bit (C bit in the PTE definition) is to probe remote gpu's L2 cache after this gpu write remote gpu's vram. Is this correct? I am still checking this point with HW engineer.

If this is correct, then the snooping (or probing) is a way to maintain certain cache coherency when one memory is access by two masters (for example two gpu). With existing AMDGPU_VM_ definitions in amdgpu_drm.h, how does a user implement the request like: I want a trunk of vram physically in a remote gpu, I want to access it in a uncached way (AMDGPU_VM_MTYPE_UC) but I want to probe remote gpu's cache when I modify this vram.

From PTE's definition, both C bit and mtype and R/W/X bits are just flags to enable user to program page access behavior. Any detail reason why we shouldn't expose the snoop bit?

Regards,
Oak

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <Oak.Zeng@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Keely, Sean <Sean.Keely@amd.com>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype is recently 
> introduced for arcturus. Also add definition for the 
> cachable/snoopable bit, which will be used later in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)	((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK	AMDGPU_PTE_MTYPE_VG10(3ULL)
>   
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +	AMDGPU_MTYPE_NC = 0,
> +	AMDGPU_MTYPE_WC = 1,
> +	AMDGPU_MTYPE_CC = 2,
> +	AMDGPU_MTYPE_UC = 3,
> +	AMDGPU_MTYPE_RW = 4,
> +};
>   
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h 
> b/include/uapi/drm/amdgpu_drm.h index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC		(3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC		(4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW		(5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED		(1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is allowed to be aware of.

Christian.

>   
>   struct drm_amdgpu_gem_va {
>   	/** GEM object handle */

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

Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 4341 bytes --]

The snoop bit is for snooping the CPU cache by the GPU when doing system memory mappings.

Alex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, August 8, 2019 12:02 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp93gSJqDPrsil@public.gmane.org.org <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; Keely, Sean <Sean.Keely@amd.com>
Subject: RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Hi Christian,

My understanding of the snoop bit (C bit in the PTE definition) is to probe remote gpu's L2 cache after this gpu write remote gpu's vram. Is this correct? I am still checking this point with HW engineer.

If this is correct, then the snooping (or probing) is a way to maintain certain cache coherency when one memory is access by two masters (for example two gpu). With existing AMDGPU_VM_ definitions in amdgpu_drm.h, how does a user implement the request like: I want a trunk of vram physically in a remote gpu, I want to access it in a uncached way (AMDGPU_VM_MTYPE_UC) but I want to probe remote gpu's cache when I modify this vram.

>From PTE's definition, both C bit and mtype and R/W/X bits are just flags to enable user to program page access behavior. Any detail reason why we shouldn't expose the snoop bit?

Regards,
Oak

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; Keely, Sean <Sean.Keely-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype is recently
> introduced for arcturus. Also add definition for the
> cachable/snoopable bit, which will be used later in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)    ((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK  AMDGPU_PTE_MTYPE_VG10(3ULL)
>
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +     AMDGPU_MTYPE_NC = 0,
> +     AMDGPU_MTYPE_WC = 1,
> +     AMDGPU_MTYPE_CC = 2,
> +     AMDGPU_MTYPE_UC = 3,
> +     AMDGPU_MTYPE_RW = 4,
> +};
>
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC          (3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC          (4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW           (5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED               (1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is allowed to be aware of.

Christian.

>
>   struct drm_amdgpu_gem_va {
>        /** GEM object handle */

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

[-- Attachment #1.2: Type: text/html, Size: 6787 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Zeng, Oak]

[-- Attachment #1.1: Type: text/plain, Size: 5750 bytes --]

+@Mirza, Jimshed<mailto:Jimshed.Mirza-5C7GfCeVMHo@public.gmane.org>: do we have the ability to snoop another GPU's cache, in a multiple GPU system?

Regards,
Oak

From: Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, August 8, 2019 12:10 PM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; Keely, Sean <Sean.Keely@amd.com>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

The snoop bit is for snooping the CPU cache by the GPU when doing system memory mappings.

Alex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>
Sent: Thursday, August 8, 2019 12:02 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig@amd.com>>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org<mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>>; Keely, Sean <Sean.Keely-5C7GfCeVMHo@public.gmane.org<mailto:Sean.Keely-5C7GfCeVMHo@public.gmane.org>>
Subject: RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Hi Christian,

My understanding of the snoop bit (C bit in the PTE definition) is to probe remote gpu's L2 cache after this gpu write remote gpu's vram. Is this correct? I am still checking this point with HW engineer.

If this is correct, then the snooping (or probing) is a way to maintain certain cache coherency when one memory is access by two masters (for example two gpu). With existing AMDGPU_VM_ definitions in amdgpu_drm.h, how does a user implement the request like: I want a trunk of vram physically in a remote gpu, I want to access it in a uncached way (AMDGPU_VM_MTYPE_UC) but I want to probe remote gpu's cache when I modify this vram.

>From PTE's definition, both C bit and mtype and R/W/X bits are just flags to enable user to program page access behavior. Any detail reason why we shouldn't expose the snoop bit?

Regards,
Oak

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>; amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org<mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig@amd.com>>; Keely, Sean <Sean.Keely-5C7GfCeVMHo@public.gmane.org<mailto:Sean.Keely-5C7GfCeVMHo@public.gmane.org>>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype is recently
> introduced for arcturus. Also add definition for the
> cachable/snoopable bit, which will be used later in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)    ((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK  AMDGPU_PTE_MTYPE_VG10(3ULL)
>
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +     AMDGPU_MTYPE_NC = 0,
> +     AMDGPU_MTYPE_WC = 1,
> +     AMDGPU_MTYPE_CC = 2,
> +     AMDGPU_MTYPE_UC = 3,
> +     AMDGPU_MTYPE_RW = 4,
> +};
>
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC          (3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC          (4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW           (5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED               (1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is allowed to be aware of.

Christian.

>
>   struct drm_amdgpu_gem_va {
>        /** GEM object handle */

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 10880 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

[Koenig, Christian]

[-- Attachment #1.1: Type: text/plain, Size: 4702 bytes --]

Yeah, exactly. This is controlling PCIe bus request behavior and routing.

That is something we never ever want to expose to userspace.

Christian.

Am 08.08.19 um 18:09 schrieb Deucher, Alexander:
The snoop bit is for snooping the CPU cache by the GPU when doing system memory mappings.

Alex
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of Zeng, Oak <Oak.Zeng@amd.com><mailto:Oak.Zeng@amd.com>
Sent: Thursday, August 8, 2019 12:02 PM
To: Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com><mailto:Felix.Kuehling@amd.com>; Keely, Sean <Sean.Keely@amd.com><mailto:Sean.Keely@amd.com>
Subject: RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Hi Christian,

My understanding of the snoop bit (C bit in the PTE definition) is to probe remote gpu's L2 cache after this gpu write remote gpu's vram. Is this correct? I am still checking this point with HW engineer.

If this is correct, then the snooping (or probing) is a way to maintain certain cache coherency when one memory is access by two masters (for example two gpu). With existing AMDGPU_VM_ definitions in amdgpu_drm.h, how does a user implement the request like: I want a trunk of vram physically in a remote gpu, I want to access it in a uncached way (AMDGPU_VM_MTYPE_UC) but I want to probe remote gpu's cache when I modify this vram.

From PTE's definition, both C bit and mtype and R/W/X bits are just flags to enable user to program page access behavior. Any detail reason why we shouldn't expose the snoop bit?

Regards,
Oak

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <Oak.Zeng@amd.com><mailto:Oak.Zeng@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com><mailto:Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>; Keely, Sean <Sean.Keely@amd.com><mailto:Sean.Keely@amd.com>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype is recently
> introduced for arcturus. Also add definition for the
> cachable/snoopable bit, which will be used later in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com><mailto:Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)    ((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK  AMDGPU_PTE_MTYPE_VG10(3ULL)
>
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +     AMDGPU_MTYPE_NC = 0,
> +     AMDGPU_MTYPE_WC = 1,
> +     AMDGPU_MTYPE_CC = 2,
> +     AMDGPU_MTYPE_UC = 3,
> +     AMDGPU_MTYPE_RW = 4,
> +};
>
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC          (3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC          (4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW           (5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED               (1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is allowed to be aware of.

Christian.

>
>   struct drm_amdgpu_gem_va {
>        /** GEM object handle */

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


[-- Attachment #1.2: Type: text/html, Size: 8075 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

RE: [PATCH 3/5] drm/amdkfd: Postpone memory mapping flags calculation to mapping time

[Zeng, Oak]

Hi Felix/Sean,

See one comment inline [Oak]

Regards,
Oak

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling@amd.com> 
Sent: Wednesday, August 7, 2019 12:05 AM
To: Zeng, Oak <Oak.Zeng@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Koenig, Christian <Christian.Koenig@amd.com>; Keely, Sean <Sean.Keely@amd.com>
Subject: Re: [PATCH 3/5] drm/amdkfd: Postpone memory mapping flags calculation to mapping time

On 2019-08-06 22:31, Zeng, Oak wrote:
> Some mapping flags are decided by memory mapping destination which is 
> not know at memory object allocation time. So it is reasonable to 
> decide memory mapping flags at mapping time, instead of alloc time. 
> Record memory allocation flags during allocation time and calculate 
> mapping flags during memory mapping time.
>
> Also made the memory mapping flage calculation to be asic-specific, 
> because different asic can have different mapping scheme.
>
> The new memory mapping flag is decided by both memory allocation flags 
> and whether the memory mapping is to a remote device.
>
> This is preparation work to introduce more sophisticated mapping scheme.
>
> Change-Id: I98e7c47d850585ad7f0a9e11617c92b7a9aced3b
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h       |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 16 +++++-----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h          |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c           | 20 +++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c            | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c            | 20 +++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c            | 21 ++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c            | 24 ++++++++++++++++++++++--
>   8 files changed, 108 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index e519df3..026475a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -58,6 +58,7 @@ struct kgd_mem {
>   	uint64_t va;
>   
>   	uint32_t mapping_flags;
> +	uint32_t alloc_flags;
>   
>   	atomic_t invalid;
>   	struct amdkfd_process_info *process_info; diff --git 
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 44a52b0..f91ef48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1081,7 +1081,6 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   	int byte_align;
>   	u32 domain, alloc_domain;
>   	u64 alloc_flags;
> -	uint32_t mapping_flags;
>   	int ret;
>   
>   	/*
> @@ -1143,16 +1142,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   			adev->asic_type != CHIP_VEGAM) ?
>   			VI_BO_SIZE_ALIGN : 1;
>   
> -	mapping_flags = AMDGPU_VM_PAGE_READABLE;
> -	if (flags & ALLOC_MEM_FLAGS_WRITABLE)
> -		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> -	if (flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> -		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> -	if (flags & ALLOC_MEM_FLAGS_COHERENT)
> -		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> -	else
> -		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> -	(*mem)->mapping_flags = mapping_flags;
> +	(*mem)->alloc_flags = flags;
>   
>   	amdgpu_sync_create(&(*mem)->sync);
>   
> @@ -1298,6 +1288,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
>   {
>   	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +	struct amdgpu_device *bo_adev;
>   	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>   	int ret;
>   	struct amdgpu_bo *bo;
> @@ -1315,6 +1306,9 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   		return -EINVAL;
>   	}
>   
> +	bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	mem->mapping_flags = amdgpu_gmc_get_vm_mapping_flags(adev, 
> +mem->alloc_flags, bo_adev != adev);
> +
>   	/* Make sure restore is not running concurrently. Since we
>   	 * don't map invalid userptr BOs, we rely on the next restore
>   	 * worker to do the mapping
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 071145a..6bd0c28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -105,6 +105,9 @@ 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 per asic vm mapping flags */
> +	uint32_t (*get_vm_mapping_flags)(struct amdgpu_device *adev,
> +			uint32_t alloc_flags, bool remote_mapping);

You'll probably need more than just a boolean for remote mappings. The type of mapping may also depend on whether the mapping uses XGMI or PCIe. (PCIe P2P isn't supported upstream yet, but it is on ROCm release branches). Therefore it's probably better to pass in the two devices and let the get_vm_mapping_flags function figure out the details, including whether both devices are in the same XGMI hive.


[Oak] Currently for all the remote vram access, either through xgmi or through pcie, we use the same mapping scheme (UC+C). Is this correct? If So a boolean remote_mapping is good enough for current scheme. Are we going to introduce new scheme for remote mapping through pcie (when access vram not in the same xgmi hive)?

Regards,
   Felix


>   };
>   
>   struct amdgpu_xgmi {
> @@ -185,6 +188,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_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))
> +#define amdgpu_gmc_get_vm_mapping_flags(adev, alloc_flags, 
> +remote_mapping) ((adev)->gmc.gmc_funcs->get_vm_mapping_flags((adev), 
> +(alloc_flags), (remote_mapping)))
>   
>   /**
>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible 
> through the BAR diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 4e3ac10..7e420e0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -415,12 +415,30 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static uint32_t gmc_v10_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping) {
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
>   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,
> -	.get_vm_pde = gmc_v10_0_get_vm_pde
> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v10_0_get_vm_mapping_flags
>   };
>   
>   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 b06d876..2b2af6a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -404,6 +404,22 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v6_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping) {
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>   					      bool value)
>   {
> @@ -1157,7 +1173,8 @@ 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
> +	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags,
> +	.get_vm_mapping_flags = gmc_v6_0_get_vm_mapping_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 75aa333..619862e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -481,6 +481,23 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v7_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping) {
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1353,7 +1370,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>   	.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
> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v7_0_get_vm_mapping_flags
>   };
>   
>   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 8bf2ba3..d2cecb30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -706,6 +706,24 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>   }
>   
> +static uint32_t gmc_v8_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping) {
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
> +
>   /**
>    * gmc_v8_0_set_fault_enable_default - update VM fault handling
>    *
> @@ -1721,7 +1739,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>   	.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
> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v8_0_get_vm_mapping_flags
>   };
>   
>   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 89064d9..adad7c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -667,12 +667,31 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>   	}
>   }
>   
> +static uint32_t gmc_v9_0_get_vm_mapping_flags(struct amdgpu_device *adev,
> +				uint32_t alloc_flags, bool remote_mapping) {
> +	uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE;
> +
> +	if (alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> +		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
> +	if (alloc_flags & ALLOC_MEM_FLAGS_COHERENT)
> +		mapping_flags |= AMDGPU_VM_MTYPE_UC;
> +	else
> +		mapping_flags |= AMDGPU_VM_MTYPE_NC;
> +
> +	return mapping_flags;
> +}
> +
> +
>   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,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
>   };
>   
>   static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = { 
> @@ -680,7 +699,8 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
>   	.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_arcturus_get_vm_pte_flags,
> -	.get_vm_pde = gmc_v9_0_get_vm_pde
> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
> +	.get_vm_mapping_flags = gmc_v9_0_get_vm_mapping_flags
>   };
>   
>   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

[PATCH 2/5] drm/amdgpu: Support new arcturus mtype

[Zeng, Oak]

Arcturus repurposed mtype WC to RW. Modify gmc functions
to support the new mtype

Change-Id: Idc338e5386a57020f45262025e2664ab4ba9f291
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c     | 3 +++
 drivers/gpu/drm/amd/include/vega10_enum.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index a2aa35e..e6adc86 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -582,6 +582,9 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
 	case AMDGPU_VM_MTYPE_WC:
 		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
 		break;
+	case AMDGPU_VM_MTYPE_RW:
+		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
+		break;
 	case AMDGPU_VM_MTYPE_CC:
 		pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_CC);
 		break;
diff --git a/drivers/gpu/drm/amd/include/vega10_enum.h b/drivers/gpu/drm/amd/include/vega10_enum.h
index c14ba65..adf1b75 100644
--- a/drivers/gpu/drm/amd/include/vega10_enum.h
+++ b/drivers/gpu/drm/amd/include/vega10_enum.h
@@ -1037,6 +1037,7 @@ TCC_CACHE_POLICY_STREAM                  = 0x00000001,
 typedef enum MTYPE {
 MTYPE_NC                                 = 0x00000000,
 MTYPE_WC                                 = 0x00000001,
+MTYPE_RW                                 = 0x00000001,
 MTYPE_CC                                 = 0x00000002,
 MTYPE_UC                                 = 0x00000003,
 } MTYPE;
-- 
2.7.4

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