Re: [Intel-xe] [PATCH v2 3/4] drm/xe: Rename exec_queue_kill_compute to xe_vm_remove_compute_exec_queue

[Matthew Brost <matthew.brost@intel.com>]

On Sun, Sep 17, 2023 at 09:53:12PM -0700, Niranjana Vishwanathapura wrote:
> On Fri, Sep 15, 2023 at 04:34:44PM -0700, Matthew Brost wrote:
> > Much better name and aligns with xe_vm_add_compute_exec_queue. As part
> > of the rename, move the implementation from xe_exec_queue.c to xe_vm.c.
> > 
> > Suggested-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_exec_queue.c | 20 ++------------------
> > drivers/gpu/drm/xe/xe_vm.c         | 21 +++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_vm.h         |  1 +
> > 3 files changed, 24 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index 93d2f0591c34..b95b62a2538f 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -775,22 +775,6 @@ int xe_exec_queue_get_property_ioctl(struct drm_device *dev, void *data,
> > 	return ret;
> > }
> > 
> > -static void exec_queue_kill_compute(struct xe_exec_queue *q)
> > -{
> > -	if (!xe_vm_in_compute_mode(q->vm))
> > -		return;
> > -
> > -	down_write(&q->vm->lock);
> > -	list_del(&q->compute.link);
> > -	--q->vm->preempt.num_exec_queues;
> > -	if (q->compute.pfence) {
> > -		dma_fence_enable_sw_signaling(q->compute.pfence);
> > -		dma_fence_put(q->compute.pfence);
> > -		q->compute.pfence = NULL;
> > -	}
> > -	up_write(&q->vm->lock);
> > -}
> > -
> > /**
> >  * xe_exec_queue_is_lr() - Whether an exec_queue is long-running
> >  * @q: The exec_queue
> > @@ -861,11 +845,11 @@ void xe_exec_queue_kill(struct xe_exec_queue *q)
> > 	list_for_each_entry_safe(eq, next, &eq->multi_gt_list,
> > 				 multi_gt_link) {
> > 		q->ops->kill(eq);
> > -		exec_queue_kill_compute(eq);
> > +		xe_vm_remove_compute_exec_queue(q->vm, eq);
> > 	}
> > 
> > 	q->ops->kill(q);
> > -	exec_queue_kill_compute(q);
> > +	xe_vm_remove_compute_exec_queue(q->vm, q);
> > }
> > 
> > int xe_exec_queue_destroy_ioctl(struct drm_device *dev, void *data,
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 03ee6993e7c7..433bdfae6759 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -380,6 +380,27 @@ int xe_vm_add_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)
> > 	return err;
> > }
> > 
> > +/**
> > + * xe_vm_remove_compute_exec_queue() - Remove compute exec queue from VM
> > + * @vm: The VM.
> > + * @q: The exec_queue
> > + */
> > +void xe_vm_remove_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)
> 
> Actually wonder if we need 'vm' as input param here as we can always use q->vm.
> If we do keep it, it will force use to assert 'vm == q->vm' here, so probably
> better remove it.

Going to leave it as is because this in xe_vm.c I think it should take a VM argument.

Matt

> In either case,
> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> 
> Niranjana
> 
> > +{
> > +	if (!xe_vm_in_compute_mode(vm))
> > +		return;
> > +
> > +	down_write(&vm->lock);
> > +	list_del(&q->compute.link);
> > +	--vm->preempt.num_exec_queues;
> > +	if (q->compute.pfence) {
> > +		dma_fence_enable_sw_signaling(q->compute.pfence);
> > +		dma_fence_put(q->compute.pfence);
> > +		q->compute.pfence = NULL;
> > +	}
> > +	up_write(&vm->lock);
> > +}
> > +
> > /**
> >  * __xe_vm_userptr_needs_repin() - Check whether the VM does have userptrs
> >  * that need repinning.
> > diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
> > index 5608e4e33169..813328ad3f7f 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.h
> > +++ b/drivers/gpu/drm/xe/xe_vm.h
> > @@ -168,6 +168,7 @@ static inline bool xe_vm_no_dma_fences(struct xe_vm *vm)
> > }
> > 
> > int xe_vm_add_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q);
> > +void xe_vm_remove_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q);
> > 
> > int xe_vm_userptr_pin(struct xe_vm *vm);
> > 
> > -- 
> > 2.34.1
> >