Hi Chris,
On Fri, 12 Sept 2025 at 16:22, Chris Brandt <[email protected]> wrote:
> Convert the limited MIPI clock calculations to a full range of settings
> based on math including H/W limitation validation.
> Since the required DSI division setting must be specified from external
> sources before calculations, expose a new API to set it.
>
> Signed-off-by: Chris Brandt <[email protected]>
> Signed-off-by: hienhuynh <[email protected]>
> Signed-off-by: Nghia Vo <[email protected]>
Thanks for your patch!
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -74,6 +74,22 @@
> #define MSTOP_OFF(conf) FIELD_GET(GENMASK(31, 16), (conf))
> #define MSTOP_MASK(conf) FIELD_GET(GENMASK(15, 0), (conf))
>
> +#define PLL5_FOUTVCO_MIN 800000000
> +#define PLL5_FOUTVCO_MAX 3000000000
> +#define PLL5_POSTDIV_MIN 1
> +#define PLL5_POSTDIV_MAX 7
> +#define PLL5_POSTDIV_DEF 1
> +#define PLL5_REFDIV_MIN 1
> +#define PLL5_REFDIV_MAX 2
Documentation says 1..63?
> +#define PLL5_REFDIV_DEF 1
> +#define PLL5_INTIN_MIN 20
> +#define PLL5_INTIN_MAX 320
> +#define PLL5_INTIN_DEF 125
> +#define PLL5_FRACIN_DEF 0
> +
> +#define PLL5_TARGET_DPI 0
> +#define PLL5_TARGET_DSI 1
These two should become an enum in include/linux/clk/renesas.h,
as their values are passed (as magic numbers) from outside.
> +
> /**
> * struct clk_hw_data - clock hardware data
> * @hw: clock hw
> @@ -129,6 +145,12 @@ struct rzg2l_pll5_param {
> u8 pl5_spread;
> };
>
> +/* PLL5 output will be used for DPI or MIPI-DSI */
> +static int dsi_div_target = PLL5_TARGET_DPI;
> +
> +/* Required division ratio for MIPI D-PHY clock changes depending on
> resolution and lanes. */
> +static int dsi_div_ab;
unsigned int
> +
> struct rzg2l_pll5_mux_dsi_div_param {
> u8 clksrc;
> u8 dsi_div_a;
> @@ -557,24 +579,102 @@ rzg2l_cpg_sd_mux_clk_register(const struct
> cpg_core_clk *core,
> }
>
> static unsigned long
> -rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_pll5_param *params,
> +rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_cpg_priv *priv,
> + struct rzg2l_pll5_param *params,
> unsigned long rate)
> {
> unsigned long foutpostdiv_rate, foutvco_rate;
> + u8 div = 1;
> + bool found = 0;
> + int a, b;
unsigned int
> +
> + if (priv->mux_dsi_div_params.clksrc)
> + div = 2;
> +
> + /* Calculate the DIV_DSI_A and DIV_DSI_B based on the final DIV DSI */
> + for (a = 0; a < 4; a++) {
> +
> + if (dsi_div_target == PLL5_TARGET_DPI && a == 0)
> + continue; /* 1/1 div not supported for
> DIV_DSI_A for DPI */
> +
> + for (b = 0; b < 16; b++) {
> +
> + if (dsi_div_target == PLL5_TARGET_DPI && b != 0)
> + continue; /* Only 1/1 div supported for
> DIV_DSI_B in DPI */
> +
> + if (((1 << a) * (b + 1)) == dsi_div_ab) {
"(b + 1) << a"?
> + priv->mux_dsi_div_params.dsi_div_a = a;
> + priv->mux_dsi_div_params.dsi_div_b = b;
> +
> + goto found_dsi_div;
> + }
> + }
> + }
> +
> +found_dsi_div:
> + /*
> + * Below conditions must be set for PLL5 parameters:
> + * - REFDIV must be between 1 and 2.
> + * - POSTDIV1/2 must be between 1 and 7.
> + * - INTIN must be between 20 and 320.
> + * - FOUTVCO must be between 800MHz and 3000MHz.
> + */
> + for (params->pl5_postdiv1 = PLL5_POSTDIV_MIN;
> + params->pl5_postdiv1 < PLL5_POSTDIV_MAX + 1;
> + params->pl5_postdiv1++) {
> + for (params->pl5_postdiv2 = PLL5_POSTDIV_MIN;
> + params->pl5_postdiv2 < PLL5_POSTDIV_MAX + 1;
> + params->pl5_postdiv2++) {
> + foutvco_rate = rate * ((1 <<
> priv->mux_dsi_div_params.dsi_div_a) *
rate * ... << priv->mux_dsi_div_params.dsi_div_a;
> +
> (priv->mux_dsi_div_params.dsi_div_b + 1)) *
> + div * params->pl5_postdiv1 *
> params->pl5_postdiv2;
> + if (foutvco_rate < PLL5_FOUTVCO_MIN + 1 ||
> + foutvco_rate > PLL5_FOUTVCO_MAX - 1)
> + continue;
> +
> + for (params->pl5_refdiv = PLL5_REFDIV_MIN;
> + params->pl5_refdiv < PLL5_REFDIV_MAX + 1;
> + params->pl5_refdiv++) {
> + params->pl5_intin = (foutvco_rate *
> params->pl5_refdiv) /
> + (EXTAL_FREQ_IN_MEGA_HZ *
> MEGA);
> + if (params->pl5_intin < PLL5_INTIN_MIN + 1 ||
> + params->pl5_intin > PLL5_INTIN_MAX - 1)
> + continue;
> + params->pl5_fracin = div_u64(((u64)
> + (foutvco_rate *
> params->pl5_refdiv) %
> + (EXTAL_FREQ_IN_MEGA_HZ *
> MEGA)) << 24,
Please move the cast to u64 outside the modulo operation, else
the latter becomes a 64-by-32 modulo, which requires using a helper
from <linux/math64.h> when compile-testing for a 32-bit platform.
> + EXTAL_FREQ_IN_MEGA_HZ *
> MEGA);
> + found = 1;
> + goto found_clk;
> + }
> + }
> + }
> +
> +found_clk:
> + if (!found) {
> + params->pl5_intin = PLL5_INTIN_DEF;
> + params->pl5_fracin = PLL5_FRACIN_DEF;
> + params->pl5_refdiv = PLL5_REFDIV_DEF;
> + params->pl5_postdiv1 = PLL5_POSTDIV_DEF;
> + params->pl5_postdiv2 = PLL5_POSTDIV_DEF;
> + }
>
> - params->pl5_intin = rate / MEGA;
> - params->pl5_fracin = div_u64(((u64)rate % MEGA) << 24, MEGA);
(u64)(rate % MEGA)
> - params->pl5_refdiv = 2;
> - params->pl5_postdiv1 = 1;
> - params->pl5_postdiv2 = 1;
> params->pl5_spread = 0x16;
>
> foutvco_rate = div_u64(mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA,
> - (params->pl5_intin << 24) +
> params->pl5_fracin),
> - params->pl5_refdiv) >> 24;
> + (params->pl5_intin << 24) + params->pl5_fracin),
> + params->pl5_refdiv) >> 24;
> foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate,
foutvco_rate is unsigned long, but the division uses the _ULL() macro variant?
> params->pl5_postdiv1 *
> params->pl5_postdiv2);
>
> + /* If foutvco is above 1.5GHz, change parent and recalculate */
> + if (priv->mux_dsi_div_params.clksrc && foutvco_rate > 1500000000) {
> + priv->mux_dsi_div_params.clksrc = 0;
> + dsi_div_ab *= 2;
> + dsi_div_target = PLL5_TARGET_DSI; /* Assume MIPI-DSI */
> + return rzg2l_cpg_get_foutpostdiv_rate(priv, params, rate);
> + }
> +
> return foutpostdiv_rate;
> }
>
> @@ -607,7 +707,7 @@ static unsigned long
> rzg2l_cpg_get_vclk_parent_rate(struct clk_hw *hw,
> struct rzg2l_pll5_param params;
> unsigned long parent_rate;
>
> - parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, rate);
> + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(priv, ¶ms, rate);
>
> if (priv->mux_dsi_div_params.clksrc)
> parent_rate /= 2;
> @@ -626,6 +726,13 @@ static int rzg2l_cpg_dsi_div_determine_rate(struct
> clk_hw *hw,
> return 0;
> }
>
> +void rzg2l_cpg_dsi_div_set_divider(int divider, int target)
unsigned int divider
> +{
> + dsi_div_ab = divider;
> + dsi_div_target = target;
> +}
> +EXPORT_SYMBOL_GPL(rzg2l_cpg_dsi_div_set_divider);
> +
> static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw,
> unsigned long rate,
> unsigned long parent_rate)
> @@ -859,7 +966,7 @@ static int rzg2l_cpg_sipll5_set_rate(struct clk_hw *hw,
>
> vclk_rate = rzg2l_cpg_get_vclk_rate(hw, rate);
> sipll5->foutpostdiv_rate =
> - rzg2l_cpg_get_foutpostdiv_rate(¶ms, vclk_rate);
> + rzg2l_cpg_get_foutpostdiv_rate(priv, ¶ms, vclk_rate);
>
> /* Put PLL5 into standby mode */
> writel(CPG_SIPLL5_STBY_RESETB_WEN, priv->base + CPG_SIPLL5_STBY);
> @@ -949,6 +1056,8 @@ rzg2l_cpg_sipll5_register(const struct cpg_core_clk
> *core,
> priv->mux_dsi_div_params.clksrc = 1; /* Use clk src 1 for DSI */
> priv->mux_dsi_div_params.dsi_div_a = 1; /* Divided by 2 */
> priv->mux_dsi_div_params.dsi_div_b = 2; /* Divided by 3 */
> + dsi_div_ab = (1 << priv->mux_dsi_div_params.dsi_div_a) *
> + (priv->mux_dsi_div_params.dsi_div_b + 1);
(priv->mux_dsi_div_params.dsi_div_b + 1) << priv->mux_dsi_div_params.dsi_div_a
>
> return clk_hw->clk;
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
