| Message ID | 1409817173-29234-1-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fe0a29e163a5d045c73faab682a8dac71c2f8012 |
| Headers | show |
On Thu, Sep 04, 2014 at 10:52:53AM +0300, Peter Ujfalusi wrote: > In case of capture we should not use rotation. The reverse and mask is > enough to get the data align correctly from the bus to MCU: Applied, thanks.
Hey Peter, all, the patch (8e3006d02b46a722729a3bbd6a774eae99c6c8b8), causes lots of terrible noise on a custom built am33xx based board, if I map the input to the output like so: arecord -f S16_LE -r 48000 -c2 -D hw:0,1 -F 0 --period-size=102 4 -B 0 --buffer-size=4096 | aplay -D hw:0,0 - If I undo your changes, everything works fine. Tested with 3.14.23 and 3.18.1. Do you have any Idea, what might cause that behavior? regards, pascal
On 01/14/2015 01:11 PM, Pascal Huerst wrote: > Hey Peter, all, > > the patch (8e3006d02b46a722729a3bbd6a774eae99c6c8b8), causes lots of > terrible noise on a custom built am33xx based board, if I map the input > to the output like so: > > arecord -f S16_LE -r 48000 -c2 -D hw:0,1 -F 0 --period-size=102 > 4 -B 0 --buffer-size=4096 | aplay -D hw:0,0 - FWIW the -F0 and -B0 is redundant and you can use -fdat instead of -f S16_LE -r 48000 -c2 > If I undo your changes, everything works fine. Tested with 3.14.23 and > 3.18.1. You mean only this single commit? > Do you have any Idea, what might cause that behavior? No, I don't. I quickly checked on am335x, am437x and on J6 boards and the command you are using works w/o any issue or distortion in recorded audio. what codec or codecs are you using? I see that you record from hw:0,1 and play it back to hw:0,0 Can you check if the recorded sample alone is correct? Just record it to a file and look at it on the PC. I did the same and the recorded sample looks fine on my PC as well. Looking at the patch again, I still think it is a valid fix.
On 14.01.2015 13:33, Peter Ujfalusi wrote: > On 01/14/2015 01:11 PM, Pascal Huerst wrote: >> Hey Peter, all, >> >> the patch (8e3006d02b46a722729a3bbd6a774eae99c6c8b8), causes lots of >> terrible noise on a custom built am33xx based board, if I map the input >> to the output like so: >> >> arecord -f S16_LE -r 48000 -c2 -D hw:0,1 -F 0 --period-size=102 >> 4 -B 0 --buffer-size=4096 | aplay -D hw:0,0 - > > FWIW the -F0 and -B0 is redundant and you can use -fdat instead of -f S16_LE > -r 48000 -c2 Ok, good to know. >> If I undo your changes, everything works fine. Tested with 3.14.23 and >> 3.18.1. > > You mean only this single commit? Yes, even this single line. - u32 rx_rotate = 0; + u32 rx_rotate = (32 - word_length) / 4; >> Do you have any Idea, what might cause that behavior? > > No, I don't. I quickly checked on am335x, am437x and on J6 boards and the > command you are using works w/o any issue or distortion in recorded audio. > what codec or codecs are you using? I see that you record from hw:0,1 and play > it back to hw:0,0 > Can you check if the recorded sample alone is correct? Just record it to a > file and look at it on the PC. I did the same and the recorded sample looks > fine on my PC as well. if I record into a file, it behaves just the same. Noise if rx_rotate == 0, correct otherwise. I guess it is a problem in the input codec, then. input codec: AK5386 output codec: TAS5086 > Looking at the patch again, I still think it is a valid fix. Yes, I agree, the patch looks right. I'll have a look into the ak5386 and see, what impact your change could have there. thanks so far!
On 01/14/2015 03:24 PM, Pascal Huerst wrote: > Yes, even this single line. > > - u32 rx_rotate = 0; > + u32 rx_rotate = (32 - word_length) / 4; > >>> Do you have any Idea, what might cause that behavior? >> >> No, I don't. I quickly checked on am335x, am437x and on J6 boards and the >> command you are using works w/o any issue or distortion in recorded audio. > >> what codec or codecs are you using? I see that you record from hw:0,1 and play >> it back to hw:0,0 >> Can you check if the recorded sample alone is correct? Just record it to a >> file and look at it on the PC. I did the same and the recorded sample looks >> fine on my PC as well. > > if I record into a file, it behaves just the same. Noise if > rx_rotate == 0, correct otherwise. I guess it is a problem in the input > codec, then. > > input codec: AK5386 > output codec: TAS5086 > >> Looking at the patch again, I still think it is a valid fix. > > Yes, I agree, the patch looks right. > > I'll have a look into the ak5386 and see, what impact your change could > have there. Can you try S24_LE format as well? I think it should work. I assume you are running the AK5386 as master which would explain the symptoms. In this mode you have 64bclk per sample (32 per channel). I believe this should fix your capture in case of S16_LE: keep the (in linux-next): fe0a29e163a5d045c73faab682a8dac71c2f8012 : ASoC: davinci-mcasp: Correct rx format unit configuration make sure you have (as in linux-next): d742b925244ce91f16d380befdca473e4536359b : ASoC: davinci-mcasp: Fix rx format when more bclk is used on the bus add the following to your machine driver's hw_params callback: ... snd_soc_dai_set_clkdiv(cpu_dai, 2, 64); ... I think the issue with your setup is that McASP expects the you have 32bclk per sample, but AK5386 generates 64 for you, this will mess up the data you are receiving since in this case we need to shift the data with 16bits before we invert it.
On 14.01.2015 15:17, Peter Ujfalusi wrote: > On 01/14/2015 03:24 PM, Pascal Huerst wrote: >> Yes, even this single line. >> >> - u32 rx_rotate = 0; >> + u32 rx_rotate = (32 - word_length) / 4; >> >>>> Do you have any Idea, what might cause that behavior? >>> >>> No, I don't. I quickly checked on am335x, am437x and on J6 boards and the >>> command you are using works w/o any issue or distortion in recorded audio. >> >>> what codec or codecs are you using? I see that you record from hw:0,1 and play >>> it back to hw:0,0 >>> Can you check if the recorded sample alone is correct? Just record it to a >>> file and look at it on the PC. I did the same and the recorded sample looks >>> fine on my PC as well. >> >> if I record into a file, it behaves just the same. Noise if >> rx_rotate == 0, correct otherwise. I guess it is a problem in the input >> codec, then. >> >> input codec: AK5386 >> output codec: TAS5086 >> >>> Looking at the patch again, I still think it is a valid fix. >> >> Yes, I agree, the patch looks right. >> >> I'll have a look into the ak5386 and see, what impact your change could >> have there. > > Can you try S24_LE format as well? I think it should work. If I record to a file everything works fine, no noise. But I can not map to the output, since the output codec does not support S24_LE > I assume you are running the AK5386 as master which would explain the > symptoms. In this mode you have 64bclk per sample (32 per channel). no I'm not. it's running in slave mode. CKS[0-2] = 0 (GND). > I believe this should fix your capture in case of S16_LE: > keep the (in linux-next): > fe0a29e163a5d045c73faab682a8dac71c2f8012 : ASoC: davinci-mcasp: Correct rx > format unit configuration > > make sure you have (as in linux-next): > d742b925244ce91f16d380befdca473e4536359b : ASoC: davinci-mcasp: Fix rx format > when more bclk is used on the bus Yes. Although AK5386 is in slave mode, this fixed the issue! > add the following to your machine driver's hw_params callback: > > ... > snd_soc_dai_set_clkdiv(cpu_dai, 2, 64); > ... That line was there already. > I think the issue with your setup is that McASP expects the you have 32bclk > per sample, but AK5386 generates 64 for you, this will mess up the data you > are receiving since in this case we need to shift the data with 16bits before > we invert it. Ok. I think I will have to look deeper into that, to really understand, where these 64bclk came from in my case (assuming its set in the machine driver). But anyways, thanks a lot for your help! pascal
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 6a6b2ff7d7d7..68347b55f6e1 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -467,8 +467,17 @@ static int davinci_config_channel_size(struct davinci_mcasp *mcasp, { u32 fmt; u32 tx_rotate = (word_length / 4) & 0x7; - u32 rx_rotate = (32 - word_length) / 4; u32 mask = (1ULL << word_length) - 1; + /* + * For captured data we should not rotate, inversion and masking is + * enoguh to get the data to the right position: + * Format data from bus after reverse (XRBUF) + * S16_LE: |LSB|MSB|xxx|xxx| |xxx|xxx|MSB|LSB| + * S24_3LE: |LSB|DAT|MSB|xxx| |xxx|MSB|DAT|LSB| + * S24_LE: |LSB|DAT|MSB|xxx| |xxx|MSB|DAT|LSB| + * S32_LE: |LSB|DAT|DAT|MSB| |MSB|DAT|DAT|LSB| + */ + u32 rx_rotate = 0; /* * if s BCLK-to-LRCLK ratio has been configured via the set_clkdiv()
In case of capture we should not use rotation. The reverse and mask is enough to get the data align correctly from the bus to MCU: Format data from bus after reverse (XRBUF) S16_LE: |LSB|MSB|xxx|xxx| |xxx|xxx|MSB|LSB| S24_3LE: |LSB|DAT|MSB|xxx| |xxx|MSB|DAT|LSB| S24_LE: |LSB|DAT|MSB|xxx| |xxx|MSB|DAT|LSB| S32_LE: |LSB|DAT|DAT|MSB| |MSB|DAT|DAT|LSB| With this patch all supported formats will work for playback and capture. Reported-by: Jyri Sarha <jsarha@ti.com> (broken S24_3LE capture) Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- sound/soc/davinci/davinci-mcasp.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)