[PATCH v16 00/12] Enable jpeg enc & dec multi-hardwares for MT8196

[PATCH v16 00/12] Enable jpeg enc & dec multi-hardwares for MT8196

[Kyrie Wu]

This series have the follow changing:
Firstly fix some bugs, including resolution change handleing, stop
streaming sw flow, fix buffer layout and clock setting to support multi-hw
jpeg working and others.
Secondly add mt8196 jpegdec and jpegenc compatible to support MT8196
kernel driver.
Lastly, Add smmu setting to support smmu and iommu at the same time.

This series has been tested with MT8196 tast test.
Encoding and decoding worked for this chip.

Patches 1 fix jpeg hw count setting to support different chips.
Patches 2 fix jpeg buffer payload setting to handle buffer
size bug while resolution changed.
Patches 3 fix jpeg dst buffer layout.
Patches 4 fix multi-core stop streaming flow
Patches 5 fix multi-core clk suspend/resume setting
Patches 6 fix buffer state update timing
Patches 7 fix decoding resolution change operation
Patches 8 fix remove buffer operation
Patches 9-11 Adds jpeg encoder and decoder compatible.
Patches 12 add jpeg smmu sid setting.
---
Changes compared with v15:
--Set max_hw_count from the DT child count and update hw_rdy from child hardware probes in patch 1.
--Fix patch 2 commit message wording per review.
--Rework patch 6 commit message per review.
--Remove redundant devm_clk_bulk_get() calls in patch 5 per review comments.
--Use %pe to print smmu_regmap error pointer in patch 12 to fix media-ci/static warnings.

Changes compared with v14:
--Rebased on top of the latest media tree

Changes compared with v13:
--Rebased on top of the latest media tree

Changes compared with v12:
--Rebased on top of the latest media tree
--fix kernel rebot build warnings in patch 5

Changes compared with v11:
--Rebased on top of the latest media tree
--Some modifications for patch v11's review comments.
--add reviewer to commit messages

Changes compared with v10:
--Rebased on top of the latest media tree
--add reviewer to commit messages

Changes compared with v9:
--Rebased on top of the latest media tree

Changes compared with v8:
--Rebased on top of the latest media tree

Changes compared with v7:
--Rebased on top of the latest media tree

Changes compared with v6:
--Rebased on top of the latest media tree

Changes compared with v5:
--reorder the patches set.
--fix commit message of patch 1-8.

Changes compared with v4:
--fix kernel robot build errors for patch 4.
--add reviewer for patch 1 and patch 2.

Changes compared with v3:
--change patch subject of jpeg encoder and decoder compatible.

Changes compared with v2:
--refactor smmu sid setting function interface
--Some modifications for patch v2's review comments.

Changes compared with v1:
--refine jpeg dt-bindings for MT8196
--optimize software code to manage jpeg HW count
--refactor smmu sid setting function interface
--Some modifications for patch v1's review comments.

Kyrie Wu (12):
  media: mediatek: jpeg: fix jpeg cores' amounts setting
  media: mediatek: jpeg: fix jpeg buffer payload size setting
  media: mediatek: jpeg: fix buffer structure size and layout
  media: mediatek: jpeg: Fix buffer completion on multi-core streaming
    stop
  media: mediatek: jpeg: Fix multi-core clk suspend and resume setting
  media: mediatek: jpeg: fix buffer state update timing
  media: mediatek: jpeg: fix resolution change event handling in decoder
  media: mediatek: jpeg: fix remove buffer removal timing for multi-core
  media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgdec
    compatible
  media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc
    compatible
  media: mediatek: jpeg: add jpeg compatible
  media: mediatek: jpeg: add jpeg smmu sid setting

 .../media/mediatek,mt8195-jpegdec.yaml        |   8 +-
 .../media/mediatek,mt8195-jpegenc.yaml        |   8 +-
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 156 +++++++++++++-----
 .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  19 ++-
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  |  84 +++++++++-
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  82 ++++++++-
 6 files changed, 300 insertions(+), 57 deletions(-)

-- 
2.51.0.windows.2

[PATCH v16 01/12] media: mediatek: jpeg: fix jpeg cores' amounts setting

[Kyrie Wu]

Different ICs have different amounts of cores,
use a variable to set the cores' amounts.

Fixes: 934e8bccac95 ("mtk-jpegenc: support jpegenc multi-hardware")
Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c  | 11 ++++++-----
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h  |  2 ++
 .../media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c    |  2 ++
 .../media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c    |  2 ++
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d147ec483..41c4cf8dc 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1325,7 +1325,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 		for_each_child_of_node(pdev->dev.of_node, child)
 			num_child++;
 
-		atomic_set(&jpeg->hw_rdy, num_child);
+		jpeg->max_hw_count = num_child;
+		atomic_set(&jpeg->hw_rdy, 0);
 		atomic_set(&jpeg->hw_index, 0);
 
 		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
@@ -1462,7 +1463,7 @@ static int mtk_jpegenc_get_hw(struct mtk_jpeg_ctx *ctx)
 	int i;
 
 	spin_lock_irqsave(&jpeg->hw_lock, flags);
-	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++) {
+	for (i = 0; i < jpeg->max_hw_count; i++) {
 		comp_jpeg = jpeg->enc_hw_dev[i];
 		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
 			hw_id = i;
@@ -1509,7 +1510,7 @@ static int mtk_jpegdec_get_hw(struct mtk_jpeg_ctx *ctx)
 	int i;
 
 	spin_lock_irqsave(&jpeg->hw_lock, flags);
-	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
+	for (i = 0; i < jpeg->max_hw_count; i++) {
 		comp_jpeg = jpeg->dec_hw_dev[i];
 		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
 			hw_id = i;
@@ -1592,7 +1593,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 		jpeg_work);
 	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
 
-	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++)
+	for (i = 0; i < jpeg->max_hw_count; i++)
 		comp_jpeg[i] = jpeg->enc_hw_dev[i];
 	i = 0;
 
@@ -1687,7 +1688,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 	struct mtk_jpeg_fb fb;
 	unsigned long flags;
 
-	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++)
+	for (i = 0; i < jpeg->max_hw_count; i++)
 		comp_jpeg[i] = jpeg->dec_hw_dev[i];
 	i = 0;
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 02ed0ed5b..6be5cf30d 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
@@ -212,6 +212,7 @@ struct mtk_jpegdec_comp_dev {
  * @reg_decbase:	jpg decode register base addr
  * @dec_hw_dev:	jpg decode hardware device
  * @hw_index:		jpg hw index
+ * @max_hw_count:	jpeg hw-core count
  */
 struct mtk_jpeg_dev {
 	struct mutex		lock;
@@ -234,6 +235,7 @@ struct mtk_jpeg_dev {
 	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
 	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
 	atomic_t hw_index;
+	u32 max_hw_count;
 };
 
 /**
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index 32372781d..9ca68cde4 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -668,6 +668,8 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 
+	atomic_inc(&master_dev->hw_rdy);
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index b6f5b2249..a3f414f92 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -390,6 +390,8 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 
+	atomic_inc(&master_dev->hw_rdy);
+
 	return 0;
 }
 
-- 
2.51.0.windows.2

[PATCH v16 02/12] media: mediatek: jpeg: fix jpeg buffer payload size setting

[Kyrie Wu]

For multi-core jpegdec, if one core gets resolution change event,
the payload size, representing the size of Y/C data, needs to change.
But others are decoding at the same time and it can not be changed
immediately, which results in the payload size not matching the real
buffer length.
The payload size must be less than the real buffer length to avoid
warning logs.

Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 41c4cf8dc..34135706a 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -702,6 +702,7 @@ static int mtk_jpeg_buf_prepare(struct vb2_buffer *vb)
 	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
 	struct mtk_jpeg_q_data *q_data = NULL;
 	struct v4l2_plane_pix_format plane_fmt = {};
+	size_t max_size;
 	int i;
 
 	q_data = mtk_jpeg_get_q_data(ctx, vb->vb2_queue->type);
@@ -710,12 +711,20 @@ static int mtk_jpeg_buf_prepare(struct vb2_buffer *vb)
 
 	for (i = 0; i < q_data->fmt->colplanes; i++) {
 		plane_fmt = q_data->pix_mp.plane_fmt[i];
+		max_size = plane_fmt.sizeimage;
+
 		if (ctx->enable_exif &&
-		    q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
-			vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
-					      MTK_JPEG_MAX_EXIF_SIZE);
-		else
-			vb2_set_plane_payload(vb, i,  plane_fmt.sizeimage);
+			q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG) {
+			max_size += MTK_JPEG_MAX_EXIF_SIZE;
+
+			vb2_set_plane_payload(vb, i,
+					      MIN(vb->planes[i].length,
+						  max_size));
+		} else {
+			vb2_set_plane_payload(vb, i,
+					      MIN(plane_fmt.sizeimage,
+						  vb->planes[i].length));
+		}
 	}
 
 	return 0;
-- 
2.51.0.windows.2

[PATCH v16 03/12] media: mediatek: jpeg: fix buffer structure size and layout

[Kyrie Wu]

Updates the `buf_struct_size` in the JPEG driver to use the correct
structure `mtk_jpeg_src_buf` instead of `v4l2_m2m_buffer`, ensuring
proper handling of source buffers. The previous buffer size led to
mismatches and potential issues during video buffer handling.

Moves `frame_num` in the `mtk_jpeg_src_buf` structure to align with
other members appropriately. The first field of the driver-specific
buffer structure must be the subsystem-specificstruct
(vb2_v4l2_buffer in the case of V4L2).

Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 2 +-
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 34135706a..7856a556c 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1092,7 +1092,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
 	dst_vq->drv_priv = ctx;
-	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	dst_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
 	dst_vq->ops = jpeg->variant->qops;
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 6be5cf30d..148fd4175 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
@@ -85,10 +85,10 @@ struct mtk_jpeg_variant {
 };
 
 struct mtk_jpeg_src_buf {
-	u32 frame_num;
 	struct vb2_v4l2_buffer b;
 	struct list_head list;
 	u32 bs_size;
+	u32 frame_num;
 	struct mtk_jpeg_dec_param dec_param;
 
 	struct mtk_jpeg_ctx *curr_ctx;
-- 
2.51.0.windows.2

[PATCH v16 04/12] media: mediatek: jpeg: Fix buffer completion on multi-core streaming stop

[Kyrie Wu]

Enhances the Mediatek JPEG driver's stability and reliability by ensuring
that all queued buffers are processed before stopping the streaming in
multi-core environments. It introduces a call to
`vb2_wait_for_all_buffers()` in the `mtk_jpeg_enc_stop_streaming()` and
`mtk_jpeg_dec_stop_streaming()` functions when the `multi_core` variant
is enabled. This change ensures that no buffers are left unprocessed,
preventing potential data loss or corruption during multi-core flow.

Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")
Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface")
Fixes: 934e8bccac95 ("mtk-jpegenc: support jpegenc multi-hardware")
Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 7856a556c..d0fb68bc8 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -850,8 +850,12 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
 static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
 {
 	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
+	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
 	struct vb2_v4l2_buffer *vb;
 
+	if (jpeg->variant->multi_core)
+		vb2_wait_for_all_buffers(q);
+
 	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
 		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
 }
@@ -859,6 +863,7 @@ static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
 static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
 {
 	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
+	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
 	struct vb2_v4l2_buffer *vb;
 
 	/*
@@ -866,6 +871,9 @@ static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
 	 * Before STREAMOFF, we still have to return the old resolution and
 	 * subsampling. Update capture queue when the stream is off.
 	 */
+	if (jpeg->variant->multi_core)
+		vb2_wait_for_all_buffers(q);
+
 	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
 	    V4L2_TYPE_IS_CAPTURE(q->type)) {
 		struct mtk_jpeg_src_buf *src_buf;
-- 
2.51.0.windows.2

[PATCH v16 05/12] media: mediatek: jpeg: Fix multi-core clk suspend and resume setting

[Kyrie Wu]

The suspend/resume callback function is defined in the dev_pm_ops
structure, which is defined in platform_driver. For multiple-core
architecture, each hardware driver will register a platform_driver
structure, so it is necessary to add a suspend/resume callback
function for each hardware to support this operation.

Fixes: 934e8bccac95 ("mtk-jpegenc: support jpegenc multi-hardware")
Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 28 +++-------
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 55 +++++++++++++++++--
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 53 ++++++++++++++++--
 3 files changed, 107 insertions(+), 29 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d0fb68bc8..a5a1f6126 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1115,6 +1115,9 @@ static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
 {
 	int ret;
 
+	if (jpeg->variant->multi_core)
+		return;
+
 	ret = clk_bulk_prepare_enable(jpeg->variant->num_clks,
 				      jpeg->variant->clks);
 	if (ret)
@@ -1123,6 +1126,9 @@ static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
 
 static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg)
 {
+	if (jpeg->variant->multi_core)
+		return;
+
 	clk_bulk_disable_unprepare(jpeg->variant->num_clks,
 				   jpeg->variant->clks);
 }
@@ -1648,13 +1654,6 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 		goto enc_end;
 	}
 
-	ret = clk_prepare_enable(comp_jpeg[hw_id]->venc_clk.clks->clk);
-	if (ret) {
-		dev_err(jpeg->dev, "%s : %d, jpegenc clk_prepare_enable fail\n",
-			__func__, __LINE__);
-		goto enc_end;
-	}
-
 	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
@@ -1751,20 +1750,13 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 	jpeg_dst_buf->frame_num = ctx->total_frame_num;
 
 	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
-	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
+	ret = pm_runtime_resume_and_get(comp_jpeg[hw_id]->dev);
 	if (ret < 0) {
 		dev_err(jpeg->dev, "%s : %d, pm_runtime_get_sync fail !!!\n",
 			__func__, __LINE__);
 		goto dec_end;
 	}
 
-	ret = clk_prepare_enable(comp_jpeg[hw_id]->jdec_clk.clks->clk);
-	if (ret) {
-		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable fail\n",
-			__func__, __LINE__);
-		goto clk_end;
-	}
-
 	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
@@ -1774,7 +1766,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 				 &dst_buf->vb2_buf, &fb)) {
 		dev_err(jpeg->dev, "%s : %d, mtk_jpeg_set_dec_dst fail\n",
 			__func__, __LINE__);
-		goto setdst_end;
+		goto set_dst_fail;
 	}
 
 	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
@@ -1795,9 +1787,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 
 	return;
 
-setdst_end:
-	clk_disable_unprepare(comp_jpeg[hw_id]->jdec_clk.clks->clk);
-clk_end:
+set_dst_fail:
 	pm_runtime_put(comp_jpeg[hw_id]->dev);
 dec_end:
 	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index 9ca68cde4..970c0a30e 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -533,13 +533,12 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
 
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
-	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
-	pm_runtime_put(cjpeg->dev);
 	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
 	atomic_inc(&master_jpeg->hw_rdy);
 	wake_up(&master_jpeg->hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegdec_put_buf(cjpeg);
+	pm_runtime_put(cjpeg->dev);
 }
 
 static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
@@ -547,7 +546,6 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct mtk_jpeg_src_buf *jpeg_src_buf;
 	enum vb2_buffer_state buf_state;
-	struct mtk_jpeg_ctx *ctx;
 	u32 dec_irq_ret;
 	u32 irq_status;
 	int i;
@@ -557,7 +555,6 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 
 	cancel_delayed_work(&jpeg->job_timeout_work);
 
-	ctx = jpeg->hw_param.curr_ctx;
 	src_buf = jpeg->hw_param.src_buffer;
 	dst_buf = jpeg->hw_param.dst_buffer;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
@@ -580,12 +577,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	buf_state = VB2_BUF_STATE_DONE;
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegdec_put_buf(jpeg);
-	pm_runtime_put(ctx->jpeg->dev);
-	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
 
 	jpeg->hw_state = MTK_JPEG_HW_IDLE;
 	wake_up(&master_jpeg->hw_wq);
 	atomic_inc(&master_jpeg->hw_rdy);
+	pm_runtime_put(jpeg->dev);
 
 	return IRQ_HANDLED;
 }
@@ -673,11 +669,58 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int mtk_jpegdec_pm_suspend(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(jpeg->jdec_clk.clk_num,
+				   jpeg->jdec_clk.clks);
+
+	return 0;
+}
+
+static int mtk_jpegdec_pm_resume(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(jpeg->jdec_clk.clk_num,
+				       jpeg->jdec_clk.clks);
+}
+
+static int mtk_jpegdec_suspend(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	v4l2_m2m_suspend(jpeg->master_dev->m2m_dev);
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int mtk_jpegdec_resume(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret < 0)
+		return ret;
+
+	v4l2_m2m_resume(jpeg->master_dev->m2m_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops mtk_jpegdec_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(mtk_jpegdec_suspend, mtk_jpegdec_resume)
+	RUNTIME_PM_OPS(mtk_jpegdec_pm_suspend, mtk_jpegdec_pm_resume, NULL)
+};
+
 static struct platform_driver mtk_jpegdec_hw_driver = {
 	.probe = mtk_jpegdec_hw_probe,
 	.driver = {
 		.name = "mtk-jpegdec-hw",
 		.of_match_table = mtk_jpegdec_hw_ids,
+		.pm             = &mtk_jpegdec_pm_ops,
 	},
 };
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index a3f414f92..5f5f071d8 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -264,13 +264,12 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
 
 	mtk_jpeg_enc_reset(cjpeg->reg_base);
-	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
-	pm_runtime_put(cjpeg->dev);
 	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
 	atomic_inc(&master_jpeg->hw_rdy);
 	wake_up(&master_jpeg->hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegenc_put_buf(cjpeg);
+	pm_runtime_put(cjpeg->dev);
 }
 
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
@@ -304,12 +303,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	buf_state = VB2_BUF_STATE_DONE;
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegenc_put_buf(jpeg);
-	pm_runtime_put(ctx->jpeg->dev);
-	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
 
 	jpeg->hw_state = MTK_JPEG_HW_IDLE;
 	wake_up(&master_jpeg->hw_wq);
 	atomic_inc(&master_jpeg->hw_rdy);
+	pm_runtime_put(jpeg->dev);
 
 	return IRQ_HANDLED;
 }
@@ -395,11 +393,58 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int mtk_jpegenc_pm_suspend(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(jpeg->venc_clk.clk_num,
+				   jpeg->venc_clk.clks);
+
+	return 0;
+}
+
+static int mtk_jpegenc_pm_resume(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(jpeg->venc_clk.clk_num,
+				       jpeg->venc_clk.clks);
+}
+
+static int mtk_jpegenc_suspend(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	v4l2_m2m_suspend(jpeg->master_dev->m2m_dev);
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int mtk_jpegenc_resume(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret < 0)
+		return ret;
+
+	v4l2_m2m_resume(jpeg->master_dev->m2m_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops mtk_jpegenc_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(mtk_jpegenc_suspend, mtk_jpegenc_resume)
+	RUNTIME_PM_OPS(mtk_jpegenc_pm_suspend, mtk_jpegenc_pm_resume, NULL)
+};
+
 static struct platform_driver mtk_jpegenc_hw_driver = {
 	.probe = mtk_jpegenc_hw_probe,
 	.driver = {
 		.name = "mtk-jpegenc-hw",
 		.of_match_table = mtk_jpegenc_drv_ids,
+		.pm = &mtk_jpegenc_pm_ops,
 	},
 };
 
-- 
2.51.0.windows.2

[PATCH v16 06/12] media: mediatek: jpeg: fix buffer state update timing

[Kyrie Wu]

Update the destination buffer state only after the decoder has selected
a hardware core and resumed its runtime PM state successfully. This avoids
assigning frame tracking data to buffers that will be returned early, such
as when a source resolution change is detected.

Protect the destination buffer state update with the hardware spinlock so
the completion path observes a consistent context and frame number. Stop
walking the done queue after completing the next expected frame, since the
remaining buffers still need to wait for their own frame order.

Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c   | 9 +++------
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c | 1 +
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c | 1 +
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index a5a1f6126..e152ebae0 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1735,7 +1735,6 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
 	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
-	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
 
 	if (mtk_jpeg_check_resolution_change(ctx,
 					     &jpeg_src_buf->dec_param)) {
@@ -1744,11 +1743,6 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 		goto getbuf_fail;
 	}
 
-	jpeg_src_buf->curr_ctx = ctx;
-	jpeg_src_buf->frame_num = ctx->total_frame_num;
-	jpeg_dst_buf->curr_ctx = ctx;
-	jpeg_dst_buf->frame_num = ctx->total_frame_num;
-
 	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
 	ret = pm_runtime_resume_and_get(comp_jpeg[hw_id]->dev);
 	if (ret < 0) {
@@ -1773,6 +1767,9 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
 	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
+	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
+	jpeg_dst_buf->curr_ctx = ctx;
+	jpeg_dst_buf->frame_num = ctx->total_frame_num;
 	ctx->total_frame_num++;
 	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
 	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index 970c0a30e..426150bff 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -513,6 +513,7 @@ static void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
 				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
 						  VB2_BUF_STATE_DONE);
 				ctx->last_done_frame_num++;
+				break;
 			}
 		}
 	}
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index 5f5f071d8..eb34698d2 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -242,6 +242,7 @@ static void mtk_jpegenc_put_buf(struct mtk_jpegenc_comp_dev *jpeg)
 				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
 						  VB2_BUF_STATE_DONE);
 				ctx->last_done_frame_num++;
+				break;
 			}
 		}
 	}
-- 
2.51.0.windows.2

[PATCH v16 07/12] media: mediatek: jpeg: fix resolution change event handling in decoder

[Kyrie Wu]

This patch refines the handling of resolution change events within
JPEG decoder worker. The `mtk_jpeg_set_queue_data` function is now
called to set up queue data before signaling a source change through
`mtk_jpeg_queue_src_chg_event`. By reorganizing these calls, the
patch ensures that necessary queue information is updated prior to
transitioning the context state to `MTK_JPEG_SOURCE_CHANGE`.
A condition is added to exit early if the context is already in the
`MTK_JPEG_SOURCE_CHANGE` state, preventing redundant operations and
improving processing efficiency.

Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index e152ebae0..edd9e2d0a 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1738,11 +1738,15 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 
 	if (mtk_jpeg_check_resolution_change(ctx,
 					     &jpeg_src_buf->dec_param)) {
-		mtk_jpeg_queue_src_chg_event(ctx);
+		mtk_jpeg_set_queue_data(ctx, &jpeg_src_buf->dec_param);
 		ctx->state = MTK_JPEG_SOURCE_CHANGE;
+		mtk_jpeg_queue_src_chg_event(ctx);
 		goto getbuf_fail;
 	}
 
+	if (ctx->state == MTK_JPEG_SOURCE_CHANGE)
+		goto getbuf_fail;
+
 	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
 	ret = pm_runtime_resume_and_get(comp_jpeg[hw_id]->dev);
 	if (ret < 0) {
-- 
2.51.0.windows.2

[PATCH v16 08/12] media: mediatek: jpeg: fix remove buffer removal timing for multi-core

[Kyrie Wu]

Move calls to v4l2_m2m_src/dst_buf_remove() inside of the spinlock
protected scope to ensure all necessary operations are performed
before buffers are removed from their queues and ensure proper
synchronization of buffer handling to avoid buffer lost.

Fixes: 86379bd9d399 ("media: mtk-jpeg: Fixes jpeg enc&dec worker sw flow")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index edd9e2d0a..0907960db 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1654,9 +1654,6 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 		goto enc_end;
 	}
 
-	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-
 	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
 			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
@@ -1674,6 +1671,8 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 			     &src_buf->vb2_buf);
 	mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base);
 	mtk_jpeg_enc_start(comp_jpeg[hw_id]->reg_base);
+	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
 
@@ -1755,9 +1754,6 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 		goto dec_end;
 	}
 
-	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-
 	mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
 	if (mtk_jpeg_set_dec_dst(ctx,
 				 &jpeg_src_buf->dec_param,
@@ -1782,6 +1778,8 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 				jpeg_src_buf->bs_size,
 				&bs,
 				&fb);
+	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
-- 
2.51.0.windows.2

[PATCH v16 09/12] media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgdec compatible

[Kyrie Wu]

Compared to the previous generation IC, the MT8196 uses SMMU
instead of IOMMU and supports features such as dynamic voltage
and frequency scaling. Therefore, add "mediatek,mt8196-jpgdec"
compatible to the binding document.

Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/media/mediatek,mt8195-jpegdec.yaml           | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegdec.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegdec.yaml
index e5448c60e..28a9a9bfd 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegdec.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegdec.yaml
@@ -14,7 +14,9 @@ description:
 
 properties:
   compatible:
-    const: mediatek,mt8195-jpgdec
+    enum:
+      - mediatek,mt8195-jpgdec
+      - mediatek,mt8196-jpgdec
 
   power-domains:
     maxItems: 1
@@ -44,7 +46,9 @@ patternProperties:
 
     properties:
       compatible:
-        const: mediatek,mt8195-jpgdec-hw
+        enum:
+          - mediatek,mt8195-jpgdec-hw
+          - mediatek,mt8196-jpgdec-hw
 
       reg:
         maxItems: 1
-- 
2.51.0.windows.2

[PATCH v16 10/12] media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible

[Kyrie Wu]

Compared to the previous generation IC, the MT8196 uses SMMU
instead of IOMMU and supports features such as dynamic voltage
and frequency scaling. Therefore, add "mediatek,mt8196-jpgenc"
compatible to the binding document.

Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/media/mediatek,mt8195-jpegenc.yaml           | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
index 596186497..e2d772ea0 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
@@ -14,7 +14,9 @@ description:
 
 properties:
   compatible:
-    const: mediatek,mt8195-jpgenc
+    enum:
+      - mediatek,mt8195-jpgenc
+      - mediatek,mt8196-jpgenc
 
   power-domains:
     maxItems: 1
@@ -44,7 +46,9 @@ patternProperties:
 
     properties:
       compatible:
-        const: mediatek,mt8195-jpgenc-hw
+        enum:
+          - mediatek,mt8195-jpgenc-hw
+          - mediatek,mt8196-jpgenc-hw
 
       reg:
         maxItems: 1
-- 
2.51.0.windows.2

[PATCH v16 11/12] media: mediatek: jpeg: add jpeg compatible

[Kyrie Wu]

Add jpeg dec and enc compatible for mt8196

Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 36 +++++++++++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  |  3 ++
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  3 ++
 3 files changed, 42 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 0907960db..8e96501a4 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1919,6 +1919,20 @@ static struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = {
 	.jpeg_worker = mtk_jpegenc_worker,
 };
 
+static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
+	.formats = mtk_jpeg_enc_formats,
+	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
+	.qops = &mtk_jpeg_enc_qops,
+	.m2m_ops = &mtk_jpeg_multicore_enc_m2m_ops,
+	.dev_name = "mtk-jpeg-enc",
+	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.multi_core = true,
+	.jpeg_worker = mtk_jpegenc_worker,
+	.support_34bit = true,
+};
+
 static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
 	.formats = mtk_jpeg_dec_formats,
 	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
@@ -1932,6 +1946,20 @@ static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
 	.jpeg_worker = mtk_jpegdec_worker,
 };
 
+static const struct mtk_jpeg_variant mtk8196_jpegdec_drvdata = {
+	.formats = mtk_jpeg_dec_formats,
+	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
+	.qops = &mtk_jpeg_dec_qops,
+	.m2m_ops = &mtk_jpeg_multicore_dec_m2m_ops,
+	.dev_name = "mtk-jpeg-dec",
+	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
+	.multi_core = true,
+	.jpeg_worker = mtk_jpegdec_worker,
+	.support_34bit = true,
+};
+
 static const struct of_device_id mtk_jpeg_match[] = {
 	{
 		.compatible = "mediatek,mt8173-jpgdec",
@@ -1953,6 +1981,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mt8195-jpgdec",
 		.data = &mtk8195_jpegdec_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8196-jpgenc",
+		.data = &mtk8196_jpegenc_drvdata,
+	},
+	{
+		.compatible = "mediatek,mt8196-jpgdec",
+		.data = &mtk8196_jpegdec_drvdata,
+	},
 	{},
 };
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index 426150bff..07452249b 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -45,6 +45,9 @@ static const struct of_device_id mtk_jpegdec_hw_ids[] = {
 	{
 		.compatible = "mediatek,mt8195-jpgdec-hw",
 	},
+	{
+		.compatible = "mediatek,mt8196-jpgdec-hw",
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_jpegdec_hw_ids);
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index eb34698d2..c04988bd5 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -52,6 +52,9 @@ static const struct of_device_id mtk_jpegenc_drv_ids[] = {
 	{
 		.compatible = "mediatek,mt8195-jpgenc-hw",
 	},
+	{
+		.compatible = "mediatek,mt8196-jpgenc-hw",
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
-- 
2.51.0.windows.2

[PATCH v16 12/12] media: mediatek: jpeg: add jpeg smmu sid setting

[Kyrie Wu]

Add a configuration to set jpeg dec & enc smmu sid

Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 29 +++++++++++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_core.h    | 15 ++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 23 +++++++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 23 +++++++++++++++
 4 files changed, 90 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 8e96501a4..bb9bdd991 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -15,6 +15,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <media/v4l2-event.h>
@@ -1602,6 +1603,22 @@ static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
 	return IRQ_HANDLED;
 }
 
+static void mtk_jpeg_set_smmu_sid(struct regmap *smmu_regmap, unsigned int sid)
+{
+	if (sid == JPG_REG_GUSER_ID_DEC_SID)
+		regmap_update_bits(smmu_regmap, JPEG_DEC_SMMU_SID,
+				   JPG_REG_GUSER_ID_MASK <<
+				   JPG_REG_DEC_GUSER_ID_SHIFT,
+				   JPG_REG_GUSER_ID_DEC_SID <<
+				   JPG_REG_DEC_GUSER_ID_SHIFT);
+	else
+		regmap_update_bits(smmu_regmap, JPEG_ENC_SMMU_SID,
+				   JPG_REG_GUSER_ID_MASK <<
+				   JPG_REG_ENC_GUSER_ID_SHIFT,
+				   JPG_REG_GUSER_ID_ENC_SID <<
+				   JPG_REG_ENC_GUSER_ID_SHIFT);
+}
+
 static void mtk_jpegenc_worker(struct work_struct *work)
 {
 	struct mtk_jpegenc_comp_dev *comp_jpeg[MTK_JPEGENC_HW_MAX];
@@ -1663,6 +1680,11 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 	jpeg_dst_buf->frame_num = ctx->total_frame_num;
 	ctx->total_frame_num++;
 	mtk_jpeg_enc_reset(comp_jpeg[hw_id]->reg_base);
+
+	if (jpeg->variant->support_smmu && comp_jpeg[hw_id]->smmu_regmap)
+		mtk_jpeg_set_smmu_sid(comp_jpeg[hw_id]->smmu_regmap,
+				      JPG_REG_GUSER_ID_ENC_SID);
+
 	mtk_jpeg_set_enc_dst(ctx,
 			     comp_jpeg[hw_id]->reg_base,
 			     &dst_buf->vb2_buf);
@@ -1772,6 +1794,11 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 	jpeg_dst_buf->frame_num = ctx->total_frame_num;
 	ctx->total_frame_num++;
 	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
+
+	if (jpeg->variant->support_smmu && comp_jpeg[hw_id]->smmu_regmap)
+		mtk_jpeg_set_smmu_sid(comp_jpeg[hw_id]->smmu_regmap,
+				      JPG_REG_GUSER_ID_DEC_SID);
+
 	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
 				jpeg->variant->support_34bit,
 				&jpeg_src_buf->dec_param,
@@ -1931,6 +1958,7 @@ static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
 	.multi_core = true,
 	.jpeg_worker = mtk_jpegenc_worker,
 	.support_34bit = true,
+	.support_smmu = true,
 };
 
 static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
@@ -1958,6 +1986,7 @@ static const struct mtk_jpeg_variant mtk8196_jpegdec_drvdata = {
 	.multi_core = true,
 	.jpeg_worker = mtk_jpegdec_worker,
 	.support_34bit = true,
+	.support_smmu = true,
 };
 
 static const struct of_device_id mtk_jpeg_match[] = {
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 148fd4175..186cd1862 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
@@ -11,6 +11,7 @@
 
 #include <linux/clk.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fh.h>
@@ -34,6 +35,14 @@
 
 #define MTK_JPEG_MAX_EXIF_SIZE	(64 * 1024)
 
+#define JPEG_DEC_SMMU_SID				0
+#define JPEG_ENC_SMMU_SID				0
+#define JPG_REG_GUSER_ID_MASK			0x7
+#define JPG_REG_GUSER_ID_DEC_SID		0x4
+#define JPG_REG_GUSER_ID_ENC_SID		0x5
+#define JPG_REG_DEC_GUSER_ID_SHIFT		8
+#define JPG_REG_ENC_GUSER_ID_SHIFT		4
+
 #define MTK_JPEG_ADDR_MASK GENMASK(1, 0)
 
 /**
@@ -65,6 +74,7 @@ enum mtk_jpeg_ctx_state {
  * @multi_core:		mark jpeg hw is multi_core or not
  * @jpeg_worker:		jpeg dec or enc worker
  * @support_34bit:	flag to check support for 34-bit DMA address
+ * @support_smmu:	flag to check if support smmu
  */
 struct mtk_jpeg_variant {
 	struct clk_bulk_data *clks;
@@ -82,6 +92,7 @@ struct mtk_jpeg_variant {
 	bool multi_core;
 	void (*jpeg_worker)(struct work_struct *work);
 	bool support_34bit;
+	bool support_smmu;
 };
 
 struct mtk_jpeg_src_buf {
@@ -150,6 +161,7 @@ struct mtk_jpegdec_clk {
  * @hw_param:		jpeg encode hw parameters
  * @hw_state:		record hw state
  * @hw_lock:		spinlock protecting the hw device resource
+ * @smmu_regmap:	SMMU registers mapping
  */
 struct mtk_jpegenc_comp_dev {
 	struct device *dev;
@@ -163,6 +175,7 @@ struct mtk_jpegenc_comp_dev {
 	enum mtk_jpeg_hw_state hw_state;
 	/* spinlock protecting the hw device resource */
 	spinlock_t hw_lock;
+	struct regmap *smmu_regmap;
 };
 
 /**
@@ -177,6 +190,7 @@ struct mtk_jpegenc_comp_dev {
  * @hw_param:			jpeg decode hw parameters
  * @hw_state:			record hw state
  * @hw_lock:			spinlock protecting hw
+ * @smmu_regmap:		SMMU registers mapping
  */
 struct mtk_jpegdec_comp_dev {
 	struct device *dev;
@@ -190,6 +204,7 @@ struct mtk_jpegdec_comp_dev {
 	enum mtk_jpeg_hw_state hw_state;
 	/* spinlock protecting the hw device resource */
 	spinlock_t hw_lock;
+	struct regmap *smmu_regmap;
 };
 
 /**
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index 07452249b..3058fdf4f 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -613,6 +613,25 @@ static int mtk_jpegdec_hw_init_irq(struct mtk_jpegdec_comp_dev *dev)
 	return 0;
 }
 
+static int mtk_jpegdec_smmu_init(struct mtk_jpegdec_comp_dev *dev)
+{
+	struct mtk_jpeg_dev *master_dev = dev->master_dev;
+
+	if (!master_dev->variant->support_smmu)
+		return 0;
+
+	dev->smmu_regmap =
+		syscon_regmap_lookup_by_phandle(dev->plat_dev->dev.of_node,
+						"mediatek,smmu-config");
+	if (IS_ERR(dev->smmu_regmap)) {
+		return dev_err_probe(dev->dev, PTR_ERR(dev->smmu_regmap),
+				     "mmap smmu_base failed(%pe)\n",
+				     dev->smmu_regmap);
+	}
+
+	return 0;
+}
+
 static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 {
 	struct mtk_jpegdec_clk *jpegdec_clk;
@@ -665,6 +684,10 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 	master_dev->reg_decbase[i] = dev->reg_base;
 	dev->master_dev = master_dev;
 
+	ret = mtk_jpegdec_smmu_init(dev);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index c04988bd5..d98a83212 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -339,6 +339,25 @@ static int mtk_jpegenc_hw_init_irq(struct mtk_jpegenc_comp_dev *dev)
 	return 0;
 }
 
+static int mtk_jpegenc_smmu_init(struct mtk_jpegenc_comp_dev *dev)
+{
+	struct mtk_jpeg_dev *master_dev = dev->master_dev;
+
+	if (!master_dev->variant->support_smmu)
+		return 0;
+
+	dev->smmu_regmap =
+		syscon_regmap_lookup_by_phandle(dev->plat_dev->dev.of_node,
+						"mediatek,smmu-config");
+	if (IS_ERR(dev->smmu_regmap)) {
+		return dev_err_probe(dev->dev, PTR_ERR(dev->smmu_regmap),
+				     "mmap smmu_base failed(%pe)\n",
+				     dev->smmu_regmap);
+	}
+
+	return 0;
+}
+
 static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 {
 	struct mtk_jpegenc_clk *jpegenc_clk;
@@ -389,6 +408,10 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 	master_dev->reg_encbase[i] = dev->reg_base;
 	dev->master_dev = master_dev;
 
+	ret = mtk_jpegenc_smmu_init(dev);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 
-- 
2.51.0.windows.2

Re: [PATCH v16 01/12] media: mediatek: jpeg: fix jpeg cores' amounts setting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:

New issues:
- [High] Stack buffer overflow due to unbound loop over `max_hw_count` into fixed-size stack arrays.
- [High] Overwriting `hw_rdy` to 0 after child hardware probes guarantees a permanent deadlock of the workqueues.
- [Medium] Missing `wake_up` on `hw_wq` in component probe functions causes worker threads to sleep indefinitely.

Pre-existing issues:
- [High] Null pointer dereference in hardware selection if a child device probe is deferred.
- [High] Use-After-Free of `m2m_dev` on module removal due to asynchronous workqueue flush.
- [High] Heap out-of-bounds writes in child probe functions due to unconstrained `hw_index`.
- [High] Failed child device probe leaves a dangling pointer in the parent's hardware array.
- [Medium] Mismatched Runtime PM references between parent and child devices cause usage counter leaks and underflows.
- [Medium] Leaked Runtime PM usage counter on `pm_runtime_get_sync` failure paths.
--

--- Patch [1]: [PATCH v16 01/12] media: mediatek: jpeg: fix jpeg cores' amounts setting ---
commit 87a5b6564c9380ac04a07956302ca1970a500438
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix jpeg cores' amounts setting
    
Different ICs have different amounts of cores,
use a variable to set the cores' amounts.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1325,7 +1325,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>  		for_each_child_of_node(pdev->dev.of_node, child)
>  			num_child++;
>  
> -		atomic_set(&jpeg->hw_rdy, num_child);
> +		jpeg->max_hw_count = num_child;

[Severity: High]
Can this overflow fixed-size arrays? num_child comes directly from the
device tree without bounds checking. Later, mtk_jpegenc_worker() and
mtk_jpegdec_worker() use jpeg->max_hw_count to loop and copy pointers into
fixed-size stack arrays like comp_jpeg[MTK_JPEGENC_HW_MAX].

If the device tree specifies more than MTK_JPEGENC_HW_MAX (2) children,
won't this blindly write past the end of the stack array?

> +		atomic_set(&jpeg->hw_rdy, 0);

[Severity: High]
Does this overwrite cause a permanent deadlock? Since
devm_of_platform_populate() runs synchronously before this code, the child
probes will have already incremented jpeg->hw_rdy correctly.

By resetting it to 0 here, the correct readiness count is wiped out. When a
worker subsequently finds an idle core and decrements hw_rdy to -1, won't
future jobs block forever on wait_event_interruptible(jpeg->hw_wq, ... > 0)?

>  		atomic_set(&jpeg->hw_index, 0);
>  
>  		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,

[ ... ]

> @@ -1462,7 +1463,7 @@ static int mtk_jpegenc_get_hw(struct mtk_jpeg_ctx *ctx)
>  	int i;
>  
>  	spin_lock_irqsave(&jpeg->hw_lock, flags);
> -	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++) {
> +	for (i = 0; i < jpeg->max_hw_count; i++) {
>  		comp_jpeg = jpeg->enc_hw_dev[i];
>  		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to dereference
comp_jpeg unconditionally here?

If a child device probe defers, the array entry in jpeg->enc_hw_dev[i]
remains NULL. Because devm_of_platform_populate() successfully returns
anyway and userspace can queue jobs, won't mtk_jpegenc_get_hw() oops when
accessing comp_jpeg->hw_state?

>  			hw_id = i;

[ ... ]

> @@ -1592,7 +1593,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  		jpeg_work);
>  	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
>  
> -	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++)
> +	for (i = 0; i < jpeg->max_hw_count; i++)
>  		comp_jpeg[i] = jpeg->enc_hw_dev[i];

[Severity: High]
Can this loop write out-of-bounds? The stack array comp_jpeg is bounded by
MTK_JPEGENC_HW_MAX, but the loop condition is now jpeg->max_hw_count.

If the device tree provides more child nodes than the hardcoded maximum,
won't this corrupt the stack?

[Severity: High]
This isn't a bug introduced by this patch, but does the worker accessing
jpeg->m2m_dev on module removal cause a use-after-free?

jpeg->workqueue is registered via devm_add_action_or_reset(), so it is
destroyed after the remove callback returns. However, mtk_jpeg_remove()
explicitly calls v4l2_m2m_release(jpeg->m2m_dev), freeing the memory.

When devres synchronously flushes the workqueue, mtk_jpegenc_worker() calls
v4l2_m2m_job_finish(jpeg->m2m_dev, ...) and accesses the freed pointer.

[Severity: Medium]
This isn't a bug introduced by this patch, but does the worker mismanage
Runtime PM references?

The worker correctly acquires the child's PM reference via
pm_runtime_get_sync(comp_jpeg[hw_id]->dev). However, mtk_jpeg_enc_done()
incorrectly calls pm_runtime_put(ctx->jpeg->dev) on the parent device.

Does this cause the child's counter to leak and the parent's to underflow?

[Severity: Medium]
This isn't a bug introduced by this patch, but does the worker leak the
Runtime PM usage counter on failure?

If pm_runtime_get_sync() fails, the code jumps directly to the enc_end
label without dropping the implicitly acquired usage counter, leaving it
permanently leaked.

>  	i = 0;

[ ... ]

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -389,6 +389,8 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	atomic_inc(&master_dev->hw_rdy);
> +

[Severity: Medium]
Does this need to wake up the workqueue? If a child device defers probing and
finishes asynchronously later, worker threads might already be waiting on
master_dev->hw_wq.

Without a corresponding wake_up(&master_dev->hw_wq), won't those sleeping
workers wait indefinitely?

[Severity: High]
This isn't a bug introduced by this patch, but does this probe function write
out-of-bounds on the parent array?

Earlier in the function, hw_index is obtained via
atomic_add_return(1, &master_dev->hw_index) - 1. This index is
unconditionally used to write pointers into master_dev->enc_hw_dev[i].

If the device tree has more child nodes than MTK_JPEGENC_HW_MAX, won't this
exceed array bounds and corrupt the parent mtk_jpeg_dev structure?

[Severity: High]
This isn't a bug introduced by this patch, but does a failed probe leave a
dangling pointer?

The dev struct is assigned to master_dev->enc_hw_dev[i] before
mtk_jpegenc_smmu_init(dev) is called. If the SMMU init fails, the probe
returns an error without clearing the array entry.

Since devres will free the dev memory, won't this leave a dangling pointer
that workers will later dereference?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=1

Re: [PATCH v16 05/12] media: mediatek: jpeg: Fix multi-core clk suspend and resume setting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Unsafe global M2M device suspend/resume from component PM callbacks.
- [Low] Unconditional assignment of dev_pm_ops structures without pm_ptr(), leading to dead code.

Pre-existing issues:
- [Critical] Race condition between hardware IRQ handler and timeout worker.
- [High] Timeout handlers advertise hardware as IDLE before fully consuming hardware parameters, leading to queue corruption.
- [High] Use-After-Free due to dangling pointers on component probe failure.
--

--- Patch [5]: [PATCH v16 05/12] media: mediatek: jpeg: Fix multi-core clk suspend and resume setting ---
commit 11ea0f3a8e261149b85cc30379d74f6a788b6148
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Date:   Mon Jul 20 13:34:25 2026 +0000

    media: mediatek: jpeg: Fix multi-core clk suspend and resume setting

The suspend/resume callback function is defined in the dev_pm_ops
structure, which is defined in platform_driver. For multiple-core
architecture, each hardware driver will register a platform_driver
structure, so it is necessary to add a suspend/resume callback
function for each hardware to support this operation.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index 9ca68cde45a48..970c0a30e3659 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -533,13 +533,12 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
>  
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
> -	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
> -	pm_runtime_put(cjpeg->dev);
>  	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
>  	atomic_inc(&master_jpeg->hw_rdy);
>  	wake_up(&master_jpeg->hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegdec_put_buf(cjpeg);

[Severity: High]
This is a pre-existing issue, but does waking up waiters before
mtk_jpegdec_put_buf() consumes the hardware parameters risk queue corruption?

If a waiting worker thread wakes up and overwrites cjpeg->hw_param before
mtk_jpegdec_put_buf() executes, the newly overwritten parameters might be
used to complete the wrong buffer, leaving the actual timed-out buffer stuck.

> +	pm_runtime_put(cjpeg->dev);
>  }
>  
>  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> @@ -547,7 +546,6 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
>  	struct mtk_jpeg_src_buf *jpeg_src_buf;
>  	enum vb2_buffer_state buf_state;
> -	struct mtk_jpeg_ctx *ctx;
>  	u32 dec_irq_ret;
>  	u32 irq_status;
>  	int i;
> @@ -557,7 +555,6 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  
>  	cancel_delayed_work(&jpeg->job_timeout_work);

[Severity: Critical]
This is a pre-existing issue, but is there a race condition between this
hardware IRQ handler and the timeout worker?

If the hardware interrupt fires at the exact moment the job timeout worker
begins execution, cancel_delayed_work() will return false. Without further
synchronization, both threads might concurrently execute v4l2_m2m_buf_done(),
pm_runtime_put(), and atomic_inc() on the same job, which could lead to a
use-after-free or memory corruption.

[ ... ]
> @@ -673,11 +669,58 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
[ ... ]
> +static int mtk_jpegdec_resume(struct device *dev)
> +{
> +	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	v4l2_m2m_resume(jpeg->master_dev->m2m_dev);

[Severity: High]
Does calling v4l2_m2m_resume() from a component's resume callback unpause
the global master job queue prematurely?

If the system enters suspend, the first component to resume will unpause the
global queue. This might cause the worker thread to dispatch jobs to other
components that have not yet resumed, potentially leading to deadlocks or
queue corruption. Should the v4l2_m2m_suspend() and v4l2_m2m_resume() calls
be restricted to the master device instead?

> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops mtk_jpegdec_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(mtk_jpegdec_suspend, mtk_jpegdec_resume)
> +	RUNTIME_PM_OPS(mtk_jpegdec_pm_suspend, mtk_jpegdec_pm_resume, NULL)
> +};
> +
>  static struct platform_driver mtk_jpegdec_hw_driver = {
>  	.probe = mtk_jpegdec_hw_probe,
>  	.driver = {
>  		.name = "mtk-jpegdec-hw",
>  		.of_match_table = mtk_jpegdec_hw_ids,
> +		.pm             = &mtk_jpegdec_pm_ops,

[Severity: Low]
Does unconditionally assigning the pm pointer here generate dead code when
CONFIG_PM is disabled?

According to the subsystem guidelines, the dev_pm_ops structure pointer
should be wrapped with pm_ptr() to eliminate dead code and prevent compiler
warnings when PM support is disabled.

>  	},
>  };

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
probe failure in mtk_jpegdec_hw_probe()?

Looking at mtk_jpegdec_hw_probe():

	i = atomic_add_return(1, &master_dev->hw_index) - 1;
	master_dev->dec_hw_dev[i] = dev;
	master_dev->reg_decbase[i] = dev->reg_base;
	dev->master_dev = master_dev;

	ret = mtk_jpegdec_smmu_init(dev);
	if (ret)
		return ret;

If mtk_jpegdec_smmu_init() fails, the component's memory is freed, but it
remains in the master_dev->dec_hw_dev array. Could the master device's worker
later iterate through this array, find this available core, and dereference
the freed memory?

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> index 44a064dc7b3d0..4a8559c359536 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -263,13 +263,12 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
>  
>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
> -	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
> -	pm_runtime_put(cjpeg->dev);
>  	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
>  	atomic_inc(&master_jpeg->hw_rdy);
>  	wake_up(&master_jpeg->hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegenc_put_buf(cjpeg);

[Severity: High]
This is a pre-existing issue, but does waking up waiters before
mtk_jpegenc_put_buf() consumes the hardware parameters risk queue corruption?

If a waiting worker thread wakes up and overwrites cjpeg->hw_param before
mtk_jpegenc_put_buf() executes, the newly overwritten parameters might be
used to complete the wrong buffer, leaving the actual timed-out buffer stuck.

> +	pm_runtime_put(cjpeg->dev);
>  }
>  

[Severity: Critical]
This is a pre-existing issue, but does mtk_jpegenc_hw_irq_handler() share the
same race condition with the timeout worker as the decoder?

Similar to the decoder, if the hardware interrupt fires when the job timeout
worker begins execution, cancel_delayed_work() will return false, potentially
leading to use-after-free or memory corruption.

[ ... ]
> @@ -394,11 +392,58 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
[ ... ]
> +static int mtk_jpegenc_resume(struct device *dev)
> +{
> +	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	v4l2_m2m_resume(jpeg->master_dev->m2m_dev);

[Severity: High]
Does calling v4l2_m2m_resume() from a component's resume callback unpause
the global master job queue prematurely?

Similar to the decoder, if the system enters suspend, the first component to
resume will unpause the global queue, which could cause the worker thread to
dispatch jobs to other components that have not yet resumed.

> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops mtk_jpegenc_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(mtk_jpegenc_suspend, mtk_jpegenc_resume)
> +	RUNTIME_PM_OPS(mtk_jpegenc_pm_suspend, mtk_jpegenc_pm_resume, NULL)
> +};
> +
>  static struct platform_driver mtk_jpegenc_hw_driver = {
>  	.probe = mtk_jpegenc_hw_probe,
>  	.driver = {
>  		.name = "mtk-jpegenc-hw",
>  		.of_match_table = mtk_jpegenc_drv_ids,
> +		.pm = &mtk_jpegenc_pm_ops,

[Severity: Low]
Does unconditionally assigning the pm pointer here generate dead code when
CONFIG_PM is disabled?

The pm_ptr() macro should be used to wrap the pointer.

>  	},
>  };

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
probe failure in mtk_jpegenc_hw_probe()?

Looking at mtk_jpegenc_hw_probe():

	i = atomic_add_return(1, &master_dev->hw_index) - 1;
	master_dev->enc_hw_dev[i] = dev;
	master_dev->reg_encbase[i] = dev->reg_base;
	dev->master_dev = master_dev;

	ret = mtk_jpegenc_smmu_init(dev);
	if (ret)
		return ret;

If mtk_jpegenc_smmu_init() fails, the component's memory is freed, but it
remains in the master_dev->enc_hw_dev array. The master device's worker
could later iterate through this array and dereference the freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=5

Re: [PATCH v16 02/12] media: mediatek: jpeg: fix jpeg buffer payload size setting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Use of uppercase MIN() macro for variable arguments bypasses kernel type checking.

Pre-existing issues:
- [Critical] Premature `v4l2_m2m_job_finish` in worker threads breaks `stop_streaming` synchronization, leading to Use-After-Free of `ctx` and DMA into freed memory.
--

--- Patch [2]: [PATCH v16 02/12] media: mediatek: jpeg: fix jpeg buffer payload size setting ---
commit b1d3acc23e7ae23715523f548df3f13e353a48e9
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix jpeg buffer payload size setting

For multi-core jpegdec, if one core gets resolution change event,
the payload size, representing the size of Y/C data, needs to change.
But others are decoding at the same time and it can not be changed
immediately, which results in the payload size not matching the real
buffer length.
The payload size must be less than the real buffer length to avoid
warning logs.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 41c4cf8dc2626..34135706a4ac3 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c

[ ... ]

> @@ -710,12 +711,20 @@ static int mtk_jpeg_buf_prepare(struct vb2_buffer *vb)
>  
>  	for (i = 0; i < q_data->fmt->colplanes; i++) {
>  		plane_fmt = q_data->pix_mp.plane_fmt[i];
> +		max_size = plane_fmt.sizeimage;
> +
>  		if (ctx->enable_exif &&
> -		    q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG)
> -			vb2_set_plane_payload(vb, i, plane_fmt.sizeimage +
> -					      MTK_JPEG_MAX_EXIF_SIZE);
> -		else
> -			vb2_set_plane_payload(vb, i,  plane_fmt.sizeimage);
> +			q_data->fmt->fourcc == V4L2_PIX_FMT_JPEG) {
> +			max_size += MTK_JPEG_MAX_EXIF_SIZE;
> +
> +			vb2_set_plane_payload(vb, i,
> +					      MIN(vb->planes[i].length,
> +						  max_size));

[Severity: Low]
Is it appropriate to use the uppercase MIN() macro here? 

The MIN() macro in linux/minmax.h is generally intended for obvious constants
because it skips type checking.

Could this bypass type safety between size_t (max_size) and unsigned int
(length)? 

Would it be better to use min() with a cast, or min_t(size_t, ...) instead?

> +		} else {
> +			vb2_set_plane_payload(vb, i,
> +					      MIN(plane_fmt.sizeimage,
> +						  vb->planes[i].length));

[Severity: Low]
Does this also have the same type checking concern with the MIN() macro as
mentioned above?

> +		}
>  	}
>  
>  	return 0;

[Severity: Critical]
While reviewing this patch, I noticed a separate issue in the worker threads.
This isn't a bug introduced by this patch, but it appears that premature job
finishing could lead to a Use-After-Free.

In mtk_jpegdec_worker():

    mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
    v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);

If the worker calls v4l2_m2m_job_finish() and removes buffers from V4L2
context tracking while the hardware is still processing them asynchronously,
could an unprivileged user closing the file descriptor trigger a release?

If mtk_jpeg_release() is called, it frees the context:

    kfree(ctx);

Then when the hardware interrupt fires later in mtk_jpegdec_hw_irq_handler():

    ctx = jpeg->hw_param.curr_ctx;
    ...
    pm_runtime_put(ctx->jpeg->dev);

Doesn't this result in the interrupt handler unconditionally accessing the
freed 'ctx' pointer and invoking V4L2 completion on freed buffers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=2

Re: [PATCH v16 03/12] media: mediatek: jpeg: fix buffer structure size and layout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Closing the device while a multi-core encoding/decoding job is running leads to Use-After-Free of the context and vb2 buffers in the interrupt handler.
- [High] Orphaned vb2 buffers in `dst_done_queue` are leaked on STREAMOFF.
- [High] Failure to cancel `job_timeout_work` in driver remove causes Use-After-Free.
- [High] Flawed reorder queue iteration in `mtk_jpegenc_put_buf` leads to stranded buffers and application hangs when hardware completes frames out of order.
- [Medium] The custom buffer structure `mtk_jpeg_src_buf` manually replicates the fields of `struct v4l2_m2m_buffer` instead of embedding it, which is an architectural anti-pattern and risks future memory corruption.
--

--- Patch [3]: [PATCH v16 03/12] media: mediatek: jpeg: fix buffer structure size and layout ---
commit 4b1c7af5910f25b2a11680a53e22df54bd4fcacd
Author: Kyrie Wu <kyrie.wu@mediatek.com>

    media: mediatek: jpeg: fix buffer structure size and layout
    
    Updates the buf_struct_size in the JPEG driver to use the correct
    structure mtk_jpeg_src_buf instead of v4l2_m2m_buffer, ensuring
    proper handling of source buffers. The previous buffer size led to
    mismatches and potential issues during video buffer handling.
    
    Moves frame_num in the mtk_jpeg_src_buf structure to align with
    other members appropriately. The first field of the driver-specific
    buffer structure must be the subsystem-specificstruct
    (vb2_v4l2_buffer in the case of V4L2).
    
    Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
    Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
    Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 34135706a4ac3..7856a556ce68e 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1092,7 +1092,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>  	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
>  	dst_vq->drv_priv = ctx;
> -	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	dst_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
>  	dst_vq->ops = jpeg->variant->qops;
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> index 6be5cf30dea1b..148fd41759b75 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> @@ -85,10 +85,10 @@ struct mtk_jpeg_variant {
>  };
>  
>  struct mtk_jpeg_src_buf {
> -	u32 frame_num;
>  	struct vb2_v4l2_buffer b;
>  	struct list_head list;
>  	u32 bs_size;
> +	u32 frame_num;
>  	struct mtk_jpeg_dec_param dec_param;
>  
>  	struct mtk_jpeg_ctx *curr_ctx;
>  };

[Severity: Medium]
This isn't a bug introduced by this patch, but is it safe to manually
replicate the fields of struct v4l2_m2m_buffer here instead of embedding it
directly?

Since v4l2_m2m_buf_queue casts mtk_jpeg_src_buf to v4l2_m2m_buffer, if
upstream modifies struct v4l2_m2m_buffer (for example, by adding a new
field), this implicit structural casting could cause v4l2_m2m_buf_queue to
access invalid memory and lead to silent kernel heap corruption.

There are also a few pre-existing regressions in this driver that I noticed
while reviewing the code.

[Severity: High]
This is a pre-existing issue, but does closing the device while a multi-core
encoding or decoding job is running lead to a use-after-free?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpegenc_worker():

	mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base);
	mtk_jpeg_enc_start(comp_jpeg[hw_id]->reg_base);
	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);

The worker prematurely calls v4l2_m2m_job_finish() and clears the job running
state while the hardware is still running. If the user closes the device fd,
mtk_jpeg_release() will not wait for hardware completion and will immediately
free the context and queues.

When the hardware interrupt fires later:

drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c:mtk_jpegenc_hw_irq_handler():

	ctx = jpeg->hw_param.curr_ctx;
	src_buf = jpeg->hw_param.src_buffer;

The interrupt handler dereferences the context and buffers which may have
already been freed. Could this cause memory corruption?


[Severity: High]
This is a pre-existing issue, but can orphaned vb2 buffers leak when
streaming is stopped?

Looking at mtk_jpeg_enc_stop_streaming() in mtk_jpeg_core.c:

	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);

The function retrieves buffers from the active v4l2_m2m queues but appears to
fail to flush the ctx->dst_done_queue. If hardware operations complete out of
order, buffers wait in ctx->dst_done_queue. Does stopping the stream abandon
these pending buffers and cause a leak?


[Severity: High]
This is a pre-existing issue, but does the driver teardown logic fail to
cancel the delayed timeout work?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpeg_remove():

static void mtk_jpeg_remove(struct platform_device *pdev)
{
	struct mtk_jpeg_dev *jpeg = platform_get_drvdata(pdev);

	pm_runtime_disable(&pdev->dev);
        ...

The driver initializes a delayed work task (jpeg->job_timeout_work) to handle
hardware timeouts, but mtk_jpeg_remove() does not appear to call
cancel_delayed_work_sync(). If the driver is unbound while a job is timing
out, could the delayed work fire after the device structures are freed and
cause a use-after-free?


[Severity: High]
This is a pre-existing issue, but does the queue iteration logic in
mtk_jpegenc_put_buf() fail to restart its scan, leaving buffers stranded?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c:mtk_jpegenc_put_buf():

	while (!list_empty(&ctx->dst_done_queue) &&
	       (pos != &ctx->dst_done_queue)) {
		list_for_each_prev_safe(pos, temp_entry, &ctx->dst_done_queue) {
                    ...
                }
        }

When frames complete out of order (e.g., 8, 9, 7), the outer while loop checks
pos != &ctx->dst_done_queue. However, list_for_each_prev_safe() always
terminates with pos pointing to the list head. Since there is no break inside
the outer loop, the outer condition evaluates to false immediately after one
pass, failing to restart the scan.

Could this leave frames indefinitely stranded in the queue, causing the
application to hang? This same pattern also appears in the decoder side in
mtk_jpegdec_put_buf() in mtk_jpeg_dec_hw.c.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=3

Re: [PATCH v16 06/12] media: mediatek: jpeg: fix buffer state update timing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Moving destination buffer state initialization after scheduling the timeout worker introduces a race condition causing list corruption and a kernel oops.

Pre-existing issues:
- [High] The hardware timeout workers are scheduled before hardware start and completely lack lock synchronization, causing data races, hardware bus faults, and use-after-free vulnerabilities.
- [High] The encoder worker permanently leaks a Runtime PM usage reference when `pm_runtime_get_sync()` fails.
--

--- Patch [6]: [PATCH v16 06/12] media: mediatek: jpeg: fix buffer state update timing ---
commit adfbbc474f76db07fc27613935d0bb09341844c6
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix buffer state update timing

Update the destination buffer state only after the decoder has selected
a hardware core and resumed its runtime PM state successfully. This avoids
assigning frame tracking data to buffers that will be returned early, such
as when a source resolution change is detected.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index a5a1f6126af73..e152ebae0f219 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1773,6 +1767,9 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
>  	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
> +	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
> +	jpeg_dst_buf->curr_ctx = ctx;
> +	jpeg_dst_buf->frame_num = ctx->total_frame_num;

[Severity: High]
Does moving this initialization after schedule_delayed_work() create a race
condition that can corrupt the destination done queue?

If the worker thread is preempted right after calling schedule_delayed_work(),
the timeout worker (mtk_jpegdec_timeout_work) can fire early before
jpeg_dst_buf->frame_num is initialized.

The timeout worker will pass this uninitialized buffer to mtk_jpegdec_put_buf(),
which adds it to ctx->dst_done_queue. Because frame_num is uninitialized,
it won't match ctx->last_done_frame_num and the buffer gets stranded in the
queue.

When the worker thread resumes, it initializes frame_num and starts the
hardware. Upon hardware completion, the IRQ handler will call
mtk_jpegdec_put_buf() again and execute list_add_tail() on the exact same
buffer, corrupting the linked list.

[Severity: High]
This isn't a bug introduced by this patch, but could this delayed work
scheduling cause hardware bus faults or memory corruption?

The timeout worker is armed before the hardware lock is acquired and the
hardware is started. Since the timeout workers (both mtk_jpegdec_timeout_work
and mtk_jpegenc_timeout_work) do not acquire hw_lock, if the main worker is
preempted right after scheduling the work, the timeout worker can fire
concurrently.

It will reset the device, complete the buffers to userspace, and call
pm_runtime_put() to potentially power off the device. When the main worker
resumes, it will write to powered-off registers (causing a bus fault), and
instruct the hardware to DMA into buffers that userspace may have already
freed or reused.

[Severity: High]
This is a pre-existing issue, but is there a Runtime PM reference leak in the
encoder worker's failure path?

In mtk_jpegenc_worker() (not modified in this diff), pm_runtime_get_sync()
increments the device's PM usage counter even if it returns an error. If the
resume operation fails, the code branches to the enc_end error path and
returns without calling pm_runtime_put_noidle(). This leaves the usage
counter elevated and permanently prevents the device from suspending again.

>  	ctx->total_frame_num++;
>  	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
>  	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=6

Re: [PATCH v16 07/12] media: mediatek: jpeg: fix resolution change event handling in decoder

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Data race on V4L2 queue format structures due to unlocked modification from the asynchronous worker thread.

Pre-existing issues:
- [High] Infinite workqueue loop and STREAMOFF deadlock due to missing .job_ready callback in multicore decoder.
--

--- Patch [7]: [PATCH v16 07/12] media: mediatek: jpeg: fix resolution change event handling in decoder ---
commit e12d8cd3e85b476d842a0ea7879755769138198f
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix resolution change event handling in decoder

This patch refines the handling of resolution change events in the JPEG
decoder worker. It updates queue data before signaling the source change and
adds an early exit condition if already in the source change state.

> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1738,11 +1738,15 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  
>  	if (mtk_jpeg_check_resolution_change(ctx,
>  					     &jpeg_src_buf->dec_param)) {
> -		mtk_jpeg_queue_src_chg_event(ctx);
> +		mtk_jpeg_set_queue_data(ctx, &jpeg_src_buf->dec_param);

[Severity: High]
Does calling mtk_jpeg_set_queue_data() from the asynchronous worker thread
create a data race?

This function writes directly to the format structures like ctx->out_q and
ctx->cap_q. Standard V4L2 ioctls like VIDIOC_G_FMT and VIDIOC_TRY_FMT read
these exact structures and rely on the video device lock (vdev->lock) for
serialization.

Since the worker executes without acquiring the device lock, could this
cause userspace to read torn or mismatched dimension/size values if
VIDIOC_G_FMT is called concurrently?

>  		ctx->state = MTK_JPEG_SOURCE_CHANGE;
> +		mtk_jpeg_queue_src_chg_event(ctx);
>  		goto getbuf_fail;
>  	}
>  
> +	if (ctx->state == MTK_JPEG_SOURCE_CHANGE)
> +		goto getbuf_fail;

[Severity: High]
This is a pre-existing issue, but does jumping to getbuf_fail while
leaving buffers in the queue cause an infinite workqueue loop and
deadlock during VIDIOC_STREAMOFF?

In getbuf_fail, v4l2_m2m_job_finish() is called without removing the
source or destination buffers from the queue.

Looking at mtk_jpeg_multicore_dec_m2m_ops, it lacks a .job_ready callback:

static const struct v4l2_m2m_ops mtk_jpeg_multicore_dec_m2m_ops = {
	.device_run = mtk_jpeg_multicore_dec_device_run,
};

Because there is no .job_ready callback, the V4L2 M2M core assumes the
context is ready and immediately requeues the job. The worker will instantly
run again, hit this same early exit, and loop infinitely.

Additionally, since the buffers are never marked done or removed, could any
subsequent attempt by userspace to stop streaming deadlock permanently in
vb2_wait_for_all_buffers()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=7

Re: [PATCH v16 08/12] media: mediatek: jpeg: fix remove buffer removal timing for multi-core

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] List corruption and race condition due to delayed buffer removal from the V4L2 M2M ready queues.

Pre-existing issues:
- [High] Potential Use-After-Free due to scheduling timeout work before acquiring hardware locks.
- [High] Lack of synchronization between the hardware IRQ handler and job timeout worker leads to double completion.
- [High] Use-After-Free of `mtk_jpeg_ctx` during file descriptor release.
- [High] Unkillable deadlock in `stop_streaming` when buffers are queued but M2M job is not started.
--

--- Patch [8]: [PATCH v16 08/12] media: mediatek: jpeg: fix remove buffer removal timing for multi-core ---
commit ddec7ab7308507663bea72ead9e975996dce1888
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix remove buffer removal timing for multi-core

This commit moves the calls to v4l2_m2m_src_buf_remove() and
v4l2_m2m_dst_buf_remove() inside the spinlock-protected scope in the jpeg
encoder and decoder workers. This is intended to ensure buffers are properly
synchronized before removal from their queues to avoid buffer loss.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1654,9 +1654,6 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  		goto enc_end;
>  	}
>  
> -	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> -	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> -
>  	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
>  			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
>  	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);

[Severity: High]
This is a pre-existing issue, but does scheduling the timeout work before
acquiring hw_lock risk a use-after-free?

If the worker thread is preempted for more than the timeout duration just
after calling schedule_delayed_work(), the timeout work could execute,
reset the hardware, and return the buffers to userspace via
v4l2_m2m_buf_done().

Userspace might then free or reuse these buffers. When this worker thread
eventually resumes, it would blindly configure the hardware with the
invalidated buffer pointers and start the hardware operation, which could
lead to memory corruption. This appears to affect both mtk_jpegenc_worker()
and mtk_jpegdec_worker().

> @@ -1674,6 +1671,8 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  			     &src_buf->vb2_buf);
>  	mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base);
>  	mtk_jpeg_enc_start(comp_jpeg[hw_id]->reg_base);
> +	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> +	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);

[Severity: High]
Can this lead to list corruption if the hardware finishes very quickly?

Because v4l2_m2m_src_buf_remove() and v4l2_m2m_dst_buf_remove() are called
after mtk_jpeg_enc_start(), the hardware can raise an interrupt before the
buffers are removed from the ready queue.

Since the IRQ handler does not acquire hw_lock, it can run concurrently on
another CPU and transition the buffer to userspace via v4l2_m2m_buf_done().
If userspace quickly dequeues and requeues the buffer, list_add_tail() will
be invoked to insert it back into the ready queue.

Because this worker thread has not yet removed the original list node,
could this corrupt the doubly-linked lists? I notice this order is used in
mtk_jpegenc_worker(), but mtk_jpegdec_worker() correctly calls the remove
functions before starting the hardware.

>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);

[Severity: High]
This is a pre-existing issue, but does calling v4l2_m2m_job_finish()
synchronously here open a window for a use-after-free of the context?

If userspace closes the file descriptor while an encode or decode job is
running, mtk_jpeg_release() invokes v4l2_m2m_ctx_release(), which tries
to wait for running jobs by checking the TRANS_RUNNING flag.

Since v4l2_m2m_job_finish() is called immediately after starting the
hardware, the flag is cleared before the hardware actually finishes. This
causes mtk_jpeg_release() to proceed without waiting and free the context.

When the hardware interrupt eventually fires, the IRQ handler dereferences
the freed context:

mtk_jpegenc_hw_irq_handler()
    mtk_jpegenc_put_buf()
        ctx = jpeg->hw_param.curr_ctx;
        spin_lock_irqsave(&ctx->done_queue_lock, flags);
        
Can this lead to memory corruption when the lock is accessed on the freed
context?

[ ... ]

While reviewing this patch, I noticed a few other pre-existing issues in
related code.

[Severity: High]
This is a pre-existing issue, but is there a race condition between the
hardware IRQ handler and the job timeout worker?

In mtk_jpeg_enc_hw.c, the IRQ handler uses the asynchronous
cancel_delayed_work():

mtk_jpegenc_hw_irq_handler() {
    ...
    cancel_delayed_work(&jpeg->job_timeout_work);
    
    ctx = jpeg->hw_param.curr_ctx;
    ...
}

If the hardware interrupt fires at the exact moment the timeout worker
begins executing, cancel_delayed_work() returns false and does not wait.
Because neither the IRQ handler nor the timeout worker acquires hw_lock,
both will execute concurrently.

Both paths will read the same buffer pointers from hw_param and invoke
v4l2_m2m_buf_done(), which could result in a double completion of the V4L2
buffers and a double increment of the hw_rdy atomic counter.

[Severity: High]
This is a pre-existing issue, but can mtk_jpeg_enc_stop_streaming()
deadlock if buffers are queued but the M2M job is not started?

In mtk_jpeg_core.c:mtk_jpeg_enc_stop_streaming():

    if (jpeg->variant->multi_core)
        vb2_wait_for_all_buffers(q);

    while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
        v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);

If userspace queues buffers on only one queue (e.g., OUTPUT but not
CAPTURE), the V4L2 mem2mem core places the buffers in the driver's ready
queue but does not schedule the job.

If userspace then calls STREAMOFF, the wait loop precedes the cleanup
loop. vb2_wait_for_all_buffers() will block waiting for the driver to
return all buffers, but the code that returns the buffers is located after
the wait call.

Could this cause the process calling STREAMOFF to block forever in
uninterruptible sleep? The same logic appears to be present in
mtk_jpeg_dec_stop_streaming() as well.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=8

Re: [PATCH v16 10/12] media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Inaccurate documentation for the `iommus` property regarding the MT8196 SMMU change.
--

--- Patch [10]: [PATCH v16 10/12] media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible ---
commit 11c0f39e5d822c22a0e461d70a256b8d2035d561
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: dt-bindings: mediatek,jpeg: Add mediatek, mt8196-jpgenc compatible

This patch adds the mediatek,mt8196-jpgenc compatible string to the
MediaTek JPEG encoder binding. The commit message notes that the MT8196
uses an SMMU instead of an IOMMU.

> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> --- a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> @@ -14,14 +14,16 @@ description:
>  
>  properties:
>    compatible:
> -    const: mediatek,mt8195-jpgenc
> +    enum:
> +      - mediatek,mt8195-jpgenc
> +      - mediatek,mt8196-jpgenc
>  
>    power-domains:
>      maxItems: 1
>  
>    iommus:
>      maxItems: 4
>      description:
>        Points to the respective IOMMU block with master port as argument, see
>        Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.

[Severity: Low]
The commit message states that MT8196 uses an SMMU instead of an IOMMU, but the
documentation for properties.iommus still hardcodes references to
mediatek,iommu.yaml and specifies using a master port as argument.

Since SMMUs typically use stream IDs rather than master ports, does this
documentation regression need to be updated or generalized to remain accurate
for the newly added mt8196-jpgenc compatible?

[ ... ]

> @@ -44,14 +46,16 @@ patternProperties:
>  
>      properties:
>        compatible:
> -        const: mediatek,mt8195-jpgenc-hw
> +        enum:
> +          - mediatek,mt8195-jpgenc-hw
> +          - mediatek,mt8196-jpgenc-hw
>  
>        reg:
>          maxItems: 1
>  
>        iommus:
>          minItems: 1
>          maxItems: 32
>          description:
>            List of the hardware port in respective IOMMU block for current Socs.
>            Refer to bindings/iommu/mediatek,iommu.yaml.

[Severity: Low]
Similarly, the patternProperties.iommus description for the hardware nodes
still hardcodes the legacy mediatek,iommu.yaml reference.

Would it make sense to update this description as well to account for the
SMMU change described in the commit message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=10

Re: [PATCH v16 12/12] media: mediatek: jpeg: add jpeg smmu sid setting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Probe failure in SMMU initialization leaves dangling pointers in the global master device arrays and permanently increments hw_index, causing UAF and array OOB on deferred probes.

Pre-existing issues:
- [High] Blind dereference of HW arrays when child probes fail early.
--

--- Patch [12]: [PATCH v16 12/12] media: mediatek: jpeg: add jpeg smmu sid setting ---
commit 73b8529184733966196568926fff60cf629320ae
Author: Kyrie Wu <kyrie.wu@mediatek.com>

    media: mediatek: jpeg: add jpeg smmu sid setting
    
    Add a configuration to set jpeg dec & enc smmu sid

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 8e96501a4f9fa..bb9bdd9913d94 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1663,6 +1680,11 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	jpeg_dst_buf->frame_num = ctx->total_frame_num;
>  	ctx->total_frame_num++;
>  	mtk_jpeg_enc_reset(comp_jpeg[hw_id]->reg_base);
> +
> +	if (jpeg->variant->support_smmu && comp_jpeg[hw_id]->smmu_regmap)
> +		mtk_jpeg_set_smmu_sid(comp_jpeg[hw_id]->smmu_regmap,
> +				      JPG_REG_GUSER_ID_ENC_SID);
> +

[Severity: High]
This isn't a bug introduced by this patch, but I have a question about how
unpopulated hardware array entries are handled when a child probe fails.

In mtk_jpegenc_worker(), it calls mtk_jpegenc_get_hw() to select the hw_id.
If an earlier probe failure left the array entry in enc_hw_dev as NULL, could
this result in a NULL pointer dereference?

mtk_jpegenc_get_hw() iterates based on max_hw_count (which is set from the
Device Tree child nodes) and accesses the state directly:

drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpegenc_get_hw() {
    ...
    for (i = 0; i < jpeg->max_hw_count; i++) {
        comp_jpeg = jpeg->enc_hw_dev[i];
        if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
    ...
}

Could this blindly dereference comp_jpeg if the child probe never completed
and left it NULL? The exact same pattern seems to exist in mtk_jpegdec_get_hw().

>  	mtk_jpeg_set_enc_dst(ctx,
>  			     comp_jpeg[hw_id]->reg_base,
>  			     &dst_buf->vb2_buf);
[ ... ]
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index 07452249bcb67..3058fdf4f6054 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
[ ... ]
> @@ -665,6 +684,10 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  	master_dev->reg_decbase[i] = dev->reg_base;
>  	dev->master_dev = master_dev;
>  
> +	ret = mtk_jpegdec_smmu_init(dev);
> +	if (ret)
> +		return ret;
> +

[Severity: Critical]
If mtk_jpegdec_smmu_init() fails and returns an error like -EPROBE_DEFER,
will this leave a dangling pointer in the master_dev->dec_hw_dev array?

Looking earlier in mtk_jpegdec_hw_probe(), dev is allocated with devm_kzalloc
and added to the master_dev array, and hw_index is incremented:

    i = atomic_add_return(1, &master_dev->hw_index) - 1;
    master_dev->dec_hw_dev[i] = dev;
    master_dev->reg_decbase[i] = dev->reg_base;
    dev->master_dev = master_dev;

    ret = mtk_jpegdec_smmu_init(dev);
    if (ret)
        return ret;

When returning ret, the devm-managed dev will be freed, but the pointer
remains in master_dev->dec_hw_dev[i] and hw_index is never decremented.
Could this lead to an out-of-bounds write on deferred probe retries and a
use-after-free when the worker thread accesses the array?

This same issue seems to be present in mtk_jpegenc_hw_probe() in
mtk_jpeg_enc_hw.c as well.

>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=12

Re: [PATCH v16 04/12] media: mediatek: jpeg: Fix buffer completion on multi-core streaming stop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added calls to `vb2_wait_for_all_buffers(q)` in `mtk_jpeg_enc_stop_streaming()` and `mtk_jpeg_dec_stop_streaming()` introduce an unkillable deadlock (local denial of service) when there are unmatched buffers in the m2m queues.

Pre-existing issues:
- [High] A race condition between the hardware IRQ handler and the job timeout worker leads to a NULL pointer dereference and kernel panic.
--

--- Patch [4]: [PATCH v16 04/12] media: mediatek: jpeg: Fix buffer completion on multi-core streaming stop ---
commit 613fe46e184bc6ab593928ce530f8144c0d9af31
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: jpeg: Fix buffer completion on multi-core streaming stop

Enhances the Mediatek JPEG driver's stability and reliability by ensuring
that all queued buffers are processed before stopping the streaming in
multi-core environments. It introduces a call to vb2_wait_for_all_buffers()
in the stop_streaming functions to prevent data loss.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 7856a556ce68e..d0fb68bc884b4 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -850,8 +850,12 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
>  static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
>  {
>  	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
>  	struct vb2_v4l2_buffer *vb;
>  
> +	if (jpeg->variant->multi_core)
> +		vb2_wait_for_all_buffers(q);

[Severity: High]
Does this introduce a deadlock if userspace queues an unmatched number of
buffers, such as only OUTPUT buffers but no CAPTURE buffers, and then calls
VIDIOC_STREAMOFF?

When VIDIOC_STREAMOFF is called, the V4L2 framework invokes
v4l2_m2m_cancel_job(), which removes the context from the job queue. This
means no hardware worker will ever process the parked buffers.

Because the worker is cancelled, the buffers remain in the queue. Calling
vb2_wait_for_all_buffers() will block uninterruptibly waiting for the
owned_by_drv_count to drop to 0, which is structurally impossible here and
results in a hung task.

> +
>  	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
>  		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
>  }
> @@ -859,6 +863,7 @@ static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
>  static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
>  {
>  	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
>  	struct vb2_v4l2_buffer *vb;
>  
>  	/*
> @@ -866,6 +871,9 @@ static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
>  	 * Before STREAMOFF, we still have to return the old resolution and
>  	 * subsampling. Update capture queue when the stream is off.
>  	 */
> +	if (jpeg->variant->multi_core)
> +		vb2_wait_for_all_buffers(q);

[Severity: High]
Does this share the same hang issue as mtk_jpeg_enc_stop_streaming() above?

Waiting for buffers here after v4l2_m2m_cancel_job() has removed the context
from the job queue could similarly lead to an unkillable task if there are
unmatched buffers.

> +
>  	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
>  	    V4L2_TYPE_IS_CAPTURE(q->type)) {

[ ... ]

[Severity: High]
This is a pre-existing issue, but can this code encounter a NULL pointer
dereference if a hardware timeout occurs exactly when the hardware IRQ fires?

If the timeout occurs concurrently with the hardware IRQ, mtk_jpeg_enc_irq()
calls cancel_delayed_work().

Since this does not wait for the work to finish if it has already started
executing on another CPU, both mtk_jpeg_enc_done() and
mtk_jpeg_job_timeout_work() can execute concurrently:

CPU1 in mtk_jpeg_job_timeout_work():
    src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
    dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);

CPU2 in mtk_jpeg_enc_done():
    src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
    dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);

Because these removals are not externally coordinated, one thread will
retrieve NULL pointers for the buffers.

This thread then unconditionally dereferences the NULL dst_buf in
vb2_set_plane_payload() or v4l2_m2m_buf_done(), leading to a panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=4

Re: [PATCH v16 11/12] media: mediatek: jpeg: add jpeg compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check on the number of DT child nodes causes out-of-bounds array accesses and stack overflows.
- [High] Incorrect calculation of the 34-bit extension address for the encoder's stall address.
--

--- Patch [11]: [PATCH v16 11/12] media: mediatek: jpeg: add jpeg compatible ---
Note: The format of this report has been altered to provide a free-form
summary without quoting the original patch code, due to recitation
restrictions.

commit f9b0fac0a51678a6983018a9ca39e5cce2cb6604
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: jpeg: add jpeg compatible

This commit adds JPEG encoder and decoder compatibles for the mt8196
platform. It sets up the variant data structures with appropriate
capabilities like multi-core and 34-bit support.

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the new
compatibles, I noticed a potential stack overflow in the core driver.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpeg_probe(),
the code iterates over the device tree child nodes and sets
jpeg->max_hw_count to the total number of children, without capping it.

Later, in mtk_jpegenc_worker() and mtk_jpegdec_worker(), this unbounded
jpeg->max_hw_count is used as a loop limit to copy elements into the
comp_jpeg array, which is a fixed-size stack array. Additionally, in
mtk_jpegenc_hw_probe(), the hw_index is incremented and used without
bounds checking to store hardware instances into master_dev->enc_hw_dev.

Could a malformed device tree with more child nodes than the hardware limit
cause a stack buffer overflow or out-of-bounds write here? Should we add
bounds checking against MTK_JPEGENC_HW_MAX and MTK_JPEGDEC_HW_MAX?

[Severity: High]
This isn't a bug introduced by this patch, but adding support_34bit = true
for mt8196_jpegenc_drvdata exposes an existing issue in the encoder address
calculation.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c at
mtk_jpeg_set_enc_dst(), when 34-bit support is enabled, the code calculates
the extended address using FIELD_PREP on the upper 32 bits of dma_addr.

It then writes the sum of this extended address and the buffer size to the
JPEG_ENC_STALL_ADDR0_EXT register. Because the extended address is a masked
bitfield representing the upper 32 bits, adding the raw byte length size
directly to this bitfield seems incorrect.

Could this result in writing an invalid, unmasked address extension to the
hardware registers, potentially leading to SMMU faults or memory corruption?
Should this instead calculate the upper 32 bits of the sum of dma_addr
and size?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=11