[PATCH v2 0/2] fix tegra-hda on tegra30 devices

[PATCH v2 0/2] fix tegra-hda on tegra30 devices

[Peter Geis]

The following patches fix tegra-hda on legacy tegra devices.
Two issues were discovered preventing tegra-hda from functioning:
-The hda clocks on tegra30 were assigned to clk_m and running at too low
of a rate to function.
-The tegra-hda encounters an input/output error when opening a stream.

Since the only mainline device that used tegra-hda until recently was the
t124, it is unknown exactly when this was broken. Fortunately a recent
patch was submitted that fixed the issue only on t194 devices. We can
apply it to the tegra30-hda device to resolve the issue across the board.
Note that downstream devices used the spdif device instead of hda for hdmi
audio. The spdif device lacks a driver on mainline.

-Checkpatch seems to have issues finding [1], but git show has no issue.
[1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
Tegra194")

Changelog:

v2:
-Added ack and reviewed-by from Jon
-Updated fix to apply to tegra30-hda vice universally (Thanks Jon)
-Updated commit to include comments from hardware team (Thanks Sameer)
-Cleaned up commit messages

Peter Geis (2):
  clk: tegra30: Add hda clock default rates to clock driver
  ALSA: hda/tegra: fix tegra-hda on tegra30 soc

 drivers/clk/tegra/clk-tegra30.c | 2 ++
 sound/pci/hda/hda_tegra.c       | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v2 1/2] clk: tegra30: Add hda clock default rates to clock driver

[Peter Geis]

Current implementation defaults the hda clocks to clk_m. This causes hda
to run too slow to operate correctly. Fix this by defaulting to pll_p and
setting the frequency to the correct rate.

This matches upstream t124 and downstream t30.

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Ion Agorria <ion@agorria.com>
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 drivers/clk/tegra/clk-tegra30.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 37244a7e68c2..9cf249c344d9 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1256,6 +1256,8 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_I2S3_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
 	{ TEGRA30_CLK_I2S4_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
 	{ TEGRA30_CLK_VIMCLK_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
+	{ TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 102000000, 0 },
+	{ TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	/* must be the last entry */
 	{ TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
 };
-- 
2.25.1

Re: [PATCH v2 1/2] clk: tegra30: Add hda clock default rates to clock driver

[Sameer Pujar]

On 1/8/2021 7:29 PM, Peter Geis wrote:
> External email: Use caution opening links or attachments
>
>
> Current implementation defaults the hda clocks to clk_m. This causes hda
> to run too slow to operate correctly. Fix this by defaulting to pll_p and
> setting the frequency to the correct rate.
>
> This matches upstream t124 and downstream t30.
>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>   drivers/clk/tegra/clk-tegra30.c | 2 ++
>   1 file changed, 2 insertions(+)

Acked-by: Sameer Pujar <spujar@nvidia.com>

Re: [PATCH v2 1/2] clk: tegra30: Add hda clock default rates to clock driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

On Fri, Jan 08, 2021 at 01:59:12PM +0000, Peter Geis wrote:
> Current implementation defaults the hda clocks to clk_m. This causes hda
> to run too slow to operate correctly. Fix this by defaulting to pll_p and
> setting the frequency to the correct rate.
> 
> This matches upstream t124 and downstream t30.
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  drivers/clk/tegra/clk-tegra30.c | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Peter Geis]

Currently hda on tegra30 fails to open a stream with an input/output error.

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

The tegra-hda device was introduced in tegra30 but only utilized in
tegra124 until recent chips. Tegra210/186 work only due to a hardware
change. For this reason it is unknown when this issue first manifested.
Discussions with the hardware team show this applies to all current tegra
chips. It has been resolved in the tegra234, which does not have hda
support at this time.

The explanation from the hardware team is this:
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.

Applying the fix in [1] universally resolves this issue on tegra30-hda.
Tested on the Ouya game console and the tf201 tablet.

[1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
Tegra194")

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Ion Agorria <ion@agorria.com>
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 sound/pci/hda/hda_tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 70164d1428d4..361cf2041911 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -388,7 +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"))
+	if (of_device_is_compatible(np, "nvidia,tegra30-hda"))
 		chip->bus.core.sdo_limit = 16;
 
 	/* codec detection */
-- 
2.25.1

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

[Sameer Pujar]

On 1/8/2021 7:29 PM, 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.
>
> 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
>
> The tegra-hda device was introduced in tegra30 but only utilized in
> tegra124 until recent chips. Tegra210/186 work only due to a hardware
> change. For this reason it is unknown when this issue first manifested.
> Discussions with the hardware team show this applies to all current tegra
> chips. It has been resolved in the tegra234, which does not have hda
> support at this time.
>
> The explanation from the hardware team is this:
> 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.
>
> Applying the fix in [1] universally resolves this issue on tegra30-hda.
> Tested on the Ouya game console and the tf201 tablet.
>
> [1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
> Tegra194")
>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>   sound/pci/hda/hda_tegra.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Thanks Peter.

Reviewed-by: Sameer Pujar <spujar@nvidia.com>

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

On Fri, Jan 08, 2021 at 01:59:13PM +0000, Peter Geis wrote:
> Currently hda on tegra30 fails to open a stream with an input/output error.
> 
> 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
> 
> The tegra-hda device was introduced in tegra30 but only utilized in
> tegra124 until recent chips. Tegra210/186 work only due to a hardware
> change. For this reason it is unknown when this issue first manifested.
> Discussions with the hardware team show this applies to all current tegra
> chips. It has been resolved in the tegra234, which does not have hda
> support at this time.
> 
> The explanation from the hardware team is this:
> 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.
> 
> Applying the fix in [1] universally resolves this issue on tegra30-hda.
> Tested on the Ouya game console and the tf201 tablet.
> 
> [1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
> Tegra194")
> 
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  sound/pci/hda/hda_tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
> index 70164d1428d4..361cf2041911 100644
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -388,7 +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"))
> +	if (of_device_is_compatible(np, "nvidia,tegra30-hda"))
>  		chip->bus.core.sdo_limit = 16;

I think ideally this would be a SoC-specific property derived from the
compatible string, but since this preexisting:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] fix tegra-hda on tegra30 devices

[Takashi Iwai]

On Fri, 08 Jan 2021 14:59:11 +0100,
Peter Geis wrote:
> 
> The following patches fix tegra-hda on legacy tegra devices.
> Two issues were discovered preventing tegra-hda from functioning:
> -The hda clocks on tegra30 were assigned to clk_m and running at too low
> of a rate to function.
> -The tegra-hda encounters an input/output error when opening a stream.
> 
> Since the only mainline device that used tegra-hda until recently was the
> t124, it is unknown exactly when this was broken. Fortunately a recent
> patch was submitted that fixed the issue only on t194 devices. We can
> apply it to the tegra30-hda device to resolve the issue across the board.
> Note that downstream devices used the spdif device instead of hda for hdmi
> audio. The spdif device lacks a driver on mainline.
> 
> -Checkpatch seems to have issues finding [1], but git show has no issue.
> [1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
> Tegra194")
> 
> Changelog:
> 
> v2:
> -Added ack and reviewed-by from Jon
> -Updated fix to apply to tegra30-hda vice universally (Thanks Jon)
> -Updated commit to include comments from hardware team (Thanks Sameer)
> -Cleaned up commit messages
> 
> Peter Geis (2):
>   clk: tegra30: Add hda clock default rates to clock driver
>   ALSA: hda/tegra: fix tegra-hda on tegra30 soc

Shall I apply both patches via sound git tree?

Or, if you want to take through clk tree, let me know.  In that case,
feel free to take my ack:
Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

Re: [PATCH v2 0/2] fix tegra-hda on tegra30 devices

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

On Tue, Jan 12, 2021 at 09:22:38AM +0100, Takashi Iwai wrote:
> On Fri, 08 Jan 2021 14:59:11 +0100,
> Peter Geis wrote:
> > 
> > The following patches fix tegra-hda on legacy tegra devices.
> > Two issues were discovered preventing tegra-hda from functioning:
> > -The hda clocks on tegra30 were assigned to clk_m and running at too low
> > of a rate to function.
> > -The tegra-hda encounters an input/output error when opening a stream.
> > 
> > Since the only mainline device that used tegra-hda until recently was the
> > t124, it is unknown exactly when this was broken. Fortunately a recent
> > patch was submitted that fixed the issue only on t194 devices. We can
> > apply it to the tegra30-hda device to resolve the issue across the board.
> > Note that downstream devices used the spdif device instead of hda for hdmi
> > audio. The spdif device lacks a driver on mainline.
> > 
> > -Checkpatch seems to have issues finding [1], but git show has no issue.
> > [1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
> > Tegra194")
> > 
> > Changelog:
> > 
> > v2:
> > -Added ack and reviewed-by from Jon
> > -Updated fix to apply to tegra30-hda vice universally (Thanks Jon)
> > -Updated commit to include comments from hardware team (Thanks Sameer)
> > -Cleaned up commit messages
> > 
> > Peter Geis (2):
> >   clk: tegra30: Add hda clock default rates to clock driver
> >   ALSA: hda/tegra: fix tegra-hda on tegra30 soc
> 
> Shall I apply both patches via sound git tree?
> 
> Or, if you want to take through clk tree, let me know.  In that case,
> feel free to take my ack:
> Acked-by: Takashi Iwai <tiwai@suse.de>

There doesn't seem to be a build-time (or even runtime) dependency
between the two patches and they fix two independent issues, so they
could also go in separately.

I'm fine either way. From a Tegra perspective, the Tegra30 clock driver
is very low activity, so I don't expect any conflicts with between this
and some other patches that might go in through the clock tree.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] fix tegra-hda on tegra30 devices

[Takashi Iwai]

On Tue, 12 Jan 2021 14:00:37 +0100,
Thierry Reding wrote:
> 
> On Tue, Jan 12, 2021 at 09:22:38AM +0100, Takashi Iwai wrote:
> > On Fri, 08 Jan 2021 14:59:11 +0100,
> > Peter Geis wrote:
> > > 
> > > The following patches fix tegra-hda on legacy tegra devices.
> > > Two issues were discovered preventing tegra-hda from functioning:
> > > -The hda clocks on tegra30 were assigned to clk_m and running at too low
> > > of a rate to function.
> > > -The tegra-hda encounters an input/output error when opening a stream.
> > > 
> > > Since the only mainline device that used tegra-hda until recently was the
> > > t124, it is unknown exactly when this was broken. Fortunately a recent
> > > patch was submitted that fixed the issue only on t194 devices. We can
> > > apply it to the tegra30-hda device to resolve the issue across the board.
> > > Note that downstream devices used the spdif device instead of hda for hdmi
> > > audio. The spdif device lacks a driver on mainline.
> > > 
> > > -Checkpatch seems to have issues finding [1], but git show has no issue.
> > > [1] commit 60019d8c650d ("ALSA: hda/tegra: workaround playback failure on
> > > Tegra194")
> > > 
> > > Changelog:
> > > 
> > > v2:
> > > -Added ack and reviewed-by from Jon
> > > -Updated fix to apply to tegra30-hda vice universally (Thanks Jon)
> > > -Updated commit to include comments from hardware team (Thanks Sameer)
> > > -Cleaned up commit messages
> > > 
> > > Peter Geis (2):
> > >   clk: tegra30: Add hda clock default rates to clock driver
> > >   ALSA: hda/tegra: fix tegra-hda on tegra30 soc
> > 
> > Shall I apply both patches via sound git tree?
> > 
> > Or, if you want to take through clk tree, let me know.  In that case,
> > feel free to take my ack:
> > Acked-by: Takashi Iwai <tiwai@suse.de>
> 
> There doesn't seem to be a build-time (or even runtime) dependency
> between the two patches and they fix two independent issues, so they
> could also go in separately.

Yes.  OTOH, it's better to be applied in a shot when you look back at
the changes later.

> I'm fine either way. From a Tegra perspective, the Tegra30 clock driver
> is very low activity, so I don't expect any conflicts with between this
> and some other patches that might go in through the clock tree.

OK, then I queued both patches now to for-linus branch in sound tree.


Thanks!

Takashi