Devicetree
 help / color / mirror / Atom feed
* [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS
@ 2026-07-19 21:13 Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type Juan Manuel López Carrillo
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Juan Manuel López Carrillo @ 2026-07-19 21:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland
  Cc: Brian Masney, Andre Przywara, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-sunxi, linux-arm-kernel,
	devicetree, linux-kernel, Juan Manuel López Carrillo

This series follows up on the discussion in [1], where Chen-Yu pointed
out that the A523/T527 GPU mod clock is not a standard divider but a
fractional one: GPU_CLK = Clock Source * ((16-M)/16), with M 0-15.

I checked this against the T527 user manual (v0.92, section 2.7.6.58,
"FACTOR_M: mask M cycles at 16 cycles") and verified it on hardware:
with the current linear model, forcing each OPP on an Orange Pi 4A and
measuring the real GPU frequency with the Mali cycle counter gives

	OPP request	programmed	measured
	150 MHz		600M, M=3	~487 MHz
	200 MHz		800M, M=3	 648 MHz
	300 MHz		600M, M=1	 560 MHz
	400 MHz		800M, M=1	 749 MHz
	600 MHz		600M, M=0	 599 MHz

so every OPP below 600 MHz silently overclocks, up to 25% above the
vendor ceiling, and thermal throttling to "400 MHz" actually raises
the GPU frequency.

Patch 1 adds a small ccu type implementing the cycle-masking
semantics.  Patch 2 switches the A523 GPU clock to it, drops
pll-periph0-800M from the selectable parents (the vendor BSP removed
it citing GPU job faults) and drops CLK_SET_RATE_PARENT (pll-gpu loses
its CLK_SET_RATE_GATE protection once the GPU runs from a periph
output).  Patch 3 registers the existing sunxi-ng mux notifier so the
GPU is parked on pll-periph0-600M while pll-gpu changes rate, as
Chen-Yu suggested in the same thread.  Patch 4 adds the BSP operating
points to the Orange Pi 4A DT, which enables panfrost devfreq.

With the series applied the same cycle-counter measurement reads
149/199/300/399/597 MHz for the five OPPs, all from the intended
parents, and devfreq scaling plus thermal throttling (emulated
temperature) behave correctly under load.

The higher speed-bin operating points of the BSP (648-792 MHz, gated
by a SID efuse bin, running from pll-gpu) are left for a follow-up.

[1] https://lore.kernel.org/linux-sunxi/20260719153122.892013-1-juanmanuellopezcarrillo@gmail.com/

Juan Manuel López Carrillo (4):
  clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type
  clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not
    linear
  clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate
  arm64: dts: allwinner: t527-orangepi-4a: add GPU OPP table

 .../dts/allwinner/sun55i-t527-orangepi-4a.dts |  30 +++
 drivers/clk/sunxi-ng/Makefile                 |   1 +
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c        |  47 ++++-
 drivers/clk/sunxi-ng/ccu_maskdiv.c            | 199 ++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_maskdiv.h            |  71 +++++++
 5 files changed, 342 insertions(+), 6 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_maskdiv.h

-- 
2.47.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type
  2026-07-19 21:13 [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS Juan Manuel López Carrillo
@ 2026-07-19 21:13 ` Juan Manuel López Carrillo
  2026-07-19 21:24   ` sashiko-bot
  2026-07-19 21:13 ` [PATCH v1 2/4] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear Juan Manuel López Carrillo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Juan Manuel López Carrillo @ 2026-07-19 21:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland
  Cc: Brian Masney, Andre Przywara, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-sunxi, linux-arm-kernel,
	devicetree, linux-kernel, 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.  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.

Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
 drivers/clk/sunxi-ng/Makefile      |   1 +
 drivers/clk/sunxi-ng/ccu_maskdiv.c | 199 +++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_maskdiv.h |  71 ++++++++++
 3 files changed, 271 insertions(+)
 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 a1c4087d7..26313083c 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_maskdiv.c b/drivers/clk/sunxi-ng/ccu_maskdiv.c
new file mode 100644
index 000000000..2d3cccf08
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu_maskdiv.c
@@ -0,0 +1,199 @@
+// 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;
+
+		/* 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);
+	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 000000000..3cbfce064
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu_maskdiv.h
@@ -0,0 +1,71 @@
+/* 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).
+ *
+ * @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_ */
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v1 2/4] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear
  2026-07-19 21:13 [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type Juan Manuel López Carrillo
@ 2026-07-19 21:13 ` Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 4/4] arm64: dts: allwinner: t527-orangepi-4a: add GPU OPP table Juan Manuel López Carrillo
  3 siblings, 0 replies; 7+ messages in thread
From: Juan Manuel López Carrillo @ 2026-07-19 21:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland
  Cc: Brian Masney, Andre Przywara, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-sunxi, linux-arm-kernel,
	devicetree, linux-kernel, 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 20dad06b3..979e53e63 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate
  2026-07-19 21:13 [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type Juan Manuel López Carrillo
  2026-07-19 21:13 ` [PATCH v1 2/4] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear Juan Manuel López Carrillo
@ 2026-07-19 21:13 ` Juan Manuel López Carrillo
  2026-07-19 21:35   ` sashiko-bot
  2026-07-19 21:13 ` [PATCH v1 4/4] arm64: dts: allwinner: t527-orangepi-4a: add GPU OPP table Juan Manuel López Carrillo
  3 siblings, 1 reply; 7+ messages in thread
From: Juan Manuel López Carrillo @ 2026-07-19 21:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland
  Cc: Brian Masney, Andre Przywara, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-sunxi, linux-arm-kernel,
	devicetree, linux-kernel, Juan Manuel López Carrillo

Register the existing sunxi-ng mux notifier on pll-gpu so the GPU mod
clock is parked on the fixed pll-periph0-600M output while the PLL is
being reprogrammed, and switched back once it has locked, following the
cpux precedent of other sunxi SoCs.  The mux switch is glitch-free per
the T527 manual (GPU_CLK_REG, "The clock selection supports glitch-free
switch").

Nothing retunes pll-gpu at runtime today (the mux does not propagate
rate requests and all standard OPPs resolve to pll-periph0 outputs),
but the higher speed-bin operating points (648-792 MHz) will need
pll-gpu as a live parent, and any direct clk_set_rate() on the PLL is
now safe.

Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
index 979e53e63..4341eb66c 100644
--- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
+++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
@@ -1667,6 +1667,17 @@ static const u32 pll_regs[] = {
 	SUN55I_A523_PLL_NPU_REG,
 };
 
+/*
+ * Park the GPU on the fixed pll-periph0-600M output while pll-gpu
+ * changes rate; the mux switch is glitch-free (T527 manual, GPU_CLK_REG).
+ */
+static struct ccu_mux_nb sun55i_a523_gpu_nb = {
+	.common		= &gpu_clk.common,
+	.cm		= &gpu_clk.mux,
+	.delay_us	= 1,
+	.bypass_index	= 1, /* pll-periph0-600M */
+};
+
 static int sun55i_a523_ccu_probe(struct platform_device *pdev)
 {
 	void __iomem *reg;
@@ -1698,6 +1709,10 @@ static int sun55i_a523_ccu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* Reparent the GPU during pll-gpu rate changes */
+	ccu_mux_notifier_register(pll_gpu_clk.common.hw.clk,
+				  &sun55i_a523_gpu_nb);
+
 	return 0;
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v1 4/4] arm64: dts: allwinner: t527-orangepi-4a: add GPU OPP table
  2026-07-19 21:13 [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS Juan Manuel López Carrillo
                   ` (2 preceding siblings ...)
  2026-07-19 21:13 ` [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate Juan Manuel López Carrillo
@ 2026-07-19 21:13 ` Juan Manuel López Carrillo
  3 siblings, 0 replies; 7+ messages in thread
From: Juan Manuel López Carrillo @ 2026-07-19 21:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland
  Cc: Brian Masney, Andre Przywara, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-sunxi, linux-arm-kernel,
	devicetree, linux-kernel, 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.

All operating points use 920 mV: the BSP table specifies 900 mV for
every universal OPP, but on this board the GPU rail (AXP717 DCDC2,
vdd-gpu-sys) is a fixed 920 mV supply, so the OPP voltage states the
actual rail value and voltage transitions are 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>
---
 .../dts/allwinner/sun55i-t527-orangepi-4a.dts | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts b/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
index 055be86e5..0fd73a1bb 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
@@ -30,6 +30,35 @@ ext_osc32k: ext-osc32k-clk {
 		clock-output-names = "ext_osc32k";
 	};
 
+	gpu_opp_table: opp-table-gpu {
+		compatible = "operating-points-v2";
+
+		opp-150000000 {
+			opp-hz = /bits/ 64 <150000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <920000>;
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -116,6 +145,7 @@ &gmac1 {
 
 &gpu {
 	mali-supply = <&reg_dcdc2>;
+	operating-points-v2 = <&gpu_opp_table>;
 	status = "okay";
 };
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type
  2026-07-19 21:13 ` [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type Juan Manuel López Carrillo
@ 2026-07-19 21:24   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-19 21:24 UTC (permalink / raw)
  To: Juan Manuel López Carrillo; +Cc: linux-sunxi, robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ccu_maskdiv_determine_rate ignores the CLK_SET_RATE_PARENT flag, breaking rate propagation.
- [Medium] ccu_maskdiv_set_rate fails to apply CCU_FEATURE_UPDATE_BIT and CCU_FEATURE_KEY_FIELD, causing inconsistent state latching.
--

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

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

This commit introduces a cycle-masking divider clock type for sunxi-ng.
Instead of a linear divider, it masks M pulses out of every 2^width parent
cycles, which is used for hardware like the A523/T527 GPU clock.

> diff --git a/drivers/clk/sunxi-ng/ccu_maskdiv.c b/drivers/clk/sunxi-ng/ccu_maskdiv.c
> new file mode 100644
> index 0000000000000..2d3cccf081493
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu_maskdiv.c
> @@ -0,0 +1,199 @@

[ ... ]

> +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);

[Severity: High]
Does this implementation ignore the CLK_SET_RATE_PARENT flag?

By using clk_hw_get_rate(parent) instead of clk_hw_round_rate(), this loop
only evaluates parents at their currently locked rates. It appears this
prevents the framework from adjusting the parent's rate (such as the
underlying pll-gpu).

Could this silently break Dynamic Voltage and Frequency Scaling (DVFS) by
locking the GPU to a masked fraction of the PLL's fixed boot rate?

> +
> +		if (new_rate > req->rate)
> +			continue;

[ ... ]

> +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);
> +	writel(reg | (m << cmd->shift), cmd->common.base + cmd->common.reg);

[Severity: Medium]
Will hardware register updates fail to latch here if the clock requires the
CCU_SUNXI_UPDATE_BIT or CCU_FEATURE_KEY_FIELD?

The standard ccu_common framework relies on these feature bits to
synchronize hardware changes, which ccu_mux_helper_set_parent() applies
automatically.

Since this direct write ignores cmd->common.features, could rate changes
silently fail to apply for clocks that require the update bit?

> +
> +	spin_unlock_irqrestore(cmd->common.lock, flags);
> +
> +	return 0;
> +}

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate
  2026-07-19 21:13 ` [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate Juan Manuel López Carrillo
@ 2026-07-19 21:35   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-19 21:35 UTC (permalink / raw)
  To: Juan Manuel López Carrillo; +Cc: linux-sunxi, robh, conor+dt, devicetree

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

New issues:
- [High] The `pll_gpu_clk` retains the `CLK_SET_RATE_GATE` flag, which completely prevents the intended dynamic rate changes while the GPU is active.
- [High] Missing `clk_notifier_unregister()` on driver unbind/module unload leads to a Use-After-Free.
- [Medium] The return value of `ccu_mux_notifier_register()` is silently ignored.

Pre-existing issues:
- [High] Unhandled `ABORT_RATE_CHANGE` event in the mux notifier callback permanently parks the GPU on the bypass clock.
- [Medium] Out-of-bounds array read on `gpu_mux_table` via `ccu_mux_helper_set_parent()`.
--

--- Patch [3]: [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate ---
commit faed0fa03ac4f89885fa0cb42878fa743f6eb4a4
Author: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>

clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate

This commit registers a mux notifier on pll-gpu to park the GPU mod clock
on a fixed output while the PLL is being reprogrammed. This is intended to
allow glitch-free rate changes for higher speed-bin operating points.

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

[ ... ]

> @@ -1698,6 +1709,10 @@ static int sun55i_a523_ccu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/* Reparent the GPU during pll-gpu rate changes */
> +	ccu_mux_notifier_register(pll_gpu_clk.common.hw.clk,
> +				  &sun55i_a523_gpu_nb);
> +

[Severity: High]
Does pll_gpu_clk still have the CLK_SET_RATE_GATE flag set?

Looking at drivers/clk/sunxi-ng/ccu-sun55i-a523.c, pll_gpu_clk is defined
with:

    .hw.init = CLK_HW_INIT_PARENTS_DATA("pll-gpu", osc24M,
                                        &ccu_nkmp_ops,
                                        CLK_SET_RATE_GATE),

The core clock framework enforces this in clk_core_set_rate_nolock():

    if (clk_core_rate_is_protected(core))
        return -EBUSY;

If the GPU driver calls clk_set_rate() while the clock is active, will the
core framework return -EBUSY and prevent the rate change from happening,
rendering this new notifier unused?

[Severity: Medium]
What happens if ccu_mux_notifier_register fails and returns an error?

If there is a low memory condition and clk_notifier_register returns
-ENOMEM, the registration will fail but the probe continues. Will this
leave the GPU clock without the mandatory glitch-free bypass protection
during rate changes?

[Severity: High]
Is it possible this introduces a use-after-free during module unload?

sun55i_a523_ccu_driver does not appear to have a .remove callback to
unregister this notifier.

If the module is unloaded (e.g. via rmmod), the statically allocated
sun55i_a523_gpu_nb is freed. If a clock rate change occurs on pll-gpu
afterwards, or if the global clock notifier list is traversed, could this
crash the system?

[Severity: High]
This is a pre-existing issue, but how does the notifier handle an aborted
rate change?

In drivers/clk/sunxi-ng/ccu_mux.c, ccu_mux_notifier_cb only handles
PRE_RATE_CHANGE and POST_RATE_CHANGE:

    if (event == PRE_RATE_CHANGE) {
        ...
    } else if (event == POST_RATE_CHANGE) {
        ...

If a clock rate change fails downstream after PRE_RATE_CHANGE, the core
emits ABORT_RATE_CHANGE. Since this event is dropped, will the GPU clock
remain permanently stuck at the bypass rate of 600MHz?

[Severity: Medium]
This is a pre-existing issue, but can ccu_mux_helper_set_parent read out of
bounds during POST_RATE_CHANGE?

If the bootloader leaves the hardware mux register with an unmapped value
like 6 or 7, ccu_mux_helper_get_parent returns this raw index. Later,
during the notifier callback, ccu_mux_helper_set_parent blindly uses it:

    if (cm->table)
        index = cm->table[index];

Since gpu_mux_table only has 5 elements:

    static const u8 gpu_mux_table[] = { 0, 2, 3, 4, 5 };

Could this result in an out-of-bounds read and write junk back to the
hardware register?

>  	return 0;
>  }

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-19 21:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 21:13 [PATCH v1 0/4] clk: sunxi-ng: fix the A523/T527 GPU clock model, enable GPU DVFS Juan Manuel López Carrillo
2026-07-19 21:13 ` [PATCH v1 1/4] clk: sunxi-ng: add cycle-masking divider (maskdiv) clock type Juan Manuel López Carrillo
2026-07-19 21:24   ` sashiko-bot
2026-07-19 21:13 ` [PATCH v1 2/4] clk: sunxi-ng: sun55i-a523: GPU clock divider is fractional, not linear Juan Manuel López Carrillo
2026-07-19 21:13 ` [PATCH v1 3/4] clk: sunxi-ng: sun55i-a523: reparent GPU while pll-gpu changes rate Juan Manuel López Carrillo
2026-07-19 21:35   ` sashiko-bot
2026-07-19 21:13 ` [PATCH v1 4/4] arm64: dts: allwinner: t527-orangepi-4a: add GPU OPP table Juan Manuel López Carrillo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox