[Intel-xe] [PATCH V3] drm/xe/xe2: Add workaround 18032095049 and 16021639441

[Matt Roper]

On Wed, Dec 06, 2023 at 11:16:15AM +0530, Tejas Upadhyay wrote:
> This workaround applies to graphics 20.04 on all engines.
> 
> Workaround has three parts :
> 1. Pipe flush before MI_ATOMIC - This part isn't relevant to Xe
>    (at least not right now) since we don't use MI_ATOMIC anywhere
>    in the kernel mode driver.
> 2. Memory-based interrupt masking - Memory-based interrupt processing
>    isn't supported on physical functions, only virtual functions,
>    according to bspec 60352. So this is probably only relevant once
>    SRIOV support lands in the driver.
> 3. Disabling CSB/timestamp updates to the ghwsp and pphwsp - Workaround
>    is added by this change.
> 
> The CSB reports to gHWSP and ppHWSP have been discussed as part
> of a different topic on some internal threads and we've confirmed
> that neither the KMD nor the GuC firmware use those for anything,
> so disabling them is always "safe" and should have no functional
> or performance impact on system operation.  The same is true for
> the timestamp updates in the ppHWSP as well.  Given that, it might
> make sense to just combine these two workarounds into a single
> record (and single patch) and apply it on all steppings. Disabling
> the reports for RCS on higher steppings doesn't have any kind of
> negative impact and will simplify the overall situation.
> 
> V3(MattR):
>   - Combine WA apply same WA for all engines, no performance impact
> V2(MattR):
>   - Mention detail in commit message
>   - Reorder bit define
>   - Improve bit naming
>   - Remove workaround part which isnt relevant
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_engine_regs.h |  6 ++++++
>  drivers/gpu/drm/xe/xe_wa.c               | 14 ++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> index 444ff9b83bb1..ecdbc04a2d91 100644
> --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> @@ -40,6 +40,7 @@
>  #define RING_NOPID(base)			XE_REG((base) + 0x94)
>  
>  #define RING_IMR(base)				XE_REG((base) + 0xa8)
> +
>  #define   RING_MAX_NONPRIV_SLOTS  12

You still have an unwanted whitespace change here.

With that removed,

        Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


Matt

>  
>  #define RING_EIR(base)				XE_REG((base) + 0xb0)
> @@ -47,6 +48,11 @@
>  #define RING_ESR(base)				XE_REG((base) + 0xb8)
>  
>  #define RING_CMD_CCTL(base)			XE_REG((base) + 0xc4, XE_REG_OPTION_MASKED)
> +
> +#define CSFE_CHICKEN1_REG(base)			XE_REG((base) + 0xd4, XE_REG_OPTION_MASKED)
> +#define   GHWSP_CSB_REPORT_DIS			REG_BIT(15)
> +#define   PPHWSP_CSB_AND_TIMESTAMP_REPORT_DIS	REG_BIT(14)
> +
>  /*
>   * CMD_CCTL read/write fields take a MOCS value and _not_ a table index.
>   * The lsb of each can be considered a separate enabling bit for encryption.
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index ce897f2d49be..23f1285135b8 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -596,6 +596,20 @@ static const struct xe_rtp_entry_sr engine_was[] = {
>  	  XE_RTP_RULES(GRAPHICS_VERSION(2004), FUNC(xe_rtp_match_first_render_or_compute)),
>  	  XE_RTP_ACTIONS(SET(ROW_CHICKEN, EARLY_EOT_DIS))
>  	},
> +	/*
> +	 * These two workarounds are the same, just applying to different
> +	 * engines.  Although Wa_18032095049 (for the RCS) isn't required on
> +	 * all steppings, disabling these reports has no impact for our
> +	 * driver or the GuC, so we go ahead and treat it the same as
> +	 * Wa_16021639441 which does apply to all steppings.
> +	 */
> +	{ XE_RTP_NAME("18032095049, 16021639441"),
> +	  XE_RTP_RULES(GRAPHICS_VERSION(2004)),
> +	  XE_RTP_ACTIONS(SET(CSFE_CHICKEN1_REG(0),
> +			     GHWSP_CSB_REPORT_DIS |
> +			     PPHWSP_CSB_AND_TIMESTAMP_REPORT_DIS,
> +			     XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> +	},
>  
>  	{}
>  };
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation