Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver

From: Dmitry Baryshkov
Date: Tue Jan 03 2023 - 09:25:24 EST

Next message: Krzysztof Kozlowski: "Re: RFC on drivers/memory vs drivers/edac memory mapping for DDR Controller"
Previous message: Srinivas Kandagatla: "Re: [PATCH v2] nvmem: core: Fix race in nvmem_register()"
In reply to: Konrad Dybcio: "Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver"
Next in thread: Neil Armstrong: "Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, 3 Jan 2023 at 15:54, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
>
> Add support for the display clock controller found in SM8550
> based devices.
>
> This clock controller feeds the Multimedia Display SubSystem (MDSS).
> This driver is based on the SM8450 support.

Looks good, few minor nits below:

>
> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> ---
> drivers/clk/qcom/Kconfig | 9 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/dispcc-sm8550.c | 1814 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1824 insertions(+)
>

[skipped]

> +static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
> + .reg = 0x8120,
> + .shift = 0,
> + .width = 4,
> + .clkr.hw.init = &(struct clk_init_data) {
> + .name = "disp_cc_mdss_byte0_div_clk_src",
> + .parent_data = &(const struct clk_parent_data) {
> + .hw = &disp_cc_mdss_byte0_clk_src.clkr.hw,
> + },

We can use parent_hws in such cases (here and below).

> + .num_parents = 1,
> + .ops = &clk_regmap_div_ops,
> + },
> +};
> +
> +static struct clk_regmap_div disp_cc_mdss_byte1_div_clk_src = {
> + .reg = 0x813c,
> + .shift = 0,
> + .width = 4,
> + .clkr.hw.init = &(struct clk_init_data) {
> + .name = "disp_cc_mdss_byte1_div_clk_src",
> + .parent_data = &(const struct clk_parent_data) {
> + .hw = &disp_cc_mdss_byte1_clk_src.clkr.hw,
> + },
> + .num_parents = 1,
> + .ops = &clk_regmap_div_ops,
> + },
> +};

[skipped most of the clocks]

> +static struct gdsc mdss_gdsc = {
> + .gdscr = 0x9000,
> + .pd = {
> + .name = "mdss_gdsc",
> + },
> + .pwrsts = PWRSTS_OFF_ON,
> + .flags = HW_CTRL | RETAIN_FF_ENABLE,
> +};
> +
> +static struct gdsc mdss_int2_gdsc = {
> + .gdscr = 0xb000,
> + .pd = {
> + .name = "mdss_int2_gdsc",
> + },
> + .pwrsts = PWRSTS_OFF_ON,
> + .flags = HW_CTRL | RETAIN_FF_ENABLE,
> +};
> +
> +static struct clk_regmap *disp_cc_sm8550_clocks[] = {
> + [DISP_CC_MDSS_ACCU_CLK] = &disp_cc_mdss_accu_clk.clkr,
> + [DISP_CC_MDSS_AHB1_CLK] = &disp_cc_mdss_ahb1_clk.clkr,
> + [DISP_CC_MDSS_AHB_CLK] = &disp_cc_mdss_ahb_clk.clkr,
> + [DISP_CC_MDSS_AHB_CLK_SRC] = &disp_cc_mdss_ahb_clk_src.clkr,
> + [DISP_CC_MDSS_BYTE0_CLK] = &disp_cc_mdss_byte0_clk.clkr,
> + [DISP_CC_MDSS_BYTE0_CLK_SRC] = &disp_cc_mdss_byte0_clk_src.clkr,
> + [DISP_CC_MDSS_BYTE0_DIV_CLK_SRC] = &disp_cc_mdss_byte0_div_clk_src.clkr,
> + [DISP_CC_MDSS_BYTE0_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.clkr,
> + [DISP_CC_MDSS_BYTE1_CLK] = &disp_cc_mdss_byte1_clk.clkr,
> + [DISP_CC_MDSS_BYTE1_CLK_SRC] = &disp_cc_mdss_byte1_clk_src.clkr,
> + [DISP_CC_MDSS_BYTE1_DIV_CLK_SRC] = &disp_cc_mdss_byte1_div_clk_src.clkr,
> + [DISP_CC_MDSS_BYTE1_INTF_CLK] = &disp_cc_mdss_byte1_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_AUX_CLK] = &disp_cc_mdss_dptx0_aux_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_AUX_CLK_SRC] = &disp_cc_mdss_dptx0_aux_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX0_CRYPTO_CLK] = &disp_cc_mdss_dptx0_crypto_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_LINK_CLK] = &disp_cc_mdss_dptx0_link_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_LINK_CLK_SRC] = &disp_cc_mdss_dptx0_link_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX0_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx0_link_div_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX0_LINK_INTF_CLK] = &disp_cc_mdss_dptx0_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_PIXEL0_CLK] = &disp_cc_mdss_dptx0_pixel0_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx0_pixel0_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX0_PIXEL1_CLK] = &disp_cc_mdss_dptx0_pixel1_clk.clkr,
> + [DISP_CC_MDSS_DPTX0_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx0_pixel1_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX0_USB_ROUTER_LINK_INTF_CLK] =
> + &disp_cc_mdss_dptx0_usb_router_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_AUX_CLK] = &disp_cc_mdss_dptx1_aux_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_AUX_CLK_SRC] = &disp_cc_mdss_dptx1_aux_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX1_CRYPTO_CLK] = &disp_cc_mdss_dptx1_crypto_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_LINK_CLK] = &disp_cc_mdss_dptx1_link_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_LINK_CLK_SRC] = &disp_cc_mdss_dptx1_link_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX1_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx1_link_div_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX1_LINK_INTF_CLK] = &disp_cc_mdss_dptx1_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_PIXEL0_CLK] = &disp_cc_mdss_dptx1_pixel0_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx1_pixel0_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX1_PIXEL1_CLK] = &disp_cc_mdss_dptx1_pixel1_clk.clkr,
> + [DISP_CC_MDSS_DPTX1_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx1_pixel1_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX1_USB_ROUTER_LINK_INTF_CLK] =
> + &disp_cc_mdss_dptx1_usb_router_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_AUX_CLK] = &disp_cc_mdss_dptx2_aux_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_AUX_CLK_SRC] = &disp_cc_mdss_dptx2_aux_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX2_CRYPTO_CLK] = &disp_cc_mdss_dptx2_crypto_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_LINK_CLK] = &disp_cc_mdss_dptx2_link_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_LINK_CLK_SRC] = &disp_cc_mdss_dptx2_link_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX2_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx2_link_div_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX2_LINK_INTF_CLK] = &disp_cc_mdss_dptx2_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_PIXEL0_CLK] = &disp_cc_mdss_dptx2_pixel0_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx2_pixel0_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX2_PIXEL1_CLK] = &disp_cc_mdss_dptx2_pixel1_clk.clkr,
> + [DISP_CC_MDSS_DPTX2_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx2_pixel1_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX3_AUX_CLK] = &disp_cc_mdss_dptx3_aux_clk.clkr,
> + [DISP_CC_MDSS_DPTX3_AUX_CLK_SRC] = &disp_cc_mdss_dptx3_aux_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX3_CRYPTO_CLK] = &disp_cc_mdss_dptx3_crypto_clk.clkr,
> + [DISP_CC_MDSS_DPTX3_LINK_CLK] = &disp_cc_mdss_dptx3_link_clk.clkr,
> + [DISP_CC_MDSS_DPTX3_LINK_CLK_SRC] = &disp_cc_mdss_dptx3_link_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX3_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx3_link_div_clk_src.clkr,
> + [DISP_CC_MDSS_DPTX3_LINK_INTF_CLK] = &disp_cc_mdss_dptx3_link_intf_clk.clkr,
> + [DISP_CC_MDSS_DPTX3_PIXEL0_CLK] = &disp_cc_mdss_dptx3_pixel0_clk.clkr,
> + [DISP_CC_MDSS_DPTX3_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx3_pixel0_clk_src.clkr,
> + [DISP_CC_MDSS_ESC0_CLK] = &disp_cc_mdss_esc0_clk.clkr,
> + [DISP_CC_MDSS_ESC0_CLK_SRC] = &disp_cc_mdss_esc0_clk_src.clkr,
> + [DISP_CC_MDSS_ESC1_CLK] = &disp_cc_mdss_esc1_clk.clkr,
> + [DISP_CC_MDSS_ESC1_CLK_SRC] = &disp_cc_mdss_esc1_clk_src.clkr,
> + [DISP_CC_MDSS_MDP1_CLK] = &disp_cc_mdss_mdp1_clk.clkr,
> + [DISP_CC_MDSS_MDP_CLK] = &disp_cc_mdss_mdp_clk.clkr,
> + [DISP_CC_MDSS_MDP_CLK_SRC] = &disp_cc_mdss_mdp_clk_src.clkr,
> + [DISP_CC_MDSS_MDP_LUT1_CLK] = &disp_cc_mdss_mdp_lut1_clk.clkr,
> + [DISP_CC_MDSS_MDP_LUT_CLK] = &disp_cc_mdss_mdp_lut_clk.clkr,
> + [DISP_CC_MDSS_NON_GDSC_AHB_CLK] = &disp_cc_mdss_non_gdsc_ahb_clk.clkr,
> + [DISP_CC_MDSS_PCLK0_CLK] = &disp_cc_mdss_pclk0_clk.clkr,
> + [DISP_CC_MDSS_PCLK0_CLK_SRC] = &disp_cc_mdss_pclk0_clk_src.clkr,
> + [DISP_CC_MDSS_PCLK1_CLK] = &disp_cc_mdss_pclk1_clk.clkr,
> + [DISP_CC_MDSS_PCLK1_CLK_SRC] = &disp_cc_mdss_pclk1_clk_src.clkr,
> + [DISP_CC_MDSS_RSCC_AHB_CLK] = &disp_cc_mdss_rscc_ahb_clk.clkr,
> + [DISP_CC_MDSS_RSCC_VSYNC_CLK] = &disp_cc_mdss_rscc_vsync_clk.clkr,
> + [DISP_CC_MDSS_VSYNC1_CLK] = &disp_cc_mdss_vsync1_clk.clkr,
> + [DISP_CC_MDSS_VSYNC_CLK] = &disp_cc_mdss_vsync_clk.clkr,
> + [DISP_CC_MDSS_VSYNC_CLK_SRC] = &disp_cc_mdss_vsync_clk_src.clkr,
> + [DISP_CC_PLL0] = &disp_cc_pll0.clkr,
> + [DISP_CC_PLL1] = &disp_cc_pll1.clkr,
> + [DISP_CC_SLEEP_CLK] = &disp_cc_sleep_clk.clkr,
> + [DISP_CC_SLEEP_CLK_SRC] = &disp_cc_sleep_clk_src.clkr,
> + [DISP_CC_XO_CLK_SRC] = &disp_cc_xo_clk_src.clkr,
> +};
> +
> +static const struct qcom_reset_map disp_cc_sm8550_resets[] = {
> + [DISP_CC_MDSS_CORE_BCR] = { 0x8000 },
> + [DISP_CC_MDSS_CORE_INT2_BCR] = { 0xa000 },
> + [DISP_CC_MDSS_RSCC_BCR] = { 0xc000 },
> +};
> +
> +static struct gdsc *disp_cc_sm8550_gdscs[] = {
> + [MDSS_GDSC] = &mdss_gdsc,
> + [MDSS_INT2_GDSC] = &mdss_int2_gdsc,
> +};
> +
> +static const struct regmap_config disp_cc_sm8550_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = 0x11008,
> + .fast_io = true,
> +};
> +
> +static struct qcom_cc_desc disp_cc_sm8550_desc = {
> + .config = &disp_cc_sm8550_regmap_config,
> + .clks = disp_cc_sm8550_clocks,
> + .num_clks = ARRAY_SIZE(disp_cc_sm8550_clocks),
> + .resets = disp_cc_sm8550_resets,
> + .num_resets = ARRAY_SIZE(disp_cc_sm8550_resets),
> + .gdscs = disp_cc_sm8550_gdscs,
> + .num_gdscs = ARRAY_SIZE(disp_cc_sm8550_gdscs),
> +};
> +
> +static const struct of_device_id disp_cc_sm8550_match_table[] = {
> + { .compatible = "qcom,sm8550-dispcc" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, disp_cc_sm8550_match_table);
> +
> +static void disp_cc_sm8550_pm_runtime_disable(void *data)
> +{
> + pm_runtime_disable(data);
> +}
> +
> +static int disp_cc_sm8550_probe(struct platform_device *pdev)
> +{
> + struct regmap *regmap;
> + int ret;
> +
> + pm_runtime_enable(&pdev->dev);

We can use devm_pm_runtime_enable() here.

> +
> + ret = devm_add_action_or_reset(&pdev->dev, disp_cc_sm8550_pm_runtime_disable, &pdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(&pdev->dev);
> + if (ret)
> + return ret;
> +
> + regmap = qcom_cc_map(pdev, &disp_cc_sm8550_desc);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + clk_lucid_evo_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
> + clk_lucid_evo_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
> +
> + /* Enable clock gating for MDP clocks */
> + regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);
> +
> + /*
> + * Keep clocks always enabled:
> + * disp_cc_xo_clk
> + */
> + regmap_update_bits(regmap, 0xe054, BIT(0), BIT(0));
> +
> + ret = qcom_cc_really_probe(pdev, &disp_cc_sm8550_desc, regmap);
> +
> + pm_runtime_put(&pdev->dev);
> +
> + return ret;
> +}
> +
> +static struct platform_driver disp_cc_sm8550_driver = {
> + .probe = disp_cc_sm8550_probe,
> + .driver = {
> + .name = "disp_cc-sm8550",
> + .of_match_table = disp_cc_sm8550_match_table,
> + },
> +};
> +
> +static int __init disp_cc_sm8550_init(void)
> +{
> + return platform_driver_register(&disp_cc_sm8550_driver);
> +}
> +subsys_initcall(disp_cc_sm8550_init);
> +
> +static void __exit disp_cc_sm8550_exit(void)
> +{
> + platform_driver_unregister(&disp_cc_sm8550_driver);
> +}
> +module_exit(disp_cc_sm8550_exit);
> +
> +MODULE_DESCRIPTION("QTI DISPCC SM8550 Driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1

--
With best wishes
Dmitry

Next message: Krzysztof Kozlowski: "Re: RFC on drivers/memory vs drivers/edac memory mapping for DDR Controller"
Previous message: Srinivas Kandagatla: "Re: [PATCH v2] nvmem: core: Fix race in nvmem_register()"
In reply to: Konrad Dybcio: "Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver"
Next in thread: Neil Armstrong: "Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]