[PATCH] drm/xe: Write all slices if its mcr register

[Lucas De Marchi]

On Tue, Aug 06, 2024 at 09:14:15AM GMT, Upadhyay, Tejas wrote:
>
>
>> -----Original Message-----
>> From: Roper, Matthew D <matthew.d.roper at intel.com>
>> Sent: Thursday, August 1, 2024 1:52 AM
>> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
>> Cc: intel-xe at lists.freedesktop.org
>> Subject: Re: [PATCH] drm/xe: Write all slices if its mcr register
>>
>> On Wed, Jul 31, 2024 at 03:01:52PM +0530, Tejas Upadhyay wrote:
>> > Register GAMREQSTRM_CTRL should be considered mcr register which
>> > should write to all slices as per documentation.
>> >
>> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
>>
>> Bspec: 71185
>> Fixes: 01570b446939 ("drm/xe/bmg: implement Wa_16023588340")
>> Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>>
>> > ---
>> >  drivers/gpu/drm/xe/regs/xe_gt_regs.h | 2 +-
>> >  drivers/gpu/drm/xe/xe_gt.c           | 8 ++++----
>> >  2 files changed, 5 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > index 3b87f95f9ecf..b131e3171899 100644
>> > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > @@ -80,7 +80,7 @@
>> >  #define   LE_CACHEABILITY_MASK			REG_GENMASK(1, 0)
>> >  #define   LE_CACHEABILITY(value)
>> 	REG_FIELD_PREP(LE_CACHEABILITY_MASK, value)
>> >
>> > -#define XE2_GAMREQSTRM_CTRL			XE_REG(0x4194)
>> > +#define XE2_GAMREQSTRM_CTRL
>> 	XE_REG_MCR(0x4194)
>> >  #define   CG_DIS_CNTLBUS			REG_BIT(6)
>> >
>> >  #define CCS_AUX_INV				XE_REG(0x4208)
>> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> > index 58895ed22f6e..c6296dc8897f 100644
>> > --- a/drivers/gpu/drm/xe/xe_gt.c
>> > +++ b/drivers/gpu/drm/xe/xe_gt.c
>> > @@ -110,9 +110,9 @@ static void xe_gt_enable_host_l2_vram(struct xe_gt
>> > *gt)
>> >
>> >  	if (!xe_gt_is_media_type(gt)) {
>> >  		xe_mmio_write32(gt, SCRATCH1LPFC,
>> EN_L3_RW_CCS_CACHE_FLUSH);
>> > -		reg = xe_mmio_read32(gt, XE2_GAMREQSTRM_CTRL);
>> > +		reg = xe_gt_mcr_unicast_read_any(gt,
>> XE2_GAMREQSTRM_CTRL);
>
>This is showing regression, mcr init is little later after this call. Should we read first slice default using  = xe_mmio_read32() or we should shuffle order of mcr_init and xe_gt_enable_host_l2_vram(). Any suggestion?

we were already calling xe_gt_mcr_multicast_write() in the end of this
function.  Although it works since we just rely on the
mcr_lock/mcr_unlock since multicast is the default, I don't think we
should call it before the xe_gt_mcr_init(). It access the hw via the
init() callbacks, but they are mostly sw-only initialization or reading
a fuse. It shouldn't have any impact on l2 vram being enabled/disabled
afaict.

so... shuffle the init and make sure we initialize
mcr before xe_gt_enable_host_l2_vram().

Lucas De Marchi

>
>Thanks,
>Tejas
>> >  		reg |= CG_DIS_CNTLBUS;
>> > -		xe_mmio_write32(gt, XE2_GAMREQSTRM_CTRL, reg);
>> > +		xe_gt_mcr_multicast_write(gt, XE2_GAMREQSTRM_CTRL,
>> reg);
>> >  	}
>> >
>> >  	xe_gt_mcr_multicast_write(gt, XEHPC_L3CLOS_MASK(3), 0x3); @@ -
>> 134,9
>> > +134,9 @@ static void xe_gt_disable_host_l2_vram(struct xe_gt *gt)
>> >  	if (WARN_ON(err))
>> >  		return;
>> >
>> > -	reg = xe_mmio_read32(gt, XE2_GAMREQSTRM_CTRL);
>> > +	reg = xe_gt_mcr_unicast_read_any(gt, XE2_GAMREQSTRM_CTRL);
>> >  	reg &= ~CG_DIS_CNTLBUS;
>> > -	xe_mmio_write32(gt, XE2_GAMREQSTRM_CTRL, reg);
>> > +	xe_gt_mcr_multicast_write(gt, XE2_GAMREQSTRM_CTRL, reg);
>> >
>> >  	xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);  }
>> > --
>> > 2.25.1
>> >
>>
>> --
>> Matt Roper
>> Graphics Software Engineer
>> Linux GPU Platform Enablement
>> Intel Corporation