[Intel-xe] [PATCH 7/7] drm/xe/guc: Port workarounds to OOB infra

[Souza, Jose]

On Thu, 2023-05-04 at 00:32 -0700, Lucas De Marchi wrote:
> Let xe_guc.c start using XE_WA() for workarounds. One workaround
> couldn't be converted due to no support for function hook checks.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc.c | 40 +++++++++++++------------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index f7d32b744247..eea5ed015282 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -19,7 +19,9 @@
>  #include "xe_guc_submit.h"
>  #include "xe_mmio.h"
>  #include "xe_platform_types.h"
> +#include "xe_rtp.h"
>  #include "xe_uc_fw.h"
> +#include "xe_wa.h"
>  #include "xe_wopcm.h"
>  
>  #define MEDIA_GUC_HOST_INTERRUPT        XE_REG(0x190304)
> @@ -136,29 +138,22 @@ static u32 guc_ctl_wa_flags(struct xe_guc *guc)
>  	struct xe_gt *gt = guc_to_gt(guc);
>  	u32 flags = 0;
>  
> -	/* Wa_22012773006:gen11,gen12 < XeHP */
> -	if (GRAPHICS_VER(xe) >= 11 &&
> -	    GRAPHICS_VERx100(xe) < 1250)
> +	if (XE_WA(xe, "22012773006", GRAPHICS_VERSION_RANGE(1100, 1250)))
>  		flags |= GUC_WA_POLLCS;

Just a suggestion but would be better to have the workaround rule defined somewhere else and here only have 'if (XE_WA(xe, "22012773006"))'.


Mesa now has something like that.
We have a Python script that generates a JSON file with workaround data from the workaround database API.
During build time, another Python script generates the intel_wa.c file using the information in JSON, setting a bit for each workaround that applies
to each platform.
In runtime it get the bit field with the workarounds that applies to running platform on given stepping.

In code Mesa we have 'if (intel_needs_workaround(devinfo, 14015055625))', Xe kmd would probably need additional checks like 'if (engine == BLIT &&
XE_WA(xe, "xxxxxx"))'.

This solve the problem we have were workarounds are left enabled in platforms that already have a HW fix for it.

Not sure if such think would be acceptable in Linux, just sharing what Mesa have done.

>  
> -	/* Wa_16011759253 */
> -	/* Wa_22011383443 */
> -	if (IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G10, STEP_A0, STEP_B0))
> +	if (XE_WA(xe, "16011759253, 22011383443",
> +		  SUBPLATFORM(DG2, G10), STEP(A0, B0)))
>  		flags |= GUC_WA_GAM_CREDITS;
>  
> -	/* Wa_14014475959 */
> -	if (IS_PLATFORM_STEP(xe, XE_METEORLAKE, STEP_A0, STEP_B0) ||
> -	    xe->info.platform == XE_DG2)
> +	if (XE_WA(xe, "14014475959[mtl]", PLATFORM(METEORLAKE), STEP(A0, B0)) ||
> +	    XE_WA(xe, "14014475959[dg2]", PLATFORM(DG2)))
>  		flags |= GUC_WA_HOLD_CCS_SWITCHOUT;
>  
>  	/*
> -	 * Wa_14012197797
> -	 * Wa_22011391025
> -	 *
>  	 * The same WA bit is used for both and 22011391025 is applicable to
>  	 * all DG2.
>  	 */
> -	if (xe->info.platform == XE_DG2)
> +	if (XE_WA(xe, "14012197797, 22011391025", PLATFORM(DG2)))
>  		flags |= GUC_WA_DUAL_QUEUE;
>  
>  	/*
> @@ -169,27 +164,20 @@ static u32 guc_ctl_wa_flags(struct xe_guc *guc)
>  	if (GRAPHICS_VERx100(xe) < 1270)
>  		flags |= GUC_WA_PRE_PARSER;
>  
> -	/* Wa_16011777198 */
> -	if (IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G10, STEP_A0, STEP_C0) ||
> -	    IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G11, STEP_A0,
> -				STEP_B0))
> +	if (XE_WA(xe, "16011777198[dg2_g10]", SUBPLATFORM(DG2, G10), STEP(A0, C0)) ||
> +	    XE_WA(xe, "16011777198[dg2_g11]", SUBPLATFORM(DG2, G11), STEP(A0, B0)))
>  		flags |= GUC_WA_RCS_RESET_BEFORE_RC6;
>  
> -	/*
> -	 * Wa_22012727170
> -	 * Wa_22012727685
> -	 */
> -	if (IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G10, STEP_A0, STEP_C0) ||
> -	    IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G11, STEP_A0,
> -				STEP_FOREVER))
> +	if (XE_WA(xe, "22012727170, 22012727685 [dg2_g10]", SUBPLATFORM(DG2, G10), STEP(A0, C0)) ||
> +	    XE_WA(xe, "22012727170, 22012727685 [dg2_g11]", SUBPLATFORM(DG2, G11), STEP(A0, FOREVER)))
>  		flags |= GUC_WA_CONTEXT_ISOLATION;
>  
>  	/* Wa_16015675438, Wa_18020744125 */
> +	/* FIXME: currently there is no support for gt checks in XE_WA() */
>  	if (!xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_RENDER))
>  		flags |= GUC_WA_RCS_REGS_IN_CCS_REGS_LIST;
>  
> -	/* Wa_1509372804 */
> -	if (IS_PLATFORM_STEP(xe, XE_PVC, STEP_B0, STEP_C0))
> +	if (XE_WA(xe, "1509372804", PLATFORM(PVC), STEP(B0, C0)))
>  		flags |= GUC_WA_RENDER_RST_RC6_EXIT;
>  
>  	return flags;