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

[PATCH v15 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 decoding buffer number setting timing issue
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 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 decoding buffer number setting timing issue
  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    | 153 +++++++++++++-----
 .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  19 ++-
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  |  90 ++++++++++-
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  88 +++++++++-
 6 files changed, 310 insertions(+), 56 deletions(-)

-- 
2.45.2

[PATCH v15 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   | 8 ++++----
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h   | 2 ++
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c | 1 +
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c | 1 +
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d147ec483081..1e014fd698ae 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1462,7 +1462,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 +1509,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 +1592,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 +1687,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 02ed0ed5b736..6be5cf30dea1 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 32372781daf5..4534caeb104f 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -664,6 +664,7 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 	master_dev->dec_hw_dev[i] = dev;
 	master_dev->reg_decbase[i] = dev->reg_base;
 	dev->master_dev = master_dev;
+	master_dev->max_hw_count++;
 
 	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 b6f5b2249f1f..2765dafab4ad 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -386,6 +386,7 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 	master_dev->enc_hw_dev[i] = dev;
 	master_dev->reg_encbase[i] = dev->reg_base;
 	dev->master_dev = master_dev;
+	master_dev->max_hw_count++;
 
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
-- 
2.45.2

[PATCH v15 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 to not match the real
buffer length.
The payload size must less than the real buffer length to remove
the warnning logs.

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    | 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 1e014fd698ae..3960ad21d7f0 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.45.2

[PATCH v15 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 3960ad21d7f0..6d6a999a22fc 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 6be5cf30dea1..148fd41759b7 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.45.2

[PATCH v15 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>
---
 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 6d6a999a22fc..76a5b49b7f43 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.45.2

[PATCH v15 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  | 62 +++++++++++++++++--
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 60 ++++++++++++++++--
 3 files changed, 121 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 76a5b49b7f43..89048aba8dca 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);
 }
@@ -1647,13 +1653,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);
 
@@ -1750,20 +1749,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);
 
@@ -1773,7 +1765,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,
@@ -1794,9 +1786,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 4534caeb104f..9a8dbca6af00 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;
 }
@@ -668,15 +664,69 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
+	ret = devm_clk_bulk_get(dev->dev,
+				jpegdec_clk->clk_num,
+				jpegdec_clk->clks);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to init clk\n");
+		return ret;
+	}
 
 	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 2765dafab4ad..5d1c217fea0f 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;
 }
@@ -390,15 +388,69 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
+	ret = devm_clk_bulk_get(dev->dev,
+				jpegenc_clk->clk_num,
+				jpegenc_clk->clks);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to init clk\n");
+		return ret;
+	}
+
+	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.45.2

[PATCH v15 06/12] media: mediatek: jpeg: fix decoding buffer number setting timing issue

[Kyrie Wu]

The src buffer doesn't need set information and dst buf parameters
only need to set when the power set succussed and protect the
setting by spinlock ensuring that any later operations acting
on this buffer reflect accurate state and frame data.

Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface")
Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.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 89048aba8dca..4dc574e03bd5 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1734,7 +1734,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)) {
@@ -1743,11 +1742,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) {
@@ -1772,6 +1766,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 9a8dbca6af00..e4d2c5d4ec73 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 5d1c217fea0f..2adea3aca50b 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.45.2

[PATCH v15 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>
---
 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 4dc574e03bd5..02efaed198b8 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1737,11 +1737,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.45.2

[PATCH v15 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 02efaed198b8..0b9a3b66895a 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1653,9 +1653,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));
 
@@ -1673,6 +1670,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);
 
@@ -1754,9 +1753,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,
@@ -1781,6 +1777,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.45.2

[PATCH v15 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 e5448c60e3eb..28a9a9bfdbf8 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.45.2

[PATCH v15 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 596186497b68..e2d772ea0fb0 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.45.2

[PATCH v15 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 0b9a3b66895a..da7e723fd1f3 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1918,6 +1918,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,
@@ -1931,6 +1945,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",
@@ -1952,6 +1980,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 e4d2c5d4ec73..5a485e905787 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 2adea3aca50b..1cfdfac98829 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.45.2

[PATCH v15 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 da7e723fd1f3..5127a7e1e05c 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>
@@ -1601,6 +1602,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];
@@ -1662,6 +1679,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);
@@ -1771,6 +1793,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,
@@ -1930,6 +1957,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 = {
@@ -1957,6 +1985,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 148fd41759b7..186cd1862028 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 5a485e905787..2e079a6e5d5d 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(%ld)\n",
+				     PTR_ERR(dev->smmu_regmap));
+	}
+
+	return 0;
+}
+
 static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 {
 	struct mtk_jpegdec_clk *jpegdec_clk;
@@ -666,6 +685,10 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 	dev->master_dev = master_dev;
 	master_dev->max_hw_count++;
 
+	ret = mtk_jpegdec_smmu_init(dev);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 	ret = devm_clk_bulk_get(dev->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 1cfdfac98829..0001d69ae7ba 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(%ld)\n",
+				     PTR_ERR(dev->smmu_regmap));
+	}
+
+	return 0;
+}
+
 static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 {
 	struct mtk_jpegenc_clk *jpegenc_clk;
@@ -390,6 +409,10 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 	dev->master_dev = master_dev;
 	master_dev->max_hw_count++;
 
+	ret = mtk_jpegenc_smmu_init(dev);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, dev);
 	pm_runtime_enable(&pdev->dev);
 	ret = devm_clk_bulk_get(dev->dev,
-- 
2.45.2

Re: [PATCH v15 06/12] media: mediatek: jpeg: fix decoding buffer number setting timing issue

[sashiko-bot]

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

New issues:
- [High] Delayed initialization of `jpeg_dst_buf` metadata introduces a race condition with the timeout worker.

Pre-existing issues:
- [High] PM usage counter leak in `mtk_jpegenc_worker` on `pm_runtime_get_sync()` failure.
- [High] Double completion and severe list corruption due to IRQ handler and timeout worker race condition.
--

commit 938df83523dee529816f6765923d0c66f07e6d22
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix decoding buffer number setting timing issue

The patch delays the initialization of curr_ctx and frame_num for jpeg_dst_buf
by moving it into a spinlock-protected section in mtk_jpegdec_worker,
aiming to ensure any later operations act on an accurate state.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 89048aba8dca2..4dc574e03bd5a 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1743,11 +1742,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) {

[Severity: High]
This isn't a bug introduced by this patch, but while the decoder worker uses
pm_runtime_resume_and_get() here, the encoder worker still uses
pm_runtime_get_sync().

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpegenc_worker(),
if pm_runtime_get_sync() fails, it jumps to enc_end without dropping the usage
counter via pm_runtime_put_noidle(). This PM usage counter leak prevents the
device from entering deep sleep states.

Does it make sense to update mtk_jpegenc_worker() to use
pm_runtime_resume_and_get() as well?

> @@ -1772,6 +1766,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 this delayed initialization introduce a race condition with the timeout
worker?

By moving the initialization of jpeg_dst_buf->curr_ctx and frame_num here,
they are set after schedule_delayed_work() has already been called above.
If the worker thread gets preempted before taking the spinlock, the timeout
worker could execute and read hw_param.curr_ctx without taking the lock,
observing uninitialized or stale frame numbers.

>  	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 9a8dbca6af006..e4d2c5d4ec73d 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;
>  			}
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but can the IRQ handler and timeout worker
concurrently execute mtk_jpegdec_put_buf() and corrupt the dst_done_queue?

The hardware IRQ handler mtk_jpegdec_hw_irq_handler() calls
cancel_delayed_work(&jpeg->job_timeout_work). If the timeout worker is
already running on another CPU, it will not be stopped because this is
not the sync variant.

If both concurrently call mtk_jpegdec_put_buf() (and similarly
mtk_jpegenc_put_buf() in the encoder), they will both execute
list_add_tail(&dst_done_buf->list, &ctx->dst_done_queue). Adding a node
that is already on the list overwrites its next and prev pointers,
creating a circular link that can hang the kernel during traversal.

Are there plans to synchronize these paths?

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

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

[sashiko-bot]

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

New issues:
- [Critical] Unbounded max_hw_count growth on probe failure causes stack and heap buffer overflows.
- [High] Uninitialized stack memory access in worker threads upon retry.
- [High] Mismatched tracking of available hardware causes an infinite busy-loop and job failure.
- [High] Data race and missing memory barriers between child probe and worker threads cause NULL pointer dereferences.

Pre-existing issues:
- [High] Use-After-Free during device removal due to asymmetric devres workqueue teardown.
--

commit 0ee6ba9604510a9c4d1afe2cbb648113729e18a6
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix jpeg cores' amounts setting

This commit updates the driver to track the number of available hardware
cores dynamically using a max_hw_count variable instead of relying on a
hardcoded macro. It modifies the probe routines to increment this counter
and bounds the iterations in the worker threads based on the new variable.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec4830811..1e014fd698ae0 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c

[ ... ]

> @@ -1592,7 +1592,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;

[Severity: Critical]
Since max_hw_count can increment on probe retries (as noted below in the
probe routines), could this unbounded count exceed the fixed size of the
comp_jpeg stack array and cause a buffer overflow?

[Severity: High]
Since max_hw_count and the enc_hw_dev pointers are updated during child
probes without locks or memory barriers, could this worker thread observe
an incremented max_hw_count but read a NULL or uninitialized enc_hw_dev
pointer if the operations are reordered?

[Severity: High]
If a deferred child probe completes while waiting, and a subsequent call to
mtk_jpegenc_get_hw() returns a newly probed hw_id, does the retry loop use
an uninitialized pointer from the comp_jpeg array?

mtk_jpegenc_worker() {
    ...
retry_select:
    hw_id = mtk_jpegenc_get_hw(ctx);
    if (hw_id < 0) {
        ret = wait_event_interruptible(...);
        ...
        goto retry_select;
    }
    ...
    ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
}

Because the comp_jpeg array initialization loop happens before retry_select,
it is not re-run to pick up the newly assigned pointers.

[Severity: High]
Does the mismatch between hw_rdy and max_hw_count cause an infinite busy
loop when active cores are busy?

The wait condition relies on hw_rdy, which is initialized to the total
number of child nodes during the master probe:

    ret = wait_event_interruptible(jpeg->hw_wq,
                                   atomic_read(&jpeg->hw_rdy) > 0);

If a child node defers probing, max_hw_count will be less than hw_rdy.
When active cores are busy, this overcount could cause the worker to wake
up immediately and retry until it exhausts the retry limit and fails.

[Severity: High]
This is a pre-existing issue, but does device removal have a use-after-free
regression risk with the worker threads? 

During unbind, mtk_jpeg_remove() synchronously releases the m2m device:

mtk_jpeg_remove()
    ...
    v4l2_m2m_release(jpeg->m2m_dev);

However, the workqueue teardown is registered via devres and executes after
remove completes. If a worker thread is running during unbind, could it
call v4l2_m2m_job_finish() and access the already-freed m2m_dev context?

> 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 b6f5b2249f1f2..2765dafab4ad4 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -386,6 +386,7 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  	master_dev->enc_hw_dev[i] = dev;
>  	master_dev->reg_encbase[i] = dev->reg_base;
>  	dev->master_dev = master_dev;
> +	master_dev->max_hw_count++;
>  
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);

[Severity: Critical]
If a failure occurs later in the probe routine, such as in
mtk_jpegenc_smmu_init(), are the master_dev->max_hw_count increment and
pointer assignments rolled back? 

Without a rollback, a subsequent probe retry would increment max_hw_count
again and leak the previous hw_index, potentially overflowing counters and
corrupting adjacent memory structures.

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

Re: [PATCH v15 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] Calling `vb2_wait_for_all_buffers()` in `stop_streaming` causes an unrecoverable deadlock.

Pre-existing issues:
- [High] Multi-core worker threads intentionally call `v4l2_m2m_job_finish()` prematurely, breaking V4L2 M2M streamoff synchronization and causing UAF/Deadlock.
--

commit adc05e09fb60cf68d6bb34203f2c1f593ffa1ccb
Author: Kyrie Wu <kyrie.wu@mediatek.com>

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.

> 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
> @@ -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 unconditional wait risk causing an unrecoverable deadlock?

If a userspace application queues buffers, starts streaming, and then closes
the device node before the hardware has processed them, mtk_jpeg_release()
will cancel the background worker thread via cancel_work_sync().

After the worker is cancelled, any unprocessed queued buffers will never be
sent to the hardware, and thus will never complete. Since
v4l2_m2m_ctx_release() eventually calls stop_streaming(), wouldn't
vb2_wait_for_all_buffers() block forever waiting for the driver to return
the buffers?

Since the cleanup loop that aborts these buffers is located after this wait,
it appears it will never be reached. Could this cause the process to hang
indefinitely in uninterruptible sleep while holding the driver-wide mutex?

> +
>  	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]
Could the same deadlock occur here in the decoder's stop_streaming path?

This isn't a bug introduced by this patch, but does the multi-core worker
thread prematurely finish the V4L2 M2M job?

Looking at mtk_jpegenc_worker() in
drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:

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

And similarly in mtk_jpegdec_worker():

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

By calling v4l2_m2m_job_finish() immediately after starting the hardware,
long before the asynchronous hardware interrupts arrive, the TRANS_RUNNING
flag is cleared.

Does this deceive the V4L2 framework into believing the hardware is idle
while DMA operations are still ongoing?

Because TRANS_RUNNING is cleared, v4l2_m2m_cancel_job() fails to wait for
ongoing hardware operations during streamoff. Is this underlying breakage
in the streamoff synchronization the root cause of the use-after-free and
DMA corruption that the wait loop here is trying to fix?

>  	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
>  	    V4L2_TYPE_IS_CAPTURE(q->type)) {
>  		struct mtk_jpeg_src_buf *src_buf;

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

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

[sashiko-bot]

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

New issues:
- [High] Component-level system resume callbacks unpause the shared V4L2 M2M queue prematurely, causing jobs to be dispatched to components that are still in system suspend.
- [High] Missing pm_runtime_disable() on the probe error path leaves runtime PM enabled.
- [Medium] Double acquisition of clocks via devm_clk_bulk_get() causes duplicate resources and leaks original references.
- [Low] Unconditional assignment of dev_pm_ops produces dead code when PM is disabled.

Pre-existing issues:
- [Critical] Race condition between timeout work and IRQ handler leads to double-completion and PM usage underflow.
- [High] Unfixed pm_runtime_get_sync() usage counter leak in the encoder worker error path.
- [High] The worker threads call v4l2_m2m_job_finish() immediately after starting the hardware, leading to a UAF upon device release.
- [High] Missed wakeup in IRQ handler due to incorrect ordering of wake_up() and atomic_inc().
--

commit 5cd8fa923e48c4448d2b551ff3153b7043979096
Author: Kyrie Wu <kyrie.wu@mediatek.com>
Subject: media: mediatek: jpeg: Fix multi-core clk suspend and resume setting

This commit adds component-level suspend and resume callbacks for the MediaTek
JPEG multi-core architecture. It registers power management operations for
each hardware driver to handle clock preparation and power management across
system sleep states.

> 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

[ ... ]

> @@ -1647,13 +1653,6 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  		goto enc_end;
>  	}

[Severity: High]
This is a pre-existing issue, but if pm_runtime_get_sync() fails earlier in
mtk_jpegenc_worker(), does the error path to enc_end skip calling
pm_runtime_put_noidle() and leak the usage counter?

The same fix applied to mtk_jpegdec_worker() using pm_runtime_resume_and_get()
does not appear to be applied here.

[Severity: High]
This is a pre-existing issue, but does calling v4l2_m2m_job_finish()
immediately after starting the hardware cause a use-after-free upon device
release?

In mtk_jpegenc_worker() and mtk_jpegdec_worker(), the hardware starts and
then the worker calls v4l2_m2m_job_finish() prematurely:

    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);

When a user closes the file descriptor, v4l2_m2m_ctx_release() and
stop_streaming() are called. The interrupt handler could then call
v4l2_m2m_buf_done(), immediately waking up the waiter in stop_streaming().
This allows the context to be freed while the interrupt handler is still
manipulating it, causing a use-after-free write.

> -	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);

[ ... ]

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c

[ ... ]

> @@ -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 does calling cancel_delayed_work() instead
of cancel_delayed_work_sync() here cause a race condition if the timeout
handler is concurrently executing on another processor?

Since neither path takes a lock to serialize completion, both could proceed
to reset the hardware, call v4l2_m2m_buf_done() on the same buffer, increment
the ready counter twice, and invoke pm_runtime_put(). Could this corrupt the
buffer linked list or cause a power management usage counter underflow?

> -	ctx = jpeg->hw_param.curr_ctx;

[ ... ]

> @@ -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);

[Severity: High]
This is a pre-existing issue, but does calling wake_up() immediately before
atomic_inc() cause a missed wakeup?

If the waiting worker thread wakes up and checks atomic_read() before the
increment happens, it will go back to sleep. Since the subsequent atomic_inc()
is not followed by another wake_up(), could the worker thread hang
indefinitely?

> +	pm_runtime_put(jpeg->dev);
>  
>  	return IRQ_HANDLED;
>  }

[ ... ]

> @@ -668,15 +664,69 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
> +	ret = devm_clk_bulk_get(dev->dev,
> +				jpegdec_clk->clk_num,
> +				jpegdec_clk->clks);

[Severity: Medium]
Does this second devm_clk_bulk_get() overwrite the clocks already acquired
by devm_clk_bulk_get_all() earlier in the probe function?

This appears to acquire the clocks a second time, overwriting the array
pointers and leaking the original references until device unbind.

> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to init clk\n");
> +		return ret;
> +	}

[Severity: High]
If devm_clk_bulk_get() fails here, does the probe function return without
disabling the runtime power management?

Returning directly leaves the power management core tracking runtime status
as enabled for a device that failed to probe. Should pm_runtime_disable()
be called in this error path?

> +
> +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]
When a component resumes, does calling v4l2_m2m_resume() on the single shared
master queue unpause it prematurely?

If the shared queue is unpaused while sibling components are still suspended,
could a job be immediately dispatched to a suspended sibling, causing
pm_runtime_resume_and_get() to fail or block improperly?

> +
> +	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 mtk_jpegdec_pm_ops produce dead code when
power management is disabled?

Guidelines suggest wrapping the assignment with pm_ptr() to eliminate dead
code and compiler warnings when power management support is disabled.

>  	},
>  };

[ ... ]

> 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

[ ... ]

> @@ -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);

[Severity: Critical]
This is a pre-existing issue, but does calling cancel_delayed_work() earlier
in this interrupt handler cause a race condition if the timeout handler is
executing concurrently? Both paths could proceed to reset the hardware and call
v4l2_m2m_buf_done() on the same buffer locklessly.

[Severity: High]
This is also a pre-existing issue, but does calling wake_up() immediately
before atomic_inc() here cause a missed wakeup if the worker thread checks the
counter before the increment?

> +	pm_runtime_put(jpeg->dev);
>  
>  	return IRQ_HANDLED;
>  }

[ ... ]

> @@ -390,15 +388,69 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
> +	ret = devm_clk_bulk_get(dev->dev,
> +				jpegenc_clk->clk_num,
> +				jpegenc_clk->clks);

[Severity: Medium]
Similarly for the encoder, does this devm_clk_bulk_get() overwrite the clocks
acquired by devm_clk_bulk_get_all() earlier in the probe function?

> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to init clk\n");
> +		return ret;
> +	}

[Severity: High]
If devm_clk_bulk_get() fails here, does this error path also skip calling
pm_runtime_disable()?

> +
> +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]
Like the decoder, does calling v4l2_m2m_resume() here unpause the shared
master queue prematurely while other siblings might still be suspended?

> +
> +	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]
Should this assignment be wrapped with pm_ptr() to avoid dead code when
power management is disabled?

>  	},
>  };

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

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

[sashiko-bot]

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

New issues:
- [High] Calling `mtk_jpeg_set_queue_data()` asynchronously in the worker thread causes a lockless data race on V4L2 queue parameters and violates the source change event contract.

Pre-existing issues:
- [Critical] Calling `VIDIOC_STREAMOFF` on the decoder after a resolution change results in an unrecoverable kernel deadlock due to stranded buffers.
- [High] Missing `.job_ready` callback in multi-core ops combined with early worker exit causes an infinite workqueue scheduling loop that consumes 100% CPU.
--

commit 6ba9311a964d59efcd536c9a96787b7a4babde92
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 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>

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 4dc574e03bd5a..02efaed198b85 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1737,11 +1737,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 this code introduce a lockless data race on V4L2 queue parameters?

Modifying format queues via mtk_jpeg_set_queue_data() directly from the
worker thread in process context runs without holding the V4L2 device
lock or vb2 queue locks.

Could this lead to torn reads or state corruption if userspace concurrently
queries or sets the format via VIDIOC_G_FMT or VIDIOC_S_FMT? It also appears
to violate the V4L2 contract, which specifies that the format should remain
unchanged until userspace acknowledges the event via VIDIOC_STREAMOFF.

>  		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 this early exit without consuming
buffers lead to an infinite workqueue scheduling loop?

Because mtk_jpeg_multicore_dec_m2m_ops lacks a .job_ready callback, the
V4L2 M2M framework defaults to assuming the context is always ready if
buffers are present. Since this early exit path drops to getbuf_fail
without removing the buffers, v4l2_m2m_job_finish() will immediately
reschedule the context.

Could this loop continuously without delay, causing a 100% CPU lockup on
a single core?

[Severity: Critical]
This isn't a bug introduced by this patch, but does taking this getbuf_fail
path after a resolution change cause an unrecoverable deadlock later?

When the worker drops out here, the buffers remain in the v4l2_m2m queues
because v4l2_m2m_buf_done() is never called.

If userspace then handles the resolution change by calling VIDIOC_STREAMOFF
on the capture queue, mtk_jpeg_dec_stop_streaming() executes:

drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpeg_dec_stop_streaming() {
    ...
    if (jpeg->variant->multi_core)
        vb2_wait_for_all_buffers(q);
    ...
}

Since the driver still owns the stranded buffers, will vb2_wait_for_all_buffers
block forever? The code to flush the m2m queues is placed after the wait
call, which seems to guarantee a deadlock.

> +
>  	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>  	ret = pm_runtime_resume_and_get(comp_jpeg[hw_id]->dev);

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The schema update adds support for MT8196 but omits definitions for its newly advertised hardware features. While DVFS relies on existing standard `power-domains` properties without needing schema additions, the advertised SMMU support introduces a hard dependency on a new `mediatek,smmu-config` property in the child nodes (implemented in a subsequent patch 'media: mediatek: jpeg: add jpeg smmu sid setting'). Because `additionalProperties: false` is enforced, any device tree attempting to specify this property will fail schema validation. Conversely, if the property is omitted to satisfy validation, the driver will fail to probe.
- [Low] The `iommus` property description incorrectly references the MediaTek IOMMU binding for hardware that explicitly uses an SMMU.
--

commit 375a0f1aaa853e038449c5b8176a2c5ea4a805df
Author: Kyrie Wu <kyrie.wu@mediatek.com>

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

This commit adds the mediatek,mt8196-jpgenc compatible string to the binding.
It notes that the MT8196 hardware uses an SMMU instead of an IOMMU and
supports dynamic voltage and frequency scaling.

> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> index 596186497b684..e2d772ea0fb01 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

[Severity: Low]
The commit message notes that MT8196 uses an SMMU instead of the proprietary
MediaTek IOMMU. Does the existing description for the iommus property need
updating to reflect this?

Currently, the property definition explicitly points to the IOMMU bindings:

  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.
      Ports are according to the HW.

Could this static reference cause confusion for developers configuring the
new MT8196 hardware?

[ ... ]

> @@ -44,7 +46,9 @@ patternProperties:
>  
>      properties:
>        compatible:
> -        const: mediatek,mt8195-jpgenc-hw
> +        enum:
> +          - mediatek,mt8195-jpgenc-hw
> +          - mediatek,mt8196-jpgenc-hw

[Severity: High]
Since this child node is enforced by additionalProperties: false, will adding
support for MT8196 without defining its new SMMU features cause validation or
probe failures?

The commit message advertises SMMU support, which introduces a dependency
on a new mediatek,smmu-config property in the child nodes. Because the child
node enforces additionalProperties: false:

    required:
      - compatible
      - reg
      - iommus
      - interrupts
      - clocks
      - clock-names
      - power-domains

    additionalProperties: false

If device tree authors provide the mediatek,smmu-config property, it will
cause dtbs_check validation failures.

If they omit it to pass validation, the driver will fail to probe since it
won't be able to look up the syscon regmap. 

Should mediatek,smmu-config be added to the properties list for the child
node to prevent this issue?

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

Re: [PATCH v15 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] Misuse of the untyped MIN() macro instead of standard min_t().

Pre-existing issues:
- [High] V4L2 API violation and data corruption due to unconditionally overwriting and clamping the payload size for OUTPUT buffers.
--

commit d7dd9a5c943d9d5a39c948601677f1b3e207a5e1
Author: Kyrie Wu <kyrie.wu@mediatek.com>

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

For multi-core jpegdec, a resolution change event requires the payload size to
change, but concurrent decoding by other cores prevents immediate updates.
This results in payload sizes mismatching the real buffer length. This patch
clamps the payload size to be less than the real buffer length to suppress
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 1e014fd698ae0..3960ad21d7f0c 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));
> +		} else {
> +			vb2_set_plane_payload(vb, i,
> +					      MIN(plane_fmt.sizeimage,
> +						  vb->planes[i].length));

[Severity: High]
This is a pre-existing issue, but does unconditionally clamping and
overwriting the payload size here violate the V4L2 API for output buffers?

For the compressed bitstream queue, userspace sets bytesused to the exact
bitstream length during QBUF. mtk_jpeg_buf_prepare() unconditionally
overwrites this with the clamped value. Because mtk_jpeg_set_dec_src()
calculates the hardware DMA end_addr using vb2_get_plane_payload():

mtk_jpeg_set_dec_src()
    bs->end_addr = bs->str_addr +
                   round_up(vb2_get_plane_payload(src_buf, 0), 16);

the hardware and software parsers are forced to read up to the full buffer
capacity, which can lead to parsing uninitialized memory or trailing garbage.

Additionally, for capture buffers during a resolution change, if userspace
queues an old, undersized buffer, plane_fmt.sizeimage will exceed
vb->planes[i].length. Instead of returning -EINVAL to synchronously reject
the buffer as V4L2 requires, this clamping merely silences the framework's
WARN_ON_ONCE. The undersized buffer is accepted and will fail asynchronously
later.


[Severity: Low]
Is it safer to use min_t() instead of the untyped MIN() macro here?

vb->planes[i].length is an unsigned int, while max_size (and
plane_fmt.sizeimage) is a size_t. Using min_t(size_t, ...) maintains proper
type safety and adheres to kernel coding standards, avoiding potential type
mismatch issues.

> +		}
>  	}
>  
>  	return 0;

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

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

[sashiko-bot]

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

New issues:
- [Low] The `mtk8196_jpegenc_drvdata` structure is missing the `const` qualifier.

Pre-existing issues:
- [High] Out-of-bounds array access when probing hardware child nodes.
- [High] The upper 32 bits of the DMA stall address are incorrectly computed by adding the buffer size to the already-prepared extension bits.
--

Note: Due to recitation restrictions, the standard inline quoting format has
been altered. The following is a free-form summary of the findings rather than
an inline diff review.

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

This commit adds jpeg dec and enc compatible for mt8196.

[Severity: Low]
Looking at the new mtk8196_jpegenc_drvdata structure in
drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c, is it possible to
declare this as const?

The mtk8196_jpegdec_drvdata structure includes the const qualifier, but it
appears to be missing from the encoder variant. Adding it would ensure the
driver data is placed in the read-only .rodata section.

[Severity: High]
This is a pre-existing issue, but does the hardware probe logic overflow
the device arrays if a device tree specifies too many cores?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c during
mtk_jpegdec_hw_probe(), the code increments master_dev->hw_index and uses it
to index master_dev->dec_hw_dev and master_dev->reg_decbase without a bounds
check. The same pattern occurs in mtk_jpegenc_hw_probe() for the encoder.
Could a malformed device tree cause this unbounded index to write past the
end of these fixed-size arrays?

[Severity: High]
This is a pre-existing issue, but does enabling 34-bit support in this
patch expose a bug in the DMA stall address calculation?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c, the new
mtk8196_jpegenc_drvdata structure sets support_34bit to true.

Looking at mtk_jpeg_set_enc_dst() in
drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c, the stall extension
address is calculated as:

addr_ext = FIELD_PREP(MTK_JPEG_ADDR_MASK, upper_32_bits(dma_addr));
writel(addr_ext, base + JPEG_ENC_DEST_ADDR0_EXT);
writel(addr_ext + size, base + JPEG_ENC_STALL_ADDR0_EXT);

Is it mathematically valid to add the buffer size directly to the shifted
upper 32 bits? If the buffer crosses a 4GB boundary, wouldn't the large
byte size overflow the extension bits and pass incorrect upper address bits
to the hardware?

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

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

[sashiko-bot]

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

New issues:
- [High] In `mtk_jpegenc_worker`, `v4l2_m2m_src_buf_remove()` and `v4l2_m2m_dst_buf_remove()` are incorrectly called after `mtk_jpeg_enc_start()`. This introduces a severe race condition where an early hardware completion interrupt can process the buffers before they are removed from the ready queue.

Pre-existing issues:
- [High] The job timeout delayed work is scheduled before acquiring the hardware lock, creating a window where it can execute prematurely and lead to DMA memory corruption or list corruption.
- [High] Concurrent execution of the hardware IRQ handler and the timeout work leads to list corruption and state desynchronization.
- [Medium] `mtk_jpegenc_worker` leaks a PM runtime usage counter reference on error paths.
--

commit 4ce8f75cccbcd44894ec25eb120040d2e022ccdb
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: fix remove buffer removal timing for multi-core
    
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.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 02efaed198b85..0b9a3b66895a0 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1653,9 +1653,6 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  		goto enc_end;
>  	}

[Severity: Medium]
This is a pre-existing issue, but does pm_runtime_get_sync() increment
the usage counter even on failure?

If it fails here in mtk_jpegenc_worker() and jumps to enc_end without
dropping the reference, couldn't this leak the usage counter and prevent
the device from suspending?

Would using pm_runtime_put_noidle() in the error path fix this?

> -	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));

[Severity: High]
This is a pre-existing issue, but by scheduling the timeout work before
acquiring the hw_lock, could the timeout expire while the worker is
preempted?

If the worker thread is preempted or suspended longer than the timeout
period, the lockless timeout handler could execute, reset the hardware,
and return the buffer to userspace.

When the worker thread resumes, it blindly acquires the lock and configures
the hardware with pointers to the now-completed buffers. Could this result
in the hardware forcing DMA into memory that has already been freed or
reused by userspace?

This same pattern appears in mtk_jpegdec_worker() as well.

> @@ -1673,6 +1670,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]
By moving the buffer removal after mtk_jpeg_enc_start(), does this create a
race condition where the hardware completion interrupt processes the buffers
before they are removed from the V4L2 M2M ready queue?

If the hardware finishes instantly and triggers mtk_jpegenc_hw_irq_handler()
on another CPU, the IRQ handler will complete the buffer and return it to
userspace.

If userspace quickly dequeues and requeues it via VIDIOC_QBUF before the
worker thread calls v4l2_m2m_src_buf_remove(), the framework might call
list_add_tail() on a node already linked in the ready queue. Could this
corrupt the list pointers and lead to a kernel panic?

Should the buffers be removed from the queues before starting the hardware
to avoid this window, similar to how it is correctly handled in the decoder
modifications below?

>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);

[ ... ]


[Severity: High]
This is a pre-existing issue not touched by this patch, but could there be a
race condition between the hardware IRQ handler and the timeout work?

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

    cancel_delayed_work(&jpeg->job_timeout_work);

If the timer expires at the exact same moment the hardware finishes,
cancel_delayed_work() will return false, meaning the timeout work could
execute concurrently on another CPU.

Since both threads process the same buffers and simultaneously call
mtk_jpegenc_put_buf() (which executes list_add_tail()), could this corrupt
the list structure by adding the same node concurrently? Would it also lead
to double-incrementing master_jpeg->hw_rdy and double-decrementing PM
runtime?

This same vulnerability appears to exist in mtk_jpegdec_hw_irq_handler()
for the decoder.

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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Permanent increment of `hw_index` and `max_hw_count` on probe failure leads to Buffer Overflow and Use-After-Free.
- [High] Missing `pm_runtime_put_noidle()` when `pm_runtime_get_sync()` fails in the encoder worker.
- [High] Race condition between HW IRQ handler and timeout worker leading to double buffer completion.
--

commit 4f42651eeefeb9f2b2cb2fedba4846b1c45e369e
Author: Kyrie Wu <kyrie.wu@mediatek.com>

media: mediatek: jpeg: add jpeg smmu sid setting

This commit adds a configuration to set the JPEG decoder and encoder SMMU SID.
It maps the SMMU register base from the device tree and updates the SID
settings during worker initialization.

> 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 5a485e905787d..2e079a6e5d5d3 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
[ ... ]
> @@ -666,6 +685,10 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  	dev->master_dev = master_dev;
>  	master_dev->max_hw_count++;
>  
> +	ret = mtk_jpegdec_smmu_init(dev);
> +	if (ret)
> +		return ret;
> +

[Severity: Critical]
This is a pre-existing issue, but can this early return cause an out-of-bounds
array write on subsequent probe attempts?

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c:mtk_jpegdec_hw_probe(),
the counters are incremented prior to this block:

	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;
	master_dev->max_hw_count++;

If mtk_jpegdec_smmu_init() (or the pre-existing devm_clk_bulk_get() call below
it) returns an error such as -EPROBE_DEFER, the probe aborts without
decrementing master_dev->hw_index or max_hw_count. 

When the system retries probing, these counters will be permanently
incremented, rapidly exceeding the bounds of the dec_hw_dev array and
leading to memory corruption.

>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>  	ret = devm_clk_bulk_get(dev->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 1cfdfac98829b..0001d69ae7bab 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 +409,10 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
>  	dev->master_dev = master_dev;
>  	master_dev->max_hw_count++;
>  
> +	ret = mtk_jpegenc_smmu_init(dev);
> +	if (ret)
> +		return ret;
> +

[Severity: Critical]
This isn't a bug introduced by this patch, but could this early return also
lead to an out-of-bounds write here?

Just like in the decoder, if mtk_jpegenc_smmu_init() fails in
drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c:mtk_jpegenc_hw_probe(),
the function returns without rolling back master_dev->hw_index and
max_hw_count. Subsequent probe retries will use an out-of-bounds index for
the enc_hw_dev array.

>  	platform_set_drvdata(pdev, dev);
>  	pm_runtime_enable(&pdev->dev);
>  	ret = devm_clk_bulk_get(dev->dev,

[Severity: High]
This isn't a bug introduced by this patch, but does this error handling path
leak the PM runtime usage counter?

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

	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
	if (ret < 0) {
		dev_err(jpeg->dev, "%s : %d, pm_runtime_get_sync fail !!!\n",
			__func__, __LINE__);
		goto enc_end;
	}

If pm_runtime_get_sync() fails, the code jumps directly to enc_end without
calling pm_runtime_put_noidle() to drop the acquired reference, potentially
preventing the device from ever entering a low-power state.

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

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

	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;

If the hardware times out, mtk_jpegenc_timeout_work() executes and
unconditionally completes the buffers and drops PM references:

	v4l2_m2m_buf_done(src_buf, buf_state);
	mtk_jpegenc_put_buf(cjpeg);
	pm_runtime_put(cjpeg->dev);

If the hardware interrupt fires concurrently on another CPU,
cancel_delayed_work() will return false without waiting. Both contexts will
proceed to process the completion, potentially leading to double buffer
completion and a double PM runtime put.

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