[PATCH 2/2] drm/i915/gsc: implement wa 14015076503

John Harrison john.c.harrison at intel.com
Wed Mar 22 19:44:00 UTC 2023


On 3/20/2023 14:10, Daniele Ceraolo Spurio wrote:
> The WA states that we need to alert the GSC FW before doing a GSC engine
> reset and then wait for 200ms. The GuC owns engine reset, so on the i915
> side we only need to apply this for full GT reset.
>
> Given that we do full GT resets in the resume paths to cleanup the HW
> state and that a long wait in those scenarios would not be acceptable,
> a faster path has been introduced where, if the GSC is idle, we try first
> to individually reset the GuC and all engines except the GSC and only fall
> back to full reset if that fails.
I'm confused. If the code path is a resume then surely all engines are 
idle. But, there is presumably a reason why we do a full GT reset on the 
resume - because the hardware state is not guaranteed to be sensible at 
that point. So how is it safe to skip the GSC reset?

>
> Note: according to the WA specs, if the GSC is idle it should be possible
> to only wait for the uC wakeup time (~15ms) instead of the whole 200ms.
> However, the GSC FW team have mentioned that the wakeup time can change
> based on other things going on in the HW and pcode, so a good security
> margin would be required. Given that when the GSC is idle we already
> skip the wait & reset entirely and that this reduced wait would still
> likely be too long to use in resume paths, it's not worth adding support
> for this reduced wait.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: John Harrison <john.c.harrison at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_reset.c     | 77 +++++++++++++++++++++--
>   drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h |  2 +
>   drivers/gpu/drm/i915/i915_reg.h           | 14 ++++-
>   3 files changed, 86 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 2c3463f77e5c..5f75f59122cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -14,6 +14,8 @@
>   
>   #include "gt/intel_gt_regs.h"
>   
> +#include "gt/uc/intel_gsc_fw.h"
> +
>   #include "i915_drv.h"
>   #include "i915_file_private.h"
>   #include "i915_gpu_error.h"
> @@ -695,6 +697,67 @@ static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>   		return NULL;
>   }
>   
> +static int __reset_guc(struct intel_gt *gt)
> +{
> +	u32 guc_domain =
> +		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
> +
> +	return gen6_hw_domain_reset(gt, guc_domain);
> +}
> +
> +static bool needs_wa_14015076503(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> +{
> +	if (!IS_METEORLAKE(gt->i915) || !HAS_ENGINE(gt, GSC0))
> +		return false;
> +
> +	if (!__HAS_ENGINE(engine_mask, GSC0))
> +		return false;
> +
> +	return intel_gsc_uc_fw_init_done(&gt->uc.gsc);
> +}
> +
> +static intel_engine_mask_t
> +wa_14015076503_start(struct intel_gt *gt, intel_engine_mask_t engine_mask, bool first)
> +{
> +	if (!needs_wa_14015076503(gt, engine_mask))
> +		return engine_mask;
> +
> +	/*
> +	 * 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. However, if this the first ALL_ENGINES reset
if this is

John.

> +	 * attempt and the GSC is not busy, we can try to instead reset the GuC
> +	 * and all the other engines individually to avoid the 200ms wait.
> +	 */
> +	if (engine_mask == ALL_ENGINES && first && intel_engine_is_idle(gt->engine[GSC0])) {
> +		__reset_guc(gt);
> +		engine_mask = gt->info.engine_mask & ~BIT(GSC0);
> +	} else {
> +		intel_uncore_rmw(gt->uncore,
> +				 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 */
> +		intel_uncore_rmw(gt->uncore,
> +				 HECI_H_CSR(MTL_GSC_HECI2_BASE),
> +				 HECI_H_CSR_RST, HECI_H_CSR_IG);
> +		msleep(200);
> +	}
> +
> +	return engine_mask;
> +}
> +
> +static void
> +wa_14015076503_end(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> +{
> +	if (!needs_wa_14015076503(gt, engine_mask))
> +		return;
> +
> +	intel_uncore_rmw(gt->uncore,
> +			 HECI_H_GS1(MTL_GSC_HECI2_BASE),
> +			 HECI_H_GS1_ER_PREP, 0);
> +}
> +
>   int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>   {
>   	const int retries = engine_mask == ALL_ENGINES ? RESET_MAX_RETRIES : 1;
> @@ -712,10 +775,16 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>   	 */
>   	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
>   	for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
> -		GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
> +		intel_engine_mask_t reset_mask;
> +
> +		reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
> +
> +		GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
>   		preempt_disable();
> -		ret = reset(gt, engine_mask, retry);
> +		ret = reset(gt, reset_mask, retry);
>   		preempt_enable();
> +
> +		wa_14015076503_end(gt, reset_mask);
>   	}
>   	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>   
> @@ -740,14 +809,12 @@ bool intel_has_reset_engine(const struct intel_gt *gt)
>   
>   int intel_reset_guc(struct intel_gt *gt)
>   {
> -	u32 guc_domain =
> -		GRAPHICS_VER(gt->i915) >= 11 ? GEN11_GRDOM_GUC : GEN9_GRDOM_GUC;
>   	int ret;
>   
>   	GEM_BUG_ON(!HAS_GT_UC(gt->i915));
>   
>   	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
> -	ret = gen6_hw_domain_reset(gt, guc_domain);
> +	ret = __reset_guc(gt);
>   	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
>   
>   	return ret;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> index 4b5dbb44afb4..f4c1106bb2a9 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> @@ -9,7 +9,9 @@
>   #include <linux/types.h>
>   
>   struct intel_gsc_uc;
> +struct intel_uncore;
>   
>   int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
>   bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
> +
>   #endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d22ffd7a32dc..80e33ede7fac 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -928,8 +928,18 @@
>   #define DG1_GSC_HECI2_BASE	0x00259000
>   #define DG2_GSC_HECI1_BASE	0x00373000
>   #define DG2_GSC_HECI2_BASE	0x00374000
> -
> -
> +#define MTL_GSC_HECI1_BASE	0x00116000
> +#define MTL_GSC_HECI2_BASE	0x00117000
> +
> +#define HECI_H_CSR(base)	_MMIO(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)
> +
> +#define HECI_H_GS1(base)	_MMIO(base + 0xc4c)
> +#define   HECI_H_GS1_ER_PREP	REG_BIT(0)
>   
>   #define HSW_GTT_CACHE_EN	_MMIO(0x4024)
>   #define   GTT_CACHE_EN_ALL	0xF0007FFF



More information about the dri-devel mailing list