[PATCH 0/2] Add MT8196 VMM driver support

[PATCH 0/2] Add MT8196 VMM driver support

[Nancy.Lin]

From: Nancy Lin <nancy.lin@mediatek.com>

Add a driver for the MediaTek MT8196 VMM (Vcore for MultiMedia)
controller.

Nancy Lin (2):
  dt-bindings: regulator: mediatek: Add MT8196 vmm controller
  soc: mediatek: Add MT8196 VMM driver support

 .../mediatek,mt8196-vmm-regulator.yaml        |  70 +++
 drivers/soc/mediatek/Kconfig                  |  12 +
 drivers/soc/mediatek/Makefile                 |   1 +
 drivers/soc/mediatek/mtk-vmm-drv.c            | 471 ++++++++++++++++++
 4 files changed, 554 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
 create mode 100644 drivers/soc/mediatek/mtk-vmm-drv.c

-- 
2.45.2

[PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Nancy.Lin]

From: Nancy Lin <nancy.lin@mediatek.com>

Add a device tree binding document for the MediaTek MT8196 VMM (Vcore
for MultiMedia) regulator controller. The VMM controller acts as the
main power supplier for multimedia power domains, such as those used
by display, video encode and decode subsystems. It provides virtual
regulators that serve as the power sources for various multimedia IPs,
and coordinates with the hardware common clock framework (hwccf) and
the Video Companion Processor (VCP) to manage the power domains of
these components. The regulator is controlled by the VCP firmware,
and the operating system signals its requirement through a voting
hardware block (hwccf).

Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
---
 .../mediatek,mt8196-vmm-regulator.yaml        | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
new file mode 100644
index 000000000000..a50e35c2e238
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/regulator/mediatek,mt8196-vmm-regulator.yaml#"
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT8196 VMM (Vcore for MultiMedia) Regulator Controller
+
+maintainers:
+  - Nancy Lin <nancy.lin@mediatek.com>
+
+description: |
+  The MediaTek MT8196 VMM (Vcore for Multi Media) controller acts as the
+  main power supplier for multimedia power domains, such as those used by
+  display, video encode and decode subsystems. The VMM hardware block
+  provides virtual regulators that serve as the power sources (suppliers)
+  for various multimedia IPs. It coordinates with the MediaTek hardware
+  common clock framework (HWCCF) and the Video Companion Processor (VCP)
+  to manage the power domains of these multimedia components.
+
+  Each child node under the VMM node represents a virtual regulator
+  (e.g., vdisp, vdec-vcore) and must specify a 'regulator-name'.
+
+properties:
+  compatible:
+    const: "mediatek,mt8196-vmm"
+
+  mediatek,hw-ccf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Phandle to the hardware common clock framework syscon controller.
+
+  mediatek,vcp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Phandle to the Video Co-Processor (VCP) node.
+
+patternProperties:
+  "^(vdisp|vdec-vcore)$":
+    type: object
+    description: |
+      Virtual regulator for a specific multimedia domain.
+      The node name should match the supported regulator (e.g., vdisp, vdec-vcore).
+    properties:
+      regulator-name:
+        type: string
+        description: The name of the virtual regulator.
+    required:
+      - regulator-name
+    additionalProperties: false
+
+required:
+  - compatible
+  - mediatek,hw-ccf
+  - mediatek,vcp
+
+additionalProperties: false
+
+examples:
+  - |
+    vmm: vmm {
+      compatible = "mediatek,mt8196-vmm";
+      mediatek,hw-ccf = <&mm_hwv>;
+      mediatek,vcp = <&vcp>;
+
+      vdisp: vdisp {
+        regulator-name = "vdisp";
+      };
+      vdec_vcore: vdec-vcore {
+        regulator-name = "vdec-vcore";
+      };
+    };
-- 
2.45.2

[PATCH 2/2] soc: mediatek: Add MT8196 VMM driver support

[Nancy.Lin]

From: Nancy Lin <nancy.lin@mediatek.com>

Add a driver for the MediaTek MT8196 VMM (Vcore for MultiMedia)
controller, which acts as the main power supplier for multimedia power
domains such as display, video encode, and video decode on MediaTek SoCs.

The VMM controller provides virtual regulators for multimedia IPs and
coordinates with the hardware common clock framework (hwccf) and the
Video Companioin Processor (VCP) to manage power domains. The driver
uses a hardware voter through HWCCF to notify the VCP to turn on or
off VMM-related bucks.

Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
---
 drivers/soc/mediatek/Kconfig       |  12 +
 drivers/soc/mediatek/Makefile      |   1 +
 drivers/soc/mediatek/mtk-vmm-drv.c | 471 +++++++++++++++++++++++++++++
 3 files changed, 484 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-vmm-drv.c

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index d7293977f06e..4db4a0876083 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -69,6 +69,18 @@ config MTK_MMSYS
 	  Say yes here to add support for the MediaTek Multimedia
 	  Subsystem (MMSYS).
 
+config MTK_VMM
+	tristate "MediaTek VMM driver"
+	help
+	  Say Y here to enable support for the MediaTek VMM (Vcore for
+	  MultiMedia) controller, which acts as the main power supplier
+	  for multimedia power domains such as display, video encode and
+	  decode on MediaTek SoCs. The VMM controller provides virtual
+	  regulators for multimedia IPs and coordinates with the hardware
+	  common clock framework (hwccf) and the Video Companion Processor
+	  (VCP) to manage power domains. The VMM driver uses hardware voter
+	  through hwccf to notify VCP to turn on/off VMM-related bucks.
+
 config MTK_SVS
 	tristate "MediaTek Smart Voltage Scaling(SVS)"
 	depends on NVMEM_MTK_EFUSE && NVMEM
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 0665573e3c4b..2b2071614ac4 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_REGULATOR_COUPLER) += mtk-regulator-coupler.o
 obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
+obj-$(CONFIG_MTK_VMM) += mtk-vmm-drv.o
 obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
 obj-$(CONFIG_MTK_SVS) += mtk-svs.o
 obj-$(CONFIG_MTK_SOCINFO) += mtk-socinfo.o
diff --git a/drivers/soc/mediatek/mtk-vmm-drv.c b/drivers/soc/mediatek/mtk-vmm-drv.c
new file mode 100644
index 000000000000..de4ceb7d59fa
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-vmm-drv.c
@@ -0,0 +1,471 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ * Author: Yunfei Dong <yunfei.dong@mediatek.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kthread.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)
+#include <linux/remoteproc.h>
+#include <linux/remoteproc/mtk_vcp_public.h>
+#endif
+
+#define mtk_vmm_dbg(dev, fmt, args...) \
+	dev_dbg(dev, "[vmm] %s(%d): " fmt "\n", __func__, __LINE__, ##args)
+
+#define mtk_vmm_err(dev, fmt, args...) \
+	dev_err(dev, "[vmm] err %s(%d): " fmt "\n", __func__, __LINE__, ##args)
+
+/* vdisp' id=0, vdec' id=1 */
+#define HW_CCF_XPU0_BACKUP_SET(id)      (0x230 + (id) * 0x8)
+#define HW_CCF_XPU0_BACKUP_CLR(id)      (0x234 + (id) * 0x8)
+#define HW_CCF_BACKUP_ENABLE(id)        (0x1430 + (id) * 0x10)
+#define HW_CCF_BACKUP_STATUS(id)        (0x1434 + (id) * 0x10)
+#define HW_CCF_BACKUP_STATUS_DBG(id)    (0x1438 + (id) * 0x10)
+#define HW_CCF_BACKUP_DONE(id)          (0x143c + (id) * 0x10)
+#define HW_CCF_BACKUP_SET_STATUS(id)    (0x1484 + (id) * 0x8)
+#define HW_CCF_BACKUP_CLR_STATUS(id)    (0x1488 + (id) * 0x8)
+
+#define DELAY_US      (1)
+#define POLL_TIMEOUT  (10000)
+
+#define MTK_VMM_REGULAR(match, _name)                  \
+[MTK_VMM_ID_##_name] = {                               \
+	.desc = {                                      \
+		.name = match,                         \
+		.of_match = of_match_ptr(match),       \
+		.ops = &mtk_vmm_regulator_ops,         \
+		.type = REGULATOR_VOLTAGE,             \
+		.id = MTK_VMM_ID_##_name,              \
+		.owner = THIS_MODULE,                  \
+	},                                             \
+}
+
+/*
+ * enum mtk_vmm_id - mtk vmm id' information
+ *
+ * @MTK_VMM_ID_VDISP: vdisp BIT, id
+ * @MTK_VMM_ID_VDEC_VCORE: vdec BIT, id
+ * @MTK_VMM_ID_MAX: invalid id
+ */
+enum mtk_vmm_id {
+	MTK_VMM_ID_VDISP = 0,
+	MTK_VMM_ID_VDEC_VCORE,
+	MTK_VMM_ID_MAX
+};
+
+/*
+ * struct mtk_vmm_drv - mtk vmm driver' information
+ *
+ * @vcp_device: vcp device
+ * @vcp_nb: vcp notifier block
+ * @hwccf_regmap: hardware common clock framework register map
+ * @dev: vmm device
+ * @kthr_vcp: vcp task thread
+ * @vcp_phandle: vcp handle
+ * @vcp_is_active: vcp active status
+ * @need_update: vcore regulator enable and disable
+ * @disable_status: disable status.
+ */
+struct mtk_vmm_drv {
+	struct mtk_vcp_device  *vcp_device;
+	struct notifier_block  vcp_nb;
+	struct regmap          *hwccf_regmap;
+	/* lock for hwccf control */
+	struct mutex           ctrl_mutex[MTK_VMM_ID_MAX];
+	struct device          *dev;
+	struct task_struct     *kthr_vcp;
+	phandle                vcp_phandle;
+	bool                   vcp_is_active;
+	bool                   need_update[MTK_VMM_ID_MAX];
+	bool                   disable_status[MTK_VMM_ID_MAX];
+};
+
+/*
+ * struct mtk_vmm_regulator - mtk vmm regulators' information
+ *
+ * @desc: standard fields of regulator description.
+ */
+struct mtk_vmm_regulator {
+	struct regulator_desc	desc;
+};
+
+/*
+ * struct mtk_vmm_regulator_init_data - mtk vmm regulators' init data
+ *
+ * @size: num of regulators
+ * @regulator_info: regulator info.
+ */
+struct mtk_vmm_regulator_init_data {
+	u32 size;
+	struct mtk_vmm_regulator *regulator_info;
+};
+
+static void mtk_vmm_debug_dump(struct mtk_vmm_drv *vmm_drv, int line, int id)
+{
+	u32 value0, value1, value2, value3, value4, value5, value6, value7;
+	u32 ccf_set;
+	u32 ccf_clr;
+	u32 ccf_enable;
+	u32 ccf_status;
+	u32 ccf_status_dbg;
+	u32 ccf_done;
+	u32 ccf_set_status;
+	u32 ccf_clr_status;
+
+	ccf_set = HW_CCF_XPU0_BACKUP_SET(id);
+	ccf_clr = HW_CCF_XPU0_BACKUP_CLR(id);
+	ccf_enable = HW_CCF_BACKUP_ENABLE(id);
+	ccf_status = HW_CCF_BACKUP_STATUS(id);
+	ccf_status_dbg = HW_CCF_BACKUP_STATUS_DBG(id);
+	ccf_done = HW_CCF_BACKUP_DONE(id);
+	ccf_set_status = HW_CCF_BACKUP_SET_STATUS(id);
+	ccf_clr_status = HW_CCF_BACKUP_CLR_STATUS(id);
+
+	regmap_read(vmm_drv->hwccf_regmap, ccf_set, &value0);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_clr, &value1);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_enable, &value2);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_status, &value3);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_status_dbg, &value4);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_done, &value5);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_set_status, &value6);
+	regmap_read(vmm_drv->hwccf_regmap, ccf_clr_status, &value7);
+
+	mtk_vmm_err(vmm_drv->dev, "id:%d %d: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+		    id, line, value0, value1, value2,
+		    value3, value4, value5, value6, value7);
+}
+
+static int mtk_vmm_hwccf_ctrl(struct mtk_vmm_drv *vmm_drv, bool enable, int id)
+{
+	u32 ctrl_reg;
+	u32 hwccf_ctrl_status;
+	u32 hwccf_done;
+	u32 bit_val;
+	int tmp, ret = 0;
+
+	if (IS_ERR_OR_NULL(vmm_drv->hwccf_regmap)) {
+		mtk_vmm_err(vmm_drv->dev, "hwccf_reg_base is null, need to map first.");
+		return -ENODEV;
+	}
+
+	if (!vmm_drv->vcp_is_active) {
+		vmm_drv->need_update[id] = true;
+		vmm_drv->disable_status[id] = !enable;
+		goto ctrl_end;
+	}
+
+	ctrl_reg = enable ? HW_CCF_XPU0_BACKUP_SET(id) : HW_CCF_XPU0_BACKUP_CLR(id);
+	hwccf_ctrl_status = enable ? HW_CCF_BACKUP_SET_STATUS(id) : HW_CCF_BACKUP_CLR_STATUS(id);
+	hwccf_done = HW_CCF_BACKUP_DONE(id);
+	bit_val = BIT(id);
+
+	/* polling hwccf write data done */
+	ret = regmap_read_poll_timeout(vmm_drv->hwccf_regmap, hwccf_done, tmp,
+				       ((tmp & bit_val) == bit_val), DELAY_US, POLL_TIMEOUT);
+	if (ret < 0) {
+		mtk_vmm_debug_dump(vmm_drv, __LINE__, id);
+		goto ctrl_end;
+	}
+
+	/* set and clr data */
+	ret = regmap_write(vmm_drv->hwccf_regmap, ctrl_reg, bit_val);
+	if (ret) {
+		mtk_vmm_debug_dump(vmm_drv, __LINE__, id);
+		goto ctrl_end;
+	}
+
+	/* polling hwccf write data done */
+	ret = regmap_read_poll_timeout(vmm_drv->hwccf_regmap, hwccf_done, tmp,
+				       ((tmp & bit_val) == bit_val), DELAY_US, POLL_TIMEOUT);
+	if (ret < 0) {
+		mtk_vmm_debug_dump(vmm_drv, __LINE__, id);
+		goto ctrl_end;
+	}
+
+	/* wait for current done */
+	ret = regmap_read_poll_timeout(vmm_drv->hwccf_regmap, hwccf_ctrl_status, tmp,
+				       !(tmp & bit_val), DELAY_US, POLL_TIMEOUT);
+	if (ret < 0) {
+		mtk_vmm_debug_dump(vmm_drv, __LINE__, id);
+		goto ctrl_end;
+	}
+ctrl_end:
+
+	return ret;
+}
+
+static int mtk_vmm_is_enabled(struct regulator_dev *rdev)
+{
+	u32 hwccf_enable;
+	u32 bit_val;
+	struct mtk_vmm_drv *vmm_drv = rdev_get_drvdata(rdev);
+	int ret = 0;
+	u32 val;
+	int id = rdev->desc->id;
+
+	if (id >= MTK_VMM_ID_MAX) {
+		mtk_vmm_err(vmm_drv->dev, "desc id is error %d\n", __LINE__);
+		goto ctrl_end;
+	}
+
+	hwccf_enable = HW_CCF_BACKUP_ENABLE(id);
+	bit_val = BIT(id);
+
+	ret = regmap_read(vmm_drv->hwccf_regmap, hwccf_enable, &val);
+	if (ret < 0)
+		mtk_vmm_err(vmm_drv->dev, "vmm id:%d read buck status failed\n", id);
+	else
+		ret = !!(val & bit_val);
+ctrl_end:
+
+	return ret;
+}
+
+static int mtk_vmm_enable(struct regulator_dev *rdev)
+{
+	struct mtk_vmm_drv *vmm_drv = rdev_get_drvdata(rdev);
+	int ret;
+	int id = rdev->desc->id;
+
+	mtk_vmm_dbg(vmm_drv->dev, "id:%d enable vmm driver %d",
+		    id, vmm_drv->vcp_is_active);
+	if (id >= MTK_VMM_ID_MAX) {
+		mtk_vmm_err(vmm_drv->dev, "desc id is error %d\n", __LINE__);
+		return -ENODEV;
+	}
+
+	mutex_lock(&vmm_drv->ctrl_mutex[id]);
+	ret = mtk_vmm_hwccf_ctrl(vmm_drv, true, id);
+	if (ret < 0)
+		mtk_vmm_err(vmm_drv->dev, "failed to enable hwccf, ret=%d\n", ret);
+	mutex_unlock(&vmm_drv->ctrl_mutex[id]);
+
+	return ret;
+}
+
+static int mtk_vmm_disable(struct regulator_dev *rdev)
+{
+	struct mtk_vmm_drv *vmm_drv = rdev_get_drvdata(rdev);
+	int ret = 0;
+	int id = rdev->desc->id;
+
+	mtk_vmm_dbg(vmm_drv->dev, "id:%d disable vmm driver %d",
+		    id, vmm_drv->vcp_is_active);
+	if (id >= MTK_VMM_ID_MAX) {
+		mtk_vmm_err(vmm_drv->dev, "desc id is error %d\n", __LINE__);
+		return -ENODEV;
+	}
+
+	mutex_lock(&vmm_drv->ctrl_mutex[id]);
+	ret = mtk_vmm_hwccf_ctrl(vmm_drv, false, id);
+	if (ret < 0)
+		mtk_vmm_err(vmm_drv->dev, "failed to disable hwccf, ret=%d\n", ret);
+	mutex_unlock(&vmm_drv->ctrl_mutex[id]);
+
+	return ret;
+}
+
+#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)
+static int vmm_vcp_notifier(struct notifier_block *nb, unsigned long vcp_event, void *unused)
+{
+	struct mtk_vmm_drv *vmm_drv = container_of(nb, struct mtk_vmm_drv, vcp_nb);
+	int ret = NOTIFY_DONE;
+	int id;
+
+	switch (vcp_event) {
+	case VCP_EVENT_SUSPEND:
+	case VCP_EVENT_STOP:
+		mtk_vmm_dbg(vmm_drv->dev, "vcp notifier suspend");
+		break;
+	case VCP_EVENT_READY:
+	case VCP_EVENT_RESUME:
+		for (id = MTK_VMM_ID_VDISP; id < MTK_VMM_ID_MAX; id++) {
+			mutex_lock(&vmm_drv->ctrl_mutex[id]);
+			if (vmm_drv->need_update[id]) {
+				ret = mtk_vmm_hwccf_ctrl(vmm_drv, true, id);
+				if (ret < 0)
+					mtk_vmm_err(vmm_drv->dev,
+						    "failed to enable hwccf %d\n", ret);
+
+				if (vmm_drv->disable_status[id]) {
+					ret = mtk_vmm_hwccf_ctrl(vmm_drv, false, id);
+					if (ret < 0)
+						mtk_vmm_err(vmm_drv->dev,
+							    "failed to disable hwccf %d\n",
+							    ret);
+					vmm_drv->disable_status[id] = false;
+				}
+
+				vmm_drv->need_update[id] = false;
+			}
+			mutex_unlock(&vmm_drv->ctrl_mutex[id]);
+		}
+
+		mtk_vmm_dbg(vmm_drv->dev, "vcp notifier ready");
+		break;
+	}
+
+	return ret;
+}
+
+static int mtk_vmm_vcp_init_thread(void *arg)
+{
+	struct mtk_vmm_drv *vmm_drv = arg;
+	struct device *dev = vmm_drv->dev;
+	struct device_link *dev_link;
+	int retry = 0, retry_cnt = 10000;
+
+	while (request_module("mtk-vcp")) {
+		if (++retry > retry_cnt) {
+			mtk_vmm_err(vmm_drv->dev, "failed to load mtk-vcp module");
+			return -ENODEV;
+		}
+		ssleep(1);
+	}
+
+	if (of_property_read_u32(dev->of_node, "mediatek,vcp", &vmm_drv->vcp_phandle)) {
+		mtk_vmm_err(vmm_drv->dev, "can't get vcp handle.\n");
+		return -ENODEV;
+	}
+
+	vmm_drv->vcp_device = mtk_vcp_get_by_phandle(vmm_drv->vcp_phandle);
+	while (!vmm_drv->vcp_device) {
+		mtk_vmm_err(vmm_drv->dev, "get vcp device failed\n");
+		return -ENODEV;
+	}
+
+	dev_link = device_link_add(vmm_drv->dev, vmm_drv->vcp_device->dev, 0);
+	if (!dev_link) {
+		mtk_vmm_err(vmm_drv->dev, "device link is NULL\n");
+		return -EINVAL;
+	}
+
+	retry = 0;
+	retry_cnt = 10000;
+	while (!vmm_drv->vcp_device->data->vcp_is_ready(MMDVFS_MMUP_FEATURE_ID) ||
+	       !vmm_drv->vcp_device->data->vcp_is_ready(MMDVFS_VCP_FEATURE_ID)) {
+		if (++retry > retry_cnt) {
+			mtk_vmm_err(vmm_drv->dev, "vcp and mmup is not ready yet.");
+			return -ETIMEDOUT;
+		}
+		ssleep(1);
+	}
+
+	vmm_drv->vcp_is_active = true;
+	vmm_drv->vcp_device->data->vcp_register_feature(vmm_drv->vcp_device, VMM_FEATURE_ID);
+
+	vmm_drv->vcp_nb.notifier_call = vmm_vcp_notifier;
+	vmm_drv->vcp_device->data->vcp_register_notify(VMM_FEATURE_ID, &vmm_drv->vcp_nb);
+
+	return 0;
+}
+#endif
+
+static const struct regulator_ops mtk_vmm_regulator_ops = {
+	.enable = mtk_vmm_enable,
+	.disable = mtk_vmm_disable,
+	.is_enabled = mtk_vmm_is_enabled,
+};
+
+static struct mtk_vmm_regulator mt8196_vmm_regulators[] = {
+	MTK_VMM_REGULAR("vdisp", VDISP),
+	MTK_VMM_REGULAR("vdec-vcore", VDEC_VCORE),
+};
+
+static const struct mtk_vmm_regulator_init_data mt8196_regulator_data = {
+	.size = ARRAY_SIZE(mt8196_vmm_regulators),
+	.regulator_info = mt8196_vmm_regulators,
+};
+
+static const struct of_device_id of_vmm_match_tbl[] = {
+	{
+		.compatible = "mediatek,mt8196-vmm",
+		.data = &mt8196_regulator_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_vmm_match_tbl);
+
+static int mtk_vmm_probe(struct platform_device *pdev)
+{
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	const struct mtk_vmm_regulator_init_data *regulator_init_data;
+	struct mtk_vmm_regulator *mtk_regulators;
+	struct mtk_vmm_drv *vmm_drv;
+	int i;
+
+	vmm_drv = devm_kzalloc(&pdev->dev, sizeof(*vmm_drv), GFP_KERNEL);
+	if (!vmm_drv)
+		return -ENOMEM;
+
+	vmm_drv->dev = &pdev->dev;
+	for (i = MTK_VMM_ID_VDISP; i < MTK_VMM_ID_MAX; i++)
+		mutex_init(&vmm_drv->ctrl_mutex[i]);
+
+	platform_set_drvdata(pdev, vmm_drv);
+
+	vmm_drv->hwccf_regmap =
+		syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "mediatek,hw-ccf");
+	if (IS_ERR(vmm_drv->hwccf_regmap))
+		return dev_err_probe(&pdev->dev, PTR_ERR(vmm_drv->hwccf_regmap),
+				     "cannot find hwccf controller.\n");
+
+	regulator_init_data = of_device_get_match_data(&pdev->dev);
+
+	mtk_regulators = regulator_init_data->regulator_info;
+	for (i = 0; i < regulator_init_data->size; i++) {
+		config.dev = &pdev->dev;
+		config.driver_data = vmm_drv;
+		rdev = devm_regulator_register(&pdev->dev, &(mtk_regulators + i)->desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register %d\n", i);
+			return PTR_ERR(rdev);
+		}
+	}
+
+#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)
+	vmm_drv->kthr_vcp = kthread_run(mtk_vmm_vcp_init_thread, vmm_drv, "vmm-vcp");
+	if (IS_ERR(vmm_drv->kthr_vcp))
+		dev_err(&pdev->dev, "create kthread failed");
+#endif
+
+	mtk_vmm_dbg(&pdev->dev, "vmm prob done.");
+	return 0;
+}
+
+static void mtk_vmm_remove(struct platform_device *pdev)
+{
+#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)
+	struct mtk_vmm_drv *vmm_drv = dev_get_drvdata(&pdev->dev);
+	struct mtk_vcp_device *vcp_device = vmm_drv->vcp_device;
+
+	vcp_device->data->vcp_deregister_feature(vcp_device, VMM_FEATURE_ID);
+	device_link_remove(vmm_drv->dev, vcp_device->dev);
+#endif
+}
+
+static struct platform_driver mtk_vmm_plat_drv = {
+	.probe = mtk_vmm_probe,
+	.remove = mtk_vmm_remove,
+	.driver = {
+		.name = "mtk-vmm-drv",
+		.of_match_table = of_vmm_match_tbl,
+	},
+};
+module_platform_driver(mtk_vmm_plat_drv);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MediaTek vmm driver");
-- 
2.45.2

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Krzysztof Kozlowski]

On 22/05/2025 17:03, Nancy.Lin wrote:
> From: Nancy Lin <nancy.lin@mediatek.com>
> 
> Add a device tree binding document for the MediaTek MT8196 VMM (Vcore
> for MultiMedia) regulator controller. The VMM controller acts as the
> main power supplier for multimedia power domains, such as those used
> by display, video encode and decode subsystems. It provides virtual
> regulators that serve as the power sources for various multimedia IPs,

Virtual regulators do not sound real, so feels like you want some sort
of power domains?

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> and coordinates with the hardware common clock framework (hwccf) and
> the Video Companion Processor (VCP) to manage the power domains of
> these components. The regulator is controlled by the VCP firmware,
> and the operating system signals its requirement through a voting
> hardware block (hwccf).
> 
> Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> ---
>  .../mediatek,mt8196-vmm-regulator.yaml        | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> new file mode 100644
> index 000000000000..a50e35c2e238
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/regulator/mediatek,mt8196-vmm-regulator.yaml#"
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT8196 VMM (Vcore for MultiMedia) Regulator Controller
> +
> +maintainers:
> +  - Nancy Lin <nancy.lin@mediatek.com>
> +
> +description: |
> +  The MediaTek MT8196 VMM (Vcore for Multi Media) controller acts as the
> +  main power supplier for multimedia power domains, such as those used by
> +  display, video encode and decode subsystems. The VMM hardware block
> +  provides virtual regulators that serve as the power sources (suppliers)
> +  for various multimedia IPs. It coordinates with the MediaTek hardware
> +  common clock framework (HWCCF) and the Video Companion Processor (VCP)
> +  to manage the power domains of these multimedia components.
> +
> +  Each child node under the VMM node represents a virtual regulator
> +  (e.g., vdisp, vdec-vcore) and must specify a 'regulator-name'.
> +
> +properties:
> +  compatible:
> +    const: "mediatek,mt8196-vmm"

Not tested...

> +
> +  mediatek,hw-ccf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the hardware common clock framework syscon controller.

No, you cannot express clocks with syscon.

> +
> +  mediatek,vcp:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the Video Co-Processor (VCP) node.

For what purpose?

> +
> +patternProperties:
> +  "^(vdisp|vdec-vcore)$":

Redundant nodes, useless. Drop these completely.


> +    type: object
> +    description: |
> +      Virtual regulator for a specific multimedia domain.
> +      The node name should match the supported regulator (e.g., vdisp, vdec-vcore).
> +    properties:
> +      regulator-name:

No, you cannot start redefining properties. This binding is nowhere
close to hardware description. Looks like some copy-paste downstream
driver, so binding to fulfill driver needs.

Please rework to match hardware. I suggest reaching internally to get
some help how upstream drivers and bindings look like *prior* sending
downstream code.

Best regards,
Krzysztof

Re: [PATCH 2/2] soc: mediatek: Add MT8196 VMM driver support

[Krzysztof Kozlowski]

On 22/05/2025 17:03, Nancy.Lin wrote:
> From: Nancy Lin <nancy.lin@mediatek.com>
> 
> Add a driver for the MediaTek MT8196 VMM (Vcore for MultiMedia)
> controller, which acts as the main power supplier for multimedia power
> domains such as display, video encode, and video decode on MediaTek SoCs.
> 
> The VMM controller provides virtual regulators for multimedia IPs and
> coordinates with the hardware common clock framework (hwccf) and the
> Video Companioin Processor (VCP) to manage power domains. The driver
> uses a hardware voter through HWCCF to notify the VCP to turn on or
> off VMM-related bucks.
> 
> Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig       |  12 +
>  drivers/soc/mediatek/Makefile      |   1 +
>  drivers/soc/mediatek/mtk-vmm-drv.c | 471 +++++++++++++++++++++++++++++
>  3 files changed, 484 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-vmm-drv.c
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index d7293977f06e..4db4a0876083 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -69,6 +69,18 @@ config MTK_MMSYS
>  	  Say yes here to add support for the MediaTek Multimedia
>  	  Subsystem (MMSYS).
>  
> +config MTK_VMM
> +	tristate "MediaTek VMM driver"
> +	help
> +	  Say Y here to enable support for the MediaTek VMM (Vcore for
> +	  MultiMedia) controller, which acts as the main power supplier
> +	  for multimedia power domains such as display, video encode and
> +	  decode on MediaTek SoCs. The VMM controller provides virtual
> +	  regulators for multimedia IPs and coordinates with the hardware
> +	  common clock framework (hwccf) and the Video Companion Processor
> +	  (VCP) to manage power domains. The VMM driver uses hardware voter
> +	  through hwccf to notify VCP to turn on/off VMM-related bucks.
> +
>  config MTK_SVS
>  	tristate "MediaTek Smart Voltage Scaling(SVS)"
>  	depends on NVMEM_MTK_EFUSE && NVMEM
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 0665573e3c4b..2b2071614ac4 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_REGULATOR_COUPLER) += mtk-regulator-coupler.o
>  obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> +obj-$(CONFIG_MTK_VMM) += mtk-vmm-drv.o
>  obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
>  obj-$(CONFIG_MTK_SVS) += mtk-svs.o
>  obj-$(CONFIG_MTK_SOCINFO) += mtk-socinfo.o
> diff --git a/drivers/soc/mediatek/mtk-vmm-drv.c b/drivers/soc/mediatek/mtk-vmm-drv.c
> new file mode 100644
> index 000000000000..de4ceb7d59fa
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-vmm-drv.c
> @@ -0,0 +1,471 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2025 MediaTek Inc.
> + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/kthread.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)

And that's the proof you send some sort of downstream code.

This does not exist.

> +#include <linux/remoteproc.h>
> +#include <linux/remoteproc/mtk_vcp_public.h>
> +#endif
> +
> +#define mtk_vmm_dbg(dev, fmt, args...) \
> +	dev_dbg(dev, "[vmm] %s(%d): " fmt "\n", __func__, __LINE__, ##args)

No, you do not get your own debug code.

NAK.

This is nowhere close to upstream code. Don't send us downstream
patterns please.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Krzysztof Kozlowski]

On 22/05/2025 17:09, Krzysztof Kozlowski wrote:
> On 22/05/2025 17:03, Nancy.Lin wrote:
>> From: Nancy Lin <nancy.lin@mediatek.com>
>>
>> Add a device tree binding document for the MediaTek MT8196 VMM (Vcore
>> for MultiMedia) regulator controller. The VMM controller acts as the
>> main power supplier for multimedia power domains, such as those used
>> by display, video encode and decode subsystems. It provides virtual
>> regulators that serve as the power sources for various multimedia IPs,
> 
> Virtual regulators do not sound real, so feels like you want some sort
> of power domains?
> 
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Wrong keyword... Please fix the subject prefixes:

For bindings, the preferred subjects are explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Rob Herring (Arm)]

On Thu, 22 May 2025 23:03:33 +0800, Nancy.Lin wrote:
> From: Nancy Lin <nancy.lin@mediatek.com>
> 
> Add a device tree binding document for the MediaTek MT8196 VMM (Vcore
> for MultiMedia) regulator controller. The VMM controller acts as the
> main power supplier for multimedia power domains, such as those used
> by display, video encode and decode subsystems. It provides virtual
> regulators that serve as the power sources for various multimedia IPs,
> and coordinates with the hardware common clock framework (hwccf) and
> the Video Companion Processor (VCP) to manage the power domains of
> these components. The regulator is controlled by the VCP firmware,
> and the operating system signals its requirement through a voting
> hardware block (hwccf).
> 
> Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> ---
>  .../mediatek,mt8196-vmm-regulator.yaml        | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml: patternProperties:^(vdisp|vdec-vcore)$:properties:regulator-name:type: 'string' is not one of ['boolean', 'object']
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.example.dtb: vmm (mediatek,mt8196-vmm): vdisp:regulator-name: ['vdisp'] is not of type 'string'
	from schema $id: http://devicetree.org/schemas/regulator/mediatek,mt8196-vmm-regulator.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.example.dtb: vmm (mediatek,mt8196-vmm): vdec-vcore:regulator-name: ['vdec-vcore'] is not of type 'string'
	from schema $id: http://devicetree.org/schemas/regulator/mediatek,mt8196-vmm-regulator.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250522150426.3418225-2-nancy.lin@mediatek.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[CK Hu (胡俊光)]

On Thu, 2025-05-22 at 23:03 +0800, Nancy.Lin wrote:
> From: Nancy Lin <nancy.lin@mediatek.com>
> 
> Add a device tree binding document for the MediaTek MT8196 VMM (Vcore
> for MultiMedia) regulator controller. The VMM controller acts as the
> main power supplier for multimedia power domains, such as those used
> by display, video encode and decode subsystems. It provides virtual
> regulators that serve as the power sources for various multimedia IPs,
> and coordinates with the hardware common clock framework (hwccf) and
> the Video Companion Processor (VCP) to manage the power domains of
> these components. The regulator is controlled by the VCP firmware,
> and the operating system signals its requirement through a voting
> hardware block (hwccf).
> 
> Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> ---
>  .../mediatek,mt8196-vmm-regulator.yaml        | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> new file mode 100644
> index 000000000000..a50e35c2e238
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-regulator.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "https://urldefense.com/v3/__http://devicetree.org/schemas/regulator/mediatek,mt8196-vmm-regulator.yaml*__;Iw!!CTRNKA9wMg0ARbw!gawATNOT3u6UBtTwtgnoZggwdwVL3VBuFZQyf-baV8j1wUMIV1sIaqBvhcqtfnZSlmXFesengtZaYxCWtDo$ "
> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!gawATNOT3u6UBtTwtgnoZggwdwVL3VBuFZQyf-baV8j1wUMIV1sIaqBvhcqtfnZSlmXFesengtZaLcnGLmw$ 
> +
> +title: MediaTek MT8196 VMM (Vcore for MultiMedia) Regulator Controller
> +
> +maintainers:
> +  - Nancy Lin <nancy.lin@mediatek.com>
> +
> +description: |
> +  The MediaTek MT8196 VMM (Vcore for Multi Media) controller acts as the
> +  main power supplier for multimedia power domains, such as those used by
> +  display, video encode and decode subsystems. The VMM hardware block
> +  provides virtual regulators that serve as the power sources (suppliers)
> +  for various multimedia IPs. It coordinates with the MediaTek hardware
> +  common clock framework (HWCCF) and the Video Companion Processor (VCP)
> +  to manage the power domains of these multimedia components.
> +
> +  Each child node under the VMM node represents a virtual regulator
> +  (e.g., vdisp, vdec-vcore) and must specify a 'regulator-name'.
> +
> +properties:
> +  compatible:
> +    const: "mediatek,mt8196-vmm"
> +
> +  mediatek,hw-ccf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the hardware common clock framework syscon controller.
> +
> +  mediatek,vcp:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the Video Co-Processor (VCP) node.
> +
> +patternProperties:
> +  "^(vdisp|vdec-vcore)$":
> +    type: object
> +    description: |
> +      Virtual regulator for a specific multimedia domain.
> +      The node name should match the supported regulator (e.g., vdisp, vdec-vcore).
> +    properties:
> +      regulator-name:
> +        type: string
> +        description: The name of the virtual regulator.
> +    required:
> +      - regulator-name
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - mediatek,hw-ccf
> +  - mediatek,vcp
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    vmm: vmm {
> +      compatible = "mediatek,mt8196-vmm";
> +      mediatek,hw-ccf = <&mm_hwv>;
> +      mediatek,vcp = <&vcp>;
> +
> +      vdisp: vdisp {
> +        regulator-name = "vdisp";
> +      };
> +      vdec_vcore: vdec-vcore {
> +        regulator-name = "vdec-vcore";
> +      };
> +    };

device tree is used to describe real hardware.
vmm is a virtual device which is used for software to integrate the related driver,
so it should not have this virtual device.
According to your description, vcp is the master to control regulator,
so I think the device tree would be

vcp {
	regulator-name = "vdisp", "vdec-vcore";
};

And vcp driver control these regulator.

I'm not sure what does hw-ccf do, if it's also controlled by vcp, the node should be

vcp {
	mediatek,hw-ccf = <&mm_hwv>;
	regulator-name = "vdisp", "vdec-vcore";
};

Regards,
CK

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Nancy Lin (林欣螢)]

Hi Krzysztof,

Thanks for your review.

On Thu, 2025-05-22 at 17:09 +0200, Krzysztof Kozlowski wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 22/05/2025 17:03, Nancy.Lin wrote:
> > From: Nancy Lin <nancy.lin@mediatek.com>
> > 
> > Add a device tree binding document for the MediaTek MT8196 VMM
> > (Vcore
> > for MultiMedia) regulator controller. The VMM controller acts as
> > the
> > main power supplier for multimedia power domains, such as those
> > used
> > by display, video encode and decode subsystems. It provides virtual
> > regulators that serve as the power sources for various multimedia
> > IPs,
> 
> Virtual regulators do not sound real, so feels like you want some
> sort
> of power domains?
> 
This regulator supplies power to the power domain. Before the power
domain can be powered up, the MediaTek PM driver first enables the
regulator that the power domain relies on, and then powers up the
domain. While "virtual" might not be the best term, it accurately
describes a regulator used to control the power switch.

Ultimately, the VCP (uP) handles the on/off control of the regulator.
Therefore, we encapsulate it as a standard regulator in the kernel.

The internal control of the VMM regulator is as follows:
   kernel                 HWCCF                          VCP (uP)
|--------------|      |--------------|   irq    |---------------------|
| VMM reglator | ---> | hardware     |  ----->  | get buck on/off irq |
|              |      | voter        |          | and then turn on/off|
|              |      |              |          | buck                |
|--------------|      |--------------|          |---------------------|

When the regulator needs to be turned on or off, it uses the voter
provided by hwccf to cast a vote for on/off. The VCP then receives the
corresponding IRQ for the regulator and performs the buck on/off
accordingly.

> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst*L18__;Iw!!CTRNKA9wMg0ARbw!lBqtzZ8DmC6f2kNnLXRCpHsof7XOYvrIMG4o_DXuvjYbg1SyFRrXrtEMLjIxS31cq82hqQowrczWRg$
> 

OK, I will remove the binding/document word in commit message body.
And change title to
   regulator: dt-bindings: Add MT8196 vmm controller


> > and coordinates with the hardware common clock framework (hwccf)
> > and
> > the Video Companion Processor (VCP) to manage the power domains of
> > these components. The regulator is controlled by the VCP firmware,
> > and the operating system signals its requirement through a voting
> > hardware block (hwccf).
> > 
> > Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> > ---
> >  .../mediatek,mt8196-vmm-regulator.yaml        | 70
> > +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-
> > regulator.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-
> > regulator.yaml
> > b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-vmm-
> > regulator.yaml
> > new file mode 100644
> > index 000000000000..a50e35c2e238
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt8196-
> > vmm-regulator.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > "https://urldefense.com/v3/__http://devicetree.org/schemas/regulato
> > r/mediatek,mt8196-vmm-
> > regulator.yaml*__;Iw!!CTRNKA9wMg0ARbw!lBqtzZ8DmC6f2kNnLXRCpHsof7XOY
> > vrIMG4o_DXuvjYbg1SyFRrXrtEMLjIxS31cq82hqQrOgUATjA$ "
> > +$schema:
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lBqtzZ8DmC6f2kNnLXRCpHsof7XOYvrIMG4o_DXuvjYbg1SyFRrXrtEMLjIxS31cq82hqQr4UitgLg$
> > +
> > +title: MediaTek MT8196 VMM (Vcore for MultiMedia) Regulator
> > Controller
> > +
> > +maintainers:
> > +  - Nancy Lin <nancy.lin@mediatek.com>
> > +
> > +description: |
> > +  The MediaTek MT8196 VMM (Vcore for Multi Media) controller acts
> > as the
> > +  main power supplier for multimedia power domains, such as those
> > used by
> > +  display, video encode and decode subsystems. The VMM hardware
> > block
> > +  provides virtual regulators that serve as the power sources
> > (suppliers)
> > +  for various multimedia IPs. It coordinates with the MediaTek
> > hardware
> > +  common clock framework (HWCCF) and the Video Companion Processor
> > (VCP)
> > +  to manage the power domains of these multimedia components.
> > +
> > +  Each child node under the VMM node represents a virtual
> > regulator
> > +  (e.g., vdisp, vdec-vcore) and must specify a 'regulator-name'.
> > +
> > +properties:
> > +  compatible:
> > +    const: "mediatek,mt8196-vmm"
> 
> Not tested...
> 
My bad, I accidentally added extra quotation marks, but I ran
dt_binding_check and didn't see this error being                      
detected.

> > +
> > +  mediatek,hw-ccf:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Phandle to the hardware common clock framework
> > syscon controller.
> 
> No, you cannot express clocks with syscon.
> 
HWCCF is MediaTek's internal naming; the functionality of the hardware
IP is voter. We use the IP to vote for buck on/off, and the VCP will
get the voting result to perform buck on/off. I will refine the
naming/description to "voter."

> > +
> > +  mediatek,vcp:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Phandle to the Video Co-Processor (VCP) node.
> 
> For what purpose?
> 
This is used to ensure that the VCP uP is ready to receive the hwccf
IRQ and handle the corresponding buck on/off operations.

> > +
> > +patternProperties:
> > +  "^(vdisp|vdec-vcore)$":
> 
> Redundant nodes, useless. Drop these completely.
> 
OK.
> 
> > +    type: object
> > +    description: |
> > +      Virtual regulator for a specific multimedia domain.
> > +      The node name should match the supported regulator (e.g.,
> > vdisp, vdec-vcore).
> > +    properties:
> > +      regulator-name:
> 
> No, you cannot start redefining properties. This binding is nowhere
> close to hardware description. Looks like some copy-paste downstream
> driver, so binding to fulfill driver needs.
> 
> Please rework to match hardware. I suggest reaching internally to get
> some help how upstream drivers and bindings look like *prior* sending
> downstream code.
> 
I will review and refine the binding internally before submitting it.
Thanks!

Best regards,
Nancy

> Best regards,
> Krzysztof

Re: [PATCH 2/2] soc: mediatek: Add MT8196 VMM driver support

[Nancy Lin (林欣螢)]

Hi Krzysztof,

Thanks for your review.

On Thu, 2025-05-22 at 17:10 +0200, Krzysztof Kozlowski wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 22/05/2025 17:03, Nancy.Lin wrote:
> > From: Nancy Lin <nancy.lin@mediatek.com>
> > 
> > Add a driver for the MediaTek MT8196 VMM (Vcore for MultiMedia)
> > controller, which acts as the main power supplier for multimedia
> > power
> > domains such as display, video encode, and video decode on MediaTek
> > SoCs.
> > 
> > The VMM controller provides virtual regulators for multimedia IPs
> > and
> > coordinates with the hardware common clock framework (hwccf) and
> > the
> > Video Companioin Processor (VCP) to manage power domains. The
> > driver
> > uses a hardware voter through HWCCF to notify the VCP to turn on or
> > off VMM-related bucks.
> > 
> > Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> > ---
> >  drivers/soc/mediatek/Kconfig       |  12 +
> >  drivers/soc/mediatek/Makefile      |   1 +
> >  drivers/soc/mediatek/mtk-vmm-drv.c | 471
> > +++++++++++++++++++++++++++++
> >  3 files changed, 484 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mtk-vmm-drv.c
> > 
> > diff --git a/drivers/soc/mediatek/Kconfig
> > b/drivers/soc/mediatek/Kconfig
> > index d7293977f06e..4db4a0876083 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -69,6 +69,18 @@ config MTK_MMSYS
> >         Say yes here to add support for the MediaTek Multimedia
> >         Subsystem (MMSYS).
> > 
> > +config MTK_VMM
> > +     tristate "MediaTek VMM driver"
> > +     help
> > +       Say Y here to enable support for the MediaTek VMM (Vcore
> > for
> > +       MultiMedia) controller, which acts as the main power
> > supplier
> > +       for multimedia power domains such as display, video encode
> > and
> > +       decode on MediaTek SoCs. The VMM controller provides
> > virtual
> > +       regulators for multimedia IPs and coordinates with the
> > hardware
> > +       common clock framework (hwccf) and the Video Companion
> > Processor
> > +       (VCP) to manage power domains. The VMM driver uses hardware
> > voter
> > +       through hwccf to notify VCP to turn on/off VMM-related
> > bucks.
> > +
> >  config MTK_SVS
> >       tristate "MediaTek Smart Voltage Scaling(SVS)"
> >       depends on NVMEM_MTK_EFUSE && NVMEM
> > diff --git a/drivers/soc/mediatek/Makefile
> > b/drivers/soc/mediatek/Makefile
> > index 0665573e3c4b..2b2071614ac4 100644
> > --- a/drivers/soc/mediatek/Makefile
> > +++ b/drivers/soc/mediatek/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> >  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> >  obj-$(CONFIG_MTK_REGULATOR_COUPLER) += mtk-regulator-coupler.o
> >  obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> > +obj-$(CONFIG_MTK_VMM) += mtk-vmm-drv.o
> >  obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
> >  obj-$(CONFIG_MTK_SVS) += mtk-svs.o
> >  obj-$(CONFIG_MTK_SOCINFO) += mtk-socinfo.o
> > diff --git a/drivers/soc/mediatek/mtk-vmm-drv.c
> > b/drivers/soc/mediatek/mtk-vmm-drv.c
> > new file mode 100644
> > index 000000000000..de4ceb7d59fa
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mtk-vmm-drv.c
> > @@ -0,0 +1,471 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/kthread.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/driver.h>
> > +#if IS_ENABLED(CONFIG_MTK_VCP_RPROC)
> 
> And that's the proof you send some sort of downstream code.
> 
> This does not exist.
> 
Sorry, I forgot add reference patch. This patch is based on [1].

[1] Add vcp driver
https://patchwork.kernel.org/project/linux-mediatek/list/?series=949232



> > +#include <linux/remoteproc.h>
> > +#include <linux/remoteproc/mtk_vcp_public.h>
> > +#endif
> > +
> > +#define mtk_vmm_dbg(dev, fmt, args...) \
> > +     dev_dbg(dev, "[vmm] %s(%d): " fmt "\n", __func__, __LINE__,
> > ##args)
> 
> No, you do not get your own debug code.
> 
> NAK.
> 
> This is nowhere close to upstream code. Don't send us downstream
> patterns please.
> 
OK, I will remove own debug code and refine it. Thanks!

Best regards,
Nancy


> Best regards,
> Krzysztof

Re: [PATCH 1/2] dt-bindings: regulator: mediatek: Add MT8196 vmm controller

[Krzysztof Kozlowski]

On 28/05/2025 06:01, Nancy Lin (林欣螢) wrote:
>>
>> Virtual regulators do not sound real, so feels like you want some
>> sort
>> of power domains?
>>
> This regulator supplies power to the power domain. Before the power
> domain can be powered up, the MediaTek PM driver first enables the
> regulator that the power domain relies on, and then powers up the
> domain. While "virtual" might not be the best term, it accurately
> describes a regulator used to control the power switch.
> 
> Ultimately, the VCP (uP) handles the on/off control of the regulator.
> Therefore, we encapsulate it as a standard regulator in the kernel.
> 
> The internal control of the VMM regulator is as follows:
>    kernel                 HWCCF                          VCP (uP)
> |--------------|      |--------------|   irq    |---------------------|
> | VMM reglator | ---> | hardware     |  ----->  | get buck on/off irq |
> |              |      | voter        |          | and then turn on/off|
> |              |      |              |          | buck                |
> |--------------|      |--------------|          |---------------------|
> 
> When the regulator needs to be turned on or off, it uses the voter
> provided by hwccf to cast a vote for on/off. The VCP then receives the
> corresponding IRQ for the regulator and performs the buck on/off
> accordingly.

OK, thanks for explanation.

> 
...

> 
>>> +
>>> +  mediatek,hw-ccf:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: Phandle to the hardware common clock framework
>>> syscon controller.
>>
>> No, you cannot express clocks with syscon.
>>
> HWCCF is MediaTek's internal naming; the functionality of the hardware
> IP is voter. We use the IP to vote for buck on/off, and the VCP will
> get the voting result to perform buck on/off. I will refine the
> naming/description to "voter."


OK, that was a bit confusing, but now I have more questions:

Why isn't this a child of the HWCCF? You do not have here any resources,
so this looks just to instantiate device for Linux.

Looking at your diagram earlier (maybe include it in binding description
if this stays as is at the end) I would imagine phandle only to hwccf.
But you have two phandles - HWCCF and VCP.

So maybe this is part of VCP device, not HWCCF? I don't know. I am
afraid the moment I will suggest you something someone from Collabora
will be pointing out that my advice us wrong and I don't know this
hardware. Yeah, I don't know it. You should know it.

> 
>>> +
>>> +  mediatek,vcp:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: Phandle to the Video Co-Processor (VCP) node.
>>
>> For what purpose?
>>
> This is used to ensure that the VCP uP is ready to receive the hwccf
> IRQ and handle the corresponding buck on/off operations.
> 
>>> +
>>> +patternProperties:
>>> +  "^(vdisp|vdec-vcore)$":
>>
>> Redundant nodes, useless. Drop these completely.
>>
> OK.
Best regards,
Krzysztof