[alsa-devel] [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup
Sorry, for the long series to do something so simple, but there were unresolved objections when I first posted this to the davinci list.
Hopefully, having this split into tiny pieces will make resolving those differences easier.
Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B mode. Before dsp_b was really being initialized as dsp_a.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index b76bcc3..ad19661 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -847,6 +847,7 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai, struct snd_soc_codec *codec = codec_dai->codec; struct aic3x_priv *aic3x = codec->private_data; u8 iface_areg, iface_breg; + int delay = 0;
iface_areg = aic3x_read_reg_cache(codec, AIC3X_ASD_INTF_CTRLA) & 0x3f; iface_breg = aic3x_read_reg_cache(codec, AIC3X_ASD_INTF_CTRLB) & 0x3f; @@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai, case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF): break; case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF): + delay = 1; + case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF): iface_breg |= (0x01 << 6); break; case (SND_SOC_DAIFMT_RIGHT_J | SND_SOC_DAIFMT_NB_NF): @@ -888,6 +891,7 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai, /* set iface */ aic3x_write(codec, AIC3X_ASD_INTF_CTRLA, iface_areg); aic3x_write(codec, AIC3X_ASD_INTF_CTRLB, iface_breg); + aic3x_write(codec, AIC3X_ASD_INTF_CTRLC, delay);
return 0; } diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h index 00a195a..40a6c2a 100644 --- a/sound/soc/codecs/tlv320aic3x.h +++ b/sound/soc/codecs/tlv320aic3x.h @@ -35,6 +35,8 @@ #define AIC3X_ASD_INTF_CTRLA 8 /* Audio serial data interface control register B */ #define AIC3X_ASD_INTF_CTRLB 9 +/* Audio serial data interface control register C */ +#define AIC3X_ASD_INTF_CTRLC 10 /* Audio overflow status and PLL R value programming register */ #define AIC3X_OVRF_STATUS_AND_PLLR_REG 11 /* Audio codec digital filter control register */
Add constants with a value of 0 to show more explicitly what is being requested.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index 2ce34d4..555ecb2 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -38,12 +38,14 @@ static int evm_hw_params(struct snd_pcm_substream *substream,
/* set codec DAI configuration */ ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S | - SND_SOC_DAIFMT_CBM_CFM); + SND_SOC_DAIFMT_CBM_CFM | + SND_SOC_DAIFMT_NB_NF); if (ret < 0) return ret;
/* set cpu DAI configuration */ - ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_CBM_CFM | + ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_IB_NF); if (ret < 0) return ret;
Document the current polarity choices.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index cf31b3b..ecb5e83 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -235,18 +235,45 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
switch (fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_IB_NF: + /* CLKRP Receive clock polarity, + * 1 - sampled on rising edge of CLKR + * valid on rising edge + * CLKXP Transmit clock polarity, + * 1 - clocked on falling edge of CLKX + * valid on rising edge + * FSRP Receive frame sync pol, 0 - active high + * FSXP Transmit frame sync pol, 0 - active high + */ w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP, 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; case SND_SOC_DAIFMT_NB_IF: + /* CLKRP Receive clock polarity, + * 0 - sampled on falling edge of CLKR + * valid on falling edge + * CLKXP Transmit clock polarity, + * 0 - clocked on rising edge of CLKX + * valid on falling edge + * FSRP Receive frame sync pol, 1 - active low + * FSXP Transmit frame sync pol, 1 - active low + */ w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP, 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; case SND_SOC_DAIFMT_IB_IF: + /* CLKRP Receive clock polarity, + * 1 - sampled on rising edge of CLKR + * valid on rising edge + * CLKXP Transmit clock polarity, + * 1 - clocked on falling edge of CLKX + * valid on rising edge + * FSRP Receive frame sync pol, 1 - active low + * FSXP Transmit frame sync pol, 1 - active low + */ w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP | @@ -255,6 +282,15 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; case SND_SOC_DAIFMT_NB_NF: + /* CLKRP Receive clock polarity, + * 0 - sampled on falling edge of CLKR + * valid on falling edge + * CLKXP Transmit clock polarity, + * 0 - clocked on rising edge of CLKX + * valid on falling edge + * FSRP Receive frame sync pol, 0 - active high + * FSXP Transmit frame sync pol, 0 - active high + */ break; default: return -EINVAL;
Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that used in the codec.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index 555ecb2..a78ff59 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -28,6 +28,8 @@
#define EVM_CODEC_CLOCK 22579200
+#define AUDIO_FORMAT (SND_SOC_DAIFMT_I2S | \ + SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_NB_NF) static int evm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -37,16 +39,12 @@ static int evm_hw_params(struct snd_pcm_substream *substream, int ret = 0;
/* set codec DAI configuration */ - ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S | - SND_SOC_DAIFMT_CBM_CFM | - SND_SOC_DAIFMT_NB_NF); + ret = snd_soc_dai_set_fmt(codec_dai, AUDIO_FORMAT); if (ret < 0) return ret;
/* set cpu DAI configuration */ - ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S | - SND_SOC_DAIFMT_CBM_CFM | - SND_SOC_DAIFMT_IB_NF); + ret = snd_soc_dai_set_fmt(cpu_dai, AUDIO_FORMAT); if (ret < 0) return ret;
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index ecb5e83..8e2cdf2 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -234,7 +234,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, }
switch (fmt & SND_SOC_DAIFMT_INV_MASK) { - case SND_SOC_DAIFMT_IB_NF: + case SND_SOC_DAIFMT_NB_NF: /* CLKRP Receive clock polarity, * 1 - sampled on rising edge of CLKR * valid on rising edge @@ -249,7 +249,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, DAVINCI_MCBSP_PCR_CLKRP, 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; - case SND_SOC_DAIFMT_NB_IF: + case SND_SOC_DAIFMT_IB_IF: /* CLKRP Receive clock polarity, * 0 - sampled on falling edge of CLKR * valid on falling edge @@ -264,7 +264,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, DAVINCI_MCBSP_PCR_FSRP, 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; - case SND_SOC_DAIFMT_IB_IF: + case SND_SOC_DAIFMT_NB_IF: /* CLKRP Receive clock polarity, * 1 - sampled on rising edge of CLKR * valid on rising edge @@ -281,7 +281,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, DAVINCI_MCBSP_PCR_FSRP, 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); break; - case SND_SOC_DAIFMT_NB_NF: + case SND_SOC_DAIFMT_IB_NF: /* CLKRP Receive clock polarity, * 0 - sampled on falling edge of CLKR * valid on falling edge
Just at little cleanup of davinci_i2s_set_dai_fmt
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index 8e2cdf2..c064348 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -200,36 +200,41 @@ static int davinci_i2s_startup(struct snd_pcm_substream *substream, return 0; }
+#define DEFAULT_BITPERSAMPLE 16 + static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) { struct davinci_mcbsp_dev *dev = cpu_dai->private_data; - u32 w; + unsigned int pcr; + unsigned int srgr; + unsigned int rcr; + unsigned int xcr; + srgr = DAVINCI_MCBSP_SRGR_FSGM | + DAVINCI_MCBSP_SRGR_FPER(DEFAULT_BITPERSAMPLE * 2 - 1) | + DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBS_CFS: - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, - DAVINCI_MCBSP_PCR_FSXM | - DAVINCI_MCBSP_PCR_FSRM | - DAVINCI_MCBSP_PCR_CLKXM | - DAVINCI_MCBSP_PCR_CLKRM); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, - DAVINCI_MCBSP_SRGR_FSGM); + /* cpu is master */ + pcr = DAVINCI_MCBSP_PCR_FSXM | + DAVINCI_MCBSP_PCR_FSRM | + DAVINCI_MCBSP_PCR_CLKXM | + DAVINCI_MCBSP_PCR_CLKRM; break; case SND_SOC_DAIFMT_CBM_CFS: /* McBSP CLKR pin is the input for the Sample Rate Generator. * McBSP FSR and FSX are driven by the Sample Rate Generator. */ - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, - DAVINCI_MCBSP_PCR_SCLKME | - DAVINCI_MCBSP_PCR_FSXM | - DAVINCI_MCBSP_PCR_FSRM); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, - DAVINCI_MCBSP_SRGR_FSGM); + pcr = DAVINCI_MCBSP_PCR_SCLKME | + DAVINCI_MCBSP_PCR_FSXM | + DAVINCI_MCBSP_PCR_FSRM; break; case SND_SOC_DAIFMT_CBM_CFM: - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, 0); + /* codec is master */ + pcr = 0; break; default: + printk(KERN_ERR "%s:bad master\n", __func__); return -EINVAL; }
@@ -244,10 +249,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, * FSRP Receive frame sync pol, 0 - active high * FSXP Transmit frame sync pol, 0 - active high */ - w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); - MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP | - DAVINCI_MCBSP_PCR_CLKRP, 1); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); + pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP); break; case SND_SOC_DAIFMT_IB_IF: /* CLKRP Receive clock polarity, @@ -259,10 +261,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, * FSRP Receive frame sync pol, 1 - active low * FSXP Transmit frame sync pol, 1 - active low */ - w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); - MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_FSXP | - DAVINCI_MCBSP_PCR_FSRP, 1); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); + pcr |= (DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP); break; case SND_SOC_DAIFMT_NB_IF: /* CLKRP Receive clock polarity, @@ -274,12 +273,8 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, * FSRP Receive frame sync pol, 1 - active low * FSXP Transmit frame sync pol, 1 - active low */ - w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG); - MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP | - DAVINCI_MCBSP_PCR_CLKRP | - DAVINCI_MCBSP_PCR_FSXP | - DAVINCI_MCBSP_PCR_FSRP, 1); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w); + pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP | + DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP); break; case SND_SOC_DAIFMT_IB_NF: /* CLKRP Receive clock polarity, @@ -296,28 +291,24 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, return -EINVAL; }
+ rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1); + xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1); switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_RIGHT_J: - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, - DAVINCI_MCBSP_RCR_RFRLEN1(1) | - DAVINCI_MCBSP_RCR_RDATDLY(0)); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, - DAVINCI_MCBSP_XCR_XFRLEN1(1) | - DAVINCI_MCBSP_XCR_XDATDLY(0) | - DAVINCI_MCBSP_XCR_XFIG); break; case SND_SOC_DAIFMT_I2S: - default: - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, - DAVINCI_MCBSP_RCR_RFRLEN1(1) | - DAVINCI_MCBSP_RCR_RDATDLY(1)); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, - DAVINCI_MCBSP_XCR_XFRLEN1(1) | - DAVINCI_MCBSP_XCR_XDATDLY(1) | - DAVINCI_MCBSP_XCR_XFIG); + case SND_SOC_DAIFMT_DSP_B: + rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1); + xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1); break; + default: + printk(KERN_ERR "%s:bad format\n", __func__); + return -EINVAL; } - + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr); + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr); + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr); + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr); return 0; }
@@ -343,12 +334,10 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, }
i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); - w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SRGR_REG); + w = DAVINCI_MCBSP_SRGR_FSGM; MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1), 1); - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, w);
i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS); - w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SRGR_REG); MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1), 1); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, w);
Minor, just move a block of code to make next patch clearer.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index c064348..c425c86 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -238,6 +238,21 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, return -EINVAL; }
+ rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1); + xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1); + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_RIGHT_J: + break; + case SND_SOC_DAIFMT_I2S: + case SND_SOC_DAIFMT_DSP_B: + rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1); + xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1); + break; + default: + printk(KERN_ERR "%s:bad format\n", __func__); + return -EINVAL; + } + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: /* CLKRP Receive clock polarity, @@ -290,21 +305,6 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, default: return -EINVAL; } - - rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1); - xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1); - switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { - case SND_SOC_DAIFMT_RIGHT_J: - break; - case SND_SOC_DAIFMT_I2S: - case SND_SOC_DAIFMT_DSP_B: - rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1); - xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1); - break; - default: - printk(KERN_ERR "%s:bad format\n", __func__); - return -EINVAL; - } davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr); davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
DaVinci does not support true I2S or right justified mode so not all I2S codecs will work with it when the codec is master. Document this limitation.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index c425c86..735ec86 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -241,9 +241,26 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1); xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1); switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { - case SND_SOC_DAIFMT_RIGHT_J: + case SND_SOC_DAIFMT_DSP_A: break; case SND_SOC_DAIFMT_I2S: + /* Davinci doesn't support TRUE I2S, but some codecs will have + * the left and right channels contiguous. This allows + * dsp_b mode to be used with an inverted normal frame clk. + * If your codec is master and does not have contiguous + * channels, then you will have sound on only one channel. + * Try using a different mode, or codec as slave. + * + * The TLV320AIC33 is an example of a codec where this works. + * It has a variable bit clock frequency allowing it to have + * valid data on every bit clock. + * + * The TLV320AIC23 is an example of a codec where this does not + * work. It has a fixed bit clock frequency with progressively + * more empty bit clock slots between channels as the sample + * rate is lowered. + */ + fmt ^= SND_SOC_DAIFMT_NB_IF; case SND_SOC_DAIFMT_DSP_B: rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1); xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
Sense DaVinci does not support true I2S mode and we don't have to use the hack, use dsp_a mode instead
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index a78ff59..6c44e4e 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -28,8 +28,8 @@
#define EVM_CODEC_CLOCK 22579200
-#define AUDIO_FORMAT (SND_SOC_DAIFMT_I2S | \ - SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_NB_NF) +#define AUDIO_FORMAT (SND_SOC_DAIFMT_DSP_A | \ + SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_IB_NF) static int evm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) {
On Thu, Dec 18, 2008 at 12:36:44PM -0700, Troy Kisky wrote:
Minor, just move a block of code to make next patch clearer.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied.
On Thu, Dec 18, 2008 at 12:36:43PM -0700, Troy Kisky wrote:
Just at little cleanup of davinci_i2s_set_dai_fmt
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied.
On Thu, Dec 18, 2008 at 12:36:42PM -0700, Troy Kisky wrote:
Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that used in the codec.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
I'll hold off on this and the other patches that I didn't apply for now until the disagreements have been resolved.
Mark Brown wrote:
On Thu, Dec 18, 2008 at 12:36:42PM -0700, Troy Kisky wrote:
Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that used in the codec.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
I'll hold off on this and the other patches that I didn't apply for now until the disagreements have been resolved.
Please drop the remaining patches. I was be sending revised patches to address my confusion between dsp_a and dsp_b which Jarkko pointed out.
Thanks
On Thu, Dec 18, 2008 at 12:36:41PM -0700, Troy Kisky wrote:
Document the current polarity choices.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied.
On Thu, Dec 18, 2008 at 12:36:40PM -0700, Troy Kisky wrote:
Add constants with a value of 0 to show more explicitly what is being requested.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Applied.
On Thu, Dec 18, 2008 at 9:36 PM, Troy Kisky troy.kisky@boundarydevices.comwrote:
Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B mode. Before dsp_b was really being initialized as dsp_a.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
@@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai, case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF): break; case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF):
delay = 1;case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF): iface_breg |= (0x01 << 6); break;
Is this correct? I'm looking the WM9713 spec and there DSP_A has the 1-bit delay.
But I like this idea to add another DSP format.
Jarkko
Jarkko Nikula wrote:
On Thu, Dec 18, 2008 at 9:36 PM, Troy Kisky troy.kisky@boundarydevices.comwrote:
Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B mode. Before dsp_b was really being initialized as dsp_a.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
@@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai, case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF): break; case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF):
delay = 1;case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF): iface_breg |= (0x01 << 6); break;Is this correct? I'm looking the WM9713 spec and there DSP_A has the 1-bit delay.
But I like this idea to add another DSP format.
Jarkko
I think you are correct. Thanks.
But, the AIC23 and SSM2602 would then be backwards too.
Troy
On Fri, Dec 19, 2008 at 8:04 PM, Troy Kisky troy.kisky@boundarydevices.comwrote:
Jarkko Nikula wrote:
Is this correct? I'm looking the WM9713 spec and there DSP_A has the
1-bit
delay.
But I like this idea to add another DSP format.
Jarkko
I think you are correct. Thanks.
But, the AIC23 and SSM2602 would then be backwards too.
And obviously omap-mcbsp.c too... grr, looks like I forgot to send a fix to it when fixing aic3x (commit 4b7d283150b35db6e5e10f72606f603ff424c92a). Will send a fix to omap-mcbsp, aic23 and osk5912 next week.
Yep, seems that SSM2602 should set the submode1 for DSP_B and submode2 for DSP_A, not vice versa. Worth to check then bf5xx-i2s.c as well.
Jarkko
On Sat, Dec 20, 2008 at 06:02:33PM +0200, Jarkko Nikula wrote:
And obviously omap-mcbsp.c too... grr, looks like I forgot to send a fix to it when fixing aic3x (commit 4b7d283150b35db6e5e10f72606f603ff424c92a). Will send a fix to omap-mcbsp, aic23 and osk5912 next week.
Takashi just said he'll be on holiday from the 23rd so if you could send a patch before then that'd be best, though if you don't make it then it's a sufficiently important bug fix to get merged later on.
On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
Sorry, for the long series to do something so simple, but there were unresolved objections when I first posted this to the davinci list.
What were the issues people had?
Hopefully, having this split into tiny pieces will make resolving those differences easier.
It certainly makes review easier.
Mark Brown wrote:
On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
Sorry, for the long series to do something so simple, but there were unresolved objections when I first posted this to the davinci list.
What were the issues people had?
Hopefully, having this split into tiny pieces will make resolving those differences easier.
It certainly makes review easier. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
The meaning change of SND_SOC_DAIFMT_NB_NF was not liked. And my comment that I2S mode is not supported on davinci was not liked. I'll let Naresh explain.
________________________________________ From: Troy Kisky [troy.kisky@boundarydevices.com] Sent: Friday, December 19, 2008 11:51 PM To: Mark Brown Cc: alsa-devel@alsa-project.org; davinci-linux-open-source@linux.davincidsp.com; Medisetty, Naresh Subject: Re: [alsa-devel] [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup
Mark Brown wrote:
On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
Sorry, for the long series to do something so simple, but there were unresolved objections when I first posted this to the davinci list.
What were the issues people had?
Hopefully, having this split into tiny pieces will make resolving those differences easier.
It certainly makes review easier. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.orgmailto:Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
The meaning change of SND_SOC_DAIFMT_NB_NF was not liked. And my comment that I2S mode is not supported on davinci was not liked. I'll let Naresh explain.
The comment regarding I2S mode is absolutely correct, since the davinci cannot support I2S where the codecs (like TLV320AIC23) has fixed bit clock frequency. Documenting this limitation is a very good idea.
Still I find difficulty to agree with Troy regarding the meaning change of SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t davinci McBSP.
Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.commailto:Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
On Mon, Dec 22, 2008 at 12:06:05PM +0530, Medisetty, Naresh wrote:
Still I find difficulty to agree with Troy regarding the meaning change of SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t davinci McBSP.
Could you expand on that a bit - what are you referring to as the standard here? This could just be a case of different hardware manufacturers implementing slightly different things and giving them the same name.
Mark,
On Mon, Dec 22, 2008 at 12:06:05PM +0530, Medisetty, Naresh wrote:
Still I find difficulty to agree with Troy regarding the meaning
change of
SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t
davinci
McBSP.
Could you expand on that a bit - what are you referring to as the standard here? This could just be a case of different hardware manufacturers implementing slightly different things and giving them the same name.
Yes, Exactly it's just a case of different manufacturers implementing the same thing in different way.
I am referring the davinci ASP default polarities (without inversion) of FS and BCLK as standard. In davinci ASP Frame sync signals internal to the ASP are active high. The Frame sync clock (either FSX or FSR) is XORed with Frame sync polarity bit (FSXP or FSRP). So if the requirement is for frame sync active low FSXP and FSRP bits in PCR register should be set.
Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit (CLKXP and CLKRP). Internally the transmit data driven on rising edge of CLKX and receive data sampled on falling edge of CLKR. To inverse this CLKXP and CLKRP should be set.
According to this the existing code is proper since it is not setting any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame )case.
And also in AIC33 codec there is no question of polarities since it is directly configurable through modes (either I2S or DSP).
Regards Naresh
On Tue, Dec 23, 2008 at 02:05:33PM +0530, Medisetty, Naresh wrote:
Mark,
Could you expand on that a bit - what are you referring to as the standard here? This could just be a case of different hardware manufacturers implementing slightly different things and giving them the same name.
Yes, Exactly it's just a case of different manufacturers implementing the same thing in different way.
The idea in ASoC is to follow a consistent naming scheme within ASoC rather than implement the APIs in a per-manufacturer fashion. A given DAI mode should always produce the same effect in hardware no matter which device is being configured. This makes the code much easier to work with since otherwise you'd not be able to tell from the code if the devices that are being connected are configured compatibly simply by looking at the set_dai_fmt() calls.
For most newer SoC CPUs (like the DaVinci) the drivers have to simulate the various clocking modes by configuring the chip rather than by having native support for the modes in the chip.
I am referring the davinci ASP default polarities (without inversion) of FS and BCLK as standard.
Like I say, the DaVinci chip defaults don't matter here - what matters is the mode the DAI is supposed to be operating in.
In davinci ASP Frame sync signals internal to the ASP are active high. The Frame sync clock (either FSX or FSR) is XORed with Frame sync polarity bit (FSXP or FSRP). So if the requirement is for frame sync active low FSXP and FSRP bits in PCR register should be set.
Troy left the frame clock settings alone so I'll assume you've no concerns there?
Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit (CLKXP and CLKRP). Internally the transmit data driven on rising edge of CLKX and receive data sampled on falling edge of CLKR. To inverse this CLKXP and CLKRP should be set.
According to this the existing code is proper since it is not setting any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame )case.
The DaVinci driver is configuring the clocks to match DSP mode (when using I2S mode it flips the inversion bits in the format before using them). For DSP mode data is available on the first rising edge of BCLK so sampling should be done on the rising edge. This means that the data has to be driven out on the falling edge to ensure that it is ready to be sampled on the rising edge. Troy's changes implement this so they look correct to me.
And also in AIC33 codec there is no question of polarities since it is directly configurable through modes (either I2S or DSP).
The inversions apply to the standard clocking for a given mode. For example, with I2S LRCLK clock inversion is essentially equivalent to swapping left and right channels. This facility is implemented by a reasonable number of codecs and made available through the API since it improves interoperability. Some CPUs (especially those with hand configured ports rather than native support for the clocking modes) have trouble doing some of the standard modes but can deal with inverted versions of one or both of the clocks.
[BTW, your MUA appears to be generating lines ~90-100 characters long - it'd help if it were to go for something less than 80 characters since it makes your messages easier to read and reply to with standard mail clients.]
Mark,
=20
Could you expand on that a bit - what are you referring to as the=20 standard here? This could just be a case of different hardware=20 manufacturers implementing slightly different things and giving
them
the same name.
=20
Yes, Exactly it's just a case of different manufacturers=20 implementing
the same thing in different way. =20 The idea in ASoC is to follow a consistent naming scheme within ASoC=20 rather than implement the APIs in a per-manufacturer fashion. A given=20 DAI mode should always produce the same effect in hardware no matter=20 which device is being configured. This makes the code much easier to=20 work with since otherwise you'd not be able to tell from the code if=20 the devices that are being connected are configured compatibly simply=20 by looking at the set_dai_fmt() calls. =20
_From this I came to conclusion in ASoC Normal FS means active high
and Normal BCLK means receive data sampled on rising edge and transmit=20
data driven on falling edge. Correct me if I am wrong here.
For most newer SoC CPUs (like the DaVinci) the drivers have to=20 simulate the various clocking modes by configuring the chip rather=20 than by having native support for the modes in the chip. =20
I am referring the davinci ASP default polarities (without=20 inversion)
of FS and BCLK as standard. =20 Like I say, the DaVinci chip defaults don't matter here - what matters=20 is the mode the DAI is supposed to be operating in. =20
In davinci ASP Frame sync signals internal to the ASP are active
high.
The Frame sync clock (either FSX or FSR) is XORed with Frame sync
polarity bit (FSXP or FSRP).
So if the requirement is for frame sync active low FSXP and FSRP=20 bits
in PCR register should be set. =20 Troy left the frame clock settings alone so I'll assume you've no=20 concerns there?
Yes, No concerns here
=20
Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit
(CLKXP and CLKRP).
Internally the transmit data driven on rising edge of CLKX and
receive data sampled on falling edge of CLKR.
To inverse this CLKXP and CLKRP should be set.
=20
According to this the existing code is proper since it is not=20 setting
any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame=20 )case. =20 The DaVinci driver is configuring the clocks to match DSP mode (when=20 using I2S mode it flips the inversion bits in the format before using=20 them). For DSP mode data is available on the first rising edge of=20 BCLK so sampling should be done on the rising edge. This means that=20 the data has to be driven out on the falling edge to ensure that it is=20 ready to be sampled on the rising edge. Troy's changes implement this=20 so they look correct to me. =20
Yes, its correct according to ASoC polarity standards
And also in AIC33 codec there is no question of polarities since it
is directly configurable through modes (either I2S or DSP). =20 The inversions apply to the standard clocking for a given mode. For=20 example, with I2S LRCLK clock inversion is essentially equivalent to=20 swapping left and right channels. This facility is implemented by a=20 reasonable number of codecs and made available through the API since=20 it improves interoperability. Some CPUs (especially those with hand=20 configured ports rather than native support for the clocking modes)=20 have trouble doing some of the standard modes but can deal with=20 inverted versions of one or both of the clocks.
Thanks =20
=20 [BTW, your MUA appears to be generating lines ~90-100 characters long=20
- it'd help if it were to go for something less than 80 characters=20
since it makes your messages easier to read and reply to with standard=20 mail clients.]
Sorry, I will take care of this=20
Regards Naresh=
On Wed, Dec 24, 2008 at 12:06:56PM +0530, Medisetty, Naresh wrote:
Mark,
DAI mode should always produce the same effect in hardware no matter which device is being configured. This makes the code much easier to
From this I came to conclusion in ASoC Normal FS means active high and Normal BCLK means receive data sampled on rising edge and transmit data driven on falling edge. Correct me if I am wrong here.
No. It means whatever is standard for the mode (DSP, I2S, left justified and so on) that the DAI is operating in. For DSP mode the above is accurate but the others differ. The reason why the same case statement is used for all modes in the DaVinci mode is that when I2S mode is selected the user provided bit/frame inversion options are changed before they are used.
participants (4)
-
Jarkko Nikula -
Mark Brown -
Medisetty, Naresh -
Troy Kisky