[PATCH v2 2/2] drm/xe: Don't issue TLB invalidations for VMAs if using execlists

[Matthew Brost]

On Thu, Feb 22, 2024 at 04:42:57PM -0500, Rodrigo Vivi wrote:
> On Wed, Feb 21, 2024 at 07:59:06PM -0800, Matthew Brost wrote:
> > TLB invalidations for VMAs are currently only implemented with the GuC
> > enabled. Do not issue TLB invalidations if using execlists. A longer
> > term fix would be up the TLB invalidation layer to be execlist aware.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pt.c | 32 +++++++++++++++++++-------------
> >  1 file changed, 19 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 7f54bc3e389d..c4de13bcfe85 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -1256,9 +1256,11 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
> >  	 */
> >  	if ((rebind && !xe_vm_in_lr_mode(vm) && !vm->batch_invalidate_tlb) ||
> >  	    (!rebind && xe_vm_has_scratch(vm) && xe_vm_in_preempt_fence_mode(vm))) {
> > -		ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
> > -		if (!ifence)
> > -			return ERR_PTR(-ENOMEM);
> > +		if (!vm->xe->info.force_execlist) {
> > +			ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
> > +			if (!ifence)
> > +				return ERR_PTR(-ENOMEM);
> > +		}
> >  	}
> >  
> >  	rfence = kzalloc(sizeof(*rfence), GFP_KERNEL);
> > @@ -1574,7 +1576,7 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queu
> >  	struct xe_vm *vm = xe_vma_vm(vma);
> >  	u32 num_entries;
> >  	struct dma_fence *fence = NULL;
> > -	struct invalidation_fence *ifence;
> > +	struct invalidation_fence *ifence = NULL;
> >  	struct xe_range_fence *rfence;
> >  
> >  	LLIST_HEAD(deferred);
> > @@ -1593,9 +1595,11 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queu
> >  	xe_pt_calc_rfence_interval(vma, &unbind_pt_update, entries,
> >  				   num_entries);
> >  
> > -	ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
> > -	if (!ifence)
> > -		return ERR_PTR(-ENOMEM);
> > +	if (!vm->xe->info.force_execlist) {
> > +		ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
> > +		if (!ifence)
> > +			return ERR_PTR(-ENOMEM);
> > +	}
> >  
> >  	rfence = kzalloc(sizeof(*rfence), GFP_KERNEL);
> >  	if (!rfence) {
> > @@ -1625,13 +1629,15 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queu
> >  			dma_fence_wait(fence, false);
> >  
> >  		/* TLB invalidation must be done before signaling unbind */
> > -		err = invalidation_fence_init(tile->primary_gt, ifence, fence, vma);
> > -		if (err) {
> > -			dma_fence_put(fence);
> > -			kfree(ifence);
> > -			return ERR_PTR(err);
> > +		if (ifence) {
> 
> I'm not so sure about the indirections of the conditions here...
> 
> probably deserves some refactor with earlier return or different functions?
> 
> But anyway, let's unbreak things first while we think on a better
> organization/separation.
> 

Let me just fix this in a slightly better way with all of in xe_gt_invalidation layer.

Matt

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> > +			err = invalidation_fence_init(tile->primary_gt, ifence, fence, vma);
> > +			if (err) {
> > +				dma_fence_put(fence);
> > +				kfree(ifence);
> > +				return ERR_PTR(err);
> > +			}
> > +			fence = &ifence->base.base;
> >  		}
> > -		fence = &ifence->base.base;
> >  
> >  		/* add shared fence now for pagetable delayed destroy */
> >  		dma_resv_add_fence(xe_vm_resv(vm), fence,
> > -- 
> > 2.34.1
> >