Re: [PATCH v2 06/15] soc: mediatek: add mmsys support for MT8196

From: AngeloGioacchino Del Regno
Date: Mon Mar 24 2025 - 13:10:04 EST


Il 21/03/25 10:33, paul-pl.chen ha scritto:
From: Nancy Lin <nancy.lin@xxxxxxxxxxxx>

1. Defining driver data and adding compatible string
for different subsystems
(DISPSYS0, DISPSYS1, OVLSYS0, OVLSYS1, VDISP_AO)
2. Adding functions to control top clocks and ddp clocks.
3. Updating the probe function to initialize clocks and
enable runtime PM if its node has the power-domains property.
4. Adding functions to configure ddp components and
set default configurations.
5. Adding the routing table for each mmsys in MT8196.

You need at least two commits for all that you're doing here... and adding MT8196
tables should be the last one.


Signed-off-by: Nancy Lin <nancy.lin@xxxxxxxxxxxx>
Signed-off-by: Paul-pl Chen <paul-pl.chen@xxxxxxxxxxxx>
---
drivers/soc/mediatek/mt8196-mmsys.h | 451 +++++++++++++++++++++++++
drivers/soc/mediatek/mtk-mmsys.c | 203 ++++++++++-
drivers/soc/mediatek/mtk-mmsys.h | 18 +
include/linux/soc/mediatek/mtk-mmsys.h | 60 ++++
4 files changed, 731 insertions(+), 1 deletion(-)
create mode 100644 drivers/soc/mediatek/mt8196-mmsys.h

diff --git a/drivers/soc/mediatek/mt8196-mmsys.h b/drivers/soc/mediatek/mt8196-mmsys.h
new file mode 100644
index 000000000000..ff841ae9939a
--- /dev/null
+++ b/drivers/soc/mediatek/mt8196-mmsys.h
@@ -0,0 +1,451 @@

..snip..

+static const struct mtk_mmsys_default mmsys_mt8196_disp0_default_table[] = {
+ {MT8196_OVLSYS_GCE_EVENT_SEL, MT8196_EVENT_GCE_EN, GENMASK(1, 0)},
+ {MT8196_DISP0_BYPASS_MUX_SHADOW,
+ MT8196_CB_BYPASS_MUX_SHADOW | MT8196_BYPASS_MUX_SHADOW,
+ MT8196_CB_BYPASS_MUX_SHADOW | MT8196_BYPASS_MUX_SHADOW},
+ {MT8196_DISP0_DLI_RELAY0, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP0_DLI_RELAY1, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP0_DLI_RELAY8, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP0_DLO_RELAY1, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP0_DLO_RELAY2, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP0_DLO_RELAY3, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+};
+
+static const struct mtk_mmsys_default mmsys_mt8196_disp1_default_table[] = {
+ {MT8196_OVLSYS_GCE_EVENT_SEL, MT8196_EVENT_GCE_EN, GENMASK(1, 0)},
+ {MT8196_DISP1_INT_MERGE, 0, BIT(0)},
+ {MT8196_DISP1_BYPASS_MUX_SHADOW,
+ MT8196_CB_BYPASS_MUX_SHADOW | MT8196_BYPASS_MUX_SHADOW,
+ MT8196_CB_BYPASS_MUX_SHADOW | MT8196_BYPASS_MUX_SHADOW},
+ {MT8196_DISP1_DLI_RELAY21, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP1_DLI_RELAY22, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP1_DLI_RELAY23, MT8196_DLI_RELAY_1T2P, GENMASK(31, 30)},
+ {MT8196_DISP1_GCE_FRAME_DONE_SEL0, MT8196_FRAME_DONE_DVO, GENMASK(5, 0)},
+ {MT8196_DISP1_GCE_FRAME_DONE_SEL1, MT8196_FRAME_DONE_DP_INTF0, GENMASK(5, 0)},
+};
+
+static const struct mtk_mmsys_routes mmsys_mt8196_ovl0_routing_table[] = {
+ {
+ DDP_COMPONENT_OVL0_EXDMA2, DDP_COMPONENT_OVL0_BLENDER1,

There's a new MMSYS_ROUTE macro that was introduced because tables contained
wrong values in multiple instances and in multiple SoCs: please use it here and
for all routing tables.

+ MT8196_OVL_RSZ_IN_CB2_MOUT_EN, MT8196_DISP_OVL_EXDMA2_1_TO_OVL_EXDMA_OUT_CB3,
+ MT8196_DISP_OVL_EXDMA2_1_TO_OVL_EXDMA_OUT_CB3

..snip..

+};
+#endif /* __SOC_MEDIATEK_MT8196_MMSYS_H */
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index bb4639ca0b8c..1d3ca4f9f237 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -4,12 +4,14 @@
* Author: James Liao <jamesjj.liao@xxxxxxxxxxxx>
*/
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/reset-controller.h>
#include <linux/soc/mediatek/mtk-mmsys.h>
@@ -21,6 +23,7 @@
#include "mt8188-mmsys.h"
#include "mt8192-mmsys.h"
#include "mt8195-mmsys.h"
+#include "mt8196-mmsys.h"
#include "mt8365-mmsys.h"
#define MMSYS_SW_RESET_PER_REG 32
@@ -144,6 +147,54 @@ static const struct mtk_mmsys_driver_data mt8195_vppsys1_driver_data = {
.is_vppsys = true,
};
+static const struct mtk_mmsys_driver_data mt8196_dispsys0_driver_data = {
+ .clk_driver = "clk-mt8196-disp0",
+ .routes = mmsys_mt8196_disp0_routing_table,
+ .num_routes = ARRAY_SIZE(mmsys_mt8196_disp0_routing_table),
+ .async_info = mmsys_mt8196_disp0_async_comp_table,
+ .num_async_info = ARRAY_SIZE(mmsys_mt8196_disp0_async_comp_table),
+ .def_config = mmsys_mt8196_disp0_default_table,
+ .num_def_config = ARRAY_SIZE(mmsys_mt8196_disp0_default_table),
+ .num_top_clk = 1,
+};
+
+static const struct mtk_mmsys_driver_data mt8196_dispsys1_driver_data = {
+ .clk_driver = "clk-mt8196-disp1",
+ .routes = mmsys_mt8196_disp1_routing_table,
+ .num_routes = ARRAY_SIZE(mmsys_mt8196_disp1_routing_table),
+ .async_info = mmsys_mt8196_disp1_async_comp_table,
+ .num_async_info = ARRAY_SIZE(mmsys_mt8196_disp1_async_comp_table),
+ .def_config = mmsys_mt8196_disp1_default_table,
+ .num_def_config = ARRAY_SIZE(mmsys_mt8196_disp1_default_table),
+ .num_top_clk = 1,
+};
+
+static const struct mtk_mmsys_driver_data mt8196_ovlsys0_driver_data = {
+ .clk_driver = "clk-mt8196-ovl0",
+ .routes = mmsys_mt8196_ovl0_routing_table,
+ .num_routes = ARRAY_SIZE(mmsys_mt8196_ovl0_routing_table),
+ .async_info = mmsys_mt8196_ovl0_async_comp_table,
+ .num_async_info = ARRAY_SIZE(mmsys_mt8196_ovl0_async_comp_table),
+ .def_config = mmsys_mt8196_ovl0_default_table,
+ .num_def_config = ARRAY_SIZE(mmsys_mt8196_ovl0_default_table),
+};
+
+static const struct mtk_mmsys_driver_data mt8196_ovlsys1_driver_data = {
+ .clk_driver = "clk-mt8196-ovl1",
+ .routes = mmsys_mt8196_ovl1_routing_table,
+ .num_routes = ARRAY_SIZE(mmsys_mt8196_ovl1_routing_table),
+ .async_info = mmsys_mt8196_ovl1_async_comp_table,
+ .num_async_info = ARRAY_SIZE(mmsys_mt8196_ovl1_async_comp_table),
+ .def_config = mmsys_mt8196_ovl0_default_table,
+ .num_def_config = ARRAY_SIZE(mmsys_mt8196_ovl0_default_table),
+};
+
+static const struct mtk_mmsys_driver_data mt8196_vdisp_ao_driver_data = {
+ .clk_driver = "clk-mt8196-vdisp_ao",
+ .def_config = mmsys_mt8196_vdisp_ao_default_table,
+ .num_def_config = ARRAY_SIZE(mmsys_mt8196_vdisp_ao_default_table),
+};
+
static const struct mtk_mmsys_driver_data mt8365_mmsys_driver_data = {
.clk_driver = "clk-mt8365-mm",
.routes = mt8365_mmsys_routing_table,
@@ -158,6 +209,9 @@ struct mtk_mmsys {
spinlock_t lock; /* protects mmsys_sw_rst_b reg */
struct reset_controller_dev rcdev;
struct cmdq_client_reg cmdq_base;
+ struct clk **async_clk;
+ int num_async_clk;
+ struct clk **top_clk;
};
static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask, u32 val,
@@ -180,6 +234,99 @@ static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask,
writel_relaxed(tmp, mmsys->regs + offset);
}
+int mtk_mmsys_top_clk_enable(struct device *dev)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ int ret, i;
+
+ if (!mmsys->data->num_top_clk)
+ return 0;
+
+ for (i = 0; i < mmsys->data->num_top_clk; i++)
+ ret = clk_prepare_enable(mmsys->top_clk[i]);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_top_clk_enable);
+
+void mtk_mmsys_top_clk_disable(struct device *dev)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < mmsys->data->num_top_clk; i++)
+ clk_disable_unprepare(mmsys->top_clk[i]);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_top_clk_disable);
+
+int mtk_mmsys_ddp_clk_enable(struct device *dev, enum mtk_ddp_comp_id comp_id)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ const struct mtk_mmsys_async_info *async = mmsys->data->async_info;
+
+ int i;
+
+ if (!mmsys->data->num_async_info)
+ return 0;
+
+ for (i = 0; i < mmsys->data->num_async_info; i++)
+ if (comp_id == async[i].comp_id)
+ return clk_prepare_enable(mmsys->async_clk[async[i].index]);

Why can't you add the clocks in the mediatek-drm nodes and handle enablement in the
drm driver?!

+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_clk_enable);
+
+void mtk_mmsys_ddp_clk_disable(struct device *dev, enum mtk_ddp_comp_id comp_id)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ const struct mtk_mmsys_async_info *async = mmsys->data->async_info;
+ int i;
+
+ if (!mmsys->data->num_async_info)
+ return;
+
+ for (i = 0; i < mmsys->data->num_async_info; i++)
+ if (comp_id == async[i].comp_id)
+ clk_disable_unprepare(mmsys->async_clk[async[i].index]);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_clk_disable);
+
+void mtk_mmsys_ddp_config(struct device *dev, enum mtk_ddp_comp_id comp_id,
+ int width, int height, struct cmdq_pkt *cmdq_pkt)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ const struct mtk_mmsys_async_info *async = mmsys->data->async_info;
+ int i;
+
+ if (!mmsys->data->num_async_info)
+ return;
+
+ for (i = 0; i < mmsys->data->num_async_info; i++)
+ if (comp_id == async[i].comp_id)
+ break;
+
+ if (i == mmsys->data->num_async_info)
+ return;
+
+ mtk_mmsys_update_bits(mmsys, async[i].offset, async[i].mask,
+ height << 16 | width, cmdq_pkt);

linux/bitfield.h provides macros that you should use for those register fields.

+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_config);
+
+void mtk_mmsys_default_config(struct device *dev)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+ const struct mtk_mmsys_default *def_config = mmsys->data->def_config;
+ int i;
+
+ if (!mmsys->data->num_def_config)
+ return;
+
+ for (i = 0; i < mmsys->data->num_def_config; i++)
+ mtk_mmsys_update_bits(mmsys, def_config[i].offset, def_config[i].mask,
+ def_config[i].val, NULL);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_default_config);
+
void mtk_mmsys_ddp_connect(struct device *dev,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
@@ -390,7 +537,7 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
struct platform_device *clks;
struct platform_device *drm;
struct mtk_mmsys *mmsys;
- int ret;
+ int ret, i;
mmsys = devm_kzalloc(dev, sizeof(*mmsys), GFP_KERNEL);
if (!mmsys)
@@ -432,6 +579,49 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
return PTR_ERR(clks);
mmsys->clks_pdev = clks;
+ if (mmsys->data->num_top_clk) {
+ struct device_node *node;
+
+ node = of_get_child_by_name(dev->of_node, "top");

No, you can't do that if there's no binding to support that.

+ if (!node) {
+ dev_err(&pdev->dev, "Couldn't find top node\n");
+ return -EINVAL;
+ }
+
+ mmsys->top_clk = devm_kmalloc_array(dev, mmsys->data->num_top_clk,
+ sizeof(*mmsys->top_clk), GFP_KERNEL);
+ if (!mmsys->top_clk)
+ return -ENOMEM;
+
+ for (i = 0; i < mmsys->data->num_top_clk; i++) {
+ mmsys->top_clk[i] = of_clk_get(node, i);
+ if (IS_ERR(mmsys->top_clk[i]))
+ return PTR_ERR(mmsys->top_clk[i]);
+ }
+ }
+
+ if (mmsys->data->num_async_info) {
+ struct device_node *node;
+
+ node = of_get_child_by_name(dev->of_node, "async");
+ if (!node) {
+ dev_err(&pdev->dev, "Couldn't find async node\n");
+ return -EINVAL;
+ }
+

Also this looks like you have children providing only clocks?!
I really need to look at the bindings to decide, but this looks wrong.

Regards,
Angelo