[PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[Jjian Zhou]

The VCP mailbox has 5 groups. Each group has corresponding interrupts,
registers, and 64 slots (each slot is 4 bytes). Since different features
share one of the mailbox groups, the VCP mailbox needs to establish a
send table and a receive table. The send table is used to record the
feature ID, mailbox ID, and the number of slots occupied. The receive table
is used to record the feature ID, mailbox ID, the number of slots occupied,
and the receive options. The API setup_mbox_table in mtk-vcp-ipc.c calculates
the slot offset and pin index for each feature ID based on the mailbox ID and
slot number in the send and receive tables (several slots form a pin, and
each pin can trigger an interrupt). These descriptions are written in the
mtk-vcp-ipc.c file -- we call it the IPC layer.

We have two questions:
How should we describe the mailbox and IPI?
Can the intermediate IPC layer be rewritten as a virtual mailbox layer?

Example of send and recve table:
Operation | mbox_id | ipi_id | msg_size | align_size | slot_ofs | pin_index |  notes
send          0          0       18          18           0          0
recv          0          1       18          18          18          9
send          1         15        8           8           0          0
send          1         16       18          18           8          4
send          1          9        2           2          26         13
recv          1         15        8           8          28         14       ack of send ipi_id=15
recv          1         17       18          18          36         18
recv          1         10        2           2          54         27       ack of send ipi_id=2
send          2         11       18          18           0          0
send          2          2        2           2          18          9
send          2          3        3           4          20         10
send          2         32        2           2          24         12
recv          2         12       18          18          26         13
recv          2          5        1           2          44         22
recv          2          2        1           2          46         23

Recv ipi_id=2 is the ack of send ipi_id=2(The ipi_id=15 is the same.)

Jjian Zhou (3):
  mailbox: mediatek: Add mtk-vcp-mailbox driver
  firmware: mediatek: Add vcp ipc protocol interface
  dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document

 .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml |  49 ++
 drivers/firmware/Kconfig                      |   9 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
 drivers/mailbox/Kconfig                       |   9 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/mtk-vcp-mailbox.c             | 179 +++++++
 include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
 include/linux/mailbox/mtk-vcp-mailbox.h       |  34 ++
 9 files changed, 915 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
 create mode 100644 drivers/firmware/mtk-vcp-ipc.c
 create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
 create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h
 create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h

-- 
2.45.2

[PATCH RFC v2 1/3] mailbox: mediatek: Add mtk-vcp-mailbox driver

[Jjian Zhou]

Add mtk-vcp-mailbox driver to support the communication with
VCP remote microprocessor.

Signed-off-by: Jjian Zhou <jjian.zhou@mediatek.com>
---
 drivers/mailbox/Kconfig                 |   9 ++
 drivers/mailbox/Makefile                |   2 +
 drivers/mailbox/mtk-vcp-mailbox.c       | 179 ++++++++++++++++++++++++
 include/linux/mailbox/mtk-vcp-mailbox.h |  34 +++++
 4 files changed, 224 insertions(+)
 create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
 create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ed52db272f4d..ffc4a5491462 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -275,6 +275,15 @@ config MTK_CMDQ_MBOX
 	  critical time limitation, such as updating display configuration
 	  during the vblank.
 
+config MTK_VCP_MBOX
+	tristate "MediaTek VCP Mailbox Support"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  Say yes here to add support for the MediaTek VCP mailbox driver.
+	  The mailbox implementation provides access from the application
+	  processor to the MediaTek Video Processing Unit.
+	  If unsure say N.
+
 config ZYNQMP_IPI_MBOX
 	tristate "Xilinx ZynqMP IPI Mailbox"
 	depends on ARCH_ZYNQMP && OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 9a1542b55539..75a200a9d2d2 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -59,6 +59,8 @@ obj-$(CONFIG_MTK_ADSP_MBOX)	+= mtk-adsp-mailbox.o
 
 obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
 
+obj-$(CONFIG_MTK_VCP_MBOX)	+= mtk-vcp-mailbox.o
+
 obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
 
 obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
diff --git a/drivers/mailbox/mtk-vcp-mailbox.c b/drivers/mailbox/mtk-vcp-mailbox.c
new file mode 100644
index 000000000000..fd7a123c71c8
--- /dev/null
+++ b/drivers/mailbox/mtk-vcp-mailbox.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 MediaTek Corporation. All rights reserved.
+ * Author: Jjian Zhou <jjian.zhou.@mediatek.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/mtk-vcp-mailbox.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+struct mtk_vcp_mbox_priv {
+	void __iomem *base;
+	struct device *dev;
+	struct mbox_controller mbox;
+	const struct mtk_vcp_mbox_cfg *cfg;
+	struct mtk_ipi_info ipi_recv;
+};
+
+struct mtk_vcp_mbox_cfg {
+	u32 set_in;
+	u32 clr_out;
+};
+
+static inline struct mtk_vcp_mbox_priv *get_mtk_vcp_mbox_priv(struct mbox_controller *mbox)
+{
+	return container_of(mbox, struct mtk_vcp_mbox_priv, mbox);
+}
+
+static irqreturn_t mtk_vcp_mbox_irq_thread(int irq, void *data)
+{
+	struct mbox_chan *chan = data;
+	struct mtk_vcp_mbox_priv *priv = get_mtk_vcp_mbox_priv(chan->mbox);
+
+	/* get irq status */
+	priv->ipi_recv.irq_status = readl(priv->base + priv->cfg->clr_out);
+
+	__ioread32_copy(priv->ipi_recv.msg, priv->base, MAX_SLOT_NUM);
+
+	mbox_chan_received_data(chan, &priv->ipi_recv);
+
+	/* clear irq status */
+	writel(priv->ipi_recv.irq_status, priv->base + priv->cfg->clr_out);
+
+	return IRQ_HANDLED;
+}
+
+static struct mbox_chan *mtk_vcp_mbox_xlate(struct mbox_controller *mbox,
+					    const struct of_phandle_args *sp)
+{
+	if (sp->args_count)
+		return NULL;
+
+	return mbox->chans;
+}
+
+static int mtk_vcp_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct mtk_vcp_mbox_priv *priv = get_mtk_vcp_mbox_priv(chan->mbox);
+	struct mtk_ipi_info *ipi_info = data;
+	u32 status;
+
+	if (!ipi_info->msg) {
+		dev_err(priv->dev, "msg buffer is NULL.\n");
+		return -ENOMEM;
+	}
+
+	status = readl(priv->base + priv->cfg->set_in) & BIT(ipi_info->index);
+	if (status) {
+		dev_err(priv->dev, "mailbox IPI %d is busy.\n", ipi_info->id);
+		return -EBUSY;
+	}
+
+	if (ipi_info->slot_ofs + ipi_info->len > MBOX_SLOT_MAX_SIZE)
+		return -EINVAL;
+	__iowrite32_copy(priv->base + ipi_info->slot_ofs, ipi_info->msg,
+			 ipi_info->len);
+
+	writel(BIT(ipi_info->index), priv->base + priv->cfg->set_in);
+
+	return 0;
+}
+
+static bool mtk_vcp_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct mtk_ipi_info *ipi_info = chan->active_req;
+	struct mtk_vcp_mbox_priv *priv = get_mtk_vcp_mbox_priv(chan->mbox);
+	u32 op;
+
+	op = readl(priv->base + priv->cfg->set_in) & BIT(ipi_info->index);
+	return !op;
+}
+
+static const struct mbox_chan_ops mtk_vcp_mbox_chan_ops = {
+	.send_data	= mtk_vcp_mbox_send_data,
+	.last_tx_done	= mtk_vcp_mbox_last_tx_done,
+};
+
+static int mtk_vcp_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_vcp_mbox_priv *priv;
+	struct mbox_controller *mbox;
+	int ret, irq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	mbox = &priv->mbox;
+	mbox->dev = dev;
+	mbox->ops = &mtk_vcp_mbox_chan_ops;
+	mbox->txdone_irq = false;
+	mbox->txdone_poll = true;
+	mbox->of_xlate = mtk_vcp_mbox_xlate;
+	mbox->num_chans = 1;
+	mbox->chans = devm_kzalloc(dev, sizeof(*mbox->chans), GFP_KERNEL);
+	if (!mbox->chans)
+		return -ENOMEM;
+
+	priv->ipi_recv.msg = devm_kzalloc(dev, sizeof(u8) * MBOX_SLOT_MAX_SIZE,
+					  GFP_KERNEL);
+	if (!priv->ipi_recv.msg)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->cfg = of_device_get_match_data(dev);
+	if (!priv->cfg)
+		return -EINVAL;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = devm_request_threaded_irq(dev, irq, NULL,
+					mtk_vcp_mbox_irq_thread, IRQF_ONESHOT,
+					dev_name(dev), mbox->chans);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, priv);
+
+	dev_dbg(dev, "MTK VCP mailbox initialized\n");
+
+	return devm_mbox_controller_register(dev, &priv->mbox);
+}
+
+static const struct mtk_vcp_mbox_cfg mt8196_cfg = {
+	.set_in		= 0x100,
+	.clr_out	= 0x10C,
+};
+
+static const struct of_device_id mtk_vcp_mbox_of_match[] = {
+	{ .compatible = "mediatek,mt8196-vcp-mbox", .data = &mt8196_cfg },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_vcp_mbox_of_match);
+
+static struct platform_driver mtk_vcp_mbox_driver = {
+	.probe		= mtk_vcp_mbox_probe,
+	.driver = {
+		.name	= "mtk_vcp_mbox",
+		.of_match_table = mtk_vcp_mbox_of_match,
+	},
+};
+module_platform_driver(mtk_vcp_mbox_driver);
+
+MODULE_AUTHOR("Jjian Zhou <jjian.zhou@mediatek.com>");
+MODULE_DESCRIPTION("MTK VCP Mailbox Controller");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mailbox/mtk-vcp-mailbox.h b/include/linux/mailbox/mtk-vcp-mailbox.h
new file mode 100644
index 000000000000..953499b7cfeb
--- /dev/null
+++ b/include/linux/mailbox/mtk-vcp-mailbox.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (c) 2024 MediaTek Inc.
+ */
+
+#ifndef __MTK_VCP_MAILBOX_H__
+#define __MTK_VCP_MAILBOX_H__
+
+#define MBOX_SLOT_MAX_SIZE	0x100 /* mbox max slot size */
+#define MAX_SLOT_NUM	64
+
+/**
+ * struct mtk_ipi_info - channel table that belong to mtk_ipi_device
+ * @msg: The share buffer between IPC and mailbox driver
+ * @len: Message length
+ * @id: IPI number
+ * @recv_opt: Recv option,  0:receive ,1: response
+ * @index: The pin groups number of the mailbox channel
+ * @slot_ofs: Slot offset of the mailbox channel
+ * @irq_status: Indicate which pin groups triggered the interrupt
+ *
+ * It is used between IPC with mailbox driver.
+ */
+struct mtk_ipi_info {
+	void *msg;
+	u32 len;
+	u32 id;
+	u32 recv_opt;
+	u32 index;
+	u32 slot_ofs;
+	u32 irq_status;
+};
+
+#endif
-- 
2.45.2

[PATCH RFC v2 2/3] firmware: mediatek: Add VCP IPC protocol interface

[Jjian Zhou]

Some of mediatek processors contain the Risc-V coprocessor.

The communication between Host CPU and vcp firmware is
taking place using a shared memory area for message passing.

VCP IPC protocol offers (send/recv) interfaces using
mediatek-mailbox APIs.

Signed-off-by: Jjian Zhou <jjian.zhou@mediatek.com>
---
 drivers/firmware/Kconfig                      |   9 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
 include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
 4 files changed, 642 insertions(+)
 create mode 100644 drivers/firmware/mtk-vcp-ipc.c
 create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 37e43f287e78..98c4ff667836 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -179,6 +179,15 @@ config MTK_ADSP_IPC
 	  ADSP exists on some mtk processors.
 	  Client might use shared memory to exchange information with ADSP.
 
+config MTK_VCP_IPC
+	tristate "MTK VCP IPC Protocol driver"
+	depends on MTK_VCP_MBOX
+	help
+	  Say yes here to add support for the MediaTek VCP IPC
+	  between host AP (Linux) and the firmware running on VCP.
+	  VCP exists on some mtk processors.
+	  Client might use shared memory to exchange information with VCP.
+
 config SYSFB
 	bool
 	select BOOT_VESA_SUPPORT
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 91efcc868a05..2b9894e5169a 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
 obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_MTK_ADSP_IPC)	+= mtk-adsp-ipc.o
+obj-$(CONFIG_MTK_VCP_IPC)	+= mtk-vcp-ipc.o
 obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
 obj-$(CONFIG_SYSFB)		+= sysfb.o
diff --git a/drivers/firmware/mtk-vcp-ipc.c b/drivers/firmware/mtk-vcp-ipc.c
new file mode 100644
index 000000000000..744937c56b67
--- /dev/null
+++ b/drivers/firmware/mtk-vcp-ipc.c
@@ -0,0 +1,481 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2024 MediaTek Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/firmware/mediatek/mtk-vcp-ipc.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/sched/clock.h>
+#include <linux/time64.h>
+#include <linux/vmalloc.h>
+
+/**
+ * struct mtk_ipi_chan_table - channel table that belong to mtk_ipi_device
+ * @mbox: the mbox channel number
+ * @mbox_pin_cb: callback function
+ * @holder: keep 1 if there are ipi waiters (to wait the reply)
+ * @ipi_record: timestamp of each ipi transmission stage
+ * @pin_buf: buffer point
+ * @prdata: private data
+ * @recv_opt: recv option,  0:receive ,1: response
+ * @notify: completion notify process
+ * @send_ofs: message offset in the slots of a mbox
+ * @send_index: bit offset in the mbox
+ * @msg_zie: slot size of the ipi message
+ *
+ * All of these data should be initialized by mtk_ipi_device_register()
+ */
+struct mtk_ipi_chan_table {
+	u32 mbox;
+	mbox_pin_cb_t mbox_pin_cb;
+	atomic_t holder;
+	void *pin_buf;
+	void *prdata;
+	u32 recv_opt;
+	struct completion notify;
+	/* define a mutex for remote response */
+	struct mutex mutex_send;
+	u32 send_ofs;
+	u32 send_index;
+	u32 msg_size;
+};
+
+/**
+ * mbox information
+ *
+ * @mbdev: mbox device
+ * @mbox_id: mbox id
+ * @slot: how many slots that mbox used
+ * @opt: option for tx mode, 0:mbox, 1:share memory 2:queue
+ * @base: mbox base address
+ * @mbox_client: mbox client
+ * @mbox_chan: mbox channel
+ */
+struct mtk_mbox_info {
+	struct mtk_vcp_ipc *vcp_ipc;
+	u32 mbox_id;
+	u32 slot;
+	u32 opt;
+	/* lock of mbox */
+	spinlock_t mbox_lock;
+	struct mbox_client cl;
+	struct mbox_chan *ch;
+	struct mtk_ipi_info ipi_info;
+};
+
+static const char * const mbox_names[VCP_MBOX_NUM] = {
+	"mbox0", "mbox1", "mbox2", "mbox3", "mbox4"
+};
+
+/**
+ * mtk_vcp_ipc_recv - recv callback used by MTK VCP mailbox
+ *
+ * @c: mbox client
+ * @msg: message received
+ *
+ * Users of VCP IPC will need to provide handle_reply and handle_request
+ * callbacks.
+ */
+static void mtk_vcp_ipc_recv(struct mbox_client *c, void *msg)
+{
+	struct mtk_mbox_info *minfo = container_of(c, struct mtk_mbox_info, cl);
+	struct mtk_vcp_ipc *vcp_ipc = minfo->vcp_ipc;
+	struct mtk_ipi_info *ipi_info = msg;
+	struct mtk_ipi_device *ipidev = vcp_ipc->ipi_priv;
+	struct mtk_ipi_chan_table *table;
+	struct mtk_mbox_recv_table *mbox_recv;
+	u32 id;
+
+	/* execute all receive pin handler */
+	for (id = 0; id < vcp_ipc->mbdev->recv_count; id++) {
+		mbox_recv = &vcp_ipc->mbdev->recv_table[id];
+		if (mbox_recv->mbox_id != minfo->mbox_id)
+			continue;
+
+		if (!(BIT(mbox_recv->pin_index) & ipi_info->irq_status))
+			continue;
+
+		table = &ipidev->table[mbox_recv->ipi_id];
+		if (!table->pin_buf) {
+			dev_err(vcp_ipc->dev, "IPI%d buf is null.\n",
+				mbox_recv->ipi_id);
+			continue;
+		}
+
+		memcpy(table->pin_buf,
+		       ipi_info->msg + mbox_recv->offset * MBOX_SLOT_SIZE,
+		       mbox_recv->msg_size * MBOX_SLOT_SIZE);
+
+		if (!mbox_recv->recv_opt && table->mbox_pin_cb)
+			table->mbox_pin_cb(mbox_recv->ipi_id,
+					   table->prdata,
+					   table->pin_buf,
+					   mbox_recv->msg_size * MBOX_SLOT_SIZE);
+
+		/* notify task */
+		if (table->recv_opt == MBOX_RECV_MESSAGE ||
+		    atomic_read(&table->holder))
+			complete(&table->notify);
+	}
+}
+
+/*
+ * mtk_vcp_ipc_send - send ipc command to MTK VCP
+ *
+ * @ipidev: VCP struct mtk_ipi_device handle
+ * @id: id of the feature IPI
+ * @data: message address
+ * @len: message length
+ *
+ * Return: Zero for success from mbox_send_message
+ *         negative value for error
+ */
+int mtk_vcp_ipc_send(struct mtk_ipi_device *ipidev, u32 id, void *data, u32 len)
+{
+	struct device *dev;
+	struct mtk_mbox_info *minfo;
+	struct mtk_ipi_chan_table *table;
+	struct mtk_vcp_ipc *vcp_ipc;
+	int ret;
+
+	if (!ipidev || !ipidev->ipi_inited || !data)
+		return IPI_UNAVAILABLE;
+	vcp_ipc = ipidev->vcp_ipc;
+	if (!vcp_ipc)
+		return IPI_UNAVAILABLE;
+
+	table = ipidev->table;
+	dev = ipidev->vcp_ipc->dev;
+	minfo = &ipidev->vcp_ipc->info_table[table[id].mbox];
+	if (!minfo) {
+		dev_err(dev, "%s IPI%d minfo is invalid.\n", ipidev->name, id);
+		return IPI_UNAVAILABLE;
+	}
+
+	if (len > table[id].msg_size)
+		return IPI_MSG_TOO_BIG;
+	else if (!len)
+		len = table[id].msg_size;
+
+	mutex_lock(&table[id].mutex_send);
+
+	minfo->ipi_info.msg = data;
+	minfo->ipi_info.len = len;
+	minfo->ipi_info.id = id;
+	minfo->ipi_info.index = table[id].send_index;
+	minfo->ipi_info.slot_ofs = table[id].send_ofs * MBOX_SLOT_SIZE;
+
+	ret = mbox_send_message(minfo->ch, &minfo->ipi_info);
+	mutex_unlock(&table[id].mutex_send);
+	if (ret < 0) {
+		dev_err(dev, "%s IPI%d send failed.\n", ipidev->name, id);
+		return IPI_MBOX_ERR;
+	}
+
+	return IPI_ACTION_DONE;
+}
+EXPORT_SYMBOL(mtk_vcp_ipc_send);
+
+/*
+ * mtk_vcp_ipc_send_compl - send ipc command to MTK VCP
+ *
+ * @ipidev: VCP struct mtk_ipi_device handle
+ * @id: id of the feature IPI
+ * @data: message address
+ * @len: message length
+ * @timeout_ms:
+ *
+ * Return: Zero for success from mbox_send_message
+ *         negative value for error
+ */
+int mtk_vcp_ipc_send_compl(struct mtk_ipi_device *ipidev, u32 id,
+			   void *data, u32 len, u32 timeout_ms)
+{
+	struct device *dev;
+	struct mtk_mbox_info *minfo;
+	struct mtk_ipi_chan_table *table;
+	struct mtk_vcp_ipc *vcp_ipc;
+	int ret;
+
+	if (!ipidev || !ipidev->ipi_inited || !data)
+		return IPI_UNAVAILABLE;
+	vcp_ipc = ipidev->vcp_ipc;
+	if (!vcp_ipc)
+		return IPI_UNAVAILABLE;
+
+	table = ipidev->table;
+	dev = ipidev->vcp_ipc->dev;
+	minfo = &ipidev->vcp_ipc->info_table[table[id].mbox];
+	if (!minfo) {
+		dev_err(dev, "%s IPI%d minfo is invalid.\n", ipidev->name, id);
+		return IPI_UNAVAILABLE;
+	}
+
+	if (len > table[id].msg_size)
+		return IPI_MSG_TOO_BIG;
+	else if (!len)
+		len = table[id].msg_size;
+
+	mutex_lock(&table[id].mutex_send);
+
+	minfo->ipi_info.msg = data;
+	minfo->ipi_info.len = len;
+	minfo->ipi_info.id = id;
+	minfo->ipi_info.index = table[id].send_index;
+	minfo->ipi_info.slot_ofs = table[id].send_ofs * MBOX_SLOT_SIZE;
+
+	atomic_inc(&table[id].holder);
+
+	ret = mbox_send_message(minfo->ch, &minfo->ipi_info);
+	if (ret < 0) {
+		atomic_set(&table[id].holder, 0);
+		mutex_unlock(&table[id].mutex_send);
+		dev_err(dev, "%s IPI%d send failed.\n", ipidev->name, id);
+		return IPI_MBOX_ERR;
+	}
+
+	/* wait for completion */
+	ret = wait_for_completion_timeout(&table[id].notify,
+					  msecs_to_jiffies(timeout_ms));
+	atomic_set(&table[id].holder, 0);
+	if (ret > 0)
+		ret = IPI_ACTION_DONE;
+
+	mutex_unlock(&table[id].mutex_send);
+
+	return ret;
+}
+EXPORT_SYMBOL(mtk_vcp_ipc_send_compl);
+
+int mtk_vcp_mbox_ipc_register(struct mtk_ipi_device *ipidev, int id,
+			      mbox_pin_cb_t cb, void *prdata, void *msg)
+{
+	if (!ipidev || !ipidev->ipi_inited)
+		return IPI_DEV_ILLEGAL;
+	if (!msg)
+		return IPI_NO_MSGBUF;
+
+	if (ipidev->table[id].pin_buf)
+		return IPI_ALREADY_USED;
+	ipidev->table[id].mbox_pin_cb = cb;
+	ipidev->table[id].pin_buf = msg;
+	ipidev->table[id].prdata = prdata;
+
+	return IPI_ACTION_DONE;
+}
+EXPORT_SYMBOL(mtk_vcp_mbox_ipc_register);
+
+int mtk_vcp_mbox_ipc_unregister(struct mtk_ipi_device *ipidev, int id)
+{
+	if (!ipidev || !ipidev->ipi_inited)
+		return IPI_DEV_ILLEGAL;
+
+	/* Drop the ipi and reset the record */
+	complete(&ipidev->table[id].notify);
+
+	ipidev->table[id].mbox_pin_cb = NULL;
+	ipidev->table[id].pin_buf = NULL;
+	ipidev->table[id].prdata = NULL;
+
+	return IPI_ACTION_DONE;
+}
+EXPORT_SYMBOL(mtk_vcp_mbox_ipc_unregister);
+
+static void mtk_fill_in_entry(struct mtk_ipi_chan_table *entry, const u32 ipi_id,
+			      const struct mtk_mbox_table *mbdev)
+{
+	const struct mtk_mbox_send_table *mbox_send = mbdev->send_table;
+	u32 index;
+
+	for (index = 0; index < mbdev->send_count; index++) {
+		if (ipi_id != mbox_send[index].ipi_id)
+			continue;
+
+		entry->send_ofs = mbox_send[index].offset;
+		entry->send_index = mbox_send[index].pin_index;
+		entry->msg_size = mbox_send[index].msg_size;
+		entry->mbox = mbox_send[index].mbox_id;
+		return;
+	}
+
+	entry->mbox = -ENOENT;
+}
+
+int mtk_vcp_ipc_device_register(struct mtk_ipi_device *ipidev,
+				u32 ipi_chan_count, struct mtk_vcp_ipc *vcp_ipc)
+{
+	struct mtk_ipi_chan_table *ipi_chan_table;
+	struct mtk_mbox_table *mbdev;
+	u32 index;
+
+	if (!vcp_ipc || !ipidev)
+		return -EINVAL;
+
+	ipi_chan_table = kcalloc(ipi_chan_count,
+				 sizeof(struct mtk_ipi_chan_table), GFP_KERNEL);
+	if (!ipi_chan_table)
+		return -ENOMEM;
+
+	mbdev = vcp_ipc->mbdev;
+	vcp_ipc->ipi_priv = (void *)ipidev;
+	ipidev->table = ipi_chan_table;
+	ipidev->vcp_ipc = vcp_ipc;
+
+	for (index = 0; index < ipi_chan_count; index++) {
+		atomic_set(&ipi_chan_table[index].holder, 0);
+		mutex_init(&ipi_chan_table[index].mutex_send);
+		init_completion(&ipi_chan_table[index].notify);
+		mtk_fill_in_entry(&ipi_chan_table[index], index, mbdev);
+	}
+
+	ipidev->ipi_inited = 1;
+
+	dev_dbg(vcp_ipc->dev, "%s (with %d IPI) has registered.\n",
+		ipidev->name, ipi_chan_count);
+
+	return IPI_ACTION_DONE;
+}
+EXPORT_SYMBOL(mtk_vcp_ipc_device_register);
+
+static int setup_mbox_table(struct mtk_mbox_table *mbdev, u32 mbox)
+{
+	struct mtk_mbox_send_table *mbox_send = &mbdev->send_table[0];
+	struct mtk_mbox_recv_table *mbox_recv = &mbdev->recv_table[0];
+	u32 i, last_ofs = 0, last_idx = 0, last_slot = 0, last_sz = 0;
+
+	for (i = 0; i < mbdev->send_count; i++) {
+		if (mbox == mbox_send[i].mbox_id) {
+			mbox_send[i].offset = last_ofs + last_slot;
+			mbox_send[i].pin_index = last_idx + last_sz;
+			last_idx = mbox_send[i].pin_index;
+			last_sz = DIV_ROUND_UP(mbox_send[i].msg_size, MBOX_SLOT_ALIGN);
+			last_ofs = last_sz * MBOX_SLOT_ALIGN;
+			last_slot = last_idx * MBOX_SLOT_ALIGN;
+		} else if (mbox < mbox_send[i].mbox_id) {
+			/* no need to search the rest id */
+			break;
+		}
+	}
+
+	for (i = 0; i < mbdev->recv_count; i++) {
+		if (mbox == mbox_recv[i].mbox_id) {
+			mbox_recv[i].offset = last_ofs + last_slot;
+			mbox_recv[i].pin_index = last_idx + last_sz;
+			last_idx = mbox_recv[i].pin_index;
+			last_sz = DIV_ROUND_UP(mbox_recv[i].msg_size, MBOX_SLOT_ALIGN);
+			last_ofs = last_sz * MBOX_SLOT_ALIGN;
+			last_slot = last_idx * MBOX_SLOT_ALIGN;
+		} else if (mbox < mbox_recv[i].mbox_id) {
+			/* no need to search the rest id */
+			break;
+		}
+	}
+
+	if (last_idx > MBOX_MAX_PIN || (last_ofs + last_slot) > MAX_SLOT_NUM)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int mtk_vcp_ipc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_vcp_ipc *vcp_ipc;
+	struct mbox_client *cl;
+	struct mtk_mbox_info *minfo;
+	int ret;
+	u32 mbox, i;
+	struct mtk_mbox_table *mbox_data = dev_get_platdata(dev);
+
+	device_set_of_node_from_dev(&pdev->dev, pdev->dev.parent);
+
+	vcp_ipc = devm_kzalloc(dev, sizeof(*vcp_ipc), GFP_KERNEL);
+	if (!vcp_ipc)
+		return -ENOMEM;
+
+	if (!mbox_data) {
+		dev_err(dev, "No platform data available\n");
+		return -EINVAL;
+	}
+	vcp_ipc->mbdev = mbox_data;
+
+	/* alloc and init mmup_mbox_info */
+	vcp_ipc->info_table = vzalloc(sizeof(*vcp_ipc->info_table) * VCP_MBOX_NUM);
+	if (!vcp_ipc->info_table)
+		return -ENOMEM;
+
+	/* create mbox dev */
+	for (mbox = 0; mbox < VCP_MBOX_NUM; mbox++) {
+		minfo = &vcp_ipc->info_table[mbox];
+		minfo->mbox_id = mbox;
+		minfo->vcp_ipc = vcp_ipc;
+		spin_lock_init(&minfo->mbox_lock);
+
+		ret = setup_mbox_table(vcp_ipc->mbdev, mbox);
+		if (ret)
+			return ret;
+
+		cl = &minfo->cl;
+		cl->dev = &pdev->dev;
+		cl->tx_block = false;
+		cl->knows_txdone = false;
+		cl->tx_prepare = NULL;
+		cl->rx_callback = mtk_vcp_ipc_recv;
+		minfo->ch = mbox_request_channel_byname(cl, mbox_names[mbox]);
+		if (IS_ERR(minfo->ch)) {
+			ret = PTR_ERR(minfo->ch);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to request mbox channel %s ret %d\n",
+					mbox_names[mbox], ret);
+
+			for (i = 0; i < mbox; i++) {
+				minfo = &vcp_ipc->info_table[i];
+				mbox_free_channel(minfo->ch);
+			}
+
+			vfree(vcp_ipc->info_table);
+			return ret;
+		}
+	}
+
+	vcp_ipc->dev = dev;
+	dev_set_drvdata(dev, vcp_ipc);
+	dev_dbg(dev, "MTK VCP IPC initialized\n");
+
+	return 0;
+}
+
+static void mtk_vcp_ipc_remove(struct platform_device *pdev)
+{
+	struct mtk_vcp_ipc *vcp_ipc = dev_get_drvdata(&pdev->dev);
+	struct mtk_mbox_info *minfo;
+	int i;
+
+	for (i = 0; i < VCP_MBOX_NUM; i++) {
+		minfo = &vcp_ipc->info_table[i];
+		mbox_free_channel(minfo->ch);
+	}
+
+	vfree(vcp_ipc->info_table);
+}
+
+static struct platform_driver mtk_vcp_ipc_driver = {
+	.probe = mtk_vcp_ipc_probe,
+	.remove = mtk_vcp_ipc_remove,
+	.driver = {
+		.name = "mtk-vcp-ipc",
+	},
+};
+builtin_platform_driver(mtk_vcp_ipc_driver);
+
+MODULE_AUTHOR("Jjian Zhou <jjian.zhou@mediatek.com>");
+MODULE_DESCRIPTION("MediaTek VCP IPC Controller");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/mediatek/mtk-vcp-ipc.h b/include/linux/firmware/mediatek/mtk-vcp-ipc.h
new file mode 100644
index 000000000000..dc34b0ba9dd8
--- /dev/null
+++ b/include/linux/firmware/mediatek/mtk-vcp-ipc.h
@@ -0,0 +1,151 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (c) 2024 MediaTek Inc.
+ */
+
+#ifndef __MTK_VCP_IPC_H__
+#define __MTK_VCP_IPC_H__
+
+#include <linux/completion.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/mtk-vcp-mailbox.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* IPI result definition */
+#define IPI_ACTION_DONE	  0
+#define IPI_DEV_ILLEGAL	 -1 /* ipi device is not initialized */
+#define IPI_ALREADY_USED	 -2 /* the ipi has be registered */
+#define IPI_UNAVAILABLE	 -3 /* the ipi can't be found */
+#define IPI_NO_MSGBUF		 -4 /* receiver doesn't have message buffer */
+#define IPI_MSG_TOO_BIG		 -5 /* message length is larger than defined */
+#define IPI_MBOX_ERR		-99 /* some error from rpmsg layer */
+
+/* mbox recv action definition */
+enum mtk_ipi_recv_opt {
+	MBOX_RECV_MESSAGE  = 0,
+	MBOX_RECV_ACK      = 1,
+};
+
+/* mbox table item number definition */
+#define send_item_num	3
+#define recv_item_num	4
+#define VCP_MBOX_NUM	5
+
+/* mbox slot size definition: 1 slot for 4 bytes */
+#define MBOX_SLOT_SIZE	0x4
+#define MBOX_MAX_PIN	32
+#define VCP_MBOX_NUM	5
+#define MBOX_SLOT_ALIGN	2
+
+struct mtk_vcp_ipc;
+struct mtk_ipi_chan_table;
+
+typedef int (*mbox_pin_cb_t)(u32 ipi_id, void *prdata, void *data, u32 len);
+
+/**
+ * mbox pin structure, this is for send definition,
+ * @offset: message offset in the slots of a mbox
+ * @msg_size: message used slots in the mbox, 4 bytes alignment
+ * @pin_index: bit offset in the mbox
+ * @ipi_id: ipi enum number
+ * @mbox_id: mbox number id
+ */
+struct mtk_mbox_send_table {
+	u32 offset;
+	u32 msg_size;
+	u32 pin_index;
+	u32 ipi_id;
+	u32 mbox_id;
+};
+
+/**
+ * mbox pin structure, this is for receive definition,
+ * @offset: message offset in the slots of a mbox
+ * @recv_opt: recv option,  0:receive ,1: response
+ * @msg_size: message used slots in the mbox, 4 bytes alignment
+ * @pin_index: bit offset in the mbox
+ * @ipi_id: ipi enum number
+ * @mbox_id: mbox number id
+ */
+struct mtk_mbox_recv_table {
+	u32 offset;
+	u32 recv_opt;
+	u32 msg_size;
+	u32 pin_index;
+	u32 ipi_id;
+	u32 mbox_id;
+};
+
+/**
+ * struct mtk_ipi_device - device for represent the tinysys using mtk ipi
+ * @name: name of tinysys device
+ * @id: device id (used to match between rpmsg drivers and devices)
+ * @vcp_ipc: vcp ipc structure for tinysys device
+ * @table: channel table with endpoint & channel_info & mbox_pin info
+ * @prdata: private data for the callback use
+ * @ipi_inited: set when vcp_ipi_device_register() done
+ */
+struct mtk_ipi_device  {
+	const char *name;
+	struct mtk_vcp_ipc *vcp_ipc;
+	struct mtk_ipi_chan_table *table;
+	void *prdata;
+	int ipi_inited;
+};
+
+/**
+ * The mtk_mbox_table is a structure used to record the send
+ * table and recv table. The send table is used to record
+ * the feature ID and size of the sent data. The recv table
+ * is used to record the feature ID and size of the received
+ * data, and whether a callback needs to be invoked.
+ *
+ * Following are platform specific interfacer
+ * @recv_table: structure mtk_mbox_recv_table
+ * @send_table: structure mtk_mbox_send_table
+ * @recv_count: receive feature number in this channel
+ * @send_count: send feature number in this channel
+ */
+struct mtk_mbox_table {
+	struct mtk_mbox_recv_table recv_table[32];
+	struct mtk_mbox_send_table send_table[32];
+	u32 recv_count;
+	u32 send_count;
+};
+
+/**
+ * Mbox is a dedicate hardware of a tinysys consists of:
+ * 1) a share memory tightly coupled to the tinysys
+ * 2) several IRQs
+ *
+ * Following are platform specific interface
+ * @dev: vcp device
+ * @name: identity of the device
+ * @info_table: mbox info structure
+ * @ipi_priv: private data for synchronization layer
+ * @mbox_id: mbox number
+ * @mbdev: mtk_mbox_table structure
+ */
+struct mtk_vcp_ipc {
+	struct device *dev;
+	const char *name;
+	struct mtk_mbox_info *info_table;
+	void *ipi_priv;
+	void *mbox_id;
+	struct mtk_mbox_table *mbdev;
+};
+
+int mtk_vcp_ipc_device_register(struct mtk_ipi_device *ipidev,
+				u32 ipi_chan_count,
+				struct mtk_vcp_ipc *vcp_ipc);
+int mtk_vcp_ipc_send(struct mtk_ipi_device *ipidev, u32 ipi_id,
+		     void *data, u32 len);
+int mtk_vcp_ipc_send_compl(struct mtk_ipi_device *ipidev, u32 ipi_id,
+			   void *data, u32 len, u32 timeout_ms);
+int mtk_vcp_mbox_ipc_register(struct mtk_ipi_device *ipidev, int ipi_id,
+			      mbox_pin_cb_t cb, void *prdata, void *msg);
+int mtk_vcp_mbox_ipc_unregister(struct mtk_ipi_device *ipidev, int ipi_id);
+
+#endif
-- 
2.45.2

[PATCH RFC v2 3/3] dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document

[Jjian Zhou]

This patch adds document for mediatek vcp mbox.

Signed-off-by: Jjian Zhou <jjian.zhou@mediatek.com>
---
 .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml b/Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
new file mode 100644
index 000000000000..bd1b024e22f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/mtk,mt8196-vcp-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Video Companion Processor (VCP) mailbox
+
+maintainers:
+  - Jjian Zhou <Jjian.Zhou@mediatek.com>
+
+description:
+  The MTK VCP mailbox enables the SoC to communicate with the VCP by passing
+  messages through 64 32-bit wide registers. It has 32 interrupt vectors in
+  either direction for signalling purposes.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8196-vcp-mbox
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#mbox-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    mailbox@31b80000 {
+        compatible = "mediatek,mt8196-vcp-mbox";
+        reg = <0x31b80000 0x1000>;
+        interrupts = <GIC_SPI 789 IRQ_TYPE_LEVEL_HIGH 0>;
+        #mbox-cells = <0>;
+    };
-- 
2.45.2

Re: [PATCH RFC v2 3/3] dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document

[Krzysztof Kozlowski]

On 17/03/2025 09:38, Jjian Zhou wrote:
> This patch adds document for mediatek vcp mbox.
> 
> Signed-off-by: Jjian Zhou <jjian.zhou@mediatek.com>
> ---
>  .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml | 49 +++++++++++++++++++

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml

Filename based on compatible. You already got this comment. Copy paste
your filename here:

<here goes filename>

and here your compatible:

<here goes compatible>

Are the the same?

Best regards,
Krzysztof

Re: [PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[AngeloGioacchino Del Regno]

Il 17/03/25 09:38, Jjian Zhou ha scritto:
> The VCP mailbox has 5 groups. Each group has corresponding interrupts,
> registers, and 64 slots (each slot is 4 bytes). Since different features
> share one of the mailbox groups, the VCP mailbox needs to establish a
> send table and a receive table. The send table is used to record the
> feature ID, mailbox ID, and the number of slots occupied. The receive table
> is used to record the feature ID, mailbox ID, the number of slots occupied,
> and the receive options. The API setup_mbox_table in mtk-vcp-ipc.c calculates
> the slot offset and pin index for each feature ID based on the mailbox ID and
> slot number in the send and receive tables (several slots form a pin, and
> each pin can trigger an interrupt). These descriptions are written in the
> mtk-vcp-ipc.c file -- we call it the IPC layer.
> 
> We have two questions:
> How should we describe the mailbox and IPI?
> Can the intermediate IPC layer be rewritten as a virtual mailbox layer?
> 

So, for this remote processor messaging system you have:
  - Dynamic channel allocation
    - Each channel has its own endpoint
    - Each channel has its own interrupt
  - Data send operation
    - Both with and without ACK indication from the remote processor
    - To channel -> endpoint
  - Data receive operation
    - From channel <- endpoint
    - When interrupt fires
    - Could use polling to avoid blocking in a few cases
  - A custom message structure not adhering to any standard

Check drivers/rpmsg/ :-)

On MediaTek platforms, there are many IPI to handle in many subsystems for
all of the remote processors that are integrated in the SoC and, at this
point, you might as well just aggregate all of the inter processor communication
stuff in one place, having an API that is made just exactly for that, instead
of keeping to duplicate the IPI stuff over and over (and yes I know that for each
remote processor the TX/RX is slightly different).

If you aggregate the IPI messaging in one place, maintenance is going to be easier,
and we stop getting duplication... more or less like it was done with the mtk_scp
IPI and mtk-vcodec .. and that's also something that is partially handled as RPMSG
because, well, it is a remote processor messaging driver.

Just to make people understand *how heavily* MediaTek SoCs rely on IPI, there's
a *partial* list of SoC IPs that use IPI communcation:

thermal, ccu, ccd, tinysys, vcp, atsp, sspm, slbc, mcupm, npu, mvpu, aps, mdla,
qos, audio, cm_mgr.... and... again, it's a partial list!

That said... any other opinion from anyone else?

Cheers,
Angelo

> Example of send and recve table:
> Operation | mbox_id | ipi_id | msg_size | align_size | slot_ofs | pin_index |  notes
> send          0          0       18          18           0          0
> recv          0          1       18          18          18          9
> send          1         15        8           8           0          0
> send          1         16       18          18           8          4
> send          1          9        2           2          26         13
> recv          1         15        8           8          28         14       ack of send ipi_id=15
> recv          1         17       18          18          36         18
> recv          1         10        2           2          54         27       ack of send ipi_id=2
> send          2         11       18          18           0          0
> send          2          2        2           2          18          9
> send          2          3        3           4          20         10
> send          2         32        2           2          24         12
> recv          2         12       18          18          26         13
> recv          2          5        1           2          44         22
> recv          2          2        1           2          46         23
> 
> Recv ipi_id=2 is the ack of send ipi_id=2(The ipi_id=15 is the same.)
> 
> Jjian Zhou (3):
>    mailbox: mediatek: Add mtk-vcp-mailbox driver
>    firmware: mediatek: Add vcp ipc protocol interface
>    dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document
> 
>   .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml |  49 ++
>   drivers/firmware/Kconfig                      |   9 +
>   drivers/firmware/Makefile                     |   1 +
>   drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
>   drivers/mailbox/Kconfig                       |   9 +
>   drivers/mailbox/Makefile                      |   2 +
>   drivers/mailbox/mtk-vcp-mailbox.c             | 179 +++++++
>   include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
>   include/linux/mailbox/mtk-vcp-mailbox.h       |  34 ++
>   9 files changed, 915 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
>   create mode 100644 drivers/firmware/mtk-vcp-ipc.c
>   create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
>   create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h
>   create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h
> 

Re: [PATCH RFC v2 3/3] dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document

[Krzysztof Kozlowski]

On 17/03/2025 10:00, Krzysztof Kozlowski wrote:
> On 17/03/2025 09:38, Jjian Zhou wrote:
>> This patch adds document for mediatek vcp mbox.
>>
>> Signed-off-by: Jjian Zhou <jjian.zhou@mediatek.com>
>> ---
>>  .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml | 49 +++++++++++++++++++
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
>>  1 file changed, 49 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
> 
> Filename based on compatible. You already got this comment. Copy paste
> your filename here:
> 
> <here goes filename>
> 
> and here your compatible:
> 
> <here goes compatible>
> 
> Are the the same?

Can you do this exercise here, instead of sending next versions?

Best regards,
Krzysztof

Re: [PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[Chen-Yu Tsai]

On Mon, Mar 17, 2025 at 6:07 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 17/03/25 09:38, Jjian Zhou ha scritto:
> > The VCP mailbox has 5 groups. Each group has corresponding interrupts,
> > registers, and 64 slots (each slot is 4 bytes). Since different features
> > share one of the mailbox groups, the VCP mailbox needs to establish a
> > send table and a receive table. The send table is used to record the
> > feature ID, mailbox ID, and the number of slots occupied. The receive table
> > is used to record the feature ID, mailbox ID, the number of slots occupied,
> > and the receive options. The API setup_mbox_table in mtk-vcp-ipc.c calculates
> > the slot offset and pin index for each feature ID based on the mailbox ID and
> > slot number in the send and receive tables (several slots form a pin, and
> > each pin can trigger an interrupt). These descriptions are written in the
> > mtk-vcp-ipc.c file -- we call it the IPC layer.
> >
> > We have two questions:
> > How should we describe the mailbox and IPI?
> > Can the intermediate IPC layer be rewritten as a virtual mailbox layer?
> >
>
> So, for this remote processor messaging system you have:
>   - Dynamic channel allocation
>     - Each channel has its own endpoint

The rpmsg model has:

- device -> the remote processor
- channel
- endpoint

However here for the VCP and possibly all the coprocessors using the
tinysys model, channel and endpoint are basically the same. If we
consider the "channel" to be the storage plus the interrupt vector,
and the "endpoint" to be the function running on the remote processor
servicing a given IPI ID, then it's always one endpoint per channel.

IMHO rpmsg gives too much latitude to make things confusing here.

rpmsg also requires the remote processor to support name service
announcements, which really doesn't exist. The endpoints and how
they map to the various hardware mailbox interrupt vectors and
storage is statically allocated, and thus needs to be described
in the driver.

>     - Each channel has its own interrupt
>   - Data send operation
>     - Both with and without ACK indication from the remote processor
>     - To channel -> endpoint
>   - Data receive operation
>     - From channel <- endpoint
>     - When interrupt fires
>     - Could use polling to avoid blocking in a few cases
>   - A custom message structure not adhering to any standard
>
> Check drivers/rpmsg/ :-)

While discussing this internally, I felt like that wasn't a really
correct model. IIUC rpmsg was first created to allow userspace to
pass messages to the remote processor. Then somehow devices were
being created on top of those channels.

Also, the existing mtk_rpmsg driver seemed a bit weird, like requiring
a DT node for each rpmsg endpoint.

That's why I thought mailboxes made more sense, as the terminology mapped
better. As a result I never brought up rpmsg in the discussion.

Perhaps that could be improved with better documentation for the MediaTek
specific implementation.

> On MediaTek platforms, there are many IPI to handle in many subsystems for
> all of the remote processors that are integrated in the SoC and, at this
> point, you might as well just aggregate all of the inter processor communication
> stuff in one place, having an API that is made just exactly for that, instead
> of keeping to duplicate the IPI stuff over and over (and yes I know that for each
> remote processor the TX/RX is slightly different).
>
> If you aggregate the IPI messaging in one place, maintenance is going to be easier,
> and we stop getting duplication... more or less like it was done with the mtk_scp
> IPI and mtk-vcodec .. and that's also something that is partially handled as RPMSG
> because, well, it is a remote processor messaging driver.
>
> Just to make people understand *how heavily* MediaTek SoCs rely on IPI, there's
> a *partial* list of SoC IPs that use IPI communcation:
>
> thermal, ccu, ccd, tinysys, vcp, atsp, sspm, slbc, mcupm, npu, mvpu, aps, mdla,
> qos, audio, cm_mgr.... and... again, it's a partial list!

Indeed, the newest chip has become quite complicated.

> That said... any other opinion from anyone else?

I tried to describe why I thought a virtual mailbox was better.


Thanks
ChenYu

> Cheers,
> Angelo
>
> > Example of send and recve table:
> > Operation | mbox_id | ipi_id | msg_size | align_size | slot_ofs | pin_index |  notes
> > send          0          0       18          18           0          0
> > recv          0          1       18          18          18          9
> > send          1         15        8           8           0          0
> > send          1         16       18          18           8          4
> > send          1          9        2           2          26         13
> > recv          1         15        8           8          28         14       ack of send ipi_id=15
> > recv          1         17       18          18          36         18
> > recv          1         10        2           2          54         27       ack of send ipi_id=2
> > send          2         11       18          18           0          0
> > send          2          2        2           2          18          9
> > send          2          3        3           4          20         10
> > send          2         32        2           2          24         12
> > recv          2         12       18          18          26         13
> > recv          2          5        1           2          44         22
> > recv          2          2        1           2          46         23
> >
> > Recv ipi_id=2 is the ack of send ipi_id=2(The ipi_id=15 is the same.)
> >
> > Jjian Zhou (3):
> >    mailbox: mediatek: Add mtk-vcp-mailbox driver
> >    firmware: mediatek: Add vcp ipc protocol interface
> >    dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document
> >
> >   .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml |  49 ++
> >   drivers/firmware/Kconfig                      |   9 +
> >   drivers/firmware/Makefile                     |   1 +
> >   drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
> >   drivers/mailbox/Kconfig                       |   9 +
> >   drivers/mailbox/Makefile                      |   2 +
> >   drivers/mailbox/mtk-vcp-mailbox.c             | 179 +++++++
> >   include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
> >   include/linux/mailbox/mtk-vcp-mailbox.h       |  34 ++
> >   9 files changed, 915 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
> >   create mode 100644 drivers/firmware/mtk-vcp-ipc.c
> >   create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
> >   create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h
> >   create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h
> >
>
>
>

Re: [PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[AngeloGioacchino Del Regno]

Il 18/03/25 08:44, Chen-Yu Tsai ha scritto:
> On Mon, Mar 17, 2025 at 6:07 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 17/03/25 09:38, Jjian Zhou ha scritto:
>>> The VCP mailbox has 5 groups. Each group has corresponding interrupts,
>>> registers, and 64 slots (each slot is 4 bytes). Since different features
>>> share one of the mailbox groups, the VCP mailbox needs to establish a
>>> send table and a receive table. The send table is used to record the
>>> feature ID, mailbox ID, and the number of slots occupied. The receive table
>>> is used to record the feature ID, mailbox ID, the number of slots occupied,
>>> and the receive options. The API setup_mbox_table in mtk-vcp-ipc.c calculates
>>> the slot offset and pin index for each feature ID based on the mailbox ID and
>>> slot number in the send and receive tables (several slots form a pin, and
>>> each pin can trigger an interrupt). These descriptions are written in the
>>> mtk-vcp-ipc.c file -- we call it the IPC layer.
>>>
>>> We have two questions:
>>> How should we describe the mailbox and IPI?
>>> Can the intermediate IPC layer be rewritten as a virtual mailbox layer?
>>>
>>
>> So, for this remote processor messaging system you have:
>>    - Dynamic channel allocation
>>      - Each channel has its own endpoint
> 
> The rpmsg model has:
> 
> - device -> the remote processor
> - channel
> - endpoint
> 
> However here for the VCP and possibly all the coprocessors using the
> tinysys model, channel and endpoint are basically the same.

For now, yes. Though, I expect multiple endpoints to become a thing in future
iterations of MediaTek SoCs, and this is based off how the hardware seems to
be evolving.

> If we
> consider the "channel" to be the storage plus the interrupt vector,
> and the "endpoint" to be the function running on the remote processor
> servicing a given IPI ID, then it's always one endpoint per channel.

Like this, yes - but if you consider ipi_id as the endpoint things will change.

Alternatively, if you consider the endpoint as function running on the remote
processor as you propose, and that I think could be the better alternative,
I still expect functions to grow in future SoCs.

> 
> IMHO rpmsg gives too much latitude to make things confusing here.
> 
> rpmsg also requires the remote processor to support name service
> announcements, which really doesn't exist.

I have doubts about that: all this is not properly documented and a kind of
service announcement could actually be existing - but let's assume that it
does not as that's the right thing to do.

There's still a way around that anyway, and even though it's not the prettiest
thing on Earth, it's not a big deal imo.

> The endpoints and how
> they map to the various hardware mailbox interrupt vectors and
> storage is statically allocated, and thus needs to be described
> in the driver.
> 

I'm not sure I understand this sentence, but it feels like this can be avoided
by simply using a cell in devicetree.

rpmsg = <&something 1 0>;  or  rpmsg = <&something 0>;

>>      - Each channel has its own interrupt
>>    - Data send operation
>>      - Both with and without ACK indication from the remote processor
>>      - To channel -> endpoint
>>    - Data receive operation
>>      - From channel <- endpoint
>>      - When interrupt fires
>>      - Could use polling to avoid blocking in a few cases
>>    - A custom message structure not adhering to any standard
>>
>> Check drivers/rpmsg/ :-)
> 
> While discussing this internally, I felt like that wasn't a really
> correct model. IIUC rpmsg was first created to allow userspace to
> pass messages to the remote processor. Then somehow devices were
> being created on top of those channels.
> 

Heh, if I recall correctly, I did see some userspace messaging in one of the
downstream kernels for other chips that are heavily using the IPI - check below
for a model hint :-)

> Also, the existing mtk_rpmsg driver seemed a bit weird, like requiring
> a DT node for each rpmsg endpoint.
> 

Weird... it's weird, agreed - but I call that necessary evil.
The other way around could be worse (note: that statement is purely by heart and
general knowledge around MediaTek SoCs, not about any specific code in particular).

> That's why I thought mailboxes made more sense, as the terminology mapped
> better. As a result I never brought up rpmsg in the discussion.

I think I do understand your thinking here - and I am not *strongly* disagreeing,
but I don't really agree for the reasons that I'm explaining in this reply.

> 
> Perhaps that could be improved with better documentation for the MediaTek
> specific implementation.
> 

Now that's what I'd really like to see here, because I feel like many things around
MediaTek SoCs are suboptimally engineered (in the software sense, because in the HW
sense I really do like them) and the *primary* reason for this is exactly the lack
of documentation... -> even internally <-.

>> On MediaTek platforms, there are many IPI to handle in many subsystems for
>> all of the remote processors that are integrated in the SoC and, at this
>> point, you might as well just aggregate all of the inter processor communication
>> stuff in one place, having an API that is made just exactly for that, instead
>> of keeping to duplicate the IPI stuff over and over (and yes I know that for each
>> remote processor the TX/RX is slightly different).
>>
>> If you aggregate the IPI messaging in one place, maintenance is going to be easier,
>> and we stop getting duplication... more or less like it was done with the mtk_scp
>> IPI and mtk-vcodec .. and that's also something that is partially handled as RPMSG
>> because, well, it is a remote processor messaging driver.
>>
>> Just to make people understand *how heavily* MediaTek SoCs rely on IPI, there's
>> a *partial* list of SoC IPs that use IPI communcation:
>>
>> thermal, ccu, ccd, tinysys, vcp, atsp, sspm, slbc, mcupm, npu, mvpu, aps, mdla,
>> qos, audio, cm_mgr.... and... again, it's a partial list!
> 
> Indeed, the newest chip has become quite complicated.
> 

..and I'd like to add: for many good reasons :-)

>> That said... any other opinion from anyone else?
> 
> I tried to describe why I thought a virtual mailbox was better.
> 

The implementation issue arising with a virtual mailbox driver is that then each
driver for each IP (thermal, ccu, vcp, slbc, this and that) will contain a direct
copy of the same part-two implementation for IPI communication: this can especially
be seen on downstream kernels for MediaTek Dimensity 9xxx smartphone chips.

If done with a mailbox, there's going to be no way around it - describing it all
will be very long, so I am not doing that right now in this reply, but I invite
you to check the MT6989 kernel to understand what I'm talking about :-)

Cheers!

> 
> Thanks
> ChenYu
> 
>> Cheers,
>> Angelo
>>
>>> Example of send and recve table:
>>> Operation | mbox_id | ipi_id | msg_size | align_size | slot_ofs | pin_index |  notes
>>> send          0          0       18          18           0          0
>>> recv          0          1       18          18          18          9
>>> send          1         15        8           8           0          0
>>> send          1         16       18          18           8          4
>>> send          1          9        2           2          26         13
>>> recv          1         15        8           8          28         14       ack of send ipi_id=15
>>> recv          1         17       18          18          36         18
>>> recv          1         10        2           2          54         27       ack of send ipi_id=2
>>> send          2         11       18          18           0          0
>>> send          2          2        2           2          18          9
>>> send          2          3        3           4          20         10
>>> send          2         32        2           2          24         12
>>> recv          2         12       18          18          26         13
>>> recv          2          5        1           2          44         22
>>> recv          2          2        1           2          46         23
>>>
>>> Recv ipi_id=2 is the ack of send ipi_id=2(The ipi_id=15 is the same.)
>>>
>>> Jjian Zhou (3):
>>>     mailbox: mediatek: Add mtk-vcp-mailbox driver
>>>     firmware: mediatek: Add vcp ipc protocol interface
>>>     dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document
>>>
>>>    .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml |  49 ++
>>>    drivers/firmware/Kconfig                      |   9 +
>>>    drivers/firmware/Makefile                     |   1 +
>>>    drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
>>>    drivers/mailbox/Kconfig                       |   9 +
>>>    drivers/mailbox/Makefile                      |   2 +
>>>    drivers/mailbox/mtk-vcp-mailbox.c             | 179 +++++++
>>>    include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
>>>    include/linux/mailbox/mtk-vcp-mailbox.h       |  34 ++
>>>    9 files changed, 915 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
>>>    create mode 100644 drivers/firmware/mtk-vcp-ipc.c
>>>    create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
>>>    create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h
>>>    create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h
>>>
>>
>>
>>

Re: [PATCH RFC v2 0/3] add VCP mailbox and IPC driver

[Chen-Yu Tsai]

Hi,

Sorry for the late reply.

On Wed, Apr 2, 2025 at 5:58 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 18/03/25 08:44, Chen-Yu Tsai ha scritto:
> > On Mon, Mar 17, 2025 at 6:07 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 17/03/25 09:38, Jjian Zhou ha scritto:
> >>> The VCP mailbox has 5 groups. Each group has corresponding interrupts,
> >>> registers, and 64 slots (each slot is 4 bytes). Since different features
> >>> share one of the mailbox groups, the VCP mailbox needs to establish a
> >>> send table and a receive table. The send table is used to record the
> >>> feature ID, mailbox ID, and the number of slots occupied. The receive table
> >>> is used to record the feature ID, mailbox ID, the number of slots occupied,
> >>> and the receive options. The API setup_mbox_table in mtk-vcp-ipc.c calculates
> >>> the slot offset and pin index for each feature ID based on the mailbox ID and
> >>> slot number in the send and receive tables (several slots form a pin, and
> >>> each pin can trigger an interrupt). These descriptions are written in the
> >>> mtk-vcp-ipc.c file -- we call it the IPC layer.
> >>>
> >>> We have two questions:
> >>> How should we describe the mailbox and IPI?
> >>> Can the intermediate IPC layer be rewritten as a virtual mailbox layer?
> >>>
> >>
> >> So, for this remote processor messaging system you have:
> >>    - Dynamic channel allocation
> >>      - Each channel has its own endpoint
> >
> > The rpmsg model has:
> >
> > - device -> the remote processor
> > - channel
> > - endpoint
> >
> > However here for the VCP and possibly all the coprocessors using the
> > tinysys model, channel and endpoint are basically the same.
>
> For now, yes. Though, I expect multiple endpoints to become a thing in future
> iterations of MediaTek SoCs, and this is based off how the hardware seems to
> be evolving.

I don't see it happening yet. What I think will happen is they will add
more coprocessors and corresponding mailbox controllers. And I think we
should focus on what the current firmware implements and try to model
that.

I think we should just ask MediaTek if their firmware interface is fixed
and what they would do if they ran out of IPI space.

> > If we
> > consider the "channel" to be the storage plus the interrupt vector,
> > and the "endpoint" to be the function running on the remote processor
> > servicing a given IPI ID, then it's always one endpoint per channel.
>
> Like this, yes - but if you consider ipi_id as the endpoint things will change.
>
> Alternatively, if you consider the endpoint as function running on the remote
> processor as you propose, and that I think could be the better alternative,
> I still expect functions to grow in future SoCs.

Sure, but it's more than likely they will add more IPIs to handle that.

> >
> > IMHO rpmsg gives too much latitude to make things confusing here.
> >
> > rpmsg also requires the remote processor to support name service
> > announcements, which really doesn't exist.
>
> I have doubts about that: all this is not properly documented and a kind of
> service announcement could actually be existing - but let's assume that it
> does not as that's the right thing to do.
>
> There's still a way around that anyway, and even though it's not the prettiest
> thing on Earth, it's not a big deal imo.
>
> > The endpoints and how
> > they map to the various hardware mailbox interrupt vectors and
> > storage is statically allocated, and thus needs to be described
> > in the driver.
> >
>
> I'm not sure I understand this sentence, but it feels like this can be avoided
> by simply using a cell in devicetree.
>
> rpmsg = <&something 1 0>;  or  rpmsg = <&something 0>;

That could work. Then again rpmsg is some software construct. Also not
all coprocessors implement the same thing. See below.

> >>      - Each channel has its own interrupt
> >>    - Data send operation
> >>      - Both with and without ACK indication from the remote processor
> >>      - To channel -> endpoint
> >>    - Data receive operation
> >>      - From channel <- endpoint
> >>      - When interrupt fires
> >>      - Could use polling to avoid blocking in a few cases
> >>    - A custom message structure not adhering to any standard
> >>
> >> Check drivers/rpmsg/ :-)
> >
> > While discussing this internally, I felt like that wasn't a really
> > correct model. IIUC rpmsg was first created to allow userspace to
> > pass messages to the remote processor. Then somehow devices were
> > being created on top of those channels.
> >
>
> Heh, if I recall correctly, I did see some userspace messaging in one of the
> downstream kernels for other chips that are heavily using the IPI - check below
> for a model hint :-)

Is that direct messaging or going through some ioctl interface? I think
direct messaging to some system critical firmware without any sanity
checks is potentially dangerous.

> > Also, the existing mtk_rpmsg driver seemed a bit weird, like requiring
> > a DT node for each rpmsg endpoint.
> >
>
> Weird... it's weird, agreed - but I call that necessary evil.
> The other way around could be worse (note: that statement is purely by heart and
> general knowledge around MediaTek SoCs, not about any specific code in particular).
>
> > That's why I thought mailboxes made more sense, as the terminology mapped
> > better. As a result I never brought up rpmsg in the discussion.
>
> I think I do understand your thinking here - and I am not *strongly* disagreeing,
> but I don't really agree for the reasons that I'm explaining in this reply.
>
> >
> > Perhaps that could be improved with better documentation for the MediaTek
> > specific implementation.
> >
>
> Now that's what I'd really like to see here, because I feel like many things around
> MediaTek SoCs are suboptimally engineered (in the software sense, because in the HW
> sense I really do like them) and the *primary* reason for this is exactly the lack
> of documentation... -> even internally <-.

That I agree with.

> >> On MediaTek platforms, there are many IPI to handle in many subsystems for
> >> all of the remote processors that are integrated in the SoC and, at this
> >> point, you might as well just aggregate all of the inter processor communication
> >> stuff in one place, having an API that is made just exactly for that, instead
> >> of keeping to duplicate the IPI stuff over and over (and yes I know that for each
> >> remote processor the TX/RX is slightly different).
> >>
> >> If you aggregate the IPI messaging in one place, maintenance is going to be easier,
> >> and we stop getting duplication... more or less like it was done with the mtk_scp
> >> IPI and mtk-vcodec .. and that's also something that is partially handled as RPMSG
> >> because, well, it is a remote processor messaging driver.
> >>
> >> Just to make people understand *how heavily* MediaTek SoCs rely on IPI, there's
> >> a *partial* list of SoC IPs that use IPI communcation:
> >>
> >> thermal, ccu, ccd, tinysys, vcp, atsp, sspm, slbc, mcupm, npu, mvpu, aps, mdla,
> >> qos, audio, cm_mgr.... and... again, it's a partial list!
> >
> > Indeed, the newest chip has become quite complicated.
> >
>
> ..and I'd like to add: for many good reasons :-)

It also creates new problems, such as resource handover. There are
internal regulators or clock controls that are controlled by the
firmware after the coprocessor is brought up. But until then, in
some cases those resources are still under the control of the kernel;
in other cases they are left turned on by the bootloader so as not
to block consumers. Some delicate (read: fragile) handover was done,
which I'm not very fond of.

> >> That said... any other opinion from anyone else?
> >
> > I tried to describe why I thought a virtual mailbox was better.
> >
>
> The implementation issue arising with a virtual mailbox driver is that then each
> driver for each IP (thermal, ccu, vcp, slbc, this and that) will contain a direct
> copy of the same part-two implementation for IPI communication: this can especially
> be seen on downstream kernels for MediaTek Dimensity 9xxx smartphone chips.

For the code that MediaTek has provided for MT8196, I can say that there
are at least three types of firmware interfaces:

- ADSP

We're running SOF, so that might be different from the Android stuff that
you saw for 9300. The ADSP mailboxes don't have any shared storage or
channels. They are simply doorbells. The shared storage interface is
likely common to SOF implementations

- Tinysys over SCMI

This seems to be the main coprocessor, which seems to be the SSPM.
Judging by the associated drivers, the firmware implements a vendor
extension protocol (0x80) over SCMI, over which it provides various
commands.

From the code we have it looks like this part manages the QOS, SLBC, and
CM_MGR parts.

- Tinysys over mailbox controllers (with scratch registers)

This covers the other coprocessors such as VCP, GPUEB, MCUPM and
probably others that we don't currently have included in our tree.
This is the part we are discussing.

Given how the communication channels are crammed into the mailboxes,
it's unlikely you can get rid of said part-two implementation. There
needs to be something that maps individual channels onto the mailbox
channels and combined storage. The question is simply how we want to
model this. And regardless of whether it be rpmsg or virtual mailbox,
we will end up with some shared library code to implement this
translation. This is what currently exists as mtk-mbox.c and
mtk_tinysys_ipi.c under drivers/soc/mediaktek/ in our tree. I assume
there is something similar in the code that you have, given that
most of the code we have for MT8196 was ported over from their
internal trees without much modification.

mtk-mbox.c handles the grouped mailboxes, while mtk_tinysys_ipi.c provides
the IPI interface. I assume the latter is what you are referring to as
"part-two IPI". mtk_tinysys_ipi.c goes even further as it builds the
IPI interface on top of rpmsg (implemented in drivers/rpmsg/mtk_rpmsg_mbox.c).

What I would like to see is to drop the low level mailbox stuff from
mtk-mbox.c, move the mailbox combining and repartitioning bits to a
library that implements the IPI interface, using rpmsg or virtual
mailboxes or whatever. I don't see why this would end up with multiple
copies. The only thing that coprocessor drivers need to have is a
table on how the IPIs map to the mailboxes.

> If done with a mailbox, there's going to be no way around it - describing it all
> will be very long, so I am not doing that right now in this reply, but I invite
> you to check the MT6989 kernel to understand what I'm talking about :-)

You'll have to provide a link to that. However I think a lot of the code
for MT8196 is derived from the same source.


Thanks
ChenYu


> Cheers!
>
> >
> > Thanks
> > ChenYu
> >
> >> Cheers,
> >> Angelo
> >>
> >>> Example of send and recve table:
> >>> Operation | mbox_id | ipi_id | msg_size | align_size | slot_ofs | pin_index |  notes
> >>> send          0          0       18          18           0          0
> >>> recv          0          1       18          18          18          9
> >>> send          1         15        8           8           0          0
> >>> send          1         16       18          18           8          4
> >>> send          1          9        2           2          26         13
> >>> recv          1         15        8           8          28         14       ack of send ipi_id=15
> >>> recv          1         17       18          18          36         18
> >>> recv          1         10        2           2          54         27       ack of send ipi_id=2
> >>> send          2         11       18          18           0          0
> >>> send          2          2        2           2          18          9
> >>> send          2          3        3           4          20         10
> >>> send          2         32        2           2          24         12
> >>> recv          2         12       18          18          26         13
> >>> recv          2          5        1           2          44         22
> >>> recv          2          2        1           2          46         23
> >>>
> >>> Recv ipi_id=2 is the ack of send ipi_id=2(The ipi_id=15 is the same.)
> >>>
> >>> Jjian Zhou (3):
> >>>     mailbox: mediatek: Add mtk-vcp-mailbox driver
> >>>     firmware: mediatek: Add vcp ipc protocol interface
> >>>     dt-bindings: mailbox: mtk,vcp-mbox: add mtk vcp-mbox document
> >>>
> >>>    .../bindings/mailbox/mtk,mt8196-vcp-mbox.yaml |  49 ++
> >>>    drivers/firmware/Kconfig                      |   9 +
> >>>    drivers/firmware/Makefile                     |   1 +
> >>>    drivers/firmware/mtk-vcp-ipc.c                | 481 ++++++++++++++++++
> >>>    drivers/mailbox/Kconfig                       |   9 +
> >>>    drivers/mailbox/Makefile                      |   2 +
> >>>    drivers/mailbox/mtk-vcp-mailbox.c             | 179 +++++++
> >>>    include/linux/firmware/mediatek/mtk-vcp-ipc.h | 151 ++++++
> >>>    include/linux/mailbox/mtk-vcp-mailbox.h       |  34 ++
> >>>    9 files changed, 915 insertions(+)
> >>>    create mode 100644 Documentation/devicetree/bindings/mailbox/mtk,mt8196-vcp-mbox.yaml
> >>>    create mode 100644 drivers/firmware/mtk-vcp-ipc.c
> >>>    create mode 100644 drivers/mailbox/mtk-vcp-mailbox.c
> >>>    create mode 100644 include/linux/firmware/mediatek/mtk-vcp-ipc.h
> >>>    create mode 100644 include/linux/mailbox/mtk-vcp-mailbox.h
> >>>
> >>
> >>
> >>