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@xxxxxxx>
> Signed-off-by: Shane Xiao <shane.xiao@xxxxxxx>
> ---
> 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;
>
