Re: [PATCH 4/5] drm/xe: Rename internal vram helper function

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

On Tue, May 28, 2024 at 02:35:15PM -0700, Matt Roper wrote:
> On Mon, May 27, 2024 at 07:35:53PM +0200, Michal Wajdeczko wrote:
> > Drop no longer applicable "xe_mmio_" prefix and downgrade the
> > existing kernel-doc for internal function to normal comment.
> 
> As noted on the previous patch, there are several other functions with
> "xe_" prefixes even though they're static (xe_resize_vram_bar,
> xe_pci_resource_valid, xe_determine_lmem_bar_size, and the
> xe_mmio_tile_vram_size you're updating here).  Maybe we should drop the
> "xe_" prefix from all of them (and "xe_mmio" from this one) before the
> movement of VRAM code to a new file?
> 

We have talked about this before and I think the consensous was "xe_"
prefixes for static functions are fine, perhaps even desired. I can't
find a reference but I do recall a discussion on the list about this.

I think the maintainers should make / document a rule wrt to "xe_"
prefixes before we start making changes in this area as it is not clear.

Matt

> 
> Matt
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vram.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
> > index d8b81e4e050c..411e8d23fd4d 100644
> > --- a/drivers/gpu/drm/xe/xe_vram.c
> > +++ b/drivers/gpu/drm/xe/xe_vram.c
> > @@ -192,8 +192,8 @@ static inline u64 get_flat_ccs_offset(struct xe_gt *gt, u64 tile_size)
> >  	return offset;
> >  }
> >  
> > -/**
> > - * xe_mmio_tile_vram_size() - Collect vram size and offset information
> > +/*
> > + * tile_vram_size() - Collect vram size and offset information
> >   * @tile: tile to get info for
> >   * @vram_size: available vram (size - device reserved portions)
> >   * @tile_size: actual vram size
> > @@ -211,8 +211,8 @@ static inline u64 get_flat_ccs_offset(struct xe_gt *gt, u64 tile_size)
> >   * NOTE: multi-tile bases will include the tile offset.
> >   *
> >   */
> > -static int xe_mmio_tile_vram_size(struct xe_tile *tile, u64 *vram_size,
> > -				  u64 *tile_size, u64 *tile_offset)
> > +static int tile_vram_size(struct xe_tile *tile, u64 *vram_size,
> > +			  u64 *tile_size, u64 *tile_offset)
> >  {
> >  	struct xe_device *xe = tile_to_xe(tile);
> >  	struct xe_gt *gt = tile->primary_gt;
> > @@ -287,7 +287,7 @@ int xe_vram_probe(struct xe_device *xe)
> >  
> >  	/* Get the size of the root tile's vram for later accessibility comparison */
> >  	tile = xe_device_get_root_tile(xe);
> > -	err = xe_mmio_tile_vram_size(tile, &vram_size, &tile_size, &tile_offset);
> > +	err = tile_vram_size(tile, &vram_size, &tile_size, &tile_offset);
> >  	if (err)
> >  		return err;
> >  
> > @@ -302,7 +302,7 @@ int xe_vram_probe(struct xe_device *xe)
> >  
> >  	/* tile specific ranges */
> >  	for_each_tile(tile, xe, id) {
> > -		err = xe_mmio_tile_vram_size(tile, &vram_size, &tile_size, &tile_offset);
> > +		err = tile_vram_size(tile, &vram_size, &tile_size, &tile_offset);
> >  		if (err)
> >  			return err;
> >  
> > -- 
> > 2.43.0
> > 
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation