Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for WM8978 i2c dac #89

Merged
merged 11 commits into from
Apr 10, 2021
Merged

Conversation

wizmo2
Copy link

@wizmo2 wizmo2 commented Apr 6, 2021

Per discussions on squeezebox thread, the WM8978 DAC uses a 9bit I2C format and does not support I2C read (needs simulating). It also requires an external MCLK signal, that is supplied by the ESP32 system clock.
This code adds a new driver based on the existing external format.
I included a default dac_controlset to make it simple for beginners. (dac_config is "model=WM8978,bck=33,ws=25,do=26,sda=19,scl=18,i2c=26" for the TTGO TAudio/ESP-SNOW board.)
I also added JSON support for headset and speaker on/off, but had to disable as the jack-handler is currently on chip only and none of the external GPIO is exposed to allow the ESP to monitor.

I uploaded a quick (demo video) that includes full LED control too!. I'm going to do a separate draft pull request. (a). as I want to clean up the code, and (b). I'll need some help/advise on the best way to properly implement it (I'm thinking it should be integrated into the standard display module as an additional screen)

@sle118 sle118 requested a review from philippe44 April 9, 2021 02:39
@philippe44
Copy link
Collaborator

Thanks but that PR made me think that it was time to refactor a bit the DAC "framework" as I see all drivers doing the same thing over and over. I'm a bit obsessed by minimizing code (I really hate duplication) and I like to do these things early enough so that it does not come un-manageable a year or two later (that's how you end up with spaghetti code).

Anyway, I put together some common function and re-wrote the TAX5xxx, AC101 and I2S to support that, as examples. I think that will make your driver much smaller. Let me know if this is a complicated change and I'll help. Also, the use of the JSON should really be reserved for dac_external. Built-in driver should write directly by register. I can add a buffer write if needed, otherwise you can create a table with a loop.

I would suggest that you write your own adac_write_byte/word function that uses the original adac_write_byte/word and writes the shadow registers and then add an adac_read_byte/word that simply read in the shadow registers.

@wizmo2
Copy link
Author

wizmo2 commented Apr 9, 2021

No problem, will have a look this weekend.

My latest headache is trying to get a CS-less SPI 240x240 ST7789 display working. (probably the same one that schmurtz was using - I seem to always pick the challenging parts!). I've tested with other code, but I'm not getting anything with the same IO config here. Its possible I missing something on the LMS setup. What is the expected operation, for example should I see a splash screen regardless?

@philippe44
Copy link
Collaborator

Yes, you should see a splashscreen at boot no matter what. It's strange b/c ST7789 works well, I have one on one of my test systems. Did you connect RST or pull it up? It's very important. Did you alos make sure that backlight is activated?

@philippe44
Copy link
Collaborator

philippe44 commented Apr 9, 2021

Here is a proposal - you need to tweak a few things still (like the WM address) and verify that it works ... If you could give it a go, then I'd release this weekend a new FW version

wm8978.zip

@wizmo2
Copy link
Author

wizmo2 commented Apr 9, 2021

I'll test tonight. Initially, it looks like this will work for basic functionality.
The only problem is that you will no longer be able to read or write to the dac i2c outside of the driver. (Where you could potentially have have with the old code structure).

@wizmo2
Copy link
Author

wizmo2 commented Apr 9, 2021

Also, the use of the JSON should really be reserved for dac_external.
I liked the JSON concept, and wanted to give the same flexibility for users to customize the configuration. Additionally it is possible other implementations of the chip may use different I/O configurations and require different initialization codes.

@wizmo2 wizmo2 closed this Apr 9, 2021
@wizmo2 wizmo2 reopened this Apr 9, 2021
@philippe44
Copy link
Collaborator

philippe44 commented Apr 9, 2021

I liked the JSON concept, and wanted to give the same flexibility for users to customize the configuration. Additionally it is possible other implementations of the chip may use different I/O configurations and require different initialization codes.

Well candidly it's a bit clunky. I did that as I had no other option for dac_external, but when a chip is supported internally, I'd rather do full/proper support built-in. Now, the next step if we want real external configuration is to add this JavaScript option. I'm not ready to do that for now, but that would give you the real possibility to use chip-specific features.

The good thing is that now that I've created the adac_core features, we can make these usable from JS so that's a step in the right direction :-)

@wizmo2
Copy link
Author

wizmo2 commented Apr 10, 2021

This does work.
Right now there is no need for the shadow table or read function (just the i2c write conversion).
As mentioned, its more a TTGO TAudio/ESP32-SNOW configuration. There's a lot of assumptions on how the WM8978 chip is connected to the board.

`/*

  • Squeezelite for esp32
  • (c) Wizmo 2021
  • Sebastien 2019
  •  Philippe G. 2019, philippe_44@outlook.com
    
  • This software is released under the MIT License.
  • https://opensource.org/licenses/MIT

*/

#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
#include <driver/i2s.h>
#include "driver/i2c.h"
#include "esp_log.h"
#include "adac.h"

#define WM8978 0x1A // for TTGO TAudio

static const char TAG[] = "WM8978";

static void speaker(bool active) { }
static void headset(bool active) { }
static bool volume(unsigned left, unsigned right) { return false; }
static void power(adac_power_e mode);
static bool init(char *config, int i2c_port_num, i2s_config_t *i2s_config);

static esp_err_t i2c_write_shadow(uint8_t reg, uint16_t val);
static uint16_t i2c_read_shadow(uint8_t reg);

const struct adac_s dac_wm8978 = { "WM8978", init, adac_deinit, power, speaker, headset, volume };

// initiation table for non-readbale 9-bit i2c registers
static uint16_t WM8978_REGVAL_TBL[58] = {
0X0000, 0X0000, 0X0000, 0X0000, 0X0050, 0X0000, 0X0140, 0X0000,
0X0000, 0X0000, 0X0000, 0X00FF, 0X00FF, 0X0000, 0X0100, 0X00FF,
0X00FF, 0X0000, 0X012C, 0X002C, 0X002C, 0X002C, 0X002C, 0X0000,
0X0032, 0X0000, 0X0000, 0X0000, 0X0000, 0X0000, 0X0000, 0X0000,
0X0038, 0X000B, 0X0032, 0X0000, 0X0008, 0X000C, 0X0093, 0X00E9,
0X0000, 0X0000, 0X0000, 0X0000, 0X0003, 0X0010, 0X0010, 0X0100,
0X0100, 0X0002, 0X0001, 0X0001, 0X0039, 0X0039, 0X0039, 0X0039,
0X0001, 0X0001
};

/****************************************************************************************

  • init
    */
    static bool init(char *config, int i2c_port, i2s_config_t *i2s_config) {
    // TODO: maybe add something here to confirm it is actually detected?
    adac_init(config, i2c_port);

    ESP_LOGI(TAG, "WM8978 detected");

    // init sequence
    i2c_write_shadow(0, 0);
    i2c_write_shadow(4, 16);
    i2c_write_shadow(6, 0);
    i2c_write_shadow(10, 8);
    i2c_write_shadow(43, 16);
    i2c_write_shadow(49, 102);

    // Configure system clk to GPIO0 for DAC MCLK input
    ESP_LOGI(TAG, "Configuring MCLK on pin:%d", 0);
    PIN_FUNC_SELECT(PERIPHS_IO_MUX_GPIO0_U, FUNC_GPIO0_CLK_OUT1);
    REG_WRITE(PIN_CTRL, 0xFFFFFFF0);

    return true;
    }

/****************************************************************************************

  • power
    */
    static void power(adac_power_e mode) {
    uint16_t *data, off[] = {0, 0, 0}, on[] = {11, 384, 111};
    data = (mode == ADAC_STANDBY || mode == ADAC_OFF) ? off : on;
    i2c_write_shadow(1, data[0]);
    i2c_write_shadow(2, data[1]);
    i2c_write_shadow(3, data[2]);
    }

/****************************************************************************************

  • Write with custom reg/value structure
    */
    static esp_err_t i2c_write_shadow(uint8_t reg, uint16_t val) {
    WM8978_REGVAL_TBL[reg] = val;
    reg = (reg << 1) | ((val >> 8) & 0x01);
    val &= 0xff;
    return adac_write_byte(WM8978, reg, val);
    }

/****************************************************************************************

  • Return local register value
    */
    static uint16_t i2c_read_shadow(uint8_t reg) {
    return WM8978_REGVAL_TBL[reg];
    }
    `

@philippe44
Copy link
Collaborator

philippe44 commented Apr 10, 2021

Could you update your PR? If you thing the WM8978 address can change, one thing we can do is leave the address configurable

static bool init(char *config, int i2c_port, i2s_config_t *i2s_config) {	 
	// TODO: maybe add something here to confirm it is actually detected?
	WM8978 = adac_init(config, i2c_port);
	
	if (!WM8978) WM8978 = 0x1a;
	ESP_LOGI(TAG, "WM8978 detected @%d", WM8978);
	...

and set WM8978 as a "static int WM8978" vs a define

@wizmo2
Copy link
Author

wizmo2 commented Apr 10, 2021

OK. Not sure if that worked (bit if a novice on github) but hopefully you have what need here.

@philippe44 philippe44 merged commit 68db286 into sle118:master-cmake Apr 10, 2021
@wizmo2 wizmo2 deleted the wm8978 branch November 26, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants