[PATCH v2 0/2] Fix RK3576/RK3588 fractional PLL rate calculation

[PATCH v2 0/2] Fix RK3576/RK3588 fractional PLL rate calculation

[Alexey Charkov]

Fractional PLL rate recalculation for RK3576/RK3588 doesn't correspond to
what the hardware actually does due to two issues: wrong denominator for
the fractional component, and wrong signedness of the fractional
component. The result is the kernel-visible PLL rate being off by about
2 MHz for the two affected PLL rates vs. what the hardware produces.

While this fixes a real bug it's not a regression, as the issue was
introduced in the same commit that added the RK3576/RK3588 support.

Note that there is a separate unrelated issue with the rate table, namely
the 2256000000 Hz entry currently leads to a VCO frequency of 4512 MHz,
which is just above the TRM-stated maximum of 4500 MHz. Also multiple
entries in the table end up with Fvco < 3 GHz, which according to the
TRM leads to a PLL period jitter of +-2% vs. the +-1% for Fvco > 3 GHz.
To be revisited and optimized separately.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
Changes in v2:
- Split out the denominator fix into its own patch (thanks Heiko)
- Reword the commit description for clarity (thanks Quentin and Sebastian)
- Add relevant clock derivation formulas and key constraints directly into
  a code comment for easier future reference
- Link to v1: https://patch.msgid.link/20260721-rk3588-fracpll-v1-1-b289bf17cf17@flipper.net

To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Brian Masney <bmasney@redhat.com>
To: Heiko Stuebner <heiko@sntech.de>
To: Elaine Zhang <zhangqing@rock-chips.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Wyon Bi <bivvy.bi@rock-chips.com>
To: Finley Xiao <finley.xiao@rock-chips.com>
To: Liang Chen <cl@rock-chips.com>
To: YouMin Chen <cym@rock-chips.com>
Cc: linux-clk@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: Detlev Casanova <detlev.casanova@collabora.com>
Cc: Dragan Simic <dsimic@manjaro.org>

---
Alexey Charkov (2):
      clk: rockchip: pll: Fix the fractional part denominator on RK3588/RK3576
      clk: rockchip: Fractional PLL coefficient on RK3588/RK3576 is two's complement

 drivers/clk/rockchip/clk-pll.c    | 16 ++++++++++++----
 drivers/clk/rockchip/clk-rk3576.c |  4 ++--
 drivers/clk/rockchip/clk-rk3588.c |  4 ++--
 drivers/clk/rockchip/clk.h        |  8 ++++----
 4 files changed, 20 insertions(+), 12 deletions(-)
---
base-commit: b4515cf4156356e8f4fe6e0fdc17f59adab9772f
change-id: 20260721-rk3588-fracpll-f4377cd80983

Best regards,
--  
Alexey Charkov <alchark@flipper.net>


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 1/2] clk: rockchip: pll: Fix the fractional part denominator on RK3588/RK3576

[Alexey Charkov]

According to the TRM, the fractional PLL coefficient should be divided by
65536 rather than 65535 to obtain the output rate.

Fix the denominator and add a comment with the TRM provided clock formulae
for future reference.

See RK3576 TRM Part 1 V1.2 section 2.13.1.4 Setting Guide on P, M, S and K
or equivalently RK3588 TRM part 1 V1.0 section 2.17.1.4 Setting Guide on P,
M, S and K.

Fractional PLL rates don't seem to be used by any current mainline
consumers, so this is purely a correctness fix. It will also be important
to properly support DisplayPort output going forward, as the video output
controller derives its pixel clock from system PLLs with no dedicated PHY
PLL option for DP unlike HDMI, and some display modes are only achievable
with fractional PLL rates.

Fixes: 8f6594494b1c ("clk: rockchip: add pll type for RK3588")
Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 drivers/clk/rockchip/clk-pll.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index 6b853800cb6b..bf8acf7cee0d 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -900,6 +900,13 @@ static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll,
 	rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK);
 }
 
+/*
+ * 2250 MHz <= Fvco <= 4500 MHz
+ * For Fvco > 3 GHz: period jitter +-1% frac PLL, +-0.75% int PLL
+ * For Fvco < 3 GHz: period jitter +-2% frac PLL, +-1.50% int PLL
+ * Fvco = ((m + k / 65536) * Fin) / p
+ * Fout = ((m + k / 65536) * Fin) / (p * 2^s)
+ */
 static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
 {
 	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
@@ -915,7 +922,7 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
 		/* fractional mode */
 		u64 frac_rate64 = prate * cur.k;
 
-		postdiv = cur.p * 65535;
+		postdiv = cur.p * 65536;
 		do_div(frac_rate64, postdiv);
 		rate64 += frac_rate64;
 	}

-- 
2.54.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/2] clk: rockchip: Fractional PLL coefficient on RK3588/RK3576 is two's complement

[Alexey Charkov]

When the PLL rates table was first committed for RK3588 (and later reused
for RK3576), the fractional PLL coefficient was defined as an unsigned
value, while the TRM clearly states that it is a two's complement 16-bit
value.

Treating the fractional PLL coefficient as unsigned in rate recalculation
results in a kernel-visible rate which deviates from what the hardware
actually generates by Fin / (p * 2^s), or 2 MHz for the two affected table
entries.

Rockchip's downstream kernel later revised the fractional PLL code [1] to
account for the two's complement nature of the coefficient, but that
change wasn't upstreamed.

Change the PLL table definition to use two's complement for the
fractional coefficient and update its users accordingly.

Note that a negative fractional coefficient is meant to be subtracted from
the next larger integer multiplier, so the m values in the table are
also adjusted accordingly for the two negative-k entries.

Rockchip's downstream commit introducing the two's complement logic for k
also does unrelated tweaks to the PLL parameters which are not explained
by the switch to the two's complement, so they are not replicated here.
If any of the parameters prove to need further tweaks (e.g. for precision
or jitter) that would better be done in targeted follow-up commits.

Fractional PLL rates don't seem to be used by any current mainline
consumers, so this is purely a correctness fix. It will also be important
to properly support DisplayPort output going forward, as the video output
controller derives its pixel clock from system PLLs with no dedicated PHY
PLL option for DP unlike HDMI, and some display modes are only achievable
using fractional PLL rates.

Link: https://github.com/flipperdevices/rockchip-linux/commit/7a72bc05dcc3a51e85ae531749e6270bf9b9212d [1]
Fixes: f1c506d152ff ("clk: rockchip: add clock controller for the RK3588")
Fixes: cc40f5baa91b ("clk: rockchip: Add clock controller for the RK3576")
Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 drivers/clk/rockchip/clk-pll.c    | 7 ++++---
 drivers/clk/rockchip/clk-rk3576.c | 4 ++--
 drivers/clk/rockchip/clk-rk3588.c | 4 ++--
 drivers/clk/rockchip/clk.h        | 8 ++++----
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index bf8acf7cee0d..706ca4b344d3 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/clk-provider.h>
 #include <linux/iopoll.h>
+#include <linux/math64.h>
 #include <linux/regmap.h>
 #include <linux/clk.h>
 #include "clk.h"
@@ -906,6 +907,7 @@ static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll,
  * For Fvco < 3 GHz: period jitter +-2% frac PLL, +-1.50% int PLL
  * Fvco = ((m + k / 65536) * Fin) / p
  * Fout = ((m + k / 65536) * Fin) / (p * 2^s)
+ * -32768 <= k <= 32767 (only available in frac PLLs, not int PLLs)
  */
 static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
 {
@@ -920,11 +922,10 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
 
 	if (cur.k) {
 		/* fractional mode */
-		u64 frac_rate64 = prate * cur.k;
+		s64 frac_rate64 = (s64)prate * cur.k;
 
 		postdiv = cur.p * 65536;
-		do_div(frac_rate64, postdiv);
-		rate64 += frac_rate64;
+		rate64 += div_s64(frac_rate64, postdiv);
 	}
 	rate64 = rate64 >> cur.s;
 
diff --git a/drivers/clk/rockchip/clk-rk3576.c b/drivers/clk/rockchip/clk-rk3576.c
index 2557358e0b9d..63f229e73a45 100644
--- a/drivers/clk/rockchip/clk-rk3576.c
+++ b/drivers/clk/rockchip/clk-rk3576.c
@@ -79,13 +79,13 @@ static struct rockchip_pll_rate_table rk3576_pll_rates[] = {
 	RK3588_PLL_RATE(1008000000, 2, 336, 2, 0),
 	RK3588_PLL_RATE(1000000000, 3, 500, 2, 0),
 	RK3588_PLL_RATE(983040000, 4, 655, 2, 23592),
-	RK3588_PLL_RATE(955520000, 3, 477, 2, 49806),
+	RK3588_PLL_RATE(955520000, 3, 478, 2, -15730),
 	RK3588_PLL_RATE(903168000, 6, 903, 2, 11009),
 	RK3588_PLL_RATE(900000000, 2, 300, 2, 0),
 	RK3588_PLL_RATE(816000000, 2, 272, 2, 0),
 	RK3588_PLL_RATE(786432000, 2, 262, 2, 9437),
 	RK3588_PLL_RATE(786000000, 1, 131, 2, 0),
-	RK3588_PLL_RATE(785560000, 3, 392, 2, 51117),
+	RK3588_PLL_RATE(785560000, 3, 393, 2, -14419),
 	RK3588_PLL_RATE(722534400, 8, 963, 2, 24850),
 	RK3588_PLL_RATE(600000000, 2, 200, 2, 0),
 	RK3588_PLL_RATE(594000000, 2, 198, 2, 0),
diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 75d42fea2a11..24baa0ef9bf3 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -79,14 +79,14 @@ static struct rockchip_pll_rate_table rk3588_pll_rates[] = {
 	RK3588_PLL_RATE(1008000000, 2, 336, 2, 0),
 	RK3588_PLL_RATE(1000000000, 3, 500, 2, 0),
 	RK3588_PLL_RATE(983040000, 4, 655, 2, 23592),
-	RK3588_PLL_RATE(955520000, 3, 477, 2, 49806),
+	RK3588_PLL_RATE(955520000, 3, 478, 2, -15730),
 	RK3588_PLL_RATE(903168000, 6, 903, 2, 11009),
 	RK3588_PLL_RATE(900000000, 2, 300, 2, 0),
 	RK3588_PLL_RATE(850000000, 3, 425, 2, 0),
 	RK3588_PLL_RATE(816000000, 2, 272, 2, 0),
 	RK3588_PLL_RATE(786432000, 2, 262, 2, 9437),
 	RK3588_PLL_RATE(786000000, 1, 131, 2, 0),
-	RK3588_PLL_RATE(785560000, 3, 392, 2, 51117),
+	RK3588_PLL_RATE(785560000, 3, 393, 2, -14419),
 	RK3588_PLL_RATE(722534400, 8, 963, 2, 24850),
 	RK3588_PLL_RATE(600000000, 2, 200, 2, 0),
 	RK3588_PLL_RATE(594000000, 2, 198, 2, 0),
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 9e3503e2ffc2..72b36bba3152 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -635,10 +635,10 @@ struct rockchip_pll_rate_table {
 		};
 		struct {
 			/* for RK3588 */
-			unsigned int m;
-			unsigned int p;
-			unsigned int s;
-			unsigned int k;
+			unsigned int m;	/* main divider, 10 bit unsigned */
+			unsigned int p;	/* pre-divider, 6 bit unsigned */
+			unsigned int s;	/* scaler, 3 bit unsigned */
+			s16 k;		/* fractional part, 16 bit two's complement */
 		};
 	};
 };

-- 
2.54.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/2] clk: rockchip: pll: Fix the fractional part denominator on RK3588/RK3576

[Chaoyi Chen]

Hello Alexey,

On 7/23/2026 6:21 PM, Alexey Charkov wrote:
> According to the TRM, the fractional PLL coefficient should be divided by
> 65536 rather than 65535 to obtain the output rate.
> 
> Fix the denominator and add a comment with the TRM provided clock formulae
> for future reference.
> 
> See RK3576 TRM Part 1 V1.2 section 2.13.1.4 Setting Guide on P, M, S and K
> or equivalently RK3588 TRM part 1 V1.0 section 2.17.1.4 Setting Guide on P,
> M, S and K.
> 
> Fractional PLL rates don't seem to be used by any current mainline
> consumers, so this is purely a correctness fix. It will also be important
> to properly support DisplayPort output going forward, as the video output
> controller derives its pixel clock from system PLLs with no dedicated PHY
> PLL option for DP unlike HDMI, and some display modes are only achievable
> with fractional PLL rates.
> 
> Fixes: 8f6594494b1c ("clk: rockchip: add pll type for RK3588")
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>  drivers/clk/rockchip/clk-pll.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
> index 6b853800cb6b..bf8acf7cee0d 100644
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -900,6 +900,13 @@ static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll,
>  	rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK);
>  }
>  
> +/*
> + * 2250 MHz <= Fvco <= 4500 MHz
> + * For Fvco > 3 GHz: period jitter +-1% frac PLL, +-0.75% int PLL
> + * For Fvco < 3 GHz: period jitter +-2% frac PLL, +-1.50% int PLL
> + * Fvco = ((m + k / 65536) * Fin) / p
> + * Fout = ((m + k / 65536) * Fin) / (p * 2^s)
> + */
>  static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
>  {
>  	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
> @@ -915,7 +922,7 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
>  		/* fractional mode */
>  		u64 frac_rate64 = prate * cur.k;
>  
> -		postdiv = cur.p * 65535;
> +		postdiv = cur.p * 65536;


How about using 65536ULL? Will the multiplication here overflow?

>  		do_div(frac_rate64, postdiv);
>  		rate64 += frac_rate64;
>  	}
> 

-- 
Best, 
Chaoyi



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/2] clk: rockchip: pll: Fix the fractional part denominator on RK3588/RK3576

[Quentin Schulz]

Hi Chaoyi,

On 7/23/26 12:32 PM, Chaoyi Chen wrote:
> Hello Alexey,
> 
> On 7/23/2026 6:21 PM, Alexey Charkov wrote:
>> According to the TRM, the fractional PLL coefficient should be divided by
>> 65536 rather than 65535 to obtain the output rate.
>>
>> Fix the denominator and add a comment with the TRM provided clock formulae
>> for future reference.
>>
>> See RK3576 TRM Part 1 V1.2 section 2.13.1.4 Setting Guide on P, M, S and K
>> or equivalently RK3588 TRM part 1 V1.0 section 2.17.1.4 Setting Guide on P,
>> M, S and K.
>>
>> Fractional PLL rates don't seem to be used by any current mainline
>> consumers, so this is purely a correctness fix. It will also be important
>> to properly support DisplayPort output going forward, as the video output
>> controller derives its pixel clock from system PLLs with no dedicated PHY
>> PLL option for DP unlike HDMI, and some display modes are only achievable
>> with fractional PLL rates.
>>
>> Fixes: 8f6594494b1c ("clk: rockchip: add pll type for RK3588")
>> Signed-off-by: Alexey Charkov <alchark@flipper.net>
>> ---
>>   drivers/clk/rockchip/clk-pll.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
>> index 6b853800cb6b..bf8acf7cee0d 100644
>> --- a/drivers/clk/rockchip/clk-pll.c
>> +++ b/drivers/clk/rockchip/clk-pll.c
>> @@ -900,6 +900,13 @@ static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll,
>>   	rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK);
>>   }
>>   
>> +/*
>> + * 2250 MHz <= Fvco <= 4500 MHz
>> + * For Fvco > 3 GHz: period jitter +-1% frac PLL, +-0.75% int PLL
>> + * For Fvco < 3 GHz: period jitter +-2% frac PLL, +-1.50% int PLL
>> + * Fvco = ((m + k / 65536) * Fin) / p
>> + * Fout = ((m + k / 65536) * Fin) / (p * 2^s)
>> + */
>>   static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
>>   {
>>   	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
>> @@ -915,7 +922,7 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
>>   		/* fractional mode */
>>   		u64 frac_rate64 = prate * cur.k;
>>   
>> -		postdiv = cur.p * 65535;
>> +		postdiv = cur.p * 65536;
> 
> 
> How about using 65536ULL? Will the multiplication here overflow?
> 

cur.p is an unsigned int. p comes from the HW register directly and it's 
masked with 0x3f so it can be max 63 * 65536 which is well below 
UINT_MAX and also below U64_MAX so should be fine. It could overflow if 
it's set wrong in the static table we have in the driver, but then in 
any case we would write something that cannot work in registers (as it's 
masked when being written and wouldn't match the value set in the table).

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip