Re: [PATCH] drm/xe: Invalidate media_gt TLBs

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 18-08-2024 02:34, Matthew Brost wrote:
> Testing on LNL has shown media TLBs need to be invalidated via the GuC,
> update xe_vm_invalidate_vma appropriately.
> 
> Fixes: 3330361543fc ("drm/xe/lnl: Add LNL platform definition")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_vm.c | 42 +++++++++++++++++++++++++++-----------
>   1 file changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 6dd76f77b504..ac98973a90cc 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3184,8 +3184,10 @@ int xe_vm_invalidate_vma(struct xe_vma *vma)
>   {
>   	struct xe_device *xe = xe_vma_vm(vma)->xe;
>   	struct xe_tile *tile;
> -	struct xe_gt_tlb_invalidation_fence fence[XE_MAX_TILES_PER_DEVICE];
> -	u32 tile_needs_invalidate = 0;
> +	struct xe_gt *gt;
> +	struct xe_gt_tlb_invalidation_fence
> +		fence[XE_MAX_TILES_PER_DEVICE * XE_MAX_GT_PER_TILE];


XE_MAX_TILES_PER_DEVICE * XE_MAX_GT_PER_TILE is not really needed.
Current driver code is with assumption that multi tile and multi gt (on 
same tile) are mutually exclusive.

But I see no harm with this and patch LGTM.
Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>


> +	u32 gt_needs_invalidate = 0;
>   	u8 id;
>   	int ret = 0;
>   
> @@ -3211,29 +3213,45 @@ int xe_vm_invalidate_vma(struct xe_vma *vma)
>   	}
>   
>   	for_each_tile(tile, xe, id) {
> +		u32 fence_id = id * XE_MAX_TILES_PER_DEVICE;
> +
>   		if (xe_pt_zap_ptes(tile, vma)) {
>   			xe_device_wmb(xe);
>   			xe_gt_tlb_invalidation_fence_init(tile->primary_gt,
> -							  &fence[id], true);
> +							  &fence[fence_id],
> +							  true);
>   
> -			/*
> -			 * FIXME: We potentially need to invalidate multiple
> -			 * GTs within the tile
> -			 */
>   			ret = xe_gt_tlb_invalidation_vma(tile->primary_gt,
> -							 &fence[id], vma);
> +							 &fence[fence_id], vma);
> +			if (ret < 0) {
> +				xe_gt_tlb_invalidation_fence_fini(&fence[fence_id]);
> +				goto wait;
> +			}
> +
> +			gt_needs_invalidate |= BIT(fence_id);
> +
> +			if (!tile->media_gt)
> +				continue;
> +
> +			++fence_id;
> +			xe_gt_tlb_invalidation_fence_init(tile->media_gt,
> +							  &fence[fence_id],
> +							  true);
> +
> +			ret = xe_gt_tlb_invalidation_vma(tile->media_gt,
> +							 &fence[fence_id], vma);
>   			if (ret < 0) {
> -				xe_gt_tlb_invalidation_fence_fini(&fence[id]);
> +				xe_gt_tlb_invalidation_fence_fini(&fence[fence_id]);
>   				goto wait;
>   			}
>   
> -			tile_needs_invalidate |= BIT(id);
> +			gt_needs_invalidate |= BIT(fence_id);
>   		}
>   	}
>   
>   wait:
> -	for_each_tile(tile, xe, id)
> -		if (tile_needs_invalidate & BIT(id))
> +	for_each_gt(gt, xe, id)
> +		if (gt_needs_invalidate & BIT(id))
>   			xe_gt_tlb_invalidation_fence_wait(&fence[id]);
>   
>   	vma->tile_invalidated = vma->tile_mask;