Re: [PATCH v4 09/30] drm/xe: Add some members to xe_vma_ops

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

On Mon, Mar 25, 2024 at 10:10:43AM -0600, Zeng, Oak wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew
> > Brost
> > Sent: Friday, March 8, 2024 12:08 AM
> > To: intel-xe@lists.freedesktop.org
> > Cc: Brost, Matthew <matthew.brost@intel.com>
> > Subject: [PATCH v4 09/30] drm/xe: Add some members to xe_vma_ops
> > 
> > This will help with moving to single jobs for many bind operations.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vm.c       | 13 ++++++++++---
> >  drivers/gpu/drm/xe/xe_vm_types.h |  8 ++++++++
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index a6bae2f5e296..f8b27746e5a7 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1307,9 +1307,16 @@ static void xe_vm_free_scratch(struct xe_vm *vm)
> >  	}
> >  }
> > 
> > -static void xe_vma_ops_init(struct xe_vma_ops *vops)
> > +static void xe_vma_ops_init(struct xe_vma_ops *vops, struct xe_vm *vm,
> > +			    struct xe_exec_queue *q,
> > +			    struct xe_sync_entry *syncs, u32 num_syncs)
> >  {
> > +	memset(vops, 0, sizeof(*vops));
> >  	INIT_LIST_HEAD(&vops->list);
> > +	vops->vm = vm;
> > +	vops->q = q;
> > +	vops->syncs = syncs;
> > +	vops->num_syncs = num_syncs;
> >  }
> > 
> >  struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
> > @@ -1333,7 +1340,7 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32
> > flags)
> >  	init_rwsem(&vm->lock);
> >  	mutex_init(&vm->snap_mutex);
> > 
> > -	xe_vma_ops_init(&vm->dummy_ops.vops);
> > +	xe_vma_ops_init(&vm->dummy_ops.vops, vm, NULL, NULL, 0);
> 
> 
> You mentioned in previous review, that vm->dummy_ops will be dropped... I guess this line will also be dropped.
> 

Yes, will drop the dummy ops.

Matt

> Otherwise, this patch looks good to me.
> 
> Oak
> 
> >  	INIT_LIST_HEAD(&vm->dummy_ops.op.link);
> >  	list_add(&vm->dummy_ops.op.link, &vm->dummy_ops.vops.list);
> > 
> > @@ -3176,7 +3183,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void
> > *data, struct drm_file *file)
> >  		goto free_syncs;
> >  	}
> > 
> > -	xe_vma_ops_init(&vops);
> > +	xe_vma_ops_init(&vops, vm, q, syncs, num_syncs);
> >  	for (i = 0; i < args->num_binds; ++i) {
> >  		u64 range = bind_ops[i].range;
> >  		u64 addr = bind_ops[i].addr;
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> > b/drivers/gpu/drm/xe/xe_vm_types.h
> > index 0bba5543ac28..f6ea4df64e79 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -211,6 +211,14 @@ struct xe_vma_op {
> >  struct xe_vma_ops {
> >  	/** @list: list of VMA operations */
> >  	struct list_head list;
> > +	/** @vm: VM */
> > +	struct xe_vm *vm;
> > +	/** @q: exec queue these operations */
> > +	struct xe_exec_queue *q;
> > +	/** @syncs: syncs these operation */
> > +	struct xe_sync_entry *syncs;
> > +	/** @num_syncs: number of syncs */
> > +	u32 num_syncs;
> >  };
> > 
> >  struct xe_vm {
> > --
> > 2.34.1
>