diff mbox series

drm/i915/reset: Add additional steps for Wa_22011802037 for execlist backend

Message ID 20220510221439.448756-1-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/reset: Add additional steps for Wa_22011802037 for execlist backend | expand

Commit Message

Umesh Nerlige Ramappa May 10, 2022, 10:14 p.m. UTC
From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

For execlists backend, current implementation of Wa_22011802037 is to
stop the CS before doing a reset of the engine. This WA was further
extended to wait for any pending MI FORCE WAKEUPs before issuing a
reset. Add the extended steps in the execlist path of reset.

In addition, extend the WA to gen11.

v2: (Tvrtko)
- Clarify comments, commit message, fix typos
- Use IS_GRAPHICS_VER for gen 11/12 checks

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Fixes: f6aa0d713c88 ("drm/i915: Add Wa_22011802037 force cs halt")
---
 drivers/gpu/drm/i915/gt/intel_engine.h        |  2 +
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 85 ++++++++++++++++++-
 .../drm/i915/gt/intel_execlists_submission.c  |  7 ++
 .../gpu/drm/i915/gt/intel_ring_submission.c   |  7 ++
 drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  4 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 81 ++----------------
 6 files changed, 107 insertions(+), 79 deletions(-)

Comments

Tvrtko Ursulin May 11, 2022, 3:26 p.m. UTC | #1
On 10/05/2022 23:14, Nerlige Ramappa, Umesh wrote:
> From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> 
> For execlists backend, current implementation of Wa_22011802037 is to
> stop the CS before doing a reset of the engine. This WA was further
> extended to wait for any pending MI FORCE WAKEUPs before issuing a
> reset. Add the extended steps in the execlist path of reset.
> 
> In addition, extend the WA to gen11.
> 
> v2: (Tvrtko)
> - Clarify comments, commit message, fix typos
> - Use IS_GRAPHICS_VER for gen 11/12 checks
> 
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Fixes: f6aa0d713c88 ("drm/i915: Add Wa_22011802037 force cs halt")
> ---
>   drivers/gpu/drm/i915/gt/intel_engine.h        |  2 +
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 85 ++++++++++++++++++-
>   .../drm/i915/gt/intel_execlists_submission.c  |  7 ++
>   .../gpu/drm/i915/gt/intel_ring_submission.c   |  7 ++
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  4 +-
>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 81 ++----------------
>   6 files changed, 107 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 1431f1e9dbee..04e435bce79b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -201,6 +201,8 @@ int intel_ring_submission_setup(struct intel_engine_cs *engine);
>   int intel_engine_stop_cs(struct intel_engine_cs *engine);
>   void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine);
>   
> +void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine);
> +
>   void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask);
>   
>   u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 14c6ddbbfde8..9943cf9655b2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1282,10 +1282,10 @@ static int __intel_engine_stop_cs(struct intel_engine_cs *engine,
>   	intel_uncore_write_fw(uncore, mode, _MASKED_BIT_ENABLE(STOP_RING));
>   
>   	/*
> -	 * Wa_22011802037 : gen12, Prior to doing a reset, ensure CS is
> +	 * Wa_22011802037 : gen11, gen12, Prior to doing a reset, ensure CS is
>   	 * stopped, set ring stop bit and prefetch disable bit to halt CS
>   	 */
> -	if (GRAPHICS_VER(engine->i915) == 12)
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
>   		intel_uncore_write_fw(uncore, RING_MODE_GEN7(engine->mmio_base),
>   				      _MASKED_BIT_ENABLE(GEN12_GFX_PREFETCH_DISABLE));
>   
> @@ -1308,6 +1308,18 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
>   		return -ENODEV;
>   
>   	ENGINE_TRACE(engine, "\n");
> +	/*
> +	 * TODO: Find out why occasionally stopping the CS times out. Seen
> +	 * especially with gem_eio tests.
> +	 *
> +	 * Occasionally trying to stop the cs times out, but does not adversely
> +	 * affect functionality. The timeout is set as a config parameter that
> +	 * defaults to 100ms. In most cases the follow up operation is to wait
> +	 * for pending MI_FORCE_WAKES. The assumption is that this timeout is
> +	 * sufficient for any pending MI_FORCEWAKEs to complete. Once root
> +	 * caused, the caller must check and handle the return from this
> +	 * function.
> +	 */
>   	if (__intel_engine_stop_cs(engine, 1000, stop_timeout(engine))) {
>   		ENGINE_TRACE(engine,
>   			     "timed out on STOP_RING -> IDLE; HEAD:%04x, TAIL:%04x\n",
> @@ -1334,6 +1346,75 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>   	ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
>   }
>   
> +static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
> +{
> +	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
> +		[RCS0] = MSG_IDLE_CS,
> +		[BCS0] = MSG_IDLE_BCS,
> +		[VCS0] = MSG_IDLE_VCS0,
> +		[VCS1] = MSG_IDLE_VCS1,
> +		[VCS2] = MSG_IDLE_VCS2,
> +		[VCS3] = MSG_IDLE_VCS3,
> +		[VCS4] = MSG_IDLE_VCS4,
> +		[VCS5] = MSG_IDLE_VCS5,
> +		[VCS6] = MSG_IDLE_VCS6,
> +		[VCS7] = MSG_IDLE_VCS7,
> +		[VECS0] = MSG_IDLE_VECS0,
> +		[VECS1] = MSG_IDLE_VECS1,
> +		[VECS2] = MSG_IDLE_VECS2,
> +		[VECS3] = MSG_IDLE_VECS3,
> +		[CCS0] = MSG_IDLE_CS,
> +		[CCS1] = MSG_IDLE_CS,
> +		[CCS2] = MSG_IDLE_CS,
> +		[CCS3] = MSG_IDLE_CS,
> +	};
> +	u32 val;
> +
> +	if (!_reg[engine->id].reg)
> +		return 0;

Should this actually be a GEM_WARN_ON condition, to catch failures to 
update for new platforms?

> +
> +	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
> +
> +	/* bits[29:25] & bits[13:9] >> shift */
> +	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
> +}
> +
> +static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
> +{
> +	int ret;
> +
> +	/* Ensure GPM receives fw up/down after CS is stopped */
> +	udelay(1);
> +
> +	/* Wait for forcewake request to complete in GPM */
> +	ret =  __intel_wait_for_register_fw(gt->uncore,
> +					    GEN9_PWRGT_DOMAIN_STATUS,
> +					    fw_mask, fw_mask, 5000, 0, NULL);

No fast-slow split as discussed but okay, it's not making anything worse 
that it already is.

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> +
> +	/* Ensure CS receives fw ack from GPM */
> +	udelay(1);
> +
> +	if (ret)
> +		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
> +}
> +
> +/*
> + * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
> + * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
> + * pending status is indicated by bits[13:9] (masked by bits[29:25]) in the
> + * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
> + * are concerned only with the gt reset here, we use a logical OR of pending
> + * forcewakeups from all reset domains and then wait for them to complete by
> + * querying PWRGT_DOMAIN_STATUS.
> + */
> +void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine)
> +{
> +	u32 fw_pending = __cs_pending_mi_force_wakes(engine);
> +
> +	if (fw_pending)
> +		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
> +}
> +
>   static u32
>   read_subslice_reg(const struct intel_engine_cs *engine,
>   		  int slice, int subslice, i915_reg_t reg)
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 86f7a9ac1c39..2caa1af77064 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -2958,6 +2958,13 @@ static void execlists_reset_prepare(struct intel_engine_cs *engine)
>   	ring_set_paused(engine, 1);
>   	intel_engine_stop_cs(engine);
>   
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
> +		intel_engine_wait_for_pending_mi_fw(engine);
> +
>   	engine->execlists.reset_ccid = active_ccid(engine);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index 5423bfd301ad..a7808eff33c5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -323,6 +323,13 @@ static void reset_prepare(struct intel_engine_cs *engine)
>   	ENGINE_TRACE(engine, "\n");
>   	intel_engine_stop_cs(engine);
>   
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
> +		intel_engine_wait_for_pending_mi_fw(engine);
> +
>   	if (!stop_ring(engine)) {
>   		/* G45 ring initialization often fails to reset head to zero */
>   		ENGINE_TRACE(engine,
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 2c4ad4a65089..8c6885f43d1a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -310,8 +310,8 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
>   	if (IS_DG2(gt->i915))
>   		flags |= GUC_WA_DUAL_QUEUE;
>   
> -	/* Wa_22011802037: graphics version 12 */
> -	if (GRAPHICS_VER(gt->i915) == 12)
> +	/* Wa_22011802037: graphics version 11/12 */
> +	if (IS_GRAPHICS_VER(gt->i915, 11, 12))
>   		flags |= GUC_WA_PRE_PARSER;
>   
>   	/* Wa_16011777198:dg2 */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 75291e9846c5..9b21c7345ffd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1527,87 +1527,18 @@ static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub)
>   	lrc_update_regs(ce, engine, head);
>   }
>   
> -static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
> -{
> -	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
> -		[RCS0] = MSG_IDLE_CS,
> -		[BCS0] = MSG_IDLE_BCS,
> -		[VCS0] = MSG_IDLE_VCS0,
> -		[VCS1] = MSG_IDLE_VCS1,
> -		[VCS2] = MSG_IDLE_VCS2,
> -		[VCS3] = MSG_IDLE_VCS3,
> -		[VCS4] = MSG_IDLE_VCS4,
> -		[VCS5] = MSG_IDLE_VCS5,
> -		[VCS6] = MSG_IDLE_VCS6,
> -		[VCS7] = MSG_IDLE_VCS7,
> -		[VECS0] = MSG_IDLE_VECS0,
> -		[VECS1] = MSG_IDLE_VECS1,
> -		[VECS2] = MSG_IDLE_VECS2,
> -		[VECS3] = MSG_IDLE_VECS3,
> -		[CCS0] = MSG_IDLE_CS,
> -		[CCS1] = MSG_IDLE_CS,
> -		[CCS2] = MSG_IDLE_CS,
> -		[CCS3] = MSG_IDLE_CS,
> -	};
> -	u32 val;
> -
> -	if (!_reg[engine->id].reg)
> -		return 0;
> -
> -	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
> -
> -	/* bits[29:25] & bits[13:9] >> shift */
> -	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
> -}
> -
> -static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
> -{
> -	int ret;
> -
> -	/* Ensure GPM receives fw up/down after CS is stopped */
> -	udelay(1);
> -
> -	/* Wait for forcewake request to complete in GPM */
> -	ret =  __intel_wait_for_register_fw(gt->uncore,
> -					    GEN9_PWRGT_DOMAIN_STATUS,
> -					    fw_mask, fw_mask, 5000, 0, NULL);
> -
> -	/* Ensure CS receives fw ack from GPM */
> -	udelay(1);
> -
> -	if (ret)
> -		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
> -}
> -
> -/*
> - * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
> - * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
> - * pending status is indicated by bits[13:9] (masked by bits[ 29:25]) in the
> - * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
> - * are concerned only with the gt reset here, we use a logical OR of pending
> - * forcewakeups from all reset domains and then wait for them to complete by
> - * querying PWRGT_DOMAIN_STATUS.
> - */
>   static void guc_engine_reset_prepare(struct intel_engine_cs *engine)
>   {
> -	u32 fw_pending;
> -
> -	if (GRAPHICS_VER(engine->i915) != 12)
> +	if (!IS_GRAPHICS_VER(engine->i915, 11, 12))
>   		return;
>   
> -	/*
> -	 * Wa_22011802037
> -	 * TODO: Occasionally trying to stop the cs times out, but does not
> -	 * adversely affect functionality. The timeout is set as a config
> -	 * parameter that defaults to 100ms. Assuming that this timeout is
> -	 * sufficient for any pending MI_FORCEWAKEs to complete, ignore the
> -	 * timeout returned here until it is root caused.
> -	 */
>   	intel_engine_stop_cs(engine);
>   
> -	fw_pending = __cs_pending_mi_force_wakes(engine);
> -	if (fw_pending)
> -		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	intel_engine_wait_for_pending_mi_fw(engine);
>   }
>   
>   static void guc_reset_nop(struct intel_engine_cs *engine)
Ceraolo Spurio, Daniele June 9, 2022, 11:51 p.m. UTC | #2
On 5/10/2022 3:14 PM, Nerlige Ramappa, Umesh wrote:
> From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>
> For execlists backend, current implementation of Wa_22011802037 is to
> stop the CS before doing a reset of the engine. This WA was further
> extended to wait for any pending MI FORCE WAKEUPs before issuing a
> reset. Add the extended steps in the execlist path of reset.
>
> In addition, extend the WA to gen11.
>
> v2: (Tvrtko)
> - Clarify comments, commit message, fix typos
> - Use IS_GRAPHICS_VER for gen 11/12 checks
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Fixes: f6aa0d713c88 ("drm/i915: Add Wa_22011802037 force cs halt")
> ---
>   drivers/gpu/drm/i915/gt/intel_engine.h        |  2 +
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 85 ++++++++++++++++++-
>   .../drm/i915/gt/intel_execlists_submission.c  |  7 ++
>   .../gpu/drm/i915/gt/intel_ring_submission.c   |  7 ++
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  4 +-
>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 81 ++----------------
>   6 files changed, 107 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 1431f1e9dbee..04e435bce79b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -201,6 +201,8 @@ int intel_ring_submission_setup(struct intel_engine_cs *engine);
>   int intel_engine_stop_cs(struct intel_engine_cs *engine);
>   void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine);
>   
> +void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine);
> +
>   void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask);
>   
>   u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 14c6ddbbfde8..9943cf9655b2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1282,10 +1282,10 @@ static int __intel_engine_stop_cs(struct intel_engine_cs *engine,
>   	intel_uncore_write_fw(uncore, mode, _MASKED_BIT_ENABLE(STOP_RING));
>   
>   	/*
> -	 * Wa_22011802037 : gen12, Prior to doing a reset, ensure CS is
> +	 * Wa_22011802037 : gen11, gen12, Prior to doing a reset, ensure CS is
>   	 * stopped, set ring stop bit and prefetch disable bit to halt CS
>   	 */
> -	if (GRAPHICS_VER(engine->i915) == 12)
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
>   		intel_uncore_write_fw(uncore, RING_MODE_GEN7(engine->mmio_base),
>   				      _MASKED_BIT_ENABLE(GEN12_GFX_PREFETCH_DISABLE));
>   
> @@ -1308,6 +1308,18 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine)
>   		return -ENODEV;
>   
>   	ENGINE_TRACE(engine, "\n");
> +	/*
> +	 * TODO: Find out why occasionally stopping the CS times out. Seen
> +	 * especially with gem_eio tests.
> +	 *
> +	 * Occasionally trying to stop the cs times out, but does not adversely
> +	 * affect functionality. The timeout is set as a config parameter that
> +	 * defaults to 100ms. In most cases the follow up operation is to wait
> +	 * for pending MI_FORCE_WAKES. The assumption is that this timeout is
> +	 * sufficient for any pending MI_FORCEWAKEs to complete. Once root
> +	 * caused, the caller must check and handle the return from this
> +	 * function.
> +	 */
>   	if (__intel_engine_stop_cs(engine, 1000, stop_timeout(engine))) {
>   		ENGINE_TRACE(engine,
>   			     "timed out on STOP_RING -> IDLE; HEAD:%04x, TAIL:%04x\n",
> @@ -1334,6 +1346,75 @@ void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
>   	ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
>   }
>   
> +static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
> +{
> +	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
> +		[RCS0] = MSG_IDLE_CS,
> +		[BCS0] = MSG_IDLE_BCS,
> +		[VCS0] = MSG_IDLE_VCS0,
> +		[VCS1] = MSG_IDLE_VCS1,
> +		[VCS2] = MSG_IDLE_VCS2,
> +		[VCS3] = MSG_IDLE_VCS3,
> +		[VCS4] = MSG_IDLE_VCS4,
> +		[VCS5] = MSG_IDLE_VCS5,
> +		[VCS6] = MSG_IDLE_VCS6,
> +		[VCS7] = MSG_IDLE_VCS7,
> +		[VECS0] = MSG_IDLE_VECS0,
> +		[VECS1] = MSG_IDLE_VECS1,
> +		[VECS2] = MSG_IDLE_VECS2,
> +		[VECS3] = MSG_IDLE_VECS3,
> +		[CCS0] = MSG_IDLE_CS,
> +		[CCS1] = MSG_IDLE_CS,
> +		[CCS2] = MSG_IDLE_CS,
> +		[CCS3] = MSG_IDLE_CS,
> +	};
> +	u32 val;
> +
> +	if (!_reg[engine->id].reg)
> +		return 0;
> +
> +	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
> +
> +	/* bits[29:25] & bits[13:9] >> shift */
> +	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
> +}
> +
> +static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
> +{
> +	int ret;
> +
> +	/* Ensure GPM receives fw up/down after CS is stopped */
> +	udelay(1);
> +
> +	/* Wait for forcewake request to complete in GPM */
> +	ret =  __intel_wait_for_register_fw(gt->uncore,
> +					    GEN9_PWRGT_DOMAIN_STATUS,
> +					    fw_mask, fw_mask, 5000, 0, NULL);
> +
> +	/* Ensure CS receives fw ack from GPM */
> +	udelay(1);
> +
> +	if (ret)
> +		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
> +}
> +
> +/*
> + * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
> + * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
> + * pending status is indicated by bits[13:9] (masked by bits[29:25]) in the
> + * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
> + * are concerned only with the gt reset here, we use a logical OR of pending
> + * forcewakeups from all reset domains and then wait for them to complete by
> + * querying PWRGT_DOMAIN_STATUS.
> + */
> +void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine)
> +{
> +	u32 fw_pending = __cs_pending_mi_force_wakes(engine);
> +
> +	if (fw_pending)
> +		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
> +}
> +
>   static u32
>   read_subslice_reg(const struct intel_engine_cs *engine,
>   		  int slice, int subslice, i915_reg_t reg)
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 86f7a9ac1c39..2caa1af77064 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -2958,6 +2958,13 @@ static void execlists_reset_prepare(struct intel_engine_cs *engine)
>   	ring_set_paused(engine, 1);
>   	intel_engine_stop_cs(engine);
>   
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
> +		intel_engine_wait_for_pending_mi_fw(engine);
> +
>   	engine->execlists.reset_ccid = active_ccid(engine);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index 5423bfd301ad..a7808eff33c5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -323,6 +323,13 @@ static void reset_prepare(struct intel_engine_cs *engine)
>   	ENGINE_TRACE(engine, "\n");
>   	intel_engine_stop_cs(engine);
>   
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
> +		intel_engine_wait_for_pending_mi_fw(engine);
> +

Ringbuffer submission is not supported on gen 11 and 12, so no need for 
this.
With this removed:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

>   	if (!stop_ring(engine)) {
>   		/* G45 ring initialization often fails to reset head to zero */
>   		ENGINE_TRACE(engine,
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 2c4ad4a65089..8c6885f43d1a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -310,8 +310,8 @@ static u32 guc_ctl_wa_flags(struct intel_guc *guc)
>   	if (IS_DG2(gt->i915))
>   		flags |= GUC_WA_DUAL_QUEUE;
>   
> -	/* Wa_22011802037: graphics version 12 */
> -	if (GRAPHICS_VER(gt->i915) == 12)
> +	/* Wa_22011802037: graphics version 11/12 */
> +	if (IS_GRAPHICS_VER(gt->i915, 11, 12))
>   		flags |= GUC_WA_PRE_PARSER;
>   
>   	/* Wa_16011777198:dg2 */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 75291e9846c5..9b21c7345ffd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1527,87 +1527,18 @@ static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub)
>   	lrc_update_regs(ce, engine, head);
>   }
>   
> -static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
> -{
> -	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
> -		[RCS0] = MSG_IDLE_CS,
> -		[BCS0] = MSG_IDLE_BCS,
> -		[VCS0] = MSG_IDLE_VCS0,
> -		[VCS1] = MSG_IDLE_VCS1,
> -		[VCS2] = MSG_IDLE_VCS2,
> -		[VCS3] = MSG_IDLE_VCS3,
> -		[VCS4] = MSG_IDLE_VCS4,
> -		[VCS5] = MSG_IDLE_VCS5,
> -		[VCS6] = MSG_IDLE_VCS6,
> -		[VCS7] = MSG_IDLE_VCS7,
> -		[VECS0] = MSG_IDLE_VECS0,
> -		[VECS1] = MSG_IDLE_VECS1,
> -		[VECS2] = MSG_IDLE_VECS2,
> -		[VECS3] = MSG_IDLE_VECS3,
> -		[CCS0] = MSG_IDLE_CS,
> -		[CCS1] = MSG_IDLE_CS,
> -		[CCS2] = MSG_IDLE_CS,
> -		[CCS3] = MSG_IDLE_CS,
> -	};
> -	u32 val;
> -
> -	if (!_reg[engine->id].reg)
> -		return 0;
> -
> -	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
> -
> -	/* bits[29:25] & bits[13:9] >> shift */
> -	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
> -}
> -
> -static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
> -{
> -	int ret;
> -
> -	/* Ensure GPM receives fw up/down after CS is stopped */
> -	udelay(1);
> -
> -	/* Wait for forcewake request to complete in GPM */
> -	ret =  __intel_wait_for_register_fw(gt->uncore,
> -					    GEN9_PWRGT_DOMAIN_STATUS,
> -					    fw_mask, fw_mask, 5000, 0, NULL);
> -
> -	/* Ensure CS receives fw ack from GPM */
> -	udelay(1);
> -
> -	if (ret)
> -		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
> -}
> -
> -/*
> - * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
> - * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
> - * pending status is indicated by bits[13:9] (masked by bits[ 29:25]) in the
> - * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
> - * are concerned only with the gt reset here, we use a logical OR of pending
> - * forcewakeups from all reset domains and then wait for them to complete by
> - * querying PWRGT_DOMAIN_STATUS.
> - */
>   static void guc_engine_reset_prepare(struct intel_engine_cs *engine)
>   {
> -	u32 fw_pending;
> -
> -	if (GRAPHICS_VER(engine->i915) != 12)
> +	if (!IS_GRAPHICS_VER(engine->i915, 11, 12))
>   		return;
>   
> -	/*
> -	 * Wa_22011802037
> -	 * TODO: Occasionally trying to stop the cs times out, but does not
> -	 * adversely affect functionality. The timeout is set as a config
> -	 * parameter that defaults to 100ms. Assuming that this timeout is
> -	 * sufficient for any pending MI_FORCEWAKEs to complete, ignore the
> -	 * timeout returned here until it is root caused.
> -	 */
>   	intel_engine_stop_cs(engine);
>   
> -	fw_pending = __cs_pending_mi_force_wakes(engine);
> -	if (fw_pending)
> -		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
> +	/*
> +	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
> +	 * to wait for any pending mi force wakeups
> +	 */
> +	intel_engine_wait_for_pending_mi_fw(engine);
>   }
>   
>   static void guc_reset_nop(struct intel_engine_cs *engine)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 1431f1e9dbee..04e435bce79b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -201,6 +201,8 @@  int intel_ring_submission_setup(struct intel_engine_cs *engine);
 int intel_engine_stop_cs(struct intel_engine_cs *engine);
 void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine);
 
+void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine);
+
 void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask);
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 14c6ddbbfde8..9943cf9655b2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1282,10 +1282,10 @@  static int __intel_engine_stop_cs(struct intel_engine_cs *engine,
 	intel_uncore_write_fw(uncore, mode, _MASKED_BIT_ENABLE(STOP_RING));
 
 	/*
-	 * Wa_22011802037 : gen12, Prior to doing a reset, ensure CS is
+	 * Wa_22011802037 : gen11, gen12, Prior to doing a reset, ensure CS is
 	 * stopped, set ring stop bit and prefetch disable bit to halt CS
 	 */
-	if (GRAPHICS_VER(engine->i915) == 12)
+	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
 		intel_uncore_write_fw(uncore, RING_MODE_GEN7(engine->mmio_base),
 				      _MASKED_BIT_ENABLE(GEN12_GFX_PREFETCH_DISABLE));
 
@@ -1308,6 +1308,18 @@  int intel_engine_stop_cs(struct intel_engine_cs *engine)
 		return -ENODEV;
 
 	ENGINE_TRACE(engine, "\n");
+	/*
+	 * TODO: Find out why occasionally stopping the CS times out. Seen
+	 * especially with gem_eio tests.
+	 *
+	 * Occasionally trying to stop the cs times out, but does not adversely
+	 * affect functionality. The timeout is set as a config parameter that
+	 * defaults to 100ms. In most cases the follow up operation is to wait
+	 * for pending MI_FORCE_WAKES. The assumption is that this timeout is
+	 * sufficient for any pending MI_FORCEWAKEs to complete. Once root
+	 * caused, the caller must check and handle the return from this
+	 * function.
+	 */
 	if (__intel_engine_stop_cs(engine, 1000, stop_timeout(engine))) {
 		ENGINE_TRACE(engine,
 			     "timed out on STOP_RING -> IDLE; HEAD:%04x, TAIL:%04x\n",
@@ -1334,6 +1346,75 @@  void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine)
 	ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
 }
 
+static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
+{
+	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
+		[RCS0] = MSG_IDLE_CS,
+		[BCS0] = MSG_IDLE_BCS,
+		[VCS0] = MSG_IDLE_VCS0,
+		[VCS1] = MSG_IDLE_VCS1,
+		[VCS2] = MSG_IDLE_VCS2,
+		[VCS3] = MSG_IDLE_VCS3,
+		[VCS4] = MSG_IDLE_VCS4,
+		[VCS5] = MSG_IDLE_VCS5,
+		[VCS6] = MSG_IDLE_VCS6,
+		[VCS7] = MSG_IDLE_VCS7,
+		[VECS0] = MSG_IDLE_VECS0,
+		[VECS1] = MSG_IDLE_VECS1,
+		[VECS2] = MSG_IDLE_VECS2,
+		[VECS3] = MSG_IDLE_VECS3,
+		[CCS0] = MSG_IDLE_CS,
+		[CCS1] = MSG_IDLE_CS,
+		[CCS2] = MSG_IDLE_CS,
+		[CCS3] = MSG_IDLE_CS,
+	};
+	u32 val;
+
+	if (!_reg[engine->id].reg)
+		return 0;
+
+	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
+
+	/* bits[29:25] & bits[13:9] >> shift */
+	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
+}
+
+static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
+{
+	int ret;
+
+	/* Ensure GPM receives fw up/down after CS is stopped */
+	udelay(1);
+
+	/* Wait for forcewake request to complete in GPM */
+	ret =  __intel_wait_for_register_fw(gt->uncore,
+					    GEN9_PWRGT_DOMAIN_STATUS,
+					    fw_mask, fw_mask, 5000, 0, NULL);
+
+	/* Ensure CS receives fw ack from GPM */
+	udelay(1);
+
+	if (ret)
+		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
+}
+
+/*
+ * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
+ * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
+ * pending status is indicated by bits[13:9] (masked by bits[29:25]) in the
+ * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
+ * are concerned only with the gt reset here, we use a logical OR of pending
+ * forcewakeups from all reset domains and then wait for them to complete by
+ * querying PWRGT_DOMAIN_STATUS.
+ */
+void intel_engine_wait_for_pending_mi_fw(struct intel_engine_cs *engine)
+{
+	u32 fw_pending = __cs_pending_mi_force_wakes(engine);
+
+	if (fw_pending)
+		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
+}
+
 static u32
 read_subslice_reg(const struct intel_engine_cs *engine,
 		  int slice, int subslice, i915_reg_t reg)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 86f7a9ac1c39..2caa1af77064 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -2958,6 +2958,13 @@  static void execlists_reset_prepare(struct intel_engine_cs *engine)
 	ring_set_paused(engine, 1);
 	intel_engine_stop_cs(engine);
 
+	/*
+	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
+	 * to wait for any pending mi force wakeups
+	 */
+	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
+		intel_engine_wait_for_pending_mi_fw(engine);
+
 	engine->execlists.reset_ccid = active_ccid(engine);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
index 5423bfd301ad..a7808eff33c5 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
@@ -323,6 +323,13 @@  static void reset_prepare(struct intel_engine_cs *engine)
 	ENGINE_TRACE(engine, "\n");
 	intel_engine_stop_cs(engine);
 
+	/*
+	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
+	 * to wait for any pending mi force wakeups
+	 */
+	if (IS_GRAPHICS_VER(engine->i915, 11, 12))
+		intel_engine_wait_for_pending_mi_fw(engine);
+
 	if (!stop_ring(engine)) {
 		/* G45 ring initialization often fails to reset head to zero */
 		ENGINE_TRACE(engine,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 2c4ad4a65089..8c6885f43d1a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -310,8 +310,8 @@  static u32 guc_ctl_wa_flags(struct intel_guc *guc)
 	if (IS_DG2(gt->i915))
 		flags |= GUC_WA_DUAL_QUEUE;
 
-	/* Wa_22011802037: graphics version 12 */
-	if (GRAPHICS_VER(gt->i915) == 12)
+	/* Wa_22011802037: graphics version 11/12 */
+	if (IS_GRAPHICS_VER(gt->i915, 11, 12))
 		flags |= GUC_WA_PRE_PARSER;
 
 	/* Wa_16011777198:dg2 */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 75291e9846c5..9b21c7345ffd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1527,87 +1527,18 @@  static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub)
 	lrc_update_regs(ce, engine, head);
 }
 
-static u32 __cs_pending_mi_force_wakes(struct intel_engine_cs *engine)
-{
-	static const i915_reg_t _reg[I915_NUM_ENGINES] = {
-		[RCS0] = MSG_IDLE_CS,
-		[BCS0] = MSG_IDLE_BCS,
-		[VCS0] = MSG_IDLE_VCS0,
-		[VCS1] = MSG_IDLE_VCS1,
-		[VCS2] = MSG_IDLE_VCS2,
-		[VCS3] = MSG_IDLE_VCS3,
-		[VCS4] = MSG_IDLE_VCS4,
-		[VCS5] = MSG_IDLE_VCS5,
-		[VCS6] = MSG_IDLE_VCS6,
-		[VCS7] = MSG_IDLE_VCS7,
-		[VECS0] = MSG_IDLE_VECS0,
-		[VECS1] = MSG_IDLE_VECS1,
-		[VECS2] = MSG_IDLE_VECS2,
-		[VECS3] = MSG_IDLE_VECS3,
-		[CCS0] = MSG_IDLE_CS,
-		[CCS1] = MSG_IDLE_CS,
-		[CCS2] = MSG_IDLE_CS,
-		[CCS3] = MSG_IDLE_CS,
-	};
-	u32 val;
-
-	if (!_reg[engine->id].reg)
-		return 0;
-
-	val = intel_uncore_read(engine->uncore, _reg[engine->id]);
-
-	/* bits[29:25] & bits[13:9] >> shift */
-	return (val & (val >> 16) & MSG_IDLE_FW_MASK) >> MSG_IDLE_FW_SHIFT;
-}
-
-static void __gpm_wait_for_fw_complete(struct intel_gt *gt, u32 fw_mask)
-{
-	int ret;
-
-	/* Ensure GPM receives fw up/down after CS is stopped */
-	udelay(1);
-
-	/* Wait for forcewake request to complete in GPM */
-	ret =  __intel_wait_for_register_fw(gt->uncore,
-					    GEN9_PWRGT_DOMAIN_STATUS,
-					    fw_mask, fw_mask, 5000, 0, NULL);
-
-	/* Ensure CS receives fw ack from GPM */
-	udelay(1);
-
-	if (ret)
-		GT_TRACE(gt, "Failed to complete pending forcewake %d\n", ret);
-}
-
-/*
- * Wa_22011802037:gen12: In addition to stopping the cs, we need to wait for any
- * pending MI_FORCE_WAKEUP requests that the CS has initiated to complete. The
- * pending status is indicated by bits[13:9] (masked by bits[ 29:25]) in the
- * MSG_IDLE register. There's one MSG_IDLE register per reset domain. Since we
- * are concerned only with the gt reset here, we use a logical OR of pending
- * forcewakeups from all reset domains and then wait for them to complete by
- * querying PWRGT_DOMAIN_STATUS.
- */
 static void guc_engine_reset_prepare(struct intel_engine_cs *engine)
 {
-	u32 fw_pending;
-
-	if (GRAPHICS_VER(engine->i915) != 12)
+	if (!IS_GRAPHICS_VER(engine->i915, 11, 12))
 		return;
 
-	/*
-	 * Wa_22011802037
-	 * TODO: Occasionally trying to stop the cs times out, but does not
-	 * adversely affect functionality. The timeout is set as a config
-	 * parameter that defaults to 100ms. Assuming that this timeout is
-	 * sufficient for any pending MI_FORCEWAKEs to complete, ignore the
-	 * timeout returned here until it is root caused.
-	 */
 	intel_engine_stop_cs(engine);
 
-	fw_pending = __cs_pending_mi_force_wakes(engine);
-	if (fw_pending)
-		__gpm_wait_for_fw_complete(engine->gt, fw_pending);
+	/*
+	 * Wa_22011802037:gen11/gen12: In addition to stopping the cs, we need
+	 * to wait for any pending mi force wakeups
+	 */
+	intel_engine_wait_for_pending_mi_fw(engine);
 }
 
 static void guc_reset_nop(struct intel_engine_cs *engine)