[PATCH v3 00/10] MediaTek SoC safe clock muxing and GPU clocks

[PATCH v3 00/10] MediaTek SoC safe clock muxing and GPU clocks

[AngeloGioacchino Del Regno]

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/20220628120224.81180-1-angelogioacchino.delregno@collabora.com/

* 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 v3:
 - Clarified commit description in patch [05/10]

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

[PATCH v3 00/10] MediaTek SoC safe clock muxing and GPU clocks

[AngeloGioacchino Del Regno]

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/20220628120224.81180-1-angelogioacchino.delregno@collabora.com/

* 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 v3:
 - Clarified commit description in patch [05/10]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 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

[PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 02/10] clk: mediatek: mt8183: mfgcfg: Propagate rate changes to parent

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

The only clock in the MT8183 MFGCFG block feeds the GPU. Propagate its
rate change requests to its parent, so that DVFS for the GPU can work
properly.

Fixes: acddfc2c261b ("clk: mediatek: Add MT8183 clock support")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
index d774edaf760b..230299728859 100644
--- a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
+++ b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
@@ -18,9 +18,9 @@ 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_sel", 0)
-- 
2.37.2

[PATCH v3 02/10] clk: mediatek: mt8183: mfgcfg: Propagate rate changes to parent

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

The only clock in the MT8183 MFGCFG block feeds the GPU. Propagate its
rate change requests to its parent, so that DVFS for the GPU can work
properly.

Fixes: acddfc2c261b ("clk: mediatek: Add MT8183 clock support")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
index d774edaf760b..230299728859 100644
--- a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
+++ b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
@@ -18,9 +18,9 @@ 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_sel", 0)
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 03/10] clk: mediatek: mux: add clk notifier functions

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
[Angelo: Changed mtk_mux_nb to hold a pointer to clk_ops instead of mtk_mux]
Co-developed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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 <owen.chen@mediatek.com>
  */
 
+#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

[PATCH v3 03/10] clk: mediatek: mux: add clk notifier functions

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
[Angelo: Changed mtk_mux_nb to hold a pointer to clk_ops instead of mtk_mux]
Co-developed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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 <owen.chen@mediatek.com>
  */
 
+#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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 04/10] clk: mediatek: mt8183: Add clk mux notifier for MFG mux

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
[Angelo: Rebased to assign clk_ops in mtk_mux_nb]
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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

[PATCH v3 04/10] clk: mediatek: mt8183: Add clk mux notifier for MFG mux

[AngeloGioacchino Del Regno]

From: Chen-Yu Tsai <wenst@chromium.org>

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 <wenst@chromium.org>
[Angelo: Rebased to assign clk_ops in mtk_mux_nb]
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 05/10] clk: mediatek: clk-mt8195-mfg: Reparent mfg_bg3d and propagate rate changes

[AngeloGioacchino Del Regno]

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.

The clock gate comes after all the muxes, so its parent is
mfg_ck_fast_reg, not top_mfg_core_tmp.
Reparent MFG_BG3D to the latter to match the hardware 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.

Fixes: 35016f10c0e5 ("clk: mediatek: Add MT8195 mfgcfg clock support")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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

[PATCH v3 05/10] clk: mediatek: clk-mt8195-mfg: Reparent mfg_bg3d and propagate rate changes

[AngeloGioacchino Del Regno]

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.

The clock gate comes after all the muxes, so its parent is
mfg_ck_fast_reg, not top_mfg_core_tmp.
Reparent MFG_BG3D to the latter to match the hardware 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.

Fixes: 35016f10c0e5 ("clk: mediatek: Add MT8195 mfgcfg clock support")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 06/10] clk: mediatek: clk-mt8195-topckgen: Register mfg_ck_fast_ref as generic mux

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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

[PATCH v3 06/10] clk: mediatek: clk-mt8195-topckgen: Register mfg_ck_fast_ref as generic mux

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 07/10] clk: mediatek: clk-mt8195-topckgen: Add GPU clock mux notifier

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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

[PATCH v3 07/10] clk: mediatek: clk-mt8195-topckgen: Add GPU clock mux notifier

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Miles Chen <miles.chen@mediatek.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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

[PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 09/10] clk: mediatek: clk-mt8192-mfg: Propagate rate changes to parent

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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

[PATCH v3 09/10] clk: mediatek: clk-mt8192-mfg: Propagate rate changes to parent

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 10/10] clk: mediatek: clk-mt8192: Add clock mux notifier for mfg_pll_sel

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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

[PATCH v3 10/10] clk: mediatek: clk-mt8192: Add clock mux notifier for mfg_pll_sel

[AngeloGioacchino Del Regno]

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 <angelogioacchino.delregno@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 00/10] MediaTek SoC safe clock muxing and GPU clocks

[Chen-Yu Tsai]

On Tue, Sep 27, 2022 at 6:11 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> 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/20220628120224.81180-1-angelogioacchino.delregno@collabora.com/
>
> * 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 v3:
>  - Clarified commit description in patch [05/10]
>
> 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


I've queued all the clk patches up here [1] and will send a pull request
to the clock maintainer later this week.

The dts patch needs to go through the soc tree.

ChenYu

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/log/?h=clk-mtk-for-6.1

Re: [PATCH v3 00/10] MediaTek SoC safe clock muxing and GPU clocks

[Chen-Yu Tsai]

On Tue, Sep 27, 2022 at 6:11 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> 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/20220628120224.81180-1-angelogioacchino.delregno@collabora.com/
>
> * 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 v3:
>  - Clarified commit description in patch [05/10]
>
> 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


I've queued all the clk patches up here [1] and will send a pull request
to the clock maintainer later this week.

The dts patch needs to go through the soc tree.

ChenYu

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/log/?h=clk-mtk-for-6.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[MandyJH Liu (劉人僖)]

On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  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[] = {
There might be a problem here. Since the univpll_d6 and univpll_d7 are
available parents in hardware design and they can be selected other
than kernel stage, like bootloader, the clk tree listed in clk_summary
cannot show the real parent-child relationship in such case.

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[MandyJH Liu (劉人僖)]

On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  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[] = {
There might be a problem here. Since the univpll_d6 and univpll_d7 are
available parents in hardware design and they can be selected other
than kernel stage, like bootloader, the clk tree listed in clk_summary
cannot show the real parent-child relationship in such case.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>> angelogioacchino.delregno@collabora.com>
>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>> ---
>>   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[] = {
> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> available parents in hardware design and they can be selected other
> than kernel stage, like bootloader, the clk tree listed in clk_summary
> cannot show the real parent-child relationship in such case.

I agree about that, but the clock framework will change the parent to
the "best parent" in that case... this was done to avoid writing complicated
custom clock ops just for that one.

This issue is present only on MT8195, so it can be safely solved this way,
at least for now.

Should this become a thing on another couple SoCs, it'll then make sense
to write custom clock ops just for the MFG.

Regards,
Angelo

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>> angelogioacchino.delregno@collabora.com>
>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>> ---
>>   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[] = {
> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> available parents in hardware design and they can be selected other
> than kernel stage, like bootloader, the clk tree listed in clk_summary
> cannot show the real parent-child relationship in such case.

I agree about that, but the clock framework will change the parent to
the "best parent" in that case... this was done to avoid writing complicated
custom clock ops just for that one.

This issue is present only on MT8195, so it can be safely solved this way,
at least for now.

Should this become a thing on another couple SoCs, it'll then make sense
to write custom clock ops just for the MFG.

Regards,
Angelo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> > On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >> angelogioacchino.delregno@collabora.com>
> >> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >> ---
> >>   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[] = {
> > There might be a problem here. Since the univpll_d6 and univpll_d7 are
> > available parents in hardware design and they can be selected other
> > than kernel stage, like bootloader, the clk tree listed in clk_summary
> > cannot show the real parent-child relationship in such case.
>
> I agree about that, but the clock framework will change the parent to
> the "best parent" in that case... this was done to avoid writing complicated
> custom clock ops just for that one.
>
> This issue is present only on MT8195, so it can be safely solved this way,
> at least for now.
>
> Should this become a thing on another couple SoCs, it'll then make sense
> to write custom clock ops just for the MFG.

Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
the clk tree to a state that we like (mfgpll->fast_mux->gate) work?

ChenYu

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> > On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >> angelogioacchino.delregno@collabora.com>
> >> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >> ---
> >>   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[] = {
> > There might be a problem here. Since the univpll_d6 and univpll_d7 are
> > available parents in hardware design and they can be selected other
> > than kernel stage, like bootloader, the clk tree listed in clk_summary
> > cannot show the real parent-child relationship in such case.
>
> I agree about that, but the clock framework will change the parent to
> the "best parent" in that case... this was done to avoid writing complicated
> custom clock ops just for that one.
>
> This issue is present only on MT8195, so it can be safely solved this way,
> at least for now.
>
> Should this become a thing on another couple SoCs, it'll then make sense
> to write custom clock ops just for the MFG.

Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
the clk tree to a state that we like (mfgpll->fast_mux->gate) work?

ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>>>> angelogioacchino.delregno@collabora.com>
>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>>> ---
>>>>    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[] = {
>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
>>> available parents in hardware design and they can be selected other
>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
>>> cannot show the real parent-child relationship in such case.
>>
>> I agree about that, but the clock framework will change the parent to
>> the "best parent" in that case... this was done to avoid writing complicated
>> custom clock ops just for that one.
>>
>> This issue is present only on MT8195, so it can be safely solved this way,
>> at least for now.
>>
>> Should this become a thing on another couple SoCs, it'll then make sense
>> to write custom clock ops just for the MFG.
> 
> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?

I'm not sure that it would, and then this would mean that we'd have to add
assigned-clock-parents to the devicetree and the day we will introduce the
"complicated custom clock ops" for that, we'll most probably have to change
the devicetree as well... which is something that I'm a bit reluctant to do
as a kernel upgrade doesn't automatically mean that you upgrade the DT with
it to get the "new full functionality".

Introducing the new clock ops for the mfg mux is something that will happen
for sure, but if we don't get new SoCs with a similar "issue", I don't feel
confident to write them, as I fear these won't be as flexible as needed and
will eventually need a rewrite; that's why I want to wait to get the same
situation on "something new".

In my opinion, it is safe to keep this change as it is, even though I do
understand the shown concerns about the eventual unability to show the tree
relationship in case the bootloader chooses to initialize the mfg mux with
a univpll parent.

Regards,
Angelo

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>>>> angelogioacchino.delregno@collabora.com>
>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>>> ---
>>>>    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[] = {
>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
>>> available parents in hardware design and they can be selected other
>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
>>> cannot show the real parent-child relationship in such case.
>>
>> I agree about that, but the clock framework will change the parent to
>> the "best parent" in that case... this was done to avoid writing complicated
>> custom clock ops just for that one.
>>
>> This issue is present only on MT8195, so it can be safely solved this way,
>> at least for now.
>>
>> Should this become a thing on another couple SoCs, it'll then make sense
>> to write custom clock ops just for the MFG.
> 
> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?

I'm not sure that it would, and then this would mean that we'd have to add
assigned-clock-parents to the devicetree and the day we will introduce the
"complicated custom clock ops" for that, we'll most probably have to change
the devicetree as well... which is something that I'm a bit reluctant to do
as a kernel upgrade doesn't automatically mean that you upgrade the DT with
it to get the "new full functionality".

Introducing the new clock ops for the mfg mux is something that will happen
for sure, but if we don't get new SoCs with a similar "issue", I don't feel
confident to write them, as I fear these won't be as flexible as needed and
will eventually need a rewrite; that's why I want to wait to get the same
situation on "something new".

In my opinion, it is safe to keep this change as it is, even though I do
understand the shown concerns about the eventual unability to show the tree
relationship in case the bootloader chooses to initialize the mfg mux with
a univpll parent.

Regards,
Angelo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> > On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> >>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >>>> angelogioacchino.delregno@collabora.com>
> >>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>> ---
> >>>>    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[] = {
> >>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> >>> available parents in hardware design and they can be selected other
> >>> than kernel stage, like bootloader, the clk tree listed in clk_summary
> >>> cannot show the real parent-child relationship in such case.
> >>
> >> I agree about that, but the clock framework will change the parent to
> >> the "best parent" in that case... this was done to avoid writing complicated
> >> custom clock ops just for that one.
> >>
> >> This issue is present only on MT8195, so it can be safely solved this way,
> >> at least for now.
> >>
> >> Should this become a thing on another couple SoCs, it'll then make sense
> >> to write custom clock ops just for the MFG.
> >
> > Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> > the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
>
> I'm not sure that it would, and then this would mean that we'd have to add
> assigned-clock-parents to the devicetree and the day we will introduce the
> "complicated custom clock ops" for that, we'll most probably have to change
> the devicetree as well... which is something that I'm a bit reluctant to do
> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
> it to get the "new full functionality".

You can also do it by doing clk_set_parent() in the clock driver after the
clocks are registered, or just write to the register before the clock is
registered.

We do the latter in some of the sunxi-ng drivers, though IIRC it was to
force a certain divider on what we expose as a fixed divider clock.

ChenYu

> Introducing the new clock ops for the mfg mux is something that will happen
> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
> confident to write them, as I fear these won't be as flexible as needed and
> will eventually need a rewrite; that's why I want to wait to get the same
> situation on "something new".
>
> In my opinion, it is safe to keep this change as it is, even though I do
> understand the shown concerns about the eventual unability to show the tree
> relationship in case the bootloader chooses to initialize the mfg mux with
> a univpll parent.
>
> Regards,
> Angelo
>

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> > On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> >>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >>>> angelogioacchino.delregno@collabora.com>
> >>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>> ---
> >>>>    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[] = {
> >>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> >>> available parents in hardware design and they can be selected other
> >>> than kernel stage, like bootloader, the clk tree listed in clk_summary
> >>> cannot show the real parent-child relationship in such case.
> >>
> >> I agree about that, but the clock framework will change the parent to
> >> the "best parent" in that case... this was done to avoid writing complicated
> >> custom clock ops just for that one.
> >>
> >> This issue is present only on MT8195, so it can be safely solved this way,
> >> at least for now.
> >>
> >> Should this become a thing on another couple SoCs, it'll then make sense
> >> to write custom clock ops just for the MFG.
> >
> > Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> > the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
>
> I'm not sure that it would, and then this would mean that we'd have to add
> assigned-clock-parents to the devicetree and the day we will introduce the
> "complicated custom clock ops" for that, we'll most probably have to change
> the devicetree as well... which is something that I'm a bit reluctant to do
> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
> it to get the "new full functionality".

You can also do it by doing clk_set_parent() in the clock driver after the
clocks are registered, or just write to the register before the clock is
registered.

We do the latter in some of the sunxi-ng drivers, though IIRC it was to
force a certain divider on what we expose as a fixed divider clock.

ChenYu

> Introducing the new clock ops for the mfg mux is something that will happen
> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
> confident to write them, as I fear these won't be as flexible as needed and
> will eventually need a rewrite; that's why I want to wait to get the same
> situation on "something new".
>
> In my opinion, it is safe to keep this change as it is, even though I do
> understand the shown concerns about the eventual unability to show the tree
> relationship in case the bootloader chooses to initialize the mfg mux with
> a univpll parent.
>
> Regards,
> Angelo
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 11:02, Chen-Yu Tsai ha scritto:
> On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
>>> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
>>>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>>>>>> angelogioacchino.delregno@collabora.com>
>>>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>>> ---
>>>>>>     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[] = {
>>>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
>>>>> available parents in hardware design and they can be selected other
>>>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
>>>>> cannot show the real parent-child relationship in such case.
>>>>
>>>> I agree about that, but the clock framework will change the parent to
>>>> the "best parent" in that case... this was done to avoid writing complicated
>>>> custom clock ops just for that one.
>>>>
>>>> This issue is present only on MT8195, so it can be safely solved this way,
>>>> at least for now.
>>>>
>>>> Should this become a thing on another couple SoCs, it'll then make sense
>>>> to write custom clock ops just for the MFG.
>>>
>>> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
>>> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
>>
>> I'm not sure that it would, and then this would mean that we'd have to add
>> assigned-clock-parents to the devicetree and the day we will introduce the
>> "complicated custom clock ops" for that, we'll most probably have to change
>> the devicetree as well... which is something that I'm a bit reluctant to do
>> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
>> it to get the "new full functionality".
> 
> You can also do it by doing clk_set_parent() in the clock driver after the
> clocks are registered, or just write to the register before the clock is
> registered.
> 

I honestly don't like doing that - but I can try if that works and, if it does,
I can send a commit with a Fixes tag later, perhaps?


> We do the latter in some of the sunxi-ng drivers, though IIRC it was to
> force a certain divider on what we expose as a fixed divider clock.
> 
> ChenYu
> 
>> Introducing the new clock ops for the mfg mux is something that will happen
>> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
>> confident to write them, as I fear these won't be as flexible as needed and
>> will eventually need a rewrite; that's why I want to wait to get the same
>> situation on "something new".
>>
>> In my opinion, it is safe to keep this change as it is, even though I do
>> understand the shown concerns about the eventual unability to show the tree
>> relationship in case the bootloader chooses to initialize the mfg mux with
>> a univpll parent.
>>
>> Regards,
>> Angelo
>>

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[AngeloGioacchino Del Regno]

Il 30/09/22 11:02, Chen-Yu Tsai ha scritto:
> On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
>>> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
>>>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
>>>>>> angelogioacchino.delregno@collabora.com>
>>>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>>> ---
>>>>>>     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[] = {
>>>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
>>>>> available parents in hardware design and they can be selected other
>>>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
>>>>> cannot show the real parent-child relationship in such case.
>>>>
>>>> I agree about that, but the clock framework will change the parent to
>>>> the "best parent" in that case... this was done to avoid writing complicated
>>>> custom clock ops just for that one.
>>>>
>>>> This issue is present only on MT8195, so it can be safely solved this way,
>>>> at least for now.
>>>>
>>>> Should this become a thing on another couple SoCs, it'll then make sense
>>>> to write custom clock ops just for the MFG.
>>>
>>> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
>>> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
>>
>> I'm not sure that it would, and then this would mean that we'd have to add
>> assigned-clock-parents to the devicetree and the day we will introduce the
>> "complicated custom clock ops" for that, we'll most probably have to change
>> the devicetree as well... which is something that I'm a bit reluctant to do
>> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
>> it to get the "new full functionality".
> 
> You can also do it by doing clk_set_parent() in the clock driver after the
> clocks are registered, or just write to the register before the clock is
> registered.
> 

I honestly don't like doing that - but I can try if that works and, if it does,
I can send a commit with a Fixes tag later, perhaps?


> We do the latter in some of the sunxi-ng drivers, though IIRC it was to
> force a certain divider on what we expose as a fixed divider clock.
> 
> ChenYu
> 
>> Introducing the new clock ops for the mfg mux is something that will happen
>> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
>> confident to write them, as I fear these won't be as flexible as needed and
>> will eventually need a rewrite; that's why I want to wait to get the same
>> situation on "something new".
>>
>> In my opinion, it is safe to keep this change as it is, even though I do
>> understand the shown concerns about the eventual unability to show the tree
>> relationship in case the bootloader chooses to initialize the mfg mux with
>> a univpll parent.
>>
>> Regards,
>> Angelo
>>




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 5:04 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 11:02, Chen-Yu Tsai ha scritto:
> > On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> >>> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> >>> <angelogioacchino.delregno@collabora.com> wrote:
> >>>>
> >>>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> >>>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >>>>>> angelogioacchino.delregno@collabora.com>
> >>>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>>>> ---
> >>>>>>     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[] = {
> >>>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> >>>>> available parents in hardware design and they can be selected other
> >>>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
> >>>>> cannot show the real parent-child relationship in such case.
> >>>>
> >>>> I agree about that, but the clock framework will change the parent to
> >>>> the "best parent" in that case... this was done to avoid writing complicated
> >>>> custom clock ops just for that one.
> >>>>
> >>>> This issue is present only on MT8195, so it can be safely solved this way,
> >>>> at least for now.
> >>>>
> >>>> Should this become a thing on another couple SoCs, it'll then make sense
> >>>> to write custom clock ops just for the MFG.
> >>>
> >>> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> >>> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
> >>
> >> I'm not sure that it would, and then this would mean that we'd have to add
> >> assigned-clock-parents to the devicetree and the day we will introduce the
> >> "complicated custom clock ops" for that, we'll most probably have to change
> >> the devicetree as well... which is something that I'm a bit reluctant to do
> >> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
> >> it to get the "new full functionality".
> >
> > You can also do it by doing clk_set_parent() in the clock driver after the
> > clocks are registered, or just write to the register before the clock is
> > registered.
> >
>
> I honestly don't like doing that - but I can try if that works and, if it does,
> I can send a commit with a Fixes tag later, perhaps?

Sounds good to me.

FWIW, I think it's OK for drivers to reinitialize hardware to a known
state that it can work with. As long as it doesn't break the system
while doing so.

ChenYu

>
> > We do the latter in some of the sunxi-ng drivers, though IIRC it was to
> > force a certain divider on what we expose as a fixed divider clock.
> >
> > ChenYu
> >
> >> Introducing the new clock ops for the mfg mux is something that will happen
> >> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
> >> confident to write them, as I fear these won't be as flexible as needed and
> >> will eventually need a rewrite; that's why I want to wait to get the same
> >> situation on "something new".
> >>
> >> In my opinion, it is safe to keep this change as it is, even though I do
> >> understand the shown concerns about the eventual unability to show the tree
> >> relationship in case the bootloader chooses to initialize the mfg mux with
> >> a univpll parent.
> >>
> >> Regards,
> >> Angelo
> >>
>
>
>

Re: [PATCH v3 08/10] clk: mediatek: clk-mt8195-topckgen: Drop univplls from mfg mux parents

[Chen-Yu Tsai]

On Fri, Sep 30, 2022 at 5:04 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/09/22 11:02, Chen-Yu Tsai ha scritto:
> > On Fri, Sep 30, 2022 at 4:58 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 30/09/22 10:44, Chen-Yu Tsai ha scritto:
> >>> On Fri, Sep 30, 2022 at 4:29 PM AngeloGioacchino Del Regno
> >>> <angelogioacchino.delregno@collabora.com> wrote:
> >>>>
> >>>> Il 30/09/22 07:59, MandyJH Liu (劉人僖) ha scritto:
> >>>>> On Tue, 2022-09-27 at 12:11 +0200, AngeloGioacchino Del Regno 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 <
> >>>>>> angelogioacchino.delregno@collabora.com>
> >>>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>>>> ---
> >>>>>>     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[] = {
> >>>>> There might be a problem here. Since the univpll_d6 and univpll_d7 are
> >>>>> available parents in hardware design and they can be selected other
> >>>>> than kernel stage, like bootloader, the clk tree listed in clk_summary
> >>>>> cannot show the real parent-child relationship in such case.
> >>>>
> >>>> I agree about that, but the clock framework will change the parent to
> >>>> the "best parent" in that case... this was done to avoid writing complicated
> >>>> custom clock ops just for that one.
> >>>>
> >>>> This issue is present only on MT8195, so it can be safely solved this way,
> >>>> at least for now.
> >>>>
> >>>> Should this become a thing on another couple SoCs, it'll then make sense
> >>>> to write custom clock ops just for the MFG.
> >>>
> >>> Would CLK_SET_RATE_NO_REPARENT on the fast mux coupled with forcing
> >>> the clk tree to a state that we like (mfgpll->fast_mux->gate) work?
> >>
> >> I'm not sure that it would, and then this would mean that we'd have to add
> >> assigned-clock-parents to the devicetree and the day we will introduce the
> >> "complicated custom clock ops" for that, we'll most probably have to change
> >> the devicetree as well... which is something that I'm a bit reluctant to do
> >> as a kernel upgrade doesn't automatically mean that you upgrade the DT with
> >> it to get the "new full functionality".
> >
> > You can also do it by doing clk_set_parent() in the clock driver after the
> > clocks are registered, or just write to the register before the clock is
> > registered.
> >
>
> I honestly don't like doing that - but I can try if that works and, if it does,
> I can send a commit with a Fixes tag later, perhaps?

Sounds good to me.

FWIW, I think it's OK for drivers to reinitialize hardware to a known
state that it can work with. As long as it doesn't break the system
while doing so.

ChenYu

>
> > We do the latter in some of the sunxi-ng drivers, though IIRC it was to
> > force a certain divider on what we expose as a fixed divider clock.
> >
> > ChenYu
> >
> >> Introducing the new clock ops for the mfg mux is something that will happen
> >> for sure, but if we don't get new SoCs with a similar "issue", I don't feel
> >> confident to write them, as I fear these won't be as flexible as needed and
> >> will eventually need a rewrite; that's why I want to wait to get the same
> >> situation on "something new".
> >>
> >> In my opinion, it is safe to keep this change as it is, even though I do
> >> understand the shown concerns about the eventual unability to show the tree
> >> relationship in case the bootloader chooses to initialize the mfg mux with
> >> a univpll parent.
> >>
> >> Regards,
> >> Angelo
> >>
>
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Nícolas F. R. A. Prado]

On Tue, Sep 27, 2022 at 12:11:19PM +0200, AngeloGioacchino Del Regno wrote:
> From: Chen-Yu Tsai <wenst@chromium.org>
> 
> 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 <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Hi,

it seems that while all other patches on this series were applied by Chen-Yu
through the clk tree, this commit never made it to the mediatek tree.

As a result, MT8183-based machines (or at least mt8183-kukui-jacuzzi, where I
tested on) currently hang during boot not only on next, but also on mainline,
v6.1-rc8. With this commit applied I've confirmed that the machine boots fine
again.

Matthias, could you please apply this commit and make sure it makes its way to
v6.1? Given the Fixes tag it should eventually make its way there anyway, but if
still possible would be good to have it fixed right from v6.1.

Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

> ---
>  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
> 
> 
> 

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Nícolas F. R. A. Prado]

On Tue, Sep 27, 2022 at 12:11:19PM +0200, AngeloGioacchino Del Regno wrote:
> From: Chen-Yu Tsai <wenst@chromium.org>
> 
> 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 <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Hi,

it seems that while all other patches on this series were applied by Chen-Yu
through the clk tree, this commit never made it to the mediatek tree.

As a result, MT8183-based machines (or at least mt8183-kukui-jacuzzi, where I
tested on) currently hang during boot not only on next, but also on mainline,
v6.1-rc8. With this commit applied I've confirmed that the machine boots fine
again.

Matthias, could you please apply this commit and make sure it makes its way to
v6.1? Given the Fixes tag it should eventually make its way there anyway, but if
still possible would be good to have it fixed right from v6.1.

Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

> ---
>  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
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Matthias Brugger]

On 06/12/2022 19:30, Nícolas F. R. A. Prado wrote:
> On Tue, Sep 27, 2022 at 12:11:19PM +0200, AngeloGioacchino Del Regno wrote:
>> From: Chen-Yu Tsai <wenst@chromium.org>
>>
>> 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 <wenst@chromium.org>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Hi,
> 
> it seems that while all other patches on this series were applied by Chen-Yu
> through the clk tree, this commit never made it to the mediatek tree.
> 
> As a result, MT8183-based machines (or at least mt8183-kukui-jacuzzi, where I
> tested on) currently hang during boot not only on next, but also on mainline,
> v6.1-rc8. With this commit applied I've confirmed that the machine boots fine
> again.
> 
> Matthias, could you please apply this commit and make sure it makes its way to
> v6.1? Given the Fixes tag it should eventually make its way there anyway, but if
> still possible would be good to have it fixed right from v6.1.
> 
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Applied now.

Sorry for the late reply.
Matthias

> 
> Thanks,
> Nícolas
> 
>> ---
>>   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
>>
>>
>>

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Matthias Brugger]

On 06/12/2022 19:30, Nícolas F. R. A. Prado wrote:
> On Tue, Sep 27, 2022 at 12:11:19PM +0200, AngeloGioacchino Del Regno wrote:
>> From: Chen-Yu Tsai <wenst@chromium.org>
>>
>> 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 <wenst@chromium.org>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Hi,
> 
> it seems that while all other patches on this series were applied by Chen-Yu
> through the clk tree, this commit never made it to the mediatek tree.
> 
> As a result, MT8183-based machines (or at least mt8183-kukui-jacuzzi, where I
> tested on) currently hang during boot not only on next, but also on mainline,
> v6.1-rc8. With this commit applied I've confirmed that the machine boots fine
> again.
> 
> Matthias, could you please apply this commit and make sure it makes its way to
> v6.1? Given the Fixes tag it should eventually make its way there anyway, but if
> still possible would be good to have it fixed right from v6.1.
> 
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Applied now.

Sorry for the late reply.
Matthias

> 
> Thanks,
> Nícolas
> 
>> ---
>>   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
>>
>>
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Matthias Brugger]

On 27/09/2022 12:11, AngeloGioacchino Del Regno wrote:
> From: Chen-Yu Tsai <wenst@chromium.org>
> 
> 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 <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Both patches applied, thanks!

> ---
>   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>,

Re: [PATCH v3 01/10] arm64: dts: mt8183: Fix Mali GPU clock

[Matthias Brugger]

On 27/09/2022 12:11, AngeloGioacchino Del Regno wrote:
> From: Chen-Yu Tsai <wenst@chromium.org>
> 
> 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 <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Both patches applied, thanks!

> ---
>   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>,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel