[Intel-xe] [PATCH 07/12] drm/xe/gsc: Implement WA 14015076503

[John Harrison]

On 10/27/2023 15:29, Daniele Ceraolo Spurio wrote:
> When the GSC FW is loaded, we need to inform it when a GSCCS reset is
> coming and then wait 200ms for it to get ready to process the reset.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: John Harrison <john.c.harrison at intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile           |  2 +-
>   drivers/gpu/drm/xe/regs/xe_gsc_regs.h | 10 ++++++
>   drivers/gpu/drm/xe/xe_gt.c            | 44 +++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_wa_oob.rules    |  1 +
>   4 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 474b6044d054..4a442dcf4d79 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -37,7 +37,7 @@ quiet_cmd_wa_oob = GEN     $(notdir $(generated_oob))
>   $(generated_oob) &: $(obj)/xe_gen_wa_oob $(srctree)/$(src)/xe_wa_oob.rules
>   	$(call cmd,wa_oob)
>   
> -$(obj)/xe_guc.o $(obj)/xe_migrate.o $(obj)/xe_ring_ops.o $(obj)/xe_vm.o $(obj)/xe_wa.o: $(generated_oob)
> +$(obj)/xe_gt.o $(obj)/xe_guc.o $(obj)/xe_migrate.o $(obj)/xe_ring_ops.o $(obj)/xe_vm.o $(obj)/xe_wa.o: $(generated_oob)
Is it not worth splitting this into multiple lines?

>   
>   # Please keep these build lists sorted!
>   
> diff --git a/drivers/gpu/drm/xe/regs/xe_gsc_regs.h b/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
> index 22d2ad9cb64d..9a84b55d66ee 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gsc_regs.h
> @@ -16,6 +16,13 @@
>   #define MTL_GSC_HECI1_BASE	0x00116000
>   #define MTL_GSC_HECI2_BASE	0x00117000
>   
> +#define HECI_H_CSR(base)	XE_REG((base) + 0x4)
> +#define   HECI_H_CSR_IE		REG_BIT(0)
> +#define   HECI_H_CSR_IS		REG_BIT(1)
> +#define   HECI_H_CSR_IG		REG_BIT(2)
> +#define   HECI_H_CSR_RDY	REG_BIT(3)
> +#define   HECI_H_CSR_RST	REG_BIT(4)
> +
>   /*
>    * The FWSTS register values are FW defined and can be different between
>    * HECI1 and HECI2
> @@ -26,4 +33,7 @@
>   #define   HECI1_FWSTS1_PROXY_STATE_NORMAL		5
>   #define   HECI1_FWSTS1_INIT_COMPLETE			REG_BIT(9)
>   
> +#define HECI_H_GS1(base)	XE_REG((base) + 0xc4c)
> +#define   HECI_H_GS1_ER_PREP	REG_BIT(0)
> +
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 73c090762771..0e8e24bded1c 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -10,8 +10,10 @@
>   #include <drm/drm_managed.h>
>   #include <drm/xe_drm.h>
>   
> +#include "generated/xe_wa_oob.h"
>   #include "instructions/xe_gfxpipe_commands.h"
>   #include "instructions/xe_mi_commands.h"
> +#include "regs/xe_gsc_regs.h"
>   #include "regs/xe_gt_regs.h"
>   #include "xe_assert.h"
>   #include "xe_bb.h"
> @@ -504,16 +506,58 @@ int xe_gt_init(struct xe_gt *gt)
>   	return 0;
>   }
>   
> +static int gsc_fw_is_loaded(struct xe_gt *gt)
> +{
> +	return xe_mmio_read32(gt, HECI_FWSTS1(MTL_GSC_HECI1_BASE)) &
> +			      HECI1_FWSTS1_INIT_COMPLETE;
> +}
This function is duplicated exactly in the GSC source file. That seems 
bad. Either it should be exported and shared or the usage here should be 
moved to the GSC specific file.

John.

> +
> +static void wa_14015076503_start(struct xe_gt *gt)
> +{
> +	/* WA only applies if the GSC is loaded */
> +	if (!XE_WA(gt, 14015076503) || !gsc_fw_is_loaded(gt))
> +		return;
> +
> +	/*
> +	 * wa_14015076503: if the GSC FW is loaded, we need to alert it that
> +	 * we're going to do a GSC engine reset and then wait for 200ms for the
> +	 * FW to get ready for it.
> +	 */
> +	xe_mmio_rmw32(gt, HECI_H_GS1(MTL_GSC_HECI2_BASE),
> +		      0, HECI_H_GS1_ER_PREP);
> +
> +	/* make sure the reset bit is clear when writing the CSR reg */
> +	xe_mmio_rmw32(gt, HECI_H_CSR(MTL_GSC_HECI2_BASE),
> +		      HECI_H_CSR_RST, HECI_H_CSR_IG);
> +	msleep(200);
> +
> +	return;
> +}
> +
> +static void
> +wa_14015076503_end(struct xe_gt *gt)
> +{
> +	if (!XE_WA(gt, 14015076503))
> +		return;
> +
> +	xe_mmio_rmw32(gt, HECI_H_GS1(MTL_GSC_HECI2_BASE),
> +		      HECI_H_GS1_ER_PREP, 0);
> +}
> +
>   static int do_gt_reset(struct xe_gt *gt)
>   {
>   	int err;
>   
> +	wa_14015076503_start(gt);
> +
>   	xe_mmio_write32(gt, GDRST, GRDOM_FULL);
>   	err = xe_mmio_wait32(gt, GDRST, GRDOM_FULL, 0, 5000, NULL, false);
>   	if (err)
>   		xe_gt_err(gt, "failed to clear GEN11_GRDOM_FULL (%pe)\n",
>   			  ERR_PTR(err));
>   
> +	wa_14015076503_end(gt);
> +
>   	return err;
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> index f3ff774dc4aa..132c35497f74 100644
> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> @@ -19,3 +19,4 @@
>   		SUBPLATFORM(DG2, G12)
>   16017236439	PLATFORM(PVC)
>   22010954014	PLATFORM(DG2)
> +14015076503	MEDIA_VERSION(1300)