[PATCH v3] ALSA: hda: Reset response counter when a verb times out

[songxiebing]

From: Bob Song <songxiebing@xxxxxxxxxx>

On some controllers the HDA link reports a verb timeout and the driver
falls back to polling mode.  After that, every subsequent verb sent to
that codec address keeps timing out (roughly one second per verb), while
the hardware itself looks perfectly healthy: the RIRB write pointer keeps
advancing, interrupts are still delivered, and verbs that carry no reply
payload -- notably writes -- still take effect on the codec.

The reason is that bus->rirb.cmds[addr] is incremented for every verb and
only decremented in snd_hdac_bus_update_rirb() when a matching RIRB
response is read, without any rollback for a response that never arrives.
A controller that cannot fall back to the single-command mode
(chip->fallback_to_single_cmd == 0, e.g. the ACPI platform, Tegra and CIX
controllers) returns an error right away and never performs the bus reset
that would reinitialize CORB/RIRB and clear the counter, so a single lost
response leaves bus->rirb.cmds[addr] non-zero forever.  Each following
verb then still gets one response that only brings the count back to the
stale baseline of 1, and snd_hdac_bus_get_response() keeps timing out even
though the codec answers normally.

Handle it in the core helper snd_hdac_bus_exec_verb_unlocked(): whenever
get_response() reports an error, reset the pending counter for the codec
address, so a late response is simply treated as a spurious response and
the following verbs recover.  Doing it here covers all HD-audio
controllers instead of relying on each controller's fallback setting.

Add a small helper snd_hdac_bus_reset_response_counter() for this.

Signed-off-by: Bob Song <songxiebing@xxxxxxxxxx>
---
Changes in v3:
- Drop the unneeded EXPORT_SYMBOL_GPL() and move the declaration from
  include/sound/hdaudio.h into the internal core/local.h, since the helper
  is used only within the hda-core module.

Changes in v2:
- Move the fix from azx_rirb_get_response() in the HDA controller helper
  into the HD-audio core (snd_hdac_bus_exec_verb_unlocked()), as suggested
  by Takashi Iwai, so that it covers all HD-audio controllers and not only
  the ones using the shared azx helper.
- Add the snd_hdac_bus_reset_response_counter() helper instead of
  open-coding the counter reset.

 sound/hda/core/bus.c        |  6 +++++-
 sound/hda/core/controller.c | 19 +++++++++++++++++++
 sound/hda/core/local.h      |  2 ++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
index 81498f1e413e..ecc4b7a780a4 100644
--- a/sound/hda/core/bus.c
+++ b/sound/hda/core/bus.c
@@ -121,11 +121,15 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr,
 			break;
 		/* process pending verbs */
 		err = bus->ops->get_response(bus, addr, &tmp);
-		if (err)
+		if (err) {
+			snd_hdac_bus_reset_response_counter(bus, addr);
 			break;
+		}
 	}
 	if (!err && res) {
 		err = bus->ops->get_response(bus, addr, res);
+		if (err)
+			snd_hdac_bus_reset_response_counter(bus, addr);
 		trace_hda_get_response(bus, addr, *res);
 	}
 	return err;
diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
index 78855ac357c6..9993f5df9ee5 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -396,6 +396,25 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_get_response);
 
+/**
+ * snd_hdac_bus_reset_response_counter - reset the pending response count
+ * @bus: HD-audio core bus
+ * @addr: codec address
+ *
+ * Drop the number of commands that are still waiting for a response from
+ * the given codec address.  This is called when a verb is known to be dead,
+ * e.g. after a response timeout, so that a lost response won't leave the
+ * counter non-zero forever and block all later verbs to the same codec.
+ */
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus, unsigned int addr)
+{
+	if (addr >= HDA_MAX_CODECS)
+		return;
+
+	guard(spinlock_irq)(&bus->reg_lock);
+	bus->rirb.cmds[addr] = 0;
+}
+
 #define HDAC_MAX_CAPS 10
 /**
  * snd_hdac_bus_parse_capabilities - parse capability structure
diff --git a/sound/hda/core/local.h b/sound/hda/core/local.h
index 5f03b203c416..b9e58395decb 100644
--- a/sound/hda/core/local.h
+++ b/sound/hda/core/local.h
@@ -16,6 +16,8 @@ int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec);
 void snd_hdac_bus_remove_device(struct hdac_bus *bus,
 				struct hdac_device *codec);
 void snd_hdac_bus_queue_event(struct hdac_bus *bus, u32 res, u32 res_ex);
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus,
+					 unsigned int addr);
 int snd_hdac_bus_exec_verb(struct hdac_bus *bus, unsigned int addr,
 			   unsigned int cmd, unsigned int *res);
 
-- 
2.25.1