thesofproject / linux

Linux kernel source tree
Other
89 stars 129 forks source link

[BUG] Samsung Galaxy Book2 Pro 360 no sound through speaker #4055

Open datmischa opened 2 years ago

datmischa commented 2 years ago

Describe the bug On my Galaxy Book2 Pro 360, there is no audio output through the internal speakers. Audio works fine with headphones (wired as well as bluetooth), could not test over usbc - displayport cable yet, but guessing this works too. If I play some test sound through speaker-test, I can see the indicator in the plasma-pa applet moving. I tested the internal microphone with arecord, it works as well.

What have you tried to diagnose or workaround this issue? Installed mainline kernel (5.19-rc5) and sof-firmware 2.2 with no effect.

To Reproduce Installed / booted various Linux derivates (Gentoo, Arch, Void, Ubuntu, Fedora) with the same outcome.

Reproduction Rate 100% of the time.

Expected behavior Sound is playing through internal speakers

Impact deal breaker, no sound on the go

Environment 1) Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).

3) Name of the platform(s) on which the bug is observed.

dmesg dmesg_grep_sof.txt dmesg.txt

sof-logger freezes my system

alsa-info.txt

koma-code-mk commented 6 months ago

@j-m-harris I have same dump for Razer Blade 14 2023, but I also can not make it work. Pin setting that differ are for microphones and detection for headphone jack. Which are not making any difference for sound play.

Do you mind uploading the dumps? I am also considering of creating them. Did you use RtHDDump or QEMU?

j-m-harris commented 6 months ago

Do you mind uploading the dumps? I am also considering of creating them. Did you use RtHDDump or QEMU?

Sure, dumped from QEMU using instructions based on https://github.com/Conmanx360/QemuHDADump/tree/master

Converted using https://github.com/Conmanx360/ca0132-tools

koma-code-mk commented 6 months ago

Thank you, then I get an idea, what to expect, when I try it on my blade.

j-m-harris commented 4 months ago

This fix is working for me on a Razer Blade 16 - RZ09-0483: https://bugzilla.kernel.org/attachment.cgi?id=306157&action=edit

crinavar commented 4 months ago

This fix is working for me on a Razer Blade 16 - RZ09-0483: https://bugzilla.kernel.org/attachment.cgi?id=306157&action=edit

The same. Fix working on the Razer Blade 16 running Arch Linux.

scolton99 commented 4 months ago

This fix is working for me on a Razer Blade 16 - RZ09-0483: https://bugzilla.kernel.org/attachment.cgi?id=306157&action=edit

The same. Fix working on the Razer Blade 16 running Arch Linux.

Same, working on a Razer Blade 18 running Arch. Audio is pretty tinny but that's not a dealbreaker, just glad to have it at all!

sblommers commented 4 months ago

Yes works! :heart:

hamfirst commented 3 months ago

Hello all - I have taken the verbs provided by @joshuagrisham and turned them into a working kernel patch!

The verbs themselves can't be minimized that much from my experiments. There seems to be a few categories of them

  1. Generic register sets
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x82
    hda-verb /dev/snd/hwC0D0 0x20 0x444 0x8

This takes the value 0x4408 and writes it to register 0x82

  1. Data passed to the amp

    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x26
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x26
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x23
    hda-verb /dev/snd/hwC0D0 0x20 0x420 0x12
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x0
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x6f
    hda-verb /dev/snd/hwC0D0 0x20 0x4b0 0x11
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x23
    hda-verb /dev/snd/hwC0D0 0x20 0x420 0x12
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x0
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x6f
    hda-verb /dev/snd/hwC0D0 0x20 0x4b0 0x11
  2. Amp target set. 0x0038 or 0x0039 gets written to register 0x22

    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x22
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x38
  3. No idea but it's necessary. Occasionally while writing data to the amp, 0x0000 gets written to register 0x89. I'm really not sure what this does but audio never turns on fully without it.

    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x89
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x0
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x23
    hda-verb /dev/snd/hwC0D0 0x20 0x421 0x0
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0xd0
    hda-verb /dev/snd/hwC0D0 0x20 0x495 0xe
    hda-verb /dev/snd/hwC0D0 0x20 0x4b0 0x17
    hda-verb /dev/snd/hwC0D0 0x20 0x500 0x89
    hda-verb /dev/snd/hwC0D0 0x20 0x400 0x0

Based on this example: https://patchwork.kernel.org/project/alsa-devel/patch/441231b80626cca3862320ff8b8d35430234518c@ap2c.com/#25337697 It appears that there's some back and forth communication between the chip and the driver in that after writing out a block of data in register 0x23 - 0x26. You can see in the verbs it's always writing out 0xb011 or 0xb017 to 0x26, but looking at the stored values through

echo 1 |sudo tee /sys/module/snd_hda_codec/parameters/dump_coef
cat /proc/asound/card0/codec#0 > lin_codec-dump

it appears that the 0x0010 bit is always set back to 0.

Using this information, I tried eliminating a lot of the initial register queries of this type which worked

hda-verb /dev/snd/hwC0D0 0x20 0x500 0x0
hda-verb /dev/snd/hwC0D0 0x20 0x500 0x0
hda-verb /dev/snd/hwC0D0 0x20 0x500 0x1
hda-verb /dev/snd/hwC0D0 0x20 0x500 0x1
hda-verb /dev/snd/hwC0D0 0x20 0x500 0x2
hda-verb /dev/snd/hwC0D0 0x20 0x500 0x2
...

My guess is that the verb sniffer doesn't track "get coefficient" verbs, but this is the windows driver querying ever register so that it can avoid unnecessary writes. The verb list also seems to do everything twice, which was unnecessary in getting this all working.

This seems to work about as well as running the verbs script by hand, meaning that it doesn't work perfectly. The left speaker is still slightly louder than the right.

I experimented a bit with the verb list from @EverardNisse as well, but I could never get them to work. Interestingly, their verb list contains data for two additional amps at 0x003c and 0x003d. It also seems to occasionally write a small amount of data to each amp

0x22, 0x0038
0x22, 0x0038
        { 0x203a, 0x0000, 0x0081, 0xb011 }, { 0x23ff, 0x0000, 0x0001, 0xb011 }, 
0x22, 0x0039
0x22, 0x0039
        { 0x203a, 0x0000, 0x0081, 0xb011 }, { 0x23ff, 0x0000, 0x0001, 0xb011 }, 
0x22, 0x003c
0x22, 0x003c
        { 0x203a, 0x0000, 0x0081, 0xb011 }, { 0x23ff, 0x0000, 0x0001, 0xb011 }, 
0x22, 0x003d
0x22, 0x003d
        { 0x203a, 0x0000, 0x0081, 0xb011 }, { 0x23ff, 0x0000, 0x0001, 0xb011 }, 

I'm not sure what any of this is for either, and the data is completely opaque to me. I'm including the patch file and my analysis of both verb dumps. It would be great to get another set of verbs that fully gets audio working. Going to try and pull my own verb list later if I have time.

patch.txt Josh Analysis.txt Everard Analysis.txt

hamfirst commented 1 month ago

Just to wrap this up on my end - I've created a new set of verbs using a custom version of qemu that's available here: https://github.com/hamfirst/qemu-corb-snoop

These verbs get the speakers working a little better (no left side being slightly softer). I've updated the kernel patch to match the new verbs and submitted it to the kernel. We'll see if it gets accepted! hda-verbs.txt

acrilique commented 1 month ago

Hi @hamfirst , I tested that latest list of verbs on my LG gram 17ZD90R-G.AX75B To me, the longer list of verbs made the speakers usable but they sounded too loud at max volume and the right speaker was louder than the left. This one fixes both things: both speakers sound equally loud and maximum volume is now acceptable, it doesn't sound like the speakers will break. Thanks everyone for their contributions. Let's hope this gets accepted in the kernel.

hamfirst commented 1 month ago

The patch did indeed make it into the 6.11 merge window. However, the fix only applies to hardware id 0x144dc1ca. You can get the hardware id with this command:

cat /proc/asound/card0/codec#0 | grep Subsystem

If those verbs are working for you, but the HW id is different, I can submit another patch to tie it to the HW id for your laptop. Just out of curiosity, does your laptop have a "Sound by AKG" logo on it somewhere? Mine has that on the bottom and I'm wondering if that's the hardware that both of our laptops have in common that this set of verbs is operating on.

acrilique commented 1 month ago

Hey, my hardware ID is 0x1854048a The laptop has no "Sound by AKG" logos anywhere. Maybe it has the same cpu as yours? It would be nice to know what do they have in common and that way maybe apply it to a wider range of laptops with that same similarity.

hamfirst commented 1 month ago

Sure I can put in another patch for that hardware ID if you're able to compile and test it.

acrilique commented 1 month ago

What exactly do I need to compile? The kernel with the patch applied? (sorry if the question is stupid, but I can definitely compile anything, it's just that I don't know what you're referring to)

hamfirst commented 1 month ago

No worries - yes just asking if you're willing to compile the kernel with the patch and make sure it doesn't cause any problems

Elvenrunes commented 1 month ago

Would you mind doing one for 0x144dc870 as well if it's not too much work? I'd be happy to try compiling it at as well - hopefully what you write for acrilique should suffice to point me in the right direction too :)

bjorn@bjorn-950XED:~$ cat /proc/asound/card0/codec#0 | grep Subsystem Subsystem Id: 0x144dc870

acrilique commented 1 month ago

I'm willing to compile the kernel with the custom patch this week. High level overview of what I need to do (in case it's also useful for @Elvenrunes ):

Please make any corrections/suggestions, it will be my first time compiling the kernel.

hamfirst commented 1 month ago

@acrilique @Elvenrunes Great thank you!

The kernel repo you'll want to clone is this: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git It's the official sound subsystem maintainer's repo

I'll attach the patch here. It's just a one line change in the file for each of your systems. If this works, I'll probably refactor my original patch a bit so it's less Samsung specific since it seems to help on LG hardware and maybe others. Hopefully this can fix quite a few laptop+linux audio issues.

patch.txt

acrilique commented 1 month ago

Hi. I managed to compile and install a custom kernel cloned from the sound subsystem mantainer's repo (with the added line SND_PCI_QUIRK(0x1854, 0x048a, "LG gram 17ZD90R-G.AX75B", ALC298_FIXUP_SAMSUNG_AMP2), in patch_realtek.c). It took me longer than expected because when I edited the Makefile to put a custom subversion number, I added an extra space (" ") after the version number and that just broke the entire make process and make install script. Newbie error hahah. Anyway. After going through that, the new kernel booted just fine. I disabled necessary_verbs.service which is the service I did setup for making the audio work, I rebooted and audio is still working. No issues so far.

@hamfirst if you're going to refactor your original patch, I would have no problem testing that too.

hamfirst commented 1 month ago

That's great news!

Just to be sure you did a full power cycle when you tested? I've noticed while debugging that I could get false positives for my sound working if I just did a shutdown -r

acrilique commented 1 month ago

I've been testing again, to be sure. In order to reboot I'm using the shutdown button from KDE's start menu (I'm not rebooting, just pressing shutdown and then booting again via physical button). I've got 2 kernels installed:

If I boot the first one (6.10.2), sound devices are recognized but there's no sound through speakers. Booting with the second makes sound from the speakers work.

hamfirst commented 1 month ago

Perfect! I'll let you know when I get the patch updated

joshuagrisham commented 3 weeks ago

Hi and first of all I just wanted to say @hamfirst really super job in experimenting and fine-tuning more of what seems to be needed here :partying_face: And then that you have managed to get it into mainline is really fantastic!

I am not sure if it is the end of the world but one thing I wanted to mention is that @Elvenrunes 's codec device ID is the same as mine and based on their prompt I assume have the exact same device as me (Galaxy Book2 Pro 15") and not exactly "Samsung Galaxy Book" as you wrote in this latest attached patch :angel:

$ cat /proc/asound/card0/codec#0 | grep Subsystem
Subsystem Id: 0x144dc870

$ sudo dmidecode
...
Handle 0x0001, DMI type 1, 27 bytes
System Information
    Manufacturer: SAMSUNG ELECTRONICS CO., LTD.
    Product Name: 950XED
...
    Family: Galaxy Book2 Pro

Handle 0x0002, DMI type 2, 15 bytes
Base Board Information
    Manufacturer: SAMSUNG ELECTRONICS CO., LTD.
    Product Name: NP950XED-KA2SE
...

I am actually making the kernel now (6.11-rc3) and planning to give it a spin :sunglasses:

One thing I thought to also mention: a few months ago I tried to look into this a bit more and took around 20x new traces after powering off and then tried to do a diff across all of them to really nail down what seems to be the "core" of what is needed. The weird thing was that no one of the traces were exactly the same as the other--they all had a little bit of variation. I have had an idea that it would be good to try and do more tracing where I make sure to save and parse the timestamp of each payload that is sent, and keep track of the exact minute/second different things are happening, so I can try to better figure out what is happening during initial startup vs "continuous background stuff" vs what happens when you actually play audio etc, PLUS that (as you mentioned) it would probably be really good to see if it is possible to add traces for reads and see if there is any kind of logic for if/why certain payloads are sent as a response to something that is read.

Super job you have done to fine-tune but as I think many of us has said, it really feels like we are aiming a bit in the dark here and hoping for the best. It sure would be nice to have a bit more of a handle on exactly what is happening, when, and why.

Plus a little help from Realtek and/or Samsung would be amazing but I have tried to reach out several times with not so much success :( (and even submitted a formal open source request to Samsung in a request form that they have, but got a denial back....)

hamfirst commented 3 weeks ago

Thanks Joshua!

I did try to get more of a handle on where the data is coming from but I mostly ran into dead ends. My main drive was attempting to get a memory breakpoint on the CORB buffer so I could get a backtrace into which part of the driver was doing these writes. In QEMU, I tried writing out the CORB buffer physical memory address to the console and then pausing the VM. I also had an instance of WinDBG with kernel debugging mode enabled to try and set the breakpoint based on the address, but no dice - I never got the breakpoint to hit.

Digging around some of the files in the driver - I found some files that specifically pertained to my hardware ID (0x144dc1ca) and they did contain some data that might explain the data. I know that some of the initial verbs set things like speaker channels, sample rates, and digital audio support by running each verb one at a time and then diffing the text produced by this command: cat /proc/asound/card0/codec#0

Probably the most interesting thing in that file is that it has some per-program equalizer settings (skype, chrome, VLC, etc). So if this is actually a thing, there must be something in the windows driver that's monitoring processes as they start up, and sending some data to the amp to modify how the audio filters work. Another theory that I haven't validated at all is that when there's no sound playing it's putting the amp into some kind of low power usage mode. An experiment that I never got around to was trying to see when/what verbs were sent to the sound card in response to things I was doing in windows.

Anyway - I've been trying to update the patch but 6.11.rc1 and 6.11.rc2 both haven't booted for me (I even disabled my kernel change) so I'll be trying out rc3 shortly to see if I can make some progress

joshuagrisham commented 2 weeks ago

Hi @hamfirst I also had problems with various 6.11 RCs bus was able to build with 6.11-rc14 and it seems to be working well.

(Yes, the speakers work! :sunglasses: )

One suggestion I would have for your quirk table entries (including your latest patch file here) are just the names--IMO the name of 0x144d, 0xc870 should be Samsung Galaxy Book2 Pro (NP950XED) and the name of your device should be Samsung Galaxy Book3 Pro 360 (NP960QFG) i.e. without the keyboard layout suffix since it is (AFAIK?) basically the same device all over the world other than having a different keyboard and different default language / region settings in the as-shipped Windows installation.

SND_PCI_QUIRK(0x144d, 0xc870, "Samsung Galaxy Book2 Pro (NP950XED)", ALC298_FIXUP_SAMSUNG_AMP2),
SND_PCI_QUIRK(0x144d, 0xc1ca, "Samsung Galaxy Book3 Pro 360 (NP960QFG)", ALC298_FIXUP_SAMSUNG_AMP2),

Regarding how to debug and really try to nail down what is happening, some of what you have found is very interesting! I have tried to snoop around in the driver code but reading disassembled code is unfortunately definitely not my strong suit. :laughing:

One thing I have done, though, is added some timestamps to the QEMU log variant I have been working with and was able to make a few more captures where it is much more clear when things were happening (for what I think are both writes and reads) e.g. you can quite clearly see what seem to be "start up"-related stuff vs "playing"-related stuff vs "shutdown"-related stuff which for me is a huge step forward, BUT, we are still talking THOUSANDS of lines of events, no two captures seem to be the same even if you do EXACTLY the same thing during the traces, and it is pretty easy to quickly get lost in the forest IMO.

I will try to clean up and push a bit of what I did somewhere in case it helps anyone else, including my additions to QEMU with per-millisecond time, plus several copies of traces in different scenarios (disabled all Windows theme sounds including login sound and then not playing audio at all and waiting several minutes between startup vs shutdown, waiting minutes then playing audio, then waiting minutes and shutting down again.. etc etc) plus a Python parsing script which I was using to format the data into CSV so that I could then try to sort through it in a spreadsheet..

semyon-gordeev commented 1 week ago

@hamfirst big thanks for your work. I have Samsung Galaxy Book 3 Pro (NP964XFG-KC1US) with "Sound by AKG" logo and Subsystem Id: 0x144dc886. And your verbs from https://github.com/thesofproject/linux/issues/4055#issuecomment-2241846759 work great for me. Much better than all other variants I found on the internet: no difference between left and right speakers, rich sound. I'd appreciate it if you add my subsystem ID to your patch (if it's not late yet). Thanks a lot

joshuagrisham commented 1 week ago

Ok @hamfirst (and anyone else who is wanting to dig into this, for that matter :) ) -- I think I am getting somewhere :sunglasses:

First I needed to try and figure out exactly what I was looking for... I eventually stumbled on this page: https://github.com/acidanthera/AppleALC/wiki/Dumping-processing-coefficients

Though it is within the context of getting stuff to work on macOS, I think the stuff they are talking about here applies to Intel HDA regardless of the OS, so I was able to glean a lot of good information/understanding after reading this!

Then a little reminder from the Kernel docs (https://www.kernel.org/doc/html/latest/sound/hd-audio/notes.html#speaker-and-headphone-output):

Some Realtek codecs require special vendor-specific coefficients to turn on the amplifier. See patch_realtek.c.

Then I remembered that most of the time Realtek people seem to be asking everyone to run this RtHDDump.exe program so I dusted that off, booted into my Windows environment, and took a series of dumps while at different states: disabled all startup/shutdown/Windows sounds and then ran the program to take a dump after a fresh start, during playback of an audio file, after stopping, etc just so I could start comparing them to each other and to their context (e.g. what happens after a fresh start but before any audio is played? etc).

Bingo, yes, it makes more sense: in the RtHDDump files you can see all of these coefficient values (I think you were already alluding to these in your previous posts here -- took me some time to catch up, it seems! :smile_cat: )

I tried just manually "brute-force" setting all of the coefficient index values to what was shown in the Realtek dump files, but, of course, no dice... it seems like these payload sequences are actually necessary in able to "kick on" some things with our device.

Ok, so then I tweaked my QEMU tracing setup a bit more and added some timestamps to the log output for these special cases (see here: https://github.com/joshuagrisham/galaxy-book2-pro-linux/tree/main/sound/qemu) including a simple script to parse and reformat the verb-related stuff into CSV which I then put into some spreadsheets and did a lot of comparison against my own various traces plus a big comparison against your latest hda-verbs.txt file that you shared here.

From all of this, and especially with help of having timestamps on my trace information, and knowing a lot more about what each event meant (e.g. setting a coefficient index vs setting its value, reading a value at a given index, etc), I was able to get somewhere, I think!

One thing that is really good to know: all of my original verb files (which by the way seem to have made their rounds across the internet a bit, unfortunately!!) have a LOT of garbage in them. This is because I used a script called realtek-verb-tools/cleanverbs.py to originally parse and clean my list but the problem with our specific device here is that the script is leaving a bit of a mess because it was leaving in parts of sequences and removing other parts; at the end we did not have all of the actual events in the file and the final state was so messed up compared to what actually happened that I feel pretty lucky it ever worked for anyone to begin with :laughing: I think the key is that it kept in a lot of the coefficient "index setters" (0x500 0xXX) but then filtered away their various reads and writes, so we just ended up with a ton of extra coeff index setters that never did anything afterwards...

Now in my new script hda-verb-log-to-csv.py I am taking the log output from my patched QEMU and it is giving you everything in the result, including reads and the value of the coeff at the time the code in the patch runs (little risk that some race condition happens so the "response" might be updated again before the code reads it, but I think it is super minimal). Then what you can do is 1) filter for Wid 0x20 as that happens to be the widget we are interested for this device -- see Realtek Dump to get more info on this!, 2) "pair" all of the coeff index setters (0x500 0xXX) to their subsequent payload sequences (e.g. everything until the next 0x500 0xXX), and really start to look "event by event" what is really happening.

The conclusion that I came to is that the Windows driver appears to be doing the following:

  1. Read all coeff index values just to see what they currently are at startup, assuming that they store them all into some kind of structure or variables in their driver code. This is the long list of of index setters (0x500 0xXX) followed by a read (0x0C 0x00) you will see at the beginning of any trace. IMO I don't think we really care about these? Because we can "assume" it will always start at a fresh/default state?
  2. Set a few various general coefficient index values (don't seem to be speaker-specific??)

Then an "init" sequence starts for each speaker which seems to include:

  1. "Select" the speaker to act on by writing a per-speaker value to coeff index 0x22 (my device has 2 speakers, but it seems your device has 4 -- more on that later ;) )
  2. "Turn off" / disable the speaker (assuming to avoid any physical damage while the screwdrivers and hammers are banging around under the hood)
  3. Run a sort of "init" sequence against the speaker so it can be turned on
  4. Run a very long list of payloads in what appears to be some kind of per-speaker equalizer (e.g. volume adjustments per Hz step or dB step or something?? I tried to make sense of the values but am not familiar enough with sound technology to recognize anything e.g. common hertz or other values that might be coming into play here?)

When any audio is played, the driver then

  1. "Turns on" / enables all of the speakers

And when the audio is finished playing, the driver

  1. "Turns off" / disables all of the speakers

So knowing this, how do we make this all work?

I have created individual scripts to try and cover these scenarios in my repository now, just to get something down "on paper" to start with.

For # 1 I did nothing -- as said, if we can assume that the "defaults" from cold start are fine then we don't care to read them in? but maybe for different devices this will become important down the road?

For # 2 I have this script: init-initial-coef-values.sh but TBH I don't know if these are actually needed at all? I have now been running without them and though my coeff values do not match what comes from the Realtek dump for the indexes that are here.. sound seems to work so far just fine without them!

For # 3 + # 4 + # 5 I kind of packaged together in a per-speaker script.

Speaking of per-speakers, as I mentioned before, my device seems to have two speakers while your device seems to have 4. In my case, the speakers are recognized in the OS as "front left" and "front right" -- I assume this is from a "sound stage" perspective -- larger speakers that are closer to the screen are probably "front" and then smaller speakers closer to the edge of the laptop, I am assuming, are "back" (e.g. imagine your face is hovering above the middle of the keyboard, then to you the "front" speakers are the ones by the screen, and the "back" speakers are the ones that are just behind where your ears are, i.e. the ones near the front edge of the laptop). This is an assumption on my part but it is why I named my files the way that I did. With all of that said, here is what I think the speakers are, along with their coefficient index 0x22 value:

The scripts I currently have in my repo to "control" each of the speakers are broken into 3 parts: an "init" script (covers as said # 3, 4, and 5 above), plus a separate script to "turn on"/enable each speaker, plus a separate script to "turn off"/disable each speaker.

Front left (0x38)

Front right (0x39)

Back left (0x3C)

Back right (0x3D)

I have tested this a bit but not super-extensively on my NP950XED and it seems to be working quite well to just run the "init" script and then to turn them on. This means that I do not run the init-initial-coef-values.sh and I do not do any of the long list of per-speaker EQ values.

(Quick note on that: the main thing I noticed different between your device and mine was that my device seems to raise some values that yours does not, and then those similar values in the "back" left and right speakers are explicitly raised in your device instead; my assumption is that they are purposefully moving/emphasizing some frequencies or something to the smaller speakers and minimizing them on the other speakers to try and create some kind of sound effect??)

It is my opinion and hope that we can just let the speakers be at their "default" state when they come up (we just enable them and turn them on) and then any kind of EQ can happen by the user or other various audio applications, thereby having more of a "say" at the user-level and likely a better user experience to control these things. I think this depends a bit on if you actually see 4 individual speakers in your mixer or if you just see "left" and "right" ?

Anyway, if that assumption is correct, then my "take" on a minimum effort to init and enable the speakers from a fresh / cold start for my device (NP950XED) is with my scripts as follows:

./init-front-left.sh
./init-front-right.sh

./front-left-on.sh
./front-right-on.sh

And for your device (NP960QFG) is as follows:

./init-front-left.sh
./init-front-right.sh
./init-back-left.sh
./init-back-right.sh

./front-left-on.sh
./front-right-on.sh
./back-left-on.sh
./back-right-on.sh

As far as how to use this "on" vs "off" feature in the driver, I am not sure! Is it possible to have something execute before audio plays and after audio playback has halted? Otherwise maybe we would just need to kick the speakers to "on" and then leave them for the entire duration? (maybe run the payloads to turn them "off" if/when the driver is unloaded or something??)

Another side note in case it helps: looking at my QEMU traces again it seems like for shutdown the Windows driver is setting a list of coeff indexes to various values (looks to be most if not all of the same values from my # 1 above) and then ends with turning all of the speakers "off" as the very last thing that comes from the driver before the machine powers off

@hamfirst I realize this is "a lot" ( :laughing: ) but my hope is that we can really clean this up and get it down to a good critical mass for exactly what should be happening. One sentiment I have often read is that it is maybe a good idea to "touch only what you need to make it work." Even though we can see that some values don't match with the Windows driver, I am of the opinion that this would probably be best and certainly makes the driver cleaner and easier to understand if we just don't mess with them! This is why I am thinking it is good idea to just ignore the init-initial-coef-values.sh stuff as well as the per-speaker EQ stuff.

It also seems like we need some way in the driver to identify if the device has 2 vs 4 speakers, but maybe this is just as easily solved with a separate quirk for 2 vs 4 ? Otherwise this could be a case for reading some coeff indexes... ?

Compared to your update that has already been merged, it seems like it would change a fair amount; does it still feel ok with the way you have structured the patch_realtek.c updates and to have a separate "samsung helper" or is it reduced enough that maybe it makes sense to fold everything back into patch_realtek.c ?

Maybe not; it is still a fair amount of stuff even though I trimmed all of the EQ stuff out for now -- any thoughts? Otherwise I had considered to take a look myself but it might be a few days before I have time to dig into it...

hamfirst commented 6 days ago

Thanks Joshua

Oh boy that's a lot of analysis. Now that I've finally got my kernel patch working again (turns out the ipu6 module was just crashing me on startup), I can work on this again. Finding out that you don't need to send the EQ data for sound to work is a great discovery! I'll try removing that and see if it has any noticeable effect on sound quality. I think keeping the EQ data in samsung_helper.c and running it for specific audio devices makes sense, but having a trimmed down version that just gets the audio working in patch_realtek.c would be a good way to proceed.

I submitted a patch over the weekend which just renamed a bunch of stuff from samsung to a more generic name, but I got the feedback from Takashi Iwai that he didn't feel like it was justified. Basically - we don't know in particular what hardware this code is actually interfacing with. The fact that it's working with non-Samsung laptops like the LG-Gram (and potentially Razor Blade and others), to me indicates that this might be some 3rd party hardware outside of Samsung or Realtek. At the very least, this might give us some more avenues to get more info about it if Samsung and Realtek are unresponsive. Just having a name for whatever the hardware is would be enough to clean things up a lot.

As far as determining if there are two speakers instead of four, I think that would be a good next step. Did you notice any proc coefficients that might indicate that? I'll post a copy of what I get for coefficients when I get a sec so we can compare. The more interesting question for me though - is why they are addressed by 0x38, 0x39, 0x3c and 0x3d.

joshuagrisham commented 6 days ago

Thanks @hamfirst yes here are my coefficient values for widget 0x20 without running any of our stuff (also including that I blacklisted the realtek codec etc just to try and be sure nothing would change anything, but they are actually same with and without blacklist except that I am missing a mic amp and the name becomes Realtek Generic instead of Realtek ALC298)

Node 0x20 [Vendor Defined Widget] wcaps 0xf00040: Mono
  Processing caps: benign=0, ncoeff=150
    Coeff 0x00: 0x4013
    Coeff 0x01: 0xa5a8
    Coeff 0x02: 0x8e95
    Coeff 0x03: 0x0002
    Coeff 0x04: 0xaf67
    Coeff 0x05: 0x2fe0
    Coeff 0x06: 0x1100
    Coeff 0x07: 0x0200
    Coeff 0x08: 0x200f
    Coeff 0x09: 0xd010
    Coeff 0x0a: 0x0100
    Coeff 0x0b: 0x0000
    Coeff 0x0c: 0x011e
    Coeff 0x0d: 0x2800
    Coeff 0x0e: 0x6f40
    Coeff 0x0f: 0x0022
    Coeff 0x10: 0x0c20
    Coeff 0x11: 0x7418
    Coeff 0x12: 0xe3c0
    Coeff 0x13: 0x242f
    Coeff 0x14: 0x2c60
    Coeff 0x15: 0x8ccc
    Coeff 0x16: 0x0018
    Coeff 0x17: 0xff00
    Coeff 0x18: 0x0003
    Coeff 0x19: 0x0017
    Coeff 0x1a: 0xc000
    Coeff 0x1b: 0x0068
    Coeff 0x1c: 0x0000
    Coeff 0x1d: 0x0000
    Coeff 0x1e: 0x0000
    Coeff 0x1f: 0x0000
    Coeff 0x20: 0x0020
    Coeff 0x21: 0x8847
    Coeff 0x22: 0x0000
    Coeff 0x23: 0x0000
    Coeff 0x24: 0x0000
    Coeff 0x25: 0x0000
    Coeff 0x26: 0x3000
    Coeff 0x27: 0x0000
    Coeff 0x28: 0x0000
    Coeff 0x29: 0x0000
    Coeff 0x2a: 0x0f08
    Coeff 0x2b: 0x0d10
    Coeff 0x2c: 0x000d
    Coeff 0x2d: 0x4020
    Coeff 0x2e: 0x0250
    Coeff 0x2f: 0x404c
    Coeff 0x30: 0x2421
    Coeff 0x31: 0x0000
    Coeff 0x32: 0x0000
    Coeff 0x33: 0x0208
    Coeff 0x34: 0x5614
    Coeff 0x35: 0x1aa5
    Coeff 0x36: 0x6ac0
    Coeff 0x37: 0x1000
    Coeff 0x38: 0x04c1
    Coeff 0x39: 0x7418
    Coeff 0x3a: 0xe800
    Coeff 0x3b: 0x223e
    Coeff 0x3c: 0x0c60
    Coeff 0x3d: 0x8ccc
    Coeff 0x3e: 0x0858
    Coeff 0x3f: 0x00ff
    Coeff 0x40: 0x41ff
    Coeff 0x41: 0xff00
    Coeff 0x42: 0x0003
    Coeff 0x43: 0x0520
    Coeff 0x44: 0x0000
    Coeff 0x45: 0x0000
    Coeff 0x46: 0x0300
    Coeff 0x47: 0x8010
    Coeff 0x48: 0x0000
    Coeff 0x49: 0x4004
    Coeff 0x4a: 0x1012
    Coeff 0x4b: 0x3401
    Coeff 0x4c: 0x0910
    Coeff 0x4d: 0x0400
    Coeff 0x4e: 0x43fe
    Coeff 0x4f: 0xb029
    Coeff 0x50: 0x3000
    Coeff 0x51: 0x9420
    Coeff 0x52: 0x50c9
    Coeff 0x53: 0x0496
    Coeff 0x54: 0xcf00
    Coeff 0x55: 0x0000
    Coeff 0x56: 0x001e
    Coeff 0x57: 0x0004
    Coeff 0x58: 0x0000
    Coeff 0x59: 0x0000
    Coeff 0x5a: 0x0000
    Coeff 0x5b: 0x0000
    Coeff 0x5c: 0x0000
    Coeff 0x5d: 0x0030
    Coeff 0x5e: 0x3fff
    Coeff 0x5f: 0x1ca7
    Coeff 0x60: 0x0f99
    Coeff 0x61: 0xad65
    Coeff 0x62: 0xafe7
    Coeff 0x63: 0x1b02
    Coeff 0x64: 0x7f01
    Coeff 0x65: 0x0095
    Coeff 0x66: 0x0404
    Coeff 0x67: 0x1110
    Coeff 0x68: 0x1016
    Coeff 0x69: 0x273f
    Coeff 0x6a: 0x30aa
    Coeff 0x6b: 0x7006
    Coeff 0x6c: 0xfc05
    Coeff 0x6d: 0x6908
    Coeff 0x6e: 0x110a
    Coeff 0x6f: 0x0010
    Coeff 0x70: 0x60d8
    Coeff 0x71: 0x00d4
    Coeff 0x72: 0xc23a
    Coeff 0x73: 0xaa28
    Coeff 0x74: 0x8001
    Coeff 0x75: 0xc802
    Coeff 0x76: 0x0000
    Coeff 0x77: 0x207e
    Coeff 0x78: 0xa000
    Coeff 0x79: 0x6ac0
    Coeff 0x7a: 0x0000
    Coeff 0x7b: 0x0000
    Coeff 0x7c: 0x0000
    Coeff 0x7d: 0x03c8
    Coeff 0x7e: 0x2028
    Coeff 0x7f: 0xf008
    Coeff 0x80: 0x0000
    Coeff 0x81: 0x5fc0
    Coeff 0x82: 0x0408
    Coeff 0x83: 0x1641
    Coeff 0x84: 0x31bb
    Coeff 0x85: 0x0000
    Coeff 0x86: 0x0193
    Coeff 0x87: 0x0193
    Coeff 0x88: 0xa0a0
    Coeff 0x89: 0x0000
    Coeff 0x8a: 0x0008
    Coeff 0x8b: 0x06fc
    Coeff 0x8c: 0x3aff
    Coeff 0x8d: 0x0000
    Coeff 0x8e: 0x0000
    Coeff 0x8f: 0x1008
    Coeff 0x90: 0x002f
    Coeff 0x91: 0x0077
    Coeff 0x92: 0x0000
    Coeff 0x93: 0x0cd0
    Coeff 0x94: 0x0001
    Coeff 0x95: 0x4aa0

But I still wonder -- does your device actually show 4 speakers or just 2? E.g. if you run amixer how does the output look?

For my device it is this:

Simple mixer control 'Master',0
  Capabilities: pvolume pswitch pswitch-joined
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 65536
  Mono:
  Front Left: Playback 24195 [37%] [on]
  Front Right: Playback 24195 [37%] [on]
Simple mixer control 'Capture',0
  Capabilities: cvolume cswitch cswitch-joined
  Capture channels: Front Left - Front Right
  Limits: Capture 0 - 65536
  Front Left: Capture 65536 [100%] [on]
  Front Right: Capture 65536 [100%] [on]

(and otherwise I am not sure how else we can check this via /sys/ or /proc/ etc and would just need to look into it a bit .. )

hamfirst commented 5 days ago

Here's my copy of the coefficients (default without the patch): DefaultCoef.txt

And here's what amixer gives:

Simple mixer control 'Master',0
  Capabilities: pvolume pvolume-joined pswitch pswitch-joined
  Playback channels: Mono
  Limits: Playback 0 - 127
  Mono: Playback 127 [100%] [0.00dB] [on]
Simple mixer control 'Headphone',0
  Capabilities: pvolume pswitch
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 127
  Mono:
  Front Left: Playback 76 [60%] [-25.50dB] [off]
  Front Right: Playback 76 [60%] [-25.50dB] [off]
Simple mixer control 'Speaker',0
  Capabilities: pvolume pswitch
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 127
  Mono:
  Front Left: Playback 88 [69%] [-19.50dB] [off]
  Front Right: Playback 88 [69%] [-19.50dB] [off]
joshuagrisham commented 4 days ago

Hi @hamfirst is this Coef file with your updated patch_realtek.c running ? As it seems like your values match very closely to what Windows has and I would be sort of surprised if your device were like this out of the box without touching anything :smile:

If so, is it possible for you to either blacklist or boot to a different kernel without the updated codec module and then get a new list of values?

Also it looks like per your amixer there are only 2 speakers identified on the Speaker mixer control. Is this the same in Windows and/or what does it say when you run RtHDDump.exe ? It would be interesting to compare all of the output amplifiers that you have there as well I think!

joshuagrisham commented 4 days ago

Also just to add... I got a bit of an itch to take a look at this and thought it would be fun to try and re-think from scratch based on the information I posted above (the idea that we would want a one-time more simplified init of speaker amps without any of the EQ stuff, and then we should enable / disable the speakers based on their usage just like how it works in the Windows driver).

Absolutely nothing at all against everything you have already done @hamfirst and also I would not have been able to put this together without all of the info you have also added here but I just thought it was an interesting experiment nonetheless :smiley:

With that said, here I have attached a first attempt at a patch (it is going against https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tag/?h=sound-6.11-rc6 ) where the idea is to re-use as much as is possible already existing in patch_realtek.c but then to implement the scenarios I posted above (one-time init of speaker amps and then enable or disable speaker amps only when audio is going to be played).

alc298_samsung_v2.patch.txt

Basically, as said, I tried to just think from a clean slate how it might make sense to try and do this.. so I ended up dropping the samsung_helper entirely and just added a few extra things to the existing patch_realtek.c -- hopefully it makes sense!

Regarding how to select 2 speakers vs 4 was a bit clumsy .. I tried a few approaches including having two totally separate quirk names but felt it was a lot cleaner to just drive everything from the same quirk and have yet-another-quirk-table in the patch ( :crying_cat_face: ) -- it would be interesting if there were another way to handle this :)

But @hamfirst even if it would make sense to try something like this attached patch file, I think it depends a bit on your test as well-- does it work for your device to just enable the speaker amps using my above scripts, and is the sound sufficiently "full" or do you feel that something is lacking?

Also still curious about your clean coef dump and possible RtHDDump from Windows :)

For me it actually works well -- so far so good. Will try it out a few days and see if I run into any problems!

(it can also be interesting to add a few codec_info() kind of messages in the code to see when the amps are enabled vs disabled etc in case you are curious to see how/what is happening!)

joshuagrisham commented 4 days ago

Also just wanted to add something else I tweaked regarding naming of ALC298_FIXUP_SAMSUNG_AMP2 vs ALC298_FIXUP_SAMSUNG_AMP_V2 -- my take on this was that there are some existing quirks which are called things like _MIC2 but in those cases it is actually the 2nd microphone on a device, actually rightfully labeled "Mic 2". I decided to do a super minor tweak to this name so that it did not suggest that it was for the "2nd" amp on ALC298 Samsung but instead was still just the main amp, though a newer version of how it should be handled. Hope that makes sense, as well!

hamfirst commented 3 days ago

Ah you're right. I rebooted into my production kernel instead of doing a full power cycle.

Here's the coefficients after doing a proper shutdown: DefaultCoef2.txt

joshuagrisham commented 3 days ago

Thanks @hamfirst Sadly, they are exactly the same :laughing: But I would be very interested to see an RtHDDump.exe dump if you might be able to create one. Also very curious if/how it works for you in your "production kernel" with just init-ing and enabling the speaker amps? :angel: Both "does it work" at all plus does the sound still sound ok/"as full" as without doing any of the EQ stuff?

Like this I guess?

./init-front-left.sh
./init-front-right.sh
./init-back-left.sh
./init-back-right.sh

./front-left-on.sh
./front-right-on.sh
./back-left-on.sh
./back-right-on.sh
joshuagrisham commented 1 day ago

I managed to borrow a Galaxy Book3 Pro 360 (subsystem id 0x144dc1ca) and got a RtHDDump from it. The short version is that, from what I could tell, all of the output amplifier and pretty much everything is exactly the same as with my device even though the Book3 Pro 360 has 4 speakers (https://youtu.be/ApzQ5SxtzxI?feature=shared&t=636) and my device has 2 (https://www.youtube.com/watch?v=Bk2DiNBbljM&t=303s).

Also in Windows it is only 2 speaker channels (left and right) on this device even though there are 4 physical speakers.. but this also lines up with what I could see in the RtHDDump.

At the end of the day it means that it is not really possible to control EQ "per speaker" in the user space unless we do it in the driver like this, but my hope is that the difference is very negligible but instead what you get in Windows is more of a biased "sound-stage flavor" that Samsung/Realtek has tried to create as opposed to having the driver give something more of a flat sound and then using software to control EQ after the fact.

I planned to try some of these other tests with this laptop as well maybe in the coming day or two just to see if there is a noticeable change in volume or quality -- will try to measure as best I can with whatever equipment and software I have lying around here for a more objective comparison both with and without the EQ stuff .... unless anyone else beats me to this or has any better ideas? :smile:

sn99 commented 1 day ago

@hamfirst

I can confirm that the patch works for:

cat /proc/asound/card0/codec#0 | grep Subsystem
Subsystem Id: 0x18540488

Model: LG gram 16 (40.64) Ultra-lightweight with 16:10 IPS Anti glare Display and Intel® Evo 13th Gen. Processors SKU: 16Z90R-G.CH74A2

Although it sounds a little muffled, I remember there being some software on windows used to boost audio but it had been long since I used it.

Anyhow your patch seems to work on ny device too and could be included upstream for my device ID.

joshuagrisham commented 17 hours ago

Hi again!

As mentioned I was able to get my hands on a Galaxy Book3 Pro 360 (with codec subsystem 0x144dc1ca) and managed to do a round of testing with it. Interesting results!

TL;DR

Test details

For my testing I set up a separate PC with a Creative HyperX Quadcast (not exactly best fit for this, but not bad either!) and Audacity on a flat dining table with nothing else on it (no cloth etc) and recorded the same song coming from the speakers of the GB3. I recorded all of the tracks in mono as I was interested in the total result of the "sound" and did not want to mix up what the mic picked up for left vs right etc... The song I chose was Sufjan Stevens's "Carrie and Lowell" partly because I like it and partly because it has quite a lot going on and I thought would be a pretty good test of different ranges and combo of soft sounds + pops, sustained bass, mids, highs, etc. Maybe there is a better way, but that was what I did :) I also took excruciating care to try and ensure that I did not move or touch anything between tests and always had the same environment for each test.

The different scenarios that I tested were a combination of Windows (as a reference) plus Linux using Ubuntu 24 and just running various hda-verb scripts to "mimic" different scenarios. I chose the volume of 60% as the base as it was this volume from Windows where it was just before the audio started to clip/crack a bit, and it seemed like it was quite loud (actually too loud, IMO!). Here were my different scenarios:

I also did a pass where I took the two 80% volume tests in Linux (both with and without EQ payloads) and recorded with balance moved full to LEFT, and again with balance moved full to RIGHT, so that I could test and see if there was any measurable volume difference in the left and right channels

Anecdotally I can just mention that the sound seemed quite "clear" and good in Linux and I did not really notice a huge difference between the different modes just while I was doing the recordings, other than the noticeable volume boost difference and how in Windows the sound seems a bit "overblown" and the sound even gets close to cracking at higher volumes.

Once I grabbed all of these recordings, I took some time to clean up and move around them in Audacity so that they all lined up perfectly with each other, so that it would be easier to pull measurement samples from the exact same parts of the song as well as it was a nice way to be able to A/B them and just try to "hear" if I could notice any difference. Pretty cool test! Here is what it looked like once I got everything lined up:

speakertest-audacity

I then wanted to compare just "overall volume" so I ran Audacity's RMS Measurement analysis over the entire track as well as specific windows on the track for each test and then plotted the results in a table and ran the values through a formula for "loudness difference" which I derived from here: https://sengpielaudio.com/calculator-thd.htm

The end result was that the volume in Linux at 80% was really close to the volume in Windows using "Dolby Atmos" at 60%... here is a bit more what the results looked like:

speakertest-volumes

Something else which you can see in this image is the comparison of LEFT-only vs RIGHT-only -- I would say that they are both equal so there did not seem to be a volume difference created when "just" running init+enable vs running the full set of payloads including the EQ stuff -- volume is balanced both ways!

I also had a bit of a reflection that the "non-EQ'd" version of the sound in Linux sounded a bit more "clear" but the highs seemed like they were a little bit overblown and could be slightly harsher than the other scenarios, so I thought it would be worth to try and figure out a way to look at the data per-frequency and per range of frequencies, to see what is getting emphasized or reduced across different ranges (i.e. what is the actual effect of all of these EQ differences?). So I ran Audacity's Plot Spectrum tool with a size of 32768 for each different track, one at a time, and dumped the results into a spreadsheet, then created a pivot table to try and figure out a way to "aggregate" the data to give a more clear and summarized picture of the differences. Here is what I came up with:

speakertest-freq-differences

Essentially in this table, I am trying to take the average dB difference between my "reference" (Windows with "Dolby") for all frequencies in these different frequency ranges, and then running the same "loudness difference" formula on average per-frequency dB level difference.

Some interesting observations here:

  1. My "reflection" I mentioned above is backed by the data -- the Presence range was in fact a bit louder compared to Windows and all other scenarios when I init/enabled the speaker amps but did not run the EQ payloads
  2. Windows seems to be dropping sub bass a bit (or maybe in a Chrome profile or something since I listed to the audio using Chrome??), while boosting bass and lower midrange a bit as compared to what we are seeing in Linux
  3. Higher ranges were also a fair amount lower on the full script with EQs vs turning on the speaker amps without running the EQ payloads. I think this also backs up the observation that the sound is a bit "muddier" when running the EQ values compared to not.

When I just A/B the sound listening through my Sony MDR-7506 studio monitors then I do actually prefer the sound from the speakers without running the EQ payloads (init+enable only), as I find it more clear and you can hear more detail, but would definitely want to try and smooth out that presence range a bit so the highs ar just a touch less "sharp" -- I think something like Easy Effects would work perfectly for this (as well as to make any other adjustments desired on a per-user basis?).

One thing I was very curious about was if there was a major difference in the 4 speaker "sound" if you did not run the EQ, but to be honest I do not think there was (and, as said, to my ears I actually like the sound better without running the EQ stuff, plus the frequency levels are actually closer to what I saw in Windows with the "Dolby Atmos" mode enabled).

And to wrap up I just thought to add that if anyone is super interested, I can try to share my spreadsheet and even the audio files so that you can do your own analysis and A/B listening to see if you have any preference or can spot anything else that I might have missed?

joshuagrisham commented 17 hours ago

@hamfirst I am not sure if you have any thoughts on all of the above, but for me all of this kind of validated what I wondered/suspected a few posts back--I am of the opinion that I think it is better to just init and enable the speakers instead of running any of these EQ payloads. Then we can run some other kind of EQ tool to tweak the sound to each individual user's liking.

What do you think about this @hamfirst ? I actually have a kernel patch prepared which is an update to the one I already posted which refines/simplifies it a bit more, including removing the need to have multiple quirk lists (everything just goes from the original quirk list table). If you would not have any major objections or have any other ideas then I can prep it for sending to the mailing list in the next day or two... or is this whole thing crazy? Thoughts?

A "pet project" for someone that might be interested would be to really figure out what all of these EQ payloads need to look like in order to steer the values in various ways, and create a tool to enable hardware-based EQ adjustments and profiles for Realtek speaker amps in Linux -- would certainly be a challenge!