On Fri, Feb 10, 2023 at 3:57 PM Jason Montleon <jmontleo@xxxxxxxxxx> wrote: > > On Fri, Feb 10, 2023 at 8:10 AM Cezary Rojewski > <cezary.rojewski@xxxxxxxxx> 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/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. > > > > 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@xxxxxxxxxx > Red Hat, Inc. | gpg key: 0x069E3022 > Cell: 508-496-0663 | irc: jmontleo / jmontleon > Congrats to all of you people !! Thanks for the fix. Sasa
