On 2023-02-09 5:13 PM, Jason Montleon wrote:
I've done some more digging. The only line that needs to be reverted
from f2bd1c5ae2cb0cf9525c9bffc0038c12dd7e1338, moving from
snd_hda_codec_device_init back to snd_hda_codec_device_new is:
codec->core.exec_verb = codec_exec_verb;
(https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/pci/hda/hda_codec.c?h=v6.1.11#n931)
I added a bunch of debug statements and all the code in
codec_exec_verb runs at boot with this in snd_hda_codec_device_init,
whereas it does not when in snd_hda_codec_device_new.
From what I can tell we end up in snd_hda_power_up_pm and then get
hung up at snd_hdac_power_up.
There are a bunch of pin port messages that show up from
hdac_hdmi_query_port_connlist when things are working, that never
appear when broken:
[ 14.618805] HDMI HDA Codec ehdaudio0D2: No connections found for pin:port 5:0
[ 14.619242] HDMI HDA Codec ehdaudio0D2: No connections found for pin:port 5:1
[ 14.619703] HDMI HDA Codec ehdaudio0D2: No connections found for pin:port 5:2
...
I do see hdac_hdmi_runtime_suspend run a moment before things go bad,
but I have no idea if it is related.
Without patching anything and CONFIG_PM unset everything works.
I don't know if that helps anyone see where the problem is. If not
I'll keep plugging away.
Incidentally, commit 3fd63658caed9494cca1d4789a66d3d2def2a0ab, pointed
to by my second bisect, starts making using of skl_codec_device_init
where I believe snd_hda_codec_device_init is called and starts the
problem. I believe this is why reverting either of the two works
around the problem.
This is some exceptional debugging, Jason.
I believe this finding reveals a long standing problem that was covered
by a very specific codec-fields initialization order:
During initial part of codec-device initialization, VERBs execution
follows different flow than one happening once the device is fully
initialized. This comes down to the if-statement preset in
snd_hdac_exec_verb() and the fact codec_exec_verb() differs from
snd_hdac_bus_exec_verb() in PM-handling - the latter is devoid of it.
That is until ->exec_verb gets initialized and codec_exec_verb() becomes
the sole handler of VERB execution process. As PM is not yet configured
at the time - snd_hda_codec_device_init() happens early, whereas PM
configuration is done later with snd_hda_set_power_save() during
skl_hda_audio_probe() in sound/soc/intel/boards/skl_hda_dsp_generic.c -
it should not be touched yet.
I'm up for reverting this single line to where it was before the
offending patch. We still want to avoid the page fault the very patch is
addressing.
Does the proposed change address both problems? i.e. no sound and hang
during shutdown?
Kind regards,
Czarek
