[PATCH 3/3] drm/xe/xe2: Add performance tuning for L3 cache flushing

[Gustavo Sousa]

Quoting Upadhyay, Tejas (2024-09-19 05:22:50-03:00)
>
>
>> -----Original Message-----
>> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Gustavo
>> Sousa
>> Sent: Thursday, September 19, 2024 2:18 AM
>> To: intel-xe at lists.freedesktop.org
>> Cc: Roper, Matthew D <matthew.d.roper at intel.com>
>> Subject: [PATCH 3/3] drm/xe/xe2: Add performance tuning for L3 cache
>> flushing
>> 
>> A recommended performance tuning for LNL related to L3 cache flushing was
>> recently introduced in Bspec. Implement it.
>> 
>> Bspec: 70821
>
>Yes bspec needs an update.

Yep.

>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/regs/xe_gt_regs.h | 5 +++++
>>  drivers/gpu/drm/xe/xe_tuning.c       | 8 ++++++++
>>  2 files changed, 13 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> index 6ec2d2c11d77..ccd18cdd5b50 100644
>> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> @@ -389,6 +389,9 @@
>>  #define L3SQCREG3                                XE_REG_MCR(0xb108)
>>  #define   COMPPWOVERFETCHEN                        REG_BIT(28)
>> 
>> +#define SCRATCH3LBCF
>>         XE_REG_MCR(0xb154)
>
>Please name this SCRATCH3 only as bspec mentions.

Looking at our register database, it looks like there are other
"SCRATCH" registers from other units. So wouldn't it be better not to
use plain SCRATCH3 here?

Probably SCRATCH3_LBCF for more readability...

>
>> +#define   RWFLUSHALLEN                                REG_BIT(17)
>> +
>>  #define XEHP_L3SQCREG5
>>         XE_REG_MCR(0xb158)
>>  #define   L3_PWM_TIMER_INIT_VAL_MASK                REG_GENMASK(9, 0)
>> 
>> @@ -406,6 +409,8 @@
>> 
>>  #define XE2LPM_L3SQCREG3                        XE_REG_MCR(0xb608)
>> 
>> +#define XE2LPM_SCRATCH3LBCF
>>         XE_REG_MCR(0xb654)
>
>Agree with other review that we should name this MEDIA_SCRATCH3 for future use well. Also this does not look to be MCR reg. Please double check.

I believe the prefix XE2LPM_ is enough to differentiate this from the
primary GT version of this register. It is common to prefix registers
with the name of the IP that introduce the difference with respect to
other versions of the register.

Please note that the same pattern is used for other registers that have
different offsets on the media GT.

--
Gustavo Sousa

>
>With that addressed, 
>Reviewed-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
>
>> +
>>  #define XE2LPM_L3SQCREG5                        XE_REG_MCR(0xb658)
>> 
>>  #define XE2_TDF_CTRL                                XE_REG(0xb418)
>> diff --git a/drivers/gpu/drm/xe/xe_tuning.c b/drivers/gpu/drm/xe/xe_tuning.c
>> index f62622f0be85..4dd77b44ac82 100644
>> --- a/drivers/gpu/drm/xe/xe_tuning.c
>> +++ b/drivers/gpu/drm/xe/xe_tuning.c
>> @@ -80,6 +80,14 @@ static const struct xe_rtp_entry_sr gt_tunings[] = {
>> 
>> XE_RTP_ACTIONS(FIELD_SET(XELPMP_STATELESS_COMPRESSION_CTRL,
>> UNIFIED_COMPRESSION_FORMAT,
>> 
>> REG_FIELD_PREP(UNIFIED_COMPRESSION_FORMAT, 0)))
>>          },
>> +        { XE_RTP_NAME("Tuning: L3 RW flush all Cache"),
>> +          XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2004,
>> XE_RTP_END_VERSION_UNDEFINED)),
>> +          XE_RTP_ACTIONS(SET(SCRATCH3, RWFLUSHALLEN))
>> +        },
>> +        { XE_RTP_NAME("Tuning: L3 RW flush all cache - media"),
>> +          XE_RTP_RULES(MEDIA_VERSION_RANGE(2000,
>> XE_RTP_END_VERSION_UNDEFINED)),
>> +          XE_RTP_ACTIONS(SET(XE2LPM_SCRATCH3LBCF, RWFLUSHALLEN))
>> +        },
>>          {}
>>  };
>> 
>> --
>> 2.46.1
>