* [PATCH] clk: tegra: Make vde a child of pll_c3 @ 2018-06-11 8:20 Thierry Reding 2018-06-11 14:17 ` Dmitry Osipenko 2018-07-09 0:06 ` Stephen Boyd 0 siblings, 2 replies; 6+ messages in thread From: Thierry Reding @ 2018-06-11 8:20 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd Cc: Peter De Schrijver, Dmitry Osipenko, linux-clk, linux-tegra From: Thierry Reding <treding@nvidia.com> The current default is to leave the VDE clock's parent at the default, which is clk_m. However, that is not a configuration that will allow the VDE to function. Reparent it to pll_c3 instead to make sure the hardware can actually decode video content. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/clk/tegra/clk-tegra124.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c index f5048f82c0b9..b6cf28ca2ed2 100644 --- a/drivers/clk/tegra/clk-tegra124.c +++ b/drivers/clk/tegra/clk-tegra124.c @@ -1267,7 +1267,7 @@ static struct tegra_clk_init_table common_init_table[] __initdata = { { TEGRA124_CLK_I2S2, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, { TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, { TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, - { TEGRA124_CLK_VDE, TEGRA124_CLK_CLK_MAX, 600000000, 0 }, + { TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_C3, 600000000, 0 }, { TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 }, { TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 }, { TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 }, -- 2.17.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: tegra: Make vde a child of pll_c3 2018-06-11 8:20 [PATCH] clk: tegra: Make vde a child of pll_c3 Thierry Reding @ 2018-06-11 14:17 ` Dmitry Osipenko 2018-06-11 16:05 ` Thierry Reding 2018-07-09 0:06 ` Stephen Boyd 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Osipenko @ 2018-06-11 14:17 UTC (permalink / raw) To: Thierry Reding, Peter De Schrijver Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-tegra On Monday, 11 June 2018 11:20:37 MSK Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The current default is to leave the VDE clock's parent at the default, > which is clk_m. However, that is not a configuration that will allow the > VDE to function. Reparent it to pll_c3 instead to make sure the hardware > can actually decode video content. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/clk/tegra/clk-tegra124.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/tegra/clk-tegra124.c > b/drivers/clk/tegra/clk-tegra124.c index f5048f82c0b9..b6cf28ca2ed2 100644 > --- a/drivers/clk/tegra/clk-tegra124.c > +++ b/drivers/clk/tegra/clk-tegra124.c > @@ -1267,7 +1267,7 @@ static struct tegra_clk_init_table common_init_table[] > __initdata = { { TEGRA124_CLK_I2S2, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > { TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > { TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > - { TEGRA124_CLK_VDE, TEGRA124_CLK_CLK_MAX, 600000000, 0 }, > + { TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_C3, 600000000, 0 }, > { TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 }, > { TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > { TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 }, If clk_m isn't a valid configuration, why VDE could select it? At least TRM lists clk_m as a valid source and clk_m is running on a safe 120 MHz, VDE HW should work fine. Sounds like a clock driver bug to me. Seems VDE clock on should be ~366 MHz according to the T40/T124 TRM. See "5.3.8 PLLC, PLLC2, PLLC3, and PLLC4" of the T124 TRM. Would be nice if you could adjust the VDE clock rate / parent-clock on all Tegra's, i.e. explicitly set the parent clock and the rate to 300-400 MHz. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: tegra: Make vde a child of pll_c3 2018-06-11 14:17 ` Dmitry Osipenko @ 2018-06-11 16:05 ` Thierry Reding 2018-06-11 16:45 ` Dmitry Osipenko 2018-06-12 15:43 ` Peter De Schrijver 0 siblings, 2 replies; 6+ messages in thread From: Thierry Reding @ 2018-06-11 16:05 UTC (permalink / raw) To: Dmitry Osipenko Cc: Peter De Schrijver, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra [-- Attachment #1: Type: text/plain, Size: 3101 bytes --] On Mon, Jun 11, 2018 at 05:17:31PM +0300, Dmitry Osipenko wrote: > On Monday, 11 June 2018 11:20:37 MSK Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > The current default is to leave the VDE clock's parent at the default, > > which is clk_m. However, that is not a configuration that will allow the > > VDE to function. Reparent it to pll_c3 instead to make sure the hardware > > can actually decode video content. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/clk/tegra/clk-tegra124.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/tegra/clk-tegra124.c > > b/drivers/clk/tegra/clk-tegra124.c index f5048f82c0b9..b6cf28ca2ed2 100644 > > --- a/drivers/clk/tegra/clk-tegra124.c > > +++ b/drivers/clk/tegra/clk-tegra124.c > > @@ -1267,7 +1267,7 @@ static struct tegra_clk_init_table common_init_table[] > > __initdata = { { TEGRA124_CLK_I2S2, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > { TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > { TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > - { TEGRA124_CLK_VDE, TEGRA124_CLK_CLK_MAX, 600000000, 0 }, > > + { TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_C3, 600000000, 0 }, > > { TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 }, > > { TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > { TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > If clk_m isn't a valid configuration, why VDE could select it? At least TRM > lists clk_m as a valid source and clk_m is running on a safe 120 MHz, VDE HW > should work fine. Sounds like a clock driver bug to me. I didn't say the configuration was invalid, I just said that it didn't work. clk_m runs at 12 MHz on the system that I was testing this on (and I think 12 MHz is the standard clk_m frequency for anything up to but not including Tegra210), and that is either much too slow or for some other reason causes VDE to malfunction. Also, I don't think "safe" in this case means that VDE should always work properly with it. The only things "safe" means is that the VDE can be accessed with that clock and frequency. The bottom line is that with the VDE parent set to clk_m I keep getting BSEV timeouts with no bytes from the bitstream getting consumed. Setting the parent to pll_c3 makes everything work just fine. > Seems VDE clock on should be ~366 MHz according to the T40/T124 TRM. See > "5.3.8 PLLC, PLLC2, PLLC3, and PLLC4" of the T124 TRM. pll_c3 is automatically set to 300 MHz on boot. I haven't found any place where we hard-code it, so I'm not exactly sure why it's being set to that value. Peter, any ideas on where this default frequency of 300 MHz for pll_c3 comes from? > Would be nice if you could adjust the VDE clock rate / parent-clock on all > Tegra's, i.e. explicitly set the parent clock and the rate to 300-400 MHz. From what I can tell they should all be running at valid frequencies by default, except that on Tegra124 the parent is wrong. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: tegra: Make vde a child of pll_c3 2018-06-11 16:05 ` Thierry Reding @ 2018-06-11 16:45 ` Dmitry Osipenko 2018-06-12 15:43 ` Peter De Schrijver 1 sibling, 0 replies; 6+ messages in thread From: Dmitry Osipenko @ 2018-06-11 16:45 UTC (permalink / raw) To: Thierry Reding, Stephen Boyd Cc: Peter De Schrijver, Michael Turquette, linux-clk, linux-tegra On Monday, 11 June 2018 19:05:19 MSK Thierry Reding wrote: > On Mon, Jun 11, 2018 at 05:17:31PM +0300, Dmitry Osipenko wrote: > > On Monday, 11 June 2018 11:20:37 MSK Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The current default is to leave the VDE clock's parent at the default, > > > which is clk_m. However, that is not a configuration that will allow the > > > VDE to function. Reparent it to pll_c3 instead to make sure the hardware > > > can actually decode video content. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > > > > drivers/clk/tegra/clk-tegra124.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/tegra/clk-tegra124.c > > > b/drivers/clk/tegra/clk-tegra124.c index f5048f82c0b9..b6cf28ca2ed2 > > > 100644 > > > --- a/drivers/clk/tegra/clk-tegra124.c > > > +++ b/drivers/clk/tegra/clk-tegra124.c > > > @@ -1267,7 +1267,7 @@ static struct tegra_clk_init_table > > > common_init_table[] __initdata = { { TEGRA124_CLK_I2S2, > > > TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },> > > > > { TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > > { TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > > > > > - { TEGRA124_CLK_VDE, TEGRA124_CLK_CLK_MAX, 600000000, 0 }, > > > + { TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_C3, 600000000, 0 }, > > > > > > { TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 }, > > > { TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > > { TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > > > If clk_m isn't a valid configuration, why VDE could select it? At least > > TRM > > lists clk_m as a valid source and clk_m is running on a safe 120 MHz, VDE > > HW should work fine. Sounds like a clock driver bug to me. > > I didn't say the configuration was invalid, I just said that it didn't > work. clk_m runs at 12 MHz on the system that I was testing this on (and > I think 12 MHz is the standard clk_m frequency for anything up to but > not including Tegra210), and that is either much too slow or for some > other reason causes VDE to malfunction. Also, I don't think "safe" in > this case means that VDE should always work properly with it. The only > things "safe" means is that the VDE can be accessed with that clock and > frequency. > Yeah, I meant 12 MHz. > The bottom line is that with the VDE parent set to clk_m I keep getting > BSEV timeouts with no bytes from the bitstream getting consumed. Setting > the parent to pll_c3 makes everything work just fine. > Okay, though that's a bit odd. > > Seems VDE clock on should be ~366 MHz according to the T40/T124 TRM. See > > "5.3.8 PLLC, PLLC2, PLLC3, and PLLC4" of the T124 TRM. > > pll_c3 is automatically set to 300 MHz on boot. I haven't found any > place where we hard-code it, so I'm not exactly sure why it's being set > to that value. > > Peter, any ideas on where this default frequency of 300 MHz for pll_c3 > comes from? > > > Would be nice if you could adjust the VDE clock rate / parent-clock on all > > Tegra's, i.e. explicitly set the parent clock and the rate to 300-400 MHz. > > From what I can tell they should all be running at valid frequencies by > default, except that on Tegra124 the parent is wrong. I wouldn't rely on the bootloaders configuration, especially because there are multiple choices for the bootloader and its versions. Let's correct the VDE clock setup on all Tegra's to not rely on the bootloaders setup at all. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: tegra: Make vde a child of pll_c3 2018-06-11 16:05 ` Thierry Reding 2018-06-11 16:45 ` Dmitry Osipenko @ 2018-06-12 15:43 ` Peter De Schrijver 1 sibling, 0 replies; 6+ messages in thread From: Peter De Schrijver @ 2018-06-12 15:43 UTC (permalink / raw) To: Thierry Reding Cc: Dmitry Osipenko, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra On Mon, Jun 11, 2018 at 06:05:19PM +0200, Thierry Reding wrote: > On Mon, Jun 11, 2018 at 05:17:31PM +0300, Dmitry Osipenko wrote: > > On Monday, 11 June 2018 11:20:37 MSK Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The current default is to leave the VDE clock's parent at the default, > > > which is clk_m. However, that is not a configuration that will allow the > > > VDE to function. Reparent it to pll_c3 instead to make sure the hardware > > > can actually decode video content. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/clk/tegra/clk-tegra124.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/tegra/clk-tegra124.c > > > b/drivers/clk/tegra/clk-tegra124.c index f5048f82c0b9..b6cf28ca2ed2 100644 > > > --- a/drivers/clk/tegra/clk-tegra124.c > > > +++ b/drivers/clk/tegra/clk-tegra124.c > > > @@ -1267,7 +1267,7 @@ static struct tegra_clk_init_table common_init_table[] > > > __initdata = { { TEGRA124_CLK_I2S2, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > > { TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > > { TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 }, > > > - { TEGRA124_CLK_VDE, TEGRA124_CLK_CLK_MAX, 600000000, 0 }, > > > + { TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_C3, 600000000, 0 }, > > > { TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 }, > > > { TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > > { TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 }, > > > > If clk_m isn't a valid configuration, why VDE could select it? At least TRM > > lists clk_m as a valid source and clk_m is running on a safe 120 MHz, VDE HW > > should work fine. Sounds like a clock driver bug to me. > > I didn't say the configuration was invalid, I just said that it didn't > work. clk_m runs at 12 MHz on the system that I was testing this on (and > I think 12 MHz is the standard clk_m frequency for anything up to but > not including Tegra210), and that is either much too slow or for some > other reason causes VDE to malfunction. Also, I don't think "safe" in > this case means that VDE should always work properly with it. The only > things "safe" means is that the VDE can be accessed with that clock and > frequency. > > The bottom line is that with the VDE parent set to clk_m I keep getting > BSEV timeouts with no bytes from the bitstream getting consumed. Setting > the parent to pll_c3 makes everything work just fine. > > > Seems VDE clock on should be ~366 MHz according to the T40/T124 TRM. See > > "5.3.8 PLLC, PLLC2, PLLC3, and PLLC4" of the T124 TRM. > > pll_c3 is automatically set to 300 MHz on boot. I haven't found any > place where we hard-code it, so I'm not exactly sure why it's being set > to that value. > > Peter, any ideas on where this default frequency of 300 MHz for pll_c3 > comes from? > Yes, this comes from the fact we initialize these PLLs to half of the VCOmin rate which is 300MHz. > > Would be nice if you could adjust the VDE clock rate / parent-clock on all > > Tegra's, i.e. explicitly set the parent clock and the rate to 300-400 MHz. > > From what I can tell they should all be running at valid frequencies by > default, except that on Tegra124 the parent is wrong. > Same remark here as with vic03. Consider doing this in DT. Peter. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: tegra: Make vde a child of pll_c3 2018-06-11 8:20 [PATCH] clk: tegra: Make vde a child of pll_c3 Thierry Reding 2018-06-11 14:17 ` Dmitry Osipenko @ 2018-07-09 0:06 ` Stephen Boyd 1 sibling, 0 replies; 6+ messages in thread From: Stephen Boyd @ 2018-07-09 0:06 UTC (permalink / raw) To: Michael Turquette, Thierry Reding Cc: Peter De Schrijver, Dmitry Osipenko, linux-clk, linux-tegra Quoting Thierry Reding (2018-06-11 01:20:37) > From: Thierry Reding <treding@nvidia.com> > = > The current default is to leave the VDE clock's parent at the default, > which is clk_m. However, that is not a configuration that will allow the > VDE to function. Reparent it to pll_c3 instead to make sure the hardware > can actually decode video content. > = > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- Applied to clk-next ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-09 0:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-11 8:20 [PATCH] clk: tegra: Make vde a child of pll_c3 Thierry Reding 2018-06-11 14:17 ` Dmitry Osipenko 2018-06-11 16:05 ` Thierry Reding 2018-06-11 16:45 ` Dmitry Osipenko 2018-06-12 15:43 ` Peter De Schrijver 2018-07-09 0:06 ` Stephen Boyd
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.