On Wed, 2018-03-14 at 11:37 -0500, Pierre-Louis Bossart wrote:
On 03/14/2018 11:04 AM, Liam Girdwood wrote:
On Wed, 2018-03-14 at 23:12 +0800, Keyon Jie wrote:
Previously TFT and RFT are set to 0s which means we are using threshold level 1, where interrupt asserts for every valid bits of each slot(e.g. for s16_le capture, interrupts assert for each 16 bits), but DMAC will treat it as notification of one full sample(e.g. 32 bits for s16_le stereo) and read those all bits, this will lead to capture with double speed at format s16_le stereo.
Here add handle for FIFO trigger Threshold level, set them to bytes for each full sample(active_slots * valid_bits / 8), and the issue is fixed.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
src/drivers/apl-ssp.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index 1ee107c..ecf70fb 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -43,6 +43,19 @@ #define trace_ssp_error(__e) trace_error(TRACE_CLASS_SSP, __e) #define tracev_ssp(__e) tracev_event(TRACE_CLASS_SSP, __e)
+/* FIXME: move this to a helper and optimize */ +static int hweight_32(uint32_t mask)
count_set_bits32() and can you move to lib/lib.c probably separate patch too.
Came from ssp.c, something I never optimized...
+{
- int i;
- int count = 0;
- for (i = 0; i < 32; i++) {
count += mask & 1;mask >>= 1;- }
- return count;
+}
- /* save SSP context prior to entering D3 */ static int ssp_context_store(struct dai *dai) {
@@ -96,6 +109,9 @@ static inline int ssp_set_config(struct dai *dai, uint32_t dummy_stop; uint32_t frame_len = 0; uint32_t bdiv_min;
- uint32_t active_tx_slots = 2;
- uint32_t active_rx_slots = 2;
why do we use 2 as default value, for stereo. ?
This value should always be set based on channel mask so I would set to 0 here and check that it's not 0 later on (return error and produce trace if 0).
this was an 'optimization' based on the fact that for I2S/LEFT_J we can keep this as stereo. If we don't set those then we have to manually set this to two in the switch case.
Some of the data used comes from topology so we would need to make sure that topology tx/rx slot mask would be non zero as they are used to multiply this value;.
- bool inverted_frame = false; int ret = 0;
@@ -353,6 +369,9 @@ static inline int ssp_set_config(struct dai *dai, sspsp |= SSPSP_SFRMP(!inverted_frame); sspsp |= SSPSP_FSRT;
active_tx_slots = hweight_32(config->tx_slot_mask);active_rx_slots = hweight_32(config->rx_slot_mask);- break; case SOF_DAI_FMT_DSP_B:
@@ -371,6 +390,9 @@ static inline int ssp_set_config(struct dai *dai, */ sspsp |= SSPSP_SFRMP(!inverted_frame);
active_tx_slots = hweight_32(config->tx_slot_mask);active_rx_slots = hweight_32(config->rx_slot_mask);- break; default: trace_ssp_error("eca");
@@ -408,6 +430,9 @@ static inline int ssp_set_config(struct dai *dai, /* bypass divider for MCLK */ mdivr = 0x00000fff;
- sscr3 |= SSCR3_TX(active_tx_slots * config->sample_valid_bits /
- |
SSCR3_RX(active_rx_slots * config->sample_valid_bits /8);
For Baytrail, we had:
/* FIXME: * watermarks - (RFT + 1) should equal DMA SRC_MSIZE */ sfifott = (SFIFOTT_TX(2*active_tx_slots) | SFIFOTT_RX(2*active_rx_slots));Don't we need to align RFT/TFT to the DMA_SRC_MSIZE as well? e.g. for 24 bits can we actually have a threshold that would be a multiple of 3??
Not sure, iirc one of the watermark registers is deprecated.
Liam
trace_ssp("coe"); ssp_write(dai, SSCR0, sscr0); ssp_write(dai, SSCR1, sscr1);
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware