Add display clock controller driver for SDM845
v3 cover Add display clock controller driver for SDM845
[v3]
* Move frequency table macro to common file,
add the patch along to maintain dependency.
[v2]
* Removed unused header file includes.
* Moved the frequency table macro to a common file [1].
* Move to pll config to probe.
* Update SoC name in device tree binding and also update the Kconfig.
Add support for the display clock controller found on SDM845 based devices. This would allow display drivers to probe and control their clocks.
Taniya Das (3):
clk: qcom: Move frequency table macro to common file
dt-bindings: clock: Introduce QCOM Display clock bindings
clk: qcom: Add display clock controller driver for SDM845
.../devicetree/bindings/clock/qcom,dispcc.txt | 19 + drivers/clk/qcom/Kconfig | 10 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-rcg.h | 2 + drivers/clk/qcom/dispcc-sdm845.c | 674 +++++++++++++++++++++ drivers/clk/qcom/gcc-apq8084.c | 2 - drivers/clk/qcom/gcc-ipq4019.c | 2 - drivers/clk/qcom/gcc-ipq8074.c | 2 - drivers/clk/qcom/gcc-msm8916.c | 2 - drivers/clk/qcom/gcc-msm8974.c | 2 - drivers/clk/qcom/gcc-msm8994.c | 2 - drivers/clk/qcom/gcc-msm8996.c | 2 - drivers/clk/qcom/gcc-msm8998.c | 2 - drivers/clk/qcom/gcc-sdm845.c | 2 - drivers/clk/qcom/mmcc-apq8084.c | 2 - drivers/clk/qcom/mmcc-msm8974.c | 2 - drivers/clk/qcom/mmcc-msm8996.c | 2 - drivers/clk/qcom/videocc-sdm845.c | 2 - include/dt-bindings/clock/qcom,dispcc-sdm845.h | 45 ++ 19 files changed, 751 insertions(+), 26 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt create mode 100644 drivers/clk/qcom/dispcc-sdm845.c create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v3 1/3 clk: qcom: Move frequency table macro to common file
Frequency table macro is used by multiple clock drivers, move frequency table macro to common header file.
--- drivers/clk/qcom/clk-rcg.h | 2 ++ drivers/clk/qcom/gcc-apq8084.c | 2 -- drivers/clk/qcom/gcc-ipq4019.c | 2 -- drivers/clk/qcom/gcc-ipq8074.c | 2 -- drivers/clk/qcom/gcc-msm8916.c | 2 -- drivers/clk/qcom/gcc-msm8974.c | 2 -- drivers/clk/qcom/gcc-msm8994.c | 2 -- drivers/clk/qcom/gcc-msm8996.c | 2 -- drivers/clk/qcom/gcc-msm8998.c | 2 -- drivers/clk/qcom/gcc-sdm845.c | 2 -- drivers/clk/qcom/mmcc-apq8084.c | 2 -- drivers/clk/qcom/mmcc-msm8974.c | 2 -- drivers/clk/qcom/mmcc-msm8996.c | 2 -- drivers/clk/qcom/videocc-sdm845.c | 2 -- 14 files changed, 2 insertions(+), 26 deletions(-)
drivers/clk/qcom/clk-rcg.h +2 -0
@@ -7,6 +7,8 @@#include <linux/clk-provider.h> #include "clk-regmap.h" +#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } +struct freq_tbl {unsigned long freq;u8 src;
drivers/clk/qcom/gcc-apq8084.c +0 -2
@@ -106,8 +106,6 @@ enum { "sleep_clk_src", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_pll gpll0 = { .l_reg =0x0004 , .m_reg =0x0008 ,
drivers/clk/qcom/gcc-ipq4019.c +0 -2
@@ -179,8 +179,6 @@ struct clk_fepll { "ddrpllapss", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static const struct freq_tbl ftbl_gcc_audio_pwm_clk[] = {F (48000000 ,P_XO ,1 ,0 ,0 ),F (200000000 ,P_FEPLL200 ,1 ,0 ,0 ),
drivers/clk/qcom/gcc-ipq8074.c +0 -2
@@ -32,8 +32,6 @@#include "clk-regmap-mux.h" #include "reset.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_XO ,P_GPLL0 ,
drivers/clk/qcom/gcc-msm8916.c +0 -2
@@ -264,8 +264,6 @@ enum { "sleep_clk", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_pll gpll0 = { .l_reg =0x21004 , .m_reg =0x21008 ,
drivers/clk/qcom/gcc-msm8974.c +0 -2
@@ -62,8 +62,6 @@ enum { "gpll4_vote", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_pll gpll0 = { .l_reg =0x0004 , .m_reg =0x0008 ,
drivers/clk/qcom/gcc-msm8994.c +0 -2
@@ -57,8 +57,6 @@ enum { "gpll4", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_fixed_factor xo = { .mult =1 , .div =1 ,
drivers/clk/qcom/gcc-msm8996.c +0 -2
@@ -32,8 +32,6 @@#include "reset.h" #include "gdsc.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_XO ,P_GPLL0 ,
drivers/clk/qcom/gcc-msm8998.c +0 -2
@@ -25,8 +25,6 @@#include "reset.h" #include "gdsc.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_AUD_REF_CLK ,P_CORE_BI_PLL_TEST_SE ,
drivers/clk/qcom/gcc-sdm845.c +0 -2
@@ -25,8 +25,6 @@#include "gdsc.h" #include "reset.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_BI_TCXO ,P_AUD_REF_CLK ,
drivers/clk/qcom/mmcc-apq8084.c +0 -2
@@ -219,8 +219,6 @@ enum { "sleep_clk_src", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_pll mmpll0 = { .l_reg =0x0004 , .m_reg =0x0008 ,
drivers/clk/qcom/mmcc-msm8974.c +0 -2
@@ -184,8 +184,6 @@ enum { "dsi1pllbyte", }; -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -static struct clk_pll mmpll0 = { .l_reg =0x0004 , .m_reg =0x0008 ,
drivers/clk/qcom/mmcc-msm8996.c +0 -2
@@ -34,8 +34,6 @@#include "reset.h" #include "gdsc.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_XO ,P_MMPLL0 ,
drivers/clk/qcom/videocc-sdm845.c +0 -2
@@ -18,8 +18,6 @@#include "clk-pll.h" #include "gdsc.h" -#define F (f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } -enum {P_BI_TCXO ,P_CORE_BI_PLL_TEST_SE ,
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [PATCH v3 1/3] clk: qcom: Move frequency table macro to common file
Quoting Taniya Das (2018-06-23 07:19:25)
Quoted 5 lines
Frequency table macro is used by multiple clock drivers, move frequency table macro to common header file.
Signed-off-by Taniya Das <[email protected]>---
Applied to clk-next
PATCH v3 2/3 dt-bindings: clock: Introduce QCOM Display clock bindings
In reply to Taniya DasAdd device tree bindings for display clock controller for Qualcomm Technology Inc's SDM845 SoCs.
--- .../devicetree/bindings/clock/qcom,dispcc.txt | 19 +++++++++ include/dt-bindings/clock/qcom,dispcc-sdm845.h | 45 ++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.txt create mode 100644 include/dt-bindings/clock/qcom,dispcc-sdm845.h
Documentation/devicetree/bindings/clock/qcom,dispcc.txt new file +19 -0
@@ -0,0 +1,19 @@ +Qualcomm Technologies, Inc. Display Clock Controller Binding +------------------------------------------------------------ + +Required properties : + +- compatible : shall contain "qcom,sdm845-dispcc" +- reg : shall contain base register location and length. +- #clock-cells : from common clock binding, shall contain 1. +- #reset-cells : from common reset binding, shall contain 1. +- #power-domain-cells : from generic power domain binding, shall contain 1. + +Example: + dispcc: clock-controller@af00000 { + compatible = "qcom,sdm845-dispcc"; + reg = <0xaf00000 0x100000>; + #clock-cells = <1>; + #reset-cells = <1>; + #power-domain-cells = <1>; + };
include/dt-bindings/clock/qcom,dispcc-sdm845.h new file +45 -0
45 lines not on this page · raw message
Signature · 2 lines
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [PATCH v3 2/3] dt-bindings: clock: Introduce QCOM Display clock bindings
Quoting Taniya Das (2018-06-23 07:19:26)
Quoted 6 lines
Add device tree bindings for display clock controller for Qualcomm Technology Inc's SDM845 SoCs.
Signed-off-by Taniya Das <[email protected]>Reviewed-by Rob Herring <[email protected]>---
Applied to clk-next
PATCH v3 3/3 clk: qcom: Add display clock controller driver for SDM845
In reply to Taniya DasAdd support for the display clock controller found on SDM845 based devices. This would allow display drivers to probe and control their clocks.
--- drivers/clk/qcom/Kconfig | 10 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/dispcc-sdm845.c | 674 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 685 insertions(+) create mode 100644 drivers/clk/qcom/dispcc-sdm845.c
drivers/clk/qcom/Kconfig +10 -0
@@ -245,6 +245,16 @@ config SDM_VIDEOCC_845 Say Y if you want to support video devices and functionality such as video encode and decode. +config SDM_DISPCC_845 + tristate "SDM845 Display Clock Controller" + select SDM_GCC_845 + depends on COMMON_CLK_QCOM + help + Support for the display clock controller on Qualcomm Technologies, Inc + SDM845 devices. + Say Y if you want to support display devices and functionality such as + splash screen. + config SPMI_PMIC_CLKDIV tristate "SPMI PMIC clkdiv Support" depends on (COMMON_CLK_QCOM && SPMI) || COMPILE_TEST
drivers/clk/qcom/Makefile +1 -0
@@ -38,6 +38,7 @@ obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o +obj-$(CONFIG_SDM_DISPCC_845) += dispcc-sdm845.o obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
drivers/clk/qcom/dispcc-sdm845.c new file +674 -0
674 lines not on this page · raw message
Signature · 2 lines
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoting Taniya Das (2018-06-23 07:19:27)
Quoted 7 lines
drivers/clk/qcom/dispcc-sdm845.c new file +1
-0@@ -0,0 +1,674 @@ +// SPDX-License-Identifier: GPL-2.0
[...]
Quoted 13 lines
drivers/clk/qcom/dispcc-sdm845.c new file +13
-0+static struct clk_alpha_pll disp_cc_pll0 = { + .offset =0x0 , + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA ], + .clkr = { + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_pll0" , + .parent_names = (const char *[]){"bi_tcxo" }, + .num_parents =1 , + .ops = &clk_alpha_pll_fabia_ops, + }, + }, +}; +
drivers/clk/qcom/dispcc-sdm845.c line 110
Quoted 10 lines
drivers/clk/qcom/dispcc-sdm845.c new file +10
-0+static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = { + .cmd_rcgr =0x20d0 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_byte0_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE ,
Why is there the no cache flag? Last time I asked I don't think I got any answer, and there isn't a comment here so please at least add a comment to the code so we don't forget.
Quoted 51 lines
drivers/clk/qcom/dispcc-sdm845.c new file +51
-0+ .ops = &clk_byte2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_byte1_clk_src = { + .cmd_rcgr =0x20ec , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_byte1_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_byte2_ops, + }, +}; + +static const struct freq_tbl ftbl_disp_cc_mdss_esc0_clk_src[] = { +F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), + { } +}; + +static struct clk_rcg2 disp_cc_mdss_esc0_clk_src = { + .cmd_rcgr =0x2108 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_esc0_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .ops = &clk_rcg2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_esc1_clk_src = { + .cmd_rcgr =0x2120 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_esc1_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .ops = &clk_rcg2_ops, + }, +}; +
[...]
drivers/clk/qcom/dispcc-sdm845.c line 674
Quoted 2 lines
drivers/clk/qcom/dispcc-sdm845.c new file +2
-0+ +MODULE_LICENSE ("GPL v2" );
MODULE_DESCRIPTION?
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Hello Stephen,
Thanks for your review comments.
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-06-23 07:19:27)
Quoted 7 lines
drivers/clk/qcom/dispcc-sdm845.c new file +1
-0@@ -0,0 +1,674 @@ +// SPDX-License-Identifier: GPL-2.0 [...]
Quoted 19 lines
drivers/clk/qcom/dispcc-sdm845.c new file +19
-0+static struct clk_alpha_pll disp_cc_pll0 = { + .offset =0x0 , + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA ], + .clkr = { + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_pll0" , + .parent_names = (const char *[]){"bi_tcxo" }, + .num_parents =1 , + .ops = &clk_alpha_pll_fabia_ops, + }, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = { + .cmd_rcgr =0x20d0 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){
Quoted 9 lines
drivers/clk/qcom/dispcc-sdm845.c line 110
Quoted 4 lines
drivers/clk/qcom/dispcc-sdm845.c new file +4
-0+ .name = "disp_cc_mdss_byte0_clk_src", + .parent_names = disp_cc_parent_names_0, + .num_parents = 4, + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,Why is there the no cache flag? Last time I asked I don't think I got any answer, and there isn't a comment here so please at least add a comment to the code so we don't forget.
I think you missed my comment from the earlier email. I would add the comment and submit again.
Quoted 2 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 57 lines
Quoted 51 lines
drivers/clk/qcom/dispcc-sdm845.c new file +51
-0+ .ops = &clk_byte2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_byte1_clk_src = { + .cmd_rcgr =0x20ec , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_byte1_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_byte2_ops, + }, +}; + +static const struct freq_tbl ftbl_disp_cc_mdss_esc0_clk_src[] = { +F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), + { } +}; + +static struct clk_rcg2 disp_cc_mdss_esc0_clk_src = { + .cmd_rcgr =0x2108 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_esc0_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .ops = &clk_rcg2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_esc1_clk_src = { + .cmd_rcgr =0x2120 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .freq_tbl = ftbl_disp_cc_mdss_esc0_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_esc1_clk_src" , + .parent_names = disp_cc_parent_names_0, + .num_parents =4 , + .ops = &clk_rcg2_ops, + }, +}; +[...]
drivers/clk/qcom/dispcc-sdm845.c line 674
Quoted 2 lines
drivers/clk/qcom/dispcc-sdm845.c new file +2
-0+ +MODULE_LICENSE ("GPL v2" );
MODULE_DESCRIPTION?
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoting Taniya Das (2018-07-08 20:38:03)
Quoted 43 lines
Hello Stephen,
Thanks for your review comments.
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-06-23 07:19:27)
Quoted 7 lines
drivers/clk/qcom/dispcc-sdm845.c new file +1
-0@@ -0,0 +1,674 @@ +// SPDX-License-Identifier: GPL-2.0 [...]
Quoted 19 lines
drivers/clk/qcom/dispcc-sdm845.c new file +19
-0+static struct clk_alpha_pll disp_cc_pll0 = { + .offset =0x0 , + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA ], + .clkr = { + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_pll0" , + .parent_names = (const char *[]){"bi_tcxo" }, + .num_parents =1 , + .ops = &clk_alpha_pll_fabia_ops, + }, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = { + .cmd_rcgr =0x20d0 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){Quoted 9 lines
drivers/clk/qcom/dispcc-sdm845.c line 110
Quoted 4 lines
drivers/clk/qcom/dispcc-sdm845.c new file +4
-0+ .name = "disp_cc_mdss_byte0_clk_src", + .parent_names = disp_cc_parent_names_0, + .num_parents = 4, + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,Why is there the no cache flag? Last time I asked I don't think I got any answer, and there isn't a comment here so please at least add a comment to the code so we don't forget.
Quoted 2 lines
I think you missed my comment from the earlier email. I would add the comment and submit again.
Hmm.. ok.
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoted 53 lines · Stephen Boyd
Quoting Taniya Das (2018-07-08 20:38:03)
Quoted 43 lines
Hello Stephen,
Thanks for your review comments.
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-06-23 07:19:27)
Quoted 7 lines
drivers/clk/qcom/dispcc-sdm845.c new file +1
-0@@ -0,0 +1,674 @@ +// SPDX-License-Identifier: GPL-2.0 [...]
Quoted 19 lines
drivers/clk/qcom/dispcc-sdm845.c new file +19
-0+static struct clk_alpha_pll disp_cc_pll0 = { + .offset =0x0 , + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA ], + .clkr = { + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_pll0" , + .parent_names = (const char *[]){"bi_tcxo" }, + .num_parents =1 , + .ops = &clk_alpha_pll_fabia_ops, + }, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = { + .cmd_rcgr =0x20d0 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_0, + .clkr.hw.init = &(struct clk_init_data ){Quoted 9 lines
drivers/clk/qcom/dispcc-sdm845.c line 110
Quoted 4 lines
drivers/clk/qcom/dispcc-sdm845.c new file +4
-0+ .name = "disp_cc_mdss_byte0_clk_src", + .parent_names = disp_cc_parent_names_0, + .num_parents = 4, + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,Why is there the no cache flag? Last time I asked I don't think I got any answer, and there isn't a comment here so please at least add a comment to the code so we don't forget.
Quoted 2 lines
I think you missed my comment from the earlier email. I would add the comment and submit again.
Hmm.. ok.
Quoted 4 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
Quoted 8 lines
Quoted 3 lines
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 11 lines · Stephen Boyd
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 7 lines
Quoted 4 lines
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoted 16 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 15 lines · Stephen Boyd
Quoted 12 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
Quoted 7 lines
Quoted 3 lines
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 23 lines · Stephen Boyd
Quoted 20 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 18 lines · Stephen Boyd
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
Quoted 8 lines
Quoted 3 lines
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Quoted 7 lines
Quoted 3 lines
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
++ Display driver team,
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 27 lines · Stephen Boyd
Quoted 23 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 11 lines · Stephen Boyd
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 7 lines
Quoted 4 lines
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Quoted 7 lines
Quoted 3 lines
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
As per my discussions offline with the display teams,
There is a use-case where the clock framework is unaware of the PLL VCO frequency change and thus the drivers would query to get the actual HW frequency rather than the cached one.
Do you think keeping these flags would have any impact other than always getting the non-cached rates?
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoting Taniya Das (2018-07-12 10:21:33)
Quoted 38 lines
++ Display driver team,
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 27 lines · Stephen Boyd
Quoted 23 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 11 lines · Stephen Boyd
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 7 lines
Quoted 4 lines
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Quoted 7 lines
Quoted 3 lines
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
Quoted 9 lines
As per my discussions offline with the display teams,
There is a use-case where the clock framework is unaware of the PLL VCO frequency change and thus the drivers would query to get the actual HW frequency rather than the cached one.
Do you think keeping these flags would have any impact other than always getting the non-cached rates?
The flag will make it so clk_get_rate() works in spite of something changing the frequency behind the framework's back, but I want to understand what and why it's changing without framework involvement. We shouldn't need the flag here, because this flag is typically for clks that are controlled by some other entity that the kernel doesn't have control over. In this case, it seems like we have full control of the clk tree for the display PLL down to this clk, so it should be perfectly fine to not have this flag. The presence of the flag means that the display driver is doing something wrong.
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Quoted 49 lines · Stephen Boyd
Quoting Taniya Das (2018-07-12 10:21:33)
Quoted 38 lines
++ Display driver team,
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 27 lines · Stephen Boyd
Quoted 23 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 11 lines · Stephen Boyd
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 7 lines
Quoted 4 lines
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Quoted 7 lines
Quoted 3 lines
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
Quoted 9 lines
As per my discussions offline with the display teams,
There is a use-case where the clock framework is unaware of the PLL VCO frequency change and thus the drivers would query to get the actual HW frequency rather than the cached one.
Do you think keeping these flags would have any impact other than always getting the non-cached rates?
Quoted 9 lines
The flag will make it so
clk_get_rate()works in spite of something changing the frequency behind the framework's back, but I want to understand what and why it's changing without framework involvement. We shouldn't need the flag here, because this flag is typically for clks that are controlled by some other entity that the kernel doesn't have control over. In this case, it seems like we have full control of the clk tree for the display PLL down to this clk, so it should be perfectly fine to not have this flag. The presence of the flag means that the display driver is doing something wrong.
These clocks are sourced from DSI PLL. In dsi command mode there is an idle use case when there is no activity on display, we switch of the source DSI PLL to save power by calling clk_disable_unprepare(). In this scenario if some client queries the clk_get_rate(), then if NO_CACHE flag is used the clk driver will return the cached rate which is some non-zero value set when we last called clk_set_rate(), before enabling the clock, whereas actually in HW this clk is disabled. So we used the NO_CACHE flag for the call to land in clk_recalc_rate and we return zero if the PLL is disabled.
I remember some customer had scripts that runs through all the clocks during idle screen scenario and call clk_get_rate(), where they complained display clk_get_rate returns none zero value.
Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
Hello Stephen,
Quoted 69 lines · On 7/13/2018 1:55 PM, [email protected] wrote:
Quoted 58 lines · Stephen Boyd
Quoting Taniya Das (2018-07-12 10:21:33)
Quoted 47 lines
++ Display driver team,
Quoted 10 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 9 lines · Stephen Boyd
Quoted 6 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 5 lines · Stephen Boyd
Quoted 2 lines
Quoted 2 lines
> Why is the nocache flag needed? Applies to all clks in
this file.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
>
This flag is required for all RCGs whose PLLs are controlled
outside the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
clock controller. The display code would require the
recalculated rate
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
always.
Right. Why is the PLL controlled outside of the clock
controller? The
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but
in the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
display subsystem which are managed by the DRM drivers. When DRM
drivers
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
query for the rate clock driver should always return the non
cached rates.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Why? Is the DSI PLL changing rate all the time, randomly, without
going
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
through the clk APIs to do so?
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock
driver.
Quoted 5 lines
Quoted 1 line
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
Quoted 9 lines
As per my discussions offline with the display teams,
There is a use-case where the clock framework is unaware of the PLL VCO frequency change and thus the drivers would query to get the actual HW frequency rather than the cached one.
Do you think keeping these flags would have any impact other than always getting the non-cached rates?
Quoted 9 lines
The flag will make it so
clk_get_rate()works in spite of something changing the frequency behind the framework's back, but I want to understand what and why it's changing without framework involvement. We shouldn't need the flag here, because this flag is typically for clks that are controlled by some other entity that the kernel doesn't have control over. In this case, it seems like we have full control of the clk tree for the display PLL down to this clk, so it should be perfectly fine to not have this flag. The presence of the flag means that the display driver is doing something wrong.
Quoted 10 lines
These clocks are sourced from DSI PLL. In dsi command mode there is an idle use case when there is no activity on display, we switch of the source DSI PLL to save power by calling
clk_disable_unprepare(). In this scenario if some client queries theclk_get_rate(), then ifNO_CACHEflag is used the clk driver will return the cached rate which is some non-zero value set when we last calledclk_set_rate(), before enabling the clock, whereas actually in HW this clk is disabled. So we used theNO_CACHEflag for the call to land inclk_recalc_rateand we return zero if the PLL is disabled.
I remember some customer had scripts that runs through all the clocks during idle screen scenario and callclk_get_rate(), where they complained displayclk_get_ratereturns none zero value.
Do you think the clock driver needs any update for flag for the next series?
Quoted 4 lines
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerRe: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845
In reply to [email protected]Quoting [email protected] (2018-07-13 01:25:49)
Quoted 63 lines · Stephen Boyd
Quoted 51 lines
Quoting Taniya Das (2018-07-12 10:21:33)
Quoted 40 lines
++ Display driver team,
Quoted 28 lines · Stephen Boyd
Quoting Taniya Das (2018-07-09 02:34:07)
Quoted 27 lines · Stephen Boyd
Quoted 23 lines
Quoting Taniya Das (2018-07-09 00:07:21)
Quoted 11 lines · Stephen Boyd
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Why is the nocache flag needed? Applies to all clks in this file.
This flag is required for all RCGs whose PLLs are controlled outside the clock controller. The display code would require the recalculated rate always.
Quoted 7 lines
Quoted 4 lines
Right. Why is the PLL controlled outside of the clock controller? The rate should propagate upward to the PLL from here, so who's going outside of that?
The DSI0/1 PLL are not part of the display clock controller, but in the display subsystem which are managed by the DRM drivers. When DRM drivers query for the rate clock driver should always return the non cached rates.
Why? Is the DSI PLL changing rate all the time, randomly, without going through the clk APIs to do so?
Quoted 7 lines
Quoted 3 lines
Hmm, I am afraid I do not have an answer for this, but this was the requirement to always return the non cached rates from the clock driver.
Ok. Who knows about this requirement? Can we add someone from the display driver to understand more?
As per my discussions offline with the display teams,
Quoted 9 lines
There is a use-case where the clock framework is unaware of the PLL VCO
frequency change and thus the drivers would query to get the actual HW frequency rather than the cached one.Do you think keeping these flags would have any impact other than always
getting the non-cached rates?Quoted 10 lines
The flag will make it so
clk_get_rate()works in spite of something changing the frequency behind the framework's back, but I want to understand what and why it's changing without framework involvement. We shouldn't need the flag here, because this flag is typically for clks that are controlled by some other entity that the kernel doesn't have control over. In this case, it seems like we have full control of the clk tree for the display PLL down to this clk, so it should be perfectly
fine to not have this flag. The presence of the flag means that the display driver is doing something wrong.
Quoted 9 lines
These clocks are sourced from DSI PLL. In dsi command mode there is an idle use case when there
is no activity on display, we switch of the source DSI PLL to save power by callingclk_disable_unprepare().
In this scenario if some client queries theclk_get_rate(), then ifNO_CACHEflag is used the clk
driver will return the cached rate which is some non-zero value set when we last calledclk_set_rate(),
before enabling the clock, whereas actually in HW this clk is disabled.
If the clk is disabled in hardware it doesn't mean clk_get_rate() should return 0 Hz. The frequency of the clk is set to something, so we should return what that frequency is by reading the hardware.
Quoted 3 lines
So we used the
NO_CACHEflag
for the call to land inclk_recalc_rateand we return zero if the PLL is disabled.
This is super wrong. If the PLL is disabled it still has some frequency configured.