On Fri, Feb 10, 2023 at 3:57 PM Jason Montleon jmontleo@redhat.com wrote:
On Fri, Feb 10, 2023 at 8:10 AM Cezary Rojewski cezary.rojewski@intel.com wrote:
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/...)
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.
Thank you for the explanation! I was not following this well, but it makes sense to me now.
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?
Yes, moving the one line back fixes both the no sound and shutdown hang.
Thank you! Jason Montleon
Kind regards, Czarek
-- Jason Montleon | email: jmontleo@redhat.com Red Hat, Inc. | gpg key: 0x069E3022 Cell: 508-496-0663 | irc: jmontleo / jmontleon
Congrats to all of you people !! Thanks for the fix. Sasa