[Intel-xe] [PATCH v3 9/9] drm/xe/guc: Port workarounds to OOB infra
Matt Roper
matthew.d.roper at intel.com
Wed May 17 21:25:52 UTC 2023
On Tue, May 16, 2023 at 03:19:50PM -0700, Lucas De Marchi wrote:
> Let xe_guc.c start using XE_WA() for workarounds. The one missing
> workaround is left with a "FIXME" since it's not properly implemented
> yet.
>
> Also, some changes were made to a few workarounds:
>
> - Wa_22011383443 not implemented due to being an early PVC
> stepping. Wa_16011759253 that has the same condition is the
> one covering DG2.
>
> - Wa_22011391025 and Wa_14012197797 were split in 2, since they
> cover different steppings and we do want to report both as
> implemented. The latter may eventually be removed due to being
> early DG2 steppings, but the former should remain.
>
> - Wa_22012727170 and Wa_22012727685 were split in 2 so both of
> them can be reported as active, regardless if they (currently)
> have the same rules.
>
> - Wa_16015675438 and Wa_18020744125 were not checking by
> platform or IP version, hence making them not future-proof.
> Those workarounds should only be active in PVC and DG2,
> besides the check for "no render engine".
>
> DG2 system used for test:
> XE_DG2 G12 56b2:0001 dgfx:1 gfx:Xe_HPG (12.55) media:Xe_HPM (12.55) ...
> Stepping = (G:A1, M:A1, D:C0, B:**)
>
> Output is as expected for the current OOB workarounds:
>
> $ cat /sys/kernel/debug/dri/0/workarounds
> ...
> OOB Workarounds
> 14014475959
> 22011391025
> 14012197797
> 16015675438
> 16016323070
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 2 +-
> drivers/gpu/drm/xe/xe_guc.c | 43 ++++++++----------------------
> drivers/gpu/drm/xe/xe_wa_oob.rules | 16 +++++++++++
> 3 files changed, 28 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index fcf04951a309..07af0d022263 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -38,7 +38,7 @@ $(XE_WA_OOB) &: $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
> @mkdir -p $(@D)
> $(call cmd,wa_oob)
>
> -$(obj)/xe_wa.o: $(XE_WA_OOB)
> +$(obj)/xe_wa.o $(obj)/xe_guc.o: $(XE_WA_OOB)
>
> # Please keep these build lists sorted!
>
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index 1b5d0b6ad180..252a4bfe6d63 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -5,6 +5,7 @@
>
> #include "xe_guc.h"
>
> +#include "generated/xe_wa_oob.h"
> #include "regs/xe_gt_regs.h"
> #include "regs/xe_guc_regs.h"
> #include "xe_bo.h"
> @@ -20,6 +21,7 @@
> #include "xe_mmio.h"
> #include "xe_platform_types.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,16 @@ 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))
> flags |= GUC_WA_POLLCS;
>
> - /* Wa_16011759253 */
> - /* Wa_22011383443 */
> - if (IS_SUBPLATFORM_STEP(xe, XE_DG2, XE_SUBPLATFORM_DG2_G10, STEP_A0, STEP_B0))
> + if (XE_WA(xe, 16011759253))
> 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))
> flags |= GUC_WA_HOLD_CCS_SWITCHOUT;
Hmm, this workaround was already implemented incorrectly, but it does do
a good job of highlighting a shortcoming that we need to address...see
comment farther down.
>
> - /*
> - * 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, 22011391025) || XE_WA(xe, 14012197797))
> flags |= GUC_WA_DUAL_QUEUE;
>
> /*
> @@ -169,27 +158,17 @@ 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))
> 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) || XE_WA(xe, 22012727685))
> flags |= GUC_WA_CONTEXT_ISOLATION;
>
> - /* Wa_16015675438, Wa_18020744125 */
> - if (!xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_RENDER))
> + if ((XE_WA(xe, 16015675438) || XE_WA(xe, 16016323070)) &&
> + !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))
> flags |= GUC_WA_RENDER_RST_RC6_EXIT;
>
> return flags;
> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> index e69de29bb2d1..060ff25496a7 100644
> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> @@ -0,0 +1,16 @@
> +22012773006 GRAPHICS_VERSION_RANGE(1100, 1250)
> +16011759253 SUBPLATFORM(DG2, G10), STEP(A0, B0)
> +14014475959 PLATFORM(METEORLAKE), STEP(A0, B0)
In general we can't match on platform anymore for disaggregated IP
platforms; we need to match solely on IP+version now. For example, this
workaround applies to 12.70, but does not apply to 12.71 and also does
not apply to the media GT for media version 13. The old way of doing
things with subplatforms also doesn't work well anymore since we have to
be ready for this 12.70 graphics chiplet to potentially be re-used in
some future non-MTL platform.
A first step of doing
14014475959 GRAPHICS_VERSION(1270), STEP(A0, B0)
isn't quite enough since it will still result in this workaround being
applied every time guc_ctl_wa_flags() gets called on a platform with
graphics version 12.70. In reality, we only want this workaround to be
applied on the _primary_ GT if the graphics version is 12.70. When we
loop around and run the function a second time on the media GT, it
should not be applied since media version 13 does not need this
workaround. Other workarounds might apply to both graphics and media
GTs, but only specific steppings of each, so we need to specify which IP
to check the stepping for. At the moment XE_RTP_MATCH_STEP can only
cope with graphics steppings in rule_matches().
So maybe we want something like:
12345678900 GRAPHICS_VER_STEP(1270, B0, B3)
MEDIA_VER_STEP(1300, C1, F4)
that ties the stepping to the specific IP and version. And then we'd
probably also want XE_WA to take an xe_gt as a parameter so that it can
make sure that gt->type is appropriate as well.
Matt
> + PLATFORM(DG2)
> +22011391025 PLATFORM(DG2)
> +14012197797 PLATFORM(DG2), STEP(A0, B0)
> +16011777198 SUBPLATFORM(DG2, G10), STEP(A0, C0)
> + SUBPLATFORM(DG2, G11), STEP(A0, B0)
> +22012727170 SUBPLATFORM(DG2, G10), STEP(A0, C0)
> + SUBPLATFORM(DG2, G11), STEP(A0, FOREVER)
> +22012727685 SUBPLATFORM(DG2, G10), STEP(A0, C0)
> + SUBPLATFORM(DG2, G11), STEP(A0, FOREVER)
> +16015675438 PLATFORM(PVC)
> + PLATFORM(DG2)
> +16016323070 PLATFORM(PVC)
> +1509372804 PLATFORM(PVC), STEP(B0, C0)
> --
> 2.40.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list