Re: [PATCH v2 8/8] clk: imx: pll14xx: Support dynamic rates

[Abel Vesa <abel.vesa@nxp.com>]

On 22-02-25 16:13:06, Abel Vesa wrote:
> On 22-02-25 09:29:37, Sascha Hauer wrote:
> > The pll1443x PLL so far only supports rates from a rate table passed
> > during initialization. Calculating PLL settings dynamically helps audio
> > applications to get their desired rates, so support for this is added
> > in this patch.
> > 
> > The strategy to get to the PLL setting for a rate is:
> > 
> > - First try to only adjust kdiv which specifies the fractional part of the PLL.
> >   This setting can be changed without glitches on the output and is therefore
> >   preferred
> > - When that isn't possible then the rate table is searched for suitable rates,
> >   so for standard rates the same settings are used as without this patch
> > - As a last resort the best settings are calculated dynamically
> > 
> > The code in this patch is based on patches from Adrian Alonso <adrian.alonso@nxp.com>
> > and Mads Bligaard Nielsen <bli@bang-olufsen.dk>
> 
> Hmm, I wish this was also possible for SSCG plls.
> 
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/clk/imx/clk-pll14xx.c | 143 ++++++++++++++++++++++++++++++----
> >  1 file changed, 126 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> > index 646e0ce7d6242..eef0f3b693ed9 100644
> > --- a/drivers/clk/imx/clk-pll14xx.c
> > +++ b/drivers/clk/imx/clk-pll14xx.c
> > @@ -29,6 +29,8 @@
> >  #define PDIV_MASK	GENMASK(9, 4)
> >  #define SDIV_MASK	GENMASK(2, 0)
> >  #define KDIV_MASK	GENMASK(15, 0)
> > +#define KDIV_MIN	SHRT_MIN
> > +#define KDIV_MAX	SHRT_MAX
> >  
> >  #define LOCK_TIMEOUT_US		10000
> >  
> > @@ -113,7 +115,106 @@ static long pll14xx_calc_rate(struct clk_pll14xx *pll, int mdiv, int pdiv,
> >  	return fvco;
> >  }
> >  
> > -static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
> > +static long pll1443x_calc_kdiv(int mdiv, int pdiv, int sdiv,
> > +		unsigned long rate, unsigned long prate)
> > +{
> > +	long kdiv;
> > +
> > +	/* calc kdiv = round(rate * pdiv * 65536 * 2^sdiv / prate) - (mdiv * 65536) */
> > +	kdiv = ((rate * ((pdiv * 65536) << sdiv) + prate / 2) / prate) - (mdiv * 65536);
> > +
> > +	return clamp_t(short, kdiv, KDIV_MIN, KDIV_MAX);
> > +}
> > +
> > +static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rate,
> > +				      unsigned long prate, struct imx_pll14xx_rate_table *t)
> > +{
> > +	u32 pll_div_ctl0, pll_div_ctl1;
> > +	int mdiv, pdiv, sdiv, kdiv;
> > +	long fvco, rate_min, rate_max, dist, best = LONG_MAX;
> > +	const struct imx_pll14xx_rate_table *tt;
> > +
> > +	/*
> > +	 * Fractional PLL constrains:
> > +	 *
> > +	 * a) 6MHz <= prate <= 25MHz
> > +	 * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz)
> > +	 * c) 64 <= m <= 1023
> > +	 * d) 0 <= s <= 6
> > +	 * e) -32768 <= k <= 32767
> > +	 *
> > +	 * fvco = (m * 65536 + k) * prate / (p * 65536)
> > +	 */
> > +
> > +	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> > +	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
> > +	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
> > +	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
> > +	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> > +
> > +	/* First see if we can get the desired rate by only adjusting kdiv (glitch free) */
> > +	rate_min = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MIN, prate);
> > +	rate_max = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MAX, prate);
> > +
> > +	if (rate >= rate_min && rate <= rate_max) {
> > +		kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
> > +		pr_debug("%s: in=%ld, want=%ld Only adjust kdiv %ld -> %d\n",
> > +			 clk_hw_get_name(&pll->hw), prate, rate,
> > +			 FIELD_GET(KDIV_MASK, pll_div_ctl1), kdiv);
> > +		fvco = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
> > +		t->rate = (unsigned int)fvco;
> > +		t->mdiv = mdiv;
> > +		t->pdiv = pdiv;
> > +		t->sdiv = sdiv;
> > +		t->kdiv = kdiv;
> > +		return;
> > +	}
> > +
> > +	/* Then try if we can get the desired rate from one of the static entries */
> > +	tt = imx_get_pll_settings(pll, rate);
> 
> Shouldn't we try this one first? Maybe we don't need to compute kdiv at
> all.
> 

Ping.

> > +	if (tt) {
> > +		pr_debug("%s: in=%ld, want=%ld, Using PLL setting from table\n",
> > +			 clk_hw_get_name(&pll->hw), prate, rate);
> > +		t->rate = tt->rate;
> > +		t->mdiv = tt->mdiv;
> > +		t->pdiv = tt->pdiv;
> > +		t->sdiv = tt->sdiv;
> > +		t->kdiv = tt->kdiv;
> > +		return;
> > +	}
> > +
> > +	/* Finally calculate best values */
> > +	for (pdiv = 1; pdiv <= 7; pdiv++) {
> > +		for (sdiv = 0; sdiv <= 6; sdiv++) {
> > +			/* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */
> > +			mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate);
> > +			mdiv = clamp(mdiv, 64, 1023);
> > +
> > +			kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
> > +			fvco = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
> > +
> > +			/* best match */
> > +			dist = abs((long)rate - (long)fvco);
> > +			if (dist < best) {
> > +				best = dist;
> > +				t->rate = (unsigned int)fvco;
> > +				t->mdiv = mdiv;
> > +				t->pdiv = pdiv;
> > +				t->sdiv = sdiv;
> > +				t->kdiv = kdiv;
> > +
> > +				if (!dist)
> > +					goto found;
> > +			}
> > +		}
> > +	}
> > +found:
> > +	pr_debug("%s: in=%ld, want=%ld got=%d (pdiv=%d sdiv=%d mdiv=%d kdiv=%d)\n",
> > +		 clk_hw_get_name(&pll->hw), prate, rate, t->rate, t->pdiv, t->sdiv,
> > +		 t->mdiv, t->kdiv);
> > +}
> > +
> > +static long clk_pll1416x_round_rate(struct clk_hw *hw, unsigned long rate,
> >  			unsigned long *prate)
> >  {
> >  	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> > @@ -129,6 +230,17 @@ static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
> >  	return rate_table[pll->rate_count - 1].rate;
> >  }
> >  
> > +static long clk_pll1443x_round_rate(struct clk_hw *hw, unsigned long rate,
> > +			unsigned long *prate)
> > +{
> > +	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> > +	struct imx_pll14xx_rate_table t;
> > +
> > +	imx_pll14xx_calc_settings(pll, rate, *prate, &t);
> > +
> > +	return t.rate;
> > +}
> > +
> >  static unsigned long clk_pll14xx_recalc_rate(struct clk_hw *hw,
> >  						  unsigned long parent_rate)
> >  {
> > @@ -239,25 +351,21 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> >  				 unsigned long prate)
> >  {
> >  	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> > -	const struct imx_pll14xx_rate_table *rate;
> > +	struct imx_pll14xx_rate_table rate;
> >  	u32 gnrl_ctl, div_ctl0;
> >  	int ret;
> >  
> > -	rate = imx_get_pll_settings(pll, drate);
> > -	if (!rate) {
> > -		pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
> > -			drate, clk_hw_get_name(hw));
> > -		return -EINVAL;
> > -	}
> > +	imx_pll14xx_calc_settings(pll, drate, prate, &rate);
> >  
> >  	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> >  
> > -	if (!clk_pll14xx_mp_change(rate, div_ctl0)) {
> > +	if (!clk_pll14xx_mp_change(&rate, div_ctl0)) {
> > +		/* only sdiv and/or kdiv changed - no need to RESET PLL */
> >  		div_ctl0 &= ~SDIV_MASK;
> > -		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate->sdiv);
> > +		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate.sdiv);
> >  		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> >  
> > -		writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv),
> > +		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
> >  			       pll->base + DIV_CTL1);
> >  
> >  		return 0;
> > @@ -272,11 +380,12 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> >  	gnrl_ctl |= BYPASS_MASK;
> >  	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
> >  
> > -	div_ctl0 = FIELD_PREP(MDIV_MASK, rate->mdiv) |
> > -		   FIELD_PREP(PDIV_MASK, rate->pdiv) |
> > -		   FIELD_PREP(SDIV_MASK, rate->sdiv);
> > +	div_ctl0 = FIELD_PREP(MDIV_MASK, rate.mdiv) |
> > +		   FIELD_PREP(PDIV_MASK, rate.pdiv) |
> > +		   FIELD_PREP(SDIV_MASK, rate.sdiv);
> >  	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> > -	writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
> > +
> > +	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base + DIV_CTL1);
> >  
> >  	/*
> >  	 * According to SPEC, t3 - t2 need to be greater than
> > @@ -359,7 +468,7 @@ static const struct clk_ops clk_pll1416x_ops = {
> >  	.unprepare	= clk_pll14xx_unprepare,
> >  	.is_prepared	= clk_pll14xx_is_prepared,
> >  	.recalc_rate	= clk_pll14xx_recalc_rate,
> > -	.round_rate	= clk_pll14xx_round_rate,
> > +	.round_rate	= clk_pll1416x_round_rate,
> >  	.set_rate	= clk_pll1416x_set_rate,
> >  };
> >  
> > @@ -372,7 +481,7 @@ static const struct clk_ops clk_pll1443x_ops = {
> >  	.unprepare	= clk_pll14xx_unprepare,
> >  	.is_prepared	= clk_pll14xx_is_prepared,
> >  	.recalc_rate	= clk_pll14xx_recalc_rate,
> > -	.round_rate	= clk_pll14xx_round_rate,
> > +	.round_rate	= clk_pll1443x_round_rate,
> >  	.set_rate	= clk_pll1443x_set_rate,
> >  };
> >  
> > -- 
> > 2.30.2
> >