diff mbox series

[v2,1/2] clk: qcom: add support for SM8350 DISPCC

Message ID 20210519001802.1863-1-jonathan@marek.ca (mailing list archive)
State Changes Requested, archived
Headers show
Series [v2,1/2] clk: qcom: add support for SM8350 DISPCC | expand

Commit Message

Jonathan Marek May 19, 2021, 12:18 a.m. UTC
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 <jonathan@marek.ca>
---
v2: improved diff (don't move sm8150 case around), update comment

 drivers/clk/qcom/Kconfig         |  4 +--
 drivers/clk/qcom/dispcc-sm8250.c | 61 +++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 3 deletions(-)

Comments

Stephen Boyd June 2, 2021, 9:34 p.m. UTC | #1
Quoting Jonathan Marek (2021-05-18 17:18:01)
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index de09cd5c209f..5f22a715e2f0 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -36,6 +36,10 @@ static struct pll_vco vco_table[] = {
>         { 249600000, 2000000000, 0 },
>  };
>  
> +static struct pll_vco lucid_5lpe_vco[] = {

const

> +       { 249600000, 1750000000, 0 },
> +};
> +
>  static struct alpha_pll_config disp_cc_pll0_config = {
>         .l = 0x47,
>         .alpha = 0xE000,
> @@ -1039,6 +1043,7 @@ static const struct qcom_cc_desc disp_cc_sm8250_desc = {
>  static const struct of_device_id disp_cc_sm8250_match_table[] = {
>         { .compatible = "qcom,sm8150-dispcc" },
>         { .compatible = "qcom,sm8250-dispcc" },
> +       { .compatible = "qcom,sm8350-dispcc" },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table);
> @@ -1051,7 +1056,7 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
>         if (IS_ERR(regmap))
>                 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,sm8150-dispcc")) {
>                 disp_cc_pll0_config.config_ctl_hi_val = 0x00002267;
> @@ -1062,8 +1067,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[] = {

Please move this to global scope and give it a name.

> +                       &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[] = {

Move global.

> +                       &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 i;

int i? I doubt being unsigned helps.

> +               static bool offset_applied = false;
> +
> +               /* only apply the offsets once (in case of deferred probe) */
> +               if (!offset_applied) {

Maybe instead of doing this in probe it can be done when the driver is
added in module init? It would mean searching the DT for the compatible
string and then if it is present running the subtraction code, but at
least we would only do it once and the code would be thrown away after
init.

> +                       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;

Lowercase hex please.

> +               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 */

Is this a TODO?

>         clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
>         clk_lucid_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
>
diff mbox series

Patch

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 45646b867cdb..cc60e6ee1654 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -484,11 +484,11 @@  config SDX_GCC_55
 	  SPI, I2C, USB, SD/UFS, PCIe etc.
 
 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 de09cd5c209f..5f22a715e2f0 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -36,6 +36,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,
@@ -1039,6 +1043,7 @@  static const struct qcom_cc_desc disp_cc_sm8250_desc = {
 static const struct of_device_id disp_cc_sm8250_match_table[] = {
 	{ .compatible = "qcom,sm8150-dispcc" },
 	{ .compatible = "qcom,sm8250-dispcc" },
+	{ .compatible = "qcom,sm8350-dispcc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, disp_cc_sm8250_match_table);
@@ -1051,7 +1056,7 @@  static int disp_cc_sm8250_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		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,sm8150-dispcc")) {
 		disp_cc_pll0_config.config_ctl_hi_val = 0x00002267;
@@ -1062,8 +1067,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 i;
+		static bool offset_applied = false;
+
+		/* 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);