[PATCH v2] 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 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.

 include/sound/hdaudio.h     |  2 ++
 sound/hda/core/bus.c        |  6 +++++-
 sound/hda/core/controller.c | 20 ++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index aa994d6e6d35..b0fbb61fa093 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -398,6 +398,8 @@ void snd_hdac_codec_link_down(struct hdac_device *codec);
 int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val);
 int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 			      unsigned int *res);
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus,
+					 unsigned int addr);
 int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus);
 
 bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset);
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..1babc5109049 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -396,6 +396,26 @@ 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;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_response_counter);
+
 #define HDAC_MAX_CAPS 10
 /**
  * snd_hdac_bus_parse_capabilities - parse capability structure
-- 
2.25.1