Re: [PATCH v2 3/7] clk: qcom: videocc-x1p42100: Add support for video clock controller

[Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>]

On 3/5/2026 5:50 AM, Dmitry Baryshkov wrote:
> On Wed, Mar 04, 2026 at 11:10:53PM +0530, Jagadeesh Kona wrote:
>> Add support for the video clock controller for video clients to be
>> able to request for videocc clocks on X1P42100 platform. Although
>> X1P42100 is derived from X1E80100, the video clock controller differs
>> significantly. The BSE clocks are newly added, several cdiv clocks have
>> been removed, and most RCG frequency tables have been updated. Initial
>> PLL configurations also require changes, hence introduce a separate
>> videocc driver for X1P42100 platform.
>>
>> Reviewed-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> Signed-off-by: Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>
>> ---
>>  drivers/clk/qcom/Kconfig            |  10 +
>>  drivers/clk/qcom/Makefile           |   1 +
>>  drivers/clk/qcom/videocc-x1p42100.c | 585 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 596 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index a8a86ea6bb7445e396048a5bba23fce8d719281f..b1b1ed70871c1b348985d6f5149152e57637d904 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -155,6 +155,16 @@ config CLK_X1P42100_GPUCC
>>  	  Say Y if you want to support graphics controller devices and
>>  	  functionality such as 3D graphics.
>>  
>> +config CLK_X1P42100_VIDEOCC
>> +	tristate "X1P42100 Video Clock Controller"
>> +	depends on ARM64 || COMPILE_TEST
>> +	select CLK_X1E80100_GCC
>> +	help
>> +	  Support for the video clock controller on Qualcomm Technologies, Inc.
>> +	  X1P42100 devices.
>> +	  Say Y if you want to support video devices and functionality such as
>> +	  video encode/decode.
>> +
>>  config CLK_QCM2290_GPUCC
>>  	tristate "QCM2290 Graphics Clock Controller"
>>  	depends on ARM64 || COMPILE_TEST
>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index 6b0ad8832b55f1914079f15323b8cdd1608ad4c0..b746e5046774259cebe3da8dad1f226561268a8b 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_CLK_X1E80100_GCC) += gcc-x1e80100.o
>>  obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
>>  obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
>>  obj-$(CONFIG_CLK_X1P42100_GPUCC) += gpucc-x1p42100.o
>> +obj-$(CONFIG_CLK_X1P42100_VIDEOCC) += videocc-x1p42100.o
>>  obj-$(CONFIG_CLK_QCM2290_GPUCC) += gpucc-qcm2290.o
>>  obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
>>  obj-$(CONFIG_IPQ_APSS_5424) += apss-ipq5424.o
>> diff --git a/drivers/clk/qcom/videocc-x1p42100.c b/drivers/clk/qcom/videocc-x1p42100.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..4e92f0ab7e4016724af1f31aa4896655b1dc8308
>> --- /dev/null
>> +++ b/drivers/clk/qcom/videocc-x1p42100.c
>> @@ -0,0 +1,585 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <dt-bindings/clock/qcom,sm8650-videocc.h>
>> +
>> +#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 "gdsc.h"
>> +#include "reset.h"
>> +
>> +enum {
>> +	DT_BI_TCXO,
>> +};
>> +
>> +enum {
>> +	P_BI_TCXO,
>> +	P_VIDEO_CC_PLL0_OUT_MAIN,
>> +	P_VIDEO_CC_PLL1_OUT_MAIN,
>> +};
>> +
>> +static const struct pll_vco lucid_ole_vco[] = {
>> +	{ 249600000, 2300000000, 0 },
>> +};
>> +
>> +/* 420.0 MHz Configuration */
>> +static const struct alpha_pll_config video_cc_pll0_config = {
>> +	.l = 0x15,
>> +	.alpha = 0xe000,
>> +	.config_ctl_val = 0x20485699,
>> +	.config_ctl_hi_val = 0x00182261,
>> +	.config_ctl_hi1_val = 0x82aa299c,
>> +	.test_ctl_val = 0x00000000,
>> +	.test_ctl_hi_val = 0x00000003,
>> +	.test_ctl_hi1_val = 0x00009000,
>> +	.test_ctl_hi2_val = 0x00000034,
>> +	.user_ctl_val = 0x00000000,
>> +	.user_ctl_hi_val = 0x00000005,
>> +};
>> +
>> +static struct clk_alpha_pll video_cc_pll0 = {
>> +	.offset = 0x0,
>> +	.config = &video_cc_pll0_config,
>> +	.vco_table = lucid_ole_vco,
>> +	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>> +	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
>> +	.clkr = {
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_pll0",
>> +			.parent_data = &(const struct clk_parent_data) {
>> +				.index = DT_BI_TCXO,
>> +			},
>> +			.num_parents = 1,
>> +			.ops = &clk_alpha_pll_lucid_evo_ops,
>> +		},
>> +	},
>> +};
>> +
>> +/* 1050.0 MHz Configuration */
>> +static const struct alpha_pll_config video_cc_pll1_config = {
>> +	.l = 0x36,
>> +	.alpha = 0xb000,
>> +	.config_ctl_val = 0x20485699,
>> +	.config_ctl_hi_val = 0x00182261,
>> +	.config_ctl_hi1_val = 0x82aa299c,
>> +	.test_ctl_val = 0x00000000,
>> +	.test_ctl_hi_val = 0x00000003,
>> +	.test_ctl_hi1_val = 0x00009000,
>> +	.test_ctl_hi2_val = 0x00000034,
>> +	.user_ctl_val = 0x00000000,
>> +	.user_ctl_hi_val = 0x00000005,
>> +};
>> +
>> +static struct clk_alpha_pll video_cc_pll1 = {
>> +	.offset = 0x1000,
>> +	.config = &video_cc_pll1_config,
>> +	.vco_table = lucid_ole_vco,
>> +	.num_vco = ARRAY_SIZE(lucid_ole_vco),
>> +	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE],
>> +	.clkr = {
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_pll1",
>> +			.parent_data = &(const struct clk_parent_data) {
>> +				.index = DT_BI_TCXO,
>> +			},
>> +			.num_parents = 1,
>> +			.ops = &clk_alpha_pll_lucid_evo_ops,
>> +		},
>> +	},
>> +};
>> +
>> +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[] = {
>> +	{ .index = DT_BI_TCXO },
>> +};
>> +
>> +static const struct parent_map video_cc_parent_map_1[] = {
>> +	{ P_BI_TCXO, 0 },
>> +	{ P_VIDEO_CC_PLL0_OUT_MAIN, 1 },
>> +};
>> +
>> +static const struct clk_parent_data video_cc_parent_data_1[] = {
>> +	{ .index = DT_BI_TCXO },
>> +	{ .hw = &video_cc_pll0.clkr.hw },
>> +};
>> +
>> +static const struct parent_map video_cc_parent_map_2[] = {
>> +	{ P_BI_TCXO, 0 },
>> +	{ P_VIDEO_CC_PLL1_OUT_MAIN, 1 },
>> +};
>> +
>> +static const struct clk_parent_data video_cc_parent_data_2[] = {
>> +	{ .index = DT_BI_TCXO },
>> +	{ .hw = &video_cc_pll1.clkr.hw },
>> +};
>> +
>> +static const struct freq_tbl ftbl_video_cc_mvs0_bse_clk_src[] = {
>> +	F(420000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
>> +	F(600000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
>> +	F(670000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
>> +	F(848000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
>> +	F(920000000, P_VIDEO_CC_PLL0_OUT_MAIN, 1, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 video_cc_mvs0_bse_clk_src = {
>> +	.cmd_rcgr = 0x8154,
>> +	.mnd_width = 0,
>> +	.hid_width = 5,
>> +	.parent_map = video_cc_parent_map_1,
>> +	.freq_tbl = ftbl_video_cc_mvs0_bse_clk_src,
>> +	.hw_clk_ctrl = true,
>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs0_bse_clk_src",
>> +		.parent_data = video_cc_parent_data_1,
>> +		.num_parents = ARRAY_SIZE(video_cc_parent_data_1),
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_rcg2_shared_ops,
>> +	},
>> +};
>> +
>> +static const struct freq_tbl ftbl_video_cc_mvs0_clk_src[] = {
>> +	F(210000000, P_VIDEO_CC_PLL0_OUT_MAIN, 2, 0, 0),
>> +	F(300000000, P_VIDEO_CC_PLL0_OUT_MAIN, 2, 0, 0),
>> +	F(335000000, P_VIDEO_CC_PLL0_OUT_MAIN, 2, 0, 0),
>> +	F(424000000, P_VIDEO_CC_PLL0_OUT_MAIN, 2, 0, 0),
>> +	F(460000000, P_VIDEO_CC_PLL0_OUT_MAIN, 2, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 video_cc_mvs0_clk_src = {
>> +	.cmd_rcgr = 0x8000,
>> +	.mnd_width = 0,
>> +	.hid_width = 5,
>> +	.parent_map = video_cc_parent_map_1,
>> +	.freq_tbl = ftbl_video_cc_mvs0_clk_src,
>> +	.hw_clk_ctrl = true,
> 
> sm8550/x1e don't enable hw_clk_ctrl here. Is this an expected
> difference?
> 
>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs0_clk_src",
>> +		.parent_data = video_cc_parent_data_1,
>> +		.num_parents = ARRAY_SIZE(video_cc_parent_data_1),
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_rcg2_shared_ops,
>> +	},
>> +};
>> +
>> +static const struct freq_tbl ftbl_video_cc_mvs1_clk_src[] = {
>> +	F(1050000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
>> +	F(1350000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
>> +	F(1650000000, P_VIDEO_CC_PLL1_OUT_MAIN, 1, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 video_cc_mvs1_clk_src = {
>> +	.cmd_rcgr = 0x8018,
>> +	.mnd_width = 0,
>> +	.hid_width = 5,
>> +	.parent_map = video_cc_parent_map_2,
>> +	.freq_tbl = ftbl_video_cc_mvs1_clk_src,
>> +	.hw_clk_ctrl = true,
> 
> sm8550/x1e don't enable hw_clk_ctrl here. Is this an expected
> difference?
> 

It is recommended to set HW_CLK_CONTROL bit for all RCG's, hence
hw_clk_ctrl here is expected only. Will check the applicability
on other targets also and post the changes accordingly.

>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs1_clk_src",
>> +		.parent_data = video_cc_parent_data_2,
>> +		.num_parents = ARRAY_SIZE(video_cc_parent_data_2),
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_rcg2_shared_ops,
>> +	},
>> +};
>> +
>> +static const struct freq_tbl ftbl_video_cc_xo_clk_src[] = {
>> +	F(19200000, P_BI_TCXO, 1, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 video_cc_xo_clk_src = {
>> +	.cmd_rcgr = 0x810c,
>> +	.mnd_width = 0,
>> +	.hid_width = 5,
>> +	.parent_map = video_cc_parent_map_0,
>> +	.freq_tbl = ftbl_video_cc_xo_clk_src,
>> +	.clkr.hw.init = &(const 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_SET_RATE_PARENT,
>> +		.ops = &clk_rcg2_shared_ops,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_div video_cc_mvs0_bse_div4_div_clk_src = {
>> +	.reg = 0x817c,
>> +	.shift = 0,
>> +	.width = 4,
>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs0_bse_div4_div_clk_src",
>> +		.parent_hws = (const struct clk_hw*[]) {
>> +			&video_cc_mvs0_bse_clk_src.clkr.hw,
>> +		},
>> +		.num_parents = 1,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_regmap_div_ro_ops,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_div video_cc_mvs1_div_clk_src = {
>> +	.reg = 0x80ec,
>> +	.shift = 0,
>> +	.width = 4,
>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs1_div_clk_src",
>> +		.parent_hws = (const struct clk_hw*[]) {
>> +			&video_cc_mvs1_clk_src.clkr.hw,
>> +		},
>> +		.num_parents = 1,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_regmap_div_ro_ops,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_div video_cc_mvs1c_div2_div_clk_src = {
>> +	.reg = 0x809c,
>> +	.shift = 0,
>> +	.width = 4,
>> +	.clkr.hw.init = &(const struct clk_init_data) {
>> +		.name = "video_cc_mvs1c_div2_div_clk_src",
>> +		.parent_hws = (const struct clk_hw*[]) {
>> +			&video_cc_mvs1_clk_src.clkr.hw,
>> +		},
>> +		.num_parents = 1,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +		.ops = &clk_regmap_div_ro_ops,
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs0_bse_clk = {
>> +	.halt_reg = 0x8170,
>> +	.halt_check = BRANCH_HALT,
>> +	.clkr = {
>> +		.enable_reg = 0x8170,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs0_bse_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_mvs0_bse_div4_div_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs0_clk = {
>> +	.halt_reg = 0x80b8,
>> +	.halt_check = BRANCH_HALT_VOTED,
> 
> x1e has BRANCH_HALT_SKIP here. Is there a reason for the difference?
> 

Ideally for SM8550/X1E also, it should be BRANCH_HALT_VOTED. Will check
internally and post BRANCH_HALT_VOTED for SM8550/X1E also.

>> +	.hwcg_reg = 0x80b8,
>> +	.hwcg_bit = 1,
>> +	.clkr = {
>> +		.enable_reg = 0x80b8,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs0_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_mvs0_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs0_shift_clk = {
>> +	.halt_reg = 0x8128,
>> +	.halt_check = BRANCH_HALT_VOTED,
> 
> No .hwcg_reg?
> 

Purwa doesn't have HW_CTRL bit in this CBCR, so hwcg_reg is not required.


>> +	.clkr = {
>> +		.enable_reg = 0x8128,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs0_shift_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_xo_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs0c_clk = {
>> +	.halt_reg = 0x8064,
>> +	.halt_check = BRANCH_HALT,
>> +	.clkr = {
>> +		.enable_reg = 0x8064,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs0c_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_mvs0_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs0c_shift_clk = {
>> +	.halt_reg = 0x812c,
>> +	.halt_check = BRANCH_HALT_VOTED,
>> +	.clkr = {
>> +		.enable_reg = 0x812c,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs0c_shift_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_xo_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs1_clk = {
>> +	.halt_reg = 0x80e0,
>> +	.halt_check = BRANCH_HALT_VOTED,
>> +	.hwcg_reg = 0x80e0,
>> +	.hwcg_bit = 1,
>> +	.clkr = {
>> +		.enable_reg = 0x80e0,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs1_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_mvs1_div_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs1_shift_clk = {
>> +	.halt_reg = 0x8130,
>> +	.halt_check = BRANCH_HALT_VOTED,
>> +	.clkr = {
>> +		.enable_reg = 0x8130,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs1_shift_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_xo_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs1c_clk = {
>> +	.halt_reg = 0x8090,
>> +	.halt_check = BRANCH_HALT,
>> +	.clkr = {
>> +		.enable_reg = 0x8090,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs1c_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_mvs1c_div2_div_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_branch video_cc_mvs1c_shift_clk = {
>> +	.halt_reg = 0x8134,
>> +	.halt_check = BRANCH_HALT_VOTED,
>> +	.clkr = {
>> +		.enable_reg = 0x8134,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "video_cc_mvs1c_shift_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&video_cc_xo_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct gdsc video_cc_mvs0c_gdsc = {
>> +	.gdscr = 0x804c,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0x6,
>> +	.pd = {
>> +		.name = "video_cc_mvs0c_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct gdsc video_cc_mvs0_gdsc = {
>> +	.gdscr = 0x80a4,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0x6,
>> +	.pd = {
>> +		.name = "video_cc_mvs0_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +	.parent = &video_cc_mvs0c_gdsc.pd,
>> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct gdsc video_cc_mvs1c_gdsc = {
>> +	.gdscr = 0x8078,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0xf,
> 
> x1e has 0x6 here. Is this one correct?
> 

These fields are taken from HW default PoR values, and
yes on Purwa the default PoR value is 0xf, while it is
0x6 on X1E, so this is expected only.

>> +	.pd = {
>> +		.name = "video_cc_mvs1c_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct gdsc video_cc_mvs1_gdsc = {
>> +	.gdscr = 0x80cc,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0xf,
> 
> And here.
> 

Same as above

>> +	.pd = {
>> +		.name = "video_cc_mvs1_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +	.parent = &video_cc_mvs1c_gdsc.pd,
>> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct clk_regmap *video_cc_x1p42100_clocks[] = {
>> +	[VIDEO_CC_MVS0_BSE_CLK] = &video_cc_mvs0_bse_clk.clkr,
>> +	[VIDEO_CC_MVS0_BSE_CLK_SRC] = &video_cc_mvs0_bse_clk_src.clkr,
>> +	[VIDEO_CC_MVS0_BSE_DIV4_DIV_CLK_SRC] = &video_cc_mvs0_bse_div4_div_clk_src.clkr,
>> +	[VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
>> +	[VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
>> +	[VIDEO_CC_MVS0_SHIFT_CLK] = &video_cc_mvs0_shift_clk.clkr,
>> +	[VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
>> +	[VIDEO_CC_MVS0C_SHIFT_CLK] = &video_cc_mvs0c_shift_clk.clkr,
>> +	[VIDEO_CC_MVS1_CLK] = &video_cc_mvs1_clk.clkr,
>> +	[VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
>> +	[VIDEO_CC_MVS1_DIV_CLK_SRC] = &video_cc_mvs1_div_clk_src.clkr,
>> +	[VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
>> +	[VIDEO_CC_MVS1C_CLK] = &video_cc_mvs1c_clk.clkr,
>> +	[VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC] = &video_cc_mvs1c_div2_div_clk_src.clkr,
>> +	[VIDEO_CC_MVS1C_SHIFT_CLK] = &video_cc_mvs1c_shift_clk.clkr,
>> +	[VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
>> +	[VIDEO_CC_PLL1] = &video_cc_pll1.clkr,
>> +	[VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
>> +};
>> +
>> +static struct gdsc *video_cc_x1p42100_gdscs[] = {
>> +	[VIDEO_CC_MVS0_GDSC] = &video_cc_mvs0_gdsc,
>> +	[VIDEO_CC_MVS0C_GDSC] = &video_cc_mvs0c_gdsc,
>> +	[VIDEO_CC_MVS1_GDSC] = &video_cc_mvs1_gdsc,
>> +	[VIDEO_CC_MVS1C_GDSC] = &video_cc_mvs1c_gdsc,
>> +};
>> +
>> +static const struct qcom_reset_map video_cc_x1p42100_resets[] = {
>> +	[CVP_VIDEO_CC_INTERFACE_BCR] = { 0x80f0 },
>> +	[CVP_VIDEO_CC_MVS0_BCR] = { 0x80a0 },
>> +	[CVP_VIDEO_CC_MVS0C_BCR] = { 0x8048 },
>> +	[CVP_VIDEO_CC_MVS1_BCR] = { 0x80c8 },
>> +	[CVP_VIDEO_CC_MVS1C_BCR] = { 0x8074 },
>> +	[VIDEO_CC_MVS0_BSE_BCR] = { 0x816c },
>> +	[VIDEO_CC_MVS0C_CLK_ARES] = { 0x8064, 2 },
>> +	[VIDEO_CC_MVS1C_CLK_ARES] = { 0x8090, 2 },
>> +	[VIDEO_CC_XO_CLK_ARES] = { 0x8124, 2 },
> 
> C99 for non-single-reg entries, please. SM8550 also hase udelay here, is
> there a reason for not having it here?
> 

I will add the delay and non-single-reg entries similar to SM8550.

Thanks,
Jagadeesh

>> +};
>> +
>> +static struct clk_alpha_pll *video_cc_x1p42100_plls[] = {
>> +	&video_cc_pll0,
>> +	&video_cc_pll1,
>> +};
>> +
>> +static u32 video_cc_x1p42100_critical_cbcrs[] = {
>> +	0x80f4, /* VIDEO_CC_AHB_CLK */
>> +	0x8150, /* VIDEO_CC_SLEEP_CLK */
>> +	0x8124, /* VIDEO_CC_XO_CLK */
>> +};
>> +
>> +static const struct regmap_config video_cc_x1p42100_regmap_config = {
>> +	.reg_bits = 32,
>> +	.reg_stride = 4,
>> +	.val_bits = 32,
>> +	.max_register = 0x9f54,
>> +	.fast_io = true,
>> +};
>> +
>> +static struct qcom_cc_driver_data video_cc_x1p42100_driver_data = {
>> +	.alpha_plls = video_cc_x1p42100_plls,
>> +	.num_alpha_plls = ARRAY_SIZE(video_cc_x1p42100_plls),
>> +	.clk_cbcrs = video_cc_x1p42100_critical_cbcrs,
>> +	.num_clk_cbcrs = ARRAY_SIZE(video_cc_x1p42100_critical_cbcrs),
>> +};
>> +
>> +static const struct qcom_cc_desc video_cc_x1p42100_desc = {
>> +	.config = &video_cc_x1p42100_regmap_config,
>> +	.clks = video_cc_x1p42100_clocks,
>> +	.num_clks = ARRAY_SIZE(video_cc_x1p42100_clocks),
>> +	.resets = video_cc_x1p42100_resets,
>> +	.num_resets = ARRAY_SIZE(video_cc_x1p42100_resets),
>> +	.gdscs = video_cc_x1p42100_gdscs,
>> +	.num_gdscs = ARRAY_SIZE(video_cc_x1p42100_gdscs),
>> +	.use_rpm = true,
>> +	.driver_data = &video_cc_x1p42100_driver_data,
>> +};
>> +
>> +static const struct of_device_id video_cc_x1p42100_match_table[] = {
>> +	{ .compatible = "qcom,x1p42100-videocc" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, video_cc_x1p42100_match_table);
>> +
>> +static int video_cc_x1p42100_probe(struct platform_device *pdev)
>> +{
>> +	return qcom_cc_probe(pdev, &video_cc_x1p42100_desc);
>> +}
>> +
>> +static struct platform_driver video_cc_x1p42100_driver = {
>> +	.probe = video_cc_x1p42100_probe,
>> +	.driver = {
>> +		.name = "videocc-x1p42100",
>> +		.of_match_table = video_cc_x1p42100_match_table,
>> +	},
>> +};
>> +
>> +module_platform_driver(video_cc_x1p42100_driver);
>> +
>> +MODULE_DESCRIPTION("QTI VIDEOCC X1P42100 Driver");
>> +MODULE_LICENSE("GPL");
>>
>> -- 
>> 2.34.1
>>
>