On Tue, 18 Feb 2025 20:26:31 +0100
Jernej Škrabec <jernej.skrabec@xxxxxxxxx> wrote:
Hi,
> 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@xxxxxxx>
> > ---
> > 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.
Yes indeed, fixed.
> > +
> > +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.
Good point, I actually missed that. I now added a new feature flag in a
new patch to flag those clocks that need bit 27 set whenever we change
something. An update bitmask, defaulting to 0, sounds more elegant, but
gets tricky because we would need that in all those clock routines (gate,
div, mux), so hardcoding bit 27 behind a flag (which is already available
to all those functions) gives a much easier implementation. Feel free to
disagree on that new patch ;-)
> > +
> > +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.
Mhh, I don't remember either, and cannot find 18750 anywhere in the
manuals. Maybe I forgot to remove this comment when implementing the below
clock? Just removed it.
> > +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.
Thanks, I added this.
> > +
> > +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.
Nice catch, fixed.
> > +
> > +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.
Yes, thanks, added.
Cheers,
Andre
>
> 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,
> > },
> > };
> >
> >
>
>
>
>
