On 28/01/2020 08:59, Ben Dooks wrote:
On 27/01/2020 19:23, Dmitry Osipenko wrote:
27.01.2020 22:20, Dmitry Osipenko пишет:
24.01.2020 19:50, Jon Hunter пишет:
On 23/01/2020 19:38, Ben Dooks wrote:
On 07/01/2020 01:39, Dmitry Osipenko wrote:
06.01.2020 22:00, Ben Dooks пишет: > On 05/01/2020 10:53, Ben Dooks wrote: >> >> >> On 2020-01-05 01:48, Dmitry Osipenko wrote: >>> 05.01.2020 03:04, Ben Dooks пишет: >>>> [snip] >>>> >>>> I've just gone through testing. >>>> >>>> Some simple data tests show 16 and 32-bits work. >>>> >>>> The 24 bit case seems to be weird, it looks like the 24-bit >>>> expects >>>> 24 bit samples in 32 bit words. I can't see any packing >>>> options to >>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample >>>> support >>>> (which is a shame) >>>> >>>> My preference is to remove the 24-bit support and keep the 32 >>>> bit in. >>>> >>> >>> Interesting.. Jon, could you please confirm that 24bit format >>> isn't >>> usable on T30? >> >> If there is an option of 24 packed into 32, then I think that would >> work. >> >> I can try testing that with raw data on Monday. > > I need to check some things, I assumed 24 was 24 packed bits, it > looks > like the default is 24 in 32 bits so we may be ok. However I need to > re-write my test case which assumed it was 24bits in 3 bytes > (S24_3LE). > > I'll follow up later,
Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression. _________________________________
I did a quick test with the following:
sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav synth sine 500 vol 0.5
The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...
I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.
Ben is S24_3LE what you really need to support?
Dmitry, does the following fix your problem?
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c index dbed3c5408e7..92845c4b63f4 100644 --- a/sound/soc/tegra/tegra30_i2s.c +++ b/sound/soc/tegra/tegra30_i2s.c @@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream, audio_bits = TEGRA30_AUDIOCIF_BITS_16; sample_size = 16; break; - case SNDRV_PCM_FORMAT_S24_LE: + case SNDRV_PCM_FORMAT_S24_3LE: val = TEGRA30_I2S_CTRL_BIT_SIZE_24; audio_bits = TEGRA30_AUDIOCIF_BITS_24; sample_size = 24; @@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = { .channels_max = 2, .rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE | - SNDRV_PCM_FMTBIT_S24_LE | + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, }, .capture = { @@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = { .channels_max = 2, .rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE | - SNDRV_PCM_FMTBIT_S24_LE | + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, }, .ops = &tegra30_i2s_dai_ops,
Jon
It should solve the problem in my particular case, but I'm not sure that the solution is correct.
The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?
I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.
As far as I am aware that works and we can hit the audio rates for it.
I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.
Can you try Ben's testcase on Tegra30 (ie. generate a tone using sox and use aplay to play)?
Jon