[Sound-open-firmware] [PATCH] apl-ssp: fix for I2S/DSP_A/DSP_B/LEFT_J modes

Pierre-Louis Bossart pierre-louis.bossart at linux.intel.com
Fri Mar 9 16:29:35 CET 2018



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 
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.



>
> 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