On 4/18/2024 10:10, Nirmoy Das wrote:
Currently intel_gt_reset() happens as follows:
reset_prepare() ---> Sends GDRST to GuC, GuC is in GS_MIA_IN_RESET
do_reset()
intel_gt_reset_all_engines()
*_engine_reset_prepare() -->RESET_CTL expects running GuC
Not technically correct. There is no direct connection between RESET_CTL
and GuC.
*_reset_engines()
intel_gt_init_hw() --> GuC comes out of GS_MIA_IN_RESET with FW loaded.
Fix the issue by sanitizing the GuC only after resetting requested
engines and before intel_gt_init_hw().
You never actually state what the issue is.
The problem is that there is a dedicated CSB FIFO going to GuC (and
nothing else has access to it). If that FIFO fills up, the hardware will
block on the next context switch until there is space. If no-one (i.e.
GuC) is draining it, that means the system is effectively hung. If an
engine is reset whilst actively executing a context, a CSB entry will be
sent to say that the context has gone idle. Thus if you reset a very
busy system and start with killing GuC before killing the engines and
only then re-enabling GuC, you run the risk of generating more CSB
entries than will fit in the FIFO and deadlocking. Whereas, if the
system is idle then you can reset the engines as much as you like while
GuC is dead and it won't be a problem.
Note intel_uc_reset_finish() and intel_uc_reset() are nop when
guc submission is disabled.
Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 6504e8ba9c58..bd166f5aca4b 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -907,8 +907,17 @@ static intel_engine_mask_t reset_prepare(struct intel_gt *gt)
intel_engine_mask_t awake = 0;
enum intel_engine_id id;
- /* For GuC mode, ensure submission is disabled before stopping ring */
- intel_uc_reset_prepare(>->uc);
+ /**
+ * For GuC mode with submission enabled, ensure submission
+ * is disabled before stopping ring.
+ *
+ * For GuC mode with submission disabled, ensure that GuC is not
+ * sanitized, do that at the end in reset_finish(). reset_prepare()
+ * is followed by engine reset which in this mode requires GuC to
+ * be functional to process engine reset events.
-> to process any CSB FIFO entries generated by the resets.
John.
+ */
+ if (intel_uc_uses_guc_submission(>->uc))
+ intel_uc_reset_prepare(>->uc);
for_each_engine(engine, gt, id) {
if (intel_engine_pm_get_if_awake(engine))
@@ -1255,6 +1264,9 @@ void intel_gt_reset(struct intel_gt *gt,
intel_overlay_reset(gt->i915);
+ /* sanitize uC after engine reset */
+ if (!intel_uc_uses_guc_submission(>->uc))
+ intel_uc_reset_prepare(>->uc);
/*
* Next we need to restore the context, but we don't use those
* yet either...
