[Sound-open-firmware] [PATCH 0/5] SSP fixes
This patchset adds corrections for APL and BYT.
For APL, the first two patches add support for a 'mclk_id' needed for specific boards, in addition to allowing for independent dividers and sources for bclk and clkc. Follow-up patches will be needed to add a 'mclk_id' topology token and kernel support.
For BYT, the patches add support for DSP_A/B modes as well as the slave mode (only CBM_CFM was tested), tested with Minnowboards and LogicPro.
Comments welcome.
Pierre-Louis Bossart (5): ssp: rename clk_id as mclk_id, remove dead code apl-ssp: fix MCLK and BCLK source selection byt-ssp: fixes for DSP modes byt-ssp: fix frame sync polarity for DSP modes byt-ssp: minor changes to test slave mode
src/drivers/apl-ssp.c | 132 +++++++++++++++++++++++++++++++++---------------- src/drivers/byt-ssp.c | 49 +++++++----------- src/drivers/hsw-ssp.c | 25 ---------- src/include/sof/ssp.h | 5 ++ src/include/uapi/ipc.h | 2 +- 5 files changed, 112 insertions(+), 101 deletions(-)
base-commit: f6f3ab0d55ee9a26235d3912851c72ffea34fca7
clk_id was never used so remove all the related code. However since we need an identifier for the mclk reclaim it with a name change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 25 +------------------------ src/drivers/byt-ssp.c | 25 ------------------------- src/drivers/hsw-ssp.c | 25 ------------------------- src/include/uapi/ipc.h | 2 +- 4 files changed, 2 insertions(+), 75 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index 6571b90..03360cb 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -233,30 +233,7 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
-#ifdef CLK_TYPE /* not enabled, keep the code for reference */ - /* TODO: allow topology to define SSP clock type */ - config->ssp.clk_id = SSP_CLK_EXT; - - /* clock source */ - switch (config->ssp.clk_id) { - case SSP_CLK_AUDIO: - sscr0 |= SSCR0_ACS; - break; - case SSP_CLK_NET_PLL: - sscr0 |= SSCR0_MOD; - break; - case SSP_CLK_EXT: - sscr0 |= SSCR0_ECS; - break; - case SSP_CLK_NET: - sscr0 |= SSCR0_NCS | SSCR0_MOD; - break; - default: - trace_ssp_error("ec4"); - ret = -EINVAL; - goto out; - } -#elif CONFIG_APOLLOLAKE +if CONFIG_APOLLOLAKE sscr0 |= SSCR0_MOD | SSCR0_ACS | SSCR0_ECS; #else sscr0 |= SSCR0_MOD | SSCR0_ACS; diff --git a/src/drivers/byt-ssp.c b/src/drivers/byt-ssp.c index d058c5f..0119f8d 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -239,31 +239,6 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
-#ifdef CLK_TYPE /* not enabled, keep the code for reference */ - /* TODO: allow topology to define SSP clock type */ - config->ssp.clk_id = SSP_CLK_EXT; - - /* clock source */ - switch (config->ssp.clk_id) { - case SSP_CLK_AUDIO: - sscr0 |= SSCR0_ACS; - break; - case SSP_CLK_NET_PLL: - sscr0 |= SSCR0_MOD; - break; - case SSP_CLK_EXT: - sscr0 |= SSCR0_ECS; - break; - case SSP_CLK_NET: - sscr0 |= SSCR0_NCS | SSCR0_MOD; - break; - default: - trace_ssp_error("ec4"); - ret = -EINVAL; - goto out; - } -#endif - /* BCLK is generated from MCLK - must be divisable */ if (config->ssp.mclk_rate % config->ssp.bclk_rate) { trace_ssp_error("ec5"); diff --git a/src/drivers/hsw-ssp.c b/src/drivers/hsw-ssp.c index 0d74e7d..479ae27 100644 --- a/src/drivers/hsw-ssp.c +++ b/src/drivers/hsw-ssp.c @@ -172,31 +172,6 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
-#ifdef CLK_TYPE /* not enabled, keep the code for reference */ - /* TODO: allow topology to define SSP clock type */ - config->ssp.clk_id = SSP_CLK_EXT; - - /* clock source */ - switch (config->ssp.clk_id) { - case SSP_CLK_AUDIO: - sscr0 |= SSCR0_ACS; - break; - case SSP_CLK_NET_PLL: - sscr0 |= SSCR0_MOD; - break; - case SSP_CLK_EXT: - sscr0 |= SSCR0_ECS; - break; - case SSP_CLK_NET: - sscr0 |= SSCR0_NCS | SSCR0_MOD; - break; - default: - trace_ssp_error("ec4"); - ret = -EINVAL; - goto out; - } -#endif - /* BCLK is generated from MCLK - must be divisable */ if (config->ssp.mclk_rate % config->ssp.bclk_rate) { trace_ssp_error("ec5"); diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index 88de113..aa48631 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -237,7 +237,7 @@ enum sof_ipc_dai_type { /* SSP Configuration Request - SOF_IPC_DAI_SSP_CONFIG */ struct sof_ipc_dai_ssp_params { uint16_t mode; // FIXME: do we need this? - uint16_t clk_id; // FIXME: do we need this? + uint16_t mclk_id;
uint32_t mclk_rate; /* mclk frequency in Hz */ uint32_t fsync_rate; /* fsync frequency in Hz */
The existing code does not provide support for the full flexibility of the hardaware: MCLK and BCLK can be independentely divided from Xtal or Audio cardinal clock. Add the ability to support different sources and dividers.
The M/N dividers remain bypassed since they introduce an irregular duty-cycle for the BCLK, which isn't desirable if the BCLK is used as a reference by an external device. With this patch MCLK can be divided by at most a factor of 8 which is reasonable for all (additional factors are possible but haven't been tested)
The MCLK1 output was tested on a GeminiLake hardware, confirming that the register configuration set according to the spec do result in an observable clock.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 123 +++++++++++++++++++++++++++++++++++++++----------- src/include/sof/ssp.h | 5 ++ 2 files changed, 101 insertions(+), 27 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index 03360cb..21065c3 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -44,6 +44,10 @@ #define trace_ssp_error(__e) trace_error(TRACE_CLASS_SSP, __e) #define tracev_ssp(__e) tracev_event(TRACE_CLASS_SSP, __e)
+#define F_19200_kHz 19200000 +#define F_24000_kHz 24000000 +#define F_24576_kHz 24576000 + /* FIXME: move this to a helper and optimize */ static int hweight_32(uint32_t mask) { @@ -103,6 +107,7 @@ static inline int ssp_set_config(struct dai *dai, uint32_t bdiv; uint32_t mdivc; uint32_t mdivr; + uint32_t mdivr_val; uint32_t i2s_m; uint32_t i2s_n; uint32_t data_size; @@ -233,21 +238,104 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
-if CONFIG_APOLLOLAKE - sscr0 |= SSCR0_MOD | SSCR0_ACS | SSCR0_ECS; -#else sscr0 |= SSCR0_MOD | SSCR0_ACS; + + mdivc = 0x1; +#ifdef CONFIG_CANNONLAKE + if (!config->ssp.mclk_rate || config->ssp.mclk_rate > F_24000_kHz) { + trace_ssp_error("eci"); + ret = -EINVAL; + goto out; + } + if (!config->ssp.bclk_rate || + config->ssp.bclk_rate > config->ssp.mclk_rate) { + trace_ssp_error("ecj"); + ret = -EINVAL; + goto out; + } + + if (F_24000_kHz % config->ssp.mclk_rate == 0) { + mdivr_val = F_24000_kHz / config->ssp.mclk_rate; + } else { + trace_ssp_error("eck"); + ret = -EINVAL; + goto out; + } + + if (F_24000_kHz % config->ssp.bclk_rate == 0) { + mdiv = F_24000_kHz / config->ssp.bclk_rate; + } else { + trace_ssp_error("ecl"); + ret = -EINVAL; + goto out; + } +#else + if (!config->ssp.mclk_rate || config->ssp.mclk_rate > F_24576_kHz) { + trace_ssp_error("eci"); + ret = -EINVAL; + goto out; + } + if (!config->ssp.bclk_rate || + config->ssp.bclk_rate > config->ssp.mclk_rate) { + trace_ssp_error("ecj"); + ret = -EINVAL; + goto out; + } + if (F_24576_kHz % config->ssp.mclk_rate == 0) { + /* select Audio Cardinal clock for MCLK */ + mdivc |= MCDSS(1); + mdivr_val = F_24576_kHz / config->ssp.mclk_rate; + } else if (config->ssp.mclk_rate <= F_19200_kHz && + F_19200_kHz % config->ssp.mclk_rate == 0) { + mdivr_val = F_19200_kHz / config->ssp.mclk_rate; + } else { + trace_ssp_error("eck"); + ret = -EINVAL; + goto out; + } + + if (F_24576_kHz % config->ssp.bclk_rate == 0) { + /* select Audio Cardinal clock for M/N dividers */ + mdivc |= MNDSS(1); + mdiv = F_24576_kHz / config->ssp.bclk_rate; + /* select M/N output for bclk */ + sscr0 |= SSCR0_ECS; + } else if (F_19200_kHz % config->ssp.bclk_rate == 0) { + mdiv = F_19200_kHz / config->ssp.bclk_rate; + } else { + trace_ssp_error("ecl"); + ret = -EINVAL; + goto out; + } #endif
- /* BCLK is generated from MCLK - must be divisable */ - if (config->ssp.mclk_rate % config->ssp.bclk_rate) { - trace_ssp_error("ec5"); + switch (mdivr_val) { + case 1: + mdivr = 0x00000fff; /* bypass divider for MCLK */ + break; + case 2: + mdivr = 0x0; /* 1/2 */ + break; + case 4: + mdivr = 0x2; /* 1/4 */ + break; + case 8: + mdivr = 0x6; /* 1/8 */ + break; + default: + trace_ssp_error("ecm"); + ret = -EINVAL; + goto out; + } + + if (config->ssp.mclk_id > 1) { + trace_ssp_error("ecn"); ret = -EINVAL; goto out; }
/* divisor must be within SCR range */ - mdiv = (config->ssp.mclk_rate / config->ssp.bclk_rate) - 1; + mdiv -= 1; if (mdiv > (SSCR0_SCR_MASK >> 8)) { trace_ssp_error("ec6"); ret = -EINVAL; @@ -264,7 +352,6 @@ if CONFIG_APOLLOLAKE goto out; }
- /* must be enough BCLKs for data */ bdiv = config->ssp.bclk_rate / config->ssp.fsync_rate; if (bdiv < config->ssp.tdm_slot_width * config->ssp.tdm_slots) { @@ -458,24 +545,6 @@ if CONFIG_APOLLOLAKE else sscr0 |= SSCR0_DSIZE(data_size);
-#ifdef CONFIG_CANNONLAKE - mdivc = 0x1; -#else - if (config->ssp.mclk_rate == 24576000) { - /* enable PLL, bypass M/N dividers */ - mdivc = 0x00100001; - } else if (config->ssp.mclk_rate == 19200000) { - /* no PLL, use XTAl oscillator as source */ - mdivc = 0; - } else { - trace_ssp_error("eci"); - ret = -EINVAL; - goto out; - } -#endif - /* bypass divider for MCLK */ - mdivr = 0x00000fff; - /* setting TFT and RFT */ switch (config->ssp.sample_valid_bits) { case 16: @@ -523,7 +592,7 @@ if CONFIG_APOLLOLAKE
/* TODO: move this into M/N driver */ mn_reg_write(0x0, mdivc); - mn_reg_write(0x80, mdivr); + mn_reg_write(0x80 + config->ssp.mclk_id * 0x4, mdivr); mn_reg_write(0x100 + config->id * 0x8 + 0x0, i2s_m); mn_reg_write(0x100 + config->id * 0x8 + 0x4, i2s_n);
diff --git a/src/include/sof/ssp.h b/src/include/sof/ssp.h index fdcfcac..8fa0df7 100644 --- a/src/include/sof/ssp.h +++ b/src/include/sof/ssp.h @@ -215,6 +215,11 @@ extern const struct dai_ops ssp_ops; #define SSIOC_SCOE BIT(5) #endif
+#if defined CONFIG_APOLLOLAKE || defined CONFIG_CANNONLAKE +#define MNDSS(x) ((x) << 20) +#define MCDSS(x) ((x) << 16) +#endif + /* tracing */ #define trace_ssp(__e) trace_event(TRACE_CLASS_SSP, __e) #define trace_ssp_error(__e) trace_error(TRACE_CLASS_SSP, __e)
For some reason SSCR3 fixes prevent DSP_A and DSP_B from working: DMAs don't start and an IPC error is eventually thrown.
Fall back to reset value (recommended in data sheet), this fix lets DMA go on in DSP_a and DSP_B modes.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/byt-ssp.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/drivers/byt-ssp.c b/src/drivers/byt-ssp.c index 0119f8d..caaf161 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -150,9 +150,16 @@ static inline int ssp_set_config(struct dai *dai, * sscr3 dynamic settings are FRM_MS_EN, I2S_MODE_EN, I2S_FRM_POL, * I2S_TX_EN, I2S_RX_EN, I2S_CLK_MST */ - sscr3 = SSCR3_I2S_TX_SS_FIX_EN | SSCR3_I2S_RX_SS_FIX_EN | - SSCR3_STRETCH_TX | SSCR3_STRETCH_RX | - SSCR3_SYN_FIX_EN; + sscr3 = SSCR3_SYN_FIX_EN; +#ifdef ENABLE_SSRC3_FIXES + /* + * this seems to prevent DSP modes from working but is harmless for + * I2S and LEFT_J. Keep with ifdef in case it's ever needed. + */ + sscr3 |= SSCR3_I2S_TX_SS_FIX_EN | SSCR3_I2S_RX_SS_FIX_EN | + SSCR3_STRETCH_TX | SSCR3_STRETCH_RX; +#endif + #ifdef ENABLE_CLK_EDGE_SEL /* FIXME: is this needed ? */ sscr3 |= SSCR3_CLK_EDGE_SEL; #endif
Now that these modes work, fix the polarity to align with ALSA/ASoC expectations
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/byt-ssp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/drivers/byt-ssp.c b/src/drivers/byt-ssp.c index caaf161..c9dbcaa 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -358,7 +358,7 @@ static inline int ssp_set_config(struct dai *dai, frame_len = 1;
/* handle frame polarity, DSP_A default is rising/active high */ - sspsp |= SSPSP_SFRMP(inverted_frame); + sspsp |= SSPSP_SFRMP(!inverted_frame); if (cfs) { /* set sscr frame polarity in DSP/master mode only */ sscr5 |= SSCR5_FRM_POLARITY(inverted_frame); @@ -386,7 +386,7 @@ static inline int ssp_set_config(struct dai *dai, frame_len = 1;
/* handle frame polarity, DSP_A default is rising/active high */ - sspsp |= SSPSP_SFRMP(inverted_frame); + sspsp |= SSPSP_SFRMP(!inverted_frame); if (cfs) { /* set sscr frame polarity in DSP/master mode only */ sscr5 |= SSCR5_FRM_POLARITY(inverted_frame);
Comment out SDO tristating which gets in the way of test tools and clarify comment for ETDS.
I2S and DSP modes seem to generate correct waveforms. Tested with one Up2 board in master mode and one MinnowBoard Turbot in slave, with the SDO/SDI lines samples with Logic Pro.
Only CBM_CFM mode was tested.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/byt-ssp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/drivers/byt-ssp.c b/src/drivers/byt-ssp.c index c9dbcaa..a940ff9 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -133,7 +133,10 @@ static inline int ssp_set_config(struct dai *dai, */
/* sscr1 dynamic settings are TFT, RFT, SFRMDIR, SCLKDIR, SCFR */ - sscr1 = SSCR1_TTE; + sscr1 = 0; +#ifdef ENABLE_SSCR1_TRISTATE + sscr1 |= SSCR1_TTE; /* make sure SDO line is tri-stated when inactive */ +#endif #ifdef ENABLE_TIE_RIE /* FIXME: not enabled, difference with SST driver */ sscr1 |= SSCR1_TIE | SSCR1_RIE; #endif @@ -171,7 +174,7 @@ static inline int ssp_set_config(struct dai *dai, sscr5 = 0x0;
/* sspsp dynamic settings are SCMODE, SFRMP, DMYSTRT, SFRMWDTH */ - sspsp = SSPSP_ETDS; /* make sure SDO line is tri-stated when inactive */ + sspsp = SSPSP_ETDS; /* last value (bit 0) */
ssp->config = *config; ssp->params = config->ssp;
On Mon, 2018-06-18 at 14:27 -0500, Pierre-Louis Bossart wrote:
This patchset adds corrections for APL and BYT.
For APL, the first two patches add support for a 'mclk_id' needed for specific boards, in addition to allowing for independent dividers and sources for bclk and clkc. Follow-up patches will be needed to add a 'mclk_id' topology token and kernel support.
For BYT, the patches add support for DSP_A/B modes as well as the slave mode (only CBM_CFM was tested), tested with Minnowboards and LogicPro.
All applied.
Thanks
Liam
participants (2)
-
Liam Girdwood -
Pierre-Louis Bossart