Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:05PM +0530, Imran Shaik wrote:
> The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
> but has minor differences. Update the parent data of mdss ahb/mdp clocks
> accordingly to the hardware clock plan and correct the GDSC *_wait_val and
> flags which are applicable for both QCM2290 and Shikra SoC, and add the
> support for DSI1 PHY source.
> 
> Signed-off-by: Imran Shaik <imran.shaik@xxxxxxxxxxxxxxxx>
> ---
>  drivers/clk/qcom/dispcc-qcm2290.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> index 6d88d067337fa132114b0d8666931b449f86de17..19c997f3fe9f197d2c252a9dd1e8169947200f5f 100644
> --- a/drivers/clk/qcom/dispcc-qcm2290.c
> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>   * Copyright (c) 2021, Linaro Ltd.
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>   */
>  
>  #include <linux/clk-provider.h>
> @@ -32,6 +33,8 @@ enum {
>  	P_GPLL0_OUT_DIV,
>  	P_GPLL0_OUT_MAIN,
>  	P_SLEEP_CLK,
> +	P_DSI1_PHY_PLL_OUT_BYTECLK,
> +	P_DSI1_PHY_PLL_OUT_DSICLK,
>  };
>  
>  static const struct pll_vco spark_vco[] = {
> @@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
>  
>  static const struct parent_map disp_cc_parent_map_2[] = {
>  	{ P_BI_TCXO_AO, 0 },
> -	{ P_GPLL0_OUT_DIV, 4 },
> +	{ P_GPLL0_OUT_MAIN, 4 },

Why?

>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_2[] = {
> @@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
>  static const struct clk_parent_data disp_cc_parent_data_3[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .hw = &disp_cc_pll0.clkr.hw },
> -	{ .fw_name = "gcc_disp_gpll0_clk_src" },
> +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },

Do you realize that this is an undocumented ABI chance?

>  };
>  
>  static const struct parent_map disp_cc_parent_map_4[] = {
>  	{ P_BI_TCXO, 0 },
>  	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
> +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_4[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
> +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
>  };
>  
>  static const struct parent_map disp_cc_parent_map_5[] = {
> @@ -153,8 +158,8 @@ static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
>  
>  static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
>  	F(19200000, P_BI_TCXO_AO, 1, 0, 0),
> -	F(37500000, P_GPLL0_OUT_DIV, 8, 0, 0),
> -	F(75000000, P_GPLL0_OUT_DIV, 4, 0, 0),
> +	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
> +	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),

Why? It's not mentioned in the commit message.

>  	{ }
>  };
>  
> @@ -450,11 +455,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
>  
>  static struct gdsc mdss_gdsc = {
>  	.gdscr = 0x3000,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
>  	.pd = {
>  		.name = "mdss_gdsc",
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,

And this also needs explanation. 

>  };

-- 
With best wishes
Dmitry