Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
Quoting Jonathan Marek (2020-09-23 17:54:59)
> On 9/23/20 7:30 PM, Stephen Boyd wrote:
> > Quoting Jonathan Marek (2020-09-23 09:07:16)
> >> On 9/22/20 2:46 PM, Stephen Boyd wrote:
> >>> Quoting Jonathan Marek (2020-09-03 20:09:54)
> >>>
> + .ops = _branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch video_cc_mvs0_clk = {
> + .halt_reg = 0xd34,
> + .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */
> >>>
> >>> Is this resolved?
> >>>
> >>
> >> Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream
> >> venus driver (with patches to enable sm8250), that results in a
> >> "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus
> >> enables/disables this clock on its own (venus still works without
> >> touching this clock), but I didn't want to remove this in case it might
> >> be needed. I removed these clocks in the v3 I just sent.
> >>
> >
> > Hmm. Does downstream use these clks? There have been some clk stuck
> > problems with venus recently that were attributed to improperly enabling
> > clks before enabling interconnects and power domains. Maybe it's the
> > same problem.
> >
>
> Yes, downstream uses these clks.
>
> The "stuck" problem still happens if GSDCS/interconnects are always on,
> and like I mentioned, venus works even with these clocks completely
> removed.
>
> I think venus controls these clocks (and downstream just happens to try
> enabling it at a point where venus has already enabled it?). I'm not too
> sure about this, it might have something to do with the GDSC having the
> HW_CTRL flag too..
Ok. Maybe Taniya has an idea.
Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
On 9/23/20 7:30 PM, Stephen Boyd wrote:
Quoting Jonathan Marek (2020-09-23 09:07:16)
On 9/22/20 2:46 PM, Stephen Boyd wrote:
Quoting Jonathan Marek (2020-09-03 20:09:54)
+ .ops = _branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+ .halt_reg = 0xd34,
+ .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */
Is this resolved?
Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream
venus driver (with patches to enable sm8250), that results in a
"video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus
enables/disables this clock on its own (venus still works without
touching this clock), but I didn't want to remove this in case it might
be needed. I removed these clocks in the v3 I just sent.
Hmm. Does downstream use these clks? There have been some clk stuck
problems with venus recently that were attributed to improperly enabling
clks before enabling interconnects and power domains. Maybe it's the
same problem.
Yes, downstream uses these clks.
The "stuck" problem still happens if GSDCS/interconnects are always on,
and like I mentioned, venus works even with these clocks completely
removed.
I think venus controls these clocks (and downstream just happens to try
enabling it at a point where venus has already enabled it?). I'm not too
sure about this, it might have something to do with the GDSC having the
HW_CTRL flag too..
Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
Quoting Jonathan Marek (2020-09-23 09:07:16)
> On 9/22/20 2:46 PM, Stephen Boyd wrote:
> > Quoting Jonathan Marek (2020-09-03 20:09:54)
> >
> >> + .ops = _branch2_ops,
> >> + },
> >> + },
> >> +};
> >> +
> >> +static struct clk_branch video_cc_mvs0_clk = {
> >> + .halt_reg = 0xd34,
> >> + .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */
> >
> > Is this resolved?
> >
>
> Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream
> venus driver (with patches to enable sm8250), that results in a
> "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus
> enables/disables this clock on its own (venus still works without
> touching this clock), but I didn't want to remove this in case it might
> be needed. I removed these clocks in the v3 I just sent.
>
Hmm. Does downstream use these clks? There have been some clk stuck
problems with venus recently that were attributed to improperly enabling
clks before enabling interconnects and power domains. Maybe it's the
same problem.
Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
On 9/22/20 2:46 PM, Stephen Boyd wrote:
Quoting Jonathan Marek (2020-09-03 20:09:54)
Add support for the video clock controller found on SM8250 based devices.
Derived from the downstream driver.
Signed-off-by: Jonathan Marek
---
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/videocc-sm8250.c | 518 ++
3 files changed, 528 insertions(+)
create mode 100644 drivers/clk/qcom/videocc-sm8250.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 40d7ee9886c9..95efa38211d5 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -453,6 +453,15 @@ config SM_VIDEOCC_8150
Say Y if you want to support video devices and functionality such as
video encode and decode.
+config SM_VIDEOCC_8250
+ tristate "SM8250 Video Clock Controller"
+ select SDM_GCC_8250
+ select QCOM_GDSC
+ help
+ Support for the video clock controller on SM8250 devices.
+ Say Y if you want to support video devices and functionality such as
+ video encode and decode.
+
config SPMI_PMIC_CLKDIV
tristate "SPMI PMIC clkdiv Support"
depends on SPMI || COMPILE_TEST
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 6f4c580d2728..55fb20800b66 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
+obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
diff --git a/drivers/clk/qcom/videocc-sm8250.c
b/drivers/clk/qcom/videocc-sm8250.c
new file mode 100644
index ..a814d10945c4
--- /dev/null
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
[...]
+static struct clk_rcg2 video_cc_ahb_clk_src = {
+ .cmd_rcgr = 0xbd4,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map = video_cc_parent_map_0,
+ .freq_tbl = ftbl_video_cc_ahb_clk_src,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "video_cc_ahb_clk_src",
+ .parent_data = video_cc_parent_data_0,
+ .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = _rcg2_shared_ops,
+ },
+};
+
+static struct clk_rcg2 video_cc_xo_clk_src = {
+ .cmd_rcgr = 0xecc,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map = video_cc_parent_map_0,
+ .freq_tbl = ftbl_video_cc_ahb_clk_src,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "video_cc_xo_clk_src",
+ .parent_data = video_cc_parent_data_0,
+ .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
+ .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
Similar critical clk comment, see below.
+ .ops = _rcg2_ops,
+ },
+};
+
+static struct clk_branch video_cc_ahb_clk = {
+ .halt_reg = 0xe58,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0xe58,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "video_cc_ahb_clk",
+ .parent_data = &(const struct clk_parent_data){
+ .hw = _cc_ahb_clk_src.clkr.hw,
+ },
+ .num_parents = 1,
+ .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
Similar critical clk comment, see below.
+ .ops = _branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+ .halt_reg = 0xd34,
+ .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */
Is this resolved?
Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream
venus driver (with patches to enable sm8250), that results in a
"video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus
enables/disables this clock on its own (venus still works without
touching this clock), but I didn't want to remove this in case it might
be needed. I removed these clocks in the v3 I just sent.
+ .clkr = {
+ .enable_reg = 0xd34,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "video_cc_mvs0_clk",
+ .parent_data = &(const struct clk_parent_data){
+ .hw = _cc_mvs0_div_clk_src.clkr.hw,
+ },
+
Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
Quoting Jonathan Marek (2020-09-03 20:09:54)
> Add support for the video clock controller found on SM8250 based devices.
>
> Derived from the downstream driver.
>
> Signed-off-by: Jonathan Marek
> ---
> drivers/clk/qcom/Kconfig | 9 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/videocc-sm8250.c | 518 ++
> 3 files changed, 528 insertions(+)
> create mode 100644 drivers/clk/qcom/videocc-sm8250.c
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 40d7ee9886c9..95efa38211d5 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -453,6 +453,15 @@ config SM_VIDEOCC_8150
> Say Y if you want to support video devices and functionality such as
> video encode and decode.
>
> +config SM_VIDEOCC_8250
> + tristate "SM8250 Video Clock Controller"
> + select SDM_GCC_8250
> + select QCOM_GDSC
> + help
> + Support for the video clock controller on SM8250 devices.
> + Say Y if you want to support video devices and functionality such as
> + video encode and decode.
> +
> config SPMI_PMIC_CLKDIV
> tristate "SPMI PMIC clkdiv Support"
> depends on SPMI || COMPILE_TEST
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 6f4c580d2728..55fb20800b66 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
> obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
> obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
> obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
> +obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
> obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
> obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
> obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
> diff --git a/drivers/clk/qcom/videocc-sm8250.c
> b/drivers/clk/qcom/videocc-sm8250.c
> new file mode 100644
> index ..a814d10945c4
> --- /dev/null
> +++ b/drivers/clk/qcom/videocc-sm8250.c
> @@ -0,0 +1,518 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
[...]
> +static struct clk_rcg2 video_cc_ahb_clk_src = {
> + .cmd_rcgr = 0xbd4,
> + .mnd_width = 0,
> + .hid_width = 5,
> + .parent_map = video_cc_parent_map_0,
> + .freq_tbl = ftbl_video_cc_ahb_clk_src,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "video_cc_ahb_clk_src",
> + .parent_data = video_cc_parent_data_0,
> + .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = _rcg2_shared_ops,
> + },
> +};
> +
> +static struct clk_rcg2 video_cc_xo_clk_src = {
> + .cmd_rcgr = 0xecc,
> + .mnd_width = 0,
> + .hid_width = 5,
> + .parent_map = video_cc_parent_map_0,
> + .freq_tbl = ftbl_video_cc_ahb_clk_src,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "video_cc_xo_clk_src",
> + .parent_data = video_cc_parent_data_0,
> + .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
Similar critical clk comment, see below.
> + .ops = _rcg2_ops,
> + },
> +};
> +
> +static struct clk_branch video_cc_ahb_clk = {
> + .halt_reg = 0xe58,
> + .halt_check = BRANCH_HALT,
> + .clkr = {
> + .enable_reg = 0xe58,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "video_cc_ahb_clk",
> + .parent_data = &(const struct clk_parent_data){
> + .hw = _cc_ahb_clk_src.clkr.hw,
> + },
> + .num_parents = 1,
> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
Similar critical clk comment, see below.
> + .ops = _branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch video_cc_mvs0_clk = {
> + .halt_reg = 0xd34,
> + .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */
Is this resolved?
> + .clkr = {
> + .enable_reg = 0xd34,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "video_cc_mvs0_clk",
> + .parent_data = &(const struct clk_parent_data){
> + .hw = _cc_mvs0_div_clk_src.clkr.hw,
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = _branch2_ops,
> + },
> + },
> +};
> +
[...]
> +
> +static struct clk_branch
[PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250
Add support for the video clock controller found on SM8250 based devices.
Derived from the downstream driver.
Signed-off-by: Jonathan Marek
---
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/videocc-sm8250.c | 518 ++
3 files changed, 528 insertions(+)
create mode 100644 drivers/clk/qcom/videocc-sm8250.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 40d7ee9886c9..95efa38211d5 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -453,6 +453,15 @@ config SM_VIDEOCC_8150
Say Y if you want to support video devices and functionality such as
video encode and decode.
+config SM_VIDEOCC_8250
+ tristate "SM8250 Video Clock Controller"
+ select SDM_GCC_8250
+ select QCOM_GDSC
+ help
+ Support for the video clock controller on SM8250 devices.
+ Say Y if you want to support video devices and functionality such as
+ video encode and decode.
+
config SPMI_PMIC_CLKDIV
tristate "SPMI PMIC clkdiv Support"
depends on SPMI || COMPILE_TEST
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 6f4c580d2728..55fb20800b66 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
+obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
diff --git a/drivers/clk/qcom/videocc-sm8250.c
b/drivers/clk/qcom/videocc-sm8250.c
new file mode 100644
index ..a814d10945c4
--- /dev/null
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
+#include "common.h"
+#include "reset.h"
+#include "gdsc.h"
+
+enum {
+ P_BI_TCXO,
+ P_CHIP_SLEEP_CLK,
+ P_CORE_BI_PLL_TEST_SE,
+ P_VIDEO_PLL0_OUT_MAIN,
+ P_VIDEO_PLL1_OUT_MAIN,
+};
+
+static const struct parent_map video_cc_parent_map_0[] = {
+ { P_BI_TCXO, 0 },
+};
+
+static const struct clk_parent_data video_cc_parent_data_0[] = {
+ { .fw_name = "bi_tcxo_ao" },
+};
+
+static struct pll_vco lucid_vco[] = {
+ { 24960, 20, 0 },
+};
+
+static const struct alpha_pll_config video_pll0_config = {
+ .l = 0x25,
+ .alpha = 0x8000,
+ .config_ctl_val = 0x20485699,
+ .config_ctl_hi_val = 0x2261,
+ .config_ctl_hi1_val = 0x329A699C,
+ .user_ctl_val = 0x,
+ .user_ctl_hi_val = 0x0805,
+ .user_ctl_hi1_val = 0x,
+};
+
+static struct clk_alpha_pll video_pll0 = {
+ .offset = 0x42c,
+ .vco_table = lucid_vco,
+ .num_vco = ARRAY_SIZE(lucid_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "video_pll0",
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "bi_tcxo",
+ },
+ .num_parents = 1,
+ .ops = _alpha_pll_lucid_ops,
+ },
+ },
+};
+
+static const struct alpha_pll_config video_pll1_config = {
+ .l = 0x2B,
+ .alpha = 0xC000,
+ .config_ctl_val = 0x20485699,
+ .config_ctl_hi_val = 0x2261,
+ .config_ctl_hi1_val = 0x329A699C,
+ .user_ctl_val = 0x,
+ .user_ctl_hi_val = 0x0805,
+ .user_ctl_hi1_val = 0x,
+};
+
+static struct clk_alpha_pll video_pll1 = {
+ .offset = 0x7d0,
+ .vco_table = lucid_vco,
+ .num_vco = ARRAY_SIZE(lucid_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "video_pll1",
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "bi_tcxo",
+ },
+ .num_parents = 1,
+ .ops = _alpha_pll_lucid_ops,
+ },
+ },
+};
+
+static const struct parent_map video_cc_parent_map_1[] = {
+ { P_BI_TCXO, 0 },
+ { P_VIDEO_PLL0_OUT_MAIN, 1 },
+};
+
+static const struct clk_parent_data video_cc_parent_data_1[] = {
+ { .fw_name = "bi_tcxo" },
+ { .hw = _pll0.clkr.hw },
+};
+
+static const struct parent_map video_cc_parent_map_2[] = {
+ { P_BI_TCXO, 0 },
+
