Patchwork [2/2] clk: tegra: Make gr2d and gr3d clocks children of pll_c

login
register
mail settings
Submitter Thierry Reding
Date March 28, 2013, 8:31 p.m.
Message ID <1364502688-5135-2-git-send-email-thierry.reding@avionic-design.de>
Download mbox | patch
Permalink /patch/232182/
State Superseded, archived
Headers show

Comments

Thierry Reding - March 28, 2013, 8:31 p.m.
By default these clocks are children of pll_m, but in downstream kernels
they are reparented to pll_c. While at it, decrease their frequencies to
300 MHz because the defaults aren't in the specified range.

gr2d can reportedly run at much higher frequencies, but 300 MHz works
and is a more conservative default.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 drivers/clk/tegra/clk-tegra20.c | 2 ++
 1 file changed, 2 insertions(+)
Stephen Warren - March 29, 2013, 9:46 p.m.
On 03/28/2013 02:31 PM, Thierry Reding wrote:
> By default these clocks are children of pll_m, but in downstream kernels
> they are reparented to pll_c. While at it, decrease their frequencies to
> 300 MHz because the defaults aren't in the specified range.
> 
> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> and is a more conservative default.

Questions on this patch:

Do we need to do the same thing for Tegra30 and/or Tegra114?

Is 300MHz the right value?

I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Zhang - April 1, 2013, 6:03 a.m.
On 03/30/2013 05:46 AM, Stephen Warren wrote:
> On 03/28/2013 02:31 PM, Thierry Reding wrote:
>> By default these clocks are children of pll_m, but in downstream kernels
>> they are reparented to pll_c. While at it, decrease their frequencies to
>> 300 MHz because the defaults aren't in the specified range.
>>
>> gr2d can reportedly run at much higher frequencies, but 300 MHz works
>> and is a more conservative default.
> 
> Questions on this patch:
> 
> Do we need to do the same thing for Tegra30 and/or Tegra114?
> 

I think yes. For Tegra114, it's pll_c2.

> Is 300MHz the right value?
> 

Right. I use 300MHz on Tegra114 and it seems OK. I recall sometimes
276MHz can be used as well.

Mark
> I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Terje Bergström - April 2, 2013, 5:28 a.m.
On 29.03.2013 23:46, Stephen Warren wrote:
> On 03/28/2013 02:31 PM, Thierry Reding wrote:
>> By default these clocks are children of pll_m, but in downstream kernels
>> they are reparented to pll_c. While at it, decrease their frequencies to
>> 300 MHz because the defaults aren't in the specified range.
>>
>> gr2d can reportedly run at much higher frequencies, but 300 MHz works
>> and is a more conservative default.
> 
> Questions on this patch:
> 
> Do we need to do the same thing for Tegra30 and/or Tegra114?
> 
> Is 300MHz the right value?
> 
> I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.

We need a patch for all SoC's. 2D can fail subtly with the too high
clocks, even though most of the time it seems to be doing just fine.

In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
higher max clock, and in Tegra114 we drive it from a different PLL. But
for all of them 300MHz and PLLC should be a working configuration. I
haven't checked how we use PLLC in upstream, so I'm not 100% sure.

In downstream, the nvhost driver is responsible for setting a sane value
to the host1x clients. In upstream host1x, we missed that because we
didn't include the clock management code.

Terje
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - April 2, 2013, 5:47 a.m.
On Tue, Apr 02, 2013 at 08:28:53AM +0300, Terje Bergström wrote:
> On 29.03.2013 23:46, Stephen Warren wrote:
> > On 03/28/2013 02:31 PM, Thierry Reding wrote:
> >> By default these clocks are children of pll_m, but in downstream kernels
> >> they are reparented to pll_c. While at it, decrease their frequencies to
> >> 300 MHz because the defaults aren't in the specified range.
> >>
> >> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> >> and is a more conservative default.
> > 
> > Questions on this patch:
> > 
> > Do we need to do the same thing for Tegra30 and/or Tegra114?
> > 
> > Is 300MHz the right value?
> > 
> > I'm hoping that Peter, Prashant, and/or Terje can provide guidance here.
> 
> We need a patch for all SoC's.

I think we also need that for all host1x children as well. But I think
we can tackle those at the same time as driver support is added. Right
now only 2D and 3D have some form of user code.

> 2D can fail subtly with the too high clocks, even though most of the
> time it seems to be doing just fine.
> 
> In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
> higher max clock, and in Tegra114 we drive it from a different PLL. But
> for all of them 300MHz and PLLC should be a working configuration. I
> haven't checked how we use PLLC in upstream, so I'm not 100% sure.

At least on Tegra30 (CardHu) I can see that the clocks are also children
of PLLC. I can add a similar hunk for Tegra30 to the patch. I have no
Tegra114 hardware available and none of the downstream kernel branches I
have seem to include Tegra114 support either so I can't check what's in
use there, but if you say it should be fine I can include that in the
patch as well.

Thierry
Terje Bergström - April 2, 2013, 5:50 a.m.
On 02.04.2013 08:47, Thierry Reding wrote:
> On Tue, Apr 02, 2013 at 08:28:53AM +0300, Terje Bergström wrote:
>> In Tegra20, 300MHz is the max rate we drive 2D in. Later Tegras have a
>> higher max clock, and in Tegra114 we drive it from a different PLL. But
>> for all of them 300MHz and PLLC should be a working configuration. I
>> haven't checked how we use PLLC in upstream, so I'm not 100% sure.
> 
> At least on Tegra30 (CardHu) I can see that the clocks are also children
> of PLLC. I can add a similar hunk for Tegra30 to the patch. I have no

Yes, Tegra30 is similar to Tegra20 in this respect. Only max clock is
different, so similar hunk for Tegra30 would be a good idea.

> Tegra114 hardware available and none of the downstream kernel branches I
> have seem to include Tegra114 support either so I can't check what's in
> use there, but if you say it should be fine I can include that in the
> patch as well.

I think it's better we add this when we have a patch that enables host1x
driver for Tegra114 and we have tested. Until then, I suggest we just
leave the Tegra114 part out.

Terje
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter De Schrijver - April 2, 2013, 9:33 a.m.
On Thu, Mar 28, 2013 at 09:31:28PM +0100, Thierry Reding wrote:
> By default these clocks are children of pll_m, but in downstream kernels
> they are reparented to pll_c. While at it, decrease their frequencies to
> 300 MHz because the defaults aren't in the specified range.
> 
> gr2d can reportedly run at much higher frequencies, but 300 MHz works
> and is a more conservative default.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  drivers/clk/tegra/clk-tegra20.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index bf19400..b020beb 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1247,6 +1247,8 @@ static __initdata struct tegra_clk_init_table init_table[] = {
>  	{host1x, pll_c, 150000000, 0},
>  	{disp1, pll_p, 600000000, 0},
>  	{disp2, pll_p, 600000000, 0},
> +	{gr2d, pll_c, 300000000, 0},
> +	{gr3d, pll_c, 300000000, 0},
>  	{clk_max, clk_max, 0, 0}, /* This MUST be the last entry */
>  };
>  

In the end we should move to a more flexible scheme where the pll_c frequency
is determined by the needs of the various users, but for now this should do I
think.

Acked-By: Peter De Schrijver <pdeschrijver@nvidia.com>

Cheers,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index bf19400..b020beb 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1247,6 +1247,8 @@  static __initdata struct tegra_clk_init_table init_table[] = {
 	{host1x, pll_c, 150000000, 0},
 	{disp1, pll_p, 600000000, 0},
 	{disp2, pll_p, 600000000, 0},
+	{gr2d, pll_c, 300000000, 0},
+	{gr3d, pll_c, 300000000, 0},
 	{clk_max, clk_max, 0, 0}, /* This MUST be the last entry */
 };