Re: [PATCH] drm/xe/xe3: Generate and store the L3 bank mask

[Francois Dugast <francois.dugast@intel.com>]

Hi,

On Tue, Jan 14, 2025 at 01:29:54PM -0800, Matt Roper wrote:
> On Tue, Jan 14, 2025 at 12:38:53PM -0800, Matt Atwood wrote:
> > From: Francois Dugast <francois.dugast@intel.com>
> > 
> > On Xe3, the register used to indicate which L3 banks are enabled on
> > the system is a new one called MIRROR_L3BANK_ENABLE. Each bit
> > represents one bank enabled in each node.
> > Extend the existing topology code for Xe3 to read this register and
> > generate the correct L3 bank mask, which can be read by user space
> > throug the topology query.
> > 
> > Bspec: 72573, 73439
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  3 +++
> >  drivers/gpu/drm/xe/xe_gt_topology.c  | 16 +++++++++++++---
> >  2 files changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > index b4283ac030f4..096859072396 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > @@ -221,6 +221,9 @@
> >  
> >  #define MIRROR_FUSE1				XE_REG(0x911c)
> >  
> > +#define MIRROR_L3BANK_ENABLE			XE_REG(0x9130)
> > +#define   XE3_L3BANK_ENABLE			REG_GENMASK(31, 0)
> > +
> >  #define XELP_EU_ENABLE				XE_REG(0x9134)	/* "_DISABLE" on Xe_LP */
> >  #define   XELP_EU_MASK				REG_GENMASK(7, 0)
> >  #define XELP_GT_SLICE_ENABLE			XE_REG(0x9138)
> > diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
> > index df2042db7ee6..516c81e3b8dd 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_topology.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
> > @@ -129,7 +129,8 @@ static void
> >  load_l3_bank_mask(struct xe_gt *gt, xe_l3_bank_mask_t l3_bank_mask)
> >  {
> >  	struct xe_device *xe = gt_to_xe(gt);
> > -	u32 fuse3 = xe_mmio_read32(&gt->mmio, MIRROR_FUSE3);
> > +	struct xe_mmio *mmio = &gt->mmio;
> > +	u32 fuse3 = xe_mmio_read32(mmio, MIRROR_FUSE3);
> >  
> >  	/*
> >  	 * PTL platforms with media version 30.00 do not provide proper values
> > @@ -143,7 +144,16 @@ load_l3_bank_mask(struct xe_gt *gt, xe_l3_bank_mask_t l3_bank_mask)
> >  	if (XE_WA(gt, no_media_l3))
> >  		return;
> >  
> > -	if (GRAPHICS_VER(xe) >= 20) {
> > +	if (GRAPHICS_VER(xe) >= 30) {
> > +		xe_l3_bank_mask_t per_node = {};
> > +		u32 meml3_en = REG_FIELD_GET(XE2_NODE_ENABLE_MASK, fuse3);
> > +		u32 mirror_l3bank_enable = xe_mmio_read32(mmio, MIRROR_L3BANK_ENABLE);
> > +		u32 bank_val = REG_FIELD_GET(XE3_L3BANK_ENABLE, mirror_l3bank_enable);
> > +
> > +		bitmap_from_arr32(per_node, &bank_val, 32);
> 
> Doesn't each bit in L3BANK_ENABLE represent a pair of banks rather than
> a single bank?  It doesn't look like that's accounted for here.

On this platform, MEML3_EN represents enabled nodes, which can be 0x1 (1 node)
or 0x3 (2 nodes). L3BANK_ENABLE represents enabled banks in each enabled node.

If 2 nodes are enabled, each bit in L3BANK_ENABLE will account for 2 banks in
the resulting mask, one in each bank, thanks to gen_l3_mask_from_pattern().

For example, if 2 nodes are enabled with 8 banks each then:
  meml3_en = 0x3
  per_node = 0xf
  l3_bank_mask = 0x0000000f0000000f

This seems correct.

Francois

> 
> 
> Matt
> 
> > +		gen_l3_mask_from_pattern(xe, l3_bank_mask, per_node, 32,
> > +					 meml3_en);
> > +	} else if (GRAPHICS_VER(xe) >= 20) {
> >  		xe_l3_bank_mask_t per_node = {};
> >  		u32 meml3_en = REG_FIELD_GET(XE2_NODE_ENABLE_MASK, fuse3);
> >  		u32 bank_val = REG_FIELD_GET(XE2_GT_L3_MODE_MASK, fuse3);
> > @@ -155,7 +165,7 @@ load_l3_bank_mask(struct xe_gt *gt, xe_l3_bank_mask_t l3_bank_mask)
> >  		xe_l3_bank_mask_t per_node = {};
> >  		xe_l3_bank_mask_t per_mask_bit = {};
> >  		u32 meml3_en = REG_FIELD_GET(MEML3_EN_MASK, fuse3);
> > -		u32 fuse4 = xe_mmio_read32(&gt->mmio, XEHP_FUSE4);
> > +		u32 fuse4 = xe_mmio_read32(mmio, XEHP_FUSE4);
> >  		u32 bank_val = REG_FIELD_GET(GT_L3_EXC_MASK, fuse4);
> >  
> >  		bitmap_set_value8(per_mask_bit, 0x3, 0);
> > -- 
> > 2.45.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation