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

[Ben Dooks]

On 29/01/2020 14:33, Jon Hunter wrote:
> 
> On 29/01/2020 10:49, Jon Hunter wrote:
>>
>> On 28/01/2020 12:13, Mark Brown wrote:
>>> I really don't understand why this is all taking so long, this thread
>>> just seems to be going round in interminable circles long after it
>>> looked like the issue was understood.  I have to admit I've not read
>>> every single message in the thread but it's difficult to see why it
>>> doesn't seem to be making any progress.
>>
>> Sorry about that. On reviewing this with the audio team at NVIDIA, I was
>> told we don't support S24_LE for I2S. The reason being that the crossbar
>> between the DMA and I2S is not able to extract the correct 24-bits from
>> the 32-bit sample to feed to the I2S interface. The Tegra documentation
>> does show support for 24-bits, but not state explicit support for S24_LE.
>>
>> Now Ben says that he has this working, but I am unable to reproduce
>> this, so before just dropping the S24_LE support, I would like to
>> understand how this is working for Ben in case there is something that
>> we have overlooked here.
> 
> Ah, I see that part of the problem is that patches 6 and 7 are yet to be
> applied and without these the audio is completely distorted because
> there is a mismatch in the data size between the APBIF and I2S
> controller. Applying these patches it is not distorted but now I am
> observing the clocking issue Ben reported and so the tone is not quite
> right.

I thought they had been applied? I probably dragged them back in when
putting in the support for the test channel on the colibri.

> Ben, I was able to workaround the clocking issue by making the I2S word
> clock 64 bits long and not 48.

Ok, that will work for I2S case, but maybe not TDM? I'd have to check.

> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index bbf81b5aa723..3c9b4779e61b 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -143,7 +143,7 @@ static int tegra30_i2s_hw_params(struct
> snd_pcm_substream *substream,
>          case SNDRV_PCM_FORMAT_S24_LE:
>                  val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
>                  audio_bits = TEGRA30_AUDIOCIF_BITS_24;
> -               sample_size = 24;
> +               sample_size = 32;
>                  break;
>          case SNDRV_PCM_FORMAT_S32_LE:
>                  val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
> 
> For I2S I believe we only care about the edges of the word clock and so
> we make the overall period of the word clock 64 bit clocks then we no
> longer have an issue with the bit clock frequency. I assume that this
> should also be fine for TDM modes as well.
> 
> Can you let me know if this works for you?

I'll be back in the office next week to test.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html