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.
