Re: [PATCH v2 07/15] clk: sunxi-ng: a523: add video mod clocks

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne petek, 14. februar 2025 ob 13:53:51 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> Add the clocks driving the various video subsystems of the SoC: the "DE"
> display engine, the "DI" deinterlacer, the "G2D" 2D graphics system, the
> Mali "GPU", the "VE" video engine, its associated IOMMU, as well as the
> clocks for the various video output drivers (HDMI, DP, LCDs).
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 219 +++++++++++++++++++++++++
>  1 file changed, 219 insertions(+)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> index 59f45e7c0904b..0ef1fd71a1ca5 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> @@ -364,6 +364,192 @@ static SUNXI_CCU_M_DATA_WITH_MUX(apb1_clk, "apb1", apb1_parents, 0x524,
>  				 24, 3,		/* mux */
>  				 0);
>  
> +
> +/**************************************************************************
> + *                          mod clocks                                    *
> + **************************************************************************/
> +
> +static const struct clk_hw *de_parents[] = {
> +	&pll_periph0_300M_clk.hw,
> +	&pll_periph0_400M_clk.hw,
> +	&pll_video3_4x_clk.common.hw,
> +	&pll_video3_3x_clk.hw,
> +};
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(de_clk, "de", de_parents, 0x600,
> +				    0, 5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static const struct clk_hw *di_parents[] = {
> +	&pll_periph0_300M_clk.hw,
> +	&pll_periph0_400M_clk.hw,
> +	&pll_video0_4x_clk.common.hw,
> +	&pll_video1_4x_clk.common.hw,
> +};
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(di_clk, "di", di_parents, 0x620,
> +				    0, 5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static const struct clk_hw *g2d_parents[] = {
> +	&pll_periph0_400M_clk.hw,
> +	&pll_periph0_300M_clk.hw,
> +	&pll_video0_4x_clk.common.hw,
> +	&pll_video1_4x_clk.common.hw,
> +};
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(g2d_clk, "g2d", g2d_parents, 0x630,
> +				    0, 5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    0);
> +
> +static const struct clk_hw *gpu_parents[] = {
> +	&pll_gpu_clk.common.hw,
> +	&pll_periph0_800M_clk.common.hw,
> +	&pll_periph0_600M_clk.hw,
> +	&pll_periph0_400M_clk.hw,
> +	&pll_periph0_300M_clk.hw,
> +	&pll_periph0_200M_clk.hw,
> +};
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(gpu_clk, "gpu", gpu_parents, 0x670,
> +				    0, 4,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    0);

GPU clock should have CLK_SET_RATE_FLAG.

> +
> +static const struct clk_hw *ve_parents[] = {
> +	&pll_ve_clk.common.hw,
> +	&pll_periph0_480M_clk.common.hw,
> +	&pll_periph0_400M_clk.hw,
> +	&pll_periph0_300M_clk.hw,
> +};
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
> +				    0, 5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static const struct clk_parent_data iommu_parents[] = {
> +	{ .hw = &pll_periph0_600M_clk.hw },
> +	{ .hw = &pll_ddr0_clk.common.hw },
> +	{ .hw = &pll_periph0_480M_clk.common.hw },
> +	{ .hw = &pll_periph0_400M_clk.hw },
> +	{ .hw = &pll_periph0_150M_clk.hw },
> +	{ .fw_name = "hosc" },
> +};
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(iommu_clk, "iommu", iommu_parents, 0x7b0,
> +				      0, 5,	/* M */
> +				      24, 3,	/* mux */
> +				      BIT(31),	/* gate */
> +				      CLK_SET_RATE_PARENT);

This won't work. IOMMU clock has also update bit, which must be set to actually
apply the new value, same as DDR clock.

> +
> +static SUNXI_CCU_GATE_DATA(hdmi_24M_clk, "hdmi-24M", osc24M, 0xb04, BIT(31), 0);
> +
> +/* TODO: add mux between 32kOSC and PERIPH0/18750 */

Not sure what this TODO means.

> +static SUNXI_CCU_GATE_HWS_WITH_PREDIV(hdmi_cec_32k_clk, "hdmi-cec-32k",
> +				      pll_periph0_2x_hws,
> +				      0xb10, BIT(30), 36621, 0);
> +
> +static const struct clk_parent_data hdmi_cec_parents[] = {
> +	{ .fw_name = "losc" },
> +	{ .hw = &hdmi_cec_32k_clk.common.hw },
> +};
> +static SUNXI_CCU_MUX_DATA_WITH_GATE(hdmi_cec_clk, "hdmi-cec", hdmi_cec_parents,
> +				    0xb10,
> +				    24, 1,	/* mux */
> +				    BIT(31),	/* gate */
> +				    0);
> +
> +static const struct clk_parent_data mipi_dsi_parents[] = {
> +	{ .fw_name = "hosc" },
> +	{ .hw = &pll_periph0_200M_clk.hw },
> +	{ .hw = &pll_periph0_150M_clk.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(mipi_dsi0_clk, "mipi-dsi0",
> +				      mipi_dsi_parents, 0xb24,
> +				      0, 5,	/* M */
> +				      24, 3,	/* mux */
> +				      BIT(31),	/* gate */
> +				      CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(mipi_dsi1_clk, "mipi-dsi1",
> +				      mipi_dsi_parents, 0xb28,
> +				      0, 5,	/* M */
> +				      24, 3,	/* mux */
> +				      BIT(31),	/* gate */
> +				      CLK_SET_RATE_PARENT);
> +
> +static const struct clk_hw *tcon_parents[] = {
> +	&pll_video0_4x_clk.common.hw,
> +	&pll_video1_4x_clk.common.hw,
> +	&pll_video2_4x_clk.common.hw,
> +	&pll_video3_4x_clk.common.hw,
> +	&pll_periph0_2x_clk.common.hw,
> +	&pll_video0_3x_clk.hw,
> +	&pll_video1_3x_clk.hw,
> +};
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(tcon_lcd0_clk, "tcon-lcd0", tcon_parents,
> +				    0xb60,
> +				    0,  5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(tcon_lcd1_clk, "tcon-lcd1", tcon_parents,
> +				    0xb64,
> +				    0,  5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);

Missing tcon-lcd2 - see T527 manual.

> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(combophy_dsi0_clk, "combophy-dsi0",
> +				    tcon_parents, 0xb6c,
> +				    0,  5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(combophy_dsi1_clk, "combophy-dsi1",
> +				    tcon_parents, 0xb70,
> +				    0,  5,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(tcon_tv0_clk, "tcon-tv0", tcon_parents,
> +				    0xb80,
> +				    0, 4,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);
> +
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(tcon_tv1_clk, "tcon-tv1", tcon_parents,
> +				    0xb84,
> +				    0, 4,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    CLK_SET_RATE_PARENT);

TCON TV0-1 parents are subset of others, according to T527 manual.

> +
> +static const struct clk_hw *edp_parents[] = {
> +	&pll_video0_4x_clk.common.hw,
> +	&pll_video1_4x_clk.common.hw,
> +	&pll_video2_4x_clk.common.hw,
> +	&pll_video3_4x_clk.common.hw,
> +	&pll_periph0_2x_clk.common.hw,
> +};
> +static SUNXI_CCU_M_HW_WITH_MUX_GATE(edp_clk, "edp", edp_parents, 0xbb0,
> +				    0, 4,	/* M */
> +				    24, 3,	/* mux */
> +				    BIT(31),	/* gate */
> +				    0);
> +

Missing CLK_SET_RATE_PARENT flag.

Best regards,
Jernej

>  /*
>   * Contains all clocks that are controlled by a hardware register. They
>   * have a (sunxi) .common member, which needs to be initialised by the common
> @@ -394,6 +580,22 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
>  	&ahb_clk.common,
>  	&apb0_clk.common,
>  	&apb1_clk.common,
> +	&de_clk.common,
> +	&di_clk.common,
> +	&g2d_clk.common,
> +	&gpu_clk.common,
> +	&ve_clk.common,
> +	&iommu_clk.common,
> +	&hdmi_24M_clk.common,
> +	&hdmi_cec_32k_clk.common,
> +	&hdmi_cec_clk.common,
> +	&mipi_dsi0_clk.common,
> +	&mipi_dsi1_clk.common,
> +	&tcon_lcd0_clk.common,
> +	&tcon_lcd1_clk.common,
> +	&tcon_tv0_clk.common,
> +	&tcon_tv1_clk.common,
> +	&edp_clk.common,
>  };
>  
>  static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> @@ -442,6 +644,23 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
>  		[CLK_AHB]		= &ahb_clk.common.hw,
>  		[CLK_APB0]		= &apb0_clk.common.hw,
>  		[CLK_APB1]		= &apb1_clk.common.hw,
> +		[CLK_DE]		= &de_clk.common.hw,
> +		[CLK_DI]		= &di_clk.common.hw,
> +		[CLK_G2D]		= &g2d_clk.common.hw,
> +		[CLK_GPU]		= &gpu_clk.common.hw,
> +		[CLK_VE]		= &ve_clk.common.hw,
> +		[CLK_HDMI_24M]		= &hdmi_24M_clk.common.hw,
> +		[CLK_HDMI_CEC_32K]	= &hdmi_cec_32k_clk.common.hw,
> +		[CLK_HDMI_CEC]		= &hdmi_cec_clk.common.hw,
> +		[CLK_MIPI_DSI0]		= &mipi_dsi0_clk.common.hw,
> +		[CLK_MIPI_DSI1]		= &mipi_dsi1_clk.common.hw,
> +		[CLK_TCON_LCD0]		= &tcon_lcd0_clk.common.hw,
> +		[CLK_TCON_LCD1]		= &tcon_lcd1_clk.common.hw,
> +		[CLK_COMBOPHY_DSI0]	= &combophy_dsi0_clk.common.hw,
> +		[CLK_COMBOPHY_DSI1]	= &combophy_dsi1_clk.common.hw,
> +		[CLK_TCON_TV0]		= &tcon_tv0_clk.common.hw,
> +		[CLK_TCON_TV1]		= &tcon_tv1_clk.common.hw,
> +		[CLK_EDP]		= &edp_clk.common.hw,
>  	},
>  };
>  
>