[PATCH] drm/amdgpu: Update the impelmentation of AMDGPU_PTE_MTYPE_GFX12

[Felix Kuehling]

On 2024-05-20 5:14, Shane Xiao wrote:
> This patch changes the implementation of AMDGPU_PTE_MTYPE_GFX12,
> clear the bits before setting the new one.
> This fixed the potential issue that GFX12 setting memory to NC.
> 
> v2: Clear mtype field before setting the new one (Alex)
> 
> Signed-off-by: longlyao <Longlong.Yao at amd.com>
> Signed-off-by: Shane Xiao <shane.xiao at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  7 +++++--
>  drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 23 +++++++++++------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index bc71b44387b2..99b246e82ed6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -116,8 +116,11 @@ struct amdgpu_mem_stats;
>  #define AMDGPU_PTE_PRT_FLAG(adev)	\
>  	((amdgpu_ip_version((adev), GC_HWIP, 0) >= IP_VERSION(12, 0, 0)) ? AMDGPU_PTE_PRT_GFX12 : AMDGPU_PTE_PRT)
>  
> -#define AMDGPU_PTE_MTYPE_GFX12(a)	((uint64_t)(a) << 54)
> -#define AMDGPU_PTE_MTYPE_GFX12_MASK	AMDGPU_PTE_MTYPE_GFX12(3ULL)
> +#define AMDGPU_PTE_MTYPE_GFX12_SHIFT(mtype)	((uint64_t)(mytype) << 54)

You have a typo here: mytype -> mtype .

Regards,
  Felix


> +#define AMDGPU_PTE_MTYPE_GFX12_MASK	AMDGPU_PTE_MTYPE_GFX12_SHIFT(3ULL)
> +#define AMDGPU_PTE_MTYPE_GFX12(flags, mtype)	\
> +	((flags) & ((~AMDGPU_PTE_MTYPE_GFX12_MASK)) |	\
> +	  AMDGPU_PTE_MTYPE_GFX12_SHIFT(mtype))
>  
>  #define AMDGPU_PTE_IS_PTE		(1ULL << 63)
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> index e2c6ec3cc4f3..f2d331d0181f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> @@ -461,17 +461,17 @@ static uint64_t gmc_v12_0_map_mtype(struct amdgpu_device *adev, uint32_t flags)
>  {
>  	switch (flags) {
>  	case AMDGPU_VM_MTYPE_DEFAULT:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_NC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_NC);
>  	case AMDGPU_VM_MTYPE_NC:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_NC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_NC);
>  	case AMDGPU_VM_MTYPE_WC:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_WC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_WC);
>  	case AMDGPU_VM_MTYPE_CC:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_CC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_CC);
>  	case AMDGPU_VM_MTYPE_UC:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_UC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_UC);
>  	default:
> -		return AMDGPU_PTE_MTYPE_GFX12(MTYPE_NC);
> +		return AMDGPU_PTE_MTYPE_GFX12(0ULL,MTYPE_NC);
>  	}
>  }
>  
> @@ -509,8 +509,8 @@ static void gmc_v12_0_get_vm_pte(struct amdgpu_device *adev,
>  	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>  	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>  
> -	*flags &= ~AMDGPU_PTE_MTYPE_GFX12_MASK;
> -	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_GFX12_MASK);
> +	*flags = AMDGPU_PTE_MTYPE_GFX12(*flags, (mapping->flags &	\
> +			 AMDGPU_PTE_MTYPE_GFX12_MASK) >> AMDGPU_PTE_MTYPE_GFX12_SHIFT);
>  
>  	if (mapping->flags & AMDGPU_PTE_PRT_GFX12) {
>  		*flags |= AMDGPU_PTE_PRT_GFX12;
> @@ -524,8 +524,7 @@ static void gmc_v12_0_get_vm_pte(struct amdgpu_device *adev,
>  
>  	if (bo->flags & (AMDGPU_GEM_CREATE_COHERENT |
>  			       AMDGPU_GEM_CREATE_UNCACHED))
> -		*flags = (*flags & ~AMDGPU_PTE_MTYPE_GFX12_MASK) |
> -			 AMDGPU_PTE_MTYPE_GFX12(MTYPE_UC);
> +		*flags = AMDGPU_PTE_MTYPE_GFX12(*flags, MTYPE_UC);
>  
>  	bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
>  	coherent = bo->flags & AMDGPU_GEM_CREATE_COHERENT;
> @@ -534,7 +533,7 @@ static void gmc_v12_0_get_vm_pte(struct amdgpu_device *adev,
>  
>  	/* WA for HW bug */
>  	if (is_system || ((bo_adev != adev) && coherent))
> -		*flags |= AMDGPU_PTE_MTYPE_GFX12(MTYPE_NC);
> +		*flags = AMDGPU_PTE_MTYPE_GFX12(*flags, MTYPE_NC);
>  
>  }
>  
> @@ -707,7 +706,7 @@ static int gmc_v12_0_gart_init(struct amdgpu_device *adev)
>  		return r;
>  
>  	adev->gart.table_size = adev->gart.num_gpu_pages * 8;
> -	adev->gart.gart_pte_flags = AMDGPU_PTE_MTYPE_GFX12(MTYPE_UC) |
> +	adev->gart.gart_pte_flags = AMDGPU_PTE_MTYPE_GFX12(0ULL, MTYPE_UC) |
>  				    AMDGPU_PTE_EXECUTABLE |
>  				    AMDGPU_PTE_IS_PTE;
>