On 7/20/2022 7:18 PM, Felix Kuehling wrote:
On 2022-07-18 18:52, Rajneesh Bhardwaj wrote:
This simplifies existing coherence handling for Arcturus and Aldabaran
to account for !coherent && uncached scenarios.
Cc: Joseph Greathouse <joseph.greathouse@xxxxxxx>
Cc: Alex Deucher <Alexander.Deucher@xxxxxxx>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxx>
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 53 +++++++++----------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index d1657de5f875..0fdfd79f69ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -471,45 +471,44 @@ static uint64_t get_pte_flags(struct
amdgpu_device *adev, struct kgd_mem *mem)
switch (adev->asic_type) {
case CHIP_ARCTURUS:
- if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- if (bo_adev == adev)
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
- else
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
- } else {
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
- }
- break;
case CHIP_ALDEBARAN:
- if (coherent && uncached) {
- if (adev->gmc.xgmi.connected_to_cpu ||
- !(mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM))
- snoop = true;
- mapping_flags |= AMDGPU_VM_MTYPE_UC;
- } else if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
+ if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev) {
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
- if (adev->gmc.xgmi.connected_to_cpu)
+ if (uncached)
+ mapping_flags |= AMDGPU_VM_MTYPE_UC;
+ else if (coherent)
+ mapping_flags |= AMDGPU_VM_MTYPE_CC;
+ else
+ mapping_flags |= AMDGPU_VM_MTYPE_RW;
+ if (adev->asic_type == CHIP_ALDEBARAN &&
+ adev->gmc.xgmi.connected_to_cpu)
snoop = true;
} else {
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
+ if (uncached || coherent)
+ mapping_flags |= AMDGPU_VM_MTYPE_UC;
+ else
+ mapping_flags |= AMDGPU_VM_MTYPE_NC;
if (amdgpu_xgmi_same_hive(adev, bo_adev))
snoop = true;
}
} else {
+ if (uncached || coherent)
+ mapping_flags |= AMDGPU_VM_MTYPE_UC;
+ else
+ mapping_flags |= AMDGPU_VM_MTYPE_NC;
snoop = true;
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
}
break;
default:
- mapping_flags |= coherent ?
- AMDGPU_VM_MTYPE_UC : AMDGPU_VM_MTYPE_NC;
+ if (uncached || coherent)
+ mapping_flags |= AMDGPU_VM_MTYPE_UC;
+ else
+ mapping_flags |= AMDGPU_VM_MTYPE_NC;
+
+ if (!(mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM))
+ snoop = true;
+
+
With the two extra blank lines removed, this patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Please check whether a similar cleanup can be made in
svm_range_get_pte_flags, or maybe even, whether common code can be
factored out of those two functions.
Thanks Felix for the review. Do you want me to send V2 with two lines
removed or just apply to amd-staging-drm-next after deleting those two
lines?
I will check svm_range_get_pte_flags and see if I can cleanup the code
there and get back to you.
Regards,
Felix
}
pte_flags = amdgpu_gem_va_map_flags(adev, mapping_flags);
