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

[Matt Roper <matthew.d.roper@intel.com>]

On Tue, Aug 20, 2024 at 09:01:29AM -0700, Matthew Brost wrote:
> Testing on LNL has shown media TLBs need to be invalidated via the GuC,
> update xe_vm_invalidate_vma appropriately.
> 
> v2: Fix 2 tile case
> v3: Include missing local changm
> 
> Fixes: 3330361543fc ("drm/xe/lnl: Add LNL platform definition")

Is this the right Fixes line?  This may have been discovered in testing
on LNL, but presumably the fix applies to MTL as well since we also have
a standalone media GT there too, and we've had MTL support in the driver
since before LNL was added.

Of course both platforms are under force_probe at the moment (and MTL
will be forever since it's officially supported by i915 instead of Xe),
so it probably doesn't really matter too much for backporting purposes
and such.


Matt

> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c | 37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index d1bfd0b6e955..4cc13eddb6b3 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3158,9 +3158,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_tlb_invalidation_fence
> +		fence[XE_MAX_TILES_PER_DEVICE * XE_MAX_GT_PER_TILE];
>  	u8 id;
> +	u32 fence_id = 0;
>  	int ret = 0;
>  
>  	xe_assert(xe, !xe_vma_is_null(vma));
> @@ -3188,27 +3189,37 @@ int xe_vm_invalidate_vma(struct xe_vma *vma)
>  		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[id]);
> +				xe_gt_tlb_invalidation_fence_fini(&fence[fence_id]);
>  				goto wait;
>  			}
> +			++fence_id;
>  
> -			tile_needs_invalidate |= BIT(id);
> +			if (!tile->media_gt)
> +				continue;
> +
> +			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[fence_id]);
> +				goto wait;
> +			}
> +			++fence_id;
>  		}
>  	}
>  
>  wait:
> -	for_each_tile(tile, xe, id)
> -		if (tile_needs_invalidate & BIT(id))
> -			xe_gt_tlb_invalidation_fence_wait(&fence[id]);
> +	for (id = 0; id < fence_id; ++id)
> +		xe_gt_tlb_invalidation_fence_wait(&fence[id]);
>  
>  	vma->tile_invalidated = vma->tile_mask;
>  
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation