[alsa-devel] [PATCH 0/3] ASoC: omap-mcbsp, osk5912 and tlv320aic23 DSP mode change
Hello,
While I was trying to implement the four channel mode for the twl4030 codec, I have noticed, that the DSP_B mode implementation in omap-mcbsp and tlv320aic23 is not coorect, also for me it seams that the osk5912 configuration for the DSP mode is incorrect (based on the tlv320aic32 datasheet).
The following series tries to correct this by: - Adding DSP_A support for the codec (setting the LRP bit for the DSP mode) - Changing the DSP_B mode to DSP_A mode in omap-mcbsp - Correcting the dai format configuration for the osk5912 board file.
Please note, that I don't have access to osk5912 board, but based on the available documentation, I think the changes in this series are correct.
--- Peter Ujfalusi (3): ASoC: tlv320aic32: add DSP_A format support ASoC: omap-mcbsp: change the DSP_B mode to DSP_A mode ASoC: osk5912: Use the correct DSP_A mode
sound/soc/codecs/tlv320aic23.c | 2 ++ sound/soc/omap/omap-mcbsp.c | 4 ++-- sound/soc/omap/osk5912.c | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-)
Add DSP_A interface format support by setting the LRP bit in DSP mode.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320aic23.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic23.c b/sound/soc/codecs/tlv320aic23.c index c3f4afb..21f69df 100644 --- a/sound/soc/codecs/tlv320aic23.c +++ b/sound/soc/codecs/tlv320aic23.c @@ -523,6 +523,8 @@ static int tlv320aic23_set_dai_fmt(struct snd_soc_dai *codec_dai, case SND_SOC_DAIFMT_I2S: iface_reg |= TLV320AIC23_FOR_I2S; break; + case SND_SOC_DAIFMT_DSP_A: + iface_reg |= TLV320AIC23_LRP_ON; case SND_SOC_DAIFMT_DSP_B: iface_reg |= TLV320AIC23_FOR_DSP; break;
The current DSP_B configuration is valid for the DSP_A format.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 9c09b94..aa41fba 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -281,7 +281,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, regs->srgr2 |= FPER(wlen * 2 - 1); regs->srgr1 |= FWID(wlen - 1); break; - case SND_SOC_DAIFMT_DSP_B: + case SND_SOC_DAIFMT_DSP_A: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(wlen * channels - 2); break; @@ -324,7 +324,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai, regs->rcr2 |= RDATDLY(1); regs->xcr2 |= XDATDLY(1); break; - case SND_SOC_DAIFMT_DSP_B: + case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
Use DSP_A format with correct clock, frame sync polarity.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/osk5912.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/omap/osk5912.c b/sound/soc/omap/osk5912.c index a952a4e..d67e77e 100644 --- a/sound/soc/omap/osk5912.c +++ b/sound/soc/omap/osk5912.c @@ -61,8 +61,8 @@ static int osk_hw_params(struct snd_pcm_substream *substream,
/* Set codec DAI configuration */ err = snd_soc_dai_set_fmt(codec_dai, - SND_SOC_DAIFMT_DSP_B | - SND_SOC_DAIFMT_NB_IF | + SND_SOC_DAIFMT_DSP_A | + SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM); if (err < 0) { printk(KERN_ERR "can't set codec DAI configuration\n"); @@ -71,8 +71,8 @@ static int osk_hw_params(struct snd_pcm_substream *substream,
/* Set cpu DAI configuration */ err = snd_soc_dai_set_fmt(cpu_dai, - SND_SOC_DAIFMT_DSP_B | - SND_SOC_DAIFMT_NB_IF | + SND_SOC_DAIFMT_DSP_A | + SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM); if (err < 0) { printk(KERN_ERR "can't set cpu DAI configuration\n");
On Thu, 9 Apr 2009 11:34:41 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
- case SND_SOC_DAIFMT_DSP_B:
- case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
--
Are you absolutely sure on this? According to WM9713, it's the DSP_A where MSB is valid after 1 bit clock period of the FS.
On Thursday 09 April 2009 13:00:14 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Thu, 9 Apr 2009 11:34:41 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
- case SND_SOC_DAIFMT_DSP_B:
- case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
--
Are you absolutely sure on this? According to WM9713, it's the DSP_A where MSB is valid after 1 bit clock period of the FS.
I think this is correct. The data is going to be valid exactly the same way, as the WM9713 data sheet describes.
As for the DSP_B mode: I think it can be implemented like this: Invert the frame sync polarity, Then: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); /* FS pulse width is 1 */ break;
So the MSB will be in the correct place.
On Thursday 09 April 2009 13:16:55 Ujfalusi Peter (Nokia-D/Tampere) wrote:
On Thursday 09 April 2009 13:00:14 Nikula Jarkko (Nokia-D/Helsinki) wrote:
On Thu, 9 Apr 2009 11:34:41 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
- case SND_SOC_DAIFMT_DSP_B:
- case SND_SOC_DAIFMT_DSP_A: /* 0-bit data delay */ regs->rcr2 |= RDATDLY(0); regs->xcr2 |= XDATDLY(0);
--
Are you absolutely sure on this? According to WM9713, it's the DSP_A where MSB is valid after 1 bit clock period of the FS.
I think this is correct. The data is going to be valid exactly the same way, as the WM9713 data sheet describes.
As for the DSP_B mode: I think it can be implemented like this: Invert the frame sync polarity, Then: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); /* FS pulse width is 1 */ break;
So the MSB will be in the correct place.
BTW: I have draw it to paper the following to verified these: 1) how mcbsp would operate with the osk5912 config (as it was): snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_IF | SND_SOC_DAIFMT_CBM_CFM);
Than after the change: 2) how mcbsp would operate with the osk5912 config (my proposal): err = snd_soc_dai_set_fmt(codec_dai, snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_IF | SND_SOC_DAIFMT_CBM_CFM);
3) how mcbsp would operate with the omap3beagle 4 channel mode: fmt = SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF | SND_SOC_DAIFMT_CBM_CFM;
Than I compared these drawings to the pictures in the data sheets (tlv320aic32, twl4030): 1) Does not match at all 2) 100% match 3) 100% match
I think if the osk5912 wants to use the codec with DSP_B mode it should have: snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_IF | SND_SOC_DAIFMT_CBM_CFM);
Than in omap-mcbsp: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); /* FS pulse width is 1 */ break;
Than it would have been correct, I think. Probably I should send the DSP_B support also in the series?
On Thu, 9 Apr 2009 12:16:55 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Are you absolutely sure on this? According to WM9713, it's the DSP_A where MSB is valid after 1 bit clock period of the FS.
I think this is correct. The data is going to be valid exactly the same way, as the WM9713 data sheet describes.
I'll try to find working commit where my Beagle is still booting since it wasn't with the head of linux-omap and then playing a bit with McBSP3 which is available in pin header of the Beagle so we can measure this out easily.
As for the DSP_B mode: I think it can be implemented like this: Invert the frame sync polarity, Then: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); /* FS pulse width is 1
Looks nice idea. BTW, I had temporary inversion in use in the commit da6320becf31c40b60d4b1dc6b339c9a766b671c.
Jarkko
On Thu, 9 Apr 2009 12:16:55 +0200 "Ujfalusi Peter (Nokia-D/Tampere)" peter.ujfalusi@nokia.com wrote:
Are you absolutely sure on this? According to WM9713, it's the DSP_A where MSB is valid after 1 bit clock period of the FS.
I think this is correct. The data is going to be valid exactly the same way, as the WM9713 data sheet describes.
As for the DSP_B mode: I think it can be implemented like this: Invert the frame sync polarity, Then: case SND_SOC_DAIFMT_DSP_B: regs->srgr2 |= FPER(wlen * channels - 1); regs->srgr1 |= FWID(0); /* FS pulse width is 1 */ break;
So the MSB will be in the correct place.
Peter is right. There is a bug in omap-mcbsp.c. If I play maximum negative samples "aplay -f U8 /dev/zero", the MSB bit is then just after the falling edge of FS like in DSP_A format... (hmm, what I was doing in the commit bd25867a6cbe7a00ef7dbe8d9ddebc91b00b9b3f).
Is it ok to leave this over the easter to make sure next week that we'll get this finally right? I have had already few DSP format fixes there and still this is not correct... :-)
On Thu, Apr 09, 2009 at 03:30:42PM +0300, Jarkko Nikula wrote:
Is it ok to leave this over the easter to make sure next week that we'll get this finally right? I have had already few DSP format fixes there and still this is not correct... :-)
Yes, seems sensible.
On Thu, Apr 09, 2009 at 12:34:40PM +0300, Peter Ujfalusi wrote:
Add DSP_A interface format support by setting the LRP bit in DSP mode.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Applied, thanks.
participants (3)
-
Jarkko Nikula -
Mark Brown -
Peter Ujfalusi