Re: [PATCH 06/11] drm/xe/xe2hpg: Determine flat ccs offset for vram

["Vivekanandan, Balasubramani" <balasubramani.vivekanandan@intel.com>]

On 03.04.2024 12:23, Vivekanandan, Balasubramani wrote:
> On 02.04.2024 07:30, Matt Roper wrote:
> > On Tue, Apr 02, 2024 at 06:17:19PM +0530, Balasubramani Vivekanandan wrote:
> > > From: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > 
> > > on Xe2 dgfx platform determine the offset using Flat CCS size
> > > bitfield of XE2_FLAT_CCS_BASE_RANGE_[UPPER/LOWER] mcr registers.
> > > 
> > > Bspec: 68023
> > > 
> > > Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > Signed-off-by: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  5 ++++
> > >  drivers/gpu/drm/xe/xe_mmio.c         | 39 ++++++++++++++++++++++++++--
> > >  2 files changed, 42 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > index d5b21f03beaa..ea75c1f0ebd0 100644
> > > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > @@ -69,6 +69,7 @@
> > >  
> > >  #define XEHP_TILE_ADDR_RANGE(_idx)		XE_REG_MCR(0x4900 + (_idx) * 4)
> > >  #define XEHP_FLAT_CCS_BASE_ADDR			XE_REG_MCR(0x4910)
> > > +#define XEHP_FLAT_CCS_PTR			REG_GENMASK(31, 8)
> > >  
> > >  #define WM_CHICKEN3				XE_REG_MCR(0x5588, XE_REG_OPTION_MASKED)
> > >  #define   HIZ_PLANE_COMPRESSION_DIS		REG_BIT(10)
> > > @@ -141,6 +142,10 @@
> > >  
> > >  #define XE2_FLAT_CCS_BASE_RANGE_LOWER		XE_REG_MCR(0x8800)
> > >  #define   XE2_FLAT_CCS_ENABLE			REG_BIT(0)
> > > +#define XE2_FLAT_CCS_BASE_LOWER_ADDR_MASK	REG_GENMASK(31, 6)
> > > +
> > > +#define XE2_FLAT_CCS_BASE_RANGE_UPPER		XE_REG_MCR(0x8804)
> > > +#define XE2_FLAT_CCS_BASE_UPPER_ADDR_MASK	REG_GENMASK(7, 0)
> > >  
> > >  #define GSCPSMI_BASE				XE_REG(0x880c)
> > >  
> > > diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> > > index 5d13fc7cb9d2..49a32b03fcf8 100644
> > > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > > @@ -163,6 +163,42 @@ static int xe_determine_lmem_bar_size(struct xe_device *xe)
> > >  	return 0;
> > >  }
> > >  
> > > +static inline u64 get_flat_ccs_offset(struct xe_gt *gt, u64 *tile_size)
> > 
> > It doesn't seem to be necessary to pass by reference for tile_size.
> Correct, I will change it.
> > 
> > > +{
> > > +	struct xe_device *xe = gt_to_xe(gt);
> > > +	u64 offset;
> > > +	u32 reg;
> > > +
> > > +	if (GRAPHICS_VER(xe) >= 20) {
> > > +		u64 ccs_size = *tile_size / 512;
> > 
> > This value only gets used for the assertion below, but should we be 
> > taking the value from 0x8804[23:14] rather than deriving it from
> > TILE_ADDR_RANGE?
> The intention of the assertion is to verify if the flatccs base address
> provided by XE2_FLAT_CCS_BASE_RANGE_* register pair results in flatccs
> size which exactly matches the tile_size/512. In other words, it is
> validating the XE2_FLAT_CCS_BASE_RANGE_* content. 
> Therefore getting the flat_Css size from the
> XE2_FLAT_CCS_BASE_RANGE_UPPER(0x8804) itself doesn't seem to be the
> right approach to me.
> 
> > 
> > > +		u64 offset_hi, offset_lo;
> > > +		u32 nodes, num_enabled;
> > > +
> > > +		reg = xe_mmio_read32(gt, MIRROR_FUSE3);
> > > +		nodes = REG_FIELD_GET(XE2_NODE_ENABLE_MASK, reg);
> > > +		num_enabled = hweight32(nodes); /* Number of enabled l3 nodes */
> > > +
> > > +		reg = xe_gt_mcr_unicast_read_any(gt, XE2_FLAT_CCS_BASE_RANGE_LOWER);
> > > +		offset_lo = REG_FIELD_GET(XE2_FLAT_CCS_BASE_LOWER_ADDR_MASK, reg);
> > 
> > This register also appears to have a general "enable" bit for FlatCCS in
> > bit(0).  Should we be reading that to make sure FlatCCS hasn't been
> > fused off or disabled by the IFWI?
> yes, makes sense. I will update the patch.

While reworking on this change, I found that driver is already reading
this setting in the function xe_device_set_has_flat_ccs and updating the
flag has_flat_ccs.

Regards,
Bala

> 
> Regards,
> Bala
> 
> > 
> > 
> > Matt
> > 
> > > +
> > > +		reg = xe_gt_mcr_unicast_read_any(gt, XE2_FLAT_CCS_BASE_RANGE_UPPER);
> > > +		offset_hi = REG_FIELD_GET(XE2_FLAT_CCS_BASE_UPPER_ADDR_MASK, reg);
> > > +
> > > +		offset = offset_hi << 32; /* HW view bits 39:32 */
> > > +		offset |= offset_lo << 6; /* HW view bits 31:6 */
> > > +		offset *= num_enabled; /* convert to SW view */
> > > +
> > > +		/* We don't expect any holes */
> > > +		xe_assert_msg(xe, offset == (xe_mmio_read64_2x32(gt, GSMBASE) - ccs_size),
> > > +			      "Hole between CCS and GSM.\n");
> > > +	} else {
> > > +		reg = xe_gt_mcr_unicast_read_any(gt, XEHP_FLAT_CCS_BASE_ADDR);
> > > +		offset = (u64)REG_FIELD_GET(XEHP_FLAT_CCS_PTR, reg) * SZ_64K;
> > > +	}
> > > +
> > > +	return offset;
> > > +}
> > > +
> > >  /**
> > >   * xe_mmio_tile_vram_size() - Collect vram size and offset information
> > >   * @tile: tile to get info for
> > > @@ -207,8 +243,7 @@ static int xe_mmio_tile_vram_size(struct xe_tile *tile, u64 *vram_size,
> > >  
> > >  	/* minus device usage */
> > >  	if (xe->info.has_flat_ccs) {
> > > -		reg = xe_gt_mcr_unicast_read_any(gt, XEHP_FLAT_CCS_BASE_ADDR);
> > > -		offset = (u64)REG_FIELD_GET(GENMASK(31, 8), reg) * SZ_64K;
> > > +		offset = get_flat_ccs_offset(gt, tile_size);
> > >  	} else {
> > >  		offset = xe_mmio_read64_2x32(gt, GSMBASE);
> > >  	}
> > > -- 
> > > 2.25.1
> > > 
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation