[Intel-xe] [PATCH] drm/xe: fix pat[2] programming with 2M/1G pages

[Matthew Brost]

On Wed, Oct 18, 2023 at 01:34:24PM +0100, Matthew Auld wrote:
> Bit 7 in the leaf node is normally programmed with pat[2], however with
> 2M/1G pages that same bit in the PDE/PDPE also toggles 2M/1G pages. For
> 2M/1G entries the pat[2] is rather moved to bit 12, which is now free
> given that the address must be aligned to 2M or 1G, leaving bit 7 for
> toggling 2M/1G pages.
> 
> Bspec: 59510, 45038
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.h |  1 +
>  drivers/gpu/drm/xe/xe_vm.c | 17 +++++++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 78da28f4ea13..4e74201cc986 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -41,6 +41,7 @@
>  
>  #define XELPG_PPGTT_PTE_PAT3		BIT_ULL(62)
>  #define XE2_PPGTT_PTE_PAT4		BIT_ULL(61)
> +#define XE_PPGTT_PDE_PDPE_PAT2		BIT_ULL(12)

This patch looks good, but I don't think xe_bo.h is the correct place
for the page table defines. I think these were definied here in original
PoC of Xe and we never moved these to a more logical header file. I'll
probably post patch to find a home for these once this patch is merged.

Anyways:
Reviewed-by: Matthew Brost <matthew.brost at intel.com>

>  #define XE_PPGTT_PTE_PAT2		BIT_ULL(7)
>  #define XE_PPGTT_PTE_PAT1		BIT_ULL(4)
>  #define XE_PPGTT_PTE_PAT0		BIT_ULL(3)
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index fa6fa34a0663..498c0b3e1d73 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1210,7 +1210,8 @@ static u64 pde_encode_pat_index(struct xe_device *xe, u16 pat_index)
>  	return pte;
>  }
>  
> -static u64 pte_encode_pat_index(struct xe_device *xe, u16 pat_index)
> +static u64 pte_encode_pat_index(struct xe_device *xe, u16 pat_index,
> +				u32 pt_level)
>  {
>  	u64 pte = 0;
>  
> @@ -1220,8 +1221,12 @@ static u64 pte_encode_pat_index(struct xe_device *xe, u16 pat_index)
>  	if (pat_index & BIT(1))
>  		pte |= XE_PPGTT_PTE_PAT1;
>  
> -	if (pat_index & BIT(2))
> -		pte |= XE_PPGTT_PTE_PAT2;
> +	if (pat_index & BIT(2)) {
> +		if (pt_level)
> +			pte |= XE_PPGTT_PDE_PDPE_PAT2;
> +		else
> +			pte |= XE_PPGTT_PTE_PAT2;
> +	}
>  
>  	if (pat_index & BIT(3))
>  		pte |= XELPG_PPGTT_PTE_PAT3;
> @@ -1265,7 +1270,7 @@ static u64 xelp_pte_encode_bo(struct xe_bo *bo, u64 bo_offset,
>  
>  	pte = xe_bo_addr(bo, bo_offset, XE_PAGE_SIZE);
>  	pte |= XE_PAGE_PRESENT | XE_PAGE_RW;
> -	pte |= pte_encode_pat_index(xe, pat_index);
> +	pte |= pte_encode_pat_index(xe, pat_index, pt_level);
>  	pte |= pte_encode_ps(pt_level);
>  
>  	if (xe_bo_is_vram(bo) || xe_bo_is_stolen_devmem(bo))
> @@ -1284,7 +1289,7 @@ static u64 xelp_pte_encode_vma(u64 pte, struct xe_vma *vma,
>  	if (likely(!xe_vma_read_only(vma)))
>  		pte |= XE_PAGE_RW;
>  
> -	pte |= pte_encode_pat_index(xe, pat_index);
> +	pte |= pte_encode_pat_index(xe, pat_index, pt_level);
>  	pte |= pte_encode_ps(pt_level);
>  
>  	if (unlikely(xe_vma_is_null(vma)))
> @@ -1304,7 +1309,7 @@ static u64 xelp_pte_encode_addr(struct xe_device *xe, u64 addr,
>  
>  	pte = addr;
>  	pte |= XE_PAGE_PRESENT | XE_PAGE_RW;
> -	pte |= pte_encode_pat_index(xe, pat_index);
> +	pte |= pte_encode_pat_index(xe, pat_index, pt_level);
>  	pte |= pte_encode_ps(pt_level);
>  
>  	if (devmem)
> -- 
> 2.41.0
>