Re: Google Pixelbook EVE, no sound in kernel 6.1.x

From: Cezary Rojewski <cezary.rojewski@intel.com>
To: Jason Montleon <jmontleo@redhat.com>
Cc: "Sasa Ostrouska" <casaxa@gmail.com>,
	"Linux regressions mailing list" <regressions@lists.linux.dev>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	lma@semihalf.com,
	"Pierre-Louis Bossart" <pierre-louis.bossart@linux.intel.com>,
	stable@vger.kernel.org, "Takashi Iwai" <tiwai@suse.de>,
	"Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
Subject: Re: Google Pixelbook EVE, no sound in kernel 6.1.x
Date: Fri, 10 Feb 2023 14:09:51 +0100	[thread overview]
Message-ID: <478bb609-8615-31b4-e6f3-8eaaf95b84d6@intel.com> (raw)
In-Reply-To: <CAJD_bPLHPfLO=192Ohrxvg5OGtAfAgTim0O+fdmxo753VS83iQ@mail.gmail.com>

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