[alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

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.