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

[Matt Roper]

On Wed, Jan 15, 2025 at 12:25:13PM +0100, Francois Dugast wrote:
> 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 at 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 at intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood at 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.

Hmm, it seems the two bspec pages listed on this patch are inconsistent.
73439 indicates that each bit refers to a "bank pair" rather than single
bits.  However 72573 indicates "1 bank in each node."  The latter is
likely the correct one, but we may want to file a ticket to get the
spec's description fixed too, otherwise it's going to cause more
confusion.

Aside from that, things look good.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

> 
> 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

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation