Re: [Intel-xe] [PATCH 3/5] drm/xe: Change tile masks from u64 to u8

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

On Thu, Jul 13, 2023 at 04:21:47PM -0400, Rodrigo Vivi wrote:
> On Tue, Jul 11, 2023 at 02:27:46PM -0700, Matthew Brost wrote:
> > This will save us a few bytes in the xe_vma structure.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vm.c       |  8 ++++----
> >  drivers/gpu/drm/xe/xe_vm_types.h | 28 ++++++++++++++--------------
> >  2 files changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index b2847be6de6a..762aefa75ed4 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -871,7 +871,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
> >  				    u64 start, u64 end,
> >  				    bool read_only,
> >  				    bool is_null,
> > -				    u64 tile_mask)
> > +				    u8 tile_mask)
> >  {
> >  	struct xe_vma *vma;
> >  	struct xe_tile *tile;
> > @@ -2246,7 +2246,7 @@ static void print_op(struct xe_device *xe, struct drm_gpuva_op *op)
> >  static struct drm_gpuva_ops *
> >  vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
> >  			 u64 bo_offset_or_userptr, u64 addr, u64 range,
> > -			 u32 operation, u64 tile_mask, u32 region)
> > +			 u32 operation, u8 tile_mask, u32 region)
> >  {
> >  	struct drm_gem_object *obj = bo ? &bo->ttm.base : NULL;
> >  	struct ww_acquire_ctx ww;
> > @@ -2343,7 +2343,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
> >  }
> >  
> >  static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
> > -			      u64 tile_mask, bool read_only, bool is_null)
> > +			      u8 tile_mask, bool read_only, bool is_null)
> >  {
> >  	struct xe_bo *bo = op->gem.obj ? gem_to_xe_bo(op->gem.obj) : NULL;
> >  	struct xe_vma *vma;
> > @@ -3323,7 +3323,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> >  		u64 addr = bind_ops[i].addr;
> >  		u32 op = bind_ops[i].op;
> >  		u64 obj_offset = bind_ops[i].obj_offset;
> > -		u64 tile_mask = bind_ops[i].tile_mask;
> > +		u8 tile_mask = bind_ops[i].tile_mask;
> >  		u32 region = bind_ops[i].region;
> >  
> >  		ops[i] = vm_bind_ioctl_ops_create(vm, bos[i], obj_offset,
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> > index 84bf1620214c..2a8691a48c55 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -37,18 +37,6 @@ struct xe_vma {
> >  	/** @gpuva: Base GPUVA object */
> >  	struct drm_gpuva gpuva;
> >  
> > -	/** @tile_mask: Tile mask of where to create binding for this VMA */
> > -	u64 tile_mask;
> > -
> > -	/**
> > -	 * @tile_present: GT mask of binding are present for this VMA.
> > -	 * protected by vm->lock, vm->resv and for userptrs,
> > -	 * vm->userptr.notifier_lock for writing. Needs either for reading,
> > -	 * but if reading is done under the vm->lock only, it needs to be held
> > -	 * in write mode.
> > -	 */
> > -	u64 tile_present;
> > -
> >  	/** @combined_links: links into lists which are mutually exclusive */
> >  	union {
> >  		/** @userptr: link into VM repin list if userptr */
> > @@ -97,9 +85,21 @@ struct xe_vma {
> >  	/** @usm: unified shared memory state */
> >  	struct {
> >  		/** @tile_invalidated: VMA has been invalidated */
> > -		u64 tile_invalidated;
> > +		u8 tile_invalidated;
> >  	} usm;
> >  
> > +	/** @tile_mask: Tile mask of where to create binding for this VMA */
> > +	u8 tile_mask;
> > +
> > +	/**
> > +	 * @tile_present: GT mask of binding are present for this VMA.
> > +	 * protected by vm->lock, vm->resv and for userptrs,
> > +	 * vm->userptr.notifier_lock for writing. Needs either for reading,
> > +	 * but if reading is done under the vm->lock only, it needs to be held
> > +	 * in write mode.
> > +	 */
> > +	u8 tile_present;
> > +
> >  	struct {
> >  		struct list_head rebind_link;
> >  	} notifier;
> > @@ -386,7 +386,7 @@ struct xe_vma_op {
> >  	 */
> >  	struct async_op_fence *fence;
> >  	/** @tile_mask: gt mask for this operation */
> > -	u64 tile_mask;
> > +	u8 tile_mask;
> 
> I'm seeing us to use hweight_long on this tile_mask...
> 

Will change to hweight8.

Matt

> >  	/** @flags: operation flags */
> >  	enum xe_vma_op_flags flags;
> >  
> > -- 
> > 2.34.1
> >