On Fri, Aug 18, 2023 at 11:41:33AM +0530, Balasubramani Vivekanandan wrote:
>On 17.08.2023 07:47, Lucas De Marchi wrote:
>> On Wed, Aug 16, 2023 at 03:27:54PM -0700, Matt Atwood wrote:
>> > On Fri, Aug 11, 2023 at 09:06:06AM -0700, Lucas De Marchi wrote:
>> > > From: Matt Roper <matthew.d.roper at intel.com>
>> > >
>> > > Xe2 uses the same steering control register and steering semaphore
>> > > register as MTL. As with recent platforms, group/instance 0,0 is
>> > > sufficient to target a non-terminated instance for most classes of MCR
>> > > registers; the only types of ranges that need to consider platform
>> > > fusing to find a non-terminated instance are SLICE/DSS ranges and a new
>> > > SQIDI_PSMI type of range.
>> > >
>> > > Note that the range of valid bits in XE2_NODE_ENABLE_MASK may be reduced
>> > > for some Xe2 SKUs. However the lowest bits are always valid and only
>> > > the lowest instance is obtained via __ffs(), so there's no need to
>> > > complicate the masking with extra platform/subplatform checks.
>> > >
>> > > Also note that Wa_14017387313 suggests skipping MCR lock acquisition
>> > > around GAM and GAMWKR registers to prevent MCR register accesses in an
>> > > interrupt handler from deadlocking when the steering semaphore is
>> > > already held outside the interrupt context. At this time Xe never
>> > > issues MCR accesses from within an interrupt handler so the workaround
>> > > is not currently needed.
>> > >
>> > > Bspec: 71185
>> > > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> > > ---
>> > > drivers/gpu/drm/xe/regs/xe_gt_regs.h | 1 +
>> > > drivers/gpu/drm/xe/xe_gt_mcr.c | 50 +++++++++++++++++++++++++++-
>> > > drivers/gpu/drm/xe/xe_gt_types.h | 1 +
>> > > 3 files changed, 51 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > > index ad7ae6a28caca..5bc6c7fa4759f 100644
>> > > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > > @@ -145,6 +145,7 @@
>> > > #define EN_32B_ACCESS REG_BIT(30)
>> > >
>> > > #define MIRROR_FUSE3 XE_REG(0x9118)
>> > > +#define XE2_NODE_ENABLE_MASK REG_GENMASK(31, 16)
>> > > #define L3BANK_PAIR_COUNT 4
>> > > #define L3BANK_MASK REG_GENMASK(3, 0)
>> > > /* on Xe_HP the same fuses indicates mslices instead of L3 banks */
>> > > diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > index 9eb7a6a1348d6..ef9f4044ca1fd 100644
>> > > --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>> > > @@ -181,6 +181,39 @@ static const struct xe_mmio_range dg2_implicit_steering_table[] = {
>> > > {},
>> > > };
>> > >
>> > > +static const struct xe_mmio_range xe2lpg_dss_steering_table[] = {
>> > > + { 0x005200, 0x0052FF }, /* SLICE */
>> > > + { 0x005500, 0x007FFF }, /* SLICE */
>> > > + { 0x008140, 0x00815F }, /* SLICE (0x8140-0x814F), DSS (0x8150-0x815F) */
>> > > + { 0x0094D0, 0x00955F }, /* SLICE (0x94D0-0x951F), DSS (0x9520-0x955F) */
>> > > + { 0x009680, 0x0096FF }, /* DSS */
>> > > + { 0x00D800, 0x00D87F }, /* SLICE */
>> > > + { 0x00DC00, 0x00DCFF }, /* SLICE */
>> > > + { 0x00DE80, 0x00E8FF }, /* DSS (0xE000-0xE0FF reserved) */
>> > > + { 0x00E980, 0x00E9FF }, /* SLICE */
>> > > + { 0x013000, 0x0133FF }, /* DSS (0x13000-0x131FF), SLICE (0x13200-0x133FF) */
>> > > + {},
>> > > +};
>> > > +
>> > > +static const struct xe_mmio_range xe2lpg_sqidi_psmi_steering_table[] = {
>> > > + { 0x000B00, 0x000BFF },
>> > > + { 0x001000, 0x001FFF },
>> > > + {},
>> > > +};
>> > > +
>> > > +static const struct xe_mmio_range xe2lpg_instance0_steering_table[] = {
>> > > + { 0x004000, 0x004AFF }, /* GAM, rsvd, GAMWKR */
>> > ^(4200->43FF reserved)
>>
>> This is intended so we have simpler ranges to lookup on every mcr
>> register access. Intention is to keep the table as small as possible
>> without causing issues.
>>
>> If it's reserved, there shouldn't be any access going on and we can
>> coalesce the range with the next ranges. Explicitely leaving out the
>> rsvd range would benefit debugging as it would automatically create a
>> warning when accessing such a register, but I'm not sure it's worth it.
>> Matt Roper, any thoughts?
>>
>> > > + { 0x008700, 0x0087FF }, /* SQIDI, MEMPIPE */
>> > 8700->87FF is SQIDI only, to include MEMPIPE entry extend to 887F.
>>
>> ack
>>
>> > > + { 0x00B000, 0x00B3FF }, /* NODE, L3BANK */
>> > > + { 0x00C800, 0x00CFFF }, /* GAM */
>> > > + { 0x00D880, 0x00D8FF }, /* NODE */
>> > > + { 0x00DD00, 0x00DDFF }, /* MEMPIPE */
>> > > + { 0x00E900, 0x00E97F }, /* MEMPIPE */
>> > > + { 0x00EF00, 0x00F4FF }, /* GAM, GAMWKR */
>> > GAM starts ate F000, EF00->EFFF is reserved.
>>
>> same reason as above, we don't care about the reserved "holes"
>
>I guess the above range was calculated without using project filter in
>the Bspec. After applying the filter for LunarLake, the range turns out
>to be
>
> {0x00F000, 0xFFFF}, /* GAM, GAMWKR */
ack. Or an older version of the table. I'm fixing this and double
checking the ranges for next version. Thanks
Lucas De Marchi