Re: [PATCH 048/159] drm/amdgpu: new cache coherence change for Aldebaran

[Jerry Zhang]

On 2021/2/25 上午6:17, Alex Deucher wrote:

From: Eric Huang 

To support new cache coherence HW on A+A platform mainly in KFD.

Signed-off-by: Eric Huang 
Reviewed-by: Oak Zeng 
Signed-off-by: Alex Deucher 
---
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 30 +--
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c |  3 ++
  2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 8781051afb69..30e41d1b3256 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -30,6 +30,7 @@
  #include "amdgpu_amdkfd.h"
  #include "amdgpu_dma_buf.h"
  #include 
+#include "amdgpu_xgmi.h"
  
  /* BO flag to indicate a KFD userptr BO */

  #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -404,6 +405,8 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, 
struct kgd_mem *mem)
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
+   uint64_t pte_flags;
+   bool snoop = false;
  
  	mapping_flags = AMDGPU_VM_PAGE_READABLE;

if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
@@ -413,7 +416,6 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, 
struct kgd_mem *mem)
  
  	switch (adev->asic_type) {

case CHIP_ARCTURUS:
-   case CHIP_ALDEBARAN:
if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
@@ -425,12 +427,36 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, 
struct kgd_mem *mem)
AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
}
break;
+   case CHIP_ALDEBARAN:
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+   if (bo_adev == adev) {
+   mapping_flags |= AMDGPU_VM_MTYPE_RW;

For local VRAM, if it's shared by peer GPU, RW could keep cache coherence?

Do we need UC mtype in this case?


+   if (adev->gmc.xgmi.connected_to_cpu)
+   snoop = true;
+   } else {
+   mapping_flags |= AMDGPU_VM_MTYPE_NC;
+   if (amdgpu_xgmi_same_hive(adev, bo_adev))
+   snoop = true;

Do we need to check same hive id here?

if so , CHIP_ARCTURUS may add similar check.

Why not use UC mtype here for gpu-gpu memory share? NC could keep cache 
coherence among gpu sharing? by PTE_SNOOPED type?



+   }
+   } else {
+   snoop = true;
+   if (adev->gmc.xgmi.connected_to_cpu)
+   /* system memory uses NC on A+A */
+   mapping_flags |= AMDGPU_VM_MTYPE_NC;
+   else
+   mapping_flags |= coherent ?
+   AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
+   }
+   break;
default:
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
}
  
-	return amdgpu_gem_va_map_flags(adev, mapping_flags);

+   pte_flags = amdgpu_gem_va_map_flags(adev, mapping_flags);
+   pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
+
+   return pte_flags;
  }
  
  /* add_bo_to_vm - Add a BO to a VM

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 94552048aada..32b552e54e77 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1040,6 +1040,9 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device 
*adev,
!(*flags & AMDGPU_PTE_SYSTEM) &&
mapping->bo_va->is_xgmi)
*flags |= AMDGPU_PTE_SNOOPED;
+
+   if (adev->asic_type == CHIP_ALDEBARAN)
+   *flags |= mapping->flags & AMDGPU_PTE_SNOOPED;
It breaks the change of "drm/amdgpu: Fix an omission when adding 
Aldebaran support", which sets PTE_SNOOPED for xgmi VRAM bo only.


And it seems no need to set PTE_SNOOPED here, since get_pte_flags() has 
handled that.



  }
  
  static unsigned gmc_v9_0_get_vbios_fb_size(struct amdgpu_device *adev)




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

Re: [PATCH] drm/amdgpu: fix duplicated code

[Jerry Zhang]

On 03/21/2017 10:10 AM, Chunming Zhou wrote:

it could come from branch merge.

Change-Id: I16959aad6ca6d64cb8330f23ee6472eec4cf2a3e
Signed-off-by: Chunming Zhou 

Reviewed-by: Junwei Zhang 

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 81c3c75..dd7df45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -990,10 +990,6 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,

ring = container_of(vm->entity.sched, struct amdgpu_ring, sched);

-   memset(, 0, sizeof(params));
-   params.adev = adev;
-   params.src = src;
-
/* sync to everything on unmapping */
if (!(flags & AMDGPU_PTE_VALID))
owner = AMDGPU_FENCE_OWNER_UNDEFINED;


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