Re: [RFC PATCH 3/3] clk: imx: clk-fracn-gppll: Support dynamic rates

[Peng Fan <peng.fan@oss.nxp.com>]

On Mon, Feb 10, 2025 at 05:00:11PM +0100, Alexander Stein wrote:
>The fracn gppll 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:
>
>- The rate table is searched for suitable rates, so for standard rates the
>  same settings are used as without this patch
>- Then try to only adjust mfn, on fractional PLLs only, which specifies
>  the fractional part of the PLL. This setting can be changed without
>  glitches on the output and is therefore preferred
>- As a last resort the best settings are calculated dynamically
>
>Implementation is inspired by commit b09c68dc57c9d ("clk: imx: pll14xx:
>Support dynamic rates")

Not a fan of this, we have seen issues that not able to calculate
out exact audio frequency with dynamic rates. But anyway
if your setup needs this feature, it is ok to add.

>
>Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>---
>This is the first time I'm touching PLL code, I might be missing things
>or not being aware of important aspects when it comes to PLL.
>Thus this is a RFC
>
> drivers/clk/imx/clk-fracn-gppll.c | 203 ++++++++++++++++++++++++++----
> 1 file changed, 181 insertions(+), 22 deletions(-)
>
>diff --git a/drivers/clk/imx/clk-fracn-gppll.c b/drivers/clk/imx/clk-fracn-gppll.c
>index a7d57fbe93196..68c5b4a95efbe 100644
>--- a/drivers/clk/imx/clk-fracn-gppll.c
>+++ b/drivers/clk/imx/clk-fracn-gppll.c
>@@ -25,6 +25,12 @@
> 
> #define PLL_NUMERATOR		0x40
> #define PLL_MFN_MASK		GENMASK(31, 2)
>+#define PLL_MFI_MIN		0x2
>+#define PLL_MFI_MAX		0x1ff
>+#define PLL_MFN_MIN		0x0
>+#define PLL_MFN_MAX		0x3fffffff
>+#define PLL_MFD_MIN		0x1
>+#define PLL_MFD_MAX		0x3fffffff
> 
> #define PLL_DENOMINATOR		0x50
> #define PLL_MFD_MASK		GENMASK(29, 0)
>@@ -134,21 +140,6 @@ imx_get_pll_settings(struct clk_fracn_gppll *pll, unsigned long rate)
> 	return NULL;
> }
> 
>-static long clk_fracn_gppll_round_rate(struct clk_hw *hw, unsigned long rate,
>-				       unsigned long *prate)
>-{
>-	struct clk_fracn_gppll *pll = to_clk_fracn_gppll(hw);
>-	const struct imx_fracn_gppll_rate_table *rate_table = pll->rate_table;
>-	int i;
>-
>-	/* Assuming rate_table is in descending order */
>-	for (i = 0; i < pll->rate_count; i++)
>-		if (rate >= rate_table[i].rate)
>-			return rate_table[i].rate;
>-
>-	/* return minimum supported value */
>-	return rate_table[pll->rate_count - 1].rate;
>-}
> 
> static long clk_fracn_gppll_calc_rate(struct clk_fracn_gppll *pll, u32 mfn,
> 				      u32 mfd, u32 mfi, u32 rdiv, u32 odiv,
>@@ -202,6 +193,174 @@ static long clk_fracn_gppll_calc_rate(struct clk_fracn_gppll *pll, u32 mfn,
> 	return (unsigned long)fvco;
> }
> 
>+static u32 clk_fracn_gppll_calc_mfi(int rdiv, unsigned long fvco,
>+				    unsigned long fref)
>+{
>+	u32 mfi;
>+
>+	/* fvco = fref / rdiv * mfi */
>+	mfi = DIV_ROUND_CLOSEST(fvco * rdiv, fref);
>+	return clamp_t(u32, mfi, PLL_MFI_MIN, PLL_MFI_MAX);
>+}
>+
>+static long clk_fracn_gppll_calc_mfn(int mfd, int mfi, int odiv, int rdiv,
>+				     unsigned long rate, unsigned long prate)
>+{
>+	unsigned long tmp;
>+	long mfn;
>+
>+	/* calc mfn = ((rate * odiv * rdiv / prate) - mfi) * mfd */
>+	tmp = rate * odiv * rdiv - (mfi * prate);
>+	mfn = DIV_ROUND_CLOSEST(tmp * mfd, prate);
>+	return clamp_t(long, mfn, PLL_MFN_MIN, PLL_MFN_MAX);
>+}
>+
>+static void clk_fracn_gppll_calc_settings(struct clk_fracn_gppll *pll, unsigned long rate,
>+					  unsigned long prate, struct imx_fracn_gppll_rate_table *t)
>+{
>+	u32 pll_denominator, pll_numerator, pll_div;
>+	u32 mfi, mfn, mfd, rdiv, odiv;
>+	long fout, rate_min, rate_max, dist, best = LONG_MAX;
>+	const struct imx_fracn_gppll_rate_table *tt;
>+
>+	/*
>+	 * PLL constrains:
>+	 *
>+	 * a) 1 <= MFN <= 0x3fffffff (fractional only)
>+	 * b) 1 <= MFD <= 0x3fffffff (fractional only)
>+	 * c) 2 <= MFI <= 0x1ff
>+	 * d) 1 <= RDIV <= 7
>+	 * e) 2 <= ODIV <= 255
>+	 * f) -2 <= MFN/MFD <= 2
>+	 * g) 20MHz <= (Fref / rdiv) <= 40MHz
>+	 * h) 2.5GHz <= Fvco <= 5Ghz
>+	 *
>+	 * Fvco = (Fref / rdiv) * (MFI + MFN / MFD)
>+	 * Fout = Fvco / odiv
>+	 */
>+
>+	/* First try if we can get the desired rate from one of the static entries */
>+	tt = imx_get_pll_settings(pll, rate);
>+	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->mfi = tt->mfi;
>+		t->mfn = tt->mfn;
>+		t->mfd = tt->mfd;
>+		t->rdiv = tt->rdiv;
>+		t->odiv = tt->odiv;
>+		return;
>+	}
>+
>+	/* glitch free MFN adjustment only for fractional PLL */
>+	if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
>+		pll_numerator = readl_relaxed(pll->base + PLL_NUMERATOR);
>+
>+		pll_denominator = readl_relaxed(pll->base + PLL_DENOMINATOR);
>+		mfd = FIELD_GET(PLL_MFD_MASK, pll_denominator);
>+
>+		pll_div = readl_relaxed(pll->base + PLL_DIV);
>+		mfi = FIELD_GET(PLL_MFI_MASK, pll_div);
>+		rdiv = FIELD_GET(PLL_RDIV_MASK, pll_div);
>+		odiv = FIELD_GET(PLL_ODIV_MASK, pll_div);
>+
>+		/* Then see if we can get the desired rate by only adjusting MFN (glitch free) */
>+		rate_min = clk_fracn_gppll_calc_rate(pll, PLL_MFN_MIN, mfd, mfi, rdiv, odiv, prate);
>+		rate_max = clk_fracn_gppll_calc_rate(pll, PLL_MFN_MAX, mfd, mfi, rdiv, odiv, prate);
>+
>+		if (rate >= rate_min && rate <= rate_max) {
>+			mfn = clk_fracn_gppll_calc_mfn(mfd, mfi, odiv, rdiv, rate, prate);
>+			pr_debug("%s: in=%ld, want=%ld Only adjust mfn %ld -> %d\n",
>+				 clk_hw_get_name(&pll->hw), prate, rate,
>+				 FIELD_GET(PLL_MFN_MASK, pll_numerator), mfn);
>+			fout = clk_fracn_gppll_calc_rate(pll, mfn, mfd, mfi, rdiv, odiv, prate);
>+			t->rate = (unsigned int)fout;
>+			t->mfi = mfi;
>+			t->mfn = mfn;
>+			t->mfd = mfd;
>+			t->rdiv = rdiv;
>+			t->odiv = odiv;
>+			return;
>+		}
>+	}
>+
>+	/* Finally calculate best values */
>+	for (rdiv = 1; rdiv <= 7; rdiv++) {
>+		if ((prate / rdiv) < 20000000)
>+			continue;
>+		if ((prate / rdiv) > 40000000)
>+			continue;
>+
>+		for (odiv = 2; odiv <= 255; odiv++) {
>+			mfi = clk_fracn_gppll_calc_mfi(rdiv, rate * odiv, prate);
>+			mfd = 1;
>+			mfn = 0;
>+
>+			/* Try integer PLL part only first */
>+			fout = clk_fracn_gppll_calc_rate(pll, mfn, mfd, mfi, rdiv, odiv, prate);
>+			if (fout * odiv < 2500000000UL)
>+				continue;
>+			if (fout * odiv > 5000000000UL)
>+				continue;
>+
>+			if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
>+				if (!dist) {
>+					/* Disable fractional part upon exact match */
>+					mfd = 1;
>+					mfn = 0;
>+				} else {
>+					mfd = 100;

why hardcode mfd to 100?

>+					mfd = clamp(mfd, PLL_MFD_MIN, PLL_MFN_MAX);
>+
>+					mfn = clk_fracn_gppll_calc_mfn(mfd, mfi, odiv, rdiv, rate, prate);
>+					if ((mfn / mfd) > 2)
>+						continue;
>+
>+					fout = clk_fracn_gppll_calc_rate(pll, mfn, mfd, mfi, rdiv, odiv, prate);
>+					if (fout * odiv < 2500000000)
>+						continue;
>+					if (fout * odiv > 5000000000)
>+						continue;
>+				}
>+			} else {
>+				mfd = 0;
>+				mfn = 0;
>+			}
>+
>+			/* best match */
>+			dist = abs((long)rate - (long)fout);
>+			if (dist < best) {
>+				best = dist;
>+				t->rate = (unsigned int)fout;
>+				t->mfi = mfi;
>+				t->mfn = mfn;
>+				t->mfd = mfd;
>+				t->rdiv = rdiv;
>+				t->odiv = odiv;
>+
>+				if (!dist)
>+					goto found;
>+			}
>+		}
>+	}
>+found:
>+	pr_debug("%s: in=%lu, want=%lu got=%u (mfi=%u mfn=%u mfd=%u rdiv=%u odiv=%u)\n",
>+		 clk_hw_get_name(&pll->hw), prate, rate, t->rate, t->mfi, t->mfn, t->mfd,
>+		 t->rdiv, t->odiv);
>+}
>+
>+static long clk_fracn_gppll_round_rate(struct clk_hw *hw, unsigned long rate,
>+				       unsigned long *prate)
>+{
>+	struct clk_fracn_gppll *pll = to_clk_fracn_gppll(hw);
>+	struct imx_fracn_gppll_rate_table t;
>+
>+	clk_fracn_gppll_calc_settings(pll, rate, *prate, &t);
>+
>+	return t.rate;
>+}
>+
> static unsigned long clk_fracn_gppll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> {
> 	struct clk_fracn_gppll *pll = to_clk_fracn_gppll(hw);
>@@ -242,11 +401,11 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
> 				    unsigned long prate)
> {
> 	struct clk_fracn_gppll *pll = to_clk_fracn_gppll(hw);
>-	const struct imx_fracn_gppll_rate_table *rate;
>+	struct imx_fracn_gppll_rate_table rate;
> 	u32 tmp, pll_div, ana_mfn;
> 	int ret;
> 
>-	rate = imx_get_pll_settings(pll, drate);
>+	clk_fracn_gppll_calc_settings(pll, drate, prate, &rate);
> 
> 	/* Hardware control select disable. PLL is control by register */
> 	tmp = readl_relaxed(pll->base + PLL_CTRL);
>@@ -266,13 +425,13 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
> 	tmp &= ~CLKMUX_BYPASS;
> 	writel_relaxed(tmp, pll->base + PLL_CTRL);

As Sascha mentioned, set rate will first disable output, power down pll based
on current implementation. So this patch might not get tested.

Regards
Peng

> 
>-	pll_div = FIELD_PREP(PLL_RDIV_MASK, rate->rdiv) | rate->odiv |
>-		FIELD_PREP(PLL_MFI_MASK, rate->mfi);
>+	pll_div = FIELD_PREP(PLL_RDIV_MASK, rate.rdiv) | rate.odiv |
>+		FIELD_PREP(PLL_MFI_MASK, rate.mfi);
> 	writel_relaxed(pll_div, pll->base + PLL_DIV);
> 	readl(pll->base + PLL_DIV);
> 	if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
>-		writel_relaxed(rate->mfd, pll->base + PLL_DENOMINATOR);
>-		writel_relaxed(FIELD_PREP(PLL_MFN_MASK, rate->mfn), pll->base + PLL_NUMERATOR);
>+		writel_relaxed(rate.mfd, pll->base + PLL_DENOMINATOR);
>+		writel_relaxed(FIELD_PREP(PLL_MFN_MASK, rate.mfn), pll->base + PLL_NUMERATOR);
> 		readl(pll->base + PLL_NUMERATOR);
> 	}
> 
>@@ -296,7 +455,7 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
> 	ana_mfn = readl_relaxed(pll->base + PLL_STATUS);
> 	ana_mfn = FIELD_GET(PLL_MFN_MASK, ana_mfn);
> 
>-	WARN(ana_mfn != rate->mfn, "ana_mfn != rate->mfn\n");
>+	WARN(ana_mfn != rate.mfn, "ana_mfn != rate->mfn\n");
> 
> 	return 0;
> }
>-- 
>2.34.1
>