[PATCH v1 2/2] drm/xe: Move VRAM manager to struct xe_vram_region

[Piotr Piórkowski]

Rodrigo Vivi <rodrigo.vivi at intel.com> wrote on pią [2025-sty-31 05:37:42 -0500]:
> On Fri, Jan 31, 2025 at 11:09:13AM +0100, Piórkowski, Piotr wrote:
> > From: Piotr Piórkowski <piotr.piorkowski at intel.com>
> > 
> > VRAM manager is related directly to struct xe_vram_region so it
> > should be inside this structure.
> > Let's move the VRAM to struct xe_vram_region.
> > 
> > Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_device_types.h       | 5 ++---
> >  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 2 +-
> >  drivers/gpu/drm/xe/xe_tile.c               | 6 +++---
> >  3 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > index c7c285c6a6c9..a4d998add4c3 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -104,6 +104,8 @@ struct xe_vram_region {
> >  	resource_size_t actual_physical_size;
> >  	/** @mapping: pointer to VRAM mappable space */
> >  	void __iomem *mapping;
> > +	/** @vram_mgr: VRAM TTM manager */
> > +	struct xe_ttm_vram_mgr *vram_mgr;
> >  };
> >  
> >  /**
> > @@ -198,9 +200,6 @@ struct xe_tile {
> >  		 */
> >  		struct xe_vram_region vram;
> >  
> > -		/** @mem.vram_mgr: VRAM TTM manager */
> > -		struct xe_ttm_vram_mgr *vram_mgr;
> > -
> >  		/** @mem.ggtt: Global graphics translation table */
> >  		struct xe_ggtt *ggtt;
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > index b1d994d65589..98e443faabdc 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> > @@ -1560,7 +1560,7 @@ static u64 pf_query_free_lmem(struct xe_gt *gt)
> >  {
> >  	struct xe_tile *tile = gt->tile;
> >  
> > -	return xe_ttm_vram_get_avail(&tile->mem.vram_mgr->manager);
> > +	return xe_ttm_vram_get_avail(&tile->mem.vram.vram_mgr->manager);
> 
> I like your series, but I didn't like how this ended here
> vram vram
> manager manager
> 
> Then I went to see if there was something we could do about the
> xe_ttm_vram_mgr itself, but then I noticed it has pointer to
> xe_vram_region inside it and I disliked this double pointer now.
> 
> So, if we really want to take this path we probably need a bigger
> refactor, or this can bring more confusion then help.

I know what you mean - it irritated me too...
For now, I suggested this solution, because it solved my other problems...
But I agree that it looks bad, and I will try maybe to do something about it.

Thanks!
> 
> >  }
> >  
> >  static u64 pf_query_max_lmem(struct xe_gt *gt)
> > diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
> > index 2825553b568f..b13a0ec7c296 100644
> > --- a/drivers/gpu/drm/xe/xe_tile.c
> > +++ b/drivers/gpu/drm/xe/xe_tile.c
> > @@ -94,8 +94,8 @@ static int xe_tile_alloc(struct xe_tile *tile)
> >  		return -ENOMEM;
> >  	tile->mem.ggtt->tile = tile;
> >  
> > -	tile->mem.vram_mgr = drmm_kzalloc(drm, sizeof(*tile->mem.vram_mgr), GFP_KERNEL);
> > -	if (!tile->mem.vram_mgr)
> > +	tile->mem.vram.vram_mgr = drmm_kzalloc(drm, sizeof(*tile->mem.vram.vram_mgr), GFP_KERNEL);
> > +	if (!tile->mem.vram.vram_mgr)
> >  		return -ENOMEM;
> >  
> >  	return 0;
> > @@ -139,7 +139,7 @@ static int tile_ttm_mgr_init(struct xe_tile *tile)
> >  	int err;
> >  
> >  	if (tile->mem.vram.usable_size) {
> > -		err = xe_ttm_vram_mgr_init(tile, tile->mem.vram_mgr);
> > +		err = xe_ttm_vram_mgr_init(tile, tile->mem.vram.vram_mgr);
> >  		if (err)
> >  			return err;
> >  		xe->info.mem_region_mask |= BIT(tile->id) << 1;
> > -- 
> > 2.34.1
> > 

--