[PATCH 1/2] drm/xe: Add support for PTE_NC bit

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of apoorva.singh at intel.com
Sent: Tuesday, October 1, 2024 6:55 AM
To: intel-xe at lists.freedesktop.org
Cc: Zeng, Oak <oak.zeng at intel.com>; Brian Welty <brian.welty at intel.com>; Singh, Apoorva <apoorva.singh at intel.com>
Subject: [PATCH 1/2] drm/xe: Add support for PTE_NC bit
> 
> From: Brian Welty <brian.welty at intel.com>
> 
> The current implementation of access counters is incomplete in that it
> doesn't manage the PTE_NC (No Count) bit. HW has finite number of access
> counters which HW itself dynamically allocates/deallocates. As these are
> a finite resource, don't enable access counting (don't waste counters)
> for page table mappings which cannot take any action on trigger of access
> counter threshold (cannot be migrated).
> The logic for this decision is encapsulated in xe_bo_can_use_acc().
> 
> Signed-off-by: Brian Welty <brian.welty at intel.com>
> Signed-off-by: Apoorva Singh <apoorva.singh at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_bo.h |  3 +++
>  drivers/gpu/drm/xe/xe_pt.c |  4 ++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 5f2f1ec46b57..449a301c5688 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2196,6 +2196,40 @@ bool xe_bo_can_migrate(struct xe_bo *bo, u32 mem_type)
>  	return false;
>  }
>  
> +/**
> + * xe_bo_can_use_acc - Determine if BO is eligible for access counting
> + * @bo: The buffer object.
> + * @tile: Tile (set of GT engines) which page table the BO is being bound
> + *        is associated with.
> + *
> + * Number of HW access counters is finite and allocation/deallocation is
> + * managed internally by hardware. As they are precious, don't waste them
> + * on backing store for BO's that cannot be migrated.
> + * Currently this means, allow access counting if:
> + *   BO has more than one placement
> + *   BO is not already in VRAM local to @tile
> + * Note, access counters are not used unless enabled in LRC.
> + */
> +bool xe_bo_can_use_acc(struct xe_bo *bo, struct xe_tile *tile)

Tile is declared as a required function parameter, but it doesn't seem to be used or
checked at any point in the function?  The function comment notes that we should
be checking to ensure that the BO isn't already in a VRAM local to tile, but I don't see
where that's being checked in the function.
-Jonathan Cavitt

> +{
> +	struct ttm_resource *res;
> +
> +	/* userptr or using null page */
> +	if (!bo)
> +		return false;
> +
> +	res = bo->ttm.resource;
> +	/* if for some reason no backing store, nothing to migrate */
> +	if (!res)
> +		return false;
> +
> +	/* cannot migrate if single placment */
> +	if (bo->placement.num_placement <= 1)
> +		return false;
> +
> +	return true;
> +}
> +
>  static void xe_place_from_ttm_type(u32 mem_type, struct ttm_place *place)
>  {
>  	memset(place, 0, sizeof(*place));
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 6e4be52306df..b60dce0d2f1c 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -55,6 +55,8 @@
>  #define XE_64K_PTE_MASK			(XE_64K_PAGE_SIZE - 1)
>  #define XE_64K_PDE_MASK			(XE_PDE_MASK >> 4)
>  
> +#define XE_PPGTT_PTE_NC			BIT_ULL(5)
> +
>  #define XE_PL_SYSTEM		TTM_PL_SYSTEM
>  #define XE_PL_TT		TTM_PL_TT
>  #define XE_PL_VRAM0		TTM_PL_VRAM
> @@ -209,6 +211,7 @@ bool xe_bo_has_single_placement(struct xe_bo *bo);
>  uint64_t vram_region_gpu_offset(struct ttm_resource *res);
>  
>  bool xe_bo_can_migrate(struct xe_bo *bo, u32 mem_type);
> +bool xe_bo_can_use_acc(struct xe_bo *bo, struct xe_tile *tile);
>  
>  int xe_bo_migrate(struct xe_bo *bo, u32 mem_type);
>  int xe_bo_evict(struct xe_bo *bo, bool force_alloc);
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index d6353e8969f0..18cf13d548c6 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -660,6 +660,10 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>  			xe_walk.default_pte &= ~XE_USM_PPGTT_PTE_AE;
>  	}
>  
> +	/* set NoCount bit when not in need of access counting */
> +	if (!xe_bo_can_use_acc(bo, tile))
> +		xe_walk.default_pte |= XE_PPGTT_PTE_NC;
> +
>  	if (is_devmem) {
>  		xe_walk.default_pte |= XE_PPGTT_PTE_DM;
>  		xe_walk.dma_offset = vram_region_gpu_offset(bo->ttm.resource);
> -- 
> 2.34.1
> 
>