[Sound-open-firmware] [RFC PATCH 0/4] SSP fixes
I've had some of these patches on a work branch for some time now and only found the time to rebase/clean them now. The most important parts are support for DSP_A/B modes which never worked on Baytrail, Cherrytrail and clarifications on how the MCLK/Bitclock are generated on ApolloLake (parallel dividers instead of serial ones, allowing for independent selection of the source and ratios)
I could use comments on the code. I still have a couple of tests to run so it's not ready for integration but if I could have additional reviews or tests in parallel it'd be nice. Thanks!
Pierre-Louis Bossart (4): 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
src/drivers/apl-ssp.c | 132 +++++++++++++++++++++++++++++++++---------------- src/drivers/byt-ssp.c | 42 +++++----------- src/include/sof/ssp.h | 5 ++ src/include/uapi/ipc.h | 2 +- 4 files changed, 107 insertions(+), 74 deletions(-)
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/include/uapi/ipc.h | 2 +- 3 files changed, 2 insertions(+), 50 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index 53bb8be..1190596 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -232,30 +232,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 ca3385d..13b6d76 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -238,31 +238,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 b32376d..c13fc4b 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -238,7 +238,7 @@ enum sof_ipc_dai_type { struct sof_ipc_dai_ssp_params { struct sof_ipc_hdr hdr; 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 */
MCLK and BCLK can be independentely divided from Xtal or Audio cardinal clock. With this patch MCLK can be divided by at most a factor of 8.
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.
The MCLK1 output was not tested since I don't have hardware to test this configuration but the register address is set according to the specification.
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 1190596..df0c0f8 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; @@ -232,21 +237,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; @@ -263,7 +351,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) { @@ -457,24 +544,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: @@ -522,7 +591,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 d34b011..5244296 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 13b6d76..ab6b4ed 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -149,9 +149,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 ab6b4ed..928f5b1 100644 --- a/src/drivers/byt-ssp.c +++ b/src/drivers/byt-ssp.c @@ -357,7 +357,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); @@ -385,7 +385,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);
On Tue, 2018-05-22 at 19:31 -0500, Pierre-Louis Bossart wrote:
I've had some of these patches on a work branch for some time now and only found the time to rebase/clean them now. The most important parts are support for DSP_A/B modes which never worked on Baytrail, Cherrytrail and clarifications on how the MCLK/Bitclock are generated on ApolloLake (parallel dividers instead of serial ones, allowing for independent selection of the source and ratios)
I could use comments on the code. I still have a couple of tests to run so it's not ready for integration but if I could have additional reviews or tests in parallel it'd be nice. Thanks!
Pierre-Louis Bossart (4): 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
src/drivers/apl-ssp.c | 132 +++++++++++++++++++++++++++++++++---------------
src/drivers/byt-ssp.c | 42 +++++----------- src/include/sof/ssp.h | 5 ++ src/include/uapi/ipc.h | 2 +- 4 files changed, 107 insertions(+), 74 deletions(-)
Look good to me.
Liam
participants (2)
-
Liam Girdwood -
Pierre-Louis Bossart