Add support for display port clocks and clock ops
v2 cover Add support for display port clocks and clock ops
[v2]
* Update KCONFIG to select RATIONAL
* Clean up redundant code from dp_set_rate/dp_set_rate_and_parent
* Update the disp_cc_mdss_dp_link_clk_src to use the byte2_ops instead of defining the frequencies in KHz.
* Clean up CLK_GET_RATE_NOCACHE from various RCGs of DP.
[v1]
* New display port clock ops supported for display port clocks.
* Also add support for the display port related branches and RCGs.
Taniya Das (2):
clk: qcom: rcg2: Add support for display port clock ops
clk: qcom : dispcc: Add support for display port clocks
drivers/clk/qcom/Kconfig | 1 + drivers/clk/qcom/clk-rcg.h | 1 + drivers/clk/qcom/clk-rcg2.c | 81 +++++++++- drivers/clk/qcom/dispcc-sdm845.c | 216 ++++++++++++++++++++++++- include/dt-bindings/clock/qcom,dispcc-sdm845.h | 13 +- 5 files changed, 309 insertions(+), 3 deletions(-)
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v2 1/2 clk: qcom: rcg2: Add support for display port clock ops
New display port clock ops supported for display port clocks.
--- drivers/clk/qcom/Kconfig | 1 + drivers/clk/qcom/clk-rcg.h | 1 + drivers/clk/qcom/clk-rcg2.c | 81 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 1 deletion(-)
drivers/clk/qcom/Kconfig +1 -0
@@ -15,6 +15,7 @@ menuconfig COMMON_CLK_QCOM depends on ARCH_QCOM || COMPILE_TEST select REGMAP_MMIO select RESET_CONTROLLER + select RATIONAL if COMMON_CLK_QCOM
drivers/clk/qcom/clk-rcg.h +1 -0
@@ -161,6 +161,7 @@ struct clk_rcg2 {extern const struct clk_ops clk_pixel_ops;extern const struct clk_ops clk_gfx3d_ops;extern const struct clk_ops clk_rcg2_shared_ops; +extern const struct clk_ops clk_dp_ops;struct clk_rcg_dfs_data {struct clk_rcg2 *rcg;
drivers/clk/qcom/clk-rcg2.c +80 -1
95 lines not on this page · raw message
--
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 v2 1/2] clk: qcom: rcg2: Add support for display port clock ops
Quoting Taniya Das (2019-05-14 21:20:38)
drivers/clk/qcom/Kconfig menuconfig COMMON_CLK_QCOM · line 18
Quoted 9 lines
drivers/clk/qcom/Kconfig +1
-0@@ -15,6 +15,7 @@ menuconfig COMMON_CLK_QCOM depends on ARCH_QCOM || COMPILE_TEST select REGMAP_MMIO select RESET_CONTROLLER + select RATIONAL
Make this an alphabetical list of selects please.
Quoted 18 lines
drivers/clk/qcom/Kconfig +0
-0if COMMON_CLK_QCOMdrivers/clk/qcom/clk-rcg2.c +7
-0@@ -1128,3 +1129,81 @@ int qcom_cc_register_rcg_dfs(struct regmap *regmap,return 0 ; }EXPORT_SYMBOL_GPL (qcom_cc_register_rcg_dfs); + +static int clk_rcg2_dp_set_rate (struct clk_hw *hw,unsigned long rate, +unsigned long parent_rate) +{ +struct clk_rcg2 *rcg =to_clk_rcg2 (hw); +struct freq_tbl f = {0 }; +u32 mask =BIT (rcg->hid_width) -1 ;
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1148
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+u32 hid_div, cfg; +int i, num_parents =clk_hw_get_num_parents (hw); +unsigned long num, den; + +rational_best_approximation (parent_rate, rate, +GENMASK (rcg->mnd_width -1 ,0 ), +GENMASK (rcg->mnd_width -1 ,0 ), &den, &num); + +if (!num || !den) { + pr_err("Invalid MN values derived for requested rate %lu\n",
Does this ever happen? I worry that this printk could happen many times if a driver gets into a bad state and starts selecting invalid frequencies over and over again for each frame (every 16ms). Maybe just return -EINVAL instead of printing anything.
Quoted 4 lines
drivers/clk/qcom/clk-rcg2.c +4
-0+ rate); +return -EINVAL ; + } +
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1162
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+regmap_read (rcg->clkr.regmap, rcg->cmd_rcgr +CFG_REG , &cfg); + hid_div = cfg; + cfg &=CFG_SRC_SEL_MASK ; + cfg >>=CFG_SRC_SEL_SHIFT ; + +for (i =0 ; i < num_parents; i++) +if (cfg == rcg->parent_map[i].cfg) { + f.src = rcg->parent_map[i].src; +break ; + }
Weird indent for this brace. Please fix and put a brace on the for statement too.
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1170
Quoted 8 lines
drivers/clk/qcom/clk-rcg2.c +8
-0+ + f.pre_div = hid_div; + f.pre_div >>=CFG_SRC_DIV_SHIFT ; + f.pre_div &= mask; + +if (num == den) { + f.m =0 ; + f.n =0 ;
Isn't this the default? So just have if (num != den) here.
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1183
Quoted 13 lines
drivers/clk/qcom/clk-rcg2.c +13
-0+ }else { + f.m = num; + f.n = den; + } + +return clk_rcg2_configure (rcg, &f); +} + +static int clk_rcg2_dp_set_rate_and_parent (struct clk_hw *hw, +unsigned long rate,unsigned long parent_rate,u8 index) +{ +return clk_rcg2_dp_set_rate (hw, rate, parent_rate); +}
Does this need to be implemented? The parent index isn't passed to clk_rcg2_dp_set_rate() so I suspect the parent index doesn't matter? Does the parent change?
Quoted 5 lines
drivers/clk/qcom/clk-rcg2.c +5
-0+ +static int clk_rcg2_dp_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ +struct clk_rate_request parent_req = *req;
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1198
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+int ret; + + ret =__clk_determine_rate (clk_hw_get_parent (hw), &parent_req); +if (ret) +return ret; + + req->best_parent_rate = parent_req.rate; + +return 0 ; +}
Do you need this op? It's just calling determine rate on the parent, so we already do that if the proper flag is set. I'm confused about this function.
Re: [PATCH v2 1/2] clk: qcom: rcg2: Add support for display port clock ops
Hello Stephen,
Thanks for your review.
Quoted 13 lines · Stephen Boyd
Quoting Taniya Das (2019-05-14 21:20:38)
drivers/clk/qcom/Kconfig menuconfig COMMON_CLK_QCOM · line 18
Quoted 9 lines
drivers/clk/qcom/Kconfig +1
-0@@ -15,6 +15,7 @@ menuconfig COMMON_CLK_QCOM depends on ARCH_QCOM || COMPILE_TEST select REGMAP_MMIO select RESET_CONTROLLER + select RATIONALMake this an alphabetical list of selects please.
Sure, would take care in the next patch.
Quoted 25 lines
Quoted 25 lines
drivers/clk/qcom/Kconfig +0
-0if COMMON_CLK_QCOMdrivers/clk/qcom/clk-rcg2.c +14
-0@@ -1128,3 +1129,81 @@ int qcom_cc_register_rcg_dfs(struct regmap *regmap,return 0 ; }EXPORT_SYMBOL_GPL (qcom_cc_register_rcg_dfs); + +static int clk_rcg2_dp_set_rate (struct clk_hw *hw,unsigned long rate, +unsigned long parent_rate) +{ +struct clk_rcg2 *rcg =to_clk_rcg2 (hw); +struct freq_tbl f = {0 }; +u32 mask =BIT (rcg->hid_width) -1 ; +u32 hid_div, cfg; +int i, num_parents =clk_hw_get_num_parents (hw); +unsigned long num, den; + +rational_best_approximation (parent_rate, rate, +GENMASK (rcg->mnd_width -1 ,0 ), +GENMASK (rcg->mnd_width -1 ,0 ), &den, &num);
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1148
Quoted 3 lines
drivers/clk/qcom/clk-rcg2.c +3
-0+ +if (!num || !den) { + pr_err("Invalid MN values derived for requested rate %lu\n",Does this ever happen? I worry that this printk could happen many times if a driver gets into a bad state and starts selecting invalid frequencies over and over again for each frame (every 16ms). Maybe just return
-EINVALinstead of printing anything.
Would remove the pr_err.
Quoted 9 lines
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c +9
-0+ rate); +return -EINVAL ; + } + +regmap_read (rcg->clkr.regmap, rcg->cmd_rcgr +CFG_REG , &cfg); + hid_div = cfg; + cfg &=CFG_SRC_SEL_MASK ; + cfg >>=CFG_SRC_SEL_SHIFT ; +
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1162
Quoted 5 lines
drivers/clk/qcom/clk-rcg2.c +5
-0+for (i =0 ; i < num_parents; i++) +if (cfg == rcg->parent_map[i].cfg) { + f.src = rcg->parent_map[i].src; +break ; + }Weird indent for this brace. Please fix and put a brace on the for statement too.
My bad would fix in the next patch.
Quoted 11 lines
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1170
Quoted 8 lines
drivers/clk/qcom/clk-rcg2.c +8
-0+ + f.pre_div = hid_div; + f.pre_div >>=CFG_SRC_DIV_SHIFT ; + f.pre_div &= mask; + +if (num == den) { + f.m =0 ; + f.n =0 ;Isn't this the default? So just have if (num != den) here.
I would check for (num != den).
Quoted 9 lines
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c +9
-0+ }else { + f.m = num; + f.n = den; + } + +return clk_rcg2_configure (rcg, &f); +} + +static int clk_rcg2_dp_set_rate_and_parent(struct clk_hw *hw,
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1183
Quoted 4 lines
drivers/clk/qcom/clk-rcg2.c +4
-0+ unsigned long rate, unsigned long parent_rate, u8 index) +{ + return clk_rcg2_dp_set_rate(hw, rate, parent_rate); +}Does this need to be implemented? The parent index isn't passed to
clk_rcg2_dp_set_rate()so I suspect the parent index doesn't matter? Does the parent change?
I guess it is required as the RCG SRC would be 0x0 by default.
Quoted 11 lines
Quoted 11 lines
drivers/clk/qcom/clk-rcg2.c +11
-0+ +static int clk_rcg2_dp_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ +struct clk_rate_request parent_req = *req; +int ret; + + ret =__clk_determine_rate (clk_hw_get_parent (hw), &parent_req); +if (ret) +return ret; +
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c int qcom_cc_register_rcg_dfs(struct regmap *regmap, · line 1198
Quoted 4 lines
drivers/clk/qcom/clk-rcg2.c +4
-0+ req->best_parent_rate = parent_req.rate; + +return 0 ; +}Do you need this op? It's just calling determine rate on the parent, so we already do that if the proper flag is set. I'm confused about this function.
Would it be good to leave this function :).
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
PATCH v2 2/2 clk: qcom : dispcc: Add support for display port clocks
In reply to Taniya DasSDM845 dispcc supports RCG and CBCRs for display port, so add support for the same.
--- drivers/clk/qcom/dispcc-sdm845.c | 216 ++++++++++++++++++++++++- include/dt-bindings/clock/qcom,dispcc-sdm845.h | 13 +- 2 files changed, 227 insertions(+), 2 deletions(-)
drivers/clk/qcom/dispcc-sdm845.c +215 -1
@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0 /* -* Copyright (c) 2018, The Linux Foundation. All rights reserved. +* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved. */ #include <linux/clk-provider.h>
@@ -29,6 +29,8 @@ enum {P_DSI1_PHY_PLL_OUT_DSICLK ,P_GPLL0_OUT_MAIN ,P_GPLL0_OUT_MAIN_DIV , +P_DP_PHY_PLL_LINK_CLK , + P_DP_PHY_PLL_VCO_DIV_CLK, }; static const struct parent_map disp_cc_parent_map_0[] = {
@@ -45,6 +47,20 @@ enum { "core_bi_pll_test_se", }; +static const struct parent_map disp_cc_parent_map_1[] = { + {P_BI_TCXO ,0 }, + {P_DP_PHY_PLL_LINK_CLK ,1 }, + {P_DP_PHY_PLL_VCO_DIV_CLK ,2 }, + {P_CORE_BI_PLL_TEST_SE ,7 }, +}; + +static const char *const disp_cc_parent_names_1[] = { +"bi_tcxo" , +"dp_link_clk_divsel_ten" , +"dp_vco_divided_clk_src_mux" , +"core_bi_pll_test_se" , +}; +static const struct parent_map disp_cc_parent_map_2[] = { {P_BI_TCXO ,0 }, {P_CORE_BI_PLL_TEST_SE ,7 },
@@ -128,6 +144,82 @@ enum { }, }; +static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux_clk_src[] = { +F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), + { } +}; + +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = { + .cmd_rcgr =0x219c , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_2, + .freq_tbl = ftbl_disp_cc_mdss_dp_aux_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_aux_clk_src" , + .parent_names = disp_cc_parent_names_2, + .num_parents =2 , + .flags =CLK_SET_RATE_PARENT , + .ops = &clk_rcg2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = { + .cmd_rcgr =0x2154 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_crypto_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_GET_RATE_NOCACHE , + .ops = &clk_byte2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = { + .cmd_rcgr =0x2138 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_link_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_byte2_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_dp_pixel1_clk_src = { + .cmd_rcgr =0x2184 , + .mnd_width =16 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_pixel1_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_dp_ops, + }, +}; + +static struct clk_rcg2 disp_cc_mdss_dp_pixel_clk_src = { + .cmd_rcgr =0x216c , + .mnd_width =16 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_pixel_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_dp_ops, + }, +}; +static const struct freq_tbl ftbl_disp_cc_mdss_esc0_clk_src[] = {F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), { }
@@ -391,6 +483,115 @@ enum { }, }; +static struct clk_branch disp_cc_mdss_dp_aux_clk = { + .halt_reg =0x2054 , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x2054 , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_aux_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_aux_clk_src" , + }, + .num_parents =1 , + .flags =CLK_SET_RATE_PARENT , + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch disp_cc_mdss_dp_crypto_clk = { + .halt_reg =0x2048 , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x2048 , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_crypto_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_crypto_clk_src" , + }, + .num_parents =1 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch disp_cc_mdss_dp_link_clk = { + .halt_reg =0x2040 , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x2040 , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_link_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_link_clk_src" , + }, + .num_parents =1 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_branch2_ops, + }, + }, +}; + +/* reset state of disp_cc_mdss_dp_link_div_clk_src divider is 0x3 (div 4) */ +static struct clk_branch disp_cc_mdss_dp_link_intf_clk = { + .halt_reg =0x2044 , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x2044 , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_link_intf_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_link_clk_src" , + }, + .num_parents =1 , + .flags =CLK_GET_RATE_NOCACHE , + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch disp_cc_mdss_dp_pixel1_clk = { + .halt_reg =0x2050 , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x2050 , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_pixel1_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_pixel1_clk_src" , + }, + .num_parents =1 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_branch2_ops, + }, + }, +}; + +static struct clk_branch disp_cc_mdss_dp_pixel_clk = { + .halt_reg =0x204c , + .halt_check =BRANCH_HALT , + .clkr = { + .enable_reg =0x204c , + .enable_mask =BIT (0 ), + .hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_pixel_clk" , + .parent_names = (const char *[]){ +"disp_cc_mdss_dp_pixel_clk_src" , + }, + .num_parents =1 , + .flags =CLK_SET_RATE_PARENT |CLK_GET_RATE_NOCACHE , + .ops = &clk_branch2_ops, + }, + }, +}; +static struct clk_branch disp_cc_mdss_esc0_clk = { .halt_reg =0x2038 , .halt_check =BRANCH_HALT ,
@@ -589,6 +790,19 @@ enum { [DISP_CC_MDSS_BYTE1_INTF_CLK] = &disp_cc_mdss_byte1_intf_clk.clkr, [DISP_CC_MDSS_BYTE1_DIV_CLK_SRC ] = &disp_cc_mdss_byte1_div_clk_src.clkr, + [DISP_CC_MDSS_DP_AUX_CLK] = &disp_cc_mdss_dp_aux_clk.clkr, + [DISP_CC_MDSS_DP_AUX_CLK_SRC] = &disp_cc_mdss_dp_aux_clk_src.clkr, + [DISP_CC_MDSS_DP_CRYPTO_CLK] = &disp_cc_mdss_dp_crypto_clk.clkr, + [DISP_CC_MDSS_DP_CRYPTO_CLK_SRC ] = + &disp_cc_mdss_dp_crypto_clk_src.clkr, + [DISP_CC_MDSS_DP_LINK_CLK] = &disp_cc_mdss_dp_link_clk.clkr, + [DISP_CC_MDSS_DP_LINK_CLK_SRC] = &disp_cc_mdss_dp_link_clk_src.clkr, + [DISP_CC_MDSS_DP_LINK_INTF_CLK] = &disp_cc_mdss_dp_link_intf_clk.clkr, + [DISP_CC_MDSS_DP_PIXEL1_CLK] = &disp_cc_mdss_dp_pixel1_clk.clkr, + [DISP_CC_MDSS_DP_PIXEL1_CLK_SRC ] = + &disp_cc_mdss_dp_pixel1_clk_src.clkr, + [DISP_CC_MDSS_DP_PIXEL_CLK] = &disp_cc_mdss_dp_pixel_clk.clkr, + [DISP_CC_MDSS_DP_PIXEL_CLK_SRC] = &disp_cc_mdss_dp_pixel_clk_src.clkr, [DISP_CC_MDSS_ESC0_CLK] = &disp_cc_mdss_esc0_clk.clkr, [DISP_CC_MDSS_ESC0_CLK_SRC] = &disp_cc_mdss_esc0_clk_src.clkr, [DISP_CC_MDSS_ESC1_CLK] = &disp_cc_mdss_esc1_clk.clkr,
include/dt-bindings/clock/qcom,dispcc-sdm845.h +12 -1
@@ -1,6 +1,6 @@/* SPDX-License-Identifier: GPL-2.0 */ /* -* Copyright (c) 2018, The Linux Foundation. All rights reserved. +* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved. */ #ifndef _DT_BINDINGS_CLK_SDM_DISP_CC_SDM845_H
@@ -35,6 +35,17 @@#define DISP_CC_PLL0 25#define DISP_CC_MDSS_BYTE0_DIV_CLK_SRC 26#define DISP_CC_MDSS_BYTE1_DIV_CLK_SRC 27 +#define DISP_CC_MDSS_DP_AUX_CLK 28 +#define DISP_CC_MDSS_DP_AUX_CLK_SRC 29 +#define DISP_CC_MDSS_DP_CRYPTO_CLK 30 +#define DISP_CC_MDSS_DP_CRYPTO_CLK_SRC 31 +#define DISP_CC_MDSS_DP_LINK_CLK 32 +#define DISP_CC_MDSS_DP_LINK_CLK_SRC 33 +#define DISP_CC_MDSS_DP_LINK_INTF_CLK 34 +#define DISP_CC_MDSS_DP_PIXEL1_CLK 35 +#define DISP_CC_MDSS_DP_PIXEL1_CLK_SRC 36 +#define DISP_CC_MDSS_DP_PIXEL_CLK 37 +#define DISP_CC_MDSS_DP_PIXEL_CLK_SRC 38/* DISP_CC Reset */ #define DISP_CC_MDSS_RSCC_BCR 0
--
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 v2 2/2] clk: qcom : dispcc: Add support for display port clocks
Quoting Taniya Das (2019-05-14 21:20:39)
Quoted 24 lines
drivers/clk/qcom/dispcc-sdm845.c +20
-0@@ -128,6 +144,82 @@ enum { }, }; +static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux_clk_src[] = { +F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), + { } +}; + +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = { + .cmd_rcgr =0x219c , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_2, + .freq_tbl = ftbl_disp_cc_mdss_dp_aux_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_aux_clk_src" , + .parent_names = disp_cc_parent_names_2, + .num_parents =2 , + .flags =CLK_SET_RATE_PARENT , + .ops = &clk_rcg2_ops, + }, +}; +
drivers/clk/qcom/dispcc-sdm845.c enum { · line 176
Quoted 10 lines
drivers/clk/qcom/dispcc-sdm845.c +10
-0+static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = { + .cmd_rcgr =0x2154 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_crypto_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_GET_RATE_NOCACHE ,
Why do we need this flag on various clks here? I'd prefer this is removed. If it can't be removed, we need to describe in a code comment why this must be set.
If it's some sort of problem where the upstream PLL goes into bypass across a reset, then we probably need to change the display code to restore that rate across a reset by calling clk_set_rate() on the PLL directly. And we might need to think about how to inform the framework that this has happened, so that downstream clks can be notified of the change in frequency.
Re: [PATCH v2 2/2] clk: qcom : dispcc: Add support for display port clocks
Hi Stephen,
Thanks for your comments.
Quoted 40 lines · Stephen Boyd
Quoting Taniya Das (2019-05-14 21:20:39)
Quoted 24 lines
drivers/clk/qcom/dispcc-sdm845.c +20
-0@@ -128,6 +144,82 @@ enum { }, }; +static const struct freq_tbl ftbl_disp_cc_mdss_dp_aux_clk_src[] = { +F (19200000 ,P_BI_TCXO ,1 ,0 ,0 ), + { } +}; + +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = { + .cmd_rcgr =0x219c , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_2, + .freq_tbl = ftbl_disp_cc_mdss_dp_aux_clk_src, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_aux_clk_src" , + .parent_names = disp_cc_parent_names_2, + .num_parents =2 , + .flags =CLK_SET_RATE_PARENT , + .ops = &clk_rcg2_ops, + }, +}; +drivers/clk/qcom/dispcc-sdm845.c enum { · line 176
Quoted 10 lines
drivers/clk/qcom/dispcc-sdm845.c +10
-0+static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = { + .cmd_rcgr =0x2154 , + .mnd_width =0 , + .hid_width =5 , + .parent_map = disp_cc_parent_map_1, + .clkr.hw.init = &(struct clk_init_data ){ + .name ="disp_cc_mdss_dp_crypto_clk_src" , + .parent_names = disp_cc_parent_names_1, + .num_parents =4 , + .flags =CLK_GET_RATE_NOCACHE ,Why do we need this flag on various clks here? I'd prefer this is removed. If it can't be removed, we need to describe in a code comment why this must be set.
Quoted 7 lines
If it's some sort of problem where the upstream PLL goes into bypass across a reset, then we probably need to change the display code to restore that rate across a reset by calling
clk_set_rate()on the PLL directly. And we might need to think about how to inform the framework that this has happened, so that downstream clks can be notified of the change in frequency.
I had another discussion with the team and they are okay to remove these flags, so would submit the next patch with the flag removed.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --