[Sound-open-firmware] [PATCH] apl-ssp: fix for I2S/DSP_A/DSP_B/LEFT_J modes
Jie, Yang
yang.jie at intel.com
Sat Mar 10 04:21:59 CET 2018
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
>Sent: Friday, March 9, 2018 11:30 PM
>To: Keyon Jie <yang.jie at linux.intel.com>; sound-open-firmware at alsa-
>project.org
>Cc: Jie, Yang <yang.jie at intel.com>
>Subject: Re: [Sound-open-firmware] [PATCH] apl-ssp: fix for
>I2S/DSP_A/DSP_B/LEFT_J modes
>
>
>
>On 03/09/2018 06:54 AM, Keyon Jie wrote:
>> Clean up SSPSP bits to make SSP output follow ALSA ASoC definition
>> of all I2S/DSP_A/DSP_B/LEFT_J modes.
>Looks consistent to me, this should be merged.
>
>Keyon, did you test this on APL as well as CNL? I tried on Up2 and again
I tried it on APL(GPMRB) only, maybe we can test it on CNL also next
week. By the way, which tplg file and what params are you set there
for your test?(container_bits, bclk, mclk, tdm_slots, i2s/dsp_x, ...)
>no frame sync, so if you have another APL platform that shows a working
>platform this would point to a board issue.
>
>we'll have to go back to BYT, apply some of the fixes (e.g. making sure
>the bclock/fs is a multiple of 2) and double-check if indeed the
>definition of SFRMP is inverted between the two platforms, as well as
>the differences with start_delay and dummy_start.
Yep, these are all important for SSP settings, and may be different for
different generations, it will be very helpful if we can get some
explains/confirmations from silicon IP team.
Thanks,
~Keyon
>
>
>
>>
>> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
>> ---
>> src/drivers/apl-ssp.c | 88 ++++++++++++++++++++++++++++++++++++++-------
>-----
>> src/include/reef/ssp.h | 3 ++
>> 2 files changed, 71 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c
>> index c32bd35..42ffd03 100644
>> --- a/src/drivers/apl-ssp.c
>> +++ b/src/drivers/apl-ssp.c
>> @@ -93,7 +93,9 @@ static inline int ssp_set_config(struct dai *dai,
>> uint32_t i2s_n;
>> uint32_t data_size;
>> uint32_t start_delay;
>> + uint32_t dummy_stop;
>> uint32_t frame_len = 0;
>> + uint32_t bdiv_min;
>> bool inverted_frame = false;
>> int ret = 0;
>>
>> @@ -265,8 +267,7 @@ static inline int ssp_set_config(struct dai *dai,
>>
>> /* must be enouch BCLKs for data */
>> bdiv = config->bclk / config->fclk;
>> - if (bdiv < config->sample_container_bits *
>> - ((config->rx_slot_mask + 1) / 2)) {
>> + if (bdiv < config->sample_container_bits * config->num_slots) {
>> trace_ssp_error("ec8");
>> ret = -EINVAL;
>> goto out;
>> @@ -283,16 +284,27 @@ static inline int ssp_set_config(struct dai *dai,
>> switch (config->format & SOF_DAI_FMT_FORMAT_MASK) {
>> case SOF_DAI_FMT_I2S:
>>
>> - start_delay = config->sample_container_bits >
>> - config->sample_valid_bits ? 1 : 0;
>> + start_delay = 1;
>>
>> sscr0 |= SSCR0_FRDC(config->num_slots);
>>
>> - /* set asserted frame length */
>> - frame_len = config->sample_container_bits;
>> + if (bdiv % 2) {
>> + trace_ssp_error("eca");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>>
>> - /* handle frame polarity, I2S default is falling/active low */
>> - sspsp |= SSPSP_SFRMP(!inverted_frame);
>> + /* set asserted frame length to half frame length */
>> + frame_len = bdiv / 2;
>> +
>> + /*
>> + * 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;
>>
>> break;
>>
>> @@ -305,25 +317,41 @@ static inline int ssp_set_config(struct dai *dai,
>> /* LJDFD enable */
>> sscr2 &= ~SSCR2_LJDFD;
>>
>> - /* set asserted frame length */
>> - frame_len = config->sample_container_bits;
>> + if (bdiv % 2) {
>> + trace_ssp_error("ecb");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>>
>> - /* LEFT_J default is rising/active high, opposite of I2S */
>> - sspsp |= SSPSP_SFRMP(inverted_frame);
>> + /* set asserted frame length to half frame length */
>> + frame_len = bdiv / 2;
>> +
>> + /*
>> + * 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);
>>
>> break;
>> case SOF_DAI_FMT_DSP_A:
>>
>> - start_delay = config->sample_container_bits >
>> - config->sample_valid_bits ? 1 : 0;
>> + start_delay = 1;
>>
>> sscr0 |= SSCR0_MOD | SSCR0_FRDC(config->num_slots);
>>
>> /* set asserted frame length */
>> - frame_len = config->sample_container_bits;
>> + frame_len = 1;
>>
>> - /* handle frame polarity, DSP_A default is rising/active high */
>> - sspsp |= SSPSP_SFRMP(inverted_frame);
>> + /*
>> + * handle frame polarity, DSP_A 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);
>> + sspsp |= SSPSP_FSRT;
>>
>> break;
>> case SOF_DAI_FMT_DSP_B:
>> @@ -335,9 +363,13 @@ static inline int ssp_set_config(struct dai *dai,
>> /* set asserted frame length */
>> frame_len = 1;
>>
>> - /* handle frame polarity, DSP_A default is rising/active high */
>> + /*
>> + * handle frame polarity, DSP_B 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);
>> - sspsp |= SSPSP_FSRT;
>>
>> break;
>> default:
>> @@ -346,9 +378,25 @@ static inline int ssp_set_config(struct dai *dai,
>> goto out;
>> }
>>
>> - sspsp |= SSPSP_DMYSTRT(start_delay);
>> + sspsp |= SSPSP_STRTDLY(start_delay);
>> sspsp |= SSPSP_SFRMWDTH(frame_len);
>>
>> + /*
>> + * [dummy_start][valid_bits_slot[0...n-1]][dummy_stop],
>> + * but don't count dummy_start for dummy_stop calculation.
>> + */
>> + bdiv_min = config->num_slots * config->sample_valid_bits;
>> + if (bdiv < bdiv_min) {
>> + trace_ssp_error("ecc");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + dummy_stop = bdiv - bdiv_min;
>> + sspsp |= SSPSP_DMYSTOP(SSPSP_DMYSTOP_MASK & dummy_stop);
>> + sspsp |= SSPSP_EDMYSTOP(SSPSP_EDMYSTOP_MASK &
>> + (dummy_stop >> SSPSP_DMYSTOP_BITS));
>> +
>> data_size = config->sample_valid_bits;
>>
>> if (data_size > 16)
>> diff --git a/src/include/reef/ssp.h b/src/include/reef/ssp.h
>> index e041121..453aaab 100644
>> --- a/src/include/reef/ssp.h
>> +++ b/src/include/reef/ssp.h
>> @@ -160,10 +160,13 @@ extern const struct dai_ops ssp_ops;
>> #define SSPSP_SFRMDLY(x) ((x) << 9)
>> #define SSPSP_SFRMWDTH(x) ((x) << 16)
>> #define SSPSP_DMYSTOP(x) ((x) << 23)
>> +#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 SSTSS 0x38
>> #define SSCR2 0x40
>> #define SSPSP2 0x44
More information about the Sound-open-firmware
mailing list