[RFC PATCH v2 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

[Denis Pisarev <pisarevden@gmail.com>]

After resume from S4 (hibernation) on gmc_v9 parts with GFXOFF
(observed on Cezanne / Ryzen 7 PRO 5850U, kernel 7.1.8), KIQ-based TLB
flushes start failing at the moment of the thaw and keep failing for
hours of normal desktop use:

  amdgpu 0000:07:00.0: failed to write reg 28b4 wait reg 28c6
  amdgpu 0000:07:00.0: failed to write reg 1a6f4 wait reg 1a706

(80-140 errors/hour measured over 9+ hours; bugzilla 219492). The KIQ
ring stays sched.ready throughout, so readiness does not reflect the
state of the hardware in this failure mode.

Two problems follow from the current code: every failed flush burns
the full ~5 s KIQ retry window before erroring out (desktop-wide
sluggishness), and the invalidation is then silently dropped (stale
TLBs - correctness).

Make the failure observable and self-healing:

- amdgpu_gmc_fw_reg_write_reg_wait() returns 0/-ETIME, propagates MES
  errors, and counts consecutive failures per KIQ instance

- gmc_v9_0_flush_gpu_tlb() falls back to a new
  gmc_v9_0_flush_gpu_tlb_mmio() helper (the former pre-KIQ MMIO code,
  now with irqsave locking since it is reachable at runtime) when the
  KIQ submit fails, so the invalidation is no longer dropped

- after AMDGPU_KIQ_FLUSH_MAX_FAIL (3) consecutive failures the KIQ
  path is skipped entirely until the counter resets, so wedged systems
  stop paying the 5 s retry window per flush

- the MMIO fallback is restricted to process context on bare metal:
  amdgpu_gfx_off_ctrl() is used to hold the GC block awake across the
  direct register access (it may sleep), and SR-IOV VFs keep
  proxying through KIQ as before

- the counter resets on every success and in gmc_v9_0_hw_fini(); every
  suspend/resume cycle re-arms the KIQ path; nothing is disabled
  proactively

Changes since v1 (addressing the sashiko-bot review):
- hold GFXOFF off around runtime MMIO access (was: unguarded - could
  hit power-gated registers)
- per-KIQ-instance failure counter instead of a global one (multi-XCC
  cross-talk)
- invalidate_lock is now taken irqsave (the path is runtime-reachable)
- VFs and interrupt context never take the runtime MMIO fallback
- MES path errors are propagated instead of hardcoded success
- reworded the threshold warning to not promise a fallback that
  gmc_v10/v11/v12 do not implement

Known limitation / open question: a KIQ command that already timed out
stays queued in the ring; if the ring recovers late it could emit a
duplicate invalidation concurrently with a CPU MMIO flush. The command
is semantically an idempotent invalidate, but reviewer input on
whether the req/ack handshake needs protection here is welcome.

gmc_v10/v11/v12 call sites are unchanged and compile-safe (int return
used as statement). They can get the same fallback once the approach
is agreed for gmc_v9.

The sibling PASID path (amdgpu_gmc_flush_gpu_tlb_pasid) already has an
-ETIME/MMIO split; this brings the per-VMID path in line with it.

Root-cause note: with GFXOFF held off across the S4 cycle (debugfs
amdgpu_gfxoff), zero errors occur across resume and 30 min of use vs
~70-140 in the control arm; the wedge forms in the S4 resume window
while GFXOFF is allowed, consistent with the existing semaphore
workaround comment in gmc_v9_0.c.

Signed-off-by: Denis Pisarev <pisarevden@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  18 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |   2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 111 ++++++++++++++++++------
 5 files changed, 102 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410d6..cd5d9e56e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -360,6 +360,8 @@ enum amdgpu_kiq_irq {
 #define MAX_KIQ_REG_WAIT       5000 /* in usecs, 5ms */
 #define MAX_KIQ_REG_BAILOUT_INTERVAL   5 /* in msecs, 5ms */
 #define MAX_KIQ_REG_TRY 1000
+/* consecutive KIQ TLB flush failures before falling back to MMIO */
+#define AMDGPU_KIQ_FLUSH_MAX_FAIL 3
 
 /*
  * BIOS.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 54c1eb9c4..e2aceb99c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -167,6 +167,8 @@ struct amdgpu_kiq {
 	struct amdgpu_irq_src	irq;
 	const struct kiq_pm4_funcs *pmf;
 	void			*mqd_backup;
+	/* consecutive TLB flush reg access failures on this instance */
+	atomic_t		flush_failures;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 5d6149ba7..49d3d6651 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -874,7 +874,7 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 	return r;
 }
 
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst)
@@ -886,9 +886,8 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	uint32_t seq;
 
 	if (adev->mes.ring[MES_PIPE_INST(xcc_inst, 0)].sched.ready) {
-		amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
-					      ref, mask, xcc_inst);
-		return;
+		return amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
+						     ref, mask, xcc_inst);
 	}
 
 	spin_lock_irqsave(&kiq->ring_lock, flags);
@@ -919,13 +918,20 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	if (cnt > MAX_KIQ_REG_TRY)
 		goto failed_kiq;
 
-	return;
+	atomic_set(&adev->gfx.kiq[xcc_inst].flush_failures, 0);
+	return 0;
 
 failed_undo:
 	amdgpu_ring_undo(ring);
 	spin_unlock_irqrestore(&kiq->ring_lock, flags);
 failed_kiq:
-	dev_err(adev->dev, "failed to write reg %x wait reg %x\n", reg0, reg1);
+	if (atomic_inc_return(&adev->gfx.kiq[xcc_inst].flush_failures) ==
+			AMDGPU_KIQ_FLUSH_MAX_FAIL)
+		dev_warn(adev->dev,
+			 "KIQ reg access keeps failing, MMIO fallback recommended\n");
+	dev_err_ratelimited(adev->dev,
+			    "failed to write reg %x wait reg %x\n", reg0, reg1);
+	return -ETIME;
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index ddb0d500e..303e0ee98 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -447,7 +447,7 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 				   uint32_t flush_type, bool all_hub,
 				   uint32_t inst);
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 8a5c44810..11fc9085e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -764,27 +764,28 @@ static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct amdgpu_device *adev,
  */
 
 /**
- * gmc_v9_0_flush_gpu_tlb - tlb flush with certain type
+ * gmc_v9_0_flush_gpu_tlb_mmio - tlb flush via direct MMIO
  *
  * @adev: amdgpu_device pointer
+ * @hub: vmhub to flush
  * @vmid: vm instance to flush
  * @vmhub: which hub to flush
- * @flush_type: the flush type
+ * @inv_req: invalidation request payload
  *
- * Flush the TLB for the requested page table using certain type.
+ * Direct CPU access to the invalidation engine. Callers must ensure
+ * the target block cannot power gate across the access (GFXOFF needs
+ * to be held off at runtime) and must hold no other locks.
  */
-static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
-					uint32_t vmhub, uint32_t flush_type)
+static void gmc_v9_0_flush_gpu_tlb_mmio(struct amdgpu_device *adev,
+					struct amdgpu_vmhub *hub,
+					uint32_t vmid, uint32_t vmhub,
+					u32 inv_req)
 {
 	bool use_semaphore = gmc_v9_0_use_invalidate_semaphore(adev, vmhub);
-	u32 j, inv_req, tmp, sem, req, ack, inst;
 	const unsigned int eng = 17;
-	struct amdgpu_vmhub *hub;
-
-	BUG_ON(vmhub >= AMDGPU_MAX_VMHUBS);
+	unsigned long flags;
+	u32 j, tmp, sem, req, ack, inst;
 
-	hub = &adev->vmhub[vmhub];
-	inv_req = gmc_v9_0_get_invalidate_req(vmid, flush_type);
 	sem = hub->vm_inv_eng0_sem + hub->eng_distance * eng;
 	req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
 	ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
@@ -794,21 +795,8 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	else
 		inst = vmhub;
 
-	/* This is necessary for SRIOV as well as for GFXOFF to function
-	 * properly under bare metal
-	 */
-	if (adev->gfx.kiq[inst].ring.sched.ready &&
-	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
-		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
-		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
-
-		amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
-						 1 << vmid, inst);
-		return;
-	}
-
 	/* This path is needed before KIQ/MES/GFXOFF are set up */
-	spin_lock(&adev->gmc.invalidate_lock);
+	spin_lock_irqsave(&adev->gmc.invalidate_lock, flags);
 
 	/*
 	 * It may lose gpuvm invalidate acknowldege state across power-gating
@@ -870,7 +858,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 			WREG32_SOC15_IP_NO_KIQ(GC, sem, 0, GET_INST(GC, inst));
 	}
 
-	spin_unlock(&adev->gmc.invalidate_lock);
+	spin_unlock_irqrestore(&adev->gmc.invalidate_lock, flags);
 
 	if (j < adev->usec_timeout)
 		return;
@@ -878,6 +866,70 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	DRM_ERROR("Timeout waiting for VM flush ACK!\n");
 }
 
+/**
+ * gmc_v9_0_flush_gpu_tlb - tlb flush with certain type
+ *
+ * @adev: amdgpu_device pointer
+ * @vmid: vm instance to flush
+ * @vmhub: which hub to flush
+ * @flush_type: the flush type
+ *
+ * Flush the TLB for the requested page table using certain type.
+ */
+static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
+					uint32_t vmhub, uint32_t flush_type)
+{
+	u32 inv_req;
+	const unsigned int eng = 17;
+	struct amdgpu_vmhub *hub;
+	u32 inst;
+
+	BUG_ON(vmhub >= AMDGPU_MAX_VMHUBS);
+
+	hub = &adev->vmhub[vmhub];
+	inv_req = gmc_v9_0_get_invalidate_req(vmid, flush_type);
+
+	if (vmhub >= AMDGPU_MMHUB0(0))
+		inst = 0;
+	else
+		inst = vmhub;
+
+	/* This is necessary for SRIOV as well as for GFXOFF to function
+	 * properly under bare metal
+	 */
+	if (adev->gfx.kiq[inst].ring.sched.ready &&
+	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
+		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
+		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
+
+		if (atomic_read(&adev->gfx.kiq[inst].flush_failures) <
+				AMDGPU_KIQ_FLUSH_MAX_FAIL) {
+			if (!amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack,
+							      inv_req,
+							      1 << vmid, inst))
+				return;
+			/* KIQ submit failed; error already logged */
+		}
+
+		/*
+		 * MMIO fallback: invalidation must not be silently dropped
+		 * when KIQ is unresponsive. Direct register access is only
+		 * safe in process context (amdgpu_gfx_off_ctrl may sleep)
+		 * and on bare metal; SR-IOV VFs must keep proxying through
+		 * KIQ.
+		 */
+		if (in_interrupt() || amdgpu_sriov_vf(adev))
+			return;
+
+		amdgpu_gfx_off_ctrl(adev, false);
+		gmc_v9_0_flush_gpu_tlb_mmio(adev, hub, vmid, vmhub, inv_req);
+		amdgpu_gfx_off_ctrl(adev, true);
+		return;
+	}
+
+	gmc_v9_0_flush_gpu_tlb_mmio(adev, hub, vmid, vmhub, inv_req);
+}
+
 /**
  * gmc_v9_0_flush_gpu_tlb_pasid - tlb flush via pasid
  *
@@ -2237,6 +2289,13 @@ static void gmc_v9_0_gart_disable(struct amdgpu_device *adev)
 static int gmc_v9_0_hw_fini(struct amdgpu_ip_block *ip_block)
 {
 	struct amdgpu_device *adev = ip_block->adev;
+	int i;
+
+	/* KIQ instances are re-initialized on the next resume; re-arm
+	 * the MMIO fallback logic
+	 */
+	for (i = 0; i < AMDGPU_MAX_GC_INSTANCES; i++)
+		atomic_set(&adev->gfx.kiq[i].flush_failures, 0);
 
 	gmc_v9_0_gart_disable(adev);
 
-- 
2.55.0