Improve WM8960 sound quality

[probonopd]

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]

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]

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

[rsta2]

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

[probonopd]

Note to self: Bisect this...

[gdaszuta]

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.

[probonopd]

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.

[gdaszuta]

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).

[rsta2]

@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.

[swausd]

I am experimenting using circle with a Pi Zero 2 W and a keyestudio WM8960 Hat for a SDR receiver project. The datasheet of the WM8960 states that setup of the clock depends (1) on external clocking and (2) on the desired sample rate. The WM8960 boards from Adafruit, Waveshare, Seedstudio and the likes are using 24 MHz external crystals. I understand that for 48 KHz sample rate we have to derive an internal clock of 12.288 MHz. For 44.1 KHz a clock of 11.2896 MHz has to be used. To get this internal clock from the external 24 MHz clock, we have to use the PLL with different register setups (see examples in the datasheet Rev 4.4, Page 64/65, Table 45 PLL Frequency Examples). I don't see any provisions in the circle driver for changing the WM8960-setup in runtime, so we have to decide on a fixed sample rate. I am using 48 KHz for bidirectional I2S with this register setup for clocking (just started, no longtime experience):

SHIFT_BIT( 4, 0x005), // CLOCKING_1: select PLL, SYSCLKDIV=2, ADCDIV=0, DACDIV=0
SHIFT_BIT( 8, 0x1c4), // CLOCKING_2: BLKDIV=4, DCLKDIV=16
SHIFT_BIT(26, 0x???), // PWR_MGMT_2: bit 0 has to be set to 1 to enable PLL, other bits control DAC, mixer, headphone, class D amp etc.
SHIFT_BIT(52, 0x018), // PLL_N: PLL_PRESCALER_DIV=2, enable PLL_MODE_FRACTIONAL, PLL_N=8
SHIFT_BIT(53, 0x031), // PLL_K_1 K=0x3126e8
SHIFT_BIT(54, 0x026), // PLL_K_2
SHIFT_BIT(55, 0x0e8), // PLL_K_3

A 44.1 KHz setup is used in the very good documented WM8960 Arduino library and the examples from SPARKFUN (see: https://github.com/sparkfun/SparkFun_WM8960_Arduino_Library, especially Example_8).

So all in all looking at Linux register dumps helps understanding what has to be done but cannot be copied without exact knowledge of the used hardware (external clock) or the current use case (sample rate).

By the way: A big "Thank You" to the developers of the fantastic circle / circle-stdlib environment!

[rsta2]

Thanks for bringing light into this and for appreciating our projects! How it seems the sound driver for the WM8960 codec needs some additional configuration information (sample rate, internal/external clocking), which is currently not available in the class CWM8960SoundController. I'm thinking about, how this information can be provided there.

[swausd]

I think this is not an easy task, as there are at least two abstraction levels:

1. On the higher CSoundBaseDevice level only parameters of general interest like sample rate and data flow directions (read/write) are relevant.

2. On the CSoundController level more "esoteric" parameters like for DSP- and sound-effects, amplifier setups or hardware internal routing should be accessible. There are zillions of different setup possibilities.

To minimize the effort and be flexible at the same time, only a minium standard setup with a given sample rate and hardware implementation in mind could be offered - like the one already implemented. To enable even "esoteric" use cases, a deviation from this standard could be offered via a "WriteRegister" method in CSoundController. Like a "Toolkit" for the knowledgeable user. This could be reached on application level using the CSoundController::GetController method. The user then has the flexibility (and responsibility) to implement what ever is on his mind in "user space". See the examples in the above mentioned SPARKFUN lib for all the interesting use cases and register manipulations of a WM8960.

For me the special charm of circle lies in direct hardware access with near realtime response. So a little tinkering is OK.

[rsta2]

I think, I understand, what you want to say. This is not a perfect solution, but it seems, a perfect solution does not exist. There are so many different audio interfaces out there, and I cannot expect, someone will write Circle drivers for them. There was already an attempt to do so for a few interfaces and it was not completed. Thus for WM8960-based sound cards, if there would be a default setup, which works in most cases and the possibility to set user parameters with CSoundController::WriteRegister(), it could be a good compromise. Give me until Tuesday to build an opinion on this.

[swausd]

No hurry, I get along. And there doesn't seem to be that much interest in this subject anyway.

[rsta2]

I think, with your info above it should be possible to modify the WM8960 sound controller driver and the Circle sound library so that WM8960 sound cards work with the supported sample rates and in master or slave mode without adding much overhead. Perhaps I will order a WM8960 sound card to implement this.

Having a WriteRegister() method would not be enough, because the application cannot know, which I2S sound card is currently used, to be able to poke the device registers. So there must be another method, which offers this information. This would break the currently intended design of the sound controller interface, which hides this information from the application.

On the other hand you already can directly write the device registers from your application, if you need this, by using the I2C master object and the known I2C address 0x1A of the WM8960. Adding WriteRegister() is not necessary.

[swausd]

Thank you for the information that there is no need for the WriteRegister() method. I did not see how to get to the device I2C from application level. I still have to work on my object oriented programing skills. I am from the team "you can write FORTRAN programs in any language" ;-)