| Message ID | 1559121501-8566-1-git-send-email-jonathanh@nvidia.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | clk: tegra210: Fix default rates for HDA clocks | expand |
On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote: > Currently the default clock rates for the HDA and HDA2CODEC_2X clocks > are both 19.2MHz. However, the default rates for these clocks should > actually be 51MHz and 48MHz, respectively. Correct the default clock > rates for these clocks by specifying them in the clock init table for > Tegra210. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/clk/tegra/clk-tegra210.c | 2 ++ > 1 file changed, 2 insertions(+) Does this fix anything? Should this be backported to stable releases? Acked-by: Thierry Reding <treding@nvidia.com>
On 29/05/2019 14:46, Thierry Reding wrote: > On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote: >> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks >> are both 19.2MHz. However, the default rates for these clocks should >> actually be 51MHz and 48MHz, respectively. Correct the default clock >> rates for these clocks by specifying them in the clock init table for >> Tegra210. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> drivers/clk/tegra/clk-tegra210.c | 2 ++ >> 1 file changed, 2 insertions(+) > > Does this fix anything? Should this be backported to stable releases? Good point. We are aligning the clock configuration with what we ship. So I thought for completeness it would be good to test HDA playback across the various sample-rates we support (32kHz to 192kHz) but with or without this patch I am not hearing anything. Let me check on this with Sameer as I would like to see if we need to mark this for stable or not. > Acked-by: Thierry Reding <treding@nvidia.com> Thanks Jon
On 31/05/2019 15:58, Jon Hunter wrote: > > On 29/05/2019 14:46, Thierry Reding wrote: >> On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote: >>> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks >>> are both 19.2MHz. However, the default rates for these clocks should >>> actually be 51MHz and 48MHz, respectively. Correct the default clock >>> rates for these clocks by specifying them in the clock init table for >>> Tegra210. >>> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> drivers/clk/tegra/clk-tegra210.c | 2 ++ >>> 1 file changed, 2 insertions(+) >> >> Does this fix anything? Should this be backported to stable releases? > > Good point. We are aligning the clock configuration with what we ship. > So I thought for completeness it would be good to test HDA playback > across the various sample-rates we support (32kHz to 192kHz) but with or > without this patch I am not hearing anything. Let me check on this with > Sameer as I would like to see if we need to mark this for stable or not. > >> Acked-by: Thierry Reding <treding@nvidia.com> I have confirmed that this does fix HDA playback on Tegra210. Without this fix, I am seeing the following messages during playback and playback is distorted ... Write error: -32,Broken pipe [ 15.069335] tegra-mc 70019000.memory-controller: hdar: read @0x0000000000000000: EMEM address decode error (EMEM decode error) Write error: -32,Broken pipe [ 15.465362] tegra-mc 70019000.memory-controller: hdar: read @0x0000000000000000: EMEM address decode error (EMEM decode error) Write error: -32,Broken pipe [ 15.858615] tegra-mc 70019000.memory-controller: hdar: read @0x0000000000000000: EMEM address decode error (EMEM decode error) W Do you want me to update the change and resend? Jon
On Wed, Jun 05, 2019 at 12:30:31PM +0100, Jon Hunter wrote: > > On 31/05/2019 15:58, Jon Hunter wrote: > > > > On 29/05/2019 14:46, Thierry Reding wrote: > >> On Wed, May 29, 2019 at 10:18:21AM +0100, Jon Hunter wrote: > >>> Currently the default clock rates for the HDA and HDA2CODEC_2X clocks > >>> are both 19.2MHz. However, the default rates for these clocks should > >>> actually be 51MHz and 48MHz, respectively. Correct the default clock > >>> rates for these clocks by specifying them in the clock init table for > >>> Tegra210. > >>> > >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > >>> --- > >>> drivers/clk/tegra/clk-tegra210.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >> > >> Does this fix anything? Should this be backported to stable releases? > > > > Good point. We are aligning the clock configuration with what we ship. > > So I thought for completeness it would be good to test HDA playback > > across the various sample-rates we support (32kHz to 192kHz) but with or > > without this patch I am not hearing anything. Let me check on this with > > Sameer as I would like to see if we need to mark this for stable or not. > > > >> Acked-by: Thierry Reding <treding@nvidia.com> > > I have confirmed that this does fix HDA playback on Tegra210. Without > this fix, I am seeing the following messages during playback and > playback is distorted ... > > Write error: -32,Broken pipe > [ 15.069335] tegra-mc 70019000.memory-controller: hdar: read > @0x0000000000000000: EMEM address decode error (EMEM decode error) > Write error: -32,Broken pipe > [ 15.465362] tegra-mc 70019000.memory-controller: hdar: read > @0x0000000000000000: EMEM address decode error (EMEM decode error) > Write error: -32,Broken pipe > [ 15.858615] tegra-mc 70019000.memory-controller: hdar: read > @0x0000000000000000: EMEM address decode error (EMEM decode error) > W > > Do you want me to update the change and resend? Honestly I'm not sure if it's worth it. I haven't seen any bug reports for this and we haven't had audio over HDMI support for very long, so a backport may not be necessary. I guess there'd be some use to backport this so that our stable kernel testing passes these. So it's really up to you. I have a slight tendency towards backporting, because it's really tiny and then we just have it out of the way and it's not going to haunt us. Thierry
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c index ed3c7df75d1e..8b3b3d771813 100644 --- a/drivers/clk/tegra/clk-tegra210.c +++ b/drivers/clk/tegra/clk-tegra210.c @@ -3377,6 +3377,8 @@ static struct tegra_clk_init_table init_table[] __initdata = { { TEGRA210_CLK_I2S3_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 }, { TEGRA210_CLK_I2S4_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 }, { TEGRA210_CLK_VIMCLK_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 }, + { TEGRA210_CLK_HDA, TEGRA210_CLK_PLL_P, 51000000, 0 }, + { TEGRA210_CLK_HDA2CODEC_2X, TEGRA210_CLK_PLL_P, 48000000, 0 }, /* This MUST be the last entry. */ { TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 }, };
Currently the default clock rates for the HDA and HDA2CODEC_2X clocks are both 19.2MHz. However, the default rates for these clocks should actually be 51MHz and 48MHz, respectively. Correct the default clock rates for these clocks by specifying them in the clock init table for Tegra210. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/clk/tegra/clk-tegra210.c | 2 ++ 1 file changed, 2 insertions(+)