28.01.2020 16:19, Jon Hunter пишет: > > 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. Have you tried to replicate my case by playing the video file? > Can you try Ben's testcase on Tegra30 (ie. generate a tone using sox and > use aplay to play)? Surely I can try, but only if you'll share the sample file with me and give precise instructions how to test it.