[PATCH] drm/amdgpu: Refactor code to handle non coherent and uncached

[Felix Kuehling]

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 at amd.com>
> Cc: Alex Deucher <Alexander.Deucher at amd.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj at amd.com>
> ---
>   .../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 at amd.com>

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.

Regards,
   Felix


>   	}
>   
>   	pte_flags = amdgpu_gem_va_map_flags(adev, mapping_flags);