[RFC PATCH 0/2] clk: rockchip: leave npll for DCLK_VOP0(HDMI) only

[RFC PATCH 0/2] clk: rockchip: leave npll for DCLK_VOP0(HDMI) only

[Kever Yang]

To support all kinds of frequency requirement for HDMI on rk3288,
we need a PLL that can change rate at run time.

There are some discussion before at [0], I think we can just leave
the npll for HDMI(DCLK_VOP0) used to make it simple.

Comments are welcome.

[0] https://lkml.org/lkml/2014/11/4/58


Kever Yang (2):
  clk: rockchip: leave npll for VOP0 only
  arm: dts: rockchip: select npll as parent of DCLK_VOP0

 arch/arm/boot/dts/rk3288.dtsi     |  2 ++
 drivers/clk/rockchip/clk-rk3288.c | 24 ++++++++++++------------
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
1.9.1

[RFC PATCH 1/2] clk: rockchip: leave npll for VOP0 only

[Kever Yang]

We will need a pll to support all kinds of clock rate requirement
for HDMI(from DCLK_VOP0) which may change the rate at run time.

In order not to affect other clocks, remove the npll from the
parent list of other clocks and only DCLK_VOP0 can select npll
as parent.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3288.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index b4a74c2..6eb1407 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -174,7 +174,7 @@ PNAME(mux_ddrphy_p)		= { "dpll_ddr", "gpll_ddr" };
 PNAME(mux_aclk_cpu_src_p)	= { "cpll_aclk_cpu", "gpll_aclk_cpu" };
 
 PNAME(mux_pll_src_cpll_gpll_p)		= { "cpll", "gpll" };
-PNAME(mux_pll_src_npll_cpll_gpll_p)	= { "npll", "cpll", "gpll" };
+PNAME(mux_pll_src_npll_cpll_gpll_p)	= { "dummy", "cpll", "gpll" };
 PNAME(mux_pll_src_cpll_gpll_npll_p)	= { "cpll", "gpll", "npll" };
 PNAME(mux_pll_src_cpll_gpll_usb480m_p)	= { "cpll", "gpll", "usbphy480m_src" };
 PNAME(mux_pll_src_cpll_gll_usb_npll_p)	= { "cpll", "gpll", "usbphy480m_src", "npll" };
@@ -392,24 +392,24 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 			RK3288_CLKSEL_CON(30), 14, 2, MFLAGS, 8, 5, DFLAGS,
 			RK3288_CLKGATE_CON(3), 4, GFLAGS),
 
-	COMPOSITE(DCLK_VOP0, "dclk_vop0", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(DCLK_VOP0, "dclk_vop0", mux_pll_src_cpll_gpll_npll_p, CLK_SET_RATE_PARENT,
 			RK3288_CLKSEL_CON(27), 0, 2, MFLAGS, 8, 8, DFLAGS,
 			RK3288_CLKGATE_CON(3), 1, GFLAGS),
-	COMPOSITE(DCLK_VOP1, "dclk_vop1", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(DCLK_VOP1, "dclk_vop1", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(29), 6, 2, MFLAGS, 8, 8, DFLAGS,
 			RK3288_CLKGATE_CON(3), 3, GFLAGS),
 
 	COMPOSITE_NODIV(SCLK_EDP_24M, "sclk_edp_24m", mux_edp_24m_p, 0,
 			RK3288_CLKSEL_CON(28), 15, 1, MFLAGS,
 			RK3288_CLKGATE_CON(3), 12, GFLAGS),
-	COMPOSITE(SCLK_EDP, "sclk_edp", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(SCLK_EDP, "sclk_edp", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(28), 6, 2, MFLAGS, 0, 6, DFLAGS,
 			RK3288_CLKGATE_CON(3), 13, GFLAGS),
 
-	COMPOSITE(SCLK_ISP, "sclk_isp", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(SCLK_ISP, "sclk_isp", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(6), 6, 2, MFLAGS, 0, 6, DFLAGS,
 			RK3288_CLKGATE_CON(3), 14, GFLAGS),
-	COMPOSITE(SCLK_ISP_JPE, "sclk_isp_jpe", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(SCLK_ISP_JPE, "sclk_isp_jpe", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(6), 14, 2, MFLAGS, 8, 6, DFLAGS,
 			RK3288_CLKGATE_CON(3), 15, GFLAGS),
 
@@ -418,16 +418,16 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 	GATE(SCLK_HDMI_CEC, "sclk_hdmi_cec", "xin32k", 0,
 			RK3288_CLKGATE_CON(5), 11, GFLAGS),
 
-	COMPOSITE(ACLK_HEVC, "aclk_hevc", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(ACLK_HEVC, "aclk_hevc", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(39), 14, 2, MFLAGS, 8, 5, DFLAGS,
 			RK3288_CLKGATE_CON(13), 13, GFLAGS),
 	DIV(HCLK_HEVC, "hclk_hevc", "aclk_hevc", 0,
 			RK3288_CLKSEL_CON(40), 12, 2, DFLAGS),
 
-	COMPOSITE(SCLK_HEVC_CABAC, "sclk_hevc_cabac", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(SCLK_HEVC_CABAC, "sclk_hevc_cabac", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(42), 6, 2, MFLAGS, 0, 5, DFLAGS,
 			RK3288_CLKGATE_CON(13), 14, GFLAGS),
-	COMPOSITE(SCLK_HEVC_CORE, "sclk_hevc_core", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(SCLK_HEVC_CORE, "sclk_hevc_core", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(42), 14, 2, MFLAGS, 8, 5, DFLAGS,
 			RK3288_CLKGATE_CON(13), 15, GFLAGS),
 
@@ -443,7 +443,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 			RK3288_CLKSEL_CON(33), 0, 5, DFLAGS,
 			RK3288_CLKGATE_CON(5), 8, GFLAGS),
 
-	COMPOSITE(SCLK_GPU, "sclk_gpu", mux_pll_src_cpll_gll_usb_npll_p, 0,
+	COMPOSITE(SCLK_GPU, "sclk_gpu", mux_pll_src_cpll_gpll_usb480m_p, 0,
 			RK3288_CLKSEL_CON(34), 6, 2, MFLAGS, 0, 5, DFLAGS,
 			RK3288_CLKGATE_CON(5), 7, GFLAGS),
 
@@ -489,7 +489,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 	COMPOSITE(0, "sclk_tspout", mux_tspout_p, 0,
 			RK3288_CLKSEL_CON(35), 14, 2, MFLAGS, 8, 5, DFLAGS,
 			RK3288_CLKGATE_CON(4), 11, GFLAGS),
-	COMPOSITE(0, "sclk_tsp", mux_pll_src_cpll_gpll_npll_p, 0,
+	COMPOSITE(0, "sclk_tsp", mux_pll_src_cpll_gpll_p, 0,
 			RK3288_CLKSEL_CON(35), 6, 2, MFLAGS, 0, 5, DFLAGS,
 			RK3288_CLKGATE_CON(4), 10, GFLAGS),
 
@@ -520,7 +520,7 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
 			RK3288_CLKSEL_CON(38), 15, 1, MFLAGS, 8, 5, DFLAGS,
 			RK3288_CLKGATE_CON(5), 6, GFLAGS),
 
-	COMPOSITE(0, "uart0_src", mux_pll_src_cpll_gll_usb_npll_p, 0,
+	COMPOSITE(0, "uart0_src", mux_pll_src_cpll_gpll_usb480m_p, 0,
 			RK3288_CLKSEL_CON(13), 13, 2, MFLAGS, 0, 7, DFLAGS,
 			RK3288_CLKGATE_CON(1), 8, GFLAGS),
 	COMPOSITE_FRAC(0, "uart0_frac", "uart0_src", 0,
-- 
1.9.1

[RFC PATCH 2/2] arm: dts: rockchip: select npll as parent of DCLK_VOP0

[Kever Yang]

The DCLK_VOP0 will change the parent clock's rate, we don't want
to change the PLLs rate other than npll. So we select the npll
as parent directly.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm/boot/dts/rk3288.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index c3190f7..6ea6125 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -573,6 +573,8 @@
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
 		clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
+		assigned-clocks = <&cru DCLK_VOP0>;
+		assigned-clock-parents = <3>;
 		resets = <&cru SRST_LCDC0_AXI>, <&cru SRST_LCDC0_AHB>, <&cru SRST_LCDC0_DCLK>;
 		reset-names = "axi", "ahb", "dclk";
 		iommus = <&vopb_mmu>;
-- 
1.9.1

[RFC PATCH 0/2] clk: rockchip: leave npll for DCLK_VOP0(HDMI) only

[Heiko Stübner]

Hi Kever,

Am Montag, 17. November 2014, 22:55:36 schrieb Kever Yang:
> To support all kinds of frequency requirement for HDMI on rk3288,
> we need a PLL that can change rate at run time.
> 
> There are some discussion before at [0], I think we can just leave
> the npll for HDMI(DCLK_VOP0) used to make it simple.
> 
> Comments are welcome.

I think I said it in private somewhere already, but just so it's also 
available publically:

I don't think customizing/limiting the clock usage like this will fly, 
especially as this would require each and every rk3288 board to use vop0 for 
hdmi and vop1 for other stuff.

With the new rk3288 Firefly devboard this concern already becomes reality. 
There a vga converter is connected to VOP0, which leaves only vop1 for hdmi if 
one wants to support the vga connection.


>From our discussion about this problem I remember that the missing clock 
frequencies only affected more esotheric screen resolutions, so personally I'm 
not this much concerned an would like to wait till we find a better solution to 
the problem.


Heiko

[RFC PATCH 0/2] clk: rockchip: leave npll for DCLK_VOP0(HDMI) only

[Mike Turquette]

Quoting Heiko St?bner (2015-01-08 14:30:01)
> Hi Kever,
> 
> Am Montag, 17. November 2014, 22:55:36 schrieb Kever Yang:
> > To support all kinds of frequency requirement for HDMI on rk3288,
> > we need a PLL that can change rate at run time.
> > 
> > There are some discussion before at [0], I think we can just leave
> > the npll for HDMI(DCLK_VOP0) used to make it simple.
> > 
> > Comments are welcome.
> 
> I think I said it in private somewhere already, but just so it's also 
> available publically:
> 
> I don't think customizing/limiting the clock usage like this will fly, 
> especially as this would require each and every rk3288 board to use vop0 for 
> hdmi and vop1 for other stuff.
> 
> With the new rk3288 Firefly devboard this concern already becomes reality. 
> There a vga converter is connected to VOP0, which leaves only vop1 for hdmi if 
> one wants to support the vga connection.
> 
> 
> From our discussion about this problem I remember that the missing clock 
> frequencies only affected more esotheric screen resolutions, so personally I'm 
> not this much concerned an would like to wait till we find a better solution to 
> the problem.

Ack. We shouldn't have to limit the possible hardware configurations in
software just to keep things simple. This points to a deficiency in the
clock framework. This is a common concern: how to change a clock
frequency for one user without exploding all of the other users.

Do you think Tomeu's constraints API[0] might be a step in the right
direction for you?

[0] http://lkml.kernel.org/r/<1421071809-17402-3-git-send-email-tomeu.vizoso@collabora.com>

Regards,
Mike

> 
> 
> Heiko