Re: [PATCH 2/2] ALSA: hda/tegra: fix tegra-hda on tegra30 soc

[Sameer Pujar <spujar@nvidia.com>]

On 1/7/2021 2:51 AM, Peter Geis wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Jan 5, 2021 at 1:30 AM Sameer Pujar <spujar@nvidia.com> wrote:
>>
>>
>> On 12/25/2020 6:50 AM, Peter Geis wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> Currently hda on tegra30 fails to open a stream with an input/output error.
>>> This is similar to the issue referenced in [1].
>>>
>>> For example:
>>> speaker-test -Dhw:0,3 -c 2
>>>
>>> speaker-test 1.2.2
>>>
>>> Playback device is hw:0,3
>>> Stream parameters are 48000Hz, S16_LE, 2 channels
>>> Using 16 octaves of pink noise
>>> Rate set to 48000Hz (requested 48000Hz)
>>> Buffer size range from 64 to 16384
>>> Period size range from 32 to 8192
>>> Using max buffer size 16384
>>> Periods = 4
>>> was set period_size = 4096
>>> was set buffer_size = 16384
>>>    0 - Front Left
>>> Write error: -5,Input/output error
>>> xrun_recovery failed: -5,Input/output error
>>> Transfer failed: Input/output error
>>>
>>> [1] states "Due to a legacy HW design problem", implying the issue applies to all previous tegra-hda devices.
>>> The tegra-hda device was introduced in tegra30 but only utilized in tegra124 until now.
>>> For this reason it is unknown when this issue first manifested.
>>>
>>> Applying the fix in [1] universally resolves this issue on tegra30.
>>> Tested on the Ouya game console and the tf201 tablet.
>>>
>>> [1] 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on Tegra194")
>> This issue was never seen on Tegra210/Tegra186 and hence at that time it
>> was thought to be specific to Tegra194. I never tested this on Tegra30
>> since I don't have this device. I will clarify this with HW folks if
>> workaround is safer for all chips.
> So this is confirmed to not affect Tegra210 and Tegra186, but it does
> affect Tegra194 and Tegra30.
> Is it possible for the hardware team to pitch on on where the fix was
> implemented?

I discussed this with HW team. The issue is applicable to all Tegra chips.

Below is the striping formula referenced from HD audio spec.
   { ((num_channels * bits_per_sample) / number of SDOs) >= 8 }

The current issue is seen because Tegra HW has a problem with boundary 
condition (= 8) for striping. The reason why it is not seen on 
Tegra210/Tegra186 is because it uses max 2SDO lines. Max SDO lines is 
read from GCAP register.

For the given stream (channels = 2, bps = 16);
ratio = (channels * bps) / NSDO = 32 / NSDO;

On Tegra30,      ratio = 32/4 = 8  (FAIL)
On Tegra210/186, ratio = 32/2 = 16 (PASS)
On Tegra194,     ratio = 32/4 = 8  (FAIL) ==> Earlier workaround was 
applied for it

If Tegra210/186 is forced to use 4SDO, it fails there as well. So the 
behavior is consistent across all these chips.

>>> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
>>> Tested-by: Ion Agorria <ion@agorria.com>
>>> ---
>>>    sound/pci/hda/hda_tegra.c | 3 +--
>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
>>> index 70164d1428d4..f8d61e677a09 100644
>>> --- a/sound/pci/hda/hda_tegra.c
>>> +++ b/sound/pci/hda/hda_tegra.c
>>> @@ -388,8 +388,7 @@ static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
>>>            * in powers of 2, next available ratio is 16 which can be
>>>            * used as a limiting factor here.
>>>            */
>>> -       if (of_device_is_compatible(np, "nvidia,tegra194-hda"))
>>> -               chip->bus.core.sdo_limit = 16;
>>> +       chip->bus.core.sdo_limit = 16;
>> Future Tegra chips address this problem and hence cannot be enforced by
>> default. May be we can have like below:
>>
>> if (of_device_is_compatible(np, "nvidia,tegra30-hda"))
>> chip->bus.core.sdo_limit = 16;
>>
> It will need to be a bit more complicated than that, since the
> tegra186 and tegra210 device trees have "nvidia,tegra30-hda" as a
> fallback.
> Looking at the generation map, tegra30-hda can be the fallback for the
> broken implementation and tegra210-hda can be the fallback for the
> working implementation.
> Does that work for you?

As per above explanation, it is fine to apply the workaround for 
Tegra210/186 as well. So it simplifies things for all existing chips.

>>>           /* codec detection */
>>>           if (!bus->codec_mask) {
>>> --
>>> 2.25.1
>>>