[PATCH 5/5] drm/xe: Move a couple static workarounds/tunings to the static list

[Tvrtko Ursulin]

On 29/01/2025 18:51, Lucas De Marchi wrote:
> On Wed, Jan 29, 2025 at 03:56:27PM +0000, Tvrtko Ursulin wrote:
>> One workaround and one tuning are completely static so can be moved to 
>> the
>> respective static lists in xe_wa.c and xa_tunings.c respectively.
> 
> We can enable the tracker to make them show up in debugfs if needed, but
> that wouldn't be in the WA file. See more details below.

Crossed my mind if that would be possible but the lists are local to the 
functions so looked hard. Anyway, also lets go below..

>> Advantage being that they will be shown in debugfs as active, when 
>> active.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_hw_engine.c | 22 ----------------------
>> drivers/gpu/drm/xe/xe_tuning.c    |  8 ++++++++
>> drivers/gpu/drm/xe/xe_wa.c        | 16 ++++++++++++++++
>> 3 files changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c 
>> b/drivers/gpu/drm/xe/xe_hw_engine.c
>> index fc447751fe78..423fb43a1a4a 100644
>> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>> @@ -437,28 +437,6 @@ hw_engine_setup_default_state(struct xe_hw_engine 
>> *hwe)
>>                        ring_cmd_cctl_val,
>>                        XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>>         },
>> -        /*
>> -         * To allow the GSC engine to go idle on MTL we need to enable
>> -         * idle messaging and set the hysteresis value (we use 0xA=5us
>> -         * as recommended in spec). On platforms after MTL this is
>> -         * enabled by default.
>> -         */
>> -        { XE_RTP_NAME("MTL GSCCS IDLE MSG enable"),
>> -          XE_RTP_RULES(MEDIA_VERSION(1300), ENGINE_CLASS(OTHER)),
>> -          XE_RTP_ACTIONS(CLR(RING_PSMI_CTL(0),
>> -                     IDLE_MSG_DISABLE,
>> -                     XE_RTP_ACTION_FLAG(ENGINE_BASE)),
>> -                 FIELD_SET(RING_PWRCTX_MAXCNT(0),
>> -                       IDLE_WAIT_TIME,
>> -                       0xA,
>> -                       XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>> -        },
>> -        /* Enable Priority Mem Read */
>> -        { XE_RTP_NAME("Priority_Mem_Read"),
>> -          XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, 
>> XE_RTP_END_VERSION_UNDEFINED)),
>> -          XE_RTP_ACTIONS(SET(CSFE_CHICKEN1(0), CS_PRIORITY_MEM_READ,
>> -                     XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>> -        },
>>         {}
>>     };
>>
>> diff --git a/drivers/gpu/drm/xe/xe_tuning.c 
>> b/drivers/gpu/drm/xe/xe_tuning.c
>> index c5c5a6fba406..f9d178ae0a73 100644
>> --- a/drivers/gpu/drm/xe/xe_tuning.c
>> +++ b/drivers/gpu/drm/xe/xe_tuning.c
>> @@ -10,6 +10,7 @@
>> #include <drm/drm_managed.h>
>>
>> #include "regs/xe_gt_regs.h"
>> +#include "regs/xe_engine_regs.h"
>> #include "xe_gt_types.h"
>> #include "xe_platform_types.h"
>> #include "xe_rtp.h"
>> @@ -95,6 +96,13 @@ static const struct xe_rtp_entry_sr 
>> engine_tunings[] = {
> 
> these will end up as a register for guc to save restore, not the same
> thing as setting up the default engine context, so there maybe side
> effects of this change.

Ok.

> 
>>                ENGINE_CLASS(RENDER)),
>>       XE_RTP_ACTIONS(SET(SAMPLER_MODE, 
>> INDIRECT_STATE_BASE_ADDR_OVERRIDE))
>>     },
>> +
>> +    { XE_RTP_NAME("Priority_Mem_Read"),
>> +      XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, 
>> XE_RTP_END_VERSION_UNDEFINED)),
>> +      XE_RTP_ACTIONS(SET(CSFE_CHICKEN1(0), CS_PRIORITY_MEM_READ,
>> +                 XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>> +    },
>> +
>>     {}
>> };
>>
>> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>> index 744dba4fdb58..1380bcf17fac 100644
>> --- a/drivers/gpu/drm/xe/xe_wa.c
>> +++ b/drivers/gpu/drm/xe/xe_wa.c
>> @@ -458,6 +458,22 @@ static const struct xe_rtp_entry_sr engine_was[] = {
>>                FUNC(xe_rtp_match_first_render_or_compute)),
>>       XE_RTP_ACTIONS(SET(ROW_CHICKEN2, DISABLE_TDL_SVHS_GATING))
>>     },
>> +    /*
>> +     * To allow the GSC engine to go idle on MTL we need to enable
>> +     * idle messaging and set the hysteresis value (we use 0xA=5us
>> +     * as recommended in spec). On platforms after MTL this is
>> +     * enabled by default.
>> +     */
>> +    { XE_RTP_NAME("MTL GSCCS IDLE MSG enable"),
> 
> tunning != WA... this table should only contains entries that are actual
> documented hw WAs.

I try to asses which one and was swayed by the "we need" above. Sounded 
like the thing is doing two things. One is "need", other is tuning the 
hysteresis. Anyway I'll drop this patch, there is bigger fish to fry.

Regards,

Tvrtko

> 
> there's a debugfs file to dump the default context, however it seems we
> are missing it for gsc.
> 
> 
> Lucas De Marchi
> 
>> +      XE_RTP_RULES(MEDIA_VERSION(1300), ENGINE_CLASS(OTHER)),
>> +      XE_RTP_ACTIONS(CLR(RING_PSMI_CTL(0),
>> +                 IDLE_MSG_DISABLE,
>> +                 XE_RTP_ACTION_FLAG(ENGINE_BASE)),
>> +             FIELD_SET(RING_PWRCTX_MAXCNT(0),
>> +                   IDLE_WAIT_TIME,
>> +                   0xA,
>> +                   XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>> +    },
>>
>>     /* Xe2_LPG */
>>
>> -- 
>> 2.48.0
>>