[alsa-devel] [PATCH 0/1] ALSA: hda: add fallback to polling to hdac_bus_get_response()
Hey all,
while debugging issues with some Intel platforms related to display audio codec probe (see https://lists.freedesktop.org/archives/intel-gfx/2019-October/214621.html ), I found a discrepancy in behaviour between snd-hda-intel and SOF, despite using the same snd-hda-codec-hdmi as the codec driver.
The specific problem I was debugging appears in a stress test (designed to uncover the above display driver issue) where driver-unload, s3-suspend, resume and driver-reload is done in a loop and repeated for hundreds of iterations. When using SOF, I would get occasional probe fail due to a missing HDA irq. The AZX snd_hda_intel driver nicely survives this test. The explanation seems to be differences in the hdac get_response() implementation.
While the specific issue could be solved with other means, the git history shows a number of rare issues with HDA codecs where polling has helped. It would seem best to align the logic with the AZX driver implementation that has seen much more usage over the years. This will benefit SOF and any other users of the HDAC library.
Kai Vehmanen (1): ALSA: hda: add fallback to polling to hdac_bus_get_response()
sound/hda/hdac_controller.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
The AZX controller implementation in azx_rirb_get_response() implements logic to fallback to polling in case interrupt is not received from HDA codec.
Port over this same logic to the generic snd_hdac_bus_get_response() function, which is used by other HDAC clients such as SOF.
Without this fix, failures are observed in module reload stress tests with the SOF driver, while test passes on same hardware with the snd_hda_intel driver. Considering the AZX implementation has been much more widely used and there can be exceptions with other systems (and codecs), it is best to align the implementation and use the time-proven logic in all drivers.
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/hda/hdac_controller.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index d3999e7b0705..994c1dd2eb2e 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -238,14 +238,18 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr, { unsigned long timeout; unsigned long loopcounter; + int do_poll = 0;
+ again: timeout = jiffies + msecs_to_jiffies(1000);
for (loopcounter = 0;; loopcounter++) { spin_lock_irq(&bus->reg_lock); - if (bus->polling_mode) + if (bus->polling_mode || do_poll) snd_hdac_bus_update_rirb(bus); if (!bus->rirb.cmds[addr]) { + if (!do_poll) + bus->poll_count = 0; if (res) *res = bus->rirb.res[addr]; /* the last value */ spin_unlock_irq(&bus->reg_lock); @@ -262,6 +266,23 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr, } }
+ if (!bus->polling_mode && bus->poll_count < 2) { + dev_dbg(bus->dev, + "response timeout, polling the codec once: last cmd=0x%08x\n", + bus->last_cmd[addr]); + do_poll = 1; + bus->poll_count++; + goto again; + } + + if (!bus->polling_mode) { + dev_warn(bus->dev, + "response timeout, switching to polling mode: last cmd=0x%08x\n", + bus->last_cmd[addr]); + bus->polling_mode = 1; + goto again; + } + return -EIO; } EXPORT_SYMBOL_GPL(snd_hdac_bus_get_response);
On Fri, 04 Oct 2019 16:35:26 +0200, Kai Vehmanen wrote:
Hey all,
while debugging issues with some Intel platforms related to display audio codec probe (see https://lists.freedesktop.org/archives/intel-gfx/2019-October/214621.html ), I found a discrepancy in behaviour between snd-hda-intel and SOF, despite using the same snd-hda-codec-hdmi as the codec driver.
The specific problem I was debugging appears in a stress test (designed to uncover the above display driver issue) where driver-unload, s3-suspend, resume and driver-reload is done in a loop and repeated for hundreds of iterations. When using SOF, I would get occasional probe fail due to a missing HDA irq. The AZX snd_hda_intel driver nicely survives this test. The explanation seems to be differences in the hdac get_response() implementation.
While the specific issue could be solved with other means, the git history shows a number of rare issues with HDA codecs where polling has helped. It would seem best to align the logic with the AZX driver implementation that has seen much more usage over the years. This will benefit SOF and any other users of the HDAC library.
While it's OK to add the polling support in the core code, I suspect that the main problem gets solved by setting the write_sync flag as the commit 2756d9143aa5. For SOF/SST, you may set the flag unconditionally since they support only the new chipsets.
I've been traveling (still for the next week), so the further reply may be delayed.
thanks,
Takashi
Hi,
On Mon, 7 Oct 2019, Takashi Iwai wrote:
I found a discrepancy in behaviour between snd-hda-intel and SOF, despite using the same snd-hda-codec-hdmi as the codec driver.
[...]
While it's OK to add the polling support in the core code, I suspect that the main problem gets solved by setting the write_sync flag as the commit 2756d9143aa5. For SOF/SST, you may set the flag unconditionally since they support only the new chipsets.
I've been meaning to try that out, and indeed, if I set write-sync on SOF side, it helps in this specific ICL case as well. I'll push a patch via Pierre's SOF tree: https://github.com/thesofproject/linux/pull/1281
... this makes the polling fallback patch less urgent, but probably better to apply that as well, just to align behaviour between different HDAC users.
I'll also check whether we could remove the older workaround for CFL and CNL that force polling mode on these platforms. It is highly likely these workarounds are no longer needed with sync-write set.
I've been traveling (still for the next week), so the further reply may be delayed.
Ack, thanks. I'll do some more testing around these flows during the week.
Br, Kai
participants (2)
-
Kai Vehmanen -
Takashi Iwai