[PATCH v2 0/3] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS

[PATCH v2 0/3] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS

[Juan Manuel López Carrillo]

Hi,

v2 of the A523/T527 GPU clock series.  v1 is here:
https://lore.kernel.org/linux-sunxi/20260719211319.982285-1-juanmanuellopezcarrillo@gmail.com/

The GPU mod clock is not a linear M+1 divider: the M factor masks M pulses
out of every 16 parent cycles, so rate = parent * (16 - M) / 16 (T527 user
manual v0.92, section 2.7.6.58).  Modelling it with ccu_div programs a faster
clock than requested for every M > 0, which is what mainline does today.

Measured on an Orange Pi 4A with the Mali cycle counter, the OPPs labelled
150/200/300/400 MHz were really running at 487/648/560/750 MHz, and thermal
throttling to "400" actually raised the clock to 750.  With this series the
same measurement gives 149/199/300/399/597 MHz.

Changes since v1:

 - Dropped the pll-gpu reparenting notifier (patch 3/4 in v1).  I wrote it
   when the plan for the higher speed-bin points was to retune pll-gpu at
   runtime: it parked the GPU on the fixed pll-periph0-600M output while the
   PLL was being reprogrammed, so the GPU would never see it relocking.  That
   plan has been superseded.  The maskdiv in patch 1 can divide any parent
   down, and the speed-bin work on top of it (first follow-up below) pins
   pll-gpu once via assigned-clock-rates and reaches every operating point by
   moving only the mux and the divider.  The PLL therefore never changes rate
   at runtime, the notifier can never fire, and gpu_clk deliberately does not
   set CLK_SET_RATE_PARENT either.  That is how the 696 MHz point runs here
   today, with no notifier.  It would only be needed again if a future series
   reprograms the PLL instead of pinning it.  Chen-Yu, you asked for this
   notifier in the v1 discussion: tell me if you would rather have it now
   anyway and I will put it back.

 - maskdiv: honour CCU_FEATURE_UPDATE_BIT and CCU_FEATURE_KEY_FIELD in
   set_rate, clamp determine_rate() to the request bounds, and document that
   CLK_SET_RATE_PARENT is not supported.  These were folded into patch 1 so
   the new type is correct as introduced, rather than introduced and then
   fixed.  They also address the two issues the CI bot reported on v1.

 - The GPU OPP table moved from the board .dts to sun55i-a523.dtsi, and the
   operating-points-v2 reference now lives in the SoC's GPU node (Chen-Yu).

 - opp-microvolt now uses the <target min max> form, <900000 900000 920000>:
   900 mV is what the BSP universal table specifies, and the 920 mV ceiling
   covers boards whose GPU rail is a fixed 920 mV supply (Chen-Yu).

 - Rebased from v7.2-rc4 onto sunxi/for-next.

Tested on an Orange Pi 4A (T527, 2 GB): rates verified with the Mali cycle
counter under load, thermal-emulation throttling exercised, no job faults.

Two follow-ups are already working on the board and I can send them next.
Tell me which one is more useful to you first, or if you would rather they
waited:

 - GPU operating points above 600 MHz, gated on the SoC speed bin read from
   the SID.  This chip is bin 1, where the vendor table sanctions 696 MHz at
   the same 900 mV; it has been running games here without job faults.

 - The A523/T527 CPU clock unit (ccu-sun55i-a523-cpu.c) and the generic
   sunxi-ng fix it depends on, which is what cpufreq needs on this SoC.  The
   DSU/L3 clock lives in the same unit.

Juan Manuel López Carrillo (3):
  clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type
  clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not
    linear
  arm64: dts: allwinner: a523: add GPU OPP table

 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi |  30 +++
 drivers/clk/sunxi-ng/Makefile                 |   1 +
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c        |  32 ++-
 drivers/clk/sunxi-ng/ccu_common.h             |   3 +
 drivers/clk/sunxi-ng/ccu_maskdiv.c            | 213 ++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_maskdiv.h            |  76 +++++++
 drivers/clk/sunxi-ng/ccu_mux.c                |   2 -
 7 files changed, 349 insertions(+), 8 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.h


base-commit: 859c0e1925332d413ca8f9159c8ca5d04eea32a2
-- 
2.47.3

[PATCH v2 1/3] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type

[Juan Manuel López Carrillo]

Some mod clocks do not divide their parent with a linear M+1 divider:
the M factor masks (swallows) M pulses out of every 2^width parent
cycles, so the average output rate is

	rate = parent * (2^width - M) / 2^width

and the surviving pulses keep the parent period.  The A523/T527 GPU
clock (GPU_CLK_REG, 0x670) is such a divider: "FACTOR_M: mask M cycles
at 16 cycles", GPU_CLK = Clock Source * ((16-M)/16) (T527 user manual
v0.92, section 2.7.6.58).

Modelling these registers with the linear ccu_div type programs a
faster clock than requested for every M > 0 (e.g. M=1 on a 800 MHz
parent yields 750 MHz, not 400 MHz).

Add a small ccu type implementing the masking semantics.  Because the
masked output is not an even pulse train, determine_rate prefers, among
the parents that reach the requested rate, the one needing the least
masking, and clamps the result to the request's min_rate/max_rate
bounds.  set_rate_and_parent follows the same ordering rule as
clk_composite_set_rate_and_parent() so no intermediate configuration
overshoots both the old and the new rate, and honours the
CCU_FEATURE_UPDATE_BIT and CCU_FEATURE_KEY_FIELD features, so the type
can be reused on registers that need them.

CLK_SET_RATE_PARENT is deliberately not supported: the masking factor
and a parent rate change are two independent knobs and picking a
combination of both is out of scope for this type.

Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
 drivers/clk/sunxi-ng/Makefile      |   1 +
 drivers/clk/sunxi-ng/ccu_common.h  |   3 +
 drivers/clk/sunxi-ng/ccu_maskdiv.c | 213 +++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_maskdiv.h |  76 ++++++++++
 drivers/clk/sunxi-ng/ccu_mux.c     |   2 -
 5 files changed, 293 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.h

diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
index a1c4087d7241..26313083c2f8 100644
--- a/drivers/clk/sunxi-ng/Makefile
+++ b/drivers/clk/sunxi-ng/Makefile
@@ -10,6 +10,7 @@ sunxi-ccu-y			+= ccu_reset.o
 # Base clock types
 sunxi-ccu-y			+= ccu_div.o
 sunxi-ccu-y			+= ccu_frac.o
+sunxi-ccu-y			+= ccu_maskdiv.o
 sunxi-ccu-y			+= ccu_gate.o
 sunxi-ccu-y			+= ccu_mux.o
 sunxi-ccu-y			+= ccu_mult.o
diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
index d9dc24ad5503..0260af263d05 100644
--- a/drivers/clk/sunxi-ng/ccu_common.h
+++ b/drivers/clk/sunxi-ng/ccu_common.h
@@ -29,6 +29,9 @@
 /* Some clocks need this bit to actually apply register changes */
 #define CCU_SUNXI_UPDATE_BIT		BIT(27)
 
+/* Key value for clocks with CCU_FEATURE_KEY_FIELD (reads as zero) */
+#define CCU_MUX_KEY_VALUE		0x16aa0000
+
 struct device_node;
 
 struct ccu_common {
diff --git a/drivers/clk/sunxi-ng/ccu_maskdiv.c b/drivers/clk/sunxi-ng/ccu_maskdiv.c
new file mode 100644
index 000000000000..4ad49d51405b
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu_maskdiv.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2026 Juan Manuel López Carrillo
+ *
+ * Cycle-masking divider: the M factor masks M pulses out of every
+ * 2^width parent cycles instead of dividing the parent rate, so
+ *
+ *	rate = parent * (2^width - M) / 2^width
+ *
+ * The masked output is not an even pulse train: the surviving pulses
+ * keep the parent period. Rate selection therefore prefers, among the
+ * parents that reach the requested rate, the one needing the least
+ * masking.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/math64.h>
+
+#include "ccu_gate.h"
+#include "ccu_maskdiv.h"
+
+static unsigned long ccu_maskdiv_calc_rate(unsigned long parent_rate,
+					   unsigned int m, unsigned int width)
+{
+	unsigned int n = 1 << width;
+
+	return div_u64((u64)parent_rate * (n - m), n);
+}
+
+/*
+ * Smallest M (least masking) whose output does not exceed the requested
+ * rate; masking everything (M == 2^width) is never returned.
+ */
+static unsigned int ccu_maskdiv_find_m(unsigned long parent_rate,
+				       unsigned long rate, unsigned int width)
+{
+	unsigned int n = 1 << width;
+	u64 kept;
+
+	if (!parent_rate || rate >= parent_rate)
+		return 0;
+
+	kept = div64_ul((u64)rate * n, parent_rate);
+	if (!kept)
+		kept = 1;
+
+	return n - (unsigned int)kept;
+}
+
+static void ccu_maskdiv_disable(struct clk_hw *hw)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+
+	return ccu_gate_helper_disable(&cmd->common, cmd->enable);
+}
+
+static int ccu_maskdiv_enable(struct clk_hw *hw)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+
+	return ccu_gate_helper_enable(&cmd->common, cmd->enable);
+}
+
+static int ccu_maskdiv_is_enabled(struct clk_hw *hw)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+
+	return ccu_gate_helper_is_enabled(&cmd->common, cmd->enable);
+}
+
+static unsigned long ccu_maskdiv_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+	unsigned int m;
+	u32 reg;
+
+	reg = readl(cmd->common.base + cmd->common.reg);
+	m = (reg >> cmd->shift) & ((1 << cmd->width) - 1);
+
+	return ccu_maskdiv_calc_rate(parent_rate, m, cmd->width);
+}
+
+static int ccu_maskdiv_determine_rate(struct clk_hw *hw,
+				      struct clk_rate_request *req)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+	unsigned long best_rate = 0, best_parent_rate = 0;
+	struct clk_hw *best_parent = NULL;
+	unsigned int best_m = UINT_MAX;
+	unsigned int i;
+
+	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
+		struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
+		unsigned long parent_rate, new_rate;
+		unsigned int m;
+
+		if (!parent)
+			continue;
+
+		parent_rate = clk_hw_get_rate(parent);
+		m = ccu_maskdiv_find_m(parent_rate, req->rate, cmd->width);
+		new_rate = ccu_maskdiv_calc_rate(parent_rate, m, cmd->width);
+
+		if (new_rate > req->rate)
+			continue;
+
+		/*
+		 * Reject rates outside the framework's bounds: a maskdiv
+		 * rounds by masking parent cycles, so it can only produce
+		 * sub-multiples of a parent rate; without this check a
+		 * consumer asking for, say, a tight [max_rate, max_rate]
+		 * window would silently get a smaller rate.
+		 */
+		if (new_rate < req->min_rate || new_rate > req->max_rate)
+			continue;
+
+		/* Closest rate first; on ties, the least masking */
+		if (new_rate > best_rate ||
+		    (new_rate == best_rate && m < best_m)) {
+			best_rate = new_rate;
+			best_parent_rate = parent_rate;
+			best_parent = parent;
+			best_m = m;
+		}
+	}
+
+	if (!best_parent)
+		return -EINVAL;
+
+	req->best_parent_hw = best_parent;
+	req->best_parent_rate = best_parent_rate;
+	req->rate = best_rate;
+
+	return 0;
+}
+
+static int ccu_maskdiv_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+	unsigned int m;
+	unsigned long flags;
+	u32 reg;
+
+	m = ccu_maskdiv_find_m(parent_rate, rate, cmd->width);
+
+	spin_lock_irqsave(cmd->common.lock, flags);
+
+	reg = readl(cmd->common.base + cmd->common.reg);
+	reg &= ~GENMASK(cmd->shift + cmd->width - 1, cmd->shift);
+	if (cmd->common.features & CCU_FEATURE_KEY_FIELD)
+		reg |= CCU_MUX_KEY_VALUE;
+	if (cmd->common.features & CCU_FEATURE_UPDATE_BIT)
+		reg |= CCU_SUNXI_UPDATE_BIT;
+	writel(reg | (m << cmd->shift), cmd->common.base + cmd->common.reg);
+
+	spin_unlock_irqrestore(cmd->common.lock, flags);
+
+	return 0;
+}
+
+static u8 ccu_maskdiv_get_parent(struct clk_hw *hw)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+
+	return ccu_mux_helper_get_parent(&cmd->common, &cmd->mux);
+}
+
+static int ccu_maskdiv_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
+
+	return ccu_mux_helper_set_parent(&cmd->common, &cmd->mux, index);
+}
+
+static int ccu_maskdiv_set_rate_and_parent(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate, u8 index)
+{
+	/*
+	 * Same ordering rule as clk_composite_set_rate_and_parent(): if
+	 * switching the mux with the current M would overshoot the
+	 * requested rate, program the divider first, so the
+	 * intermediate rate never exceeds both the old and the new
+	 * rate.
+	 */
+	if (ccu_maskdiv_recalc_rate(hw, parent_rate) > rate) {
+		ccu_maskdiv_set_rate(hw, rate, parent_rate);
+		ccu_maskdiv_set_parent(hw, index);
+	} else {
+		ccu_maskdiv_set_parent(hw, index);
+		ccu_maskdiv_set_rate(hw, rate, parent_rate);
+	}
+
+	return 0;
+}
+
+const struct clk_ops ccu_maskdiv_ops = {
+	.disable	= ccu_maskdiv_disable,
+	.enable		= ccu_maskdiv_enable,
+	.is_enabled	= ccu_maskdiv_is_enabled,
+
+	.get_parent	= ccu_maskdiv_get_parent,
+	.set_parent	= ccu_maskdiv_set_parent,
+
+	.determine_rate	= ccu_maskdiv_determine_rate,
+	.recalc_rate	= ccu_maskdiv_recalc_rate,
+	.set_rate	= ccu_maskdiv_set_rate,
+	.set_rate_and_parent = ccu_maskdiv_set_rate_and_parent,
+};
+EXPORT_SYMBOL_NS_GPL(ccu_maskdiv_ops, "SUNXI_CCU");
diff --git a/drivers/clk/sunxi-ng/ccu_maskdiv.h b/drivers/clk/sunxi-ng/ccu_maskdiv.h
new file mode 100644
index 000000000000..e070798f1533
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu_maskdiv.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2026 Juan Manuel López Carrillo
+ */
+
+#ifndef _CCU_MASKDIV_H_
+#define _CCU_MASKDIV_H_
+
+#include <linux/clk-provider.h>
+
+#include "ccu_common.h"
+#include "ccu_mux.h"
+
+/*
+ * struct ccu_maskdiv - cycle-masking ("fractional") divider
+ *
+ * This divider does not divide the parent clock: it masks (swallows) M
+ * pulses out of every 2^width parent cycles, so the average output rate
+ * is
+ *
+ *	rate = parent * (2^width - M) / 2^width
+ *
+ * with the remaining pulses keeping the parent period. The A523/T527
+ * GPU clock (GPU_CLK_REG, 0x670) is such a divider: "FACTOR_M: mask M
+ * cycles at 16 cycles", GPU_CLK = Clock Source * ((16-M)/16) (T527 user
+ * manual v0.92, section 2.7.6.58).
+ *
+ * This type does not support CLK_SET_RATE_PARENT: determine_rate
+ * evaluates parents at their current rate and does not propagate rate
+ * requests upstream.  If a future user needs parent rate propagation,
+ * switch to clk_hw_round_rate() in the determine_rate loop.
+ *
+ * @shift:	shift of the M field in the register
+ * @width:	width of the M field; the mask window is 2^width cycles
+ */
+struct ccu_maskdiv {
+	u32			enable;
+
+	u8			shift;
+	u8			width;
+
+	struct ccu_mux_internal	mux;
+	struct ccu_common	common;
+};
+
+#define SUNXI_CCU_MASKDIV_HW_WITH_MUX_TABLE_GATE(_struct, _name,	\
+						 _parents, _table,	\
+						 _reg,			\
+						 _mshift, _mwidth,	\
+						 _muxshift, _muxwidth,	\
+						 _gate, _flags)		\
+	struct ccu_maskdiv _struct = {					\
+		.enable	= _gate,					\
+		.shift	= _mshift,					\
+		.width	= _mwidth,					\
+		.mux	= _SUNXI_CCU_MUX_TABLE(_muxshift, _muxwidth,	\
+					       _table),			\
+		.common	= {						\
+			.reg		= _reg,				\
+			.hw.init	= CLK_HW_INIT_PARENTS_HW(_name,	\
+								 _parents, \
+								 &ccu_maskdiv_ops, \
+								 _flags), \
+		},							\
+	}
+
+static inline struct ccu_maskdiv *hw_to_ccu_maskdiv(struct clk_hw *hw)
+{
+	struct ccu_common *common = hw_to_ccu_common(hw);
+
+	return container_of(common, struct ccu_maskdiv, common);
+}
+
+extern const struct clk_ops ccu_maskdiv_ops;
+
+#endif /* _CCU_MASKDIV_H_ */
diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
index 4503c9780c39..fa1f5fd2a1fd 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.c
+++ b/drivers/clk/sunxi-ng/ccu_mux.c
@@ -12,8 +12,6 @@
 #include "ccu_gate.h"
 #include "ccu_mux.h"
 
-#define CCU_MUX_KEY_VALUE		0x16aa0000
-
 static u16 ccu_mux_get_prediv(struct ccu_common *common,
 			      struct ccu_mux_internal *cm,
 			      int parent_index)
-- 
2.47.3

Re: [PATCH v2 1/3] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `ccu_maskdiv_set_rate` recalculates a different mask factor (`M`) than what `ccu_maskdiv_determine_rate` initially chose, silently programming the hardware to a lower frequency than negotiated.
- [High] `ccu_maskdiv_determine_rate` ignores the `CLK_SET_RATE_NO_REPARENT` flag, potentially causing unintended clock reparenting.
--

--- Patch [1]: [PATCH v2 1/3] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type ---
commit cf0833bab086c6beae6c2b221f361043441cc718
Author: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>

clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type

This commit adds a cycle-masking divider for the A523/T527 GPU clock.
It masks M pulses out of every 2^width parent cycles instead of using a
linear divider. It also adds custom determine_rate and set_rate functions
to support this behavior.

> diff --git a/drivers/clk/sunxi-ng/ccu_maskdiv.c b/drivers/clk/sunxi-ng/ccu_maskdiv.c
> new file mode 100644
> index 0000000000000..4ad49d51405b3
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu_maskdiv.c

[ ... ]

> +static int ccu_maskdiv_determine_rate(struct clk_hw *hw,
> +				      struct clk_rate_request *req)
> +{
> +	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
> +	unsigned long best_rate = 0, best_parent_rate = 0;
> +	struct clk_hw *best_parent = NULL;
> +	unsigned int best_m = UINT_MAX;
> +	unsigned int i;
> +
> +	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
> +		struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);

[Severity: High]
Does this iteration need to respect the CLK_SET_RATE_NO_REPARENT flag?

Unlike other sunxi-ng multiplexers that use ccu_mux_helper_determine_rate(),
this loops unconditionally across all available parents. If a driver specifies
CLK_SET_RATE_NO_REPARENT, would this silently ignore it and potentially cause
unintended clock reparenting?

> +		unsigned long parent_rate, new_rate;
> +		unsigned int m;

[ ... ]

> +static int ccu_maskdiv_set_rate(struct clk_hw *hw, unsigned long rate,
> +				unsigned long parent_rate)
> +{
> +	struct ccu_maskdiv *cmd = hw_to_ccu_maskdiv(hw);
> +	unsigned int m;
> +	unsigned long flags;
> +	u32 reg;
> +
> +	m = ccu_maskdiv_find_m(parent_rate, rate, cmd->width);

[Severity: High]
Does ccu_maskdiv_set_rate() end up programming a different mask factor (M)
than what ccu_maskdiv_determine_rate() initially chose?

Because ccu_maskdiv_calc_rate() truncates the remainder (e.g. 1000 * 13 / 16
= 812), feeding the result back into ccu_maskdiv_find_m() evaluates this
truncated output again. For example, 812 * 16 / 1000 = 12, instead of the
original 13.

Could this inappropriately increase M and silently program the hardware to
a lower frequency than what was negotiated?

> +
> +	spin_lock_irqsave(cmd->common.lock, flags);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180755.288793-1-juanmanuellopezcarrillo@gmail.com?part=1

[PATCH v2 2/3] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear

[Juan Manuel López Carrillo]

The GPU mod clock (0x670) was modelled as a linear M+1 divider, but the
M factor of this register is a cycle-masking divider: GPU_CLK = Clock
Source * ((16-M)/16) (T527 user manual v0.92, section 2.7.6.58).

With the linear model every OPP that needed M > 0 silently ran the GPU
faster than requested.  Measured on an Orange Pi 4A (T527) with the
Mali cycle counter against the programmed register:

	OPP request	programmed	real rate
	150 MHz		600M, M=3	487.5 MHz
	200 MHz		800M, M=3	650   MHz
	300 MHz		600M, M=1	562.5 MHz
	400 MHz		800M, M=1	750   MHz
	600 MHz		600M, M=0	600   MHz

i.e. the "400 MHz" OPP ran the GPU at 750 MHz, 25% above the vendor
ceiling of 600 MHz, at the low-OPP voltage.  Thermal throttling to
"400 MHz" actually overclocked the GPU.

Switch the clock to the maskdiv type.  With least-masking preference
the vendor OPP set now resolves to 600/400/300/200 MHz taken undivided
from their periph outputs and 150 MHz = pll-periph0-200M * 12/16, all
verified exact on hardware with the same cycle-counter method.

Drop pll-periph0-800M from the selectable parents (the mux table skips
hardware index 1): the vendor BSP removed it from its parent list with
the comment "If GPU use pll-peri0-800m, gpu will occur job fault", and
with the masking semantics every vendor OPP matches exactly from the
800M parent first, so it would otherwise always be chosen.

Also drop CLK_SET_RATE_PARENT: every OPP is reachable from the fixed
pll-periph0 outputs, and pll-gpu must never be reprogrammed through this
mux.  Once the GPU moves off pll-gpu the PLL is no longer prepared, so
it loses the rate protection of CLK_SET_RATE_GATE; a propagated rate
request would then reprogram the PLL while its gate is off (the lock
bit never asserts, 70 ms poll timeout per transition) and switch the
running GPU onto it before it locks.

Fixes: 6702d17f54a8 ("clk: sunxi-ng: a523: add video mod clocks")
Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 32 +++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
index 20dad06b37ca..979e53e63522 100644
--- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
+++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
@@ -21,6 +21,7 @@
 
 #include "ccu_div.h"
 #include "ccu_gate.h"
+#include "ccu_maskdiv.h"
 #include "ccu_mp.h"
 #include "ccu_mult.h"
 #include "ccu_nk.h"
@@ -442,18 +443,37 @@ static SUNXI_CCU_GATE_HWS(bus_g2d_clk, "bus-g2d", ahb_hws, 0x63c, BIT(0), 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 */
-				    CLK_SET_RATE_PARENT);
+/*
+ * Mux index 1 (pll-periph0-800M) is skipped: the vendor BSP removed it
+ * from the parent list ("If GPU use pll-peri0-800m, gpu will occur job
+ * fault"), and with the masking divider every OPP would match exactly
+ * from it first.
+ */
+static const u8 gpu_mux_table[] = { 0, 2, 3, 4, 5 };
+
+/*
+ * The M factor is a cycle-masking (fractional) divider, not a linear
+ * one: rate = source * (16 - M) / 16 (T527 manual, GPU_CLK_REG).
+ *
+ * No CLK_SET_RATE_PARENT: every GPU OPP is reachable from the fixed
+ * pll-periph0 outputs, and pll-gpu must never be reprogrammed through this mux.
+ * Once the GPU moves off pll-gpu the PLL is no longer prepared, so it loses
+ * the rate protection of CLK_SET_RATE_GATE; a propagated rate request would
+ * then reprogram the PLL while its gate is off (the lock bit never asserts,
+ * 70 ms timeout) and switch the running GPU onto it before it locks.
+ */
+static SUNXI_CCU_MASKDIV_HW_WITH_MUX_TABLE_GATE(gpu_clk, "gpu", gpu_parents,
+						gpu_mux_table, 0x670,
+						0, 4,	/* M */
+						24, 3,	/* mux */
+						BIT(31),	/* gate */
+						0);
 
 static SUNXI_CCU_GATE_HWS(bus_gpu_clk, "bus-gpu", ahb_hws, 0x67c, BIT(0), 0);
 
-- 
2.47.3

Re: [PATCH v2 2/3] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Integer truncation in `ccu_maskdiv_find_m()` causes the hardware to drift lower than the framework's requested rate when the parent rate is not a perfect multiple of the cycle width (16). When `determine_rate` computes a new rate using `new_rate = (parent_rate * (16 - m)) / 16`, integer division truncates the remainder. When `set_rate` subsequently attempts to recover `m` by computing `kept = (new_rate * 16) / parent_rate`, the loss of the remainder strictly reduces `kept` by 1. This causes `set_rate` to program `m_found = m + 1` into the hardware, masking an additional cycle and driving the clock at a lower rate than what was reported and committed by `determine_rate`.

Pre-existing issues:
- [High] The newly introduced sparse `gpu_mux_table` triggers a pre-existing flaw in `ccu_mux_helper_get_parent()`. If the vendor bootloader leaves the GPU mux at hardware index 1 (`pll-periph0-800M`), `ccu_mux_helper_get_parent()` fails to find `1` in the sparse table (`{ 0, 2, 3, 4, 5 }`) and falls back to returning the raw hardware index `1`. The clock framework interprets this `1` as a software array index and incorrectly maps it to `gpu_parents[1]` (`pll_periph0_600M_clk`). If the kernel subsequently requests a 600MHz GPU rate, the framework falsely assumes the mux is already on the correct parent, skipping the mux switch and only setting the divider. This causes the hardware to remain at the undivided 800MHz parent, triggering the exact GPU job faults this patch intends to fix.
- [High] `pll_gpu_clk` is defined with the `CLK_SET_RATE_GATE` flag, which requires the clock to be unprepared (gated) before its rate can be changed. However, as noted in the commit message, reprogramming this PLL while its gate is off causes a 70ms lock timeout because the lock bit never asserts when the clock is disabled. This means any rate change on `pll_gpu` while it is gated will stall the kernel for 70ms. The patch works around this for the `gpu` downstream consumer by dropping `CLK_SET_RATE_PARENT`, but `pll_gpu_clk` itself remains incorrectly modeled. It should use `CLK_SET_RATE_UNGATE` to inform the framework that the clock must be powered on to change its rate.
--

--- Patch [2]: [PATCH v2 2/3] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear ---
commit 5635b9edbbd86c7ae0bef62dc457d0e280dc656a
Author: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>

clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear

This patch changes the GPU clock modeling on sun55i-a523 from a linear divider
to a fractional cycle-masking divider. It updates the parent list, removes
the 800M parent due to hardware faults, and drops CLK_SET_RATE_PARENT to
prevent reprogramming the GPU PLL while it is gated.

> Also drop CLK_SET_RATE_PARENT: every OPP is reachable from the fixed
> pll-periph0 outputs, and pll-gpu must never be reprogrammed through this
> mux.  Once the GPU moves off pll-gpu the PLL is no longer prepared, so
> it loses the rate protection of CLK_SET_RATE_GATE; a propagated rate
> request would then reprogram the PLL while its gate is off (the lock
> bit never asserts, 70 ms poll timeout per transition) and switch the
> running GPU onto it before it locks.

[Severity: High]
This is a pre-existing issue, but does pll_gpu_clk use the wrong rate flag?

The commit message notes that the lock bit never asserts when the gate is
off, causing a 70ms timeout when reprogrammed. However, pll_gpu_clk is
declared in this file with CLK_SET_RATE_GATE:

drivers/clk/sunxi-ng/ccu-sun55i-a523.c:
    ...
                               CLK_SET_RATE_GATE),
    ...

Using CLK_SET_RATE_GATE enforces that the clock must be gated (unprepared)
before its rate can be changed. Since the hardware requires the PLL to be
ungated to lock, this contradicts the hardware's requirements.

Should pll_gpu_clk be updated to use CLK_SET_RATE_UNGATE instead, to inform
the framework that the clock must be powered on to change its rate?

> diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> index 20dad06b37ca3..979e53e635228 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c

[ ... ]

> @@ -442,18 +443,37 @@ static SUNXI_CCU_GATE_HWS(bus_g2d_clk, "bus-g2d", ahb_hws, 0x63c, BIT(0), 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 */
> -				    CLK_SET_RATE_PARENT);
> +/*
> + * Mux index 1 (pll-periph0-800M) is skipped: the vendor BSP removed it
> + * from the parent list ("If GPU use pll-peri0-800m, gpu will occur job
> + * fault"), and with the masking divider every OPP would match exactly
> + * from it first.
> + */
> +static const u8 gpu_mux_table[] = { 0, 2, 3, 4, 5 };

[Severity: High]
This is a pre-existing issue, but does skipping index 1 here expose a flaw
in ccu_mux_helper_get_parent()?

If the bootloader leaves the GPU mux at hardware index 1, the helper fails
to find it in this sparse table and falls back to returning the raw index:

drivers/clk/sunxi-ng/ccu_mux.c:ccu_mux_helper_get_parent() {
    ...
        for (i = 0; i < num_parents; i++)
            if (cm->table[i] == parent)
                return i;
    }

    return parent;
}

The clock framework then interprets this returned 1 as a software array
index and incorrectly maps it to gpu_parents[1] (which is now
pll_periph0_600M_clk).

If the kernel subsequently requests a 600MHz GPU rate, the framework
might falsely assume the mux is already on the correct parent and skip the
switch. This would leave the hardware at the undivided 800MHz parent,
triggering the exact GPU job faults this patch intends to fix.

> +
> +/*
> + * The M factor is a cycle-masking (fractional) divider, not a linear
> + * one: rate = source * (16 - M) / 16 (T527 manual, GPU_CLK_REG).
> + *
> + * No CLK_SET_RATE_PARENT: every GPU OPP is reachable from the fixed
> + * pll-periph0 outputs, and pll-gpu must never be reprogrammed through this mux.
> + * Once the GPU moves off pll-gpu the PLL is no longer prepared, so it loses
> + * the rate protection of CLK_SET_RATE_GATE; a propagated rate request would
> + * then reprogram the PLL while its gate is off (the lock bit never asserts,
> + * 70 ms timeout) and switch the running GPU onto it before it locks.
> + */
> +static SUNXI_CCU_MASKDIV_HW_WITH_MUX_TABLE_GATE(gpu_clk, "gpu", gpu_parents,
> +						gpu_mux_table, 0x670,
> +						0, 4,	/* M */
> +						24, 3,	/* mux */
> +						BIT(31),	/* gate */
> +						0);

[Severity: High]
Does using the maskdiv implementation here cause the hardware to drift
lower than the requested rate when the parent rate is not evenly divisible
by 16?

In ccu_maskdiv_find_m(), integer truncation occurs when recovering the
multiplier:

drivers/clk/sunxi-ng/ccu_maskdiv.c:ccu_maskdiv_find_m() {
    ...
    kept = div64_ul((u64)rate * n, parent_rate);
    ...
}

When determine_rate computes new_rate = (parent_rate * (16 - m)) / 16,
the remainder is truncated. When set_rate subsequently attempts to recover
m using the truncated rate:

drivers/clk/sunxi-ng/ccu_maskdiv.c:ccu_maskdiv_set_rate() {
    ...
    m = ccu_maskdiv_find_m(parent_rate, rate, cmd->width);
    ...
}

The loss of the remainder strictly reduces kept by 1. This causes set_rate
to program m + 1 into the hardware, masking an additional cycle and driving
the clock at a lower rate than what was reported by determine_rate.

Could DIV_ROUND_UP_ULL be used to prevent this truncation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180755.288793-1-juanmanuellopezcarrillo@gmail.com?part=2

[PATCH v2 3/3] arm64: dts: allwinner: a523: add GPU OPP table

[Juan Manuel López Carrillo]

Add the Mali-G57 operating points from the vendor BSP universal table
(150/200/300/400/600 MHz) so panfrost devfreq can scale the GPU instead
of running at the boot clock.

The table describes the SoC rather than any particular board, so it
lives in the .dtsi and is referenced from the GPU node there; every
A523/T527 board in tree already provides mali-supply.

The BSP universal table specifies 900 mV for all of these operating
points, so that is the target and minimum voltage.  The maximum is set
to 920 mV to also cover boards whose GPU rail is a fixed 920 mV supply
(the Orange Pi 4A drives it from AXP717 DCDC2, fixed at 920 mV), where
the voltage transitions are then no-ops.

The higher speed-bin points of the BSP (648-792 MHz) are not included:
they are gated by a SID efuse bin and need pll-gpu as a live parent.

Depends on the sun55i-a523 GPU clock divider fix: the OPP rates are only
produced correctly with the cycle-masking divider model.  Validated on
hardware with the Mali cycle counter: 149/199/300/399/597 MHz measured
under load, thermal-emulation throttling included.

Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index 85ef492ffeae..a3164b5dd609 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -85,6 +85,35 @@ osc24M: osc24M-clk {
 		clock-output-names = "osc24M";
 	};
 
+	gpu_opp_table: opp-table-gpu {
+		compatible = "operating-points-v2";
+
+		opp-150000000 {
+			opp-hz = /bits/ 64 <150000000>;
+			opp-microvolt = <900000 900000 920000>;
+		};
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <900000 900000 920000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <900000 900000 920000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <900000 900000 920000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <900000 900000 920000>;
+		};
+	};
+
 	pmu {
 		compatible = "arm,cortex-a55-pmu";
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
@@ -121,6 +150,7 @@ gpu: gpu@1800000 {
 			interrupt-names = "job", "mmu", "gpu";
 			clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>;
 			clock-names = "core", "bus";
+			operating-points-v2 = <&gpu_opp_table>;
 			power-domains = <&pck600 PD_GPU>;
 			resets = <&ccu RST_BUS_GPU>;
 			status = "disabled";
-- 
2.47.3