2024-04-08 11:47:24

by Dmitry Baryshkov

Subject: [PATCH 0/4] clk: qcom: dispcc: fix DisplayPort link clocks

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]>

2024-04-08 11:47:28

by Dmitry Baryshkov

Subject: [PATCH 1/4] clk: qcom: dispcc-sm8450: fix DisplayPort clocks

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

2024-04-08 11:47:37

by Dmitry Baryshkov

Subject: [PATCH 2/4] clk: qcom: dispcc-sm6350: fix DisplayPort clocks

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

2024-04-08 11:47:52

by Dmitry Baryshkov

Subject: [PATCH 3/4] clk: qcom: dispcc-sm8550: fix DisplayPort clocks

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

2024-04-08 11:48:00

by Dmitry Baryshkov

Subject: [PATCH 4/4] clk: qcom: dispcc-sm8650: fix DisplayPort clocks

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

2024-04-09 02:35:05

by Stephen Boyd

Subject: Re: [PATCH 1/4] clk: qcom: dispcc-sm8450: fix DisplayPort clocks

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?

2024-04-10 16:27:12

by Neil Armstrong

Subject: Re: [PATCH 3/4] clk: qcom: dispcc-sm8550: fix DisplayPort clocks

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

2024-04-10 16:32:00

by Neil Armstrong

Subject: Re: [PATCH 4/4] clk: qcom: dispcc-sm8650: fix DisplayPort clocks

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

2024-04-10 16:33:29

by Neil Armstrong

Subject: Re: [PATCH 2/4] clk: qcom: dispcc-sm6350: fix DisplayPort clocks

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]>

2024-04-10 17:12:00

by Neil Armstrong

Subject: Re: [PATCH 1/4] clk: qcom: dispcc-sm8450: fix DisplayPort clocks

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

2024-04-10 17:12:09

by Dmitry Baryshkov

Subject: Re: [PATCH 1/4] clk: qcom: dispcc-sm8450: fix DisplayPort clocks

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

2024-04-10 19:16:46

by Konrad Dybcio

Subject: Re: [PATCH 0/4] clk: qcom: dispcc: fix DisplayPort link clocks

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

2024-04-11 11:46:19

by Luca Weiss

Subject: Re: [PATCH 2/4] clk: qcom: dispcc-sm6350: fix DisplayPort clocks

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,
> },
> };
>

2024-04-20 18:05:22

by Bjorn Andersson

Subject: Re: [PATCH 0/4] clk: qcom: dispcc: fix DisplayPort link clocks

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]>
>