[Sound-open-firmware] [PATCH] SSP: refine padding bit setting on Broadwell
merge commit 36f23c6cc3ed ("apl-ssp: fix padding bit issues in I2S/LEFT_J mode") and refine it on Broadwell.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Rander Wang rander.wang@linux.intel.com
--- test it on Broadwell --- src/drivers/hsw-ssp.c | 99 ++++++++++++++++++++++++++++++++++++++++++--------- src/include/sof/ssp.h | 11 +++--- 2 files changed, 87 insertions(+), 23 deletions(-)
diff --git a/src/drivers/hsw-ssp.c b/src/drivers/hsw-ssp.c index 7e5ae3d..b2fa99e 100644 --- a/src/drivers/hsw-ssp.c +++ b/src/drivers/hsw-ssp.c @@ -77,11 +77,16 @@ static inline int ssp_set_config(struct dai *dai, uint32_t sscr1; uint32_t sscr2; uint32_t sspsp; + uint32_t sspsp2; uint32_t mdiv; uint32_t bdiv; uint32_t data_size; uint32_t start_delay; + uint32_t frame_end_padding; + uint32_t slot_end_padding; uint32_t frame_len = 0; + uint32_t bdiv_min; + uint32_t format; bool inverted_frame = false; int ret = 0;
@@ -130,6 +135,9 @@ static inline int ssp_set_config(struct dai *dai, /* sspsp dynamic settings are SCMODE, SFRMP, DMYSTRT, SFRMWDTH */ sspsp = SSPSP_ETDS; /* make sure SDO line is tri-stated when inactive */
+ /* sspsp2 no dynamic setting */ + sspsp2 = 0x0; + ssp->config = *config; ssp->params = config->ssp;
@@ -252,29 +260,83 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
- /* format */ - switch (config->format & SOF_DAI_FMT_FORMAT_MASK) { - case SOF_DAI_FMT_I2S: - - start_delay = 1; - - /* set asserted frame length */ - frame_len = config->ssp.tdm_slot_width; - - /* handle frame polarity, I2S default is falling/active low */ - sspsp |= SSPSP_SFRMP(!inverted_frame); + bdiv_min = config->ssp.tdm_slots * config->ssp.sample_valid_bits; + if (bdiv < bdiv_min) { + trace_ssp_error("ecc"); + ret = -EINVAL; + goto out; + }
- break; + frame_end_padding = bdiv - bdiv_min; + if (frame_end_padding > SSPSP2_FEP_MASK) { + trace_ssp_error("ecd"); + ret = -EINVAL; + goto out; + }
+ /* format */ + format = config->format & SOF_DAI_FMT_FORMAT_MASK; + switch (format) { + case SOF_DAI_FMT_I2S: case SOF_DAI_FMT_LEFT_J:
- start_delay = 0; + if (format == SOF_DAI_FMT_I2S) { + start_delay = 1; + + /* + * handle frame polarity, I2S default is falling/active low, + * non-inverted(inverted_frame=0) -- active low(SFRMP=0), + * inverted(inverted_frame=1) -- rising/active high(SFRMP=1), + * so, we should set SFRMP to inverted_frame. + */ + sspsp |= SSPSP_SFRMP(inverted_frame); + sspsp |= SSPSP_FSRT; + + } else { + start_delay = 0; + + /* + * handle frame polarity, LEFT_J default is rising/active high, + * non-inverted(inverted_frame=0) -- active high(SFRMP=1), + * inverted(inverted_frame=1) -- falling/active low(SFRMP=0), + * so, we should set SFRMP to !inverted_frame. + */ + sspsp |= SSPSP_SFRMP(!inverted_frame); + }
- /* set asserted frame length */ - frame_len = config->ssp.tdm_slot_width; + sscr0 |= SSCR0_FRDC(config->ssp.tdm_slots);
- /* LEFT_J default is rising/active high, opposite of I2S */ - sspsp |= SSPSP_SFRMP(inverted_frame); + if (bdiv % 2) { + trace_ssp_error("eca"); + ret = -EINVAL; + goto out; + } + + /* set asserted frame length to half frame length */ + frame_len = bdiv / 2; + + /* + * for I2S/LEFT_J, the padding has to happen at the end + * of each slot + */ + if (frame_end_padding % 2) { + trace_ssp_error("ece"); + ret = -EINVAL; + goto out; + } + + slot_end_padding = frame_end_padding / 2; + + if (slot_end_padding > 15) { + /* can't handle padding over 15 bits */ + trace_ssp_error("ecf"); + ret = -EINVAL; + goto out; + } + + sspsp |= SSPSP_DMYSTOP(slot_end_padding & SSPSP_DMYSTOP_MASK); + slot_end_padding >>= SSPSP_DMYSTOP_BITS; + sspsp |= SSPSP_EDMYSTOP(slot_end_padding & SSPSP_EDMYSTOP_MASK);
break; case SOF_DAI_FMT_DSP_A: @@ -288,6 +350,7 @@ static inline int ssp_set_config(struct dai *dai,
/* handle frame polarity, DSP_A default is rising/active high */ sspsp |= SSPSP_SFRMP(inverted_frame); + sspsp2 |= (frame_end_padding & SSPSP2_FEP_MASK);
break; case SOF_DAI_FMT_DSP_B: @@ -301,6 +364,7 @@ static inline int ssp_set_config(struct dai *dai,
/* handle frame polarity, DSP_A default is rising/active high */ sspsp |= SSPSP_SFRMP(inverted_frame); + sspsp2 |= (frame_end_padding & SSPSP2_FEP_MASK);
break; default: @@ -329,6 +393,7 @@ static inline int ssp_set_config(struct dai *dai, ssp_write(dai, SSPSP, sspsp); ssp_write(dai, SSTSA, config->ssp.tx_slots); ssp_write(dai, SSRSA, config->ssp.rx_slots); + ssp_write(dai, SSPSP2, sspsp2);
ssp->state[DAI_DIR_PLAYBACK] = COMP_STATE_PREPARE; ssp->state[DAI_DIR_CAPTURE] = COMP_STATE_PREPARE; diff --git a/src/include/sof/ssp.h b/src/include/sof/ssp.h index d34b011..3a1f6ed 100644 --- a/src/include/sof/ssp.h +++ b/src/include/sof/ssp.h @@ -55,12 +55,12 @@ #define SSTSA 0x30 #define SSRSA 0x34 #define SSTSS 0x38 +#define SSCR2 0x40
#if defined CONFIG_BAYTRAIL ||\ defined CONFIG_CHERRYTRAIL ||\ defined CONFIG_BROADWELL ||\ defined CONFIG_HASWELL -#define SSCR2 0x40 #define SFIFOTT 0x6C #define SSCR3 0x70 #define SSCR4 0x74 @@ -159,14 +159,13 @@ extern const struct dai_ops ssp_ops; #define SSPSP_DMYSTOP_BITS 2 #define SSPSP_DMYSTOP_MASK ((0x1 << SSPSP_DMYSTOP_BITS) - 1) #define SSPSP_FSRT (1 << 25) - -#if defined CONFIG_APOLLOLAKE || defined CONFIG_CANNONLAKE #define SSPSP_EDMYSTOP(x) ((x) << 26) #define SSPSP_EDMYSTOP_MASK 0x7 + +#define SSPSP2 0x44 #define SSPSP2_FEP_MASK 0xff -#define SSTSS 0x38 -#define SSCR2 0x40 -#define SSPSP2 0x44 + +#if defined CONFIG_APOLLOLAKE || defined CONFIG_CANNONLAKE #define SSCR3 0x48 #define SSIOC 0x4C
On Fri, 2018-06-01 at 16:50 +0800, Rander Wang wrote:
merge commit 36f23c6cc3ed ("apl-ssp: fix padding bit issues in I2S/LEFT_J mode") and refine it on Broadwell.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Rander Wang rander.wang@linux.intel.com
test it on Broadwell
src/drivers/hsw-ssp.c | 99 ++++++++++++++++++++++++++++++++++++++++++------
src/include/sof/ssp.h | 11 +++--- 2 files changed, 87 insertions(+), 23 deletions(-)
Applied.
Thanks
Lima
On 6/1/18 3:50 AM, Rander Wang wrote:
merge commit 36f23c6cc3ed ("apl-ssp: fix padding bit issues in I2S/LEFT_J mode") and refine it on Broadwell.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Rander Wang rander.wang@linux.intel.com
test it on Broadwell
are the waveforms correct with this patch? What tests did you run? I am also concerned that we are losing alignment with the Baytrail code, and it becomes difficult to track the differences in register settings between platforms.
src/drivers/hsw-ssp.c | 99 ++++++++++++++++++++++++++++++++++++++++++--------- src/include/sof/ssp.h | 11 +++--- 2 files changed, 87 insertions(+), 23 deletions(-)
diff --git a/src/drivers/hsw-ssp.c b/src/drivers/hsw-ssp.c index 7e5ae3d..b2fa99e 100644 --- a/src/drivers/hsw-ssp.c +++ b/src/drivers/hsw-ssp.c @@ -77,11 +77,16 @@ static inline int ssp_set_config(struct dai *dai, uint32_t sscr1; uint32_t sscr2; uint32_t sspsp;
- uint32_t sspsp2; uint32_t mdiv; uint32_t bdiv; uint32_t data_size; uint32_t start_delay;
- uint32_t frame_end_padding;
- uint32_t slot_end_padding; uint32_t frame_len = 0;
- uint32_t bdiv_min;
- uint32_t format; bool inverted_frame = false; int ret = 0;
@@ -130,6 +135,9 @@ static inline int ssp_set_config(struct dai *dai, /* sspsp dynamic settings are SCMODE, SFRMP, DMYSTRT, SFRMWDTH */ sspsp = SSPSP_ETDS; /* make sure SDO line is tri-stated when inactive */
- /* sspsp2 no dynamic setting */
- sspsp2 = 0x0;
- ssp->config = *config; ssp->params = config->ssp;
@@ -252,29 +260,83 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
- /* format */
- switch (config->format & SOF_DAI_FMT_FORMAT_MASK) {
- case SOF_DAI_FMT_I2S:
start_delay = 1;
/* set asserted frame length */
frame_len = config->ssp.tdm_slot_width;
/* handle frame polarity, I2S default is falling/active low */
sspsp |= SSPSP_SFRMP(!inverted_frame);
- bdiv_min = config->ssp.tdm_slots * config->ssp.sample_valid_bits;
- if (bdiv < bdiv_min) {
trace_ssp_error("ecc");
ret = -EINVAL;
goto out;
- }
break;
frame_end_padding = bdiv - bdiv_min;
if (frame_end_padding > SSPSP2_FEP_MASK) {
trace_ssp_error("ecd");
ret = -EINVAL;
goto out;
}
/* format */
format = config->format & SOF_DAI_FMT_FORMAT_MASK;
switch (format) {
case SOF_DAI_FMT_I2S: case SOF_DAI_FMT_LEFT_J:
start_delay = 0;
if (format == SOF_DAI_FMT_I2S) {
start_delay = 1;
this sort of code is not good for MISRA compliance. You want one self-contained case at a time, and no additional tests in the middle. Can you please refactor in a follow-up patch?
/*
* handle frame polarity, I2S default is falling/active low,
* non-inverted(inverted_frame=0) -- active low(SFRMP=0),
* inverted(inverted_frame=1) -- rising/active high(SFRMP=1),
* so, we should set SFRMP to inverted_frame.
*/
sspsp |= SSPSP_SFRMP(inverted_frame);
sspsp |= SSPSP_FSRT;
} else {
start_delay = 0;
/*
* handle frame polarity, LEFT_J default is rising/active high,
* non-inverted(inverted_frame=0) -- active high(SFRMP=1),
* inverted(inverted_frame=1) -- falling/active low(SFRMP=0),
* so, we should set SFRMP to !inverted_frame.
*/
sspsp |= SSPSP_SFRMP(!inverted_frame);
}
/* set asserted frame length */
frame_len = config->ssp.tdm_slot_width;
sscr0 |= SSCR0_FRDC(config->ssp.tdm_slots);
/* LEFT_J default is rising/active high, opposite of I2S */
sspsp |= SSPSP_SFRMP(inverted_frame);
if (bdiv % 2) {
trace_ssp_error("eca");
ret = -EINVAL;
goto out;
}
/* set asserted frame length to half frame length */
frame_len = bdiv / 2;
/*
* for I2S/LEFT_J, the padding has to happen at the end
* of each slot
*/
if (frame_end_padding % 2) {
trace_ssp_error("ece");
ret = -EINVAL;
goto out;
}
slot_end_padding = frame_end_padding / 2;
if (slot_end_padding > 15) {
/* can't handle padding over 15 bits */
trace_ssp_error("ecf");
ret = -EINVAL;
goto out;
}
sspsp |= SSPSP_DMYSTOP(slot_end_padding & SSPSP_DMYSTOP_MASK);
slot_end_padding >>= SSPSP_DMYSTOP_BITS;
sspsp |= SSPSP_EDMYSTOP(slot_end_padding & SSPSP_EDMYSTOP_MASK);
break; case SOF_DAI_FMT_DSP_A:
@@ -288,6 +350,7 @@ static inline int ssp_set_config(struct dai *dai,
/* handle frame polarity, DSP_A default is rising/active high */ sspsp |= SSPSP_SFRMP(inverted_frame);
sspsp2 |= (frame_end_padding & SSPSP2_FEP_MASK);
break; case SOF_DAI_FMT_DSP_B:
@@ -301,6 +364,7 @@ static inline int ssp_set_config(struct dai *dai,
/* handle frame polarity, DSP_A default is rising/active high */ sspsp |= SSPSP_SFRMP(inverted_frame);
sspsp2 |= (frame_end_padding & SSPSP2_FEP_MASK);
break; default:
@@ -329,6 +393,7 @@ static inline int ssp_set_config(struct dai *dai, ssp_write(dai, SSPSP, sspsp); ssp_write(dai, SSTSA, config->ssp.tx_slots); ssp_write(dai, SSRSA, config->ssp.rx_slots);
ssp_write(dai, SSPSP2, sspsp2);
ssp->state[DAI_DIR_PLAYBACK] = COMP_STATE_PREPARE; ssp->state[DAI_DIR_CAPTURE] = COMP_STATE_PREPARE;
diff --git a/src/include/sof/ssp.h b/src/include/sof/ssp.h index d34b011..3a1f6ed 100644 --- a/src/include/sof/ssp.h +++ b/src/include/sof/ssp.h @@ -55,12 +55,12 @@ #define SSTSA 0x30 #define SSRSA 0x34 #define SSTSS 0x38 +#define SSCR2 0x40
#if defined CONFIG_BAYTRAIL ||\ defined CONFIG_CHERRYTRAIL ||\ defined CONFIG_BROADWELL ||\ defined CONFIG_HASWELL -#define SSCR2 0x40 #define SFIFOTT 0x6C #define SSCR3 0x70 #define SSCR4 0x74 @@ -159,14 +159,13 @@ extern const struct dai_ops ssp_ops; #define SSPSP_DMYSTOP_BITS 2 #define SSPSP_DMYSTOP_MASK ((0x1 << SSPSP_DMYSTOP_BITS) - 1) #define SSPSP_FSRT (1 << 25)
-#if defined CONFIG_APOLLOLAKE || defined CONFIG_CANNONLAKE #define SSPSP_EDMYSTOP(x) ((x) << 26) #define SSPSP_EDMYSTOP_MASK 0x7
+#define SSPSP2 0x44 #define SSPSP2_FEP_MASK 0xff -#define SSTSS 0x38 -#define SSCR2 0x40 -#define SSPSP2 0x44
+#if defined CONFIG_APOLLOLAKE || defined CONFIG_CANNONLAKE #define SSCR3 0x48 #define SSIOC 0x4C
participants (3)
-
Liam Girdwood
-
Pierre-Louis Bossart
-
Rander Wang