* [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:36 ` sashiko-bot
2026-07-20 12:57 ` Nicolas Dufresne
2026-07-20 1:20 ` [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
` (12 subsequent siblings)
13 siblings, 2 replies; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Some platforms expose the video codec through the VCP coprocessor.
Use the VCP architecture when the VCP coprocessor is found.
Signed-off-by: Kyrie Wu <kyrie.wu@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 300363a40..c1642fb09 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 e936ed8df..d220b645e 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture
2026-07-20 1:20 ` [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
@ 2026-07-20 1:36 ` sashiko-bot
2026-07-20 12:57 ` Nicolas Dufresne
1 sibling, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, media-ci, devicetree, robh
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] The patch adds support for the VCP architecture but fails to handle the VCP enum in mtk_vcodec_fw_select(), resulting in guaranteed probe failure.
Pre-existing issues:
- [Critical] The driver destroys decode_workqueue before unregistering the video device, allowing concurrent ioctls to queue work to a destroyed workqueue.
- [Critical] The driver state structure (dev) is managed by devm_kzalloc but is accessed by file operations that outlive the device binding, leading to a Use-After-Free.
- [High] The video device allocated via video_device_alloc() leaks on multiple probe error paths.
- [High] Child platform devices created by of_platform_populate() are never cleaned up.
- [High] The ordered workqueue 'dev->core_workqueue' leaks in mtk_vcodec_dec_remove().
--
--- Patch [1]: [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture ---
commit 86e187a5e23e9a4483af39ff82e14ad66126f95a
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: vcodec: support vcp architecture
This commit adds support for the VCP architecture for the video codec on
some platforms. It detects the VCP coprocessor in the device tree and sets
the VCP architecture type.
> 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 300363a40158c..c1642fb09b42a 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 e936ed8dffbaf..d220b645e4556 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;
[Severity: High]
If the mediatek,vcp property is present, probe sets fw_type = VCP and later
calls mtk_vcodec_fw_select(). However, since mtk_vcodec_fw_select() does not
have a case for VCP, won't it hit the default case and return ERR_PTR(-EINVAL),
causing probe to unconditionally fail?
mtk_vcodec_fw.c:mtk_vcodec_fw_select() {
...
default:
dev_err(&plat_dev->dev, "Invalid vcodec fw type");
return ERR_PTR(-EINVAL);
}
[Severity: High]
This is a pre-existing issue, but does the video device allocated by
video_device_alloc() leak on multiple error paths in mtk_vcodec_probe()?
If initialization steps like v4l2_m2m_init(), alloc_ordered_workqueue(), or
of_platform_populate() fail, the code jumps to error labels like
err_dec_alloc, err_event_workq, or err_reg_cont, which skip calling
video_device_release(vfd_dec):
err_dec_alloc:
v4l2_device_unregister(&dev->v4l2_dev);
[Severity: High]
This isn't a bug introduced by this patch, but are the child platform devices
created by of_platform_populate() in mtk_vcodec_probe() ever cleaned up?
If probe fails after of_platform_populate(), or when the driver is unbound,
mtk_vcodec_dec_remove() executes but appears to never call
of_platform_depopulate() to unregister the child devices. Can this cause state
corruption and resource leaks on subsequent probes?
[Severity: High]
This is a pre-existing issue, but does the ordered workqueue
dev->core_workqueue leak when the driver is unbound?
In mtk_vcodec_probe(), alloc_ordered_workqueue() allocates dev->core_workqueue
for the VDEC_LAT architecture. However, mtk_vcodec_dec_remove() destroys
decode_workqueue but seems to completely omit destroying core_workqueue:
mtk_vcodec_dec_drv.c:mtk_vcodec_dec_remove() {
...
destroy_workqueue(dev->decode_workqueue);
...
}
[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free window during
driver removal?
In mtk_vcodec_dec_remove(), destroy_workqueue(dev->decode_workqueue) is
executed before video_unregister_device(dev->vfd_dec). During this window,
could userspace invoke V4L2 ioctls that call m2mops_vdec_device_run(), which
executes queue_work() on the already freed workqueue?
mtk_vcodec_dec.c:m2mops_vdec_device_run() {
...
queue_work(dev->decode_workqueue, &ctx->decode_work);
}
[Severity: Critical]
This is a pre-existing issue, but can the dev state structure be accessed
after it is freed by devres?
The driver state dev is allocated with devm_kzalloc in mtk_vcodec_probe() and
is automatically freed when mtk_vcodec_dec_remove() returns. If a process
holds /dev/videoX open during unbind, the file descriptor remains valid.
When the application later closes it, fops_vcodec_release() executes and
dereferences the freed dev pointer to acquire dev->dev_mutex:
mtk_vcodec_dec_drv.c:fops_vcodec_release() {
struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
...
mutex_lock(&dev->dev_mutex);
}
> } else {
> dev_dbg(&pdev->dev, "Could not get vdec IPI device");
> return -ENODEV;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=1
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture
2026-07-20 1:20 ` [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
2026-07-20 1:36 ` sashiko-bot
@ 2026-07-20 12:57 ` Nicolas Dufresne
1 sibling, 0 replies; 32+ messages in thread
From: Nicolas Dufresne @ 2026-07-20 12:57 UTC (permalink / raw)
To: Kyrie Wu, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]
Hi,
Le lundi 20 juillet 2026 à 09:20 +0800, Kyrie Wu a écrit :
> Some platforms expose the video codec through the VCP coprocessor.
> Use the VCP architecture when the VCP coprocessor is found.
>
> Signed-off-by: Kyrie Wu <kyrie.wu@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 300363a40..c1642fb09 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 e936ed8df..d220b645e 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;
I know I gave an rb a while ago, but then we reviewed the 8189 code, and we
underlined that this makes little sense, and that fw_type should simply be set
in the pdata. Same applies to mtk_vcodec_dec_get_chip_name(), the chip "name",
or integer form of, is clearly a 1 to 1 match for pdata.
Nicolas
> } else {
> dev_dbg(&pdev->dev, "Could not get vdec IPI device");
> return -ENODEV;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:36 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
` (11 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
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: Kyrie Wu <kyrie.wu@mediatek.com>
---
.../media/platform/mediatek/vcodec/Kconfig | 4 +
.../platform/mediatek/vcodec/common/Makefile | 4 +
.../mediatek/vcodec/common/mtk_vcodec_fw.c | 23 +-
.../mediatek/vcodec/common/mtk_vcodec_fw.h | 6 +-
.../vcodec/common/mtk_vcodec_fw_priv.h | 12 +
.../vcodec/common/mtk_vcodec_fw_scp.c | 1 +
.../vcodec/common/mtk_vcodec_fw_vcp.c | 571 ++++++++++++++++++
.../vcodec/common/mtk_vcodec_fw_vcp.h | 152 +++++
.../vcodec/common/mtk_vcodec_fw_vpu.c | 1 +
.../vcodec/decoder/mtk_vcodec_dec_drv.c | 5 +-
.../vcodec/decoder/mtk_vcodec_dec_drv.h | 2 +
.../vcodec/encoder/mtk_vcodec_enc_drv.c | 4 +-
.../vcodec/encoder/mtk_vcodec_enc_drv.h | 2 +
13 files changed, 768 insertions(+), 19 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 bc8292232..d23dad5c7 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 d0479914d..2f68692e8 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 08949b08f..552d0d8a8 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
@@ -4,25 +4,18 @@
#include "../encoder/mtk_vcodec_enc_drv.h"
#include "mtk_vcodec_fw_priv.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 mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_use fw_use,
+ mtk_vcodec_fw_init_func fw_init)
{
- struct platform_device *plat_dev;
-
- 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;
+ if (!fw_init)
+ return ERR_PTR(-EINVAL);
- 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");
+ if (fw_use != ENCODER && fw_use != DECODER) {
+ pr_err("Invalid firmware use %d\n", fw_use);
return ERR_PTR(-EINVAL);
}
+
+ return fw_init(priv, fw_use);
}
EXPORT_SYMBOL_GPL(mtk_vcodec_fw_select);
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 c1642fb09..50d93d47b 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
@@ -24,11 +24,13 @@ enum mtk_vcodec_fw_use {
struct mtk_vcodec_fw;
+typedef struct mtk_vcodec_fw *(*mtk_vcodec_fw_init_func)(void *priv,
+ enum mtk_vcodec_fw_use fw_use);
typedef void (*mtk_vcodec_ipi_handler) (void *data,
unsigned int len, void *priv);
-struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type type,
- enum mtk_vcodec_fw_use fw_use);
+struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_use fw_use,
+ mtk_vcodec_fw_init_func fw_init);
void mtk_vcodec_fw_release(struct mtk_vcodec_fw *fw);
int mtk_vcodec_fw_load_firmware(struct mtk_vcodec_fw *fw);
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 99603accd..0a2a9b010 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 1b0bc4735..1aad5c3ef 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 000000000..150d842c7
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
@@ -0,0 +1,571 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ * Author: Kyrie Wu <kyrie.wu@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 bool mtk_vcodec_vcp_ipi_id_valid(unsigned int ipi_id)
+{
+ return ipi_id < VCP_IPI_MAX;
+}
+
+static void mtk_vcodec_vcp_ipi_lock(struct mtk_vcp *vcp, u32 ipi_id)
+{
+ mutex_lock(&vcp->ipi_desc[ipi_id].lock);
+}
+
+static void mtk_vcodec_vcp_ipi_unlock(struct mtk_vcp *vcp, u32 ipi_id)
+{
+ lockdep_assert_held(&vcp->ipi_desc[ipi_id].lock);
+ mutex_unlock(&vcp->ipi_desc[ipi_id].lock);
+}
+
+static void mtk_vcodec_vcp_msq_queue_lock(struct mtk_vcodec_fw *fw, unsigned long *flags)
+{
+ spin_lock_irqsave(&fw->vcp->msg_queue.lock, *flags);
+}
+
+static void mtk_vcodec_vcp_msq_queue_unlock(struct mtk_vcodec_fw *fw, unsigned long *flags)
+{
+ spin_unlock_irqrestore(&fw->vcp->msg_queue.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(!mtk_vcodec_vcp_ipi_id_valid(ipi_id)) || 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 ||
+ kthread_should_stop());
+ 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;
+ }
+ if (kthread_should_stop())
+ break;
+
+ 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);
+
+ if (!mtk_vcodec_vcp_ipi_id_valid(obj->id)) {
+ dev_err(&fw->pdev->dev, "invalid ack ipi id %u\n", obj->id);
+ mtk_vcodec_vcp_free_msg_node(fw, msg_node);
+ continue;
+ }
+
+ 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);
+ continue;
+ }
+
+ 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;
+
+ if (!mtk_vcodec_vcp_ipi_id_valid(obj->id)) {
+ dev_err(&fw->pdev->dev, "invalid ack ipi id %u\n", obj->id);
+ return -EINVAL;
+ }
+
+ 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;
+ wait_queue_head_t *msg_wq;
+ 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 (id < 0 || !mtk_vcodec_vcp_ipi_id_valid(id)) {
+ dev_err(&fw->pdev->dev, "invalid ipi id %d\n", id);
+ return -EINVAL;
+ }
+
+ msg_signaled = &vcp->msg_signaled[id];
+ msg_wq = &vcp->msg_wq[id];
+
+ 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(vcp_device, feature_id)) {
+ if (time_after(jiffies, timeout_jiffies)) {
+ vcp->ipi_id_ack[id] = -EINVAL;
+ ret = -EINVAL;
+ goto error;
+ }
+ usleep_range(1000, 2000);
+ }
+
+ 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]);
+
+ *msg_signaled = false;
+ vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_OK;
+
+ 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_interruptible_timeout(*msg_wq, *msg_signaled,
+ msecs_to_jiffies(IPI_TIMEOUT_MS));
+ if (!ret) {
+ 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) {
+ if (retry_cnt++ < 5)
+ goto wait_ack;
+
+ dev_err(&fw->pdev->dev, "wait ipi ack err (%d)\n", vcp->ipi_id_ack[id]);
+ vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
+ } 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]);
+
+ 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 bool mtk_vcodec_vcp_driver_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_dbg(dev, "find mtk-vcp driver failed, need to reload.");
+ return false;
+ }
+
+ return true;
+}
+
+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(mtk_vcodec_vcp_driver_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 void mtk_vcodec_vcp_put_device(struct mtk_vcodec_fw *fw)
+{
+ if (!fw->vcp->vcp_device)
+ return;
+
+ rproc_put(fw->vcp->vcp_device->rproc);
+ fw->vcp->vcp_device = NULL;
+}
+
+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;
+ int i;
+
+ 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);
+ ret = -EINVAL;
+ goto err_put_device;
+ }
+ fw->vcp->feature_id = feature_id;
+ fw->vcp->mailbox_id = mailbox_id;
+ fw->vcp->is_ipi_registered = true;
+
+ fw->vcp->vcp_notify.notifier_call = mtk_vcodec_vcp_notifier;
+ fw->vcp->vcp_notify.priority = 1;
+ vcp_device->ops->register_notify(vcp_device, feature_id, &fw->vcp->vcp_notify);
+ fw->vcp->is_notify_registered = true;
+
+ if (!fw->vcp->is_register_done) {
+ ret = vcp_device->ops->register_feature(vcp_device, feature_id);
+ if (ret < 0) {
+ dev_err(&fw->pdev->dev, "%d register to vcp fail(%d)\n", feature_id, ret);
+ ret = -EINVAL;
+ goto err_unregister_notify;
+ }
+
+ fw->vcp->is_register_done = true;
+ }
+
+ fw->vcp->is_init_done = true;
+
+ for (i = 0; i < VCP_IPI_MAX; i++)
+ mutex_init(&fw->vcp->ipi_desc[i].lock);
+ mutex_init(&fw->vcp->ipi_mutex);
+
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_LAT_DECODER]);
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_CORE_DECODER]);
+ fw->vcp->msg_thread =
+ kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_vdec_msq_thread");
+ if (IS_ERR(fw->vcp->msg_thread)) {
+ ret = PTR_ERR(fw->vcp->msg_thread);
+ fw->vcp->msg_thread = NULL;
+ goto err_deregister_feature;
+ }
+
+ fw->vcp->vsi_addr = vcp_device->ops->get_mem_virt(vcp_device, mem_id);
+ fw->vcp->vsi_core_addr = fw->vcp->vsi_addr + VCODEC_VSI_LEN;
+ fw->vcp->vsi_size = vcp_device->ops->get_mem_size(vcp_device, mem_id);
+ fw->vcp->iova_addr = vcp_device->ops->get_mem_iova(vcp_device, mem_id);
+
+ 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;
+
+err_deregister_feature:
+ if (fw->vcp->is_register_done) {
+ vcp_device->ops->deregister_feature(vcp_device, feature_id);
+ fw->vcp->is_register_done = false;
+ }
+err_unregister_notify:
+ if (fw->vcp->is_notify_registered) {
+ vcp_device->ops->unregister_notify(vcp_device, feature_id,
+ &fw->vcp->vcp_notify);
+ fw->vcp->is_notify_registered = false;
+ }
+ if (fw->vcp->is_ipi_registered) {
+ mtk_vcp_mbox_ipc_unregister(vcp_get_ipidev(vcp_device), mailbox_id);
+ fw->vcp->is_ipi_registered = false;
+ }
+ fw->vcp->is_init_done = false;
+
+err_put_device:
+ mtk_vcodec_vcp_put_device(fw);
+ return ret;
+}
+
+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;
+
+ if (!fw->vcp->vcp_device) {
+ dev_err(dev, "vcp device is null\n");
+ return;
+ }
+
+ if (fw->vcp->is_ipi_registered) {
+ mtk_vcp_mbox_ipc_unregister(vcp_get_ipidev(vcp_device), fw->vcp->mailbox_id);
+ fw->vcp->is_ipi_registered = false;
+ }
+
+ if (fw->vcp->msg_thread) {
+ kthread_stop(fw->vcp->msg_thread);
+ fw->vcp->msg_thread = NULL;
+ }
+
+ if (fw->vcp->is_notify_registered) {
+ vcp_device->ops->unregister_notify(vcp_device, fw->vcp->feature_id,
+ &fw->vcp->vcp_notify);
+ fw->vcp->is_notify_registered = false;
+ }
+
+ if (!fw->vcp->is_register_done) {
+ fw->vcp->is_init_done = false;
+ goto put_device;
+ }
+
+ ret = vcp_device->ops->deregister_feature(vcp_device, fw->vcp->feature_id);
+ if (ret < 0) {
+ dev_err(dev, "deregister feature_id(%d) fail(%d)\n", fw->vcp->feature_id, ret);
+ return;
+ }
+
+ fw->vcp->is_register_done = false;
+ fw->vcp->is_init_done = false;
+
+put_device:
+ mtk_vcodec_vcp_put_device(fw);
+}
+
+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 000000000..83742096e
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
@@ -0,0 +1,152 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ * Author: Kyrie Wu <kyrie.wu@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
+ * @msg_thread: process VCP message queue
+ *
+ * @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
+ * @feature_id: registered VCP feature id
+ * @mailbox_id: registered VCP mailbox id
+ * @is_register_done: feature registration state
+ * @is_ipi_registered: mailbox IPC registration state
+ * @is_notify_registered: notifier registration state
+ */
+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];
+ int 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;
+ struct task_struct *msg_thread;
+
+ 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;
+ int feature_id;
+ int mailbox_id;
+ bool is_register_done;
+ bool is_ipi_registered;
+ bool is_notify_registered;
+};
+
+#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 3632037f7..41643db9f 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 d220b645e..ab5f6c01f 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,19 +376,22 @@ 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;
}
dma_set_max_seg_size(&pdev->dev, UINT_MAX);
- dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, DECODER);
+ dev->fw_handler = mtk_vcodec_fw_select(dev, DECODER, dev->fw_init);
if (IS_ERR(dev->fw_handler))
return PTR_ERR(dev->fw_handler);
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 c9d27534c..15e437323 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 4e4541b2f..811bc62a1 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,16 +259,18 @@ 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;
}
dma_set_max_seg_size(&pdev->dev, UINT_MAX);
- dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, ENCODER);
+ dev->fw_handler = mtk_vcodec_fw_select(dev, ENCODER, dev->fw_init);
if (IS_ERR(dev->fw_handler))
return PTR_ERR(dev->fw_handler);
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 0cddfa135..934ff6481 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp
2026-07-20 1:20 ` [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
@ 2026-07-20 1:36 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, robh, devicetree, media-ci
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Destructive `request_module` polling loop causes a massive modprobe fork bomb and breaks deferred probe.
- [High] A timeout in `mtk_vcodec_vcp_msg_ipi_send()` leaves the system vulnerable to late ACKs, causing host-firmware state desynchronization.
- [High] `ipi_mutex` serializes the entire `wait_event_interruptible_timeout()`, breaking IPI concurrency and risking deadlocks.
- [Medium] Early return in `mtk_vcodec_vcp_release()` leaks remoteproc device reference and breaks teardown state.
- [Medium] Missing IPC message length validation enables potential out-of-bounds reads from untrusted firmware.
- [Low] Unused variable `fw_type` in encoder and decoder probe functions.
--
--- Patch [2]: [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp ---
commit 06d72e77d54f36511b46df20abe564b636b8dc79
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: vcodec: add driver to support vcp
This patch changes the processor from scp to vcp for the mt8196 platform.
It adds a new firmware interface to handle communication between the kernel
and the vcp.
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
[ ... ]
> +static int mtk_vcodec_vcp_msg_process_thread(void *arg)
> +{
[ ... ]
> + 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);
> + continue;
> + }
> +
> + handler(msg, obj->len, fw->vcp->ipi_desc[obj->id].priv);
[Severity: Medium]
Could this pass an arbitrarily large length to the handler?
The firmware-provided obj->len does not appear to be validated against the
boundaries of VCP_SHARE_BUF_SIZE (64 bytes). If the firmware provides an
out-of-bounds length, it might lead to out-of-bounds reads in the receiving
handlers.
> + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
[ ... ]
> +static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
> + unsigned int len, unsigned int wait)
> +{
[ ... ]
> + mutex_lock(&vcp->ipi_mutex);
> + feature_id = VDEC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VDEC_1;
[ ... ]
> +wait_ack:
> + /* wait for VCP's ACK */
> + ret = wait_event_interruptible_timeout(*msg_wq, *msg_signaled,
> + msecs_to_jiffies(IPI_TIMEOUT_MS));
[Severity: High]
Does holding ipi_mutex across this sleep serialize all VCP communication?
Acquiring the mutex before sending the IPC message and holding it while
waiting for the ACK seems to single-thread all communication, which might
defeat the purpose of the per-ID waitqueues.
Also, if any registered IPI handler executing in the receiving msg_thread
needs to send a followup command via mtk_vcodec_vcp_msg_ipi_send(), could
it deadlock trying to acquire ipi_mutex while this thread holds it?
> + if (!ret) {
> + 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]);
[Severity: High]
Can a timeout here lead to state desynchronization with the firmware?
If the wait times out, the function returns an error and releases ipi_mutex
without invalidating the transaction (e.g., via a sequence number).
If the firmware later completes the abandoned task and sends a delayed ACK,
msg_thread will set msg_signaled[id] = true and call wake_up(). If another
thread subsequently reuses this id for a new command, could it be prematurely
awakened by the stale ACK, causing the kernel to reuse or free memory buffers
while the firmware is still accessing them?
[ ... ]
> +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(mtk_vcodec_vcp_driver_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]
Could this polling loop cause a modprobe fork bomb?
If the mtk-vcp module is missing or unable to load, this loop executes up to
10,000 times, synchronously calling request_module() on each iteration. This
will spawn an excessive number of user-space modprobe processes.
[ ... ]
> +static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> +{
[ ... ]
> + if (mtk_vcodec_vcp_get_vcp_device(fw) < 0) {
> + dev_err(&fw->pdev->dev, "vcp device is null.\n");
> + return -EINVAL;
> + }
[Severity: High]
Is it intentional to overwrite the -EPROBE_DEFER return value from
mtk_vcodec_vcp_get_vcp_device() with -EINVAL here? This will permanently
break the deferred probe mechanism for this device if the module is simply
not loaded yet.
[ ... ]
> +static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> +{
[ ... ]
> + ret = vcp_device->ops->deregister_feature(vcp_device, fw->vcp->feature_id);
> + if (ret < 0) {
> + dev_err(dev, "deregister feature_id(%d) fail(%d)\n", fw->vcp->feature_id, ret);
> + return;
> + }
> +
> + fw->vcp->is_register_done = false;
> + fw->vcp->is_init_done = false;
> +
> +put_device:
> + mtk_vcodec_vcp_put_device(fw);
> +}
[Severity: Medium]
Will this early return leak the remoteproc device reference?
If deregister_feature fails, returning early skips mtk_vcodec_vcp_put_device(),
which leaves the rproc reference elevated and prevents the remoteproc module
from being cleanly unloaded.
> --- 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,19 +376,22 @@ 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 {
[Severity: Low]
It looks like the fw_type variable is assigned here, but is never used anywhere
else in the function since the firmware selection was refactored to use
fw_init. Is there any remaining need for fw_type?
> --- 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,16 +259,18 @@ 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;
[Severity: Low]
Similarly, fw_type appears to be dead code here as well, since it is no longer
passed to mtk_vcodec_fw_select().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=2
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 01/14] media: mediatek: vcodec: support vcp architecture Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 02/14] media: mediatek: vcodec: add driver to support vcp Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:45 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id Kyrie Wu
` (10 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Encoder also need to call vcp interface to communicate with vcp,
add driver to support encoder.
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
---
.../vcodec/common/mtk_vcodec_fw_vcp.c | 52 ++++++++++++++++---
.../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, 47 insertions(+), 9 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 150d842c7..ece4e3e87 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
@@ -223,8 +225,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(vcp_device, feature_id)) {
@@ -348,7 +355,7 @@ static void mtk_vcodec_vcp_put_device(struct mtk_vcodec_fw *fw)
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;
+ int ret, feature_id, mem_id, mailbox_id;
int i;
if (fw->vcp->is_init_done) {
@@ -363,10 +370,15 @@ 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;
+ } else {
+ feature_id = VDEC_FEATURE_ID;
+ mem_id = VDEC_MEM_ID;
+ mailbox_id = IPI_IN_VDEC_1;
+ }
ret = mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
mtk_vcodec_vcp_msg_ack_isr, fw, &fw->vcp->share_data);
@@ -402,6 +414,26 @@ static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
mutex_init(&fw->vcp->ipi_desc[i].lock);
mutex_init(&fw->vcp->ipi_mutex);
+ if (fw->fw_use == ENCODER) {
+ init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_ENCODER]);
+ fw->vcp->msg_thread =
+ kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_enc_msq_thread");
+ if (IS_ERR(fw->vcp->msg_thread)) {
+ ret = PTR_ERR(fw->vcp->msg_thread);
+ fw->vcp->msg_thread = NULL;
+ goto err_deregister_feature;
+ }
+
+ fw->vcp->vsi_addr = vcp_device->ops->get_mem_virt(vcp_device, mem_id);
+ fw->vcp->vsi_size = vcp_device->ops->get_mem_size(vcp_device, mem_id);
+ fw->vcp->iova_addr = vcp_device->ops->get_mem_iova(vcp_device, 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);
+
+ return 0;
+ }
+
init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_LAT_DECODER]);
init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_CORE_DECODER]);
fw->vcp->msg_thread =
@@ -530,7 +562,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 83742096e..97ba642dc 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 0d4e94463..48cb5dded 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 908d8179b..84156c102 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder
2026-07-20 1:20 ` [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
@ 2026-07-20 1:45 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:45 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, media-ci, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `.get_venc_capa` callback in `mtk_vcodec_vcp_msg` causes a guaranteed kernel panic on encoder device open.
- [High] Late IPI acknowledgments from the firmware can falsely satisfy subsequent IPI requests after a timeout, leading to data corruption or desynchronization.
- [Medium] Early return on `deregister_feature` failure leaks the `rproc` remote processor reference.
--
--- Patch [3]: [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder ---
commit 9d12e27cbb34067860fe1673f4f8bfcc5549ebf1
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: add driver to support vcp encoder
This patch adds VCP encoder support to the mediatek vcodec driver. It enables
the encoder to call the VCP interface to communicate with the coprocessor.
> 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 150d842c7d226..ece4e3e87e62d 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
[ ... ]
> @@ -223,8 +225,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(vcp_device, feature_id)) {
[Severity: High]
Can late IPI acknowledgments from the firmware falsely satisfy subsequent IPI
requests after a timeout?
If wait_event_interruptible_timeout() in mtk_vcodec_vcp_msg_ipi_send() times
out, it aborts and unlocks the IPI mutex. If a new request is submitted reusing
the same ID, and the firmware finally sends a delayed ACK for the original
timed-out request, the background msg_thread could blindly accept it:
mtk_vcodec_vcp_msg_process_thread() {
...
fw->vcp->msg_signaled[obj->id] = true;
wake_up(&fw->vcp->msg_wq[obj->id]);
...
}
Would this cause the new request to immediately return success while reading
stale responses belonging to the timed-out request, leading to protocol
desynchronization?
[ ... ]
> @@ -530,7 +562,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;
[Severity: High]
Does this patch miss implementing the get_venc_capa callback in the
mtk_vcodec_vcp_msg operations struct?
When the V4L2 encoder device is opened, fops_vcodec_open() unconditionally
calls mtk_vcodec_fw_get_venc_capa(), which attempts to dereference:
mtk_vcodec_fw_get_venc_capa() {
return fw->ops->get_venc_capa(fw);
}
If the mtk_vcodec_vcp_msg struct does not have this callback registered,
could this result in a NULL pointer dereference when a userspace application
opens the encoder video device node?
[Severity: Medium]
In mtk_vcodec_vcp_release(), if the coprocessor's deregister_feature operation
fails, the function emits an error and returns early:
mtk_vcodec_vcp_release() {
...
ret = vcp_device->ops->deregister_feature(vcp_device, fw->vcp->feature_id);
if (ret < 0) {
dev_err(dev, "deregister feature_id(%d) fail(%d)\n", fw->vcp->feature_id, ret);
return;
}
...
}
Will this early return skip the put_device label that calls
mtk_vcodec_vcp_put_device(), leading to a leaked rproc reference since
rproc_put() would be bypassed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=3
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (2 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 03/14] media: mediatek: vcodec: add driver to support vcp encoder Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:44 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
` (9 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Getting ipi(inter-processor interrupt) id according to firmware
type and hardware index for different architecture.
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
---
.../mediatek/vcodec/common/mtk_vcodec_fw.c | 14 ++++++++++++++
.../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, 30 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 552d0d8a8..8630e871d 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
@@ -3,6 +3,20 @@
#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"
+
+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_use fw_use,
mtk_vcodec_fw_init_func fw_init)
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 50d93d47b..005ac9969 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
@@ -43,5 +43,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 2d622e85f..756fbb777 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 10359ce9b..69d607171 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 02f39954a..dd638ef44 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 391e789a5..d65e276f6 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 adbacabdb..1f0479a8f 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id
2026-07-20 1:20 ` [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id Kyrie Wu
@ 2026-07-20 1:44 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:44 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, media-ci, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing check on `mtk_vcodec_fw_get_ipi()` return value leads to out-of-bounds memory write during VPU IPI registration
--
--- Patch [4]: [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id ---
commit 74021a635ff4b0a5bb38e1b68df4fa4d276cadb5
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: get different firmware ipi id
This commit introduces a helper function to get the IPI ID based on
firmware type and hardware index for different architectures. It replaces
hardcoded SCP IPI IDs with this helper in various decoder interfaces.
> 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 552d0d8a84444..8630e871d78e2 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> @@ -3,6 +3,20 @@
> #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"
> +
> +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;
> + }
> +}
[ ... ]
> 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 10359ce9b934e..69d60717181a6 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
> @@ -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);
[Severity: High]
Does this code need to check the return value of mtk_vcodec_fw_get_ipi()
before assigning it to inst->vpu.id?
If the firmware type is not SCP or VCP, mtk_vcodec_fw_get_ipi() returns
-EINVAL. This assigns -22 to inst->vpu.id, which then gets passed down
through vpu_dec_init() into vpu_ipi_register():
drivers/media/platform/mediatek/vpu/mtk_vpu.c:vpu_ipi_register() {
...
if (id < IPI_MAX && handler) {
ipi_desc = vpu->ipi_desc;
ipi_desc[id].name = name;
ipi_desc[id].handler = handler;
ipi_desc[id].priv = priv;
return 0;
}
...
}
Since the bounds check uses a signed comparison, -22 < 7 evaluates to true,
which could lead to an out-of-bounds write to ipi_desc[-22] and corrupt
memory. This same pattern also appears in the slice initialization routines
for AV1, HEVC, VP8, and VP9.
Could this negative ID assignment be prevented by validating the return
value of mtk_vcodec_fw_get_ipi()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=4
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 05/14] media: mediatek: vcodec: get share memory address
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (3 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 04/14] media: mediatek: vcodec: get different firmware ipi id Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:40 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 06/14] media: mediatek: vcodec: add debug information Kyrie Wu
` (8 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
There is only one share memory for vcp architecture, need to
divide it into many different functions.
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
---
.../vcodec/common/mtk_vcodec_fw_vcp.c | 26 +++++++++++++-
.../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, 108 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 ece4e3e87..468c4bf4e 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
@@ -483,8 +483,31 @@ 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 unsigned int mtk_vcodec_vcp_get_venc_capa(struct mtk_vcodec_fw *fw)
{
+ return 0;
+}
+
+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;
}
@@ -549,6 +572,7 @@ static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
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,
+ .get_venc_capa = mtk_vcodec_vcp_get_venc_capa,
.map_dm_addr = mtk_vcodec_vcp_dm_addr,
.ipi_register = mtk_vcodec_vcp_set_ipi_register,
.ipi_send = mtk_vcodec_vcp_ipi_send,
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 97ba642dc..79504bb2d 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 756fbb777..4932ef469 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 69d607171..544d3bc06 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 dd638ef44..a5dd42987 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 1f0479a8f..3f4b70526 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 b35759a0b..cdb673e6b 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 05/14] media: mediatek: vcodec: get share memory address
2026-07-20 1:20 ` [PATCH v6 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
@ 2026-07-20 1:40 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:40 UTC (permalink / raw)
To: Kyrie Wu; +Cc: media-ci, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] mtk_vcodec_fw_map_dm_addr() has inconsistent error semantics across firmware types, leading to NULL or ERR_PTR dereferences.
- [High] mtk_vcodec_vcp_dm_addr() calculates shared memory offsets blindly without verifying they fit within the allocated firmware memory (vsi_size).
Pre-existing issues:
- [High] Firmware-provided sizes vsi->cdf_table_size and vsi->iq_table_size are used for memory allocation and memcpy without validation.
--
--- Patch [5]: [PATCH v6 05/14] media: mediatek: vcodec: get share memory address ---
commit 4c9f9286cbe2e7417003bc83c03e8482665fcca4
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: get share memory address
This commit adds support to get shared memory addresses for the VCP
architecture by dividing a single shared memory block into 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
[ ... ]
> @@ -483,8 +483,31 @@ 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 unsigned int mtk_vcodec_vcp_get_venc_capa(struct mtk_vcodec_fw *fw)
> {
> + return 0;
> +}
> +
> +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;
[Severity: High]
Are these offsets guaranteed to fit within the memory provided by the
hardware?
When fw->vcp->vsi_size is initialized via get_mem_size() in
mtk_vcodec_vcp_load_firmware(), the driver doesn't seem to validate that
the allocated block is large enough to contain these offsets. If a
malformed device tree or mismatched firmware provides a smaller memory
block, could this lead to out-of-bounds accesses into adjacent memory?
> + default:
> + break;
> + }
> +
> + return NULL;
> +}
[Severity: High]
Is returning NULL here safe when other firmware types return an ERR_PTR?
In drivers like vpu and scp, mapping functions return ERR_PTR(-EINVAL)
on failure. Because this VCP function returns NULL instead, callers have
mixed expectations.
For example, in vdec_av1_slice_init_cdf_table(), the code checks:
remote_cdf_table = mtk_vcodec_fw_map_dm_addr(...);
if (IS_ERR(remote_cdf_table)) {
...
}
If VCP mapping fails and returns NULL, it passes the IS_ERR() check and
can cause a NULL pointer dereference during the subsequent memcpy().
> 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));
> if (IS_ERR(remote_cdf_table)) {
> mtk_vdec_err(ctx, "failed to map cdf table\n");
> return PTR_ERR(remote_cdf_table);
[Severity: High]
This is a pre-existing issue, but does vsi->cdf_table_size need to be
validated against the maximum allowed size before it is used?
Shortly after this mapping, the code does:
memcpy(instance->cdf_table.va, remote_cdf_table, vsi->cdf_table_size);
Since vsi->cdf_table_size is provided by the firmware via shared memory, a
compromised or malformed firmware could provide a size larger than the
AV1_CDF_SIZE (0xFE80) memory region. Could this result in an out-of-bounds
read of adjacent kernel memory?
> 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]
Does this !remote_frame_ctx check properly handle the ERR_PTR(-EINVAL) that
can be returned by mtk_vcodec_fw_map_dm_addr() when using VPU or SCP?
Since the VPU and SCP mapping paths return an ERR_PTR on failure, this check
will bypass them, resulting in kmemdup() attempting to dereference the
error pointer:
vdec_vp9_slice_init_default_frame_ctx() {
...
if (!remote_frame_ctx) {
return -EINVAL;
}
...
frame_ctx = kmemdup(remote_frame_ctx, sizeof(*frame_ctx), GFP_KERNEL);
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 06/14] media: mediatek: vcodec: add debug information
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (4 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 05/14] media: mediatek: vcodec: get share memory address Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:31 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
` (7 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Print hevc/av1 output format and 10bit capture format
information to debug.
Signed-off-by: Kyrie Wu <kyrie.wu@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 2da11521f..a9e52a320 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: MT2110R (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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 06/14] media: mediatek: vcodec: add debug information
2026-07-20 1:20 ` [PATCH v6 06/14] media: mediatek: vcodec: add debug information Kyrie Wu
@ 2026-07-20 1:31 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:31 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, conor+dt, devicetree, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Lack of adequate locking on the `dbgfs_head` instance list allows concurrent list modifications, leading to Use-After-Free and list corruption.
- [High] Misuse of `snprintf` return values combined with a TOCTOU race on `inst_count` leads to an out-of-bounds read and WARN_ON_ONCE.
--
--- Patch [6]: [PATCH v6 06/14] media: mediatek: vcodec: add debug information ---
commit 339b2440735f521e0732ed61e123f2dc8ea944d3
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: add debug information
This commit adds hevc and av1 output format printing to the debugfs
output. It also introduces 10-bit capture format and bitstream
information to the debug output.
> 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..a9e52a32079d3 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c
[ ... ]
> @@ -122,9 +138,10 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
[Severity: Critical]
This is a pre-existing issue, but does the iteration surrounding this code
adequately protect against concurrent list modifications?
In mtk_vdec_dbgfs_read(), the list is traversed under dbgfs_lock:
mutex_lock(&dbgfs->dbgfs_lock);
list_for_each_entry(dbgfs_inst, &dbgfs->dbgfs_head, node) {
However, if a user process rapidly opens and closes the V4L2 device,
mtk_vcodec_dbgfs_remove() unlinks and frees the list element locklessly with
respect to the debugfs read lock:
mtk_vcodec_dbgfs_remove()
list_del(&dbgfs_inst->node);
kfree(dbgfs_inst);
Since the two paths acquire different locks, they can execute concurrently.
Can this lead to a use-after-free or list corruption when the reader hits a
freed node or poisoned list pointers?
>
> 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;
[Severity: High]
This is a pre-existing issue, but could this accumulation of used_len result
in an out-of-bounds read and a kernel warning?
The total_len is calculated earlier based on a lockless read of inst_count:
mtk_vdec_dbgfs_read()
int total_len = 200 * (dbgfs->inst_count == 0 ? 1 : dbgfs->inst_count);
If concurrent device opens increase the number of instances after this buffer
is allocated, used_len can exceed total_len.
When total_len - used_len becomes negative, snprintf receives a massive size
parameter. While vsnprintf catches sizes greater than INT_MAX and prevents a
heap overflow, it issues a WARN_ON_ONCE.
Furthermore, snprintf returns the length it would have written, which is
added to used_len. Later, simple_read_from_buffer() will use this inflated
used_len to read past the end of the heap allocation into userspace.
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=6
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (5 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 06/14] media: mediatek: vcodec: add debug information Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:32 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
` (6 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
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: Kyrie Wu <kyrie.wu@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 47070be2a..097561a1e 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 cdb673e6b..3a10b32be 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp
2026-07-20 1:20 ` [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
@ 2026-07-20 1:32 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:32 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, devicetree, media-ci, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional increase in firmware IPC message size may break backwards compatibility with older SCP/VPU firmware.
--
--- Patch [7]: [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp ---
commit 84b64e642819aa5202009e5076e47823645bcbc0
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: send share memory address to vcp
This commit updates the AP to VPU initialization message by adding a shared
memory address field. It is used specifically for the VCP architecture where
shared memory is allocated in the kernel rather than reserved in the VPU.
> 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 47070be2a9914..097561a1efdcf 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_ipi_msg.h
[ ... ]
> 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 cdb673e6b4770..3a10b32be0945 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);
[Severity: Medium]
Does this unconditional size increase break backwards compatibility with older
SCP/VPU firmware?
By adding shared_iova to struct vdec_ap_ipi_init, the struct size increases
from 16 bytes to 24 bytes. Directly after this diff hunk in vpu_dec_init(),
the message is sent to the firmware using the full sizeof(msg):
err = vcodec_vpu_send_msg(vpu, (void *)&msg, sizeof(msg));
If older firmware expects a strictly sized 16-byte initialization message,
could sending 24 bytes unconditionally cause the initialization to fail and
render the video decoder unusable on existing non-VCP devices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=7
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (6 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 07/14] media: mediatek: vcodec: send share memory address to vcp Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:29 ` sashiko-bot
2026-07-20 5:46 ` Krzysztof Kozlowski
2026-07-20 1:20 ` [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
` (5 subsequent siblings)
13 siblings, 2 replies; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Add the MT8196 compatible string to the MediaTek vcodec subdev
decoder binding.
Compared to previous ICs, the MT8196 supports a 10-bit decoder
and has a decoding capability of 4K@120fps. It also supports
36-bit DRAM IOVA address and Video Power Control to optimize
bandwidth and voltage usage.
Signed-off-by: Kyrie Wu <kyrie.wu@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 d1d209cbb..4e11d4aad 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 1:20 ` [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
@ 2026-07-20 1:29 ` sashiko-bot
2026-07-20 5:46 ` Krzysztof Kozlowski
1 sibling, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:29 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, media-ci, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The description block documenting the decoding pipeline architecture was not updated to include the newly added MT8196 SoC.
--
--- Patch [8]: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 ---
commit e70e1026361bf104bca63d4eac75dcb632c4dc87
Author: Kyrie Wu <kyrie.wu@mediatek.com>
dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
This commit adds the MT8196 compatible string to the MediaTek vcodec subdev
decoder binding. It documents that the MT8196 supports 10-bit decoding,
4K@120fps, a 36-bit DRAM IOVA address, and Video Power Control.
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> index d1d209cbbd43a..4e11d4aade2ef 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
[Severity: Low]
Does the description block documenting the decoding pipeline architecture also
need to be updated for this new SoC?
Looking at the description section higher up in this file, there is an explicit
enumeration of the pipelines:
The used video decoding pipeline architecture across various Mediatek SoC:
MT8195: LAT-SoC + LAT + Core
MT8192: LAT + Core
MT8188: LAT + Core
MT8186: Core
Since the MT8196 architecture also uses LAT and Core, should it be appended
to this description list to keep the binding documentation complete?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=8
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 1:20 ` [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
2026-07-20 1:29 ` sashiko-bot
@ 2026-07-20 5:46 ` Krzysztof Kozlowski
2026-07-20 5:49 ` Krzysztof Kozlowski
1 sibling, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-20 5:46 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, Fei Shao, Haoxiang Li, Chen-Yu Tsai,
Laurent Pinchart, Tomasz Figa, Sebastian Fricke, Philipp Zabel,
Benjamin Gaignard, Qianfeng Rong, Irui Wang, Jacopo Mondi, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Sakari Ailus
On Mon, Jul 20, 2026 at 09:20:50AM +0800, Kyrie Wu wrote:
> Add the MT8196 compatible string to the MediaTek vcodec subdev
> decoder binding.
>
> Compared to previous ICs, the MT8196 supports a 10-bit decoder
> and has a decoding capability of 4K@120fps. It also supports
> 36-bit DRAM IOVA address and Video Power Control to optimize
> bandwidth and voltage usage.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> .../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 5:46 ` Krzysztof Kozlowski
@ 2026-07-20 5:49 ` Krzysztof Kozlowski
2026-07-20 6:26 ` Kyrie Wu (吴晗)
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-20 5:49 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, Fei Shao, Haoxiang Li, Chen-Yu Tsai,
Laurent Pinchart, Tomasz Figa, Sebastian Fricke, Philipp Zabel,
Benjamin Gaignard, Qianfeng Rong, Irui Wang, Jacopo Mondi, Fan Wu,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Sakari Ailus
On 20/07/2026 07:46, Krzysztof Kozlowski wrote:
> On Mon, Jul 20, 2026 at 09:20:50AM +0800, Kyrie Wu wrote:
>> Add the MT8196 compatible string to the MediaTek vcodec subdev
>> decoder binding.
>>
>> Compared to previous ICs, the MT8196 supports a 10-bit decoder
>> and has a decoding capability of 4K@120fps. It also supports
>> 36-bit DRAM IOVA address and Video Power Control to optimize
>> bandwidth and voltage usage.
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
>>
>> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
>> Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>> ---
>> .../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 1 +
>> 1 file changed, 1 insertion(+)
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Actually no. I checked previous comments and you received feedback which
you basically ignored, so explain why this model has flexible number of
clocks
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 5:49 ` Krzysztof Kozlowski
@ 2026-07-20 6:26 ` Kyrie Wu (吴晗)
2026-07-20 10:19 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu (吴晗) @ 2026-07-20 6:26 UTC (permalink / raw)
To: krzk@kernel.org
Cc: matthias.bgg@gmail.com, fshao@chromium.org,
Yunfei Dong (董云飞), conor+dt@kernel.org,
fanwu01@zju.edu.cn, jacopo.mondi@ideasonboard.com,
haoxiang_li2024@163.com, tfiga@chromium.org,
AngeloGioacchino Del Regno,
Andrew-CT Chen (陳智迪),
linux-media@vger.kernel.org, ribalda@chromium.org,
wenst@chromium.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, kees@kernel.org,
rongqianfeng@vivo.com, Irui Wang (王瑞),
krzk+dt@kernel.org, linux-mediatek@lists.infradead.org,
Tiffany Lin (林慧珊),
sakari.ailus@linux.intel.com, benjamin.gaignard@collabora.com,
p.zabel@pengutronix.de, mchehab@kernel.org,
sebastian.fricke@collabora.com, robh@kernel.org,
linux-arm-kernel@lists.infradead.org,
nicolas.dufresne@collabora.com, hverkuil+cisco@kernel.org,
laurent.pinchart+renesas@ideasonboard.com
On Mon, 2026-07-20 at 07:49 +0200, Krzysztof Kozlowski wrote:
> On 20/07/2026 07:46, Krzysztof Kozlowski wrote:
> > On Mon, Jul 20, 2026 at 09:20:50AM +0800, Kyrie Wu wrote:
> > > Add the MT8196 compatible string to the MediaTek vcodec subdev
> > > decoder binding.
> > >
> > > Compared to previous ICs, the MT8196 supports a 10-bit decoder
> > > and has a decoding capability of 4K@120fps. It also supports
> > > 36-bit DRAM IOVA address and Video Power Control to optimize
> > > bandwidth and voltage usage.
> >
> > Please wrap commit message according to Linux coding style /
> > submission
> > process (neither too early nor over the limit):
> > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst*L597__;Iw!!CTRNKA9wMg0ARbw!gDjbq0EZNkhLTzuJM_w5PopnDFej1mG4qnkRFgyoiU7Dvhm7tj3MeG5eZYNGA0L0G_61j-cmdIxORA$
> >
> >
> > >
> > > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> > > Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > > ---
> > > .../bindings/media/mediatek,vcodec-subdev-decoder.yaml
> > > | 1 +
> > > 1 file changed, 1 insertion(+)
> >
> > Acked-by: Krzysztof Kozlowski
> > <krzysztof.kozlowski@oss.qualcomm.com>
>
> Actually no. I checked previous comments and you received feedback
> which
> you basically ignored, so explain why this model has flexible number
> of
> clocks
>
> Best regards,
> Krzysztof
Dear Krzysztof,
As mentioned in the previous version of the communication, the
information regarding clk was incorrect, and I have corrected it in
this version.
Thank you for your attention and reminder.
Regards,
Kyrie.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196
2026-07-20 6:26 ` Kyrie Wu (吴晗)
@ 2026-07-20 10:19 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-20 10:19 UTC (permalink / raw)
To: Kyrie Wu (吴晗)
Cc: matthias.bgg@gmail.com, fshao@chromium.org,
Yunfei Dong (董云飞), conor+dt@kernel.org,
fanwu01@zju.edu.cn, jacopo.mondi@ideasonboard.com,
haoxiang_li2024@163.com, tfiga@chromium.org,
AngeloGioacchino Del Regno,
Andrew-CT Chen (陳智迪),
linux-media@vger.kernel.org, ribalda@chromium.org,
wenst@chromium.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, kees@kernel.org,
rongqianfeng@vivo.com, Irui Wang (王瑞),
krzk+dt@kernel.org, linux-mediatek@lists.infradead.org,
Tiffany Lin (林慧珊),
sakari.ailus@linux.intel.com, benjamin.gaignard@collabora.com,
p.zabel@pengutronix.de, mchehab@kernel.org,
sebastian.fricke@collabora.com, robh@kernel.org,
linux-arm-kernel@lists.infradead.org,
nicolas.dufresne@collabora.com, hverkuil+cisco@kernel.org,
laurent.pinchart+renesas@ideasonboard.com
On 20/07/2026 08:26, Kyrie Wu (吴晗) wrote:
> On Mon, 2026-07-20 at 07:49 +0200, Krzysztof Kozlowski wrote:
>> On 20/07/2026 07:46, Krzysztof Kozlowski wrote:
>>> On Mon, Jul 20, 2026 at 09:20:50AM +0800, Kyrie Wu wrote:
>>>> Add the MT8196 compatible string to the MediaTek vcodec subdev
>>>> decoder binding.
>>>>
>>>> Compared to previous ICs, the MT8196 supports a 10-bit decoder
>>>> and has a decoding capability of 4K@120fps. It also supports
>>>> 36-bit DRAM IOVA address and Video Power Control to optimize
>>>> bandwidth and voltage usage.
>>>
>>> Please wrap commit message according to Linux coding style /
>>> submission
>>> process (neither too early nor over the limit):
>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst*L597__;Iw!!CTRNKA9wMg0ARbw!gDjbq0EZNkhLTzuJM_w5PopnDFej1mG4qnkRFgyoiU7Dvhm7tj3MeG5eZYNGA0L0G_61j-cmdIxORA$
>>>
>>>
>>>>
>>>> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
>>>> Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>>>> ---
>>>> .../bindings/media/mediatek,vcodec-subdev-decoder.yaml
>>>> | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>
>>> Acked-by: Krzysztof Kozlowski
>>> <krzysztof.kozlowski@oss.qualcomm.com>
>>
>> Actually no. I checked previous comments and you received feedback
>> which
>> you basically ignored, so explain why this model has flexible number
>> of
>> clocks
>>
>> Best regards,
>> Krzysztof
>
> Dear Krzysztof,
>
> As mentioned in the previous version of the communication, the
> information regarding clk was incorrect, and I have corrected it in
> this version.
>
> Thank you for your attention and reminder.
Read my message carefully instead of responding whatever necessary to
make me go away.
NAK
Where did you explain that device has flexible number of clocks?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (7 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196 Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:36 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 10/14] media: mediatek: vcodec: define MT8196 vcodec levels Kyrie Wu
` (4 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
MT8196 is lat single core architecture. Support its compatible and
use `mtk_lat_sig_core_pdata` to initialize platform data.
Signed-off-by: Kyrie Wu <kyrie.wu@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 ab5f6c01f..0d27adbf3 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;
}
@@ -566,6 +568,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 15e437323..53d1708b0 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196
2026-07-20 1:20 ` [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
@ 2026-07-20 1:36 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:36 UTC (permalink / raw)
To: Kyrie Wu; +Cc: devicetree, robh, media-ci, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added MT8196 chip was added to the `mtk_vcodec_dec_chip_name` enum, but its V4L2 capability limits were not updated in the stateless decoder's control setup, causing the driver to advertise artificially limited fallback capabilities to userspace.
--
--- Patch [9]: [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196 ---
commit 33685e9d98f41bca9dad07f1a7aa64db22f658cd
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: add decoder compatible to support mt8196
MT8196 is lat single core architecture. Support its compatible and
use `mtk_lat_sig_core_pdata` to initialize platform data.
> 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: High]
Does adding this new chip name without updating the stateless decoder's
control limits restrict the MT8196 capabilities to their lowest defaults?
For example, in drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c:
mtk_vcodec_dec_fill_h265_profile() {
switch (ctx->dev->chip_name) {
case MTK_VDEC_MT8188:
case MTK_VDEC_MT8195:
cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10;
break;
default:
cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE;
break;
}
}
Will this cause the HEVC max profile for MT8196 to fall to the default
MAIN_STILL_PICTURE limit, preventing standard Main profile and 10-bit decoding?
A similar fallback to defaults seems to occur for MT8196 in:
- mtk_vcodec_dec_fill_h265_level() (limits HEVC level to 4 instead of 5.1/5.2)
- mtk_vcodec_dec_fill_vp9_profile() (limits VP9 to Profile 1)
- mtk_vcodec_dec_fill_h264_level() (limits H.264 to Level 4.1)
Should the MT8196 chip be added to the capability limit setups for the stateless
decoder controls?
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=9
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 10/14] media: mediatek: vcodec: define MT8196 vcodec levels.
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (8 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 09/14] media: mediatek: vcodec: add decoder compatible to support mt8196 Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 11/14] media: mediatek: vcodec: support 36bit iova address Kyrie Wu
` (3 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
The supported level and profile are not the same for different
codecs and architecture. Select the correct one.
Signed-off-by: Kyrie Wu <kyrie.wu@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 ab1894fba..472ece571 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v6 11/14] media: mediatek: vcodec: support 36bit iova address
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (9 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 10/14] media: mediatek: vcodec: define MT8196 vcodec levels Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
` (2 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
Need to set dma mask to support 36bit iova address for decoder
hardware can use 36bit address to decode for mt8196.
Signed-off-by: Kyrie Wu <kyrie.wu@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 0d27adbf3..4d65c1025 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
@@ -392,6 +392,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, DECODER, dev->fw_init);
if (IS_ERR(dev->fw_handler))
--
2.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (10 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 11/14] media: mediatek: vcodec: support 36bit iova address Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 2:03 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
2026-07-20 1:20 ` [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
The driver need to clean xpc status when receive decoder hardware
interrupt for mt8196 platform.
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../vcodec/decoder/mtk_vcodec_dec_hw.c | 31 +++++++++++++++++++
.../vcodec/decoder/mtk_vcodec_dec_hw.h | 13 ++++++--
2 files changed, 42 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 881d5de41..46ac3e41c 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,34 @@ 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)
+{
+ unsigned long flags;
+ 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;
+
+ spin_lock_irqsave(&dev->main_dev->irqlock, flags);
+ 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);
+ spin_unlock_irqrestore(&dev->main_dev->irqlock, flags);
+}
+
static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
{
struct mtk_vdec_hw_dev *dev = priv;
@@ -88,6 +116,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 +196,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 83fe8b942..5c906143c 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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status
2026-07-20 1:20 ` [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
@ 2026-07-20 2:03 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 2:03 UTC (permalink / raw)
To: Kyrie Wu; +Cc: robh, devicetree, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Low] Enum type mismatch when indexing parent device's register array.
Pre-existing issues:
- [High] Null pointer dereference on spurious hardware interrupts.
- [High] Use-after-free and dirty state on probe error path.
- [High] Unconditional NULL pointer dereference in the hardware IRQ handler on platforms using syscon for VDECSYS.
--
--- Patch [12]: [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status ---
commit ff6c78cd026b1536229c06cdffd12b2498c8aeb9
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: vcodec: clean xpc status
The driver need to clean xpc status when receive decoder hardware
interrupt for 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..46ac3e41c401c 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
> @@ -88,6 +116,8 @@ static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
[Severity: High]
This is a pre-existing issue, but does mtk_vdec_hw_irq_handler() in
mtk_vcodec_dec_hw.c properly handle spurious interrupts where
mtk_vcodec_get_curr_ctx() returns NULL?
If there is no active context when the interrupt fires, ctx will be NULL.
The handler subsequently passes this NULL context to mtk_v4l2_vdec_err(),
which dereferences ctx->dev directly:
mtk_vdec_hw_irq_handler() {
...
ctx = mtk_vcodec_get_curr_ctx(dev->main_dev, dev->hw_idx);
/* check if HW active or not */
cg_status = readl(dev->reg_base[VDEC_HW_SYS] + VDEC_HW_ACTIVE_ADDR);
if (cg_status & VDEC_HW_ACTIVE_MASK) {
mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0 (0x%08x)", cg_status);
return IRQ_HANDLED;
}
...
}
Could this trigger a kernel panic on spurious interrupts?
[Severity: High]
This is also a pre-existing issue, but will dev->reg_base[VDEC_HW_SYS] be
NULL in mtk_vdec_hw_irq_handler() on platforms using syscon for VDECSYS
(like MT8195 and MT8196)?
On platforms utilizing the mediatek,vdecsys syscon, main_dev->reg_base[0] is
left NULL. The subdevice probe copies this NULL pointer to the subdevice.
When an interrupt fires, the handler performs an unconditional readl() from
this NULL address in the snippet shown above, instead of using the
vdecsys_regmap fallback.
Could this cause an unconditional panic when an interrupt fires on affected
platforms?
> 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 +196,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 mtk_vdec_hw_probe() indexing the main_dev->reg_base array with the
correct enum?
It looks like it is using VDEC_HW_MISC (from enum mtk_vdec_hw_reg_idx)
instead of the expected VDEC_MISC (from enum mtk_hw_reg_idx). While they
both currently evaluate to 1, could this cause out-of-bounds access or
incorrect address mappings if the enums drift?
> set_bit(subdev_dev->hw_idx, main_dev->subdev_bitmap);
>
> if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
[Severity: High]
This is a pre-existing issue, but does the mtk_vdec_hw_probe() error path
leave dangling pointers in the parent device?
If mtk_vdec_hw_init_irq() fails here, mtk_vdec_hw_probe() returns an error:
mtk_vdec_hw_probe() {
...
if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
ret = mtk_vdec_hw_init_irq(subdev_dev);
if (ret)
return ret;
}
...
}
Since subdev_dev was allocated with devres, it will be automatically freed.
However, the parent (main_dev) will retain the freed pointer in
main_dev->subdev_dev[hw_idx] and the bit in main_dev->subdev_bitmap remains
set.
Could this lead to a use-after-free if another component iterates over the
subdevices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=12
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (11 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 12/14] media: mediatek: vcodec: clean xpc status Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:48 ` sashiko-bot
2026-07-20 1:20 ` [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
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: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
.../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 15 +++++++++------
1 file changed, 9 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 4932ef469..14639098f 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,10 @@ 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;
+ vref->c.size = vsi->fb.c.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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo
2026-07-20 1:20 ` [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
@ 2026-07-20 1:48 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:48 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, devicetree, robh, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but this patch incompletely fixes a subsystem-wide issue by fixing AV1 while leaving the exact same bug in the VP9 stateless decoder. The C plane offset for reference frames is incorrectly calculated during dynamic resolution changes.
--
--- Patch [13]: [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo ---
commit 6fb6f33cb9871d45e294575fbf5e9c61dfa39742
Author: Kyrie Wu <kyrie.wu@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,
can fill each frame buffer size with picinfo directly.
> 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
[ ... ]
> @@ -1845,8 +1846,10 @@ 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;
> + vref->c.size = vsi->fb.c.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;
[Severity: High]
This is a pre-existing issue, but this fix for AV1 appears to leave the exact
same bug unfixed in the VP9 stateless decoder.
When a VP9 stream changes resolution mid-stream, the driver calculates the
C plane offset based on the current frame's width and height. Does this
erroneously apply the current-frame offset to all previously decoded
reference frames?
vdec_vp9_req_lat_if.c:vdec_vp9_slice_setup_core_buffer() {
...
size = ALIGN(w, 64) * ALIGN(h, 64);
...
vsi->ref[i].c.dma_addr =
vsi->ref[i].y.dma_addr + size;
...
}
If the current frame is smaller than the reference frame, would the hardware
read corrupted chroma data from the middle of the reference frame's Y plane?
If the current frame is larger, could the calculated offset applied to the
reference frame point beyond its actual chroma offset, potentially triggering
a hardware fault via the IOMMU?
Could a similar fix using ctx->picinfo.fb_sz[0] be applied to VP9?
> else
> vref->c.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=13
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi
2026-07-20 1:20 [PATCH v6 00/14] media: mediatek: vcodec: support video decoder in mt8196 Kyrie Wu
` (12 preceding siblings ...)
2026-07-20 1:20 ` [PATCH v6 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo Kyrie Wu
@ 2026-07-20 1:20 ` Kyrie Wu
2026-07-20 1:50 ` sashiko-bot
13 siblings, 1 reply; 32+ messages in thread
From: Kyrie Wu @ 2026-07-20 1:20 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, Kyrie Wu, Nicolas Dufresne,
Ricardo Ribalda, Kees Cook, Hans Verkuil, Fei Shao, Haoxiang Li,
Chen-Yu Tsai, Laurent Pinchart, Tomasz Figa, Sebastian Fricke,
Philipp Zabel, Benjamin Gaignard, Qianfeng Rong, Irui Wang,
Jacopo Mondi, Fan Wu, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Sakari Ailus
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: Kyrie Wu <kyrie.wu@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 14639098f..1ef561dae 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;
@@ -1908,7 +1944,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)
@@ -1942,9 +1978,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.51.0.windows.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi
2026-07-20 1:20 ` [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi Kyrie Wu
@ 2026-07-20 1:50 ` sashiko-bot
0 siblings, 0 replies; 32+ messages in thread
From: sashiko-bot @ 2026-07-20 1:50 UTC (permalink / raw)
To: Kyrie Wu; +Cc: conor+dt, robh, devicetree, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Buffer overflow into firmware shared memory due to unconditional sizeof() in vdec_av1_slice_vsi_to_remote()
Pre-existing issues:
- [High] Use-After-Free (UAF) due to lockless access to dynamic V4L2 control array in vdec_av1_slice_setup_tile_group()
--
--- Patch [14]: [PATCH v6 14/14] media: mediatek: decoder: support av1 extend vsi ---
commit ed7b2651547150bfaed8f1355f43d152991a7bc1
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: decoder: support av1 extend vsi
This commit introduces support for AV1 extend VSI architecture in the
Mediatek decoder. It adds tile group information to the VPU shared memory
structure for extended architectures.
> 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 14639098fd463..1ef561dae18c7 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
[ ... ]
> @@ -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];
> };
[Severity: Critical]
Does the addition of this large struct cause a buffer overflow on
non-extended architectures?
The firmware allocates only 3408 bytes (AV1_NON_EXT_VSI_SIZE) for this
memory mapping on non-extended architectures. However, since
vdec_av1_slice_vsi_to_remote() unconditionally copies the entire struct
using sizeof(*vsi), won't this write out-of-bounds data into the adjacent
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);
> }
>
> +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;
[Severity: High]
This is a pre-existing issue, but does this function access the dynamically
allocated V4L2 control payload locklessly?
vdec_av1_slice_setup_tile_group() fetches the control and later accesses
ctrl->p_cur.p without holding the control handler lock (ctrl_hdl->lock).
Since this executes in a workqueue, could a concurrent VIDIOC_S_EXT_CTRLS
ioctl cause the V4L2 core to reallocate the array and free the old memory?
This would result in a use-after-free read of the old array payload, potentially
tricking the hardware into performing out-of-bounds DMA.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012056.1026551-1-kyrie.wu@mediatek.com?part=14
^ permalink raw reply [flat|nested] 32+ messages in thread