jmontleon / pixelbook-fedora

How to install Fedora on a Pixelbook with reasonable results
60 stars 12 forks source link

Sound broken with kernel 6.1 #51

Closed jmontleon closed 1 year ago

jmontleon commented 1 year ago

Audio is broken in 6.1 again.

jmontleon commented 1 year ago

At least three kernel breakages since we got working sound:

And two Alsa UCM reshuffles:

I no longer have the desire and time to chase down problems. This latest round it seems the sound card shows up about 20% of the time when booting, but even when it does pulseaudio hangs and can't start, nor shutdown causing shutdown hangs as well.

Best I can recommend if someone wants to take over is sending an email to the kernel regressions list and seeing if someone can figure out what's gone wrong this time.

For my part, back on Chrome OS and would recommend people switch back to keep their hardware functional and at least marginally useful, or stay there if they haven't switched yet.

ntcarlson commented 1 year ago

For my part, back on Chrome OS and would recommend people switch back to keep their hardware functional and at least marginally useful, or stay there if they haven't switched yet.

Totally understandable. I don't think any hardware has given me as much of a headache as Linux on the Pixelbook has. Thanks for all your work in keeping it usable up to this point. It has been much appreciated.

For those still interested in running Linux of the Pixelbook, it looks like there may be hope. See this reddit thread and referenced kernel patch. If I understand correctly, this would make audio work out of the box without needing to copy firmware from the ChromeOS image.

saxa commented 1 year ago

@jmontleon thanks for the hard work. I can confirm that my last usable audio was with kernel 6.0.5. How would one prevent the kernel from upgrading ? Is there some kind of blacklist for dnf ?

jmontleon commented 1 year ago

From man dnf.conf:

       excludepkgs
              list

              Exclude  packages  of  this repository, specified by a name or a glob and separated by a comma, from all operations.  Can be disabled using --disableexcludes command line
              switch.  Defaults to [].

You should be able to use 6.0.18, I think was the last build of the 6.0 kernel Fedora released. I Submitted a patch to the kernel for 6.0 when we got the broken 6.0.5 update, and it made it in for 6.0.10. Anything 6.1.x as far as I am aware is broken again. I looked for a few minutes and as far as I could tell going back to the earliest rc rpm builds it was busted up to 6.1.6 or whatever was in testing at the time.

An email to the kernel regressions mailing list letting them know it's broken is your best bet for a quick-ish repair.

https://docs.kernel.org/admin-guide/reporting-regressions.html

They're supposed to take breakages of working functionality seriously and get it fixed in short order. I would NOT bother with filing a bug on bugzilla.kernel.org. It's largely unused and last go around my BZ was ignored until I started pestering people. By the time I got a response I had managed to figure out how to fix it myself

saxa commented 1 year ago

@jmontleon thanks for the dnf.conf thingy. I will try to send and email to that list.

saxa commented 1 year ago

I can confirm, for me the 6.0.18 kernel has working sound. kernel-6.0.18-300.fc37.src.rpm

saxa commented 1 year ago

I have just sent out the email to the mailing list. https://lore.kernel.org/stable/CALFERdzKUodLsm6=Ub3g2+PxpNpPtPq3bGBLbff=eZr9_S=YVA@mail.gmail.com/T/#u

jmontleon commented 1 year ago

I forgot to add you may want to exclude kernel, kernel-core, and kernel-modules. I'm not sure if excluding just kernel will prevent the others from updating. You should also be able to use a wildcard like kernel*

saxa commented 1 year ago

Ok, I just got some replies on the LKML so I will try to supply them all the needed data. As for the blacklisting stuff, I saw there is also a possibility to lock a specific package with dnf versionlock add kernel-6.0.18-300.fc37 Hope this can at least prevent me on accidentally upgrade the working kernel.

As for the excluding other kernel packages I have always been updating everything what F37 was supplying, so I will leave that like it is right now.

Lets see if kernel devs can fix the upcoming releases for the sound on this device.

jmontleon commented 1 year ago

I saw they're asking you to do a bisect. There's possibly some ways to automate this that I'm not well aware of, so I found it rather tedious. You can see me struggling through manually performing this process in https://github.com/jmontleon/pixelbook-fedora/issues/47

You are more or less finding the last known good commit and first known broken commit. Build the middle commit. Does it work? Great, move half way up the remaining commits, No? move half way down the previous commits. Eventually you should get to at least a small-ish number of commits.

From whatever to 6.0 was about 14,000 commits. I built a kernel for half way through to start: https://github.com/jmontleon/pixelbook-fedora/issues/47#issuecomment-1296366781

Move 3500 one way, then 1750, then 875, etc.

My pain with the issue above, was not understanding that they were basically making one commit per driver, and it was a couple drivers missed when inverting logic for legacy / non-legacy drivers that caused the problem, so there was a big group of interrelated commits, since there was one making the primary change, and then the others modifying per driver. My commit was a glorified copy of what they did to the other drivers. Who knows what happened this time though.

It's possible it's not even a change related to the sound system. When our wifi broke it was an IOMMU DMAR change. It looks like I did the 'bisect' for that too to locate the offending PR. I don't even remember doing that one. https://github.com/jmontleon/pixelbook-fedora/issues/45#issuecomment-1252417013 For most people that broke thunderbolt/USB hubs if I recall. I think our system and a few others had more oddball breakages.

Since I didn't have a second dev system to go mucking up with dev kerneli installs I took the source from each split, built a kernel RPM on a more powerful system because it takes multiple times longer on a lower power pixelbook and copied over/installed when done, tested, removed, rinsed, repeat.

Git bisect is a tool to help with this and might do it a little more intelligently or differently than I described above, but at it's heart it similarly centers in on the broken commit. https://git-scm.com/docs/git-bisect#_basic_bisect_commands_start_bad_good

So basically, divide, build, test, repeat. Get close, and then give them the info, for where it broke, is what they want so they can fix or revert the offending PR.

saxa commented 1 year ago

Hi Jason, thanks for the bisect thingy, I had a read on it, I do understand how that works. The pain is the go throught that many commits :) . Fortunately I have another machine same as this one where probably I will set things up for this sound issue. My problem is also the time, as day passes fast, work, family etc... But lets try to do it in the next days.

saxa commented 1 year ago

Ok, just as a side note. I have now my doubts that this is a kernel issue, but most probably some kind of configuration isso. Why ? Because I am writing now this from a new fresh install of Slackware-current linux on my spare google pixelbook. And I have here the kernel 6.1.8 runing, and I just set up my sound following the steps in the Audio section from Jasons github. So , here goes some data for you to compare.

bash-5.2$ uname -a Linux goopix.rcdiostrouska.com 6.1.8 #1 SMP PREEMPT_DYNAMIC Tue Jan 24 13:53:19 CST 2023 x86_64 Intel(R) Core(TM) i7-7Y75 CPU @ 1.30GHz GenuineIntel GNU/Linux bash-5.2$

`root@goopix:~# dmesg | grep snd

[ 5.842682] snd_hda_intel 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if info 0x040100

[ 6.170949] snd_soc_skl 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if info 0x040100

[ 6.235479] snd_soc_skl 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])

[ 7.641822] snd_soc_skl 0000:00:1f.3: ASoC: no sink widget found for AEC Capture

[ 7.644669] snd_soc_skl 0000:00:1f.3: ASoC: Failed to add route echo_ref_out cpr 7 -> direct -> AEC Capture

[ 23.928158] snd_soc_skl 0000:00:1f.3: Module list is empty

[ 23.949961] WARNING: CPU: 3 PID: 1436 at sound/soc/intel/skylake/skl-messages.c:554 skl_init_module+0x6d7/0x6f0 [snd_soc_skl]

[ 23.949979] Modules linked in: fuse 8021q garp mrp stp llc algif_hash algif_skcipher af_alg cmac bnep ipv6 joydev hid_multitouch hid_generic btusb btrtl btbcm snd_soc_skl_ssp_clk btintel snd_sof_pci_intel_skl btmtk snd_sof_intel_hda_common snd_soc_kbl_rt5663_rt5514_max98927 soundwire_intel soundwire_generic_allocation uvcvideo bluetooth soundwire_cadence videobuf2_vmalloc ecdh_generic videobuf2_memops ecc snd_sof_intel_hda snd_soc_hdac_hdmi videobuf2_v4l2 snd_sof_pci snd_sof_xtensa_dsp videobuf2_common snd_soc_dmic cros_ec_chardev snd_sof cros_ec_sysfs videodev cros_usbpd_logger intel_tcc_cooling snd_sof_utils mc cros_usbpd_charger cros_ec_sensorhub x86_pkg_temp_thermal snd_soc_avs intel_powerclamp coretemp snd_soc_hda_codec snd_soc_skl snd_soc_hdac_hda kvm_intel spi_pxa2xx_platform snd_hda_ext_core i2c_designware_platform dw_dmac intel_xhci_usb_role_switch dw_dmac_core 8250_dw i2c_designware_core cros_ec_dev roles snd_soc_sst_ipc kvm iwlmvm intel_rapl_msr i915 snd_soc_sst_dsp irqbypass

[ 23.950052] snd_soc_acpi_intel_match snd_soc_acpi mac80211 crct10dif_pclmul drm_buddy crc32_pclmul snd_hda_intel video polyval_clmulni snd_intel_dspcfg snd_intel_sdw_acpi sdhci_pci polyval_generic wmi iwlwifi ghash_clmulni_intel snd_hda_codec evdev ttm sha512_ssse3 cqhci snd_soc_rt5514 rapl intel_cstate serio_raw drm_display_helper snd_soc_rt5663 snd_soc_rt5514_spi sdhci snd_soc_max98927 snd_soc_rl6231 snd_hda_core cec i2c_i801 snd_hwdep i2c_smbus cfg80211 mmc_core rc_core snd_soc_core intel_lpss_pci snd_compress rfkill intel_lpss snd_pcm_dmaengine drm_kms_helper mei_me idma64 snd_pcm mei mfd_core snd_timer drm snd i2c_hid_acpi i2c_hid soundcore intel_gtt processor_thermal_device_pci_legacy ac97_bus regmap_i2c agpgart hid cros_ec_i2c tpm_tis_i2c_cr50 processor_thermal_device i2c_algo_bit processor_thermal_rfim fb_sys_fops cros_ec_lpcs syscopyarea processor_thermal_mbox sysfillrect processor_thermal_rapl sysimgblt xhci_pci intel_rapl_common cros_ec xhci_pci_renesas ac cros_usbpd_notify

[ 23.950154] RIP: 0010:skl_init_module+0x6d7/0x6f0 [snd_soc_skl]

[ 23.950207] skl_tplg_mixer_event+0x676/0xb40 [snd_soc_skl]

[ 23.950221] dapm_seq_check_event+0x108/0x1c0 [snd_soc_core]

[ 23.950248] dapm_seq_run_coalesced+0x84/0x1c0 [snd_soc_core]

[ 23.950272] dapm_seq_run+0x13a/0x390 [snd_soc_core]

[ 23.950297] dapm_power_widgets+0x5ca/0x960 [snd_soc_core]

[ 23.950321] ? snd_soc_dapm_stream_event+0x102/0x150 [snd_soc_core]

[ 23.950346] snd_soc_dapm_stream_event+0x102/0x150 [snd_soc_core]

[ 23.950369] __soc_pcm_prepare+0x53/0xf0 [snd_soc_core]

[ 23.950396] dpcm_fe_dai_prepare+0xa7/0x130 [snd_soc_core]

[ 23.950422] snd_pcm_do_prepare+0x26/0x40 [snd_pcm]

[ 23.950435] snd_pcm_action_single+0x33/0x80 [snd_pcm]

[ 23.950448] snd_pcm_action_nonatomic+0x95/0xa0 [snd_pcm]

[ 23.950460] snd_pcm_ioctl+0x23/0x40 [snd_pcm] root@goopix:~# `

Of course i just copied the relative firmware from the chromeos_15117.112.0_eve_recovery_stable-channel_mp-v2.bin into /lib/firmware and not even messed with anything related to ucm etc... , reboot the pixelbook and sound is playing with kernel 6.1.8.

So most probably there are some configure options in kernel that slackware uses and fedora not, or maybe some other pieces missing in fedora. But I really start to believe this is not a kernel problem.

Sound it seems very amplified and loud, nearly distorting but with 20% of volume it works ok.

jmontleon commented 1 year ago

That's interesting...

If you still have Fedora to try you can remove the pixelbook-alsa-ucm package, reboot, and see if it works at all. Could be there is a change that breaks with the alsa-ucm configuration. I see it's 6.1.8 on your slackware install. 6.1.8 is in the testing repo for Fedora. If removing the ucm files does nothing, you could try dnf -y update --enablerepo=updates-testing kernel and see if the new kernel fixes it.

jmontleon commented 1 year ago

There are a bunch of audio fixes in 6.1.7 / 6.1.8. I think we had 6.1.5 and 6.1.6 was in testing and I tried it with not luck. Nothing stands out as obvious to me but it's not impossible one of them fixed it.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/sound?h=v6.1.8

saxa commented 1 year ago

@jmontleon yup, I have Fedora 37 on my work notebook. So I will try to remove the ucm stuff and report. But I have also 6.1.7 on it and it does not work. Will see if without ucm it works.

jmontleon commented 1 year ago

I installed Fedora out of curiosity and tried 6.1.8, removed pixelbook-alsa-ucm, swapped pipewire back, and tried other things. Nothing worked.

I was getting driver crashes occasionally in dmesg and they got worse with 6.2... digging around I found similar errors where people pointing at newer coreboot releases with newer kernels (something about hardening features). I was on the second most recent before and used the most recent when I updated last night (4.18.2 and 4.19).

It's possible there is a kernel interaction with firmware versions. We had an issue like this with cros_ec_type_c. Certain firmwares and kernels would not work together, which is why it's blacklisted by the ansible playbook. Eventually it was all sorted out and a modern kernel and firmware has no issue.

So what I'm getting at is maybe compare the firmware on both and if your slackware pixelbook is behind, does sound still work if you update it?

saxa commented 1 year ago

Ok, I can confirm that sound also on slackware stopped working at some point. So definetly something is still wrong. I will test things better, but on slackware I think I have lots of other issues with touchpad needed to be configured well , and wifi seems to have troubles too.

The firmware I installed on slackware is the one from chromeos_15117.112.0_eve_recovery_stable-channel_mp-v2.bin.zip And on Fedora I have an older one chromeos_14989.107.0_eve_recovery_stable-channel_mp-v2.bin.zip So will try in the next days to play a bit with that things over the weekend and see if that changes something.

jmontleon commented 1 year ago

I was referring to https://mrchromebox.tech/ firmware rather than the files from the chromeos image. While it's possible the firmware blobs on the chromeos images are updated from time to time I'm not sure how you'd tell. Perhaps md5 or sha1 sum files.

saxa commented 1 year ago

Oh I got you. Sorry. I used the one I got listed on your website iirc, I have just a seabios version, the one you is suggested to work, dont remeber right now which one, but I have not messed with that thingy anytime. I just press CTRL-L every boot.

saxa commented 1 year ago

I tried to uninstal pixelbook-alsa-umc on my Fedora37 install and reboot into kernel 6.1.7 but sound is not working. I had just dummy output listed.

saxa commented 1 year ago

Ok, here is attached the biosdecode and dmidecode commands on my slackware install. biosdecode_dmidecode.txt

I will post later also the same from my fedora pixelbook.

jmontleon commented 1 year ago

Working on a bisect. Gonna take another day or two since I have 11 builds minimum to get there. There is/was the added complexity that my legacy dai renaming fix didn't get in until 6.1-rc6 on the 6.1 branch so I have to patch it in to fix that in order to find the new breakage. Not a big deal once it's added to the rpm spec, but anyway.

$ git bisect log
git bisect start
# status: waiting for both good and bad commits
# good: [cdf62a88c1398debeb1a36b4701537a9cdf0206b] Linux 6.0.18
git bisect good cdf62a88c1398debeb1a36b4701537a9cdf0206b
# status: waiting for bad commit, 1 good commit known
# bad: [aae703b02f92bde9264366c545e87cec451de471] Merge tag 'for-6.1-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
git bisect bad aae703b02f92bde9264366c545e87cec451de471
# good: [4fe89d07dcc2804c8b562f6c7896a45643d34b2f] Linux 6.0
git bisect good 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
# bad: [18fd049731e67651009f316195da9281b756f2cf] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
git bisect bad 18fd049731e67651009f316195da9281b756f2cf
# good: [1c2daf52185bbc91421f0e84e6bf2706bb350cce] Merge tag 'tag-chrome-platform-for-v6.1' of git://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux
git bisect good 1c2daf52185bbc91421f0e84e6bf2706bb350cce
saxa commented 1 year ago

Woow, you are a brave man :) , here is attached my biosdecode and dmidecode from my fedora pixelbook. fedora_biosdecode_dmidecode.txt

saxa commented 1 year ago

Ok, uninstalling pixelbook-alsa-ucm made sound not work also in 6.0.18. I personally experience 2 issues with 6.1.x series, sound and another issue is that I can not poweroff the laptop. This happens on both installs with 6.1.x kernels, fedora and slackware. I will try to load the driver and stuff manually and see what happens.

jmontleon commented 1 year ago

Try https://copr.fedorainfracloud.org/coprs/jmontleon/pixelbook/build/5344963/ when it's done building. kernel-6.1.8-201.pixelbook.fc37

I just replied to the regression list. A bisect implicates f2bd1c5ae2cb0cf9525c9bffc0038c12dd7e1338.

Reverting it gives me working sound, but it's not clear to me why this breaks 6.1. This code is in 6.0. My only guess is something else got dropped or added in 6.1 that's causing the issue by interacting with this code incorrectly.

jmontleon commented 1 year ago

By the way, I would guess the amplified sound you are experiencing in Slackware is due to the absence of the alsa ucm configuration. I recall it being quite loud on Fedora as well before that was sorted out,

saxa commented 1 year ago

Great work Jason !! Many thanks for this effort. In fact my sound seems amplified x10 It works well with sound slider on 10-20%.

jmontleon commented 1 year ago

I saw you uninstalled pixelbook-alsa-ucm. Don't forget to reinstall it if you havn't so headphones/mic etc. work.

jmontleon commented 1 year ago

I'm seeing a trace with working audio. I gather this is a warning from a hardening feature and should probably be addressed. I'll probably ask on the existing thread if it's worth starting another now or waiting until the current issue is resolved.

jmontleon commented 1 year ago

https://gist.github.com/jmontleon/bf12e34cbf1b6c3a925717a6c73d5b99

huang-jy commented 1 year ago

Just a heads up, I tried this and it got stuck on "loading linux" on boot (no "loading initial ramdisk" message)

So I've uninstalled that and am back to 6.0.18

Try https://copr.fedorainfracloud.org/coprs/jmontleon/pixelbook/build/5344963/ when it's done building. kernel-6.1.8-201.pixelbook.fc37

I just replied to the regression list. A bisect implicates f2bd1c5ae2cb0cf9525c9bffc0038c12dd7e1338.

Reverting it gives me working sound, but it's not clear to me why this breaks 6.1. This code is in 6.0. My only guess is something else got dropped or added in 6.1 that's causing the issue by interacting with this code incorrectly.

jmontleon commented 1 year ago

@huang-jy are you on Fedora 36 or 37? I haven't seen boot hangs with any 6.1 or 6.2 kernels with or without working audio. Not really sure why you'd see that. Plenty of shutdown hangs with broken audio, but never on boot.

I don't have an idea at the moment other than try again and try updating coreboot if it's out of date (you may need an older livedisk, there are some issues preventing the scripts from working on Fedora 37 last I checked because this hardware is cursed) if it persists.

If after that it still doesn't work, probably wait for a proper fix, and file a new issue and it can be investigated if it's still broken.

FWIW this is the only change from the stock Fedora kernel package. I wouldn't really expect it to cause boot hangs.

$ cat rpmbuild/SOURCES/kbl.patch 
commit c54b10bb9aee2ac2ca9e6918f5cc2ba864c356ba
Author: Jason Montleon <jmontleo@redhat.com>
Date:   Sat Jan 28 09:48:09 2023 -0500

    Revert "ALSA: hda: Fix page fault in snd_hda_codec_shutdown()"

    This reverts commit f2bd1c5ae2cb0cf9525c9bffc0038c12dd7e1338.

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index edd653ece70d..35d2468d5707 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -925,28 +925,8 @@ snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
    }

    codec->bus = bus;
-   codec->depop_delay = -1;
-   codec->fixup_id = HDA_FIXUP_ID_NOT_SET;
-   codec->core.dev.release = snd_hda_codec_dev_release;
-   codec->core.exec_verb = codec_exec_verb;
    codec->core.type = HDA_DEV_LEGACY;

-   mutex_init(&codec->spdif_mutex);
-   mutex_init(&codec->control_mutex);
-   snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
-   snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
-   snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
-   snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
-   snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
-   snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
-   snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
-   snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
-   INIT_LIST_HEAD(&codec->conn_list);
-   INIT_LIST_HEAD(&codec->pcm_list_head);
-   INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
-   refcount_set(&codec->pcm_ref, 1);
-   init_waitqueue_head(&codec->remove_sleep);
-
    return codec;
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_device_init);
@@ -999,8 +979,29 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
    if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
        return -EINVAL;

+   codec->core.dev.release = snd_hda_codec_dev_release;
+   codec->core.exec_verb = codec_exec_verb;
+
    codec->card = card;
    codec->addr = codec_addr;
+   mutex_init(&codec->spdif_mutex);
+   mutex_init(&codec->control_mutex);
+   snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
+   snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
+   snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
+   snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
+   snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
+   snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
+   snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
+   snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
+   INIT_LIST_HEAD(&codec->conn_list);
+   INIT_LIST_HEAD(&codec->pcm_list_head);
+   refcount_set(&codec->pcm_ref, 1);
+   init_waitqueue_head(&codec->remove_sleep);
+
+   INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
+   codec->depop_delay = -1;
+   codec->fixup_id = HDA_FIXUP_ID_NOT_SET;

 #ifdef CONFIG_PM
    codec->power_jiffies = jiffies;
huang-jy commented 1 year ago

I'm on F36 (haven't upgraded yet). I also haven't updated my mrchromebox fw for a long while. Which version are you running, just in case I'm behind?

jmontleon commented 1 year ago

4.19

huang-jy commented 1 year ago

I was on 4.17. I've updated to the Jan 23rd fw (4.19) and the kernel still hangs. I'll just leave it and wait for an upstream fix. I'll try a F37 upgrade next weekend and see if the f37 build works .

Image attached for reference image

jmontleon commented 1 year ago

Odd. There's basically no difference in the kernel spec between 36 and 37 (ignoring the differences for the kbl patch I added, which should be in all the copr builds). Sources should all be the same, of course.

--- kernel.spec 2023-01-24 15:05:14.000000000 -0500
+++ kernel.spec.v6.1.8  2023-01-28 09:55:19.826822784 -0500
@@ -126,13 +126,13 @@
 # define buildid .local
 %define specversion 6.1.8
 %define patchversion 6.1
-%define pkgrelease 100
+%define pkgrelease 200
 %define kversion 6
 %define tarfile_release 6.1.8
 # This is needed to do merge window version magic
 %define patchlevel 1
 # This allows pkg_release to have configurable %%{?dist} tag
-%define specrelease 100%{?buildid}%{?dist}
+%define specrelease 201.pixelbook%{?buildid}%{?dist}
 # This defines the kabi tarball version
 %define kabiversion 6.1.8

@@ -861,7 +861,7 @@
 ## Patches needed for building this package

 %if !%{nopatches}
-
+Patch0: kbl.patch
 Patch1: patch-%{patchversion}-redhat.patch
 %endif

@@ -1434,7 +1434,7 @@
 cp -a %{SOURCE1} .

 %if !%{nopatches}
-
+ApplyPatch kbl.patch
 ApplyOptionalPatch patch-%{patchversion}-redhat.patch
 %endif
jmontleon commented 1 year ago

Maybe booting with rhgb quiet removed from the boot options will help tell what it's hanging on.

jmontleon commented 1 year ago

To add, does the stock F36 6.1.8 hang, or just this pixelbook build?

huang-jy commented 1 year ago

To add, does the stock F36 6.1.8 hang, or just this pixelbook build?

I dnf versionlocked the kernel package at 6.1.6 to prevent installing broken kernel version and uninstalled 6.1.5 and 6.1.6. Once my machine comes off backup, I'll remove that versionlock and disable your copr on my machine to try to get the 6.1.8 stock and see what happens. I know 6.1.5 and 6.1.6 both booted fine, but wouldn't shutdown properly.

jmontleon commented 1 year ago

Ya, the shutdown issue is related to the sound problems, it would seem. The pulseaudio service gets locked up, which at the very least I'm sure isn't helping. Once the audio issues are worked around it shuts down normally again.

It looks like 6.1.8 is still in testing. You should be able to get it by doing something like sudo dnf update kernel --disablerepo=copr:copr.fedorainfracloud.org:jmontleon:pixelbook --enablerepo=updates-testing.

I'm trying to narrow down what the difference is in 6.0 and 6.1 that makes this code behave badly. Don't know if I'll get anywhere, but I guess worth a shot.

huang-jy commented 1 year ago

I had to remove your kernel build with

sudo dnf remove kernel-core-0:6.1.8-201.pixelbook.fc36.x86_64

Before it would show and update, but 6.1.8 stock also hangs in the same way as your 6.1.8 build. I removed the rhgb quiet switches and got some errors when it hung (it looks like a kernel panic):

image

EDIT: I can also confirm the same behaviour occurs on your 6.1.8 build (just uninstalled stock and put yours back in)

jmontleon commented 1 year ago

Alright, not related to the sound change then... Looks like it can't find and mount the root filesystem. I see some random other places with similar messages, but nothing with an apparent cause on a pixelbook (this one was too little ram allocated on a VM https://github.com/coreos/fedora-coreos-tracker/issues/1382 )

Any chance your /boot is too small and getting too full to fit another initramfs or somethhing?

If you want to post some information another issue would be a better place to troubleshoot so we don't muddy this one about the broken sound. But, again happy to toss out more ideas of things to look at on a separate issue.

huang-jy commented 1 year ago

Sure, I'll post a separate issue about it in the morning 👍

lyncolnmd commented 1 year ago

Heya... I thought this github would no longer be maintained but good to see it's actively working. Not an expert but if you need help testing anything on Fedora/Arch/Ubuntu/etc let me know. Ive been following your work for a minute and used it for thr basis of testing configs etc to get sound working on as many distros as possible. I can tell you that the issue youre describing has been the bane of my existence since linux 6.1 dropped on Arch. But theres something else thats somewhat broken with the sound: The HDMI/DP audio out has not been working outside of Ubuntu based distros. Have you been able to test that with your most recent fixes? Not sure if it is kernel related or config related but ive only had it work successfully on Ubuntu 22.04 and Linux Mint 21.1

joeknock90 commented 1 year ago

@huang-jy are you on Fedora 36 or 37? I haven't seen boot hangs with any 6.1 or 6.2 kernels with or without working audio. Not really sure why you'd see that. Plenty of shutdown hangs with broken audio, but never on boot.

I don't have an idea at the moment other than try again and try updating coreboot if it's out of date (you may need an older livedisk, there are some issues preventing the scripts from working on Fedora 37 last I checked because this hardware is cursed) if it persists.

If after that it still doesn't work, probably wait for a proper fix, and file a new issue and it can be investigated if it's still broken.

FWIW this is the only change from the stock Fedora kernel package. I wouldn't really expect it to cause boot hangs.

$ cat rpmbuild/SOURCES/kbl.patch 
commit c54b10bb9aee2ac2ca9e6918f5cc2ba864c356ba
Author: Jason Montleon <jmontleo@redhat.com>
Date:   Sat Jan 28 09:48:09 2023 -0500

    Revert "ALSA: hda: Fix page fault in snd_hda_codec_shutdown()"

    This reverts commit f2bd1c5ae2cb0cf9525c9bffc0038c12dd7e1338.

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index edd653ece70d..35d2468d5707 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -925,28 +925,8 @@ snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
  }

  codec->bus = bus;
- codec->depop_delay = -1;
- codec->fixup_id = HDA_FIXUP_ID_NOT_SET;
- codec->core.dev.release = snd_hda_codec_dev_release;
- codec->core.exec_verb = codec_exec_verb;
  codec->core.type = HDA_DEV_LEGACY;

- mutex_init(&codec->spdif_mutex);
- mutex_init(&codec->control_mutex);
- snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
- snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
- snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
- snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
- snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
- snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
- snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
- snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
- INIT_LIST_HEAD(&codec->conn_list);
- INIT_LIST_HEAD(&codec->pcm_list_head);
- INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
- refcount_set(&codec->pcm_ref, 1);
- init_waitqueue_head(&codec->remove_sleep);
-
  return codec;
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_device_init);
@@ -999,8 +979,29 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
  if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
      return -EINVAL;

+ codec->core.dev.release = snd_hda_codec_dev_release;
+ codec->core.exec_verb = codec_exec_verb;
+
  codec->card = card;
  codec->addr = codec_addr;
+ mutex_init(&codec->spdif_mutex);
+ mutex_init(&codec->control_mutex);
+ snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
+ snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
+ snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
+ snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
+ snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
+ snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
+ snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
+ snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
+ INIT_LIST_HEAD(&codec->conn_list);
+ INIT_LIST_HEAD(&codec->pcm_list_head);
+ refcount_set(&codec->pcm_ref, 1);
+ init_waitqueue_head(&codec->remove_sleep);
+
+ INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
+ codec->depop_delay = -1;
+ codec->fixup_id = HDA_FIXUP_ID_NOT_SET;

 #ifdef CONFIG_PM
  codec->power_jiffies = jiffies;

Is this the only patch needed? I applied this to the default Arch 6.1.8 kernel and am still not getting audio.

Just making sure I did miss something. I can just keep watching the regression list in the mean time.

jmontleon commented 1 year ago

@joeknock90 are you seeing no audio on Arch as well?

To answer your question, for me the patch above gives me functioning audio. You can try an alternate patch, but you shouldn't need both.

Basically a git bisect led me to the original commit I reverted above. But that code is in 6.0 so I did a little more digging by patching it in while running the bisect which led to a commit only in 6.1+. Reverting either works for me. The second patch is much larger because I had to revert about 5 others to cleanly revert it.

https://lore.kernel.org/regressions/7df25a70-2ec8-88ae-14ae-8ff000217924@leemhuis.info/T/#m7c9485956397a83df25ccac37578e71d19d6df1c

Sounds like the folks that work on the audio stuff don't think it's really either of these causing the problem, but for me adding either patch fixes audio, so it's about the best I can offer until they provide a proper fix.

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index eba23daf2c29..ec2a2ba21cc3 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -293,6 +293,8 @@ struct hda_codec {
 #define dev_to_hda_codec(_dev) container_of(_dev, struct hda_codec, core.dev)
 #define hda_codec_dev(_dev)    (&(_dev)->core.dev)

+#define hdac_to_hda_priv(_hdac) \
+           container_of(_hdac, struct hdac_hda_priv, codec.core)
 #define hdac_to_hda_codec(_hdac) container_of(_hdac, struct hda_codec, core)

 #define list_for_each_codec(c, bus) \
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 83aed26ab143..7192dcb299ea 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -11,6 +11,9 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
              const struct hdac_ext_bus_ops *ext_ops);

 void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
+int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
+               struct hdac_device *hdev, int type);
+void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
 void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);

 #define HDA_CODEC_REV_EXT_ENTRY(_vid, _rev, _name, drv_data) \
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 6004ea1c373e..765c40a6ccba 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -60,6 +60,59 @@ void snd_hdac_ext_bus_exit(struct hdac_bus *bus)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_exit);

+static void default_release(struct device *dev)
+{
+   snd_hdac_ext_bus_device_exit(dev_to_hdac_dev(dev));
+}
+
+/**
+ * snd_hdac_ext_bus_device_init - initialize the HDA extended codec base device
+ * @bus: hdac bus to attach to
+ * @addr: codec address
+ * @hdev: hdac device to init
+ * @type: codec type (HDAC_DEV_*) to use for this device
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
+                struct hdac_device *hdev, int type)
+{
+   char name[15];
+   int ret;
+
+   hdev->bus = bus;
+
+   snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
+
+   ret  = snd_hdac_device_init(hdev, bus, name, addr);
+   if (ret < 0) {
+       dev_err(bus->dev, "device init failed for hdac device\n");
+       return ret;
+   }
+   hdev->type = type;
+   hdev->dev.release = default_release;
+
+   ret = snd_hdac_device_register(hdev);
+   if (ret) {
+       dev_err(bus->dev, "failed to register hdac device\n");
+       snd_hdac_ext_bus_device_exit(hdev);
+       return ret;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
+
+/**
+ * snd_hdac_ext_bus_device_exit - clean up a HD-audio extended codec base device
+ * @hdev: hdac device to clean up
+ */
+void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
+{
+   snd_hdac_device_exit(hdev);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
+
 /**
  * snd_hdac_ext_bus_device_remove - remove HD-audio extended codec base devices
  *
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 8af434e14bfb..7876bdd558a7 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -246,7 +246,7 @@ static int hdac_hda_dai_hw_free(struct snd_pcm_substream *substream,
        return -EINVAL;

    hda_stream = &pcm->stream[substream->stream];
-   snd_hda_codec_cleanup(hda_pvt->codec, hda_stream, substream);
+   snd_hda_codec_cleanup(&hda_pvt->codec, hda_stream, substream);

    return 0;
 }
@@ -264,7 +264,7 @@ static int hdac_hda_dai_prepare(struct snd_pcm_substream *substream,
    int ret = 0;

    hda_pvt = snd_soc_component_get_drvdata(component);
-   hdev = &hda_pvt->codec->core;
+   hdev = &hda_pvt->codec.core;
    pcm = snd_soc_find_pcm_from_dai(hda_pvt, dai);
    if (!pcm)
        return -EINVAL;
@@ -274,7 +274,7 @@ static int hdac_hda_dai_prepare(struct snd_pcm_substream *substream,
    stream = hda_pvt->pcm[dai->id].stream_tag[substream->stream];
    format_val = hda_pvt->pcm[dai->id].format_val[substream->stream];

-   ret = snd_hda_codec_prepare(hda_pvt->codec, hda_stream,
+   ret = snd_hda_codec_prepare(&hda_pvt->codec, hda_stream,
                    stream, format_val, substream);
    if (ret < 0)
        dev_err(&hdev->dev, "codec prepare failed %d\n", ret);
@@ -299,7 +299,7 @@ static int hdac_hda_dai_open(struct snd_pcm_substream *substream,

    hda_stream = &pcm->stream[substream->stream];

-   return hda_stream->ops.open(hda_stream, hda_pvt->codec, substream);
+   return hda_stream->ops.open(hda_stream, &hda_pvt->codec, substream);
 }

 static void hdac_hda_dai_close(struct snd_pcm_substream *substream,
@@ -317,7 +317,7 @@ static void hdac_hda_dai_close(struct snd_pcm_substream *substream,

    hda_stream = &pcm->stream[substream->stream];

-   hda_stream->ops.close(hda_stream, hda_pvt->codec, substream);
+   hda_stream->ops.close(hda_stream, &hda_pvt->codec, substream);

    snd_hda_codec_pcm_put(pcm);
 }
@@ -325,7 +325,7 @@ static void hdac_hda_dai_close(struct snd_pcm_substream *substream,
 static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
                         struct snd_soc_dai *dai)
 {
-   struct hda_codec *hcodec = hda_pvt->codec;
+   struct hda_codec *hcodec = &hda_pvt->codec;
    struct hda_pcm *cpcm;
    const char *pcm_name;

@@ -394,8 +394,8 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
            snd_soc_component_get_drvdata(component);
    struct snd_soc_dapm_context *dapm =
            snd_soc_component_get_dapm(component);
-   struct hdac_device *hdev = &hda_pvt->codec->core;
-   struct hda_codec *hcodec = hda_pvt->codec;
+   struct hdac_device *hdev = &hda_pvt->codec.core;
+   struct hda_codec *hcodec = &hda_pvt->codec;
    struct hdac_ext_link *hlink;
    hda_codec_patch_t patch;
    int ret;
@@ -512,8 +512,8 @@ static void hdac_hda_codec_remove(struct snd_soc_component *component)
 {
    struct hdac_hda_priv *hda_pvt =
              snd_soc_component_get_drvdata(component);
-   struct hdac_device *hdev = &hda_pvt->codec->core;
-   struct hda_codec *codec = hda_pvt->codec;
+   struct hdac_device *hdev = &hda_pvt->codec.core;
+   struct hda_codec *codec = &hda_pvt->codec;
    struct hdac_ext_link *hlink = NULL;

    hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev));
@@ -581,6 +581,7 @@ static const struct snd_soc_component_driver hdac_hda_codec = {
 static int hdac_hda_dev_probe(struct hdac_device *hdev)
 {
    struct hdac_ext_link *hlink;
+   struct hdac_hda_priv *hda_pvt;
    int ret;

    /* hold the ref while we probe */
@@ -591,6 +592,10 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev)
    }
    snd_hdac_ext_bus_link_get(hdev->bus, hlink);

+   hda_pvt = hdac_to_hda_priv(hdev);
+   if (!hda_pvt)
+       return -ENOMEM;
+
    /* ASoC specific initialization */
    ret = devm_snd_soc_register_component(&hdev->dev,
                     &hdac_hda_codec, hdac_hda_dais,
@@ -600,6 +605,7 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev)
        return ret;
    }

+   dev_set_drvdata(&hdev->dev, hda_pvt);
    snd_hdac_ext_bus_link_put(hdev->bus, hlink);

    return ret;
diff --git a/sound/soc/codecs/hdac_hda.h b/sound/soc/codecs/hdac_hda.h
index b65560981abb..d0efc5e254ae 100644
--- a/sound/soc/codecs/hdac_hda.h
+++ b/sound/soc/codecs/hdac_hda.h
@@ -14,7 +14,7 @@ enum {
    HDAC_HDMI_1_DAI_ID,
    HDAC_HDMI_2_DAI_ID,
    HDAC_HDMI_3_DAI_ID,
-   HDAC_DAI_ID_NUM
+   HDAC_LAST_DAI_ID = HDAC_HDMI_3_DAI_ID,
 };

 struct hdac_hda_pcm {
@@ -23,8 +23,8 @@ struct hdac_hda_pcm {
 };

 struct hdac_hda_priv {
-   struct hda_codec *codec;
-   struct hdac_hda_pcm pcm[HDAC_DAI_ID_NUM];
+   struct hda_codec codec;
+   struct hdac_hda_pcm pcm[HDAC_LAST_DAI_ID];
    bool need_display_power;
 };

diff --git a/sound/soc/intel/boards/hda_dsp_common.c b/sound/soc/intel/boards/hda_dsp_common.c
index 04b7d4f7f9e2..83c7dfbccd9d 100644
--- a/sound/soc/intel/boards/hda_dsp_common.c
+++ b/sound/soc/intel/boards/hda_dsp_common.c
@@ -54,7 +54,7 @@ int hda_dsp_hdmi_build_controls(struct snd_soc_card *card,
        return -EINVAL;

    hda_pvt = snd_soc_component_get_drvdata(comp);
-   hcodec = hda_pvt->codec;
+   hcodec = &hda_pvt->codec;

    list_for_each_entry(hpcm, &hcodec->pcm_list_head, list) {
        spcm = hda_dsp_hdmi_pcm_handle(card, i);
diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
index 879ebba52832..81144efb4b44 100644
--- a/sound/soc/intel/boards/skl_hda_dsp_generic.c
+++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
@@ -190,7 +190,7 @@ static void skl_set_hda_codec_autosuspend_delay(struct snd_soc_card *card)
             * all codecs are on the same bus, so it's sufficient
             * to look up only the first one
             */
-           snd_hda_set_power_save(hda_pvt->codec->bus,
+           snd_hda_set_power_save(hda_pvt->codec.bus,
                           HDA_CODEC_AUTOSUSPEND_DELAY_MS);
            break;
        }
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 7f058acd221f..d269affcd8a8 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -689,7 +689,12 @@ static void load_codec_module(struct hda_codec *codec)

 #endif /* CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC */

-static struct hda_codec *skl_codec_device_init(struct hdac_bus *bus, int addr)
+static void skl_codec_device_exit(struct device *dev)
+{
+   snd_hdac_device_exit(dev_to_hdac_dev(dev));
+}
+
+static __maybe_unused struct hda_codec *skl_codec_device_init(struct hdac_bus *bus, int addr)
 {
    struct hda_codec *codec;
    int ret;
@@ -701,11 +706,12 @@ static struct hda_codec *skl_codec_device_init(struct hdac_bus *bus, int addr)
    }

    codec->core.type = HDA_DEV_ASOC;
+   codec->core.dev.release = skl_codec_device_exit;

    ret = snd_hdac_device_register(&codec->core);
    if (ret) {
        dev_err(bus->dev, "failed to register hdac device\n");
-       put_device(&codec->core.dev);
+       snd_hdac_device_exit(&codec->core);
        return ERR_PTR(ret);
    }

@@ -723,8 +729,9 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 #if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
    struct skl_dev *skl = bus_to_skl(bus);
    struct hdac_hda_priv *hda_codec;
+   int err;
 #endif
-   struct hda_codec *codec;
+   struct hdac_device *hdev;

    mutex_lock(&bus->cmd_mutex);
    snd_hdac_bus_send_cmd(bus, cmd);
@@ -740,22 +747,25 @@ static int probe_codec(struct hdac_bus *bus, int addr)
    if (!hda_codec)
        return -ENOMEM;

-   codec = skl_codec_device_init(bus, addr);
-   if (IS_ERR(codec))
-       return PTR_ERR(codec);
+   hda_codec->codec.bus = skl_to_hbus(skl);
+   hdev = &hda_codec->codec.core;

-   hda_codec->codec = codec;
-   dev_set_drvdata(&codec->core.dev, hda_codec);
+   err = snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
+   if (err < 0)
+       return err;

    /* use legacy bus only for HDA codecs, idisp uses ext bus */
    if ((res & 0xFFFF0000) != IDISP_INTEL_VENDOR_ID) {
-       codec->core.type = HDA_DEV_LEGACY;
-       load_codec_module(hda_codec->codec);
+       hdev->type = HDA_DEV_LEGACY;
+       load_codec_module(&hda_codec->codec);
    }
    return 0;
 #else
-   codec = skl_codec_device_init(bus, addr);
-   return PTR_ERR_OR_ZERO(codec);
+   hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
+   if (!hdev)
+       return -ENOMEM;
+
+   return snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
 #endif /* CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC */
 }

diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index f2ec2a6c2e0f..4c128ba02340 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -109,7 +109,13 @@ EXPORT_SYMBOL_NS(hda_codec_jack_check, SND_SOC_SOF_HDA_AUDIO_CODEC);
 #define is_generic_config(x)   0
 #endif

-static struct hda_codec *hda_codec_device_init(struct hdac_bus *bus, int addr, int type)
+static void hda_codec_device_exit(struct device *dev)
+{
+   snd_hdac_device_exit(dev_to_hdac_dev(dev));
+}
+
+static __maybe_unused struct hda_codec *
+hda_codec_device_init(struct hdac_bus *bus, int addr, int type)
 {
    struct hda_codec *codec;
    int ret;
@@ -121,11 +127,12 @@ static struct hda_codec *hda_codec_device_init(struct hdac_bus *bus, int addr, i
    }

    codec->core.type = type;
+   codec->core.dev.release = hda_codec_device_exit;

    ret = snd_hdac_device_register(&codec->core);
    if (ret) {
        dev_err(bus->dev, "failed to register hdac device\n");
-       put_device(&codec->core.dev);
+       snd_hdac_device_exit(&codec->core);
        return ERR_PTR(ret);
    }

@@ -138,10 +145,11 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
 {
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
    struct hdac_hda_priv *hda_priv;
+   struct hda_codec *codec;
    int type = HDA_DEV_LEGACY;
 #endif
    struct hda_bus *hbus = sof_to_hbus(sdev);
-   struct hda_codec *codec;
+   struct hdac_device *hdev;
    u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) |
        (AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
    u32 resp = -1;
@@ -164,20 +172,20 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
    if (!hda_priv)
        return -ENOMEM;

+   hda_priv->codec.bus = hbus;
+   hdev = &hda_priv->codec.core;
+   codec = &hda_priv->codec;
+
    /* only probe ASoC codec drivers for HDAC-HDMI */
    if (!hda_codec_use_common_hdmi && (resp & 0xFFFF0000) == IDISP_VID_INTEL)
        type = HDA_DEV_ASOC;

-   codec = hda_codec_device_init(&hbus->core, address, type);
-   ret = PTR_ERR_OR_ZERO(codec);
+   ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, type);
    if (ret < 0)
        return ret;

-   hda_priv->codec = codec;
-   dev_set_drvdata(&codec->core.dev, hda_priv);
-
    if ((resp & 0xFFFF0000) == IDISP_VID_INTEL) {
-       if (!hbus->core.audio_component) {
+       if (!hdev->bus->audio_component) {
            dev_dbg(sdev->dev,
                "iDisp hw present but no driver\n");
            ret = -ENOENT;
@@ -203,12 +211,15 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,

 out:
    if (ret < 0) {
-       snd_hdac_device_unregister(&codec->core);
-       put_device(&codec->core.dev);
+       snd_hdac_device_unregister(hdev);
+       put_device(&hdev->dev);
    }
 #else
-   codec = hda_codec_device_init(&hbus->core, address, HDA_DEV_ASOC);
-   ret = PTR_ERR_OR_ZERO(codec);
+   hdev = devm_kzalloc(sdev->dev, sizeof(*hdev), GFP_KERNEL);
+   if (!hdev)
+       return -ENOMEM;
+
+   ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, HDA_DEV_ASOC);
 #endif

    return ret;
joeknock90 commented 1 year ago

are you seeing no audio on Arch as well?

YUUUUUUUUUP! Basically same issues you're seeing with Fedora and kernel versions as far as I can tell. I only got this Eve device about 2 weeks ago. Installed Linux LTS and audio worked, but backlight was stuck at 100% and when I was using it on the couch at night I was getting yelled at for blinding my partner.

Thanks for this one. I'm attempting to apply it now. Looks like it's applying fine.

image

Will report back.

savostyanov commented 1 year ago

I can confirm EVE sound not working on Arch since kernel 6.1. The last working kernel is 6.0.12 in arch distros (kernels 6.0.13-6.0.18 were skipped).

jmontleon commented 1 year ago

Thanks for the confirmation. It might be worth one of you mentioning Arch is also affected on the regression list, especially any input on whether either one or neither of the patches above does anything.

I don't want to flood them with information, but that Slackware, Arch, and Fedora all seem so far to have varying degrees of the issue with the same kernel version which seems interesting to me. There seems to be other factors at play, whether that's firmware/topology versions, kernel configs, etc. I have no idea.

saxa commented 1 year ago

@jmontleon just to let you know that my fedora install with 6.1.8 have a working sound. uname -a Linux rclaptop 6.1.8-201.pixelbook.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Jan 28 17:50:38 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux