On Mon, Sep 22, 2014 at 06:46:52PM +0100, Mark Rutland wrote: > On Fri, Sep 19, 2014 at 08:53:48PM +0100, Sean Paul wrote: > > 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 <seanpaul@xxxxxxxxxxxx> > > --- > > .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt | 10 ++++++++-- > > drivers/gpu/drm/tegra/dsi.c | 18 ++++++++++++++++++ > > drivers/gpu/drm/tegra/dsi.h | 3 +++ > > 3 files changed, 29 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt > > index b48f4ef..fef2918 100644 > > --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt > > +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt > > @@ -191,6 +191,10 @@ of the following host1x client modules: > > - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection > > - nvidia,edid: supplies a binary EDID blob > > - nvidia,panel: phandle of a display panel > > + - clock-names: Can include the following entries: > > + - lp_parent: The parent clock for lp > > + - clocks: Must contain an entry for each optional entry in clock-names. > > + See ../clocks/clock-bindings.txt for details. > > Did this driver previously acquire clocks? > > What order or names did it expect if so? This is badly placed. There are clocks and clock-names properties in a "Required properties" section above this hunk which lists all the clocks that this module uses. Presumably this was added to the optional section because it isn't always needed. > > - sor: serial output resource > > > > @@ -360,8 +364,10 @@ Example: > > compatible = "nvidia,tegra20-dsi"; > > reg = <0x54300000 0x00040000>; > > clocks = <&tegra_car TEGRA20_CLK_DSI>, > > - <&tegra_car TEGRA20_CLK_PLL_D_OUT0>; > > - clock-names = "dsi", "parent"; > > + <&tegra_car TEGRA124_CLK_DSIALP>, > > + <&tegra_car TEGRA20_CLK_PLL_D_OUT0>, > > + <&tegra_car TEGRA124_CLK_PLL_P>; > > + clock-names = "dsi", "lp", "parent", "lp_parent"; > > Please document _all_ the names you expect. > > What exactly are these two new clocks? "lp" isn't actually new, it's just missing from the example. > Is this all the clocks that feed into the DSI block? Are any of these > not directly wired to the DSI block? > > Why exactly do you need to reparent it to this particular clock, and why > do you need a reference here in order to do so, given it presumably > doesn't feed directly into the DSI block? It seems like the hardware default is to use a parent clock unsuitable for the DSI low-power mode, but as discussed elsewhere this should be fixed in the clock driver where we already have a way to statically set the parent clock at boot time. Thierry
Attachment:
pgpqyq4WNVcw9.pgp
Description: PGP signature
