On several Qualcomm platforms DisplayPort link clocks used incorrect
frequency tables. Drop frequency tables and use clk_byte2_ops instead of
clk_rcg2_ops.
Note, this was tested on SM8450 only and then extended to other
platforms.
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
Dmitry Baryshkov (4):
clk: qcom: dispcc-sm8450: fix DisplayPort clocks
clk: qcom: dispcc-sm6350: fix DisplayPort clocks
clk: qcom: dispcc-sm8550: fix DisplayPort clocks
clk: qcom: dispcc-sm8650: fix DisplayPort clocks
drivers/clk/qcom/dispcc-sm6350.c | 11 +----------
drivers/clk/qcom/dispcc-sm8450.c | 20 ++++----------------
drivers/clk/qcom/dispcc-sm8550.c | 20 ++++----------------
drivers/clk/qcom/dispcc-sm8650.c | 20 ++++----------------
4 files changed, 13 insertions(+), 58 deletions(-)
---
base-commit: 8568bb2ccc278f344e6ac44af6ed010a90aa88dc
change-id: 20240408-dispcc-dp-clocks-5ee5d5926346
Best regards,
--
Dmitry Baryshkov <[email protected]>
On SM8450 DisplayPort link clocks use frequency tables inherited from
the vendor kernel, it is not applicable in the upstream kernel. Drop
frequency tables and use clk_byte2_ops for those clocks.
Fixes: 16fb89f92ec4 ("clk: qcom: Add support for Display Clock Controller on SM8450")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/clk/qcom/dispcc-sm8450.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
index 92e9c4e7b13d..49bb4f58c391 100644
--- a/drivers/clk/qcom/dispcc-sm8450.c
+++ b/drivers/clk/qcom/dispcc-sm8450.c
@@ -309,26 +309,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
},
};
-static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
- F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- { }
-};
-
static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
.cmd_rcgr = 0x819c,
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx0_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -382,13 +373,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx1_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -442,13 +432,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx2_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -502,13 +491,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx3_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
--
2.39.2
On SM6350 DisplayPort link clocks use frequency tables inherited from
the vendor kernel, it is not applicable in the upstream kernel. Drop
frequency tables and use clk_byte2_ops for those clocks.
Fixes: 837519775f1d ("clk: qcom: Add display clock controller driver for SM6350")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/clk/qcom/dispcc-sm6350.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm6350.c b/drivers/clk/qcom/dispcc-sm6350.c
index 839435362010..e4b7464c4d0e 100644
--- a/drivers/clk/qcom/dispcc-sm6350.c
+++ b/drivers/clk/qcom/dispcc-sm6350.c
@@ -221,26 +221,17 @@ static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
},
};
-static const struct freq_tbl ftbl_disp_cc_mdss_dp_link_clk_src[] = {
- F(162000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(270000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(540000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(810000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
- { }
-};
-
static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
.cmd_rcgr = 0x10f8,
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_0,
- .freq_tbl = ftbl_disp_cc_mdss_dp_link_clk_src,
.clkr.hw.init = &(struct clk_init_data){
.name = "disp_cc_mdss_dp_link_clk_src",
.parent_data = disp_cc_parent_data_0,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
--
2.39.2
On SM8550 DisplayPort link clocks use frequency tables inherited from
the vendor kernel, it is not applicable in the upstream kernel. Drop
frequency tables and use clk_byte2_ops for those clocks.
Fixes: 90114ca11476 ("clk: qcom: add SM8550 DISPCC driver")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/clk/qcom/dispcc-sm8550.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm8550.c b/drivers/clk/qcom/dispcc-sm8550.c
index 3672c73ac11c..38ecea805503 100644
--- a/drivers/clk/qcom/dispcc-sm8550.c
+++ b/drivers/clk/qcom/dispcc-sm8550.c
@@ -345,26 +345,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
},
};
-static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
- F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- { }
-};
-
static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
.cmd_rcgr = 0x8170,
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_7,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx0_link_clk_src",
.parent_data = disp_cc_parent_data_7,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_7),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -418,13 +409,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx1_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -478,13 +468,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx2_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -538,13 +527,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(struct clk_init_data) {
.name = "disp_cc_mdss_dptx3_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
--
2.39.2
On SM8650 DisplayPort link clocks use frequency tables inherited from
the vendor kernel, it is not applicable in the upstream kernel. Drop
frequency tables and use clk_byte2_ops for those clocks.
Fixes: 9e939f008338 ("clk: qcom: add the SM8650 Display Clock Controller driver")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/clk/qcom/dispcc-sm8650.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm8650.c b/drivers/clk/qcom/dispcc-sm8650.c
index 9539db0d9114..3eb64bcad487 100644
--- a/drivers/clk/qcom/dispcc-sm8650.c
+++ b/drivers/clk/qcom/dispcc-sm8650.c
@@ -343,26 +343,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
},
};
-static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
- F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
- { }
-};
-
static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
.cmd_rcgr = 0x8170,
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_7,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(const struct clk_init_data) {
.name = "disp_cc_mdss_dptx0_link_clk_src",
.parent_data = disp_cc_parent_data_7,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_7),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -416,13 +407,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(const struct clk_init_data) {
.name = "disp_cc_mdss_dptx1_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -476,13 +466,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(const struct clk_init_data) {
.name = "disp_cc_mdss_dptx2_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
@@ -536,13 +525,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
.mnd_width = 0,
.hid_width = 5,
.parent_map = disp_cc_parent_map_3,
- .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
.clkr.hw.init = &(const struct clk_init_data) {
.name = "disp_cc_mdss_dptx3_link_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_ops,
+ .ops = &clk_byte2_ops,
},
};
--
2.39.2
Quoting Dmitry Baryshkov (2024-04-08 04:47:04)
> On SM8450 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
The subject says "fix", but does this fix anything, or simply optimize?
Is something broken by having the frequency table?
On 08/04/2024 13:47, Dmitry Baryshkov wrote:
> On SM8550 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
>
> Fixes: 90114ca11476 ("clk: qcom: add SM8550 DISPCC driver")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/clk/qcom/dispcc-sm8550.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm8550.c b/drivers/clk/qcom/dispcc-sm8550.c
> index 3672c73ac11c..38ecea805503 100644
> --- a/drivers/clk/qcom/dispcc-sm8550.c
> +++ b/drivers/clk/qcom/dispcc-sm8550.c
> @@ -345,26 +345,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
> },
> };
>
> -static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
> - F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - { }
> -};
> -
> static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
> .cmd_rcgr = 0x8170,
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_7,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx0_link_clk_src",
> .parent_data = disp_cc_parent_data_7,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_7),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -418,13 +409,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx1_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -478,13 +468,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx2_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -538,13 +527,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx3_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
>
Reviewed-by: Neil Armstrong <[email protected]>
Tested-by: Neil Armstrong <[email protected]> # on SM8550-HDK
Fixes the:
[ 25.428008] msm-dp-display ae90000.displayport-controller: _opp_config_clk_single: failed to set clock rate: -22
Thanks !
Neil
On 08/04/2024 13:47, Dmitry Baryshkov wrote:
> On SM8650 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
>
> Fixes: 9e939f008338 ("clk: qcom: add the SM8650 Display Clock Controller driver")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/clk/qcom/dispcc-sm8650.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm8650.c b/drivers/clk/qcom/dispcc-sm8650.c
> index 9539db0d9114..3eb64bcad487 100644
> --- a/drivers/clk/qcom/dispcc-sm8650.c
> +++ b/drivers/clk/qcom/dispcc-sm8650.c
> @@ -343,26 +343,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
> },
> };
>
> -static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
> - F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - { }
> -};
> -
> static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
> .cmd_rcgr = 0x8170,
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_7,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(const struct clk_init_data) {
> .name = "disp_cc_mdss_dptx0_link_clk_src",
> .parent_data = disp_cc_parent_data_7,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_7),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -416,13 +407,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(const struct clk_init_data) {
> .name = "disp_cc_mdss_dptx1_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -476,13 +466,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(const struct clk_init_data) {
> .name = "disp_cc_mdss_dptx2_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -536,13 +525,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(const struct clk_init_data) {
> .name = "disp_cc_mdss_dptx3_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
>
Reviewed-by: Neil Armstrong <[email protected]>
Tested-by: Neil Armstrong <[email protected]> # on SM8650-HDK
On 08/04/2024 13:47, Dmitry Baryshkov wrote:
> On SM6350 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
>
> Fixes: 837519775f1d ("clk: qcom: Add display clock controller driver for SM6350")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/clk/qcom/dispcc-sm6350.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm6350.c b/drivers/clk/qcom/dispcc-sm6350.c
> index 839435362010..e4b7464c4d0e 100644
> --- a/drivers/clk/qcom/dispcc-sm6350.c
> +++ b/drivers/clk/qcom/dispcc-sm6350.c
> @@ -221,26 +221,17 @@ static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
> },
> };
>
> -static const struct freq_tbl ftbl_disp_cc_mdss_dp_link_clk_src[] = {
> - F(162000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(270000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(540000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(810000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - { }
> -};
> -
> static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
> .cmd_rcgr = 0x10f8,
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_0,
> - .freq_tbl = ftbl_disp_cc_mdss_dp_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data){
> .name = "disp_cc_mdss_dp_link_clk_src",
> .parent_data = disp_cc_parent_data_0,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
>
Reviewed-by: Neil Armstrong <[email protected]>
On 08/04/2024 13:47, Dmitry Baryshkov wrote:
> On SM8450 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
>
> Fixes: 16fb89f92ec4 ("clk: qcom: Add support for Display Clock Controller on SM8450")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/clk/qcom/dispcc-sm8450.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
> index 92e9c4e7b13d..49bb4f58c391 100644
> --- a/drivers/clk/qcom/dispcc-sm8450.c
> +++ b/drivers/clk/qcom/dispcc-sm8450.c
> @@ -309,26 +309,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
> },
> };
>
> -static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
> - F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> - { }
> -};
> -
> static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
> .cmd_rcgr = 0x819c,
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx0_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -382,13 +373,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx1_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -442,13 +432,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx2_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
> @@ -502,13 +491,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_3,
> - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data) {
> .name = "disp_cc_mdss_dptx3_link_clk_src",
> .parent_data = disp_cc_parent_data_3,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> .flags = CLK_SET_RATE_PARENT,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
>
Reviewed-by: Neil Armstrong <[email protected]>
I can't test, but I assume you tested on your HDK8450
Thanks,
Neil
On Wed, 10 Apr 2024 at 19:27, Neil Armstrong <[email protected]> wrote:
>
> On 08/04/2024 13:47, Dmitry Baryshkov wrote:
> > On SM8450 DisplayPort link clocks use frequency tables inherited from
> > the vendor kernel, it is not applicable in the upstream kernel. Drop
> > frequency tables and use clk_byte2_ops for those clocks.
> >
> > Fixes: 16fb89f92ec4 ("clk: qcom: Add support for Display Clock Controller on SM8450")
> > Signed-off-by: Dmitry Baryshkov <[email protected]>
> > ---
> > drivers/clk/qcom/dispcc-sm8450.c | 20 ++++----------------
> > 1 file changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
> > index 92e9c4e7b13d..49bb4f58c391 100644
> > --- a/drivers/clk/qcom/dispcc-sm8450.c
> > +++ b/drivers/clk/qcom/dispcc-sm8450.c
> > @@ -309,26 +309,17 @@ static struct clk_rcg2 disp_cc_mdss_dptx0_aux_clk_src = {
> > },
> > };
> >
> > -static const struct freq_tbl ftbl_disp_cc_mdss_dptx0_link_clk_src[] = {
> > - F(162000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> > - F(270000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> > - F(540000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> > - F(810000, P_DP0_PHY_PLL_LINK_CLK, 1, 0, 0),
> > - { }
> > -};
> > -
> > static struct clk_rcg2 disp_cc_mdss_dptx0_link_clk_src = {
> > .cmd_rcgr = 0x819c,
> > .mnd_width = 0,
> > .hid_width = 5,
> > .parent_map = disp_cc_parent_map_3,
> > - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> > .clkr.hw.init = &(struct clk_init_data) {
> > .name = "disp_cc_mdss_dptx0_link_clk_src",
> > .parent_data = disp_cc_parent_data_3,
> > .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> > .flags = CLK_SET_RATE_PARENT,
> > - .ops = &clk_rcg2_ops,
> > + .ops = &clk_byte2_ops,
> > },
> > };
> >
> > @@ -382,13 +373,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx1_link_clk_src = {
> > .mnd_width = 0,
> > .hid_width = 5,
> > .parent_map = disp_cc_parent_map_3,
> > - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> > .clkr.hw.init = &(struct clk_init_data) {
> > .name = "disp_cc_mdss_dptx1_link_clk_src",
> > .parent_data = disp_cc_parent_data_3,
> > .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> > .flags = CLK_SET_RATE_PARENT,
> > - .ops = &clk_rcg2_ops,
> > + .ops = &clk_byte2_ops,
> > },
> > };
> >
> > @@ -442,13 +432,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx2_link_clk_src = {
> > .mnd_width = 0,
> > .hid_width = 5,
> > .parent_map = disp_cc_parent_map_3,
> > - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> > .clkr.hw.init = &(struct clk_init_data) {
> > .name = "disp_cc_mdss_dptx2_link_clk_src",
> > .parent_data = disp_cc_parent_data_3,
> > .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> > .flags = CLK_SET_RATE_PARENT,
> > - .ops = &clk_rcg2_ops,
> > + .ops = &clk_byte2_ops,
> > },
> > };
> >
> > @@ -502,13 +491,12 @@ static struct clk_rcg2 disp_cc_mdss_dptx3_link_clk_src = {
> > .mnd_width = 0,
> > .hid_width = 5,
> > .parent_map = disp_cc_parent_map_3,
> > - .freq_tbl = ftbl_disp_cc_mdss_dptx0_link_clk_src,
> > .clkr.hw.init = &(struct clk_init_data) {
> > .name = "disp_cc_mdss_dptx3_link_clk_src",
> > .parent_data = disp_cc_parent_data_3,
> > .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> > .flags = CLK_SET_RATE_PARENT,
> > - .ops = &clk_rcg2_ops,
> > + .ops = &clk_byte2_ops,
> > },
> > };
> >
> >
>
>
> Reviewed-by: Neil Armstrong <[email protected]>
>
> I can't test, but I assume you tested on your HDK8450
That's how I stumbled upon it. I was not able to test other patches in
the series, but granted the similarity I assume that they should work
in the same way.
--
With best wishes
Dmitry
On 4/8/24 13:47, Dmitry Baryshkov wrote:
> On several Qualcomm platforms DisplayPort link clocks used incorrect
> frequency tables. Drop frequency tables and use clk_byte2_ops instead of
> clk_rcg2_ops.
>
> Note, this was tested on SM8450 only and then extended to other
> platforms.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On Mon Apr 8, 2024 at 1:47 PM CEST, Dmitry Baryshkov wrote:
> On SM6350 DisplayPort link clocks use frequency tables inherited from
> the vendor kernel, it is not applicable in the upstream kernel. Drop
> frequency tables and use clk_byte2_ops for those clocks.
>
> Fixes: 837519775f1d ("clk: qcom: Add display clock controller driver for SM6350")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
Appears to fix this non-critical error when enabling DisplayPort.
msm-dp-display ae90000.displayport-controller: _opp_config_clk_single: failed to set clock rate: -22
And DisplayPort (over USB-C) continues to work as expected, thanks!
Tested-by: Luca Weiss <[email protected]>
For completeness, I wrote something about this also on #linux-msm IRC on
March 22nd.
> Hi, I'm trying to get displayport to work on sm6350 but hitting a
> weird issue regarding link clk frequency. For the requested link
> rate=540000 in dp_ctrl_enable_mainlink_clocks we call
> dev_pm_opp_set_rate with target_freq=540000000 (clk name:
> disp_cc_mdss_dp_link_clk) but the clk_round_rate there makes this into
> freq=810000 and subsequently qmp_dp_link_clk_determine_rate fails
> because that's not a valid frequency, only for example 810000000.
> Without any debug statements the visible error in kernel log is:
> "msm-dp-display ae90000.displayport-controller:
> _opp_config_clk_single: failed to set clock rate: -22"
>
> So somewhere there seems to be confusion between how many zeroes
> should be where.. But not sure how this is working on other SoCs, I
> don't see anything much different for my SoC
>
> Kernel base is 6.8.1 fwiw
>
> clk_round_rate behavior feels correct as
> ftbl_disp_cc_mdss_dp_link_clk_src lists the frequencies as
> 162000/270000/540000/810000 so it rounds it to the highest available
> frequency of the clock
Regards
Luca
> ---
> drivers/clk/qcom/dispcc-sm6350.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm6350.c b/drivers/clk/qcom/dispcc-sm6350.c
> index 839435362010..e4b7464c4d0e 100644
> --- a/drivers/clk/qcom/dispcc-sm6350.c
> +++ b/drivers/clk/qcom/dispcc-sm6350.c
> @@ -221,26 +221,17 @@ static struct clk_rcg2 disp_cc_mdss_dp_crypto_clk_src = {
> },
> };
>
> -static const struct freq_tbl ftbl_disp_cc_mdss_dp_link_clk_src[] = {
> - F(162000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(270000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(540000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - F(810000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> - { }
> -};
> -
> static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
> .cmd_rcgr = 0x10f8,
> .mnd_width = 0,
> .hid_width = 5,
> .parent_map = disp_cc_parent_map_0,
> - .freq_tbl = ftbl_disp_cc_mdss_dp_link_clk_src,
> .clkr.hw.init = &(struct clk_init_data){
> .name = "disp_cc_mdss_dp_link_clk_src",
> .parent_data = disp_cc_parent_data_0,
> .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_byte2_ops,
> },
> };
>
On Mon, Apr 08, 2024 at 02:47:03PM +0300, Dmitry Baryshkov wrote:
> On several Qualcomm platforms DisplayPort link clocks used incorrect
> frequency tables. Drop frequency tables and use clk_byte2_ops instead of
> clk_rcg2_ops.
>
> Note, this was tested on SM8450 only and then extended to other
> platforms.
>
As Stephen points out, it seems from the commit messages that this just
cleans up the code because it's wrong. But both Luca and Neil points out
that it resolves a visible issue/error.
Can we please have this (the actual problem being resolved) captured in
the commit messages?
Regards,
Bjorn
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> Dmitry Baryshkov (4):
> clk: qcom: dispcc-sm8450: fix DisplayPort clocks
> clk: qcom: dispcc-sm6350: fix DisplayPort clocks
> clk: qcom: dispcc-sm8550: fix DisplayPort clocks
> clk: qcom: dispcc-sm8650: fix DisplayPort clocks
>
> drivers/clk/qcom/dispcc-sm6350.c | 11 +----------
> drivers/clk/qcom/dispcc-sm8450.c | 20 ++++----------------
> drivers/clk/qcom/dispcc-sm8550.c | 20 ++++----------------
> drivers/clk/qcom/dispcc-sm8650.c | 20 ++++----------------
> 4 files changed, 13 insertions(+), 58 deletions(-)
> ---
> base-commit: 8568bb2ccc278f344e6ac44af6ed010a90aa88dc
> change-id: 20240408-dispcc-dp-clocks-5ee5d5926346
>
> Best regards,
> --
> Dmitry Baryshkov <[email protected]>
>