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

[Jason Montleon <jmontleo@redhat.com>]

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/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@redhat.com
Red Hat, Inc.         | gpg key: 0x069E3022
Cell: 508-496-0663    | irc: jmontleo / jmontleon