circle icon indicating copy to clipboard operation
circle copied to clipboard

Improve WM8960 sound quality

Open probonopd opened this issue 2 years ago • 8 comments

I noticed that when using Circle, the WM8960 sounds a lot noisier than when using Linux. Possibly we can improve things by tweaking the default configuration of the i2c registers?

https://github.com/matemaciek/circle/blob/14a120ad68ca668b01bc24cff15afff4e841fc07/lib/i2ssoundbasedevice.cpp#L473-L515

This is a dump from a pristine (no config changes made) Raspberry Pi OS installation with the WM8960 driver from https://github.com/waveshare/WM8960-Audio-HAT:

$ sudo su
$ cat /sys/kernel/debug/regmap/1-001a/registers

00: 0127
01: 0127
02: 016d
03: 016d
04: 0049
05: 0008
06: 0000
07: 0002
08: 01c9
09: 0000
0a: 01ff
0b: 01ff
10: 0000
11: 007b
12: 0100
13: 0032
14: 0000
15: 01c3
16: 01c3
17: 01c0
18: 0000
19: 0140
1a: 0001
1b: 0000
1c: 0008
1d: 0000
20: 0138
21: 0138
22: 0170
25: 0150
26: 0000
27: 0000
28: 016d
29: 016d
2a: 0040
2b: 0000
2c: 0000
2d: 0070
2e: 0020
2f: 0000
30: 0002
31: 0037
33: 00a4
34: 0036
35: 0024
36: 00dd
37: 002f

When one unmutes Noise Gate Switch (by default it is muted) in alsamixer and sets Noise Gate Threshold to ~80%, then the following changes:

14: 00c9

https://github.com/waveshare/WM8960-Audio-HAT/blob/master/wm8960_asound.state defines no less than 58 controls that could theoretically be tweaked for this codec. But possibly using the default Linux driver configuration could already improve sound quality over what we currently have?

cc @matemaciek

probonopd avatar May 28 '22 09:05 probonopd

So I am wondering whether the following could work?

	// based on https://github.com/waveshare/WM8960-Audio-HAT
		SHIFT_BIT(0x00, 0x127),
		SHIFT_BIT(0x01, 0x127),
		SHIFT_BIT(0x02, 0x16d),
		SHIFT_BIT(0x03, 0x16d),
		SHIFT_BIT(0x04, 0x049),
		SHIFT_BIT(0x05, 0x008),
		SHIFT_BIT(0x06, 0x000),
		SHIFT_BIT(0x07, 0x002),
		SHIFT_BIT(0x08, 0x1c9),
		SHIFT_BIT(0x09, 0x000),
		SHIFT_BIT(0x0a, 0x1ff),
		SHIFT_BIT(0x0b, 0x1ff),
		SHIFT_BIT(0x10, 0x000),
		SHIFT_BIT(0x11, 0x07b),
		SHIFT_BIT(0x12, 0x100),
		SHIFT_BIT(0x13, 0x032),
		SHIFT_BIT(0x14, 0x000),
		SHIFT_BIT(0x15, 0x1c3),
		SHIFT_BIT(0x16, 0x1c3),
		SHIFT_BIT(0x17, 0x1c0),
		SHIFT_BIT(0x18, 0x000),
		SHIFT_BIT(0x19, 0x140),
		SHIFT_BIT(0x1a, 0x001),
		SHIFT_BIT(0x1b, 0x000),
		SHIFT_BIT(0x1c, 0x008),
		SHIFT_BIT(0x1d, 0x000),
		SHIFT_BIT(0x20, 0x138),
		SHIFT_BIT(0x21, 0x138),
		SHIFT_BIT(0x22, 0x170),
		SHIFT_BIT(0x25, 0x150),
		SHIFT_BIT(0x26, 0x000),
		SHIFT_BIT(0x27, 0x000),
		SHIFT_BIT(0x28, 0x16d),
		SHIFT_BIT(0x29, 0x16d),
		SHIFT_BIT(0x2a, 0x040),
		SHIFT_BIT(0x2b, 0x000),
		SHIFT_BIT(0x2c, 0x000),
		SHIFT_BIT(0x2d, 0x070),
		SHIFT_BIT(0x2e, 0x020),
		SHIFT_BIT(0x2f, 0x000),
		SHIFT_BIT(0x30, 0x002),
		SHIFT_BIT(0x31, 0x037),
		SHIFT_BIT(0x33, 0x0a4),
		SHIFT_BIT(0x34, 0x036),
		SHIFT_BIT(0x35, 0x024),
		SHIFT_BIT(0x36, 0x0dd),
		SHIFT_BIT(0x37, 0x02f)

probonopd avatar May 28 '22 09:05 probonopd

Not quite... stays silent... why?

probonopd avatar May 28 '22 10:05 probonopd

Because I do not have WM8960-based sound card, I would need a tested patch to include it in Circle.

rsta2 avatar May 29 '22 07:05 rsta2

Note to self: Bisect this...

probonopd avatar May 29 '22 17:05 probonopd

Hello @probonopd

So I just started poking around and comparing whatever is going on between circle and waveshare linux driver (from https://github.com/waveshare/WM8960-Audio-HAT/blob/master/wm8960.c) , and found some quite interesing things:

  1. Noise Gate (14h) shouldn't have anything to do with noise issues, according to the datasheet it's used mostly during recording;
  2. 28h and 29h value should be the same, it's volume output and for whatever reason it's driving me nuts;
  3. 33h have some differences in Gain settings, but it shouldn't matter (it's noisy on headphones too); And so far the most promising avenue of the investigation:
  4. 04h Clocking 1 - 0x00 Linux defaults, 0x01 my Linux dump, 0x049 your linux dump (so in all cases clock source PLL, in my pi zero clock divider for both DAC/ADC 1.0, for you 1.5x), 0x01 Circle (0 = SYSCLK derived from MCLK; 1 = SYSCLK derived from PLL output);
  5. 08h Clocking 2 - 0x01c0 Linux defaults (default value from datasheet), also 0x1c0 in my dump, for probonopd it was 0x1c9 (so mine BLCK Frequency = SYSCLK (0000), your SYSCLK/12 (1001), not set in Circle, so we should be on default value (00x1c0 / 0000);
  6. 34h PLL N - Linux defaults 0x08, Circle 0x027, my dump 0x08, 0x36
  7. 35h PLL K1 - Linux defaults 0x31, Circle 0x86, my dump 0x31, 0x24
  8. 36h PLL K2 - Linux defaults 0x26, Circle 0xC2, my dump 0x26, 0xdd
  9. 37h PLL K3 - Linux defaults 0xE9, Circle 0x26, my dump 0xe9, 0x2f

For 34h-37h linux defaults (and whatever I was able to dump right after boot-up sequence) are 100% consistent with the datasheet defaults.

So basically we have totally different clock source and clock divider settings – I don't really have experience in DAC, but I suspect that clock base mismatch can be source of some extra noise.

I am leaving this comment here because I am starting to become a bit dizzy, I will try to verify if setting any of the above to defaults from Waveshare driver helps as soon as I'll be able to build working MiniDexed – when I tried to do so, it crashed during boot up sequence.

gdaszuta avatar Mar 21 '23 18:03 gdaszuta

Thank you for your investigation @gdaszuta. I have since moved on to another DAC, but I am sure this will be helpful in case someone would like to revisit this topic.

probonopd avatar Mar 21 '23 19:03 probonopd

Ok, so I was able to test a build and I think that it's doing something:

diff --git a/lib/sound/wm8960soundcontroller.cpp b/lib/sound/wm8960soundcontroller.cpp
index 700ca2d..88a5141 100644
--- a/lib/sound/wm8960soundcontroller.cpp
+++ b/lib/sound/wm8960soundcontroller.cpp
@@ -61,10 +61,11 @@ boolean CWM8960SoundController::InitWM8960 (u8 uchI2CAddress)
                SHIFT_BIT(47, 0x03C),
                // Clock PLL
                SHIFT_BIT(4, 0x001),
-               SHIFT_BIT(52, 0x027),
-               SHIFT_BIT(53, 0x086),
-               SHIFT_BIT(54, 0x0C2),
-               SHIFT_BIT(55, 0x026),
+               SHIFT_BIT(8, 0x1C0),
+               SHIFT_BIT(52, 0x008),
+               SHIFT_BIT(53, 0x031),
+               SHIFT_BIT(54, 0x026),
+               SHIFT_BIT(55, 0x0E9),
                // ADC/DAC
                SHIFT_BIT(5, 0x000),
                SHIFT_BIT(7, 0x002),

But to be honest I cannot be 100% sure that it isn't the placebo effect, in couple days I'll try to dig up oscilloscope and do some tests to make sure that this change makes any sense (or if it doesn't, it the issue isn't some ratio of volume levels between internal amps in the chip).

gdaszuta avatar Mar 22 '23 09:03 gdaszuta

@gdaszuta It would be great, if you can provide a patch, which reliable solves this issue. Unfortunately I cannot test this here, because I do not have this DAC, so I have to rely on your judgement.

rsta2 avatar Mar 22 '23 11:03 rsta2