* [PATCH v5 01/14] media: mediatek: vcodec: support vcp architecture
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
` (12 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
Some platforms expose the video codec through the VCP coprocessor.
Use the VCP architecture when the VCP coprocessor is found.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h | 1 +
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
index 300363a40158..c1642fb09b42 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
@@ -14,6 +14,7 @@ struct mtk_vcodec_enc_dev;
enum mtk_vcodec_fw_type {
VPU,
SCP,
+ VCP,
};
enum mtk_vcodec_fw_use {
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index e936ed8dffba..d220b645e455 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -379,6 +379,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
} else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
&rproc_phandle)) {
fw_type = SCP;
+ } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vcp",
+ &rproc_phandle)) {
+ fw_type = VCP;
} else {
dev_dbg(&pdev->dev, "Could not get vdec IPI device");
return -ENODEV;
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:58 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
` (11 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The processor is changed from scp to vcp in mt8196 platform.
Adding new firmware interface to communicate kernel with vcp
for the communication method is changed.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../media/platform/mediatek/vcodec/Kconfig | 4 +
.../platform/mediatek/vcodec/common/Makefile | 4 +
.../mediatek/vcodec/common/mtk_vcodec_fw.c | 26 +-
.../vcodec/common/mtk_vcodec_fw_priv.h | 12 +
.../vcodec/common/mtk_vcodec_fw_scp.c | 1 +
.../vcodec/common/mtk_vcodec_fw_vcp.c | 505 ++++++++++++++++++
.../vcodec/common/mtk_vcodec_fw_vcp.h | 142 +++++
.../vcodec/common/mtk_vcodec_fw_vpu.c | 1 +
.../vcodec/decoder/mtk_vcodec_dec_drv.c | 3 +
.../vcodec/decoder/mtk_vcodec_dec_drv.h | 2 +
.../vcodec/encoder/mtk_vcodec_enc_drv.c | 2 +
.../vcodec/encoder/mtk_vcodec_enc_drv.h | 2 +
12 files changed, 691 insertions(+), 13 deletions(-)
create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig b/drivers/media/platform/mediatek/vcodec/Kconfig
index bc8292232530..d23dad5c78ce 100644
--- a/drivers/media/platform/mediatek/vcodec/Kconfig
+++ b/drivers/media/platform/mediatek/vcodec/Kconfig
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
+config VIDEO_MEDIATEK_VCODEC_VCP
+ bool
+
config VIDEO_MEDIATEK_VCODEC_SCP
bool
@@ -21,6 +24,7 @@ config VIDEO_MEDIATEK_VCODEC
select V4L2_MEM2MEM_DEV
select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP
+ select VIDEO_MEDIATEK_VCODEC_VCP if MTK_VCP_RPROC
select V4L2_H264
select V4L2_VP9
select MEDIA_CONTROLLER
diff --git a/drivers/media/platform/mediatek/vcodec/common/Makefile b/drivers/media/platform/mediatek/vcodec/common/Makefile
index d0479914dfb3..2f68692e8c98 100644
--- a/drivers/media/platform/mediatek/vcodec/common/Makefile
+++ b/drivers/media/platform/mediatek/vcodec/common/Makefile
@@ -14,6 +14,10 @@ ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP),)
mtk-vcodec-common-y += mtk_vcodec_fw_scp.o
endif
+ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP),)
+mtk-vcodec-common-y += mtk_vcodec_fw_vcp.o
+endif
+
ifneq ($(CONFIG_DEBUG_FS),)
obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec-dbgfs.o
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
index 08949b08fbc6..64977bcd6abd 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
@@ -3,26 +3,26 @@
#include "../decoder/mtk_vcodec_dec_drv.h"
#include "../encoder/mtk_vcodec_enc_drv.h"
#include "mtk_vcodec_fw_priv.h"
+#include "mtk_vcodec_fw_vcp.h"
struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type type,
enum mtk_vcodec_fw_use fw_use)
{
- struct platform_device *plat_dev;
+ if (fw_use == ENCODER) {
+ struct mtk_vcodec_enc_dev *enc_dev = priv;
- if (fw_use == ENCODER)
- plat_dev = ((struct mtk_vcodec_enc_dev *)priv)->plat_dev;
- else
- plat_dev = ((struct mtk_vcodec_dec_dev *)priv)->plat_dev;
+ return enc_dev->fw_init(priv, fw_use);
+ }
+
+ if (fw_use == DECODER) {
+ struct mtk_vcodec_dec_dev *dec_dev = priv;
- switch (type) {
- case VPU:
- return mtk_vcodec_fw_vpu_init(priv, fw_use);
- case SCP:
- return mtk_vcodec_fw_scp_init(priv, fw_use);
- default:
- dev_err(&plat_dev->dev, "Invalid vcodec fw type");
- return ERR_PTR(-EINVAL);
+ return dec_dev->fw_init(priv, fw_use);
}
+
+ pr_err("Error in selecting firmware, fw_use is not set");
+
+ return ERR_PTR(-EINVAL);
}
EXPORT_SYMBOL_GPL(mtk_vcodec_fw_select);
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
index 99603accd82e..0a2a9b010244 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
@@ -4,6 +4,7 @@
#define _MTK_VCODEC_FW_PRIV_H_
#include "mtk_vcodec_fw.h"
+#include "mtk_vcodec_fw_vcp.h"
struct mtk_vcodec_dec_dev;
struct mtk_vcodec_enc_dev;
@@ -13,6 +14,7 @@ struct mtk_vcodec_fw {
const struct mtk_vcodec_fw_ops *ops;
struct platform_device *pdev;
struct mtk_scp *scp;
+ struct mtk_vcp *vcp;
enum mtk_vcodec_fw_use fw_use;
};
@@ -49,4 +51,14 @@ mtk_vcodec_fw_scp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
}
#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_SCP */
+#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP)
+struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use);
+#else
+static inline struct mtk_vcodec_fw *
+mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_VCP */
+
#endif /* _MTK_VCODEC_FW_PRIV_H_ */
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
index 1b0bc47355c0..1aad5c3ef990 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_scp.c
@@ -90,3 +90,4 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_scp_init(void *priv, enum mtk_vcodec_fw_use
return fw;
}
+EXPORT_SYMBOL_GPL(mtk_vcodec_fw_scp_init);
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
new file mode 100644
index 000000000000..2f3bfcd97958
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ * Author: Yunfei Dong <yunfei.dong@mediatek.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-mapping.h>
+#include <linux/iommu.h>
+#include <linux/remoteproc/mtk_vcp_public.h>
+#include <linux/firmware/mediatek/mtk-vcp-ipc.h>
+
+#include "../decoder/mtk_vcodec_dec_drv.h"
+#include "../decoder/vdec_ipi_msg.h"
+#include "mtk_vcodec_fw_priv.h"
+
+#define IPI_SEND_TIMEOUT_MS 100U
+#define IPI_TIMEOUT_MS 100U
+
+#define VCP_IPI_HEADER_SIZE (sizeof(u32) * 2)
+#define VCP_IPI_ALIGN (4)
+
+static struct mutex *mtk_vcodec_vcp_get_ipi_lock(struct mtk_vcp *vcp, u32 ipi_id)
+{
+ return &vcp->ipi_desc[ipi_id].lock;
+}
+
+static void mtk_vcodec_vcp_ipi_lock(struct mtk_vcp *vcp, u32 ipi_id)
+{
+ struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp, ipi_id);
+
+ if (!lock)
+ return;
+
+ mutex_lock(lock);
+}
+
+static void mtk_vcodec_vcp_ipi_unlock(struct mtk_vcp *vcp, u32 ipi_id)
+{
+ struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp, ipi_id);
+
+ if (!lock)
+ return;
+
+ lockdep_assert_held(lock);
+ mutex_unlock(lock);
+}
+
+static spinlock_t *mtk_vcodec_vcp_get_msg_queue_lock(struct mtk_vcodec_fw *fw)
+{
+ return &fw->vcp->msg_queue.lock;
+}
+
+static void mtk_vcodec_vcp_msq_queue_lock(struct mtk_vcodec_fw *fw, unsigned long *flags)
+{
+ spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
+
+ if (!lock)
+ return;
+
+ spin_lock_irqsave(lock, *flags);
+}
+
+static void mtk_vcodec_vcp_msq_queue_unlock(struct mtk_vcodec_fw *fw, unsigned long *flags)
+{
+ spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
+
+ if (!lock)
+ return;
+
+ spin_unlock_irqrestore(lock, *flags);
+}
+
+static int mtk_vcodec_vcp_notifier(struct notifier_block *nb, unsigned long event, void *ptr)
+{
+ struct mtk_vcp *vcp = container_of(nb, struct mtk_vcp, vcp_notify);
+
+ switch (event) {
+ case VCP_EVENT_SUSPEND:
+ case VCP_EVENT_STOP:
+ dev_dbg(&vcp->pdev->dev, "vcp notifier suspend");
+ break;
+ case VCP_EVENT_READY:
+ case VCP_EVENT_RESUME:
+ dev_dbg(&vcp->pdev->dev, "vcp notifier ready");
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static void mtk_vcodec_vcp_free_msg_node(struct mtk_vcodec_fw *fw,
+ struct mtk_vcp_msg_node *msg_node)
+{
+ unsigned long flags;
+
+ mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
+ list_add(&msg_node->list, &fw->vcp->msg_queue.node_list);
+ mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
+}
+
+static int mtk_vcodec_vcp_ipi_register(struct mtk_vcp *vcp, u32 ipi_id, vcp_ipi_handler_t handler,
+ void *priv)
+{
+ if (!vcp)
+ return -EPROBE_DEFER;
+
+ if (WARN_ON(ipi_id >= VCP_IPI_MAX) || WARN_ON(!handler))
+ return -EINVAL;
+
+ mtk_vcodec_vcp_ipi_lock(vcp, ipi_id);
+ vcp->ipi_desc[ipi_id].handler = handler;
+ vcp->ipi_desc[ipi_id].priv = priv;
+ mtk_vcodec_vcp_ipi_unlock(vcp, ipi_id);
+
+ return 0;
+}
+
+static int mtk_vcodec_vcp_msg_process_thread(void *arg)
+{
+ struct mtk_vcodec_fw *fw = arg;
+ struct vdec_vpu_ipi_ack *msg = NULL;
+ struct mtk_vcp_share_obj *obj;
+ struct mtk_vcp_msg_node *msg_node;
+ vcp_ipi_handler_t handler;
+ unsigned long flags;
+ int ret = 0;
+
+ do {
+ ret = wait_event_interruptible(fw->vcp->msg_queue.wq,
+ atomic_read(&fw->vcp->msg_queue.cnt) > 0);
+ if (ret < 0) {
+ dev_err(&fw->pdev->dev, "wait msg queue ack timeout %d %d\n",
+ ret, atomic_read(&fw->vcp->msg_queue.cnt));
+ continue;
+ }
+
+ mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
+ msg_node = list_entry(fw->vcp->msg_queue.msg_list.next,
+ struct mtk_vcp_msg_node, list);
+ list_del(&msg_node->list);
+ atomic_dec(&fw->vcp->msg_queue.cnt);
+ mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
+
+ obj = &msg_node->ipi_data;
+ msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
+
+ if (!msg->ap_inst_addr) {
+ dev_err(&fw->pdev->dev, "invalid message address\n");
+ mtk_vcodec_vcp_free_msg_node(fw, msg_node);
+ continue;
+ }
+
+ dev_dbg(&fw->pdev->dev, "msg ack id %d len %d msg_id 0x%x\n", obj->id, obj->len,
+ msg->msg_id);
+
+ mtk_vcodec_vcp_ipi_lock(fw->vcp, obj->id);
+ handler = fw->vcp->ipi_desc[obj->id].handler;
+ if (!handler) {
+ dev_err(&fw->pdev->dev, "invalid ack ipi handler id = %d\n", obj->id);
+ mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
+ mtk_vcodec_vcp_free_msg_node(fw, msg_node);
+ return -EINVAL;
+ }
+
+ handler(msg, obj->len, fw->vcp->ipi_desc[obj->id].priv);
+ mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
+
+ fw->vcp->msg_signaled[obj->id] = true;
+ wake_up(&fw->vcp->msg_wq[obj->id]);
+
+ mtk_vcodec_vcp_free_msg_node(fw, msg_node);
+ } while (!kthread_should_stop());
+
+ return ret;
+}
+
+static int mtk_vcodec_vcp_msg_ack_isr(unsigned int id, void *prdata, void *data, unsigned int len)
+{
+ struct mtk_vcodec_fw *fw = prdata;
+ struct mtk_vcp_msg_queue *msg_queue = &fw->vcp->msg_queue;
+ struct mtk_vcp_msg_node *msg_node;
+ struct vdec_vpu_ipi_ack *msg = NULL;
+ struct mtk_vcp_share_obj *obj = data;
+ unsigned long flags;
+
+ msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
+
+ mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
+ if (!list_empty(&msg_queue->node_list)) {
+ msg_node = list_entry(msg_queue->node_list.next, struct mtk_vcp_msg_node, list);
+
+ memcpy(&msg_node->ipi_data, obj, sizeof(*obj));
+ list_move_tail(&msg_node->list, &msg_queue->msg_list);
+ atomic_inc(&msg_queue->cnt);
+ mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
+
+ dev_dbg(&fw->pdev->dev, "push ipi_id %x msg_id %x, msg cnt %d\n",
+ obj->id, msg->msg_id, atomic_read(&msg_queue->cnt));
+
+ wake_up(&msg_queue->wq);
+ } else {
+ mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
+ dev_err(&fw->pdev->dev, "no free nodes in msg queue\n");
+ }
+
+ return 0;
+}
+
+static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
+ unsigned int len, unsigned int wait)
+{
+ struct mtk_vcp *vcp = fw->vcp;
+ struct mtk_vcp_device *vcp_device = vcp->vcp_device;
+ bool *msg_signaled = &vcp->msg_signaled[id];
+ wait_queue_head_t *msg_wq = &vcp->msg_wq[id];
+ int ret, ipi_size, feature_id, mailbox_id, retry_cnt = 0;
+ unsigned long timeout_jiffies = 0;
+ struct mtk_vcp_share_obj obj = {0};
+ unsigned int *data;
+
+ if (!vcp_device) {
+ dev_dbg(&fw->pdev->dev, "vcp device is null\n");
+ return -EINVAL;
+ }
+
+ mutex_lock(&vcp->ipi_mutex);
+ feature_id = VDEC_FEATURE_ID;
+ mailbox_id = IPI_OUT_VDEC_1;
+
+ timeout_jiffies = jiffies + msecs_to_jiffies(VCP_SYNC_TIMEOUT_MS);
+ while (!vcp_device->ops->vcp_is_ready(feature_id)) {
+ if (time_after(jiffies, timeout_jiffies)) {
+ vcp->ipi_id_ack[id] = -EINVAL;
+ ret = -EINVAL;
+ goto error;
+ }
+ mdelay(1);
+ }
+
+ if (len > VCP_SHARE_BUF_SIZE) {
+ vcp->ipi_id_ack[id] = -EINVAL;
+ ret = -EINVAL;
+ goto error;
+ }
+
+ obj.id = id;
+ obj.len = len;
+ memcpy(obj.share_buf, buf, len);
+
+ ipi_size = round_up(VCP_IPI_HEADER_SIZE + len, VCP_IPI_ALIGN);
+ data = (unsigned int *)obj.share_buf;
+ dev_dbg(&fw->pdev->dev, "vcp send message: id %d len %d data 0x%x\n",
+ obj.id, obj.len, data[0]);
+
+ ret = mtk_vcp_ipc_send(vcp_get_ipidev(vcp_device), mailbox_id, &obj, ipi_size);
+ if (ret != IPI_ACTION_DONE) {
+ vcp->ipi_id_ack[id] = -EIO;
+ ret = -EIO;
+ goto error;
+ }
+
+wait_ack:
+ /* wait for VCP's ACK */
+ ret = wait_event_timeout(*msg_wq, *msg_signaled, msecs_to_jiffies(IPI_TIMEOUT_MS));
+ if (!ret || retry_cnt > 5) {
+ vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
+ dev_err(&fw->pdev->dev, "wait ipi ack timeout! %d %d\n", ret, vcp->ipi_id_ack[id]);
+ } else if (ret == -ERESTARTSYS) {
+ dev_err(&fw->pdev->dev, "wait ipi ack err (%d)\n", vcp->ipi_id_ack[id]);
+ retry_cnt++;
+ goto wait_ack;
+ } else if (ret < 0) {
+ dev_err(&fw->pdev->dev, "wait ipi ack fail ret %d %d\n", ret, vcp->ipi_id_ack[id]);
+ vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
+ }
+
+ dev_dbg(&fw->pdev->dev, "receive message: id %d len %d data 0x%x\n",
+ obj.id, obj.len, data[0]);
+
+ *msg_signaled = false;
+ mutex_unlock(&vcp->ipi_mutex);
+
+ return vcp->ipi_id_ack[id];
+
+error:
+ mutex_unlock(&vcp->ipi_mutex);
+ dev_err(&fw->pdev->dev, "send msg error type:%d msg:%d > %d ret:%d\n", fw->type, len,
+ VCP_SHARE_BUF_SIZE, ret);
+
+ return ret;
+}
+
+static int check_vcp_loaded(struct mtk_vcodec_fw *fw)
+{
+ struct device *dev = &fw->pdev->dev;
+ struct device_driver *drv;
+
+ drv = driver_find("mtk-vcp", &platform_bus_type);
+ if (!drv) {
+ dev_err(dev, "find mtk-vcp driver failed, need to reload.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int mtk_vcodec_vcp_get_vcp_device(struct mtk_vcodec_fw *fw)
+{
+ struct device *dev = &fw->pdev->dev;
+ int retry = 0, retry_cnt = 10000;
+ phandle vcp_phandle;
+
+ while (try_then_request_module(check_vcp_loaded(fw), "mtk-vcp")) {
+ if (++retry > retry_cnt) {
+ dev_err(dev, "failed to load mtk-vcp module");
+ return -EPROBE_DEFER;
+ }
+ usleep_range(1000, 2000);
+ }
+
+ if (of_property_read_u32(dev->of_node, "mediatek,vcp", &vcp_phandle)) {
+ dev_err(dev, "can't get vcp handle.\n");
+ return -ENODEV;
+ }
+
+ fw->vcp->vcp_device = mtk_vcp_get_by_phandle(vcp_phandle);
+ if (!fw->vcp->vcp_device) {
+ dev_err(dev, "get vcp device failed\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
+{
+ struct mtk_vcp_device *vcp_device;
+ int ret, feature_id, mem_id, mailbox_id, ipi_id;
+
+ if (fw->vcp->is_init_done) {
+ dev_dbg(&fw->pdev->dev, "vcp has already been initialized done.\n");
+ return 0;
+ }
+
+ if (mtk_vcodec_vcp_get_vcp_device(fw) < 0) {
+ dev_err(&fw->pdev->dev, "vcp device is null.\n");
+ return -EINVAL;
+ }
+
+ vcp_device = fw->vcp->vcp_device;
+
+ feature_id = VDEC_FEATURE_ID;
+ mem_id = VDEC_MEM_ID;
+ mailbox_id = IPI_IN_VDEC_1;
+ ipi_id = VCP_IPI_LAT_DECODER;
+
+ ret = mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
+ mtk_vcodec_vcp_msg_ack_isr, fw, &fw->vcp->share_data);
+ if (ret) {
+ dev_dbg(&fw->pdev->dev, "ipi register fail %d %d %d %d\n", ret, feature_id,
+ mem_id, mailbox_id);
+ return -EINVAL;
+ }
+
+ fw->vcp->vcp_notify.notifier_call = mtk_vcodec_vcp_notifier;
+ fw->vcp->vcp_notify.priority = 1;
+ vcp_device->ops->vcp_register_notify(feature_id, &fw->vcp->vcp_notify);
+
+ if (!fw->vcp->is_register_done) {
+ ret = vcp_device->ops->vcp_register_feature(vcp_device, feature_id);
+ if (ret < 0) {
+ dev_err(&fw->pdev->dev, "%d register to vcp fail(%d)\n", feature_id, ret);
+ return -EINVAL;
+ }
+
+ fw->vcp->is_register_done = true;
+ }
+
+ fw->vcp->is_init_done = true;
+
+ mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
+ mutex_init(&fw->vcp->ipi_mutex);
+
+ kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_vdec_msq_thread");
+
+ fw->vcp->vsi_addr = vcp_device->ops->vcp_get_mem_virt(mem_id);
+ fw->vcp->vsi_core_addr = fw->vcp->vsi_addr + VCODEC_VSI_LEN;
+ fw->vcp->vsi_size = vcp_device->ops->vcp_get_mem_size(mem_id);
+ fw->vcp->iova_addr = vcp_device->ops->vcp_get_mem_iova(mem_id);
+
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_LAT_DECODER]);
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_CORE_DECODER]);
+
+ dev_dbg(&fw->pdev->dev, "vdec vcp init done => va: %p size:0x%x iova:%p.\n",
+ fw->vcp->vsi_addr, fw->vcp->vsi_size, &fw->vcp->iova_addr);
+
+ return 0;
+}
+
+static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct mtk_vcodec_fw *fw)
+{
+ return MTK_VDEC_FORMAT_MM21 | MTK_VDEC_FORMAT_H264_SLICE | MTK_VDEC_FORMAT_VP9_FRAME |
+ MTK_VDEC_FORMAT_AV1_FRAME | MTK_VDEC_FORMAT_HEVC_FRAME |
+ MTK_VDEC_IS_SUPPORT_10BIT | MTK_VDEC_IS_SUPPORT_EXT;
+}
+
+static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 dtcm_dmem_addr)
+{
+ return NULL;
+}
+
+static int mtk_vcodec_vcp_set_ipi_register(struct mtk_vcodec_fw *fw, int id,
+ mtk_vcodec_ipi_handler handler,
+ const char *name, void *priv)
+{
+ return mtk_vcodec_vcp_ipi_register(fw->vcp, id, handler, priv);
+}
+
+static int mtk_vcodec_vcp_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
+ unsigned int len, unsigned int wait)
+{
+ return mtk_vcodec_vcp_msg_ipi_send(fw, id, buf, len, wait);
+}
+
+static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
+{
+ struct mtk_vcp_device *vcp_device = fw->vcp->vcp_device;
+ struct device *dev = &fw->pdev->dev;
+ int ret, feature_id;
+
+ if (!fw->vcp->vcp_device) {
+ dev_err(dev, "vcp device is null\n");
+ return;
+ }
+
+ if (!fw->vcp->is_register_done)
+ return;
+
+ feature_id = VDEC_FEATURE_ID;
+ ret = vcp_device->ops->vcp_deregister_feature(vcp_device, VDEC_FEATURE_ID);
+ if (ret < 0) {
+ dev_err(dev, "deregister feature_id(%d) fail(%d)\n", feature_id, ret);
+ return;
+ }
+
+ fw->vcp->is_register_done = false;
+}
+
+static const struct mtk_vcodec_fw_ops mtk_vcodec_vcp_msg = {
+ .load_firmware = mtk_vcodec_vcp_load_firmware,
+ .get_vdec_capa = mtk_vcodec_vcp_get_vdec_capa,
+ .map_dm_addr = mtk_vcodec_vcp_dm_addr,
+ .ipi_register = mtk_vcodec_vcp_set_ipi_register,
+ .ipi_send = mtk_vcodec_vcp_ipi_send,
+ .release = mtk_vcodec_vcp_release,
+};
+
+struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
+{
+ struct mtk_vcp_msg_node *msg_node;
+ struct platform_device *plat_dev;
+ struct mtk_vcodec_fw *fw;
+ int i;
+
+ if (fw_use == DECODER) {
+ struct mtk_vcodec_dec_dev *dec_dev = priv;
+
+ plat_dev = dec_dev->plat_dev;
+ } else {
+ pr_err("Invalid fw_use %d (use a reasonable fw id here)\n", fw_use);
+ return ERR_PTR(-EINVAL);
+ }
+
+ fw = devm_kzalloc(&plat_dev->dev, sizeof(*fw), GFP_KERNEL);
+ if (!fw)
+ return ERR_PTR(-ENOMEM);
+
+ fw->type = VCP;
+ fw->pdev = plat_dev;
+ fw->fw_use = fw_use;
+ fw->ops = &mtk_vcodec_vcp_msg;
+ fw->vcp = devm_kzalloc(&plat_dev->dev, sizeof(*fw->vcp), GFP_KERNEL);
+ if (!fw->vcp)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&fw->vcp->msg_queue.msg_list);
+ INIT_LIST_HEAD(&fw->vcp->msg_queue.node_list);
+ spin_lock_init(&fw->vcp->msg_queue.lock);
+ init_waitqueue_head(&fw->vcp->msg_queue.wq);
+ atomic_set(&fw->vcp->msg_queue.cnt, 0);
+ fw->vcp->pdev = plat_dev;
+
+ for (i = 0; i < VCP_MAX_MQ_NODE_CNT; i++) {
+ msg_node = devm_kzalloc(&plat_dev->dev, sizeof(*msg_node), GFP_KERNEL);
+ if (!msg_node)
+ return ERR_PTR(-ENOMEM);
+
+ list_add(&msg_node->list, &fw->vcp->msg_queue.node_list);
+ }
+
+ return fw;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_fw_vcp_init);
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
new file mode 100644
index 000000000000..1072c37078f4
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
@@ -0,0 +1,142 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ * Author: Yunfei Dong <yunfei.dong@mediatek.com>
+ */
+
+#ifndef _MTK_VCODEC_FW_VCP_H_
+#define _MTK_VCODEC_FW_VCP_H_
+
+typedef void (*vcp_ipi_handler_t) (void *data, unsigned int len, void *priv);
+
+#define VCP_MAX_MQ_NODE_CNT 6
+#define VCP_SHARE_BUF_SIZE 64
+
+#define VCODEC_VSI_LEN (0x2000)
+
+/* enum mtk_vcp_ipi_index - index used to separate different hardware */
+enum mtk_vcp_ipi_index {
+ VCP_IPI_LAT_DECODER,
+ VCP_IPI_CORE_DECODER,
+ VCP_IPI_MAX,
+};
+
+/**
+ * struct mtk_vcp_msg_queue - process the vcp message between kernel with vcp
+ *
+ * @msg_list: store share buffer list which from vcp to kernel
+ * @wq: waitqueue that can be used to wait for vcp message
+ * @lock: protect msg list
+ * @cnt: the count of share obj in msg list
+ * @node_list: share obj list
+ */
+struct mtk_vcp_msg_queue {
+ struct list_head msg_list;
+ wait_queue_head_t wq;
+ /* protect msg_list and node_list */
+ spinlock_t lock;
+ atomic_t cnt;
+ struct list_head node_list;
+};
+
+/**
+ * struct mtk_vcp_ipi_desc - store the ack handler
+ *
+ * @lock: protect ack handler data
+ * @handler: calling this handler when kernel receive ack
+ * @priv: private data when calling handler to process
+ */
+struct mtk_vcp_ipi_desc {
+ /* protect handler and priv */
+ struct mutex lock;
+ vcp_ipi_handler_t handler;
+ void *priv;
+};
+
+/**
+ * struct mtk_vcp_share_obj - share buffer used to send data to vcp
+ *
+ * @id: message index
+ * @len: message size
+ * @share_buf: message data
+ */
+struct mtk_vcp_share_obj {
+ unsigned int id;
+ unsigned int len;
+ unsigned char share_buf[VCP_SHARE_BUF_SIZE];
+};
+
+/* enum mtk_vcp_ipi_msg_status - the status when send message to vcp */
+enum mtk_vcp_ipi_msg_status {
+ VCODEC_IPI_MSG_STATUS_OK = 0,
+ VCODEC_IPI_MSG_STATUS_FAIL = -1,
+ VCODEC_IPI_MSG_STATUS_MAX_INST = -2,
+ VCODEC_IPI_MSG_STATUS_ILSEQ = -3,
+ VCODEC_IPI_MSG_STATUS_INVALID_ID = -4,
+ VCODEC_IPI_MSG_STATUS_DMA_FAIL = -5,
+};
+
+/**
+ * struct mtk_vcp_msg_node - share buffer used to send data to vcp
+ *
+ * @ipi_data: share obj data
+ * @list: list to store msg node
+ */
+struct mtk_vcp_msg_node {
+ struct mtk_vcp_share_obj ipi_data;
+ struct list_head list;
+};
+
+/**
+ * struct mtk_vcp - vcp firmware private data
+ *
+ * @is_init_done: vcp is ready to use
+ *
+ * @ipi_mutex: used to protect ipi data
+ * @msg_signaled: whether receive ack from vcp
+ * @msg_wq: wake message queue
+ *
+ * @ipi_desc: store ack handler
+ * @ipi_id_ack: the ack handler status
+ *
+ * @msg_queue: process vcp message
+ * @share_data: temp share obj data
+ *
+ * @vcp_notify: register notifier to vcp
+ *
+ * @vsi_addr: vsi virtual data address
+ * @vsi_core_addr: vsi core virtual data address
+ * @iova_addr: vsi iova address
+ * @vsi_size: vsi size
+ *
+ * @pdev: platform device
+ * @vcp_device: vcp private data
+ * @is_register_done: register vcodec to vcp
+ */
+struct mtk_vcp {
+ bool is_init_done;
+
+ /* serialize ipi message send/receive */
+ struct mutex ipi_mutex;
+ bool msg_signaled[VCP_IPI_MAX];
+ wait_queue_head_t msg_wq[VCP_IPI_MAX];
+
+ struct mtk_vcp_ipi_desc ipi_desc[VCP_IPI_MAX];
+ bool ipi_id_ack[VCP_IPI_MAX];
+
+ struct mtk_vcp_msg_queue msg_queue;
+ struct mtk_vcp_share_obj share_data;
+
+ struct notifier_block vcp_notify;
+
+ void *vsi_addr;
+ void *vsi_core_addr;
+ dma_addr_t iova_addr;
+ int vsi_size;
+
+ struct platform_device *pdev;
+ struct mtk_vcp_device *vcp_device;
+ bool is_register_done;
+};
+
+#endif
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
index 3632037f78f5..41643db9f4eb 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
@@ -130,3 +130,4 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_vpu_init(void *priv, enum mtk_vcodec_fw_use
return fw;
}
+EXPORT_SYMBOL_GPL(mtk_vcodec_fw_vpu_init);
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index d220b645e455..ce17b649022f 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -376,12 +376,15 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
&rproc_phandle)) {
fw_type = VPU;
+ dev->fw_init = mtk_vcodec_fw_vpu_init;
} else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
&rproc_phandle)) {
fw_type = SCP;
+ dev->fw_init = mtk_vcodec_fw_scp_init;
} else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vcp",
&rproc_phandle)) {
fw_type = VCP;
+ dev->fw_init = mtk_vcodec_fw_vcp_init;
} else {
dev_dbg(&pdev->dev, "Could not get vdec IPI device");
return -ENODEV;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index c9d27534c63e..15e437323af5 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -251,6 +251,7 @@ struct mtk_vcodec_dec_ctx {
* @vdecsys_regmap: VDEC_SYS register space passed through syscon
*
* @fw_handler: used to communicate with the firmware.
+ * @fw_init: firmware-specific init callback selected at probe time
* @id_counter: used to identify current opened instance
*
* @dec_mutex: decoder hardware lock
@@ -292,6 +293,7 @@ struct mtk_vcodec_dec_dev {
struct regmap *vdecsys_regmap;
struct mtk_vcodec_fw *fw_handler;
+ struct mtk_vcodec_fw *(*fw_init)(void *priv, enum mtk_vcodec_fw_use fw_use);
u64 id_counter;
/* decoder hardware mutex lock */
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
index 4e4541b2fc8e..dc54d445d98d 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
@@ -259,9 +259,11 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
&rproc_phandle)) {
fw_type = VPU;
+ dev->fw_init = mtk_vcodec_fw_vpu_init;
} else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
&rproc_phandle)) {
fw_type = SCP;
+ dev->fw_init = mtk_vcodec_fw_scp_init;
} else {
dev_err(&pdev->dev, "[MTK VCODEC] Could not get venc IPI device");
return -ENODEV;
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index 0cddfa13594f..934ff648125d 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -174,6 +174,7 @@ struct mtk_vcodec_enc_ctx {
* @venc_pdata: encoder IC-specific data
*
* @fw_handler: used to communicate with the firmware.
+ * @fw_init: firmware-specific init callback selected at probe time
* @id_counter: used to identify current opened instance
*
* @enc_mutex: encoder hardware lock.
@@ -201,6 +202,7 @@ struct mtk_vcodec_enc_dev {
const struct mtk_vcodec_enc_pdata *venc_pdata;
struct mtk_vcodec_fw *fw_handler;
+ struct mtk_vcodec_fw *(*fw_init)(void *priv, enum mtk_vcodec_fw_use fw_use);
u64 id_counter;
/* encoder hardware mutex lock */
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp
2026-06-03 8:40 ` [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
@ 2026-06-03 8:58 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:58 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, linux-media, robh, devicetree
Thank you for your contribution! Sashiko AI review found 15 potential issue(s) to consider:
- [Critical] The untrusted firmware-provided message ID is used directly as an array index without bounds checking, enabling arbitrary out-of-bounds memory accesses.
- [High] The Kconfig symbol MTK_VCP_RPROC used in a select statement does not exist in the kernel tree.
- [High] The driver fails to stop the message processing kthread, unregister the IPC mailbox, and unregister the notifier during module removal, leading to a Use-After-Free and kernel panic.
- [High] Inverted logic in `check_vcp_loaded()` causes a 15-second busy-loop and kernel log spam when the VCP driver is not loaded.
- [High] The kernel message processing thread permanently exits upon encountering an unregistered message ID, causing a Denial of Service for all subsequent IPC.
- [High] The per-IPI mutex for `VCP_IPI_CORE_DECODER` is never initialized, leading to a kernel panic when locked.
- [High] Negative error codes are assigned to the boolean array `ipi_id_ack`, which truncates them to `true` (1) and silently masks IPC failures.
- [High] Failing to unwind previously registered callbacks on the error path in `mtk_vcodec_vcp_load_firmware` leaves dangling references.
- [High] A stale ACK from a timed-out IPC message can cause subsequent IPC messages to prematurely succeed without waiting.
- [Medium] Failure to create the IPC processing kthread is silently ignored.
- [Medium] The driver bus-spins the CPU with `mdelay(1)` while holding a mutex, causing significant latency spikes.
- [Medium] The message processing kthread uses `wait_event_interruptible()` without including `kthread_should_stop()` in the wake condition, guaranteeing a deadlock if `kthread_stop()` is called.
- [Medium] The VCP device handle is retrieved but never released, leaking the reference.
- [Low] The code uses undefined macros and headers, guaranteeing compilation failure.
- [Low] Dead code in `mtk_vcodec_vcp_msg_ipi_send()` checks for impossible negative return values from `wait_event_timeout()`.
--
commit a24833f019af1320e17d5afbc734268e762b3e20
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: add driver to support vcp
The processor is changed from scp to vcp in mt8196 platform.
Adding new firmware interface to communicate kernel with vcp
for the communication method is changed.
> diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig b/drivers/media/platform/mediatek/vcodec/Kconfig
> index bc8292232530b..d23dad5c78ce3 100644
> --- a/drivers/media/platform/mediatek/vcodec/Kconfig
> +++ b/drivers/media/platform/mediatek/vcodec/Kconfig
> @@ -21,6 +24,7 @@ config VIDEO_MEDIATEK_VCODEC
> select V4L2_MEM2MEM_DEV
> select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
> select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP
> + select VIDEO_MEDIATEK_VCODEC_VCP if MTK_VCP_RPROC
[Severity: High]
Does this select statement reference a missing config symbol? According to the
Kconfig subsystem guidelines, referencing a non-existent config symbol in a
select statement leaves the intended target unselectable or causes silent
failures. A search across the kernel tree shows that MTK_VCP_RPROC is never
defined.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> new file mode 100644
> index 0000000000000..2f3bfcd97958a
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> @@ -0,0 +1,505 @@
[ ... ]
> +#include <linux/remoteproc/mtk_vcp_public.h>
> +#include <linux/firmware/mediatek/mtk-vcp-ipc.h>
[Severity: Low]
Does the mtk_vcp_public.h header exist in the kernel tree? Relying on
undeclared headers will lead to direct build errors.
[ ... ]
> +static int mtk_vcodec_vcp_msg_process_thread(void *arg)
> +{
[ ... ]
> + do {
> + ret = wait_event_interruptible(fw->vcp->msg_queue.wq,
> + atomic_read(&fw->vcp->msg_queue.cnt) > 0);
[Severity: Medium]
Will this wait condition prevent the kthread from properly stopping if
kthread_stop() is called during driver unload? Since kthread_should_stop()
is not included in the wait condition, waking the thread for teardown could
cause it to immediately go back to sleep.
[ ... ]
> + obj = &msg_node->ipi_data;
> + msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
[ ... ]
> + mtk_vcodec_vcp_ipi_lock(fw->vcp, obj->id);
> + handler = fw->vcp->ipi_desc[obj->id].handler;
[Severity: Critical]
Can this code cause an out-of-bounds memory access? The untrusted
firmware-provided obj->id is used directly as an array index without bounds
checking. A compromised or buggy firmware returning an ID >= VCP_IPI_MAX
could cause the kernel to access arbitrary memory.
> + if (!handler) {
> + dev_err(&fw->pdev->dev, "invalid ack ipi handler id = %d\n", obj->id);
> + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
> + mtk_vcodec_vcp_free_msg_node(fw, msg_node);
> + return -EINVAL;
> + }
[Severity: High]
Should this return -EINVAL instead be a continue? Returning an error here
aborts the while loop and permanently terminates the kthread context. All
subsequent VCP IPC messages will time out, completely breaking the decoder
until the driver is reloaded.
[ ... ]
> +static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
> + unsigned int len, unsigned int wait)
> +{
> + struct mtk_vcp *vcp = fw->vcp;
[ ... ]
> + mutex_lock(&vcp->ipi_mutex);
> + feature_id = VDEC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VDEC_1;
> +
> + timeout_jiffies = jiffies + msecs_to_jiffies(VCP_SYNC_TIMEOUT_MS);
[Severity: Low]
Is VCP_SYNC_TIMEOUT_MS defined anywhere in the kernel tree? Relying on an
undeclared macro will cause compilation failures.
> + while (!vcp_device->ops->vcp_is_ready(feature_id)) {
> + if (time_after(jiffies, timeout_jiffies)) {
> + vcp->ipi_id_ack[id] = -EINVAL;
[Severity: High]
Does assigning a negative error code to ipi_id_ack mask IPC failures?
Because ipi_id_ack is defined as a boolean array in struct mtk_vcp,
negative error codes like -EINVAL will truncate to true (1). Since the
function returns this value at the end, callers expecting standard negative
error codes might incorrectly assume the IPC succeeded.
> + ret = -EINVAL;
> + goto error;
> + }
> + mdelay(1);
> + }
[Severity: Medium]
Is it safe to use mdelay(1) in a loop while holding a mutex? Because the
code holds vcp->ipi_mutex, it is in a sleepable context. Using mdelay() here
bus-spins the CPU for up to the timeout duration, wasting CPU cycles and
causing significant latency spikes.
[ ... ]
> +wait_ack:
> + /* wait for VCP's ACK */
> + ret = wait_event_timeout(*msg_wq, *msg_signaled, msecs_to_jiffies(IPI_TIMEOUT_MS));
> + if (!ret || retry_cnt > 5) {
> + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> + dev_err(&fw->pdev->dev, "wait ipi ack timeout! %d %d\n", ret, vcp->ipi_id_ack[id]);
> + } else if (ret == -ERESTARTSYS) {
> + dev_err(&fw->pdev->dev, "wait ipi ack err (%d)\n", vcp->ipi_id_ack[id]);
> + retry_cnt++;
> + goto wait_ack;
> + } else if (ret < 0) {
> + dev_err(&fw->pdev->dev, "wait ipi ack fail ret %d %d\n", ret, vcp->ipi_id_ack[id]);
> + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> + }
[Severity: Low]
Are the checks for -ERESTARTSYS and negative return values dead code?
wait_event_timeout() strictly returns a positive value (remaining jiffies)
or 0 (timeout). It never returns negative error codes.
[ ... ]
> + *msg_signaled = false;
> + mutex_unlock(&vcp->ipi_mutex);
[Severity: High]
Should msg_signaled be cleared before sending the IPC message instead of
after? If a previous IPC request times out, and the firmware delivers a late
ACK, the ISR sets the flag to true. The next call to send a message would
then see the flag as already true and return immediately without waiting,
potentially reading stale data.
[ ... ]
> +static int check_vcp_loaded(struct mtk_vcodec_fw *fw)
> +{
> + struct device *dev = &fw->pdev->dev;
> + struct device_driver *drv;
> +
> + drv = driver_find("mtk-vcp", &platform_bus_type);
> + if (!drv) {
> + dev_err(dev, "find mtk-vcp driver failed, need to reload.");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_vcodec_vcp_get_vcp_device(struct mtk_vcodec_fw *fw)
> +{
> + struct device *dev = &fw->pdev->dev;
> + int retry = 0, retry_cnt = 10000;
> + phandle vcp_phandle;
> +
> + while (try_then_request_module(check_vcp_loaded(fw), "mtk-vcp")) {
> + if (++retry > retry_cnt) {
> + dev_err(dev, "failed to load mtk-vcp module");
> + return -EPROBE_DEFER;
> + }
> + usleep_range(1000, 2000);
> + }
[Severity: High]
Does this loop inadvertently busy-wait based on an inverted condition?
check_vcp_loaded() returns -EINVAL when the driver is not found.
try_then_request_module() evaluates non-zero values as true. Consequently,
if the driver is missing, this evaluates as true, skipping the module request
and keeping the while loop running for up to 10,000 iterations while flooding
the kernel log.
[ ... ]
> + fw->vcp->vcp_device = mtk_vcp_get_by_phandle(vcp_phandle);
> + if (!fw->vcp->vcp_device) {
> + dev_err(dev, "get vcp device failed\n");
> + return -ENODEV;
> + }
[Severity: Medium]
Is the reference acquired by mtk_vcp_get_by_phandle() ever released?
APIs retrieving a device by phandle increment the reference count, but there
is no corresponding put operation in the teardown or release code, which
permanently pins the remote processor driver in memory.
[ ... ]
> +static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> +{
[ ... ]
> + ret = mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
> + mtk_vcodec_vcp_msg_ack_isr, fw, &fw->vcp->share_data);
[ ... ]
> + fw->vcp->vcp_notify.notifier_call = mtk_vcodec_vcp_notifier;
> + fw->vcp->vcp_notify.priority = 1;
> + vcp_device->ops->vcp_register_notify(feature_id, &fw->vcp->vcp_notify);
> +
> + if (!fw->vcp->is_register_done) {
> + ret = vcp_device->ops->vcp_register_feature(vcp_device, feature_id);
> + if (ret < 0) {
> + dev_err(&fw->pdev->dev, "%d register to vcp fail(%d)\n", feature_id, ret);
> + return -EINVAL;
> + }
[Severity: High]
Should the error path deregister the IPC mailbox and notifier?
Returning directly leaves dangling pointers in the remote processor subsystem
pointing to the driver's memory, which could cause a use-after-free.
> + fw->vcp->is_register_done = true;
> + }
> +
> + fw->vcp->is_init_done = true;
> +
> + mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
> + mutex_init(&fw->vcp->ipi_mutex);
[Severity: High]
Is the per-IPI mutex for VCP_IPI_CORE_DECODER initialized anywhere?
Here, ipi_id is hardcoded to VCP_IPI_LAT_DECODER (0). If the core decoder
attempts to register or process an IPI message, it will lock an uninitialized
mutex.
> + kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_vdec_msq_thread");
[Severity: Medium]
Should the return value of kthread_run() be verified? If thread creation
fails, the driver will complete initialization successfully but permanently
hang on the first IPC communication because no thread exists to process ACKs.
[ ... ]
> +static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> +{
> + struct mtk_vcp_device *vcp_device = fw->vcp->vcp_device;
> + struct device *dev = &fw->pdev->dev;
> + int ret, feature_id;
> +
> + if (!fw->vcp->vcp_device) {
> + dev_err(dev, "vcp device is null\n");
> + return;
> + }
> +
> + if (!fw->vcp->is_register_done)
> + return;
> +
> + feature_id = VDEC_FEATURE_ID;
> + ret = vcp_device->ops->vcp_deregister_feature(vcp_device, VDEC_FEATURE_ID);
> + if (ret < 0) {
> + dev_err(dev, "deregister feature_id(%d) fail(%d)\n", feature_id, ret);
> + return;
> + }
> +
> + fw->vcp->is_register_done = false;
> +}
[Severity: High]
Does this release function adequately stop the kthread, unregister the IPC
mailbox, and unregister the notifier? When the driver is unbound and the
devm_kzalloc managed memory is freed, the still-running kthread and remote
callbacks will access freed memory, causing a kernel panic.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> new file mode 100644
> index 0000000000000..1072c37078f42
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
[ ... ]
> +struct mtk_vcp {
[ ... ]
> + struct mtk_vcp_ipi_desc ipi_desc[VCP_IPI_MAX];
> + bool ipi_id_ack[VCP_IPI_MAX];
[Severity: High]
Should ipi_id_ack be an integer type instead of a bool?
The code assigns negative error constants to this array upon failure.
Those non-zero values truncate to true (1), masking any IPC errors and
preventing callers from receiving accurate error codes.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 03/14] media: mediatek: vcodec: add driver to support vcp encoder
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 11:01 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 04/14] media: mediatek: vcodec: get different firmware ipi id Kyrie Wu
` (10 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
Encoder also need to call vcp interface to communicate with vcp,
add driver to support encoder.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../vcodec/common/mtk_vcodec_fw_vcp.c | 50 +++++++++++++++----
.../vcodec/common/mtk_vcodec_fw_vcp.h | 1 +
.../mediatek/vcodec/encoder/mtk_vcodec_enc.c | 1 -
.../mediatek/vcodec/encoder/mtk_vcodec_enc.h | 2 +
4 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
index 2f3bfcd97958..ee3838daf5a7 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
@@ -13,6 +13,8 @@
#include "../decoder/mtk_vcodec_dec_drv.h"
#include "../decoder/vdec_ipi_msg.h"
+#include "../encoder/mtk_vcodec_enc.h"
+#include "../encoder/mtk_vcodec_enc_drv.h"
#include "mtk_vcodec_fw_priv.h"
#define IPI_SEND_TIMEOUT_MS 100U
@@ -226,8 +228,13 @@ static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *b
}
mutex_lock(&vcp->ipi_mutex);
- feature_id = VDEC_FEATURE_ID;
- mailbox_id = IPI_OUT_VDEC_1;
+ if (fw->fw_use == ENCODER) {
+ feature_id = VENC_FEATURE_ID;
+ mailbox_id = IPI_OUT_VENC_0;
+ } else {
+ feature_id = VDEC_FEATURE_ID;
+ mailbox_id = IPI_OUT_VDEC_1;
+ }
timeout_jiffies = jiffies + msecs_to_jiffies(VCP_SYNC_TIMEOUT_MS);
while (!vcp_device->ops->vcp_is_ready(feature_id)) {
@@ -351,10 +358,17 @@ static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
vcp_device = fw->vcp->vcp_device;
- feature_id = VDEC_FEATURE_ID;
- mem_id = VDEC_MEM_ID;
- mailbox_id = IPI_IN_VDEC_1;
- ipi_id = VCP_IPI_LAT_DECODER;
+ if (fw->fw_use == ENCODER) {
+ feature_id = VENC_FEATURE_ID;
+ mem_id = VENC_MEM_ID;
+ mailbox_id = IPI_IN_VENC_0;
+ ipi_id = VCP_IPI_ENCODER;
+ } else {
+ feature_id = VDEC_FEATURE_ID;
+ mem_id = VDEC_MEM_ID;
+ mailbox_id = IPI_IN_VDEC_1;
+ ipi_id = VCP_IPI_LAT_DECODER;
+ }
ret = mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
mtk_vcodec_vcp_msg_ack_isr, fw, &fw->vcp->share_data);
@@ -383,6 +397,20 @@ static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
mutex_init(&fw->vcp->ipi_mutex);
+ if (fw->fw_use == ENCODER) {
+ kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_enc_msq_thread");
+
+ fw->vcp->vsi_addr = vcp_device->ops->vcp_get_mem_virt(mem_id);
+ fw->vcp->vsi_size = vcp_device->ops->vcp_get_mem_size(mem_id);
+ fw->vcp->iova_addr = vcp_device->ops->vcp_get_mem_iova(mem_id);
+
+ dev_dbg(&fw->pdev->dev, "enc vcp init done => va: %p size:0x%x iova:%pad.\n",
+ fw->vcp->vsi_addr, fw->vcp->vsi_size, &fw->vcp->iova_addr);
+
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_ENCODER]);
+ return 0;
+ }
+
kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_vdec_msq_thread");
fw->vcp->vsi_addr = vcp_device->ops->vcp_get_mem_virt(mem_id);
@@ -438,8 +466,8 @@ static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
if (!fw->vcp->is_register_done)
return;
- feature_id = VDEC_FEATURE_ID;
- ret = vcp_device->ops->vcp_deregister_feature(vcp_device, VDEC_FEATURE_ID);
+ feature_id = fw->fw_use == ENCODER ? VENC_FEATURE_ID : VDEC_FEATURE_ID;
+ ret = vcp_device->ops->vcp_deregister_feature(vcp_device, feature_id);
if (ret < 0) {
dev_err(dev, "deregister feature_id(%d) fail(%d)\n", feature_id, ret);
return;
@@ -464,7 +492,11 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use
struct mtk_vcodec_fw *fw;
int i;
- if (fw_use == DECODER) {
+ if (fw_use == ENCODER) {
+ struct mtk_vcodec_enc_dev *enc_dev = priv;
+
+ plat_dev = enc_dev->plat_dev;
+ } else if (fw_use == DECODER) {
struct mtk_vcodec_dec_dev *dec_dev = priv;
plat_dev = dec_dev->plat_dev;
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
index 1072c37078f4..acf8f3e87bd0 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
@@ -16,6 +16,7 @@ typedef void (*vcp_ipi_handler_t) (void *data, unsigned int len, void *priv);
/* enum mtk_vcp_ipi_index - index used to separate different hardware */
enum mtk_vcp_ipi_index {
+ VCP_IPI_ENCODER,
VCP_IPI_LAT_DECODER,
VCP_IPI_CORE_DECODER,
VCP_IPI_MAX,
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
index 0d4e94463685..48cb5dded70a 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
@@ -26,7 +26,6 @@
#define MTK_DEFAULT_FRAMERATE_NUM 1001
#define MTK_DEFAULT_FRAMERATE_DENOM 30000
-#define MTK_VENC_4K_CAPABILITY_ENABLE BIT(0)
static void mtk_venc_worker(struct work_struct *work);
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.h
index 908d8179b2d2..84156c102d8d 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.h
@@ -23,6 +23,8 @@
#define MTK_VENC_IRQ_STATUS_OFFSET 0x05C
#define MTK_VENC_IRQ_ACK_OFFSET 0x060
+#define MTK_VENC_4K_CAPABILITY_ENABLE BIT(0)
+
/**
* struct mtk_video_enc_buf - Private data related to each VB2 buffer.
* @m2m_buf: M2M buffer
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 03/14] media: mediatek: vcodec: add driver to support vcp encoder
2026-06-03 8:40 ` [PATCH v5 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
@ 2026-06-03 11:01 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 11:01 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, linux-media, devicetree, robh
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Untrusted coprocessor input `obj->id` is used directly as an array index without bounds checking, causing out-of-bounds memory access and potential arbitrary code execution.
- [High] The kthread_run() task pointer is discarded, causing a memory leak and a fatal use-after-free when the device is unbound.
- [High] The msg_signaled flag is not reset before waiting, allowing a stale, delayed ACK to trigger a false positive for a subsequent IPC message.
- [High] The driver registers an IPC mailbox and a notifier but fails to unregister them during device teardown, leading to a Use-After-Free.
- [High] The message processing thread exits permanently upon receiving an unhandled message ID, hanging the entire driver.
- [High] The driver fails to initialize the `ipi_desc` mutex for the CORE decoder, leading to lockdep warnings and potential crashes.
--
commit 7793240a286886003391171d60e17e6a2e3e89f8
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: add driver to support vcp encoder
Encoder also need to call vcp interface to communicate with vcp,
add driver to support encoder.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> @@ -226,8 +228,13 @@ static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *b
> }
>
> mutex_lock(&vcp->ipi_mutex);
> - feature_id = VDEC_FEATURE_ID;
> - mailbox_id = IPI_OUT_VDEC_1;
> + if (fw->fw_use == ENCODER) {
> + feature_id = VENC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VENC_0;
> + } else {
> + feature_id = VDEC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VDEC_1;
> + }
[Severity: High]
This is a pre-existing issue, but does mtk_vcodec_vcp_msg_ipi_send()
properly handle stale delayed ACKs?
If wait_event_timeout() times out later in this function, it appears it sets
the msg_signaled flag to false and returns. However, if a delayed ACK arrives
afterward, the worker thread will set the flag to true.
Since mtk_vcodec_vcp_msg_ipi_send() does not re-initialize the msg_signaled
flag to false before sending the next IPC message, could a subsequent wait
immediately return success without actually waiting for the new message to
be acknowledged?
[ ... ]
> @@ -383,6 +397,20 @@ static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
> mutex_init(&fw->vcp->ipi_mutex);
[Severity: High]
This isn't a bug introduced by this patch, but are all the ipi_desc mutexes
properly initialized?
Here, mutex_init() is only called for a single ipi_id. If this path is taken
for the decoder, it initializes the lock for VCP_IPI_LAT_DECODER, but it omits
VCP_IPI_CORE_DECODER.
When the decoder client later registers its CORE handler,
mtk_vcodec_vcp_ipi_register() will attempt to acquire the lock. Could locking
the uninitialized CORE mutex lead to lockdep splats or undefined behavior?
>
> + if (fw->fw_use == ENCODER) {
> + kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_enc_msq_thread");
> +
[Severity: High]
This is a pre-existing issue, but is it safe to discard the task pointer
returned by kthread_run()?
If the pointer is not saved, kthread_stop() cannot be called in
mtk_vcodec_vcp_release() during device teardown. Because the fw structure
is allocated with devm_kzalloc, the memory will be automatically freed when
the driver unbinds.
Could this cause a use-after-free when the unstopped thread wakes up
and accesses the freed fw->vcp structures?
[Severity: Critical]
This is a pre-existing issue, but does mtk_vcodec_vcp_msg_process_thread()
validate the obj->id from the coprocessor?
Looking at the thread function, it appears the ID from the IPC payload is
used directly as an array index:
handler = fw->vcp->ipi_desc[obj->id].handler;
If a compromised or malicious coprocessor sends an out-of-bounds ID, could
this read out-of-bounds function pointers and execute them when calling
handler(), leading to arbitrary kernel code execution?
[Severity: High]
This isn't a bug introduced by this patch, but how does
mtk_vcodec_vcp_msg_process_thread() handle invalid message IDs?
If the handler is null for a message, the function executes return -EINVAL;
instead of using continue; to process the next message.
Does this permanently terminate the kthread loop? If the thread exits,
wouldn't all subsequent valid IPC messages be queued but never processed,
permanently hanging active decode or encode instances?
[ ... ]
> @@ -438,8 +466,8 @@ static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> if (!fw->vcp->is_register_done)
> return;
>
> - feature_id = VDEC_FEATURE_ID;
> - ret = vcp_device->ops->vcp_deregister_feature(vcp_device, VDEC_FEATURE_ID);
> + feature_id = fw->fw_use == ENCODER ? VENC_FEATURE_ID : VDEC_FEATURE_ID;
> + ret = vcp_device->ops->vcp_deregister_feature(vcp_device, feature_id);
> if (ret < 0) {
> dev_err(dev, "deregister feature_id(%d) fail(%d)\n", feature_id, ret);
> return;
[Severity: High]
This is a pre-existing issue, but does this release function completely
tear down the IPC mailbox and notifier?
The driver registers an IPC mailbox via mtk_vcp_mbox_ipc_register() and a
notifier via vcp_register_notify() in mtk_vcodec_vcp_load_firmware().
However, these do not appear to be unregistered here during teardown.
Because fw->vcp is managed by devm_kzalloc, could this leave dangling
callbacks in the IPC or notifier frameworks that might trigger a
use-after-free if an event arrives after the device is unbound?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=3
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 04/14] media: mediatek: vcodec: get different firmware ipi id
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (2 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
` (9 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
Getting ipi(inter-processor interrupt) id according to firmware
type and hardware index for different architecture.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../platform/mediatek/vcodec/common/mtk_vcodec_fw.c | 13 +++++++++++++
.../platform/mediatek/vcodec/common/mtk_vcodec_fw.h | 1 +
.../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 5 +++--
.../vcodec/decoder/vdec/vdec_h264_req_multi_if.c | 5 +++--
.../vcodec/decoder/vdec/vdec_hevc_req_multi_if.c | 5 +++--
.../mediatek/vcodec/decoder/vdec/vdec_vp8_req_if.c | 5 +++--
.../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 5 +++--
7 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
index 64977bcd6abd..a2e6a01272b2 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
@@ -5,6 +5,19 @@
#include "mtk_vcodec_fw_priv.h"
#include "mtk_vcodec_fw_vcp.h"
+int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id)
+{
+ switch (type) {
+ case SCP:
+ return hw_id == MTK_VDEC_LAT0 ? SCP_IPI_VDEC_LAT : SCP_IPI_VDEC_CORE;
+ case VCP:
+ return hw_id == MTK_VDEC_LAT0 ? VCP_IPI_LAT_DECODER : VCP_IPI_CORE_DECODER;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_fw_get_ipi);
+
struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type type,
enum mtk_vcodec_fw_use fw_use)
{
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
index c1642fb09b42..142e2e87905c 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
@@ -41,5 +41,6 @@ int mtk_vcodec_fw_ipi_register(struct mtk_vcodec_fw *fw, int id,
int mtk_vcodec_fw_ipi_send(struct mtk_vcodec_fw *fw, int id,
void *buf, unsigned int len, unsigned int wait);
int mtk_vcodec_fw_get_type(struct mtk_vcodec_fw *fw);
+int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id);
#endif /* _MTK_VCODEC_FW_H_ */
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
index 2d622e85f827..756fbb7778b1 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
@@ -1877,6 +1877,7 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
struct vdec_av1_slice_instance *instance;
struct vdec_av1_slice_init_vsi *vsi;
+ enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
int ret;
instance = kzalloc_obj(*instance);
@@ -1884,8 +1885,8 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
return -ENOMEM;
instance->ctx = ctx;
- instance->vpu.id = SCP_IPI_VDEC_LAT;
- instance->vpu.core_id = SCP_IPI_VDEC_CORE;
+ instance->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
+ instance->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
instance->vpu.ctx = ctx;
instance->vpu.codec_type = ctx->current_codec;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index 10359ce9b934..69d60717181a 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -1204,6 +1204,7 @@ static int vdec_h264_slice_single_decode(void *h_vdec, struct mtk_vcodec_mem *bs
static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
+ enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
struct vdec_h264_slice_inst *inst;
int err, vsi_size;
unsigned char *temp;
@@ -1214,8 +1215,8 @@ static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
inst->ctx = ctx;
- inst->vpu.id = SCP_IPI_VDEC_LAT;
- inst->vpu.core_id = SCP_IPI_VDEC_CORE;
+ inst->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
+ inst->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
inst->vpu.ctx = ctx;
inst->vpu.codec_type = ctx->current_codec;
inst->vpu.capture_type = ctx->capture_fourcc;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
index 02f39954a4c2..dd638ef44083 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
@@ -855,6 +855,7 @@ static int vdec_hevc_slice_setup_core_buffer(struct vdec_hevc_slice_inst *inst,
static int vdec_hevc_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
+ enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
struct vdec_hevc_slice_inst *inst;
int err, vsi_size;
@@ -864,8 +865,8 @@ static int vdec_hevc_slice_init(struct mtk_vcodec_dec_ctx *ctx)
inst->ctx = ctx;
- inst->vpu.id = SCP_IPI_VDEC_LAT;
- inst->vpu.core_id = SCP_IPI_VDEC_CORE;
+ inst->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
+ inst->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
inst->vpu.ctx = ctx;
inst->vpu.codec_type = ctx->current_codec;
inst->vpu.capture_type = ctx->capture_fourcc;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp8_req_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp8_req_if.c
index 391e789a5a13..d65e276f6cea 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp8_req_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp8_req_if.c
@@ -272,6 +272,7 @@ static int vdec_vp8_slice_get_decode_parameters(struct vdec_vp8_slice_inst *inst
static int vdec_vp8_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
+ enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
struct vdec_vp8_slice_inst *inst;
int err;
@@ -281,8 +282,8 @@ static int vdec_vp8_slice_init(struct mtk_vcodec_dec_ctx *ctx)
inst->ctx = ctx;
- inst->vpu.id = SCP_IPI_VDEC_LAT;
- inst->vpu.core_id = SCP_IPI_VDEC_CORE;
+ inst->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
+ inst->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
inst->vpu.ctx = ctx;
inst->vpu.codec_type = ctx->current_codec;
inst->vpu.capture_type = ctx->capture_fourcc;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
index adbacabdbebc..1f0479a8f5cb 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
@@ -1844,6 +1844,7 @@ static int vdec_vp9_slice_update_core(struct vdec_vp9_slice_instance *instance,
static int vdec_vp9_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
+ enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
struct vdec_vp9_slice_instance *instance;
struct vdec_vp9_slice_init_vsi *vsi;
int ret;
@@ -1853,8 +1854,8 @@ static int vdec_vp9_slice_init(struct mtk_vcodec_dec_ctx *ctx)
return -ENOMEM;
instance->ctx = ctx;
- instance->vpu.id = SCP_IPI_VDEC_LAT;
- instance->vpu.core_id = SCP_IPI_VDEC_CORE;
+ instance->vpu.id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_LAT0);
+ instance->vpu.core_id = mtk_vcodec_fw_get_ipi(fw_type, MTK_VDEC_CORE);
instance->vpu.ctx = ctx;
instance->vpu.codec_type = ctx->current_codec;
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 05/14] media: mediatek: vcodec: get share memory address
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (3 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 04/14] media: mediatek: vcodec: get different firmware ipi id Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:59 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 06/14] media: mediatek: vcodec: define MT8196 vcodec levels Kyrie Wu
` (8 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
There is only one share memory for vcp architecture, need to
divide it into many different functions.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../vcodec/common/mtk_vcodec_fw_vcp.c | 20 ++++++++++-
.../vcodec/common/mtk_vcodec_fw_vcp.h | 13 +++++++
.../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 35 ++++++++++++++++---
.../decoder/vdec/vdec_h264_req_multi_if.c | 6 +++-
.../decoder/vdec/vdec_hevc_req_multi_if.c | 7 ++--
.../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 22 ++++++++++--
.../mediatek/vcodec/decoder/vdec_vpu_if.c | 10 +++++-
7 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
index ee3838daf5a7..061a61bda33f 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
@@ -434,8 +434,26 @@ static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct mtk_vcodec_fw *fw)
MTK_VDEC_IS_SUPPORT_10BIT | MTK_VDEC_IS_SUPPORT_EXT;
}
-static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 dtcm_dmem_addr)
+static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 mem_type)
{
+ unsigned char *vsi_core = fw->vcp->vsi_core_addr;
+
+ switch (mem_type) {
+ case ENCODER_MEM:
+ case VCODEC_LAT_MEM:
+ return fw->vcp->vsi_addr;
+ case VCODEC_CORE_MEM:
+ return vsi_core;
+ case VP9_FRAME_MEM:
+ return vsi_core + VCODEC_VSI_LEN;
+ case AV1_CDF_MEM:
+ return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE;
+ case AV1_IQ_MEM:
+ return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE + AV1_CDF_SIZE;
+ default:
+ break;
+ }
+
return NULL;
}
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
index acf8f3e87bd0..51b05765b882 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
@@ -13,6 +13,19 @@ typedef void (*vcp_ipi_handler_t) (void *data, unsigned int len, void *priv);
#define VCP_SHARE_BUF_SIZE 64
#define VCODEC_VSI_LEN (0x2000)
+#define VP9_FRAME_SIZE (0x1000)
+#define AV1_CDF_SIZE (0xFE80)
+#define AV1_IQ_TABLE_SIZE (0x12200)
+
+/* enum mtk_vcp_mem_type - memory type for different hardware */
+enum mtk_vcp_mem_type {
+ ENCODER_MEM,
+ VCODEC_LAT_MEM,
+ VCODEC_CORE_MEM,
+ VP9_FRAME_MEM,
+ AV1_CDF_MEM,
+ AV1_IQ_MEM,
+};
/* enum mtk_vcp_ipi_index - index used to separate different hardware */
enum mtk_vcp_ipi_index {
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
index 756fbb7778b1..4932ef469594 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
@@ -765,6 +765,15 @@ static void *vdec_av1_get_ctrl_ptr(struct mtk_vcodec_dec_ctx *ctx, int id)
return ctrl->p_cur.p;
}
+static u32 vdec_av1_get_cdf_table_addr(struct mtk_vcodec_dec_ctx *ctx,
+ struct vdec_av1_slice_init_vsi *vsi)
+{
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ return AV1_CDF_MEM;
+ else
+ return (u32)vsi->cdf_table_addr;
+}
+
static int vdec_av1_slice_init_cdf_table(struct vdec_av1_slice_instance *instance)
{
u8 *remote_cdf_table;
@@ -775,7 +784,7 @@ static int vdec_av1_slice_init_cdf_table(struct vdec_av1_slice_instance *instanc
ctx = instance->ctx;
vsi = instance->vpu.vsi;
remote_cdf_table = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
- (u32)vsi->cdf_table_addr);
+ vdec_av1_get_cdf_table_addr(ctx, vsi));
if (IS_ERR(remote_cdf_table)) {
mtk_vdec_err(ctx, "failed to map cdf table\n");
return PTR_ERR(remote_cdf_table);
@@ -796,6 +805,15 @@ static int vdec_av1_slice_init_cdf_table(struct vdec_av1_slice_instance *instanc
return 0;
}
+static u32 vdec_av1_get_iq_table_addr(struct mtk_vcodec_dec_ctx *ctx,
+ struct vdec_av1_slice_init_vsi *vsi)
+{
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ return AV1_IQ_MEM;
+ else
+ return (u32)vsi->iq_table_addr;
+}
+
static int vdec_av1_slice_init_iq_table(struct vdec_av1_slice_instance *instance)
{
u8 *remote_iq_table;
@@ -806,7 +824,7 @@ static int vdec_av1_slice_init_iq_table(struct vdec_av1_slice_instance *instance
ctx = instance->ctx;
vsi = instance->vpu.vsi;
remote_iq_table = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
- (u32)vsi->iq_table_addr);
+ vdec_av1_get_iq_table_addr(ctx, vsi));
if (IS_ERR(remote_iq_table)) {
mtk_vdec_err(ctx, "failed to map iq table\n");
return PTR_ERR(remote_iq_table);
@@ -1873,6 +1891,15 @@ static int vdec_av1_slice_update_core(struct vdec_av1_slice_instance *instance,
return 0;
}
+static u32 vdec_av1_get_core_vsi_addr(struct mtk_vcodec_dec_ctx *ctx,
+ struct vdec_av1_slice_init_vsi *vsi)
+{
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ return VCODEC_CORE_MEM;
+ else
+ return (u32)vsi->core_vsi;
+}
+
static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
struct vdec_av1_slice_instance *instance;
@@ -1904,8 +1931,8 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
goto error_vsi;
}
instance->init_vsi = vsi;
- instance->core_vsi = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler, (u32)vsi->core_vsi);
-
+ instance->core_vsi = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
+ vdec_av1_get_core_vsi_addr(ctx, vsi));
if (!instance->core_vsi) {
mtk_vdec_err(ctx, "failed to get AV1 core vsi\n");
ret = -EINVAL;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index 69d60717181a..544d3bc06564 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -1233,7 +1233,11 @@ static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
vsi_size = round_up(vsi_size, VCODEC_DEC_ALIGNED_64);
inst->vsi_ext = inst->vpu.vsi;
temp = (unsigned char *)inst->vsi_ext;
- inst->vsi_core_ext = (struct vdec_h264_slice_vsi_ext *)(temp + vsi_size);
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ inst->vsi_core_ext =
+ mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler, VCODEC_CORE_MEM);
+ else
+ inst->vsi_core_ext = (struct vdec_h264_slice_vsi_ext *)(temp + vsi_size);
if (inst->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_PURE_SINGLE_CORE)
inst->decode = vdec_h264_slice_single_decode_ext;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
index dd638ef44083..a5dd42987452 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_hevc_req_multi_if.c
@@ -879,8 +879,11 @@ static int vdec_hevc_slice_init(struct mtk_vcodec_dec_ctx *ctx)
vsi_size = round_up(sizeof(struct vdec_hevc_slice_vsi), VCODEC_DEC_ALIGNED_64);
inst->vsi = inst->vpu.vsi;
- inst->vsi_core =
- (struct vdec_hevc_slice_vsi *)(((char *)inst->vpu.vsi) + vsi_size);
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ inst->vsi_core = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler, VCODEC_CORE_MEM);
+ else
+ inst->vsi_core =
+ (struct vdec_hevc_slice_vsi *)(((char *)inst->vpu.vsi) + vsi_size);
inst->resolution_changed = true;
inst->realloc_mv_buf = true;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
index 1f0479a8f5cb..3f4b70526754 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
@@ -500,6 +500,15 @@ static DEFINE_MUTEX(vdec_vp9_slice_frame_ctx_lock);
static int vdec_vp9_slice_core_decode(struct vdec_lat_buf *lat_buf);
+static u32 vdec_vp9_get_frame_ctx_addr(struct mtk_vcodec_dec_ctx *ctx,
+ struct vdec_vp9_slice_init_vsi *vsi)
+{
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ return VP9_FRAME_MEM;
+ else
+ return (u32)vsi->default_frame_ctx;
+}
+
static int vdec_vp9_slice_init_default_frame_ctx(struct vdec_vp9_slice_instance *instance)
{
struct vdec_vp9_slice_frame_ctx *remote_frame_ctx;
@@ -514,7 +523,7 @@ static int vdec_vp9_slice_init_default_frame_ctx(struct vdec_vp9_slice_instance
return -EINVAL;
remote_frame_ctx = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
- (u32)vsi->default_frame_ctx);
+ vdec_vp9_get_frame_ctx_addr(ctx, vsi));
if (!remote_frame_ctx) {
mtk_vdec_err(ctx, "failed to map default frame ctx\n");
return -EINVAL;
@@ -1842,6 +1851,15 @@ static int vdec_vp9_slice_update_core(struct vdec_vp9_slice_instance *instance,
return 0;
}
+static u32 vdec_vp9_get_core_vsi_addr(struct mtk_vcodec_dec_ctx *ctx,
+ struct vdec_vp9_slice_init_vsi *vsi)
+{
+ if (mtk_vcodec_fw_get_type(ctx->dev->fw_handler) == VCP)
+ return VCODEC_CORE_MEM;
+ else
+ return (u32)vsi->core_vsi;
+}
+
static int vdec_vp9_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
@@ -1875,7 +1893,7 @@ static int vdec_vp9_slice_init(struct mtk_vcodec_dec_ctx *ctx)
}
instance->init_vsi = vsi;
instance->core_vsi = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
- (u32)vsi->core_vsi);
+ vdec_vp9_get_core_vsi_addr(ctx, vsi));
if (!instance->core_vsi) {
mtk_vdec_err(ctx, "failed to get VP9 core vsi\n");
ret = -EINVAL;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index b35759a0b353..cdb673e6b477 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -9,6 +9,14 @@
#include "vdec_ipi_msg.h"
#include "vdec_vpu_if.h"
+static u32 vpu_dec_get_vsi_addr(struct vdec_vpu_inst *vpu, const struct vdec_vpu_ipi_init_ack *msg)
+{
+ if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
+ return VCODEC_LAT_MEM;
+ else
+ return msg->vpu_inst_addr;
+}
+
static void handle_init_ack_msg(const struct vdec_vpu_ipi_init_ack *msg)
{
struct vdec_vpu_inst *vpu = (struct vdec_vpu_inst *)
@@ -19,7 +27,7 @@ static void handle_init_ack_msg(const struct vdec_vpu_ipi_init_ack *msg)
/* mapping VPU address to kernel virtual address */
/* the content in vsi is initialized to 0 in VPU */
vpu->vsi = mtk_vcodec_fw_map_dm_addr(vpu->ctx->dev->fw_handler,
- msg->vpu_inst_addr);
+ vpu_dec_get_vsi_addr(vpu, msg));
vpu->inst_addr = msg->vpu_inst_addr;
mtk_vdec_debug(vpu->ctx, "- vpu_inst_addr = 0x%x", vpu->inst_addr);
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 05/14] media: mediatek: vcodec: get share memory address
2026-06-03 8:40 ` [PATCH v5 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
@ 2026-06-03 8:59 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:59 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, conor+dt, robh, linux-media
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The `mtk_vcodec_vcp_msg` ops structure lacks the `get_venc_capa` callback, causing a NULL pointer dereference when opening a VCP encoder.
- [High] The VCP memory address mapping breaks encoder initialization because encoder drivers were not updated to pass the new enum values.
Pre-existing issues:
- [High] Missing bounds validation for firmware-provided AV1 table sizes causes potential out-of-bounds reads.
- [High] `if (!ptr)` error checks fail to catch `ERR_PTR`, leading to kernel panic on mapping failure.
--
commit 2fc79ef1812a3894cd11fa26b7b0e560b49068a2
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: get share memory address
There is only one share memory for vcp architecture, need to
divide it into many different functions.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
[ ... ]
> @@ -434,8 +434,26 @@ static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct mtk_vcodec_fw *fw)
> MTK_VDEC_IS_SUPPORT_10BIT | MTK_VDEC_IS_SUPPORT_EXT;
> }
>
> -static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 dtcm_dmem_addr)
> +static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 mem_type)
> {
[Severity: High]
Does the VCP firmware operations structure mtk_vcodec_vcp_msg support encoders?
When opening an encoder device, fops_vcodec_open() unconditionally calls
mtk_vcodec_fw_get_venc_capa(), which invokes fw->ops->get_venc_capa(fw).
If mtk_vcodec_vcp_msg does not assign the .get_venc_capa callback, won't
opening a VCP encoder trigger a NULL pointer dereference?
> + unsigned char *vsi_core = fw->vcp->vsi_core_addr;
> +
> + switch (mem_type) {
> + case ENCODER_MEM:
> + case VCODEC_LAT_MEM:
> + return fw->vcp->vsi_addr;
> + case VCODEC_CORE_MEM:
> + return vsi_core;
> + case VP9_FRAME_MEM:
> + return vsi_core + VCODEC_VSI_LEN;
> + case AV1_CDF_MEM:
> + return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE;
> + case AV1_IQ_MEM:
> + return vsi_core + VCODEC_VSI_LEN + VP9_FRAME_SIZE + AV1_CDF_SIZE;
> + default:
> + break;
> + }
> +
> return NULL;
> }
[Severity: High]
Are we sure all encoder calls have been updated to pass the new memory type
enums?
It looks like the encoder subsystem still passes physical device addresses
instead of the new enum mtk_vcp_mem_type. For example, handle_enc_init_msg()
in venc_vpu_if.c passes msg->vpu_inst_addr, and h264_enc_alloc_work_buf() in
venc_h264_if.c passes vpua.
Since this switch statement will fall through to default for those physical
addresses, won't mtk_vcodec_fw_map_dm_addr() now return NULL and trigger a
crash during encoder initialization?
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
[ ... ]
> @@ -775,7 +784,7 @@ static int vdec_av1_slice_init_cdf_table(struct vdec_av1_slice_instance *instanc
> ctx = instance->ctx;
> vsi = instance->vpu.vsi;
> remote_cdf_table = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
> - (u32)vsi->cdf_table_addr);
> + vdec_av1_get_cdf_table_addr(ctx, vsi));
[Severity: High]
This is a pre-existing issue, but is it safe to use vsi->cdf_table_size without
validation?
In vdec_av1_slice_init_cdf_table(), the size vsi->cdf_table_size is provided
by the untrusted firmware and later used in a memory copy:
memcpy(instance->cdf_table.va, remote_cdf_table, vsi->cdf_table_size);
Could a compromised firmware provide an arbitrarily large size, leading to an
out-of-bounds read from the DMA region and a corresponding out-of-bounds write
into kernel memory?
> if (IS_ERR(remote_cdf_table)) {
> mtk_vdec_err(ctx, "failed to map cdf table\n");
> return PTR_ERR(remote_cdf_table);
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
[ ... ]
> @@ -514,7 +523,7 @@ static int vdec_vp9_slice_init_default_frame_ctx(struct vdec_vp9_slice_instance
> return -EINVAL;
>
> remote_frame_ctx = mtk_vcodec_fw_map_dm_addr(ctx->dev->fw_handler,
> - (u32)vsi->default_frame_ctx);
> + vdec_vp9_get_frame_ctx_addr(ctx, vsi));
> if (!remote_frame_ctx) {
> mtk_vdec_err(ctx, "failed to map default frame ctx\n");
> return -EINVAL;
[Severity: High]
This is a pre-existing issue, but does this error check handle ERR_PTR returns
correctly?
When using SCP firmware, scp_mapping_dm_addr() returns ERR_PTR(-EINVAL) on
mapping failure. Since an error pointer evaluates as true, the
if (!remote_frame_ctx) check is bypassed.
Could this allow the invalid ERR_PTR to be passed to kmemdup() further down in
the function, causing a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=5
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 06/14] media: mediatek: vcodec: define MT8196 vcodec levels.
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (4 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:51 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 07/14] media: mediatek: vcodec: support 36bit iova address Kyrie Wu
` (7 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The supported level and profile are not the same for different
codecs and architecture. Select the correct one.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index ab1894fba0d9..472ece5713a5 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -577,6 +577,7 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_2;
break;
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_6_0;
break;
case MTK_VDEC_MT8183:
@@ -595,6 +596,7 @@ static void mtk_vcodec_dec_fill_h264_profile(struct v4l2_ctrl_config *cfg,
switch (ctx->dev->chip_name) {
case MTK_VDEC_MT8188:
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10;
break;
default:
@@ -611,6 +613,7 @@ static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1;
break;
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2;
break;
default:
@@ -625,6 +628,7 @@ static void mtk_vcodec_dec_fill_h265_profile(struct v4l2_ctrl_config *cfg,
switch (ctx->dev->chip_name) {
case MTK_VDEC_MT8188:
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10;
break;
default:
@@ -642,6 +646,7 @@ static void mtk_vcodec_dec_fill_vp9_level(struct v4l2_ctrl_config *cfg,
cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_1;
break;
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_2;
break;
case MTK_VDEC_MT8186:
@@ -659,6 +664,7 @@ static void mtk_vcodec_dec_fill_vp9_profile(struct v4l2_ctrl_config *cfg,
switch (ctx->dev->chip_name) {
case MTK_VDEC_MT8188:
case MTK_VDEC_MT8195:
+ case MTK_VDEC_MT8196:
cfg->max = V4L2_MPEG_VIDEO_VP9_PROFILE_2;
break;
default:
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 06/14] media: mediatek: vcodec: define MT8196 vcodec levels.
2026-06-03 8:40 ` [PATCH v5 06/14] media: mediatek: vcodec: define MT8196 vcodec levels Kyrie Wu
@ 2026-06-03 8:51 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:51 UTC (permalink / raw)
To: Kyrie Wu; +Cc: linux-media, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces the undeclared identifier `MTK_VDEC_MT8196` before it is defined, causing a compilation error and breaking `git bisect`.
--
commit 918de5bcb94623dd2782fada54c8dcb18dc11b48
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: define MT8196 vcodec levels.
The patch updates various vcodec control configuration functions to add
support for the MT8196 chip by setting the maximum profiles and levels for
H264, HEVC, and VP9.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> index ab1894fba0d90..472ece5713a56 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> @@ -577,6 +577,7 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
> cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_2;
> break;
> case MTK_VDEC_MT8195:
> + case MTK_VDEC_MT8196:
[Severity: Low]
Does this introduce a build break for git bisect?
The MTK_VDEC_MT8196 identifier appears to be undeclared at this point in the
commit history. While it is fixed later in the series by the commit "media:
mediatek: vcodec: add decoder compatible to support mt8196", using the
identifier before it is defined will cause compilation errors and break git
bisect.
> cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_6_0;
> break;
> case MTK_VDEC_MT8183:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=6
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 07/14] media: mediatek: vcodec: support 36bit iova address
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (5 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 06/14] media: mediatek: vcodec: define MT8196 vcodec levels Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:52 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 08/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
` (6 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
Need to set dma mask to support 36bit iova address for decoder
hardware can use 36bit address to decode for mt8196.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index ce17b649022f..68759db6f8d1 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -390,6 +390,13 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
return -ENODEV;
}
dma_set_max_seg_size(&pdev->dev, UINT_MAX);
+ if (dev->chip_name == MTK_VDEC_MT8196) {
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(36));
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to enable 36-bit DMA: %d\n", ret);
+ return ret;
+ }
+ }
dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, DECODER);
if (IS_ERR(dev->fw_handler))
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 07/14] media: mediatek: vcodec: support 36bit iova address
2026-06-03 8:40 ` [PATCH v5 07/14] media: mediatek: vcodec: support 36bit iova address Kyrie Wu
@ 2026-06-03 8:52 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:52 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, conor+dt, robh, linux-media
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch references an undeclared identifier `MTK_VDEC_MT8196`, causing a build failure and breaking `git bisect`.
--
commit 1d69905fbcb6b87b34af91dd5fb8d705e10184c1
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: support 36bit iova address
Need to set dma mask to support 36bit iova address for decoder
hardware can use 36bit address to decode for mt8196.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> index ce17b649022f7..68759db6f8d1c 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> @@ -390,6 +390,13 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> return -ENODEV;
> }
> dma_set_max_seg_size(&pdev->dev, UINT_MAX);
> + if (dev->chip_name == MTK_VDEC_MT8196) {
^^^^^^^^^^^^^^^
[Severity: Low]
Does this code compile on its own? It appears that MTK_VDEC_MT8196 is not
defined in this commit, which would cause a build failure and break git bisect
at this point in the series.
> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(36));
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable 36-bit DMA: %d\n", ret);
> + return ret;
> + }
> + }
>
> dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, DECODER);
> if (IS_ERR(dev->fw_handler))
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=7
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 08/14] media: mediatek: vcodec: clean xpc status
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (6 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 07/14] media: mediatek: vcodec: support 36bit iova address Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:54 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 09/14] media: mediatek: vcodec: add debug information Kyrie Wu
` (5 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The driver need to clean xpc status when receive decoder hardware
interrupt for mt8196 platform.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../vcodec/decoder/mtk_vcodec_dec_hw.c | 28 +++++++++++++++++++
.../vcodec/decoder/mtk_vcodec_dec_hw.h | 13 +++++++--
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
index 881d5de41e05..e4e527fe54dc 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
@@ -61,6 +61,31 @@ static int mtk_vdec_hw_prob_done(struct mtk_vcodec_dec_dev *vdec_dev)
return 0;
}
+static void mtk_vdec_hw_write_reg_mask(void __iomem *reg_base, u32 reg_offset, u32 val, u32 mask)
+{
+ void __iomem *reg_addr = reg_base + reg_offset;
+ u32 reg_val;
+
+ reg_val = readl(reg_addr);
+ reg_val &= ~mask;
+ reg_val |= (val & mask);
+ writel(reg_val, reg_addr);
+}
+
+static void mtk_vdec_hw_clean_xpc(struct mtk_vdec_hw_dev *dev)
+{
+ u32 val, mask, addr = VDEC_XPC_CLEAN_ADDR;
+
+ if (dev->main_dev->chip_name != MTK_VDEC_MT8196)
+ return;
+
+ val = dev->hw_idx == MTK_VDEC_LAT0 ? VDEC_XPC_LAT_VAL : VDEC_XPC_CORE_VAL;
+ mask = dev->hw_idx == MTK_VDEC_LAT0 ? VDEC_XPC_LAT_MASK : VDEC_XPC_CORE_MASK;
+
+ mtk_vdec_hw_write_reg_mask(dev->reg_base[VDEC_HW_XPC], addr, val, mask);
+ mtk_vdec_hw_write_reg_mask(dev->reg_base[VDEC_HW_XPC], addr, 0, mask);
+}
+
static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
{
struct mtk_vdec_hw_dev *dev = priv;
@@ -88,6 +113,8 @@ static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
writel(dec_done_status & ~VDEC_IRQ_CLR, vdec_misc_addr);
+ mtk_vdec_hw_clean_xpc(dev);
+
wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev->hw_idx);
mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d, dec_done_status=%x",
@@ -166,6 +193,7 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
subdev_dev->hw_idx = hw_idx;
subdev_dev->main_dev = main_dev;
subdev_dev->reg_base[VDEC_HW_SYS] = main_dev->reg_base[VDEC_HW_SYS];
+ subdev_dev->reg_base[VDEC_HW_XPC] = main_dev->reg_base[VDEC_HW_MISC];
set_bit(subdev_dev->hw_idx, main_dev->subdev_bitmap);
if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.h
index 83fe8b9428e6..5c906143c9af 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.h
@@ -18,17 +18,26 @@
#define VDEC_IRQ_CLR 0x10
#define VDEC_IRQ_CFG_REG 0xa4
+#define VDEC_XPC_CLEAN_ADDR 0xc
+#define VDEC_XPC_LAT_VAL BIT(0)
+#define VDEC_XPC_LAT_MASK BIT(0)
+
+#define VDEC_XPC_CORE_VAL BIT(4)
+#define VDEC_XPC_CORE_MASK BIT(4)
+
#define IS_SUPPORT_VDEC_HW_IRQ(hw_idx) ((hw_idx) != MTK_VDEC_LAT_SOC)
/**
* enum mtk_vdec_hw_reg_idx - subdev hardware register base index
- * @VDEC_HW_SYS : vdec soc register index
+ * @VDEC_HW_SYS: vdec soc register index
* @VDEC_HW_MISC: vdec misc register index
- * @VDEC_HW_MAX : vdec supported max register index
+ * @VDEC_HW_XPC: vdec xpc register index
+ * @VDEC_HW_MAX: vdec supported max register index
*/
enum mtk_vdec_hw_reg_idx {
VDEC_HW_SYS,
VDEC_HW_MISC,
+ VDEC_HW_XPC,
VDEC_HW_MAX
};
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 08/14] media: mediatek: vcodec: clean xpc status
2026-06-03 8:40 ` [PATCH v5 08/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
@ 2026-06-03 8:54 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:54 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, linux-media, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly introduced `mtk_vdec_hw_clean_xpc` function performs non-atomic read-modify-write (RMW) operations on a shared hardware register from concurrent interrupt handlers without any locking, leading to potential state corruption and lost XPC clean pulses.
- [Low] Type confusion in enum usage for indexing main_dev->reg_base.
--
commit 6d6918cc2e35280ad95cd095aa0042f4f69bdfc5
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: clean xpc status
This commit adds functionality to clean the XPC status upon receiving a
decoder hardware interrupt for the MT8196 platform.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> index 881d5de41e050..e4e527fe54dcb 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
> @@ -61,6 +61,31 @@ static int mtk_vdec_hw_prob_done(struct mtk_vcodec_dec_dev *vdec_dev)
> return 0;
> }
>
> +static void mtk_vdec_hw_write_reg_mask(void __iomem *reg_base, u32 reg_offset, u32 val, u32 mask)
> +{
> + void __iomem *reg_addr = reg_base + reg_offset;
> + u32 reg_val;
> +
> + reg_val = readl(reg_addr);
> + reg_val &= ~mask;
> + reg_val |= (val & mask);
> + writel(reg_val, reg_addr);
> +}
[Severity: High]
Since LAT0 and CORE hardware subdevices share the same parent device register
space and have separate interrupt lines, can their interrupt handlers race
here?
If both execute concurrently on different CPUs and call
mtk_vdec_hw_write_reg_mask(), could the unsynchronized read-modify-write
sequence cause one CPU to overwrite and lose the XPC clean pulse from the
other?
Does this sequence need a lock to protect the shared register state?
> +
> +static void mtk_vdec_hw_clean_xpc(struct mtk_vdec_hw_dev *dev)
> +{
> + u32 val, mask, addr = VDEC_XPC_CLEAN_ADDR;
[ ... ]
> @@ -166,6 +193,7 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
> subdev_dev->hw_idx = hw_idx;
> subdev_dev->main_dev = main_dev;
> subdev_dev->reg_base[VDEC_HW_SYS] = main_dev->reg_base[VDEC_HW_SYS];
> + subdev_dev->reg_base[VDEC_HW_XPC] = main_dev->reg_base[VDEC_HW_MISC];
[Severity: Low]
Is the index VDEC_HW_MISC correct for main_dev->reg_base?
It appears main_dev->reg_base is typically indexed by enum mtk_hw_reg_idx
(like VDEC_MISC), while VDEC_HW_MISC belongs to enum mtk_vdec_hw_reg_idx.
Even though both currently evaluate to 1 and map correctly, could this cause
issues or point to the wrong hardware registers if the enums drift apart in
future updates?
> set_bit(subdev_dev->hw_idx, main_dev->subdev_bitmap);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=8
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 09/14] media: mediatek: vcodec: add debug information
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (7 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 08/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:54 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
` (4 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
Print hevc/av1 output format and 10bit capture format
information to debug.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../mediatek/vcodec/common/mtk_vcodec_dbgfs.c | 21 +++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
index 2da11521fc7b..b3c69c46240c 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
@@ -29,6 +29,14 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_dec_ctx *ctx, char
curr_len = snprintf(buf + *used, total - *used,
"\toutput format: vp9 slice\n");
break;
+ case V4L2_PIX_FMT_HEVC_SLICE:
+ curr_len = snprintf(buf + *used, total - *used,
+ "\toutput format: hevc slice\n");
+ break;
+ case V4L2_PIX_FMT_AV1_FRAME:
+ curr_len = snprintf(buf + *used, total - *used,
+ "\toutput format: av1 slice\n");
+ break;
default:
curr_len = snprintf(buf + *used, total - *used,
"\tunsupported output format: 0x%x\n",
@@ -45,6 +53,14 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_dec_ctx *ctx, char
curr_len = snprintf(buf + *used, total - *used,
"\tcapture format: MT21C\n");
break;
+ case V4L2_PIX_FMT_MT2110T:
+ curr_len = snprintf(buf + *used, total - *used,
+ "\tcapture format: MT2110T (10bit tile mode)\n");
+ break;
+ case V4L2_PIX_FMT_MT2110R:
+ curr_len = snprintf(buf + *used, total - *used,
+ "\tcapture format: MT2110T (10bit raster mode)\n");
+ break;
default:
curr_len = snprintf(buf + *used, total - *used,
"\tunsupported capture format: 0x%x\n",
@@ -122,9 +138,10 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
if (dbgfs_index[MTK_VDEC_DBGFS_PICINFO]) {
curr_len = snprintf(buf + used_len, total_len - used_len,
- "\treal(%dx%d)=>align(%dx%d)\n",
+ "\treal(%dx%d)=>align(%dx%d) 10bit(%d)\n",
ctx->picinfo.pic_w, ctx->picinfo.pic_h,
- ctx->picinfo.buf_w, ctx->picinfo.buf_h);
+ ctx->picinfo.buf_w, ctx->picinfo.buf_h,
+ ctx->is_10bit_bitstream);
used_len += curr_len;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 09/14] media: mediatek: vcodec: add debug information
2026-06-03 8:40 ` [PATCH v5 09/14] media: mediatek: vcodec: add debug information Kyrie Wu
@ 2026-06-03 8:54 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:54 UTC (permalink / raw)
To: Kyrie Wu; +Cc: linux-media, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] Incorrect debug output string for V4L2_PIX_FMT_MT2110R (Typo)
Pre-existing issues:
- [Critical] Use-After-Free and List Corruption in debugfs due to Lock Mismatch
- [High] Out-of-Bounds Read in debugfs due to missing null terminator
--
commit 87b57cf0a11ac27db38b48d00f4d66f14eff0f96
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: add debug information
Print hevc/av1 output format and 10bit capture format
information to debug.
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
> index 2da11521fc7b9..b3c69c46240cf 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
[ ... ]
> @@ -45,6 +53,14 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_dec_ctx *ctx, char
> curr_len = snprintf(buf + *used, total - *used,
> "\tcapture format: MT21C\n");
> break;
> + case V4L2_PIX_FMT_MT2110T:
> + curr_len = snprintf(buf + *used, total - *used,
> + "\tcapture format: MT2110T (10bit tile mode)\n");
> + break;
> + case V4L2_PIX_FMT_MT2110R:
> + curr_len = snprintf(buf + *used, total - *used,
> + "\tcapture format: MT2110T (10bit raster mode)\n");
[Severity: Low]
Does this incorrectly print MT2110T instead of MT2110R for the raster mode
format?
> + break;
> default:
> curr_len = snprintf(buf + *used, total - *used,
> "\tunsupported capture format: 0x%x\n",
> @@ -122,9 +138,10 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
[Severity: High]
This is a pre-existing issue, but does mtk_vdec_dbgfs_read() safely handle
dbgfs->dbgfs_buf?
Earlier in this function, the code checks:
if (strstr(dbgfs->dbgfs_buf, "-help") || dbgfs->buf_size == 1) {
However, mtk_vdec_dbgfs_write() uses simple_write_to_buffer() to copy user
payload into dbgfs->dbgfs_buf without ensuring the string is null-terminated.
Can this cause strstr() to read out-of-bounds?
[Severity: Critical]
This is a pre-existing issue, but is the list traversal in
mtk_vdec_dbgfs_read() fully protected against concurrent modification?
The traversal iterates over &dbgfs->dbgfs_head while holding
dbgfs->dbgfs_lock:
mtk_vcodec_dbgfs.c:mtk_vdec_dbgfs_read() {
...
mutex_lock(&dbgfs->dbgfs_lock);
list_for_each_entry(dbgfs_inst, &dbgfs->dbgfs_head, node) {
...
}
However, mtk_vcodec_dbgfs_remove() mutates this same list and frees the
nodes without acquiring dbgfs->dbgfs_lock:
mtk_vcodec_dbgfs.c:mtk_vcodec_dbgfs_remove() {
...
list_for_each_entry(dbgfs_inst, &vcodec_dev->dbgfs.dbgfs_head, node) {
if (dbgfs_inst->inst_id == ctx_id) {
...
list_del(&dbgfs_inst->node);
kfree(dbgfs_inst);
...
}
Can this lead to a use-after-free or list corruption if
mtk_vcodec_dbgfs_remove() races with the mtk_vdec_dbgfs_read() traversal?
>
> if (dbgfs_index[MTK_VDEC_DBGFS_PICINFO]) {
> curr_len = snprintf(buf + used_len, total_len - used_len,
> - "\treal(%dx%d)=>align(%dx%d)\n",
> + "\treal(%dx%d)=>align(%dx%d) 10bit(%d)\n",
> ctx->picinfo.pic_w, ctx->picinfo.pic_h,
> - ctx->picinfo.buf_w, ctx->picinfo.buf_h);
> + ctx->picinfo.buf_w, ctx->picinfo.buf_h,
> + ctx->is_10bit_bitstream);
> used_len += curr_len;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=9
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (8 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 09/14] media: mediatek: vcodec: add debug information Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 11:09 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
` (3 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The share memory is allocated in kernel for vcp architecture, it's
different with vpu which share memors is reserved in vpu micro
processor. Need to send share memory address to vcp.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h | 2 ++
drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
index 47070be2a991..097561a1efdc 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
@@ -67,11 +67,13 @@ struct vdec_vpu_ipi_ack {
* @msg_id : AP_IPIMSG_DEC_INIT
* @codec_type : codec fourcc
* @ap_inst_addr : AP video decoder instance address
+ * @shared_iova : reserved share memory address
*/
struct vdec_ap_ipi_init {
uint32_t msg_id;
u32 codec_type;
uint64_t ap_inst_addr;
+ u64 shared_iova;
};
/**
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index cdb673e6b477..3a10b32be094 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -236,6 +236,8 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
msg.msg_id = AP_IPIMSG_DEC_INIT;
msg.ap_inst_addr = (unsigned long)vpu;
msg.codec_type = vpu->codec_type;
+ if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
+ msg.shared_iova = vpu->ctx->dev->fw_handler->vcp->iova_addr;
mtk_vdec_debug(vpu->ctx, "vdec_inst=%p", vpu);
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp
2026-06-03 8:40 ` [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
@ 2026-06-03 11:09 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 11:09 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, conor+dt, robh, linux-media
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The size of `struct vdec_ap_ipi_init` is increased from 16 to 24 bytes, and this larger size is unconditionally sent to all firmware architectures, risking backward compatibility breakage and firmware buffer overflows for older VPU/SCP implementations.
--
commit d463b7b0f56f0354049dbd8919b4208b451e2182
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: send share memory address to vcp
The share memory is allocated in kernel for vcp architecture, it's
different with vpu which share memors is reserved in vpu micro
processor. Need to send share memory address to vcp.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> @@ -67,11 +67,13 @@ struct vdec_vpu_ipi_ack {
> * @msg_id : AP_IPIMSG_DEC_INIT
> * @codec_type : codec fourcc
> * @ap_inst_addr : AP video decoder instance address
> + * @shared_iova : reserved share memory address
> */
> struct vdec_ap_ipi_init {
> uint32_t msg_id;
> u32 codec_type;
> uint64_t ap_inst_addr;
> + u64 shared_iova;
> };
[ ... ]
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> @@ -236,6 +236,8 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
> msg.msg_id = AP_IPIMSG_DEC_INIT;
> msg.ap_inst_addr = (unsigned long)vpu;
> msg.codec_type = vpu->codec_type;
> + if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
> + msg.shared_iova = vpu->ctx->dev->fw_handler->vcp->iova_addr;
>
> mtk_vdec_debug(vpu->ctx, "vdec_inst=%p", vpu);
[Severity: High]
Could this structure size increase break compatibility with legacy VPU and SCP
firmwares?
Adding shared_iova to struct vdec_ap_ipi_init increases the message size
from 16 to 24 bytes. A few lines later in vpu_dec_init(), the message is
sent unconditionally using the new larger size:
err = vcodec_vpu_send_msg(vpu, (void *)&msg, sizeof(msg));
If legacy firmware implementations expect exactly 16 bytes for
AP_IPIMSG_DEC_INIT, could sending a 24-byte payload cause initialization
failures or stack buffer overflows in the legacy firmware?
Perhaps the driver should only send the expanded length when communicating
with the VCP architecture, or use offsetof() to send the original 16-byte
size to older targets.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=10
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (9 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 10/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 9:03 ` sashiko-bot
2026-06-03 16:14 ` Conor Dooley
2026-06-03 8:40 ` [PATCH v5 12/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
` (2 subsequent siblings)
13 siblings, 2 replies; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The MT8196 decoder differs from previous generations in several
key aspects, most notably in its use of VCP instead of SCP.
Additionally, the MT8196 enhances codec capabilities by supporting
HEVC Main10 profile decoding. To accommodate these hardware changes,
the binding constraints specify a total of 12 clock inputs,
consisting of 9 decoder clocks and 3 VCP interface clocks,
along with 2 power domains covering both the decoder and VCP
subsystems.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
index bf8082d87ac0..74e1d88d3056 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
@@ -76,6 +76,7 @@ properties:
- mediatek,mt8186-vcodec-dec
- mediatek,mt8188-vcodec-dec
- mediatek,mt8195-vcodec-dec
+ - mediatek,mt8196-vcodec-dec
reg:
minItems: 1
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-06-03 8:40 ` [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
@ 2026-06-03 9:03 ` sashiko-bot
2026-06-03 16:14 ` Conor Dooley
1 sibling, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 9:03 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, conor+dt, devicetree, linux-media
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] The VCP message queue processing thread uses the untrusted `obj->id` directly as an array index without bounds checking, leading to out-of-bounds memory access and potential arbitrary code execution.
- [High] The VCP message queue kthreads spawned during firmware load are leaked, leading to a fatal Use-After-Free kernel panic on driver unbind.
- [High] The `ipi_id_ack` array stores negative error codes into `bool` elements, truncating them to `1`, and is never reset on success, which breaks error handling and permanently poisons the IPC state.
- [Medium] The patch adds the MT8196 compatible string but completely omits the corresponding hardware constraints mentioned in the commit message, breaking device tree validation for this SoC.
--
commit f686f0ab8d134aa32615f005292674ee98cf9fb9
Author: Yunfei Dong <yunfei.dong@mediatek.com>
dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
This commit adds the device tree binding compatible string for the MT8196
decoder. The commit message outlines hardware changes, including 12 clock
inputs, 2 power domains, and the use of VCP instead of SCP.
> The MT8196 decoder differs from previous generations in several
> key aspects, most notably in its use of VCP instead of SCP.
> Additionally, the MT8196 enhances codec capabilities by supporting
> HEVC Main10 profile decoding. To accommodate these hardware changes,
> the binding constraints specify a total of 12 clock inputs,
> consisting of 9 decoder clocks and 3 VCP interface clocks,
> along with 2 power domains covering both the decoder and VCP
> subsystems.
[Severity: Medium]
The commit message mentions that the binding constraints specify a total of 12
clock inputs, 2 power domains, and the use of VCP instead of SCP. However,
are these constraints actually added in this patch?
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> index bf8082d87ac03..74e1d88d30568 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> @@ -76,6 +76,7 @@ properties:
> - mediatek,mt8186-vcodec-dec
> - mediatek,mt8188-vcodec-dec
> - mediatek,mt8195-vcodec-dec
> + - mediatek,mt8196-vcodec-dec
Looking at the diff, it appears only the compatible string is added. The
required conditional blocks and property constraints for the clocks, power
domains, and the mediatek,vcp property seem to be missing. Would this cause
valid MT8196 device tree files to falsely fail schema validation?
I also have some questions regarding the VCP message queue handling in the
related C driver files.
In drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c:
[Severity: High]
Does this code leak the vcp_enc_msq_thread kthread?
mtk_vcodec_fw_vcp.c:mtk_vcodec_vcp_load_firmware() {
...
kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_enc_msq_thread");
...
}
It seems the task_struct pointer returned by kthread_run() is discarded, so
kthread_stop() cannot be called later in mtk_vcodec_vcp_release().
If the thread is leaked, could this lead to a use-after-free crash on driver
unbind? The leaked thread appears to block indefinitely on the waitqueue inside
the devres-managed fw->vcp struct:
mtk_vcodec_fw_vcp.c:mtk_vcodec_vcp_msg_process_thread() {
...
ret = wait_event_interruptible(fw->vcp->msg_queue.wq,
atomic_read(&fw->vcp->msg_queue.cnt) > 0);
...
}
[Severity: Critical]
Can an invalid firmware payload trigger an out-of-bounds memory access here?
mtk_vcodec_fw_vcp.c:mtk_vcodec_vcp_msg_process_thread() {
...
mtk_vcodec_vcp_ipi_lock(fw->vcp, obj->id);
handler = fw->vcp->ipi_desc[obj->id].handler;
...
handler(msg, obj->len, fw->vcp->ipi_desc[obj->id].priv);
...
}
The obj->id value comes from the firmware IPC message payload. If this value
exceeds the VCP_IPI_MAX bound, it looks like it could directly index outside
the ipi_desc array, fetching an unvalidated function pointer and immediately
executing it. Should we add a bounds check for obj->id?
[Severity: High]
Are standard negative error codes truncated in this structure?
drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h:struct mtk_vcp {
...
bool ipi_id_ack[VCP_IPI_MAX];
...
}
Because ipi_id_ack is declared as a boolean array, assigning negative error
codes to it will truncate them to 1 (true).
Additionally, it seems the flag is never reset to false on success:
mtk_vcodec_fw_vcp.c:mtk_vcodec_vcp_msg_ipi_send() {
...
return vcp->ipi_id_ack[id];
}
If a single error or timeout sets this flag to true, won't all subsequent
successful IPC sends falsely report failure because they read the stale true
state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=11
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-06-03 8:40 ` [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
2026-06-03 9:03 ` sashiko-bot
@ 2026-06-03 16:14 ` Conor Dooley
1 sibling, 0 replies; 28+ messages in thread
From: Conor Dooley @ 2026-06-03 16:14 UTC (permalink / raw)
To: Kyrie Wu
Cc: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]
On Wed, Jun 03, 2026 at 04:40:41PM +0800, Kyrie Wu wrote:
> From: Yunfei Dong <yunfei.dong@mediatek.com>
>
> The MT8196 decoder differs from previous generations in several
> key aspects, most notably in its use of VCP instead of SCP.
> Additionally, the MT8196 enhances codec capabilities by supporting
> HEVC Main10 profile decoding. To accommodate these hardware changes,
> the binding constraints specify a total of 12 clock inputs,
> consisting of 9 decoder clocks and 3 VCP interface clocks,
> along with 2 power domains covering both the decoder and VCP
> subsystems.
I'm pretty pretty confused by this statement about constraints, since
there's none added?
The vcodec-dec node doesn't even seem to permit clocks at all?
>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> .../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> index bf8082d87ac0..74e1d88d3056 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> @@ -76,6 +76,7 @@ properties:
> - mediatek,mt8186-vcodec-dec
> - mediatek,mt8188-vcodec-dec
> - mediatek,mt8195-vcodec-dec
> + - mediatek,mt8196-vcodec-dec
>
> reg:
> minItems: 1
> --
> 2.45.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 12/14] media: mediatek: vcodec: add decoder compatible to support mt8196
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (10 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 11/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 8:59 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
2026-06-03 8:40 ` [PATCH v5 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
MT8196 is lat single core architecture. Support its compatible and
use `mtk_lat_sig_core_pdata` to initialize platform data.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 6 ++++++
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index 68759db6f8d1..bee6e057b0a7 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -347,6 +347,8 @@ static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
vdec_dev->chip_name = MTK_VDEC_MT8186;
else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
vdec_dev->chip_name = MTK_VDEC_MT8188;
+ else if (of_device_is_compatible(dev->of_node, "mediatek,mt8196-vcodec-dec"))
+ vdec_dev->chip_name = MTK_VDEC_MT8196;
else
vdec_dev->chip_name = MTK_VDEC_INVAL;
}
@@ -573,6 +575,10 @@ static const struct of_device_id mtk_vcodec_match[] = {
.compatible = "mediatek,mt8188-vcodec-dec",
.data = &mtk_lat_sig_core_pdata,
},
+ {
+ .compatible = "mediatek,mt8196-vcodec-dec",
+ .data = &mtk_lat_sig_core_pdata,
+ },
{},
};
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index 15e437323af5..53d1708b012b 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -29,6 +29,7 @@ enum mtk_vcodec_dec_chip_name {
MTK_VDEC_MT8188 = 8188,
MTK_VDEC_MT8192 = 8192,
MTK_VDEC_MT8195 = 8195,
+ MTK_VDEC_MT8196 = 8196,
};
/*
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 12/14] media: mediatek: vcodec: add decoder compatible to support mt8196
2026-06-03 8:40 ` [PATCH v5 12/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
@ 2026-06-03 8:59 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 8:59 UTC (permalink / raw)
To: Kyrie Wu; +Cc: linux-media, conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Patch series breaks git bisect due to usage of undeclared identifier MTK_VDEC_MT8196 before it is defined.
--
commit ea2155f52be3df7e341354e244a5cf63f836a145
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: vcodec: add decoder compatible to support mt8196
This commit adds the MT8196 compatible string and device data for the
MediaTek vcodec decoder. It also introduces the MTK_VDEC_MT8196 enum value
used to identify the chip.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> index 15e437323af5a..53d1708b012b7 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> @@ -29,6 +29,7 @@ enum mtk_vcodec_dec_chip_name {
> MTK_VDEC_MT8188 = 8188,
> MTK_VDEC_MT8192 = 8192,
> MTK_VDEC_MT8195 = 8195,
> + MTK_VDEC_MT8196 = 8196,
[Severity: Low]
Does this series break git bisect?
Since MTK_VDEC_MT8196 is first defined here, wouldn't compiling earlier
commits in this series (such as "media: mediatek: vcodec: define MT8196
vcodec levels." or "media: mediatek: vcodec: support 36bit iova address")
fail with an undeclared identifier error?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=12
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (11 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 12/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 9:07 ` sashiko-bot
2026-06-03 8:40 ` [PATCH v5 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The buffer size of y and c plane has been calculated in vcp/scp,
can fill each frame buffer size with picinfo directly.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
index 4932ef469594..a0c7e89b8ae4 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
@@ -1811,18 +1811,19 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
{
struct vb2_buffer *vb;
struct vb2_queue *vq;
- int w, h, plane, size;
+ int plane;
int i;
plane = instance->ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes;
- w = vsi->frame.uh.upscaled_width;
- h = vsi->frame.uh.frame_height;
- size = ALIGN(w, VCODEC_DEC_ALIGNED_64) * ALIGN(h, VCODEC_DEC_ALIGNED_64);
/* frame buffer */
vsi->fb.y.dma_addr = fb->base_y.dma_addr;
+
+ vsi->fb.y.size = instance->ctx->picinfo.fb_sz[0];
+ vsi->fb.c.size = instance->ctx->picinfo.fb_sz[1];
+
if (plane == 1)
- vsi->fb.c.dma_addr = fb->base_y.dma_addr + size;
+ vsi->fb.c.dma_addr = fb->base_y.dma_addr + vsi->fb.y.size;
else
vsi->fb.c.dma_addr = fb->base_c.dma_addr;
@@ -1845,8 +1846,9 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
}
vref->y.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
+ vref->y.size = vsi->fb.y.size;
if (plane == 1)
- vref->c.dma_addr = vref->y.dma_addr + size;
+ vref->c.dma_addr = vref->y.dma_addr + vsi->fb.y.size;
else
vref->c.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo
2026-06-03 8:40 ` [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
@ 2026-06-03 9:07 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 9:07 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, devicetree, linux-media, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The C plane buffer size (`vref->c.size`) is completely missing initialization for reference frames, leaving it as zero in the shared hardware structure.
--
commit a2c9dc4836cf2314c45a5c3f24634d8cb307be25
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: decoder: fill av1 buffer size with picinfo
The buffer size of y and c plane has been calculated in vcp/scp,
and this commit fills each frame buffer size directly using picinfo.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> @@ -1811,18 +1811,19 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
> {
> struct vb2_buffer *vb;
> struct vb2_queue *vq;
> - int w, h, plane, size;
> + int plane;
> int i;
>
> plane = instance->ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes;
> - w = vsi->frame.uh.upscaled_width;
> - h = vsi->frame.uh.frame_height;
> - size = ALIGN(w, VCODEC_DEC_ALIGNED_64) * ALIGN(h, VCODEC_DEC_ALIGNED_64);
>
> /* frame buffer */
> vsi->fb.y.dma_addr = fb->base_y.dma_addr;
> +
> + vsi->fb.y.size = instance->ctx->picinfo.fb_sz[0];
> + vsi->fb.c.size = instance->ctx->picinfo.fb_sz[1];
> +
> if (plane == 1)
> - vsi->fb.c.dma_addr = fb->base_y.dma_addr + size;
> + vsi->fb.c.dma_addr = fb->base_y.dma_addr + vsi->fb.y.size;
> else
> vsi->fb.c.dma_addr = fb->base_c.dma_addr;
[ ... ]
> @@ -1845,8 +1846,9 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
> }
>
> vref->y.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> + vref->y.size = vsi->fb.y.size;
> if (plane == 1)
> - vref->c.dma_addr = vref->y.dma_addr + size;
> + vref->c.dma_addr = vref->y.dma_addr + vsi->fb.y.size;
> else
> vref->c.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
> }
[Severity: High]
Are we missing an assignment for vref->c.size here?
Earlier in vdec_av1_slice_setup_core_buffer(), vsi->fb.c.size is explicitly
initialized, but here for the reference frames only vref->y.size is assigned.
Because the underlying vsi structure is backed by zero-initialized memory,
the C plane size for all reference frames will be left as zero.
Could this missing initialization cause visual corruption or trigger a
coprocessor fault when the firmware uses these reference buffer sizes for DMA
configuration and bounds checking?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=13
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 14/14] media: mediatek: decoder: support av1 extend vsi
2026-06-03 8:40 [PATCH v5 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (12 preceding siblings ...)
2026-06-03 8:40 ` [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
@ 2026-06-03 8:40 ` Kyrie Wu
2026-06-03 9:10 ` sashiko-bot
13 siblings, 1 reply; 28+ messages in thread
From: Kyrie Wu @ 2026-06-03 8:40 UTC (permalink / raw)
To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Nicolas Dufresne, Ricardo Ribalda,
Kees Cook, Hans Verkuil, Haoxiang Li, Fei Shao, Chen-Yu Tsai,
Laurent Pinchart, Sebastian Fricke, Benjamin Gaignard,
Philipp Zabel, Qianfeng Rong, Jacopo Mondi, Irui Wang, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Kyrie Wu
Cc: Sakari Ailus, Tzung-Bi Shih, Tomasz Figa
From: Yunfei Dong <yunfei.dong@mediatek.com>
The driver can't access tile buffer address for extend architecture,
set tile group information in vcp and share it with kernel.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 59 ++++++++++++++++---
1 file changed, 52 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
index a0c7e89b8ae4..e9265b112bfb 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
@@ -25,6 +25,9 @@
#define AV1_INVALID_IDX -1
+#define AV1_NON_EXT_VSI_SIZE 0xD50
+#define AV1_TILE_SIZE 64
+
#define AV1_DIV_ROUND_UP_POW2(value, n) \
({ \
typeof(n) _n = n; \
@@ -641,6 +644,8 @@ struct vdec_av1_slice_fb {
* @frame: current frame info
* @state: status after decode done
* @cur_lst_tile_id: tile id for large scale
+ * @tile_group: tile group info
+ * @reserved: reserved
*/
struct vdec_av1_slice_vsi {
/* lat */
@@ -665,6 +670,8 @@ struct vdec_av1_slice_vsi {
struct vdec_av1_slice_frame frame;
struct vdec_av1_slice_state state;
u32 cur_lst_tile_id;
+ struct vdec_av1_slice_tile_group tile_group;
+ unsigned int reserved[4];
};
/**
@@ -1402,17 +1409,29 @@ static void vdec_av1_slice_setup_uh(struct vdec_av1_slice_instance *instance,
vdec_av1_slice_setup_tile(frame, &ctrl_fh->tile_info);
}
+static
+struct vdec_av1_slice_tile_group *vdec_av1_get_tile_group(struct vdec_av1_slice_instance *instance,
+ struct vdec_av1_slice_vsi *vsi)
+{
+ if (IS_VDEC_SUPPORT_EXT(instance->ctx->dev->dec_capability))
+ return &vsi->tile_group;
+ else
+ return &instance->tile_group;
+}
+
static int vdec_av1_slice_setup_tile_group(struct vdec_av1_slice_instance *instance,
struct vdec_av1_slice_vsi *vsi)
{
struct v4l2_ctrl_av1_tile_group_entry *ctrl_tge;
- struct vdec_av1_slice_tile_group *tile_group = &instance->tile_group;
+ struct vdec_av1_slice_tile_group *tile_group;
struct vdec_av1_slice_uncompressed_header *uh = &vsi->frame.uh;
struct vdec_av1_slice_tile *tile = &uh->tile;
struct v4l2_ctrl *ctrl;
u32 tge_size;
int i;
+ tile_group = vdec_av1_get_tile_group(instance, vsi);
+
ctrl = v4l2_ctrl_find(&instance->ctx->ctrl_hdl, V4L2_CID_STATELESS_AV1_TILE_GROUP_ENTRY);
if (!ctrl)
return -EINVAL;
@@ -1607,6 +1626,15 @@ static int vdec_av1_slice_setup_pfc(struct vdec_av1_slice_instance *instance,
return ret;
}
+static u32 vdec_av1_get_tiles_num(struct vdec_av1_slice_instance *instance,
+ struct vdec_av1_slice_vsi *vsi)
+{
+ if (IS_VDEC_SUPPORT_EXT(instance->ctx->dev->dec_capability))
+ return vsi->tile_group.num_tiles;
+ else
+ return instance->tile_group.num_tiles;
+}
+
static void vdec_av1_slice_setup_lat_buffer(struct vdec_av1_slice_instance *instance,
struct vdec_av1_slice_vsi *vsi,
struct mtk_vcodec_mem *bs,
@@ -1647,12 +1675,18 @@ static void vdec_av1_slice_setup_lat_buffer(struct vdec_av1_slice_instance *inst
vsi->tile.buf = instance->tile.dma_addr;
vsi->tile.size = instance->tile.size;
- memcpy(lat_buf->tile_addr.va, instance->tile.va, 64 * instance->tile_group.num_tiles);
vsi->cdf_table.buf = instance->cdf_table.dma_addr;
vsi->cdf_table.size = instance->cdf_table.size;
vsi->iq_table.buf = instance->iq_table.dma_addr;
vsi->iq_table.size = instance->iq_table.size;
+
+ /* lat_buf is used to share hardware decoder syntax between lat and core,
+ * there isn't only one. But there is only one tile.va for each instance.
+ * Need to copy tile information to lat_buf every time.
+ */
+ memcpy(lat_buf->tile_addr.va, instance->tile.va,
+ AV1_TILE_SIZE * vdec_av1_get_tiles_num(instance, vsi));
}
static void vdec_av1_slice_setup_seg_buffer(struct vdec_av1_slice_instance *instance,
@@ -1675,7 +1709,7 @@ static void vdec_av1_slice_setup_tile_buffer(struct vdec_av1_slice_instance *ins
struct vdec_av1_slice_vsi *vsi,
struct mtk_vcodec_mem *bs)
{
- struct vdec_av1_slice_tile_group *tile_group = &instance->tile_group;
+ struct vdec_av1_slice_tile_group *tile_group;
struct vdec_av1_slice_uncompressed_header *uh = &vsi->frame.uh;
struct vdec_av1_slice_tile *tile = &uh->tile;
u32 tile_num, tile_row, tile_col;
@@ -1686,6 +1720,8 @@ static void vdec_av1_slice_setup_tile_buffer(struct vdec_av1_slice_instance *ins
u32 *tile_info_buf = instance->tile.va;
u64 pa = (u64)bs->dma_addr;
+ tile_group = vdec_av1_get_tile_group(instance, vsi);
+
if (uh->disable_cdf_update == 0)
allow_update_cdf = 1;
@@ -1907,7 +1943,7 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
struct vdec_av1_slice_instance *instance;
struct vdec_av1_slice_init_vsi *vsi;
enum mtk_vcodec_fw_type fw_type = ctx->dev->fw_handler->type;
- int ret;
+ int ret, vsi_size = AV1_NON_EXT_VSI_SIZE;
instance = kzalloc_obj(*instance);
if (!instance)
@@ -1941,9 +1977,18 @@ static int vdec_av1_slice_init(struct mtk_vcodec_dec_ctx *ctx)
goto error_vsi;
}
- if (vsi->vsi_size != sizeof(struct vdec_av1_slice_vsi))
- mtk_vdec_err(ctx, "remote vsi size 0x%x mismatch! expected: 0x%zx\n",
- vsi->vsi_size, sizeof(struct vdec_av1_slice_vsi));
+ if (IS_VDEC_SUPPORT_EXT(ctx->dev->dec_capability)) {
+ vsi_size = sizeof(struct vdec_av1_slice_vsi);
+ vsi->iq_table_size = AV1_IQ_TABLE_SIZE;
+ vsi->cdf_table_size = AV1_CDF_SIZE;
+ }
+
+ if (vsi->vsi_size != vsi_size) {
+ mtk_vdec_err(ctx, "remote vsi size 0x%x mismatch! expected: 0x%x\n",
+ vsi->vsi_size, vsi_size);
+ ret = -EINVAL;
+ goto error_vsi;
+ }
instance->irq_enabled = 1;
instance->inneracing_mode = IS_VDEC_INNER_RACING(instance->ctx->dev->dec_capability);
--
2.45.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 14/14] media: mediatek: decoder: support av1 extend vsi
2026-06-03 8:40 ` [PATCH v5 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
@ 2026-06-03 9:10 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-06-03 9:10 UTC (permalink / raw)
To: Kyrie Wu; +Cc: linux-media, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Out-of-bounds write to VPU shared memory in `vdec_av1_slice_vsi_to_remote()` due to unconditional struct memcpy.
Pre-existing issues:
- [High] Missing bounds check on `tile_cols` and `tile_rows` leads to heap buffer overflow during AV1 tile setup.
--
commit 704ba9a36ee79563029e1b285c390dcbc873258d
Author: Yunfei Dong <yunfei.dong@mediatek.com>
media: mediatek: decoder: support av1 extend vsi
The driver can't access tile buffer address for extend architecture,
set tile group information in vcp and share it with kernel.
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> index a0c7e89b8ae4d..e9265b112bfbe 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
[ ... ]
> @@ -665,6 +670,8 @@ struct vdec_av1_slice_vsi {
> struct vdec_av1_slice_frame frame;
> struct vdec_av1_slice_state state;
> u32 cur_lst_tile_id;
> + struct vdec_av1_slice_tile_group tile_group;
> + unsigned int reserved[4];
> };
[Severity: High]
Does this struct size increase cause an out-of-bounds write in
vdec_av1_slice_vsi_to_remote()?
Since vdec_av1_slice_vsi_to_remote() unconditionally uses sizeof(*vsi):
static inline void vdec_av1_slice_vsi_to_remote(struct vdec_av1_slice_vsi *vsi,
struct vdec_av1_slice_vsi *remote_vsi)
{
memcpy(remote_vsi, vsi, sizeof(*vsi));
}
and this patch adds the large tile_group structure to it, won't this copy the
new larger size?
For non-EXT devices, the remote firmware buffer remains AV1_NON_EXT_VSI_SIZE
(0xD50), so won't this write past the end of the VPU shared memory?
[ ... ]
> @@ -1402,17 +1409,29 @@ static void vdec_av1_slice_setup_uh(struct vdec_av1_slice_instance *instance,
> vdec_av1_slice_setup_tile(frame, &ctrl_fh->tile_info);
> }
[Severity: High]
This is a pre-existing issue, but does vdec_av1_slice_setup_tile()
lack bounds checking for tile->tile_cols and tile->tile_rows?
vdec_av1_slice_setup_tile() reads ctrl_tile->tile_cols and
ctrl_tile->tile_rows from userspace control without validation:
static void vdec_av1_slice_setup_tile(struct vdec_av1_slice_frame *frame,
struct v4l2_av1_tile_info *ctrl_tile)
{
...
tile->tile_cols = ctrl_tile->tile_cols;
tile->tile_rows = ctrl_tile->tile_rows;
...
for (i = 0; i < tile->tile_cols + 1; i++)
tile->mi_col_starts[i] =
ALIGN(ctrl_tile->mi_col_starts[i], BIT(mib_size_log2)) >> mib_size_log2;
for (i = 0; i < tile->tile_rows + 1; i++)
tile->mi_row_starts[i] =
ALIGN(ctrl_tile->mi_row_starts[i], BIT(mib_size_log2)) >> mib_size_log2;
}
Since mi_col_starts and mi_row_starts are fixed-size arrays (65 elements) in
struct vdec_av1_slice_tile, could a malicious application supply values
greater than 64 and cause a heap buffer overflow when copying from
ctrl_tile?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=14
^ permalink raw reply [flat|nested] 28+ messages in thread