[PATCH 1/2] drm/tegra: Set the dsi lp clk parent and rate
On Mon, Sep 22, 2014 at 12:11:54PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Sep 22, 2014 at 11:00:56AM +0200, Lucas Stach wrote:
> > Am Freitag, den 19.09.2014, 15:53 -0400 schrieb Sean Paul:
> > > Per NVidia, this clock rate should be around 70MHz in
> > > order to properly sample reads on data lane 0. In order
> > > to achieve this rate, we need to reparent the clock from
> > > clk_m which can only achieve 12MHz. Add parent_lp to the
> > > dts bindings and set the parent & rate on init.
> > >
> > > Signed-off-by: Sean Paul
> >
> > NACK
> >
> > You are pushing SoC integration details into the binding of the device.
> >
> > You have two reasonable routes to go here: either the clock driver needs
> > to be made smarter to reparent the clock in case the required clock rate
> > could not be achieved with the current parent or you go the easy route
> > and reparent the clock as part of the initial configuration.
>
> Agreed. There doesn't seem to be a case where it would make sense to
> have this configurable per-board. Can you achieve the same effect by
> adding this to the clock initialization table?
>
> Oh, I just see that we have this in the Tegra124 clock initialization
> table:
>
> {TEGRA114_CLK_DSIALP, TEGRA114_CLK_PLL_P, 6800, 0},
> {TEGRA114_CLK_DSIBLP, TEGRA114_CLK_PLL_P, 6800, 0},
>
> Doesn't that work for you already? If not that'd be a bug that should be
> fixed in the clock driver.
This seems the better approach indeed. Unless the rate would differ based
on board or other external factors.
Cheers,
Peter.
[PATCH 0/3] drm/gk20a: support for reclocking
On Fri, Jul 11, 2014 at 04:01:02AM +0200, Ben Skeggs wrote: > On Fri, Jul 11, 2014 at 11:49 AM, Alexandre Courbot > wrote: > > On 07/10/2014 06:43 PM, Peter De Schrijver wrote: > >> > >> On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote: > >>> > >>> This series adds support for reclocking on GK20A. The first two patches > >>> touch > >>> the clock subsystem to allow GK20A to operate, by making the presence of > >>> the > >>> thermal and voltage devices optional, and allowing pstates to be provided > >>> directly instead of being probed using the BIOS (which Tegra does not > >>> have). > >>> > >>> The last patch adds the GK20A clock device. Arguably the clock can be > >>> seen as a > >>> stripped-down version of what is seen on NVE0, however instead of using > >>> NVE0 > >>> support has been written from scratch using the ChromeOS kernel as a > >>> basis. > >>> There are several reasons for this: > >>> > >>> - The ChromeOS driver uses a lookup table for the P coefficient which I > >>> could > >>>not find in the NVE0 driver, > >>> - Some registers that NVE0 expects to find are not present on GK20A (e.g. > >>>0x137120 and 0x137140), > >>> - Calculation of MNP is done differently from what is performed in > >>>nva3_pll_calc(), and it might be interesting to compare the two > >>> methods, > >>> - All the same, the programming sequence is done differently in the > >>> ChromeOS > >>>driver and NVE0 could possibly benefit from it (?) > >>> > >>> It would be interesting to try and merge both, but for now I prefer to > >>> have the > >>> two coexisting to ensure proper operation on GK20A and besure I don't > >>> break > >>> dGPU support. :) > >>> > >>> Regarding the first patch, one might argue that I could as well add > >>> thermal > >>> and voltage devices to GK20A. The reason this is not done is because > >>> these > >>> currently depend heavily on the presence of a BIOS, and will require a > >>> rework > >>> similar to that done in patch 2 for clocks. I would like to make sure > >>> this > >>> approach is approved because applying it to other subdevs. > >> > >> > >> I think this should use CCF so we can use pre and post rate change > >> notifiers > >> to hookup vdd_gpu DVS. > > > > > > Do you mean that we should turn the Nouveau gk20a clock driver into a > > consumer of this CCF clock? I have nothing against this, but note that > > Nouveau can also perform DVS on its own, as the pstates can also contain a > > voltage to be applied to the volt device (not yet implemented in this > > series). > > > > The question then becomes whether we want an additional layer of abstraction > > on these devices and whether the pre/post rate change notifiers give us any > > advantage compared to what Nouveau currently proposes. > I had a brief look at this, and personally I don't think the CCF is a > very good match at all for how we're *supposed* to manage clock > frequencies as described by a discrete GPU VBIOS, and especially for > when we get to the point of using the PMU falcon to coordinate all the > various bits and pieces that go towards power management. > For all I can see, the PMU is not involved in the mechanics of GPU frequency scaling on Tegra. Cheers, Peter.
[PATCH 0/3] drm/gk20a: support for reclocking
On Fri, Jul 11, 2014 at 03:49:06AM +0200, Alex Courbot wrote: > On 07/10/2014 06:43 PM, Peter De Schrijver wrote: > > On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote: > >> This series adds support for reclocking on GK20A. The first two patches > >> touch > >> the clock subsystem to allow GK20A to operate, by making the presence of > >> the > >> thermal and voltage devices optional, and allowing pstates to be provided > >> directly instead of being probed using the BIOS (which Tegra does not > >> have). > >> > >> The last patch adds the GK20A clock device. Arguably the clock can be seen > >> as a > >> stripped-down version of what is seen on NVE0, however instead of using > >> NVE0 > >> support has been written from scratch using the ChromeOS kernel as a basis. > >> There are several reasons for this: > >> > >> - The ChromeOS driver uses a lookup table for the P coefficient which I > >> could > >>not find in the NVE0 driver, > >> - Some registers that NVE0 expects to find are not present on GK20A (e.g. > >>0x137120 and 0x137140), > >> - Calculation of MNP is done differently from what is performed in > >>nva3_pll_calc(), and it might be interesting to compare the two methods, > >> - All the same, the programming sequence is done differently in the > >> ChromeOS > >>driver and NVE0 could possibly benefit from it (?) > >> > >> It would be interesting to try and merge both, but for now I prefer to > >> have the > >> two coexisting to ensure proper operation on GK20A and besure I don't break > >> dGPU support. :) > >> > >> Regarding the first patch, one might argue that I could as well add thermal > >> and voltage devices to GK20A. The reason this is not done is because these > >> currently depend heavily on the presence of a BIOS, and will require a > >> rework > >> similar to that done in patch 2 for clocks. I would like to make sure this > >> approach is approved because applying it to other subdevs. > > > > I think this should use CCF so we can use pre and post rate change notifiers > > to hookup vdd_gpu DVS. > > Do you mean that we should turn the Nouveau gk20a clock driver into a > consumer of this CCF clock? I have nothing against this, but note that > Nouveau can also perform DVS on its own, as the pstates can also contain > a voltage to be applied to the volt device (not yet implemented in this > series). > Yes. For Tegra I think it makes sense to move DVS out of the individual drivers. Then we can share the code which has to deal with building the OPP tables with other DVS rails (eg. vdd_core) for example. Often there are also chip specific quirks to be dealt with (such as the maximum allowed voltage step or voltage relationships between rails), which are easier to handle in common code. Cheers, Peter.
[PATCH 0/3] drm/gk20a: support for reclocking
On Thu, Jul 10, 2014 at 09:34:34AM +0200, Alexandre Courbot wrote: > This series adds support for reclocking on GK20A. The first two patches touch > the clock subsystem to allow GK20A to operate, by making the presence of the > thermal and voltage devices optional, and allowing pstates to be provided > directly instead of being probed using the BIOS (which Tegra does not have). > > The last patch adds the GK20A clock device. Arguably the clock can be seen as > a > stripped-down version of what is seen on NVE0, however instead of using NVE0 > support has been written from scratch using the ChromeOS kernel as a basis. > There are several reasons for this: > > - The ChromeOS driver uses a lookup table for the P coefficient which I could > not find in the NVE0 driver, > - Some registers that NVE0 expects to find are not present on GK20A (e.g. > 0x137120 and 0x137140), > - Calculation of MNP is done differently from what is performed in > nva3_pll_calc(), and it might be interesting to compare the two methods, > - All the same, the programming sequence is done differently in the ChromeOS > driver and NVE0 could possibly benefit from it (?) > > It would be interesting to try and merge both, but for now I prefer to have > the > two coexisting to ensure proper operation on GK20A and besure I don't break > dGPU support. :) > > Regarding the first patch, one might argue that I could as well add thermal > and voltage devices to GK20A. The reason this is not done is because these > currently depend heavily on the presence of a BIOS, and will require a rework > similar to that done in patch 2 for clocks. I would like to make sure this > approach is approved because applying it to other subdevs. I think this should use CCF so we can use pre and post rate change notifiers to hookup vdd_gpu DVS. Thanks, Peter.
[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver
On Wed, Jun 18, 2014 at 12:35:27AM +0200, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Tue, Jun 17, 2014 at 02:16:06PM +0200, Tomeu Vizoso wrote: > > On 06/16/2014 10:02 PM, Stephen Warren wrote: > > >On 06/16/2014 07:35 AM, Tomeu Vizoso wrote: > > >>+ > > >>+Child device nodes describe the memory settings for different > > >>configurations and > > >>+clock rates. > > > > > >How do the child nodes do that? The binding needs to specify the format > > >of the child node. > > > > Sorry, that file was sent before I had finished removing the bits from > > downstream that aren't needed yet. There's no current need for any child > > nodes. > > > > >This binding looks quite anaemic vs. > > >Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt; I > > >would expect that this binding needs all the EMC register data from the > > >tegra20-emc binding too. Can the two bindings be identical? > > > > There's even less stuff needed right now, as all what ultimately the EMC > > driver does is call clk_set_rate on the EMC clock. As the T124 EMC driver > > gains more features, they should get more similar. > > > > >Can you explain what the nvidia,mc and nvidia,pmc references are needed > > >for? Hopefully, this driver isn't going to reach into those devices and > > >touch their registers directly. > > > > Not really needed, see above. > > I've been working on a prototype driver for the memory controller. Part > of what I've added is programming of the latency allowance registers (it > doesn't yet expose an API to do so yet, though). I think that needs to > eventually take into account the EMC frequency (and needs to be notified > of changes to the same). > > Without having thought this through very thoroughly, I suspect that > rather than referencing the MC from the EMC it might be better to have > the MC register with the EMC for notifications. > > But perhaps there are other services from MC that EMC needs to work? > If the emc frequency change is modelled as a clock, you could use CCF notifications for this? Cheers, Peter.
