This series adds a clock notifier for MediaTek clock muxes, required
in order to achieve stability for GPU DVFS.
The GPU frequency scaling mechanism requires us to switch the GPU
mux clock to a safe parent which frequency is always less or equal
to the "current" GPU frequency before reprogramming its dedicated
"MFG" PLL.
This is needed because the PLL needs time to reconfigure for its
output to stabilize (so, for the PLL to lock again): failing to do
so will lead to instabilities such as glitches, GPU lockups and/or
full system lockups.
While at it, reparenting of some GPU clocks was also performed, as
the clock tree was slightly incorrect.
This series was tested, along with mtk-regulator-coupler [1], on
Chromebooks with different SoCs (MT8183, MT8192, MT8195*), resulting
in fully working GPU DVFS with the Panfrost driver.
[1]: https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
* MT8195 does not require mtk-regulator-coupler. This series, along
with [1], are required to perform GPU DVFS also on non-Chromebook SoCs.
Changes in v2:
- Added comment in clk-mt8195-topckgen to keep the mfg parents
documented after removal, as suggested by Chen-Yu
AngeloGioacchino Del Regno (6):
clk: mediatek: clk-mt8195-mfg: Reparent mfg_bg3d and propagate rate
changes
clk: mediatek: clk-mt8195-topckgen: Register mfg_ck_fast_ref as
generic mux
clk: mediatek: clk-mt8195-topckgen: Add GPU clock mux notifier
clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents
clk: mediatek: clk-mt8192-mfg: Propagate rate changes to parent
clk: mediatek: clk-mt8192: Add clock mux notifier for mfg_pll_sel
Chen-Yu Tsai (4):
arm64: dts: mt8183: Fix Mali GPU clock
clk: mediatek: mt8183: mfgcfg: Propagate rate changes to parent
clk: mediatek: mux: add clk notifier functions
clk: mediatek: mt8183: Add clk mux notifier for MFG mux
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 6 +--
drivers/clk/mediatek/clk-mt8183.c | 28 +++++++++++++
drivers/clk/mediatek/clk-mt8192-mfg.c | 6 ++-
drivers/clk/mediatek/clk-mt8192.c | 28 +++++++++++++
drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++-
drivers/clk/mediatek/clk-mt8195-topckgen.c | 46 +++++++++++++++-------
drivers/clk/mediatek/clk-mux.c | 38 ++++++++++++++++++
drivers/clk/mediatek/clk-mux.h | 15 +++++++
9 files changed, 153 insertions(+), 22 deletions(-)
--
2.37.2
From: Chen-Yu Tsai <[email protected]>
With device frequency scaling, the mux clock that (indirectly) feeds the
device selects between a dedicated PLL, and some other stable clocks.
When a clk rate change is requested, the (normally) upstream PLL is
reconfigured. It's possible for the clock output of the PLL to become
unstable during this process.
To avoid causing the device to glitch, the mux should temporarily be
switched over to another "stable" clock during the PLL rate change.
This is done with clk notifiers.
This patch adds common functions for notifiers to temporarily and
transparently reparent mux clocks.
This was loosely based on commit 8adfb08605a9 ("clk: sunxi-ng: mux: Add
clk notifier functions").
Signed-off-by: Chen-Yu Tsai <[email protected]>
[Angelo: Changed mtk_mux_nb to hold a pointer to clk_ops instead of mtk_mux]
Co-Developed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Miles Chen <[email protected]>
---
drivers/clk/mediatek/clk-mux.c | 38 ++++++++++++++++++++++++++++++++++
drivers/clk/mediatek/clk-mux.h | 15 ++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
index cd5f9fd8cb98..4421e4859257 100644
--- a/drivers/clk/mediatek/clk-mux.c
+++ b/drivers/clk/mediatek/clk-mux.c
@@ -4,6 +4,7 @@
* Author: Owen Chen <[email protected]>
*/
+#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/compiler_types.h>
#include <linux/container_of.h>
@@ -259,4 +260,41 @@ void mtk_clk_unregister_muxes(const struct mtk_mux *muxes, int num,
}
EXPORT_SYMBOL_GPL(mtk_clk_unregister_muxes);
+/*
+ * This clock notifier is called when the frequency of the parent
+ * PLL clock is to be changed. The idea is to switch the parent to a
+ * stable clock, such as the main oscillator, while the PLL frequency
+ * stabilizes.
+ */
+static int mtk_clk_mux_notifier_cb(struct notifier_block *nb,
+ unsigned long event, void *_data)
+{
+ struct clk_notifier_data *data = _data;
+ struct clk_hw *hw = __clk_get_hw(data->clk);
+ struct mtk_mux_nb *mux_nb = to_mtk_mux_nb(nb);
+ int ret = 0;
+
+ switch (event) {
+ case PRE_RATE_CHANGE:
+ mux_nb->original_index = mux_nb->ops->get_parent(hw);
+ ret = mux_nb->ops->set_parent(hw, mux_nb->bypass_index);
+ break;
+ case POST_RATE_CHANGE:
+ case ABORT_RATE_CHANGE:
+ ret = mux_nb->ops->set_parent(hw, mux_nb->original_index);
+ break;
+ }
+
+ return notifier_from_errno(ret);
+}
+
+int devm_mtk_clk_mux_notifier_register(struct device *dev, struct clk *clk,
+ struct mtk_mux_nb *mux_nb)
+{
+ mux_nb->nb.notifier_call = mtk_clk_mux_notifier_cb;
+
+ return devm_clk_notifier_register(dev, clk, &mux_nb->nb);
+}
+EXPORT_SYMBOL_GPL(devm_mtk_clk_mux_notifier_register);
+
MODULE_LICENSE("GPL");
diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
index 6539c58f5d7d..83ff420f4ebe 100644
--- a/drivers/clk/mediatek/clk-mux.h
+++ b/drivers/clk/mediatek/clk-mux.h
@@ -7,12 +7,14 @@
#ifndef __DRV_CLK_MTK_MUX_H
#define __DRV_CLK_MTK_MUX_H
+#include <linux/notifier.h>
#include <linux/spinlock.h>
#include <linux/types.h>
struct clk;
struct clk_hw_onecell_data;
struct clk_ops;
+struct device;
struct device_node;
struct mtk_mux {
@@ -89,4 +91,17 @@ int mtk_clk_register_muxes(const struct mtk_mux *muxes,
void mtk_clk_unregister_muxes(const struct mtk_mux *muxes, int num,
struct clk_hw_onecell_data *clk_data);
+struct mtk_mux_nb {
+ struct notifier_block nb;
+ const struct clk_ops *ops;
+
+ u8 bypass_index; /* Which parent to temporarily use */
+ u8 original_index; /* Set by notifier callback */
+};
+
+#define to_mtk_mux_nb(_nb) container_of(_nb, struct mtk_mux_nb, nb)
+
+int devm_mtk_clk_mux_notifier_register(struct device *dev, struct clk *clk,
+ struct mtk_mux_nb *mux_nb);
+
#endif /* __DRV_CLK_MTK_MUX_H */
--
2.37.2
From: Chen-Yu Tsai <[email protected]>
When the MFG PLL clock, which is upstream of the MFG clock, is changed,
the downstream clock and consumers need to be switched away from the PLL
over to a stable clock to avoid glitches.
This is done through the use of the newly added clk mux notifier. The
notifier is set on the mux itself instead of the upstream PLL, but in
practice this works, as the rate change notifitcations are propogated
throughout the sub-tree hanging off the PLL. Just before rate changes,
the MFG mux is temporarily and transparently switched to the 26 MHz
main crystal. After the rate change, the mux is switched back.
Signed-off-by: Chen-Yu Tsai <[email protected]>
[Angelo: Rebased to assign clk_ops in mtk_mux_nb]
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Miles Chen <[email protected]>
---
drivers/clk/mediatek/clk-mt8183.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mt8183.c b/drivers/clk/mediatek/clk-mt8183.c
index 8512101e1189..1860a35a723a 100644
--- a/drivers/clk/mediatek/clk-mt8183.c
+++ b/drivers/clk/mediatek/clk-mt8183.c
@@ -1198,10 +1198,33 @@ static void clk_mt8183_top_init_early(struct device_node *node)
CLK_OF_DECLARE_DRIVER(mt8183_topckgen, "mediatek,mt8183-topckgen",
clk_mt8183_top_init_early);
+/* Register mux notifier for MFG mux */
+static int clk_mt8183_reg_mfg_mux_notifier(struct device *dev, struct clk *clk)
+{
+ struct mtk_mux_nb *mfg_mux_nb;
+ int i;
+
+ mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb), GFP_KERNEL);
+ if (!mfg_mux_nb)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(top_muxes); i++)
+ if (top_muxes[i].id == CLK_TOP_MUX_MFG)
+ break;
+ if (i == ARRAY_SIZE(top_muxes))
+ return -EINVAL;
+
+ mfg_mux_nb->ops = top_muxes[i].ops;
+ mfg_mux_nb->bypass_index = 0; /* Bypass to 26M crystal */
+
+ return devm_mtk_clk_mux_notifier_register(dev, clk, mfg_mux_nb);
+}
+
static int clk_mt8183_top_probe(struct platform_device *pdev)
{
void __iomem *base;
struct device_node *node = pdev->dev.of_node;
+ int ret;
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
@@ -1227,6 +1250,11 @@ static int clk_mt8183_top_probe(struct platform_device *pdev)
mtk_clk_register_gates(node, top_clks, ARRAY_SIZE(top_clks),
top_clk_data);
+ ret = clk_mt8183_reg_mfg_mux_notifier(&pdev->dev,
+ top_clk_data->hws[CLK_TOP_MUX_MFG]->clk);
+ if (ret)
+ return ret;
+
return of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
top_clk_data);
}
--
2.37.2
The MFG_BG3D is a gate to enable/disable clock output to the GPU,
but the actual output is decided by multiple muxes; in particular:
mfg_ck_fast_ref muxes between "slow" (top_mfg_core_tmp) and
"fast" (MFGPLL) clock, while top_mfg_core_tmp muxes between the
26MHz clock and various system PLLs.
This also implies that "top_mfg_core_tmp" is a parent of the
"mfg_ck_fast_ref" mux (and not vice-versa), so reparent the
MFG_BG3D gate to the latter and add the CLK_SET_RATE_PARENT
flag to it: this way we ensure propagating rate changes that
are requested on MFG_BG3D along its entire clock tree.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c
index 9411c556a5a9..c94cb71bd9b9 100644
--- a/drivers/clk/mediatek/clk-mt8195-mfg.c
+++ b/drivers/clk/mediatek/clk-mt8195-mfg.c
@@ -17,10 +17,12 @@ static const struct mtk_gate_regs mfg_cg_regs = {
};
#define GATE_MFG(_id, _name, _parent, _shift) \
- GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
+ GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
+ _shift, &mtk_clk_gate_ops_setclr, \
+ CLK_SET_RATE_PARENT)
static const struct mtk_gate mfg_clks[] = {
- GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "top_mfg_core_tmp", 0),
+ GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_ck_fast_ref", 0),
};
static const struct mtk_clk_desc mfg_desc = {
--
2.37.2
Following what was done on MT8183 and MT8195, also propagate the rate
changes to MFG_BG3D's parent on MT8192 to allow for proper GPU DVFS.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8192-mfg.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8192-mfg.c b/drivers/clk/mediatek/clk-mt8192-mfg.c
index 3bbc7469f0e4..8ea5acdf832c 100644
--- a/drivers/clk/mediatek/clk-mt8192-mfg.c
+++ b/drivers/clk/mediatek/clk-mt8192-mfg.c
@@ -18,8 +18,10 @@ static const struct mtk_gate_regs mfg_cg_regs = {
.sta_ofs = 0x0,
};
-#define GATE_MFG(_id, _name, _parent, _shift) \
- GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
+#define GATE_MFG(_id, _name, _parent, _shift) \
+ GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
+ _shift, &mtk_clk_gate_ops_setclr, \
+ CLK_SET_RATE_PARENT)
static const struct mtk_gate mfg_clks[] = {
GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_pll_sel", 0),
--
2.37.2
Following the changes done to MT8183, register a similar notifier
for MT8195 as well, allowing safe clockrate updates for the MFGPLL.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Miles Chen <[email protected]>
---
drivers/clk/mediatek/clk-mt8195-topckgen.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mt8195-topckgen.c b/drivers/clk/mediatek/clk-mt8195-topckgen.c
index e1c3ab4e146b..4dde23bece66 100644
--- a/drivers/clk/mediatek/clk-mt8195-topckgen.c
+++ b/drivers/clk/mediatek/clk-mt8195-topckgen.c
@@ -1217,6 +1217,21 @@ static const struct of_device_id of_match_clk_mt8195_topck[] = {
{}
};
+/* Register mux notifier for MFG mux */
+static int clk_mt8195_reg_mfg_mux_notifier(struct device *dev, struct clk *clk)
+{
+ struct mtk_mux_nb *mfg_mux_nb;
+
+ mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb), GFP_KERNEL);
+ if (!mfg_mux_nb)
+ return -ENOMEM;
+
+ mfg_mux_nb->ops = &clk_mux_ops;
+ mfg_mux_nb->bypass_index = 0; /* Bypass to TOP_MFG_CORE_TMP */
+
+ return devm_mtk_clk_mux_notifier_register(dev, clk, mfg_mux_nb);
+}
+
static int clk_mt8195_topck_probe(struct platform_device *pdev)
{
struct clk_hw_onecell_data *top_clk_data;
@@ -1256,6 +1271,11 @@ static int clk_mt8195_topck_probe(struct platform_device *pdev)
goto unregister_muxes;
top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF] = hw;
+ r = clk_mt8195_reg_mfg_mux_notifier(&pdev->dev,
+ top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF]->clk);
+ if (r)
+ goto unregister_muxes;
+
r = mtk_clk_register_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), base,
&mt8195_clk_lock, top_clk_data);
if (r)
--
2.37.2
This clock was being registered as clk-composite through the helpers
for the same in the MediaTek clock APIs but, in reality, this isn't
a composite clock.
Appropriately register this clock with devm_clk_hw_register_mux().
No functional changes.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8195-topckgen.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8195-topckgen.c b/drivers/clk/mediatek/clk-mt8195-topckgen.c
index ec70e1f65eaf..e1c3ab4e146b 100644
--- a/drivers/clk/mediatek/clk-mt8195-topckgen.c
+++ b/drivers/clk/mediatek/clk-mt8195-topckgen.c
@@ -1149,11 +1149,6 @@ static const struct mtk_mux top_mtk_muxes[] = {
*/
};
-static struct mtk_composite top_muxes[] = {
- /* CLK_MISC_CFG_3 */
- MUX(CLK_TOP_MFG_CK_FAST_REF, "mfg_ck_fast_ref", mfg_fast_parents, 0x0250, 8, 1),
-};
-
static const struct mtk_composite top_adj_divs[] = {
DIV_GATE(CLK_TOP_APLL12_DIV0, "apll12_div0", "top_i2si1_mck", 0x0320, 0, 0x0328, 8, 0),
DIV_GATE(CLK_TOP_APLL12_DIV1, "apll12_div1", "top_i2si2_mck", 0x0320, 1, 0x0328, 8, 8),
@@ -1226,6 +1221,7 @@ static int clk_mt8195_topck_probe(struct platform_device *pdev)
{
struct clk_hw_onecell_data *top_clk_data;
struct device_node *node = pdev->dev.of_node;
+ struct clk_hw *hw;
int r;
void __iomem *base;
@@ -1253,15 +1249,17 @@ static int clk_mt8195_topck_probe(struct platform_device *pdev)
if (r)
goto unregister_factors;
- r = mtk_clk_register_composites(top_muxes, ARRAY_SIZE(top_muxes), base,
- &mt8195_clk_lock, top_clk_data);
- if (r)
+ hw = devm_clk_hw_register_mux(&pdev->dev, "mfg_ck_fast_ref", mfg_fast_parents,
+ ARRAY_SIZE(mfg_fast_parents), CLK_SET_RATE_PARENT,
+ (base + 0x250), 8, 1, 0, &mt8195_clk_lock);
+ if (IS_ERR(hw))
goto unregister_muxes;
+ top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF] = hw;
r = mtk_clk_register_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), base,
&mt8195_clk_lock, top_clk_data);
if (r)
- goto unregister_composite_muxes;
+ goto unregister_muxes;
r = mtk_clk_register_gates(node, top_clks, ARRAY_SIZE(top_clks), top_clk_data);
if (r)
@@ -1279,8 +1277,6 @@ static int clk_mt8195_topck_probe(struct platform_device *pdev)
mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks), top_clk_data);
unregister_composite_divs:
mtk_clk_unregister_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), top_clk_data);
-unregister_composite_muxes:
- mtk_clk_unregister_composites(top_muxes, ARRAY_SIZE(top_muxes), top_clk_data);
unregister_muxes:
mtk_clk_unregister_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), top_clk_data);
unregister_factors:
@@ -1300,7 +1296,6 @@ static int clk_mt8195_topck_remove(struct platform_device *pdev)
of_clk_del_provider(node);
mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks), top_clk_data);
mtk_clk_unregister_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), top_clk_data);
- mtk_clk_unregister_composites(top_muxes, ARRAY_SIZE(top_muxes), top_clk_data);
mtk_clk_unregister_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), top_clk_data);
mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
mtk_clk_unregister_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
--
2.37.2
These PLLs are conflicting with GPU rates that can be generated by
the GPU-dedicated MFGPLL and would require a special clock handler
to be used, for very little and ignorable power consumption benefits.
Also, we're in any case unable to set the rate of these PLLs to
something else that is sensible for this task, so simply drop them:
this will make the GPU to be clocked exclusively from MFGPLL for
"fast" rates, while still achieving the right "safe" rate during
PLL frequency locking.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8195-topckgen.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8195-topckgen.c b/drivers/clk/mediatek/clk-mt8195-topckgen.c
index 4dde23bece66..8cbab5ca2e58 100644
--- a/drivers/clk/mediatek/clk-mt8195-topckgen.c
+++ b/drivers/clk/mediatek/clk-mt8195-topckgen.c
@@ -298,11 +298,14 @@ static const char * const ipu_if_parents[] = {
"mmpll_d4"
};
+/*
+ * MFG can be also parented to "univpll_d6" and "univpll_d7":
+ * these have been removed from the parents list to let us
+ * achieve GPU DVFS without any special clock handlers.
+ */
static const char * const mfg_parents[] = {
"clk26m",
- "mainpll_d5_d2",
- "univpll_d6",
- "univpll_d7"
+ "mainpll_d5_d2"
};
static const char * const camtg_parents[] = {
--
2.37.2
Following the changes that were done for mt8183, add a clock notifier
for the GPU PLL selector mux: this allows safe clock rate changes by
temporarily reparenting the GPU to a safe clock (clk26m) while the
MFGPLL is reprogrammed and stabilizes.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8192.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..187dbffeb987 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1224,6 +1224,28 @@ static void clk_mt8192_top_init_early(struct device_node *node)
CLK_OF_DECLARE_DRIVER(mt8192_topckgen, "mediatek,mt8192-topckgen",
clk_mt8192_top_init_early);
+/* Register mux notifier for MFG mux */
+static int clk_mt8192_reg_mfg_mux_notifier(struct device *dev, struct clk *clk)
+{
+ struct mtk_mux_nb *mfg_mux_nb;
+ int i;
+
+ mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb), GFP_KERNEL);
+ if (!mfg_mux_nb)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(top_mtk_muxes); i++)
+ if (top_mtk_muxes[i].id == CLK_TOP_MFG_PLL_SEL)
+ break;
+ if (i == ARRAY_SIZE(top_mtk_muxes))
+ return -EINVAL;
+
+ mfg_mux_nb->ops = top_mtk_muxes[i].ops;
+ mfg_mux_nb->bypass_index = 0; /* Bypass to 26M crystal */
+
+ return devm_mtk_clk_mux_notifier_register(dev, clk, mfg_mux_nb);
+}
+
static int clk_mt8192_top_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
@@ -1247,6 +1269,12 @@ static int clk_mt8192_top_probe(struct platform_device *pdev)
if (r)
return r;
+ r = clk_mt8192_reg_mfg_mux_notifier(&pdev->dev,
+ top_clk_data->hws[CLK_TOP_MFG_PLL_SEL]->clk);
+ if (r)
+ return r;
+
+
return of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
top_clk_data);
}
--
2.37.2
From: Chen-Yu Tsai <[email protected]>
The actual clock feeding into the Mali GPU on the MT8183 is from the
clock gate in the MFGCFG block, not CLK_TOP_MFGPLL_CK from the TOPCKGEN
block, which itself is simply a pass-through placeholder for the MFGPLL
in the APMIXEDSYS block.
Fix the hardware description with the correct clock reference.
Fixes: a8168cebf1bc ("arm64: dts: mt8183: Add node for the Mali GPU")
Signed-off-by: Chen-Yu Tsai <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index a70b669c49ba..402136bfd535 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1678,7 +1678,7 @@ gpu: gpu@13040000 {
<GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
interrupt-names = "job", "mmu", "gpu";
- clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
+ clocks = <&mfgcfg CLK_MFG_BG3D>;
power-domains =
<&spm MT8183_POWER_DOMAIN_MFG_CORE0>,
--
2.37.2
On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> These PLLs are conflicting with GPU rates that can be generated by
> the GPU-dedicated MFGPLL and would require a special clock handler
> to be used, for very little and ignorable power consumption benefits.
> Also, we're in any case unable to set the rate of these PLLs to
> something else that is sensible for this task, so simply drop them:
> this will make the GPU to be clocked exclusively from MFGPLL for
> "fast" rates, while still achieving the right "safe" rate during
> PLL frequency locking.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> The MFG_BG3D is a gate to enable/disable clock output to the GPU,
> but the actual output is decided by multiple muxes; in particular:
> mfg_ck_fast_ref muxes between "slow" (top_mfg_core_tmp) and
> "fast" (MFGPLL) clock, while top_mfg_core_tmp muxes between the
> 26MHz clock and various system PLLs.
>
> This also implies that "top_mfg_core_tmp" is a parent of the
> "mfg_ck_fast_ref" mux (and not vice-versa), so reparent the
I don't see where this was the case though? I think what you meant
was that the direct parent for "mfg_bg3d" is "mfg_ck_fast_ref, not
"top_mfg_core_tmp"?
> MFG_BG3D gate to the latter and add the CLK_SET_RATE_PARENT
> flag to it: this way we ensure propagating rate changes that
> are requested on MFG_BG3D along its entire clock tree.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
> ---
> drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c
> index 9411c556a5a9..c94cb71bd9b9 100644
> --- a/drivers/clk/mediatek/clk-mt8195-mfg.c
> +++ b/drivers/clk/mediatek/clk-mt8195-mfg.c
> @@ -17,10 +17,12 @@ static const struct mtk_gate_regs mfg_cg_regs = {
> };
>
> #define GATE_MFG(_id, _name, _parent, _shift) \
> - GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> + GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
> + _shift, &mtk_clk_gate_ops_setclr, \
> + CLK_SET_RATE_PARENT)
>
> static const struct mtk_gate mfg_clks[] = {
> - GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "top_mfg_core_tmp", 0),
> + GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_ck_fast_ref", 0),
> };
>
> static const struct mtk_clk_desc mfg_desc = {
> --
> 2.37.2
>
On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Following the changes that were done for mt8183, add a clock notifier
> for the GPU PLL selector mux: this allows safe clock rate changes by
> temporarily reparenting the GPU to a safe clock (clk26m) while the
> MFGPLL is reprogrammed and stabilizes.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> This clock was being registered as clk-composite through the helpers
> for the same in the MediaTek clock APIs but, in reality, this isn't
> a composite clock.
>
> Appropriately register this clock with devm_clk_hw_register_mux().
> No functional changes.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Following what was done on MT8183 and MT8195, also propagate the rate
> changes to MFG_BG3D's parent on MT8192 to allow for proper GPU DVFS.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Il 26/09/22 05:27, Chen-Yu Tsai ha scritto:
> On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> The MFG_BG3D is a gate to enable/disable clock output to the GPU,
>> but the actual output is decided by multiple muxes; in particular:
>> mfg_ck_fast_ref muxes between "slow" (top_mfg_core_tmp) and
>> "fast" (MFGPLL) clock, while top_mfg_core_tmp muxes between the
>> 26MHz clock and various system PLLs.
>>
>> This also implies that "top_mfg_core_tmp" is a parent of the
>> "mfg_ck_fast_ref" mux (and not vice-versa), so reparent the
>
> I don't see where this was the case though? I think what you meant
> was that the direct parent for "mfg_bg3d" is "mfg_ck_fast_ref, not
> "top_mfg_core_tmp"?
>
MFG_BG3D's direct parent is mfg_ck_fast_ref - yes - but in the commit message
I am explaining how the clock tree for MFG_BG3D really is and, in particular,
I'm then explaining that:
* parenting MFG_BG3D to "top_mfg_core_tmp" is wrong; because
* "top_mfg_core_tmp" is a parent of "mfg_ck_fast_ref" (not the other way around).
So, the question in your comment is addressed just a little later....
>> MFG_BG3D gate to the latter and add the CLK_SET_RATE_PARENT
...here, where I say "reparent MFG_BG3D to the latter", where "the latter" is,
exactly "mfg_ck_fast_ref".
I hope you now understand what I am trying to communicate :-)
However, if even after that you still think that the commit description should
be rewritten in some less tangled and/or more understandable way, I definitely
can do that.
Please confirm :-)
>> flag to it: this way we ensure propagating rate changes that
>> are requested on MFG_BG3D along its entire clock tree.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <[email protected]>
>
>
>> ---
>> drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c
>> index 9411c556a5a9..c94cb71bd9b9 100644
>> --- a/drivers/clk/mediatek/clk-mt8195-mfg.c
>> +++ b/drivers/clk/mediatek/clk-mt8195-mfg.c
>> @@ -17,10 +17,12 @@ static const struct mtk_gate_regs mfg_cg_regs = {
>> };
>>
>> #define GATE_MFG(_id, _name, _parent, _shift) \
>> - GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
>> + GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
>> + _shift, &mtk_clk_gate_ops_setclr, \
>> + CLK_SET_RATE_PARENT)
>>
>> static const struct mtk_gate mfg_clks[] = {
>> - GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "top_mfg_core_tmp", 0),
>> + GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_ck_fast_ref", 0),
>> };
>>
>> static const struct mtk_clk_desc mfg_desc = {
>> --
>> 2.37.2
>>
On Mon, Sep 26, 2022 at 4:36 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 26/09/22 05:27, Chen-Yu Tsai ha scritto:
> > On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
> > <[email protected]> wrote:
> >>
> >> The MFG_BG3D is a gate to enable/disable clock output to the GPU,
> >> but the actual output is decided by multiple muxes; in particular:
> >> mfg_ck_fast_ref muxes between "slow" (top_mfg_core_tmp) and
> >> "fast" (MFGPLL) clock, while top_mfg_core_tmp muxes between the
> >> 26MHz clock and various system PLLs.
> >>
> >> This also implies that "top_mfg_core_tmp" is a parent of the
> >> "mfg_ck_fast_ref" mux (and not vice-versa), so reparent the
> >
> > I don't see where this was the case though? I think what you meant
> > was that the direct parent for "mfg_bg3d" is "mfg_ck_fast_ref, not
> > "top_mfg_core_tmp"?
> >
>
> MFG_BG3D's direct parent is mfg_ck_fast_ref - yes - but in the commit message
> I am explaining how the clock tree for MFG_BG3D really is and, in particular,
> I'm then explaining that:
> * parenting MFG_BG3D to "top_mfg_core_tmp" is wrong; because
> * "top_mfg_core_tmp" is a parent of "mfg_ck_fast_ref" (not the other way around).
>
> So, the question in your comment is addressed just a little later....
>
> >> MFG_BG3D gate to the latter and add the CLK_SET_RATE_PARENT
>
> ...here, where I say "reparent MFG_BG3D to the latter", where "the latter" is,
> exactly "mfg_ck_fast_ref".
>
> I hope you now understand what I am trying to communicate :-)
>
> However, if even after that you still think that the commit description should
> be rewritten in some less tangled and/or more understandable way, I definitely
> can do that.
>
> Please confirm :-)
I think
This also implies that "top_mfg_core_tmp" is a parent of the
"mfg_ck_fast_ref" mux (and not vice-versa)
actually confused me.
Maybe just say
The clock gate comes after all the muxes, so its parent is
mfg_ck_fast_ref, not top_mfg_core_tmp. Reparent mfg_bg3d to
the latter to match the hardware ...
Since you are fixing the topology, could you also add a fixes tag?
Thanks
ChenYu
> >> flag to it: this way we ensure propagating rate changes that
> >> are requested on MFG_BG3D along its entire clock tree.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> >
> > Otherwise,
> >
> > Reviewed-by: Chen-Yu Tsai <[email protected]>
> >
> >
> >> ---
> >> drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c
> >> index 9411c556a5a9..c94cb71bd9b9 100644
> >> --- a/drivers/clk/mediatek/clk-mt8195-mfg.c
> >> +++ b/drivers/clk/mediatek/clk-mt8195-mfg.c
> >> @@ -17,10 +17,12 @@ static const struct mtk_gate_regs mfg_cg_regs = {
> >> };
> >>
> >> #define GATE_MFG(_id, _name, _parent, _shift) \
> >> - GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> >> + GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
> >> + _shift, &mtk_clk_gate_ops_setclr, \
> >> + CLK_SET_RATE_PARENT)
> >>
> >> static const struct mtk_gate mfg_clks[] = {
> >> - GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "top_mfg_core_tmp", 0),
> >> + GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_ck_fast_ref", 0),
> >> };
> >>
> >> static const struct mtk_clk_desc mfg_desc = {
> >> --
> >> 2.37.2
> >>
>
>
Il 26/09/22 10:57, Chen-Yu Tsai ha scritto:
> On Mon, Sep 26, 2022 at 4:36 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Il 26/09/22 05:27, Chen-Yu Tsai ha scritto:
>>> On Thu, Sep 15, 2022 at 3:25 PM AngeloGioacchino Del Regno
>>> <[email protected]> wrote:
>>>>
>>>> The MFG_BG3D is a gate to enable/disable clock output to the GPU,
>>>> but the actual output is decided by multiple muxes; in particular:
>>>> mfg_ck_fast_ref muxes between "slow" (top_mfg_core_tmp) and
>>>> "fast" (MFGPLL) clock, while top_mfg_core_tmp muxes between the
>>>> 26MHz clock and various system PLLs.
>>>>
>>>> This also implies that "top_mfg_core_tmp" is a parent of the
>>>> "mfg_ck_fast_ref" mux (and not vice-versa), so reparent the
>>>
>>> I don't see where this was the case though? I think what you meant
>>> was that the direct parent for "mfg_bg3d" is "mfg_ck_fast_ref, not
>>> "top_mfg_core_tmp"?
>>>
>>
>> MFG_BG3D's direct parent is mfg_ck_fast_ref - yes - but in the commit message
>> I am explaining how the clock tree for MFG_BG3D really is and, in particular,
>> I'm then explaining that:
>> * parenting MFG_BG3D to "top_mfg_core_tmp" is wrong; because
>> * "top_mfg_core_tmp" is a parent of "mfg_ck_fast_ref" (not the other way around).
>>
>> So, the question in your comment is addressed just a little later....
>>
>>>> MFG_BG3D gate to the latter and add the CLK_SET_RATE_PARENT
>>
>> ...here, where I say "reparent MFG_BG3D to the latter", where "the latter" is,
>> exactly "mfg_ck_fast_ref".
>>
>> I hope you now understand what I am trying to communicate :-)
>>
>> However, if even after that you still think that the commit description should
>> be rewritten in some less tangled and/or more understandable way, I definitely
>> can do that.
>>
>> Please confirm :-)
>
> I think
>
> This also implies that "top_mfg_core_tmp" is a parent of the
> "mfg_ck_fast_ref" mux (and not vice-versa)
>
> actually confused me.
>
> Maybe just say
>
> The clock gate comes after all the muxes, so its parent is
> mfg_ck_fast_ref, not top_mfg_core_tmp. Reparent mfg_bg3d to
> the latter to match the hardware ...
>
> Since you are fixing the topology, could you also add a fixes tag?
>
>
Yeah, sure! I'll send a new version most probably today.
Thanks for the review(s)!
Cheers,
Angelo
> Thanks
> ChenYu
>
>
>>>> flag to it: this way we ensure propagating rate changes that
>>>> are requested on MFG_BG3D along its entire clock tree.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>>>
>>> Otherwise,
>>>
>>> Reviewed-by: Chen-Yu Tsai <[email protected]>
>>>
>>>
>>>> ---
>>>> drivers/clk/mediatek/clk-mt8195-mfg.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/mediatek/clk-mt8195-mfg.c b/drivers/clk/mediatek/clk-mt8195-mfg.c
>>>> index 9411c556a5a9..c94cb71bd9b9 100644
>>>> --- a/drivers/clk/mediatek/clk-mt8195-mfg.c
>>>> +++ b/drivers/clk/mediatek/clk-mt8195-mfg.c
>>>> @@ -17,10 +17,12 @@ static const struct mtk_gate_regs mfg_cg_regs = {
>>>> };
>>>>
>>>> #define GATE_MFG(_id, _name, _parent, _shift) \
>>>> - GATE_MTK(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
>>>> + GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, \
>>>> + _shift, &mtk_clk_gate_ops_setclr, \
>>>> + CLK_SET_RATE_PARENT)
>>>>
>>>> static const struct mtk_gate mfg_clks[] = {
>>>> - GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "top_mfg_core_tmp", 0),
>>>> + GATE_MFG(CLK_MFG_BG3D, "mfg_bg3d", "mfg_ck_fast_ref", 0),
>>>> };
>>>>
>>>> static const struct mtk_clk_desc mfg_desc = {
>>>> --
>>>> 2.37.2
>>>>
>>
>>