[0/1] ALSA: hda: add fallback to polling to hdac_bus_get_response()

Message

Kai Vehmanen Oct. 4, 2019, 2:35 p.m. UTC

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(-)

Comments

Takashi Iwai Oct. 7, 2019, 1:56 a.m. UTC | #1

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

Kai Vehmanen Oct. 7, 2019, 1:47 p.m. UTC | #2

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