The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5 drivider or a 1/7 divider. The common clock framework cannot deal with the two dividers directly even with the divider table which only supports integral dividers. So, the idea is to take the 1/3.5 and 1/7 dividers as separate fixed factor dividers and introduce a new multiplexer clock to be derived from the them. Then, the ldb display clock trees can be setup correctly. This series contains the necessary clock driver changes, dts code changes and imx-drm/ldb driver changes to fullfill the task.
Liu Ying (3): ARM: imx6q: refactor some ldb related clocks ARM: dts: imx6q/imx6dl: add necessary clocks for ldb node staging: drm/imx: ldb: correct the ldb di clock trees
.../devicetree/bindings/clock/imx6q-clock.txt | 6 ++-- arch/arm/boot/dts/imx6dl.dtsi | 8 +++-- arch/arm/boot/dts/imx6q.dtsi | 8 +++-- arch/arm/mach-imx/clk-imx6q.c | 25 +++++++------ drivers/staging/imx-drm/imx-ldb.c | 38 +++++++++++++++----- 5 files changed, 61 insertions(+), 24 deletions(-)
The ldb_di[0/1]_ipu_div dividers may divide their parent clock frequencies by either 3.5 or 7. The non-integral dividers cannot be dealt with the common clock framework directly, so they cannot be registered as common clock dividers. So this patch adds a fixed factor clock of 1/7 and introduces ldb_di[0/1]_div_sel multiplexers so that the fixed factor clocks of 1/3.5 and 1/7 can be set to be the parents of ldb_di[0/1]_div_sel multiplexers. The ldb_di[0/1]_podf dividers are no longer used then.
Signed-off-by: Liu Ying Ying.Liu@freescale.com --- .../devicetree/bindings/clock/imx6q-clock.txt | 6 +++-- arch/arm/mach-imx/clk-imx6q.c | 25 ++++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt index 5a90a72..90e923e 100644 --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt @@ -89,8 +89,6 @@ clocks and IDs. gpu3d_shader 74 ipu1_podf 75 ipu2_podf 76 - ldb_di0_podf 77 - ldb_di1_podf 78 ipu1_di0_pre 79 ipu1_di1_pre 80 ipu2_di0_pre 81 @@ -215,6 +213,10 @@ clocks and IDs. cko2 200 cko 201 vdoa 202 + ldb_di0_div_7 203 + ldb_di1_div_7 204 + ldb_di0_div_sel 205 + ldb_di1_div_sel 206
Examples:
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 9181a24..2b5be96 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -189,6 +189,8 @@ static const char *gpu3d_core_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_p static const char *gpu3d_shader_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll3_pfd0_720m", }; static const char *ipu_sels[] = { "mmdc_ch0_axi", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", }; static const char *ldb_di_sels[] = { "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_usb_otg", }; +static const char *ldb_di0_div_sels[] = { "ldb_di0_div_3_5", "ldb_di0_div_7", }; +static const char *ldb_di1_div_sels[] = { "ldb_di1_div_3_5", "ldb_di1_div_7", }; static const char *ipu_di_pre_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", }; static const char *ipu1_di0_sels[] = { "ipu1_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", }; static const char *ipu1_di1_sels[] = { "ipu1_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", }; @@ -233,11 +235,11 @@ enum mx6q_clks { periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf, asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root, gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf, - ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre, - ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf, - ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf, - usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf, - emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf, + ldb_di0_podf_unused, ldb_di1_podf_unused, ipu1_di0_pre, ipu1_di1_pre, + ipu2_di0_pre, ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, + ssi2_pred, ssi2_podf, ssi3_pred, ssi3_podf, uart_serial_podf, + usdhc1_podf, usdhc2_podf, usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, + emi_podf, emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf, mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial, can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet, esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb, @@ -251,7 +253,8 @@ enum mx6q_clks { ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5, sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate, usbphy2_gate, pll4_post_div, pll5_post_div, pll5_video_div, eim_slow, - spdif, cko2_sel, cko2_podf, cko2, cko, vdoa, clk_max + spdif, cko2_sel, cko2_podf, cko2, cko, vdoa, ldb_di0_div_7, ldb_di1_div_7, + ldb_di0_div_sel, ldb_di1_div_sel, clk_max };
static struct clk *clk[clk_max]; @@ -387,6 +390,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[ipu2_sel] = imx_clk_mux("ipu2_sel", base + 0x3c, 14, 2, ipu_sels, ARRAY_SIZE(ipu_sels)); clk[ldb_di0_sel] = imx_clk_mux_flags("ldb_di0_sel", base + 0x2c, 9, 3, ldb_di_sels, ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT); clk[ldb_di1_sel] = imx_clk_mux_flags("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels, ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT); + clk[ldb_di0_div_sel] = imx_clk_mux_flags("ldb_di0_div_sel", base + 0x20, 10, 1, ldb_di0_div_sels, ARRAY_SIZE(ldb_di0_div_sels), CLK_SET_RATE_PARENT); + clk[ldb_di1_div_sel] = imx_clk_mux_flags("ldb_di1_div_sel", base + 0x20, 11, 1, ldb_di1_div_sels, ARRAY_SIZE(ldb_di1_div_sels), CLK_SET_RATE_PARENT); clk[ipu1_di0_pre_sel] = imx_clk_mux("ipu1_di0_pre_sel", base + 0x34, 6, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels)); clk[ipu1_di1_pre_sel] = imx_clk_mux("ipu1_di1_pre_sel", base + 0x34, 15, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels)); clk[ipu2_di0_pre_sel] = imx_clk_mux("ipu2_di0_pre_sel", base + 0x38, 6, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels)); @@ -436,9 +441,9 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[ipu1_podf] = imx_clk_divider("ipu1_podf", "ipu1_sel", base + 0x3c, 11, 3); clk[ipu2_podf] = imx_clk_divider("ipu2_podf", "ipu2_sel", base + 0x3c, 16, 3); clk[ldb_di0_div_3_5] = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7); - clk[ldb_di0_podf] = imx_clk_divider_flags("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1, 0); + clk[ldb_di0_div_7] = imx_clk_fixed_factor("ldb_di0_div_7", "ldb_di0_sel", 1, 7); clk[ldb_di1_div_3_5] = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7); - clk[ldb_di1_podf] = imx_clk_divider_flags("ldb_di1_podf", "ldb_di1_div_3_5", base + 0x20, 11, 1, 0); + clk[ldb_di1_div_7] = imx_clk_fixed_factor("ldb_di1_div_7", "ldb_di1_sel", 1, 7); clk[ipu1_di0_pre] = imx_clk_divider("ipu1_di0_pre", "ipu1_di0_pre_sel", base + 0x34, 3, 3); clk[ipu1_di1_pre] = imx_clk_divider("ipu1_di1_pre", "ipu1_di1_pre_sel", base + 0x34, 12, 3); clk[ipu2_di0_pre] = imx_clk_divider("ipu2_di0_pre", "ipu2_di0_pre_sel", base + 0x38, 3, 3); @@ -508,8 +513,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[ipu1_di1] = imx_clk_gate2("ipu1_di1", "ipu1_di1_sel", base + 0x74, 4); clk[ipu2] = imx_clk_gate2("ipu2", "ipu2_podf", base + 0x74, 6); clk[ipu2_di0] = imx_clk_gate2("ipu2_di0", "ipu2_di0_sel", base + 0x74, 8); - clk[ldb_di0] = imx_clk_gate2("ldb_di0", "ldb_di0_podf", base + 0x74, 12); - clk[ldb_di1] = imx_clk_gate2("ldb_di1", "ldb_di1_podf", base + 0x74, 14); + clk[ldb_di0] = imx_clk_gate2("ldb_di0", "ldb_di0_div_sel", base + 0x74, 12); + clk[ldb_di1] = imx_clk_gate2("ldb_di1", "ldb_di1_div_sel", base + 0x74, 14); clk[ipu2_di1] = imx_clk_gate2("ipu2_di1", "ipu2_di1_sel", base + 0x74, 10); clk[hsi_tx] = imx_clk_gate2("hsi_tx", "hsi_tx_podf", base + 0x74, 16); if (cpu_is_imx6dl())
On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying Ying.Liu@freescale.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt index 5a90a72..90e923e 100644 --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt @@ -89,8 +89,6 @@ clocks and IDs. gpu3d_shader 74 ipu1_podf 75 ipu2_podf 76
ldb_di0_podf 77ldb_di1_podf 78 ipu1_di0_pre 79 ipu1_di1_pre 80 ipu2_di0_pre 81
This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
Quoting Fabio Estevam (2013-08-20 08:40:52)
On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying Ying.Liu@freescale.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt index 5a90a72..90e923e 100644 --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt @@ -89,8 +89,6 @@ clocks and IDs. gpu3d_shader 74 ipu1_podf 75 ipu2_podf 76
ldb_di0_podf 77ldb_di1_podf 78 ipu1_di0_pre 79 ipu1_di1_pre 80 ipu2_di0_pre 81This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
How does this fit in with the idea of having a stable binding/ABI? Seems like changing this would be a bad idea for devices in the field that have older DTBs.
Regards, Mike
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 20, 2013 at 02:18:27PM -0700, Mike Turquette wrote:
Quoting Fabio Estevam (2013-08-20 08:40:52)
On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying Ying.Liu@freescale.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt index 5a90a72..90e923e 100644 --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt @@ -89,8 +89,6 @@ clocks and IDs. gpu3d_shader 74 ipu1_podf 75 ipu2_podf 76
ldb_di0_podf 77ldb_di1_podf 78 ipu1_di0_pre 79 ipu1_di1_pre 80 ipu2_di0_pre 81This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
How does this fit in with the idea of having a stable binding/ABI? Seems like changing this would be a bad idea for devices in the field that have older DTBs.
We should be safe since none of existing DTBs refers to the clocks (they are not leaf clocks in the whole clock tree but some interconnection ones).
Shawn
On 08/20/2013 11:40 PM, Fabio Estevam wrote:
On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying Ying.Liu@freescale.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt index 5a90a72..90e923e 100644 --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt @@ -89,8 +89,6 @@ clocks and IDs. gpu3d_shader 74 ipu1_podf 75 ipu2_podf 76
ldb_di0_podf 77ldb_di1_podf 78 ipu1_di0_pre 79 ipu1_di1_pre 80 ipu2_di0_pre 81This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
I find there is a 'hole' in Documentation/devicetree/bindings/clock/imx5-clock.txt as well. The 'hole' was taken by tve_di(26) clock before.
Is this more acceptable?
ldb_di0_podf_unused 77 ldb_di1_podf_unused 78
Liu Ying
This patch adds di[0/1]_div_3_5, di[0/1]_div_7 and di[0/1]_div_sel clocks to the ldb nodes so that the ldb driver may use them to setup the display clock trees.
Signed-off-by: Liu Ying Ying.Liu@freescale.com --- arch/arm/boot/dts/imx6dl.dtsi | 8 ++++++-- arch/arm/boot/dts/imx6q.dtsi | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi index 9e8ae11..ff0d743 100644 --- a/arch/arm/boot/dts/imx6dl.dtsi +++ b/arch/arm/boot/dts/imx6dl.dtsi @@ -75,10 +75,14 @@ &ldb { clocks = <&clks 33>, <&clks 34>, <&clks 39>, <&clks 40>, - <&clks 135>, <&clks 136>; + <&clks 135>, <&clks 136>, + <&clks 184>, <&clks 185>, <&clks 203>, <&clks 204>, + <&clks 205>, <&clks 206>; clock-names = "di0_pll", "di1_pll", "di0_sel", "di1_sel", - "di0", "di1"; + "di0", "di1", + "di0_div_3_5", "di1_div_3_5", "di0_div_7", "di1_div_7", + "di0_div_sel", "di1_div_sel";
lvds-channel@0 { crtcs = <&ipu1 0>, <&ipu1 1>; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index f024ef2..b078a42 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -146,10 +146,14 @@ &ldb { clocks = <&clks 33>, <&clks 34>, <&clks 39>, <&clks 40>, <&clks 41>, <&clks 42>, - <&clks 135>, <&clks 136>; + <&clks 135>, <&clks 136>, + <&clks 184>, <&clks 185>, <&clks 203>, <&clks 204>, + <&clks 205>, <&clks 206>; clock-names = "di0_pll", "di1_pll", "di0_sel", "di1_sel", "di2_sel", "di3_sel", - "di0", "di1"; + "di0", "di1", + "di0_div_3_5", "di1_div_3_5", "di0_div_7", "di1_div_7", + "di0_div_sel", "di1_div_sel";
lvds-channel@0 { crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
In ldb split mode, the ldb_di[0/1]_ipu_div dividers should be configured as clock dividers of 1/3.5, while in others ldb modes of 1/7. This patch gets the di[0/1]_div_3_5, di[0/1]_div_7 and di[0/1]_div_sel clocks and sets the di[0/1]_div_3_5 or di[0/1]_div_7 clocks to be the parents of di[0/1]_div_sel clocks according to the ldb mode. The real dividers are the two fixed factors bewteen the ldb_di[0/1] and the pll clocks, so it's unnecessary to set the frequency for the ldb_di[0/1] clocks again after pll clock frequency is set. This patch removes the redundant clock frequency setting code as well.
Signed-off-by: Liu Ying Ying.Liu@freescale.com --- drivers/staging/imx-drm/imx-ldb.c | 38 +++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index 8af7f3b..7c553b8 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -81,6 +81,9 @@ struct imx_ldb { struct clk *clk[2]; /* our own clock */ struct clk *clk_sel[4]; /* parent of display clock */ struct clk *clk_pll[2]; /* upstream clock we can adjust */ + struct clk *clk_div_3_5[2]; /* fixed factor of 1/3.5 */ + struct clk *clk_div_7[2]; /* fixed factor of 1/7 */ + struct clk *clk_div_sel[2]; /* 1/3.5 or 1/7 */ u32 ldb_ctrl; const struct bus_mux *lvds_mux; }; @@ -150,6 +153,18 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, { int ret;
+ if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) { + ret = clk_set_parent(ldb->clk_div_sel[chno], ldb->clk_div_3_5[chno]); + if (ret) + dev_err(ldb->dev, "unable to set di%d_div_sel parent clock " + "to di%d_div_3_5\n", chno, chno); + } else { + ret = clk_set_parent(ldb->clk_div_sel[chno], ldb->clk_div_7[chno]); + if (ret) + dev_err(ldb->dev, "unable to set di%d_div_sel parent clock " + "to di%d_div_7\n", chno, chno); + } + dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__, clk_get_rate(ldb->clk_pll[chno]), serial_clk); clk_set_rate(ldb->clk_pll[chno], serial_clk); @@ -157,14 +172,6 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, dev_dbg(ldb->dev, "%s after: %ld\n", __func__, clk_get_rate(ldb->clk_pll[chno]));
- dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__, - clk_get_rate(ldb->clk[chno]), - (long int)di_clk); - clk_set_rate(ldb->clk[chno], di_clk); - - dev_dbg(ldb->dev, "%s after: %ld\n", __func__, - clk_get_rate(ldb->clk[chno])); - /* set display clock mux to LDB input clock */ ret = clk_set_parent(ldb->clk_sel[mux], ldb->clk[chno]); if (ret) { @@ -362,6 +369,21 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) if (IS_ERR(ldb->clk_pll[chno])) return PTR_ERR(ldb->clk_pll[chno]);
+ sprintf(clkname, "di%d_div_3_5", chno); + ldb->clk_div_3_5[chno] = devm_clk_get(ldb->dev, clkname); + if (IS_ERR(ldb->clk_div_3_5[chno])) + return PTR_ERR(ldb->clk_div_3_5[chno]); + + sprintf(clkname, "di%d_div_7", chno); + ldb->clk_div_7[chno] = devm_clk_get(ldb->dev, clkname); + if (IS_ERR(ldb->clk_div_7[chno])) + return PTR_ERR(ldb->clk_div_7[chno]); + + sprintf(clkname, "di%d_div_sel", chno); + ldb->clk_div_sel[chno] = devm_clk_get(ldb->dev, clkname); + if (IS_ERR(ldb->clk_div_sel[chno])) + return PTR_ERR(ldb->clk_div_sel[chno]); + return 0; }
Am Dienstag, den 20.08.2013, 16:38 +0800 schrieb Liu Ying:
The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5 drivider or a 1/7 divider. The common clock framework cannot deal with the two dividers directly even with the divider table which only supports integral dividers. So, the idea is to take the 1/3.5 and 1/7 dividers as separate fixed factor dividers and introduce a new multiplexer clock to be derived from the them. Then, the ldb display clock trees can be setup correctly. This series contains the necessary clock driver changes, dts code changes and imx-drm/ldb driver changes to fullfill the task.
I don't see how this improves the situation. Does this solve any real problem?
While I admit to having introduced the combination of 1/3.5 fixed divider and configurable 1/1,1/2 divder clocks to describe this fractional divider for the reasons you state, I think the correct solution would be to improve the table divider to support fractional values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not introduce more virtual clocks.
regards Philipp
On 08/20/2013 05:43 PM, Philipp Zabel wrote:
Am Dienstag, den 20.08.2013, 16:38 +0800 schrieb Liu Ying:
The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5 drivider or a 1/7 divider. The common clock framework cannot deal with the two dividers directly even with the divider table which only supports integral dividers. So, the idea is to take the 1/3.5 and 1/7 dividers as separate fixed factor dividers and introduce a new multiplexer clock to be derived from the them. Then, the ldb display clock trees can be setup correctly. This series contains the necessary clock driver changes, dts code changes and imx-drm/ldb driver changes to fullfill the task.
I don't see how this improves the situation. Does this solve any real problem?
I don't see any functional problem without this series. But, it may correct ldb_di[n] clock frequency returned from clk_get_rate() when using 1/7 divider. Furthermore, since this series makes the ldb related clocks from pll to ldb_di[0/1] have the CLK_SET_RATE_PARENT flag set, the imx-drm/ldb driver may set the clocks' frequency more flexibly, i.e., only calling clk_set_rate() for ldb_di[n] clock would be an alternative.
While I admit to having introduced the combination of 1/3.5 fixed divider and configurable 1/1,1/2 divder clocks to describe this fractional divider for the reasons you state, I think the correct solution would be to improve the table divider to support fractional values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not introduce more virtual clocks.
Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this). I see there is something similar in 'include/linux/sh_clk.h'.
regards Philipp
Regards, Liu Ying
Hi Ying,
On Tue, Aug 20, 2013 at 06:08:48PM +0800, Liu Ying wrote:
While I admit to having introduced the combination of 1/3.5 fixed divider and configurable 1/1,1/2 divder clocks to describe this fractional divider for the reasons you state, I think the correct solution would be to improve the table divider to support fractional values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not introduce more virtual clocks.
Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this). I see there is something similar in 'include/linux/sh_clk.h'.
Yeah, I agree with Philipp on improving table divider to support fractional values. Would you like to work on that?
Shawn
On 08/21/2013 09:59 AM, Shawn Guo wrote:
Hi Ying,
On Tue, Aug 20, 2013 at 06:08:48PM +0800, Liu Ying wrote:
While I admit to having introduced the combination of 1/3.5 fixed divider and configurable 1/1,1/2 divder clocks to describe this fractional divider for the reasons you state, I think the correct solution would be to improve the table divider to support fractional values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not introduce more virtual clocks.
Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this). I see there is something similar in 'include/linux/sh_clk.h'.
Yeah, I agree with Philipp on improving table divider to support fractional values. Would you like to work on that?
Shawn
I don't have a plan to work on that now.
Liu Ying
dri-devel@lists.freedesktop.org
-
Fabio Estevam -
Liu Ying -
Mike Turquette -
Philipp Zabel -
Shawn Guo