From: Bjorn Andersson <[email protected]>
As GDSCs are turned on and off some associated clocks are momentarily
enabled for house keeping purposes. Failure to enable these clocks seems
to have been silently ignored in the past, but starting in SM8350 this
failure will prevent the GDSC to turn on.
At least on SM8350 this operation will enable the RCG per the
configuration in CFG_REG. This means that the current model where the
current configuration is written back to CF_REG immediately after
parking the RCG doesn't work.
Instead, keep track of the currently requested rate of the clock and
upon enabling the clock reapply the configuration per the saved rate.
Fixes: 7ef6f11887bd ("clk: qcom: Configure the RCGs to a safe source as needed")
Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Vinod Koul <[email protected]>
Tested-by: Steev Klimaszewski <[email protected]>
---
drivers/clk/qcom/clk-rcg.h | 2 ++
drivers/clk/qcom/clk-rcg2.c | 32 +++++++++++++++++---------------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 00cea508d49e..8b41244b8dbf 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
* @freq_tbl: frequency table
* @clkr: regmap clock handle
* @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
+ * @current_rate: cached rate for parked RCGs
*/
struct clk_rcg2 {
u32 cmd_rcgr;
@@ -150,6 +151,7 @@ struct clk_rcg2 {
const struct freq_tbl *freq_tbl;
struct clk_regmap clkr;
u8 cfg_off;
+ unsigned long current_rate;
};
#define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index f675fd969c4d..81fd3a2db709 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -167,6 +167,7 @@ clk_rcg2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
{
struct clk_rcg2 *rcg = to_clk_rcg2(hw);
u32 cfg, hid_div, m = 0, n = 0, mode = 0, mask;
+ unsigned long rate;
regmap_read(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), &cfg);
@@ -186,7 +187,11 @@ clk_rcg2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
hid_div = cfg >> CFG_SRC_DIV_SHIFT;
hid_div &= mask;
- return calc_rate(parent_rate, m, n, mode, hid_div);
+ rate = calc_rate(parent_rate, m, n, mode, hid_div);
+ if (!rcg->current_rate)
+ rcg->current_rate = rate;
+
+ return rate;
}
static int _freq_tbl_determine_rate(struct clk_hw *hw, const struct freq_tbl *f,
@@ -978,12 +983,14 @@ static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long rate,
if (!f)
return -EINVAL;
+ rcg->current_rate = rate;
+
/*
- * In case clock is disabled, update the CFG, M, N and D registers
- * and don't hit the update bit of CMD register.
+ * In the case that the shared RCG is parked, current_rate will be
+ * applied as the clock is unparked again, so just return here.
*/
if (!__clk_is_enabled(hw->clk))
- return __clk_rcg2_configure(rcg, f);
+ return 0;
return clk_rcg2_shared_force_enable_clear(hw, f);
}
@@ -997,8 +1004,13 @@ static int clk_rcg2_shared_set_rate_and_parent(struct clk_hw *hw,
static int clk_rcg2_shared_enable(struct clk_hw *hw)
{
struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+ const struct freq_tbl *f = NULL;
int ret;
+ f = qcom_find_freq(rcg->freq_tbl, rcg->current_rate);
+ if (!f)
+ return -EINVAL;
+
/*
* Set the update bit because required configuration has already
* been written in clk_rcg2_shared_set_rate()
@@ -1007,7 +1019,7 @@ static int clk_rcg2_shared_enable(struct clk_hw *hw)
if (ret)
return ret;
- ret = update_config(rcg);
+ ret = clk_rcg2_configure(rcg, f);
if (ret)
return ret;
@@ -1017,13 +1029,6 @@ static int clk_rcg2_shared_enable(struct clk_hw *hw)
static void clk_rcg2_shared_disable(struct clk_hw *hw)
{
struct clk_rcg2 *rcg = to_clk_rcg2(hw);
- u32 cfg;
-
- /*
- * Store current configuration as switching to safe source would clear
- * the SRC and DIV of CFG register
- */
- regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
/*
* Park the RCG at a safe configuration - sourced off of safe source.
@@ -1041,9 +1046,6 @@ static void clk_rcg2_shared_disable(struct clk_hw *hw)
update_config(rcg);
clk_rcg2_clear_force_enable(hw);
-
- /* Write back the stored configuration corresponding to current rate */
- regmap_write(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, cfg);
}
const struct clk_ops clk_rcg2_shared_ops = {
--
2.32.0
Add regulator controlling MMCX power domain to be used by display clock
controller on SM8350.
Signed-off-by: Robert Foss <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index c0137bdcf94b..c49735d1b458 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -278,6 +278,14 @@ memory@80000000 {
reg = <0x0 0x80000000 0x0 0x0>;
};
+ mmcx_reg: mmcx-reg {
+ compatible = "regulator-fixed-domain";
+ power-domains = <&rpmhpd SM8350_MMCX>;
+ required-opps = <&rpmhpd_opp_nom>;
+ regulator-name = "MMCX";
+ regulator-always-on;
+ };
+
pmu {
compatible = "arm,armv8-pmuv3";
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
--
2.32.0
From: Bjorn Andersson <[email protected]>
The state of the shared RCGs found in the SM8250 dispcc can't reliably
be queried and hence doesn't implement the is_enabled() callback.
Mark the shared RCGs as CLK_ASSUME_ENABLED_WHEN_UNUSED, to ensure that
clk_disable_unused() will issue a disable and park the RCGs before it
turns off the parent PLLs - which will lock up these RCGs in any system
with continuous splash enabled.
Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Vinod Koul <[email protected]>
---
drivers/clk/qcom/dispcc-sm8250.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index db9379634fb2..22d9cbabecab 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -214,7 +214,7 @@ static struct clk_rcg2 disp_cc_mdss_ahb_clk_src = {
.name = "disp_cc_mdss_ahb_clk_src",
.parent_data = disp_cc_parent_data_3,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
- .flags = CLK_SET_RATE_PARENT,
+ .flags = CLK_SET_RATE_PARENT | CLK_ASSUME_ENABLED_WHEN_UNUSED,
.ops = &clk_rcg2_shared_ops,
},
};
@@ -546,7 +546,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
.name = "disp_cc_mdss_mdp_clk_src",
.parent_data = disp_cc_parent_data_5,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
- .flags = CLK_SET_RATE_PARENT,
+ .flags = CLK_SET_RATE_PARENT | CLK_ASSUME_ENABLED_WHEN_UNUSED,
.ops = &clk_rcg2_shared_ops,
},
};
@@ -598,7 +598,7 @@ static struct clk_rcg2 disp_cc_mdss_rot_clk_src = {
.name = "disp_cc_mdss_rot_clk_src",
.parent_data = disp_cc_parent_data_5,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
- .flags = CLK_SET_RATE_PARENT,
+ .flags = CLK_SET_RATE_PARENT | CLK_ASSUME_ENABLED_WHEN_UNUSED,
.ops = &clk_rcg2_shared_ops,
},
};
--
2.32.0
From: Jonathan Marek <[email protected]>
Add support to the SM8350 display clock controller by extending the SM8250
display clock controller, which is almost identical but has some minor
differences.
Signed-off-by: Jonathan Marek <[email protected]>
---
drivers/clk/qcom/Kconfig | 4 +--
drivers/clk/qcom/dispcc-sm8250.c | 61 +++++++++++++++++++++++++++++++-
2 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 9b1f54e634b9..1752ca0ee405 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -609,11 +609,11 @@ config SM_DISPCC_6125
splash screen
config SM_DISPCC_8250
- tristate "SM8150 and SM8250 Display Clock Controller"
+ tristate "SM8150/SM8250/SM8350 Display Clock Controller"
depends on SM_GCC_8150 || SM_GCC_8250
help
Support for the display clock controller on Qualcomm Technologies, Inc
- SM8150 and SM8250 devices.
+ SM8150/SM8250/SM8350 devices.
Say Y if you want to support display devices and functionality such as
splash screen.
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index 22d9cbabecab..95f86ffcc3b3 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -43,6 +43,10 @@ static struct pll_vco vco_table[] = {
{ 249600000, 2000000000, 0 },
};
+static struct pll_vco lucid_5lpe_vco[] = {
+ { 249600000, 1750000000, 0 },
+};
+
static struct alpha_pll_config disp_cc_pll0_config = {
.l = 0x47,
.alpha = 0xE000,
@@ -1228,6 +1232,7 @@ static const struct of_device_id disp_cc_sm8250_match_table[] = {
{ .compatible = "qcom,sc8180x-dispcc" },
{ .compatible = "qcom,sm8150-dispcc" },
{ .compatible = "qcom,sm8250-dispcc" },
+ { .compatible = "qcom,sm8350-dispcc" },
{ }
};
MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table);
@@ -1258,7 +1263,7 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
return PTR_ERR(regmap);
}
- /* note: trion == lucid, except for the prepare() op */
+ /* Apply differences for SM8150 and SM8350 */
BUILD_BUG_ON(CLK_ALPHA_PLL_TYPE_TRION != CLK_ALPHA_PLL_TYPE_LUCID);
if (of_device_is_compatible(pdev->dev.of_node, "qcom,sc8180x-dispcc") ||
of_device_is_compatible(pdev->dev.of_node, "qcom,sm8150-dispcc")) {
@@ -1270,8 +1275,62 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
disp_cc_pll1_config.config_ctl_hi1_val = 0x00000024;
disp_cc_pll1_config.user_ctl_hi1_val = 0x000000D0;
disp_cc_pll1_init.ops = &clk_alpha_pll_trion_ops;
+ } else if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8350-dispcc")) {
+ static struct clk_rcg2 * const rcgs[] = {
+ &disp_cc_mdss_byte0_clk_src,
+ &disp_cc_mdss_byte1_clk_src,
+ &disp_cc_mdss_dp_aux1_clk_src,
+ &disp_cc_mdss_dp_aux_clk_src,
+ &disp_cc_mdss_dp_link1_clk_src,
+ &disp_cc_mdss_dp_link_clk_src,
+ &disp_cc_mdss_dp_pixel1_clk_src,
+ &disp_cc_mdss_dp_pixel2_clk_src,
+ &disp_cc_mdss_dp_pixel_clk_src,
+ &disp_cc_mdss_esc0_clk_src,
+ &disp_cc_mdss_mdp_clk_src,
+ &disp_cc_mdss_pclk0_clk_src,
+ &disp_cc_mdss_pclk1_clk_src,
+ &disp_cc_mdss_rot_clk_src,
+ &disp_cc_mdss_vsync_clk_src,
+ };
+ static struct clk_regmap_div * const divs[] = {
+ &disp_cc_mdss_byte0_div_clk_src,
+ &disp_cc_mdss_byte1_div_clk_src,
+ &disp_cc_mdss_dp_link1_div_clk_src,
+ &disp_cc_mdss_dp_link_div_clk_src,
+ };
+ unsigned int i;
+ static bool offset_applied;
+
+ /* only apply the offsets once (in case of deferred probe) */
+ if (!offset_applied) {
+ for (i = 0; i < ARRAY_SIZE(rcgs); i++)
+ rcgs[i]->cmd_rcgr -= 4;
+
+ for (i = 0; i < ARRAY_SIZE(divs); i++) {
+ divs[i]->reg -= 4;
+ divs[i]->width = 4;
+ }
+
+ disp_cc_mdss_ahb_clk.halt_reg -= 4;
+ disp_cc_mdss_ahb_clk.clkr.enable_reg -= 4;
+
+ offset_applied = true;
+ }
+
+ disp_cc_mdss_ahb_clk_src.cmd_rcgr = 0x22a0;
+
+ disp_cc_pll0_config.config_ctl_hi1_val = 0x2A9A699C;
+ disp_cc_pll0_config.test_ctl_hi1_val = 0x01800000;
+ disp_cc_pll0_init.ops = &clk_alpha_pll_lucid_5lpe_ops;
+ disp_cc_pll0.vco_table = lucid_5lpe_vco;
+ disp_cc_pll1_config.config_ctl_hi1_val = 0x2A9A699C;
+ disp_cc_pll1_config.test_ctl_hi1_val = 0x01800000;
+ disp_cc_pll1_init.ops = &clk_alpha_pll_lucid_5lpe_ops;
+ disp_cc_pll1.vco_table = lucid_5lpe_vco;
}
+ /* note for SM8350: downstream lucid_5lpe configure differs slightly */
clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
clk_lucid_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
--
2.32.0
On 29/04/2022 18:12, Robert Foss wrote:
> From: Jonathan Marek <[email protected]>
>
> Add support to the SM8350 display clock controller by extending the SM8250
> display clock controller, which is almost identical but has some minor
> differences.
>
> Signed-off-by: Jonathan Marek <[email protected]>
> ---
> drivers/clk/qcom/Kconfig | 4 +--
> drivers/clk/qcom/dispcc-sm8250.c | 61 +++++++++++++++++++++++++++++++-
> 2 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 9b1f54e634b9..1752ca0ee405 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -609,11 +609,11 @@ config SM_DISPCC_6125
> splash screen
>
> config SM_DISPCC_8250
> - tristate "SM8150 and SM8250 Display Clock Controller"
> + tristate "SM8150/SM8250/SM8350 Display Clock Controller"
> depends on SM_GCC_8150 || SM_GCC_8250
> help
> Support for the display clock controller on Qualcomm Technologies, Inc
> - SM8150 and SM8250 devices.
> + SM8150/SM8250/SM8350 devices.
> Say Y if you want to support display devices and functionality such as
> splash screen.
>
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index 22d9cbabecab..95f86ffcc3b3 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -43,6 +43,10 @@ static struct pll_vco vco_table[] = {
> { 249600000, 2000000000, 0 },
> };
>
> +static struct pll_vco lucid_5lpe_vco[] = {
> + { 249600000, 1750000000, 0 },
> +};
> +
> static struct alpha_pll_config disp_cc_pll0_config = {
> .l = 0x47,
> .alpha = 0xE000,
> @@ -1228,6 +1232,7 @@ static const struct of_device_id disp_cc_sm8250_match_table[] = {
> { .compatible = "qcom,sc8180x-dispcc" },
> { .compatible = "qcom,sm8150-dispcc" },
> { .compatible = "qcom,sm8250-dispcc" },
> + { .compatible = "qcom,sm8350-dispcc" },
> { }
> };
> MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table);
> @@ -1258,7 +1263,7 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
> return PTR_ERR(regmap);
> }
>
> - /* note: trion == lucid, except for the prepare() op */
> + /* Apply differences for SM8150 and SM8350 */
> BUILD_BUG_ON(CLK_ALPHA_PLL_TYPE_TRION != CLK_ALPHA_PLL_TYPE_LUCID);
> if (of_device_is_compatible(pdev->dev.of_node, "qcom,sc8180x-dispcc") ||
> of_device_is_compatible(pdev->dev.of_node, "qcom,sm8150-dispcc")) {
> @@ -1270,8 +1275,62 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
> disp_cc_pll1_config.config_ctl_hi1_val = 0x00000024;
> disp_cc_pll1_config.user_ctl_hi1_val = 0x000000D0;
> disp_cc_pll1_init.ops = &clk_alpha_pll_trion_ops;
> + } else if (of_device_is_compatible(pdev->dev.of_node, "qcom,sm8350-dispcc")) {
> + static struct clk_rcg2 * const rcgs[] = {
> + &disp_cc_mdss_byte0_clk_src,
> + &disp_cc_mdss_byte1_clk_src,
> + &disp_cc_mdss_dp_aux1_clk_src,
> + &disp_cc_mdss_dp_aux_clk_src,
> + &disp_cc_mdss_dp_link1_clk_src,
> + &disp_cc_mdss_dp_link_clk_src,
> + &disp_cc_mdss_dp_pixel1_clk_src,
> + &disp_cc_mdss_dp_pixel2_clk_src,
> + &disp_cc_mdss_dp_pixel_clk_src,
> + &disp_cc_mdss_esc0_clk_src,
> + &disp_cc_mdss_mdp_clk_src,
> + &disp_cc_mdss_pclk0_clk_src,
> + &disp_cc_mdss_pclk1_clk_src,
> + &disp_cc_mdss_rot_clk_src,
> + &disp_cc_mdss_vsync_clk_src,
> + };
> + static struct clk_regmap_div * const divs[] = {
> + &disp_cc_mdss_byte0_div_clk_src,
> + &disp_cc_mdss_byte1_div_clk_src,
> + &disp_cc_mdss_dp_link1_div_clk_src,
> + &disp_cc_mdss_dp_link_div_clk_src,
> + };
> + unsigned int i;
> + static bool offset_applied;
> +
> + /* only apply the offsets once (in case of deferred probe) */
> + if (!offset_applied) {
> + for (i = 0; i < ARRAY_SIZE(rcgs); i++)
> + rcgs[i]->cmd_rcgr -= 4;
> +
> + for (i = 0; i < ARRAY_SIZE(divs); i++) {
> + divs[i]->reg -= 4;
> + divs[i]->width = 4;
> + }
> +
> + disp_cc_mdss_ahb_clk.halt_reg -= 4;
> + disp_cc_mdss_ahb_clk.clkr.enable_reg -= 4;
> +
> + offset_applied = true;
> + }
> +
> + disp_cc_mdss_ahb_clk_src.cmd_rcgr = 0x22a0;
> +
> + disp_cc_pll0_config.config_ctl_hi1_val = 0x2A9A699C;
> + disp_cc_pll0_config.test_ctl_hi1_val = 0x01800000;
> + disp_cc_pll0_init.ops = &clk_alpha_pll_lucid_5lpe_ops;
> + disp_cc_pll0.vco_table = lucid_5lpe_vco;
> + disp_cc_pll1_config.config_ctl_hi1_val = 0x2A9A699C;
> + disp_cc_pll1_config.test_ctl_hi1_val = 0x01800000;
> + disp_cc_pll1_init.ops = &clk_alpha_pll_lucid_5lpe_ops;
> + disp_cc_pll1.vco_table = lucid_5lpe_vco;
> }
>
> + /* note for SM8350: downstream lucid_5lpe configure differs slightly */
Isn't this already being taken care by the previous code?
With this comment removed:
Reviewed-by: Dmitry Baryshkov <[email protected]>
> clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
> clk_lucid_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
>
--
With best wishes
Dmitry