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

[PATCH v11 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 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 hw count setting
  media: mediatek: jpeg: fix jpeg buffer payload setting
  media: mediatek: jpeg: fix jpeg buffer layout
  media: mediatek: jpeg: fix stop streaming flow for multi-core
  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 decoding resolution change operation
  media: mediatek: jpeg: fix remove buffer operation 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    | 170 +++++++++++++-----
 .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  21 ++-
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 112 +++++++++++-
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 112 +++++++++++-
 6 files changed, 376 insertions(+), 55 deletions(-)

-- 
2.45.2

[PATCH v11 01/12] media: mediatek: jpeg: fix jpeg hw count setting

[Kyrie Wu]

Different ICs have different amounts of hardware,
use a variable to set the amount of hardware.

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 6268d651bdcf..6a7e01130f1c 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1467,7 +1467,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;
@@ -1514,7 +1514,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;
@@ -1597,7 +1597,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;
 
@@ -1692,7 +1692,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 e78e1d11093c..a1e54715cb7e 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 9ab27aee302a..28d05909c96f 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 v11 02/12] media: mediatek: jpeg: fix jpeg buffer payload setting

[Kyrie Wu]

For multi-core jpegdec, if one of hws gets the event of resolution
changing, the payload size, representing the size of Y/C data,
needed to change. But others hws are decoding at the same time and
it can not be changed immediately, which results that the payload
size is not equal to the real buffer length of the hw's, which occurred
resolution changing and a warnning call trace will print.
So the setting of 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 6a7e01130f1c..0cf3dc5407e4 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -709,6 +709,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 = {};
+	unsigned long max_size;
 	int i;
 
 	q_data = mtk_jpeg_get_q_data(ctx, vb->vb2_queue->type);
@@ -717,12 +718,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 v11 03/12] media: mediatek: jpeg: fix jpeg buffer layout

[Kyrie Wu]

For memory alloc operation of jpeg dst buffer: the mallocing
memory function interface use vb2_buffer as the base addr.
If structure mtk_jpeg_src_buf wants to be allocated to memory,
it needs to be placed vb2_v4l2_buffer at the starting position,
because structure vb2_buffer is at the starting position of
vb2_v4l2_buffer, and the allocated size is set to the size of
structure mtk_jpeg_src_buf, so as to ensure that structures
mtk_jpeg_src_buf, vb2_v4l2_buffer and vb2_buffer can all be
allocated memory.

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 | 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 0cf3dc5407e4..bd0afc93d491 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1099,7 +1099,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 v11 04/12] media: mediatek: jpeg: fix stop streaming flow for multi-core

[Kyrie Wu]

For jpeg multi-core architecture, if all hardware run at the
same time, some input and output buffers are occupied.
If one hardware is completed firstly, while other hardwares are
still running. The decoding completion signal calls
mtk_jpeg_dec_stop_streaming, and the function of v4l2_m2m_buf_done
is called in mtk_jpeg_dec_stop_streaming to complete all
input/output buffers. However, some buffers are occupied by other
hardwares, resulting in errors. It needs to add a counter to
calculate the used decoding buffer counts, it will increase 1 when
the buffer set to hardware and decrease to 0 until the all buffers
decoded and the function could continue to be executed.

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>
---
 .../media/platform/mediatek/jpeg/mtk_jpeg_core.c | 16 ++++++++++++++++
 .../media/platform/mediatek/jpeg/mtk_jpeg_core.h |  2 ++
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c     |  9 +++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c     |  9 +++++++++
 4 files changed, 36 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index bd0afc93d491..59fd79c89f88 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -857,8 +857,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)
+		wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
+
 	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
 		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
 }
@@ -866,6 +870,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;
 
 	/*
@@ -873,6 +878,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)
+		wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
+
 	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
 	    V4L2_TYPE_IS_CAPTURE(q->type)) {
 		struct mtk_jpeg_src_buf *src_buf;
@@ -1181,6 +1189,7 @@ static int mtk_jpeg_open(struct file *file)
 	spin_lock_init(&ctx->done_queue_lock);
 	v4l2_fh_init(&ctx->fh, vfd);
 	v4l2_fh_add(&ctx->fh, file);
+	atomic_set(&ctx->buf_list_cnt, 0);
 
 	ctx->jpeg = jpeg;
 	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
@@ -1563,6 +1572,11 @@ static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
 	return 0;
 }
 
+static void jpeg_buf_queue_inc(struct mtk_jpeg_ctx *ctx)
+{
+	atomic_inc(&ctx->buf_list_cnt);
+}
+
 static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
 {
 	struct mtk_jpeg_ctx *ctx;
@@ -1671,6 +1685,7 @@ 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);
+	jpeg_buf_queue_inc(ctx);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
 
@@ -1786,6 +1801,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 				&bs,
 				&fb);
 	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
+	jpeg_buf_queue_inc(ctx);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 148fd41759b7..33f7fbc4ca5e 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
@@ -288,6 +288,7 @@ struct mtk_jpeg_q_data {
  * @dst_done_queue:		encoded frame buffer queue
  * @done_queue_lock:		encoded frame operation spinlock
  * @last_done_frame_num:	the last encoded frame number
+ * @buf_list_cnt:		the frame buffer count own by jpeg driver
  */
 struct mtk_jpeg_ctx {
 	struct mtk_jpeg_dev		*jpeg;
@@ -306,6 +307,7 @@ struct mtk_jpeg_ctx {
 	/* spinlock protecting the encode done buffer */
 	spinlock_t done_queue_lock;
 	u32 last_done_frame_num;
+	atomic_t buf_list_cnt;
 };
 
 #endif /* _MTK_JPEG_CORE_H */
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 a1e54715cb7e..84d12eea35f7 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -519,6 +519,11 @@ static void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
 	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
 }
 
+static void jpeg_buf_queue_dec(struct mtk_jpeg_ctx *ctx)
+{
+	atomic_dec(&ctx->buf_list_cnt);
+}
+
 static void mtk_jpegdec_timeout_work(struct work_struct *work)
 {
 	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
@@ -527,9 +532,11 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 			     job_timeout_work.work);
 	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	struct mtk_jpeg_ctx *ctx;
 
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
+	ctx = cjpeg->hw_param.curr_ctx;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
 
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
@@ -540,6 +547,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	wake_up(&master_jpeg->hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegdec_put_buf(cjpeg);
+	jpeg_buf_queue_dec(ctx);
 }
 
 static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
@@ -580,6 +588,7 @@ 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);
+	jpeg_buf_queue_dec(ctx);
 	pm_runtime_put(ctx->jpeg->dev);
 	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
 
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 28d05909c96f..1862444f35f5 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -248,6 +248,11 @@ static void mtk_jpegenc_put_buf(struct mtk_jpegenc_comp_dev *jpeg)
 	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
 }
 
+static void jpeg_buf_queue_enc(struct mtk_jpeg_ctx *ctx)
+{
+	atomic_dec(&ctx->buf_list_cnt);
+}
+
 static void mtk_jpegenc_timeout_work(struct work_struct *work)
 {
 	struct delayed_work *dly_work = to_delayed_work(work);
@@ -258,9 +263,11 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
 	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	struct mtk_jpeg_ctx *ctx;
 
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
+	ctx = cjpeg->hw_param.curr_ctx;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
 
 	mtk_jpeg_enc_reset(cjpeg->reg_base);
@@ -271,6 +278,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	wake_up(&master_jpeg->hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegenc_put_buf(cjpeg);
+	jpeg_buf_queue_enc(ctx);
 }
 
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
@@ -304,6 +312,7 @@ 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);
+	jpeg_buf_queue_enc(ctx);
 	pm_runtime_put(ctx->jpeg->dev);
 	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
 
-- 
2.45.2

[PATCH v11 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  | 75 ++++++++++++++++++-
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 75 ++++++++++++++++++-
 3 files changed, 151 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 59fd79c89f88..e45d7e82b8a0 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1122,6 +1122,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)
@@ -1130,6 +1133,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);
 }
@@ -1658,13 +1664,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);
 
@@ -1762,20 +1761,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);
 
@@ -1785,7 +1777,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,
@@ -1807,9 +1799,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 84d12eea35f7..5f1557dafad6 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -540,14 +540,13 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
 
 	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);
 	jpeg_buf_queue_dec(ctx);
+	pm_runtime_put(cjpeg->dev);
 }
 
 static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
@@ -589,12 +588,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegdec_put_buf(jpeg);
 	jpeg_buf_queue_dec(ctx);
-	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;
 }
@@ -677,15 +675,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegdec_comp_dev *jpeg)
+{
+	int ret;
+
+	ret = clk_bulk_prepare_enable(jpeg->jdec_clk.clk_num,
+				      jpeg->jdec_clk.clks);
+	if (ret)
+		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable fail\n",
+			__func__, __LINE__);
+}
+
+static void mtk_jpeg_clk_off(struct mtk_jpegdec_comp_dev *jpeg)
+{
+	clk_bulk_disable_unprepare(jpeg->jdec_clk.clk_num,
+				   jpeg->jdec_clk.clks);
+}
+
+static __maybe_unused int mtk_jpegdec_pm_suspend(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	mtk_jpeg_clk_off(jpeg);
 
 	return 0;
 }
 
+static __maybe_unused int mtk_jpegdec_pm_resume(struct device *dev)
+{
+	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	mtk_jpeg_clk_on(jpeg);
+
+	return 0;
+}
+
+static __maybe_unused 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 __maybe_unused 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 ret;
+}
+
+static const struct dev_pm_ops mtk_jpegdec_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegdec_suspend, mtk_jpegdec_resume)
+	SET_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 1862444f35f5..785db5ba4770 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -271,14 +271,13 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
 
 	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);
 	jpeg_buf_queue_enc(ctx);
+	pm_runtime_put(cjpeg->dev);
 }
 
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
@@ -313,12 +312,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	mtk_jpegenc_put_buf(jpeg);
 	jpeg_buf_queue_enc(ctx);
-	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;
 }
@@ -399,15 +397,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegenc_comp_dev *jpeg)
+{
+	int ret;
+
+	ret = clk_bulk_prepare_enable(jpeg->venc_clk.clk_num,
+				      jpeg->venc_clk.clks);
+	if (ret)
+		dev_err(jpeg->dev, "%s : %d, jpegenc clk_prepare_enable fail\n",
+			__func__, __LINE__);
+}
+
+static void mtk_jpeg_clk_off(struct mtk_jpegenc_comp_dev *jpeg)
+{
+	clk_bulk_disable_unprepare(jpeg->venc_clk.clk_num,
+				   jpeg->venc_clk.clks);
+}
+
+static __maybe_unused int mtk_jpegenc_pm_suspend(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	mtk_jpeg_clk_off(jpeg);
 
 	return 0;
 }
 
+static __maybe_unused int mtk_jpegenc_pm_resume(struct device *dev)
+{
+	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
+
+	mtk_jpeg_clk_on(jpeg);
+
+	return 0;
+}
+
+static __maybe_unused 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 __maybe_unused 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 ret;
+}
+
+static const struct dev_pm_ops mtk_jpegenc_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegenc_suspend, mtk_jpegenc_resume)
+	SET_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 v11 06/12] media: mediatek: jpeg: fix decoding buffer number setting timing issue

[Kyrie Wu]

move decoding buffer increase code into spinlock
protecting aera for multi-core

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 e45d7e82b8a0..5ffaee4dcd19 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1746,7 +1746,6 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
 	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)) {
@@ -1755,11 +1754,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) {
@@ -1784,6 +1778,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 5f1557dafad6..b3142dc9be85 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 785db5ba4770..82c971936c4d 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 v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation

[Kyrie Wu]

1.add a judgement for src buffer to avoid kernel crash
in the stop streaming function;
2.When a resolution changing occurs, it needs to set new
resolution parameter immediately and then report this event.
Otherwise, if the original software process is maintained,
the resolution change event is reported firstly, the CPU is
dispatched to the app to process the event, and the driver
does not set a new resolution, which will cause parameter errors.
3.After a resolution change occurred, decoding should not continue,
needs to wait until new buffers are ready and the state machine
changed.

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 +++++++--
 1 file changed, 7 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 5ffaee4dcd19..9233bbfe2d97 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -887,7 +887,8 @@ static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
 
 		vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 		src_buf = mtk_jpeg_vb2_to_srcbuf(&vb->vb2_buf);
-		mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);
+		if (!IS_ERR_OR_NULL(src_buf))
+			mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);
 		ctx->state = MTK_JPEG_RUNNING;
 	} else if (V4L2_TYPE_IS_OUTPUT(q->type)) {
 		ctx->state = MTK_JPEG_INIT;
@@ -1749,11 +1750,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 v11 08/12] media: mediatek: jpeg: fix remove buffer operation for multi-core

[Kyrie Wu]

move remove buffer code to spinlock protect area for multi-core

Fixes: 86379bd9d399 ("media: mtk-jpeg: Fixes jpeg enc&dec worker sw flow")

Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.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 9233bbfe2d97..da1ca494ed4b 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1665,9 +1665,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));
 
@@ -1685,6 +1682,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);
 	jpeg_buf_queue_inc(ctx);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
@@ -1767,9 +1766,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,
@@ -1794,6 +1790,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);
 	jpeg_buf_queue_inc(ctx);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
-- 
2.45.2

[PATCH v11 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 v11 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 v11 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>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 34 +++++++++++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  |  3 ++
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  3 ++
 3 files changed, 40 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index da1ca494ed4b..10a588b92e76 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1932,6 +1932,19 @@ 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,
+};
+
 static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
 	.formats = mtk_jpeg_dec_formats,
 	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
@@ -1945,6 +1958,19 @@ 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,
+};
+
 static const struct of_device_id mtk_jpeg_match[] = {
 	{
 		.compatible = "mediatek,mt8173-jpgdec",
@@ -1966,6 +1992,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 b3142dc9be85..e453a1634f33 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 82c971936c4d..b30c728c3712 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 v11 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>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 37 +++++++++++++++++++
 .../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, 98 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 10a588b92e76..625dfa8468e1 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>
@@ -1613,6 +1614,20 @@ static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
 	return IRQ_HANDLED;
 }
 
+static void mtk_jpeg_enc_set_smmu_sid(struct mtk_jpegenc_comp_dev *jpeg)
+{
+	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
+
+	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
+		return;
+
+	regmap_update_bits(jpeg->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];
@@ -1674,6 +1689,9 @@ 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);
+
+	mtk_jpeg_enc_set_smmu_sid(comp_jpeg[hw_id]);
+
 	mtk_jpeg_set_enc_dst(ctx,
 			     comp_jpeg[hw_id]->reg_base,
 			     &dst_buf->vb2_buf);
@@ -1701,6 +1719,20 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 }
 
+static void mtk_jpeg_dec_set_smmu_sid(struct mtk_jpegdec_comp_dev *jpeg)
+{
+	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
+
+	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
+		return;
+
+	regmap_update_bits(jpeg->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);
+}
+
 static void mtk_jpegdec_worker(struct work_struct *work)
 {
 	struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx,
@@ -1784,6 +1816,9 @@ 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);
+
+	mtk_jpeg_dec_set_smmu_sid(comp_jpeg[hw_id]);
+
 	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
 				jpeg->variant->support_34bit,
 				&jpeg_src_buf->dec_param,
@@ -1943,6 +1978,7 @@ static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
 	.multi_core = true,
 	.jpeg_worker = mtk_jpegenc_worker,
+	.support_smmu = true,
 };
 
 static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
@@ -1969,6 +2005,7 @@ static const struct mtk_jpeg_variant mtk8196_jpegdec_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
 	.multi_core = true,
 	.jpeg_worker = mtk_jpegdec_worker,
+	.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 33f7fbc4ca5e..6e8304680393 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 e453a1634f33..da753a636eaa 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -624,6 +624,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;
@@ -677,6 +696,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 b30c728c3712..8a61d5537315 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -348,6 +348,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;
@@ -399,6 +418,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 v11 01/12] media: mediatek: jpeg: fix jpeg hw count setting

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 4839 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> Different ICs have different amounts of hardware,
> use a variable to set the amount of hardware.

Did you mean different amount of cores ? The use of "hardware" is strange in
this description.

> 
> Fixes: 934e8bccac95 ("mtk-jpegenc: support jpegenc multi-hardware")
> Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")

Its not clear from your description if this is in preparation of MT8196 or an
actual fix. If its in preparation of, drop this, if it fixes some existing SoC,
please tell use which one. And tell us how it breaks without this fix.

regards,
Nicolas

> 
> 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 6268d651bdcf..6a7e01130f1c 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1467,7 +1467,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;
> @@ -1514,7 +1514,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;
> @@ -1597,7 +1597,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;
>  
> @@ -1692,7 +1692,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 e78e1d11093c..a1e54715cb7e 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 9ab27aee302a..28d05909c96f 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);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3607 bytes --]

Hi.

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> For multi-core jpegdec, if one of hws gets the event of resolution
> changing, the payload size, representing the size of Y/C data,

-> "gets the resolution change event, ..."

> needed to change. But others hws are decoding at the same time and

needed -> needs,               hws -> cores ?

It this specific to decoders or any type of cores ?

> it can not be changed immediately, which results that the payload

                                                    in the playload

> size is not equal to the real buffer length of the hw's, which occurred

size to not match the real buffer lenght for the ... hw ?

> resolution changing and a warnning call trace will print.

Can't parse. You can probably split that large sentence, and it needs to be
rework.

> So the setting of payload size must less than the real buffer length
> to remove the warnning logs.
> 
> Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-hardware")
> 

Don't add blank line in tags please.

> 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 6a7e01130f1c..0cf3dc5407e4 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -709,6 +709,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 = {};
> +	unsigned long max_size;

size_t ?

>  	int i;
>  
>  	q_data = mtk_jpeg_get_q_data(ctx, vb->vb2_queue->type);
> @@ -717,12 +718,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));

This is still not quite right. sizeimage, straight from s_fmt should already
account for the EXIF headers. If enable_exif is unknown at the moment, then you
should just always include that space. This way, the buffer length will only be
bigger if userspace asked for more at allocation time, and wil never be smaller
then this.

The point of sizeimage it to allow userspace allocate externally the right size,
this driver failed at that task it seems.

Stepping back a little, I don't even understand why you set the payload size
like this here. Why isn't the true payload size written once, when the buffer is
encoded ? Are you using bytesused to tell the HW how much space can be written
too ? Just give it the full space, wich is the allocated size (length).

Nicolas


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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v11 03/12] media: mediatek: jpeg: fix jpeg buffer layout

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> For memory alloc operation of jpeg dst buffer: the mallocing
> memory function interface use vb2_buffer as the base addr.
> If structure mtk_jpeg_src_buf wants to be allocated to memory,
> it needs to be placed vb2_v4l2_buffer at the starting position,
> because structure vb2_buffer is at the starting position of
> vb2_v4l2_buffer, and the allocated size is set to the size of
> structure mtk_jpeg_src_buf, so as to ensure that structures
> mtk_jpeg_src_buf, vb2_v4l2_buffer and vb2_buffer can all be
> allocated memory.

Please correct the wording, "mallocing" is not a word, addr -> address. I tend
to do the same, but refrain from giving the code a personality, the vb2_buffer
have no will. This is overall complicated way to simply say:

	Fix the size of the allocated mtk_jpeg_src_buf structure.

> 
> Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
> 

Drop the blank line.

> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.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 0cf3dc5407e4..bd0afc93d491 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1099,7 +1099,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;

I like the change, but this shouldn't be an issue if you use container_of, which
is safer then a plain cast. Please review the code and make sure to use it. It
may be confusing to include a cosmetic change in a fixes.

Nicolas

>  	struct mtk_jpeg_dec_param dec_param;
>  
>  	struct mtk_jpeg_ctx *curr_ctx;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v11 04/12] media: mediatek: jpeg: fix stop streaming flow for multi-core

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 9442 bytes --]

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> For jpeg multi-core architecture, if all hardware run at the
> same time, some input and output buffers are occupied.
> If one hardware is completed firstly, while other hardwares are

is compelted -> completes
harwares -> core, matches mtk_jpeg_core.c file name

> still running. The decoding completion signal calls

decoding -> decode.

> mtk_jpeg_dec_stop_streaming, and the function of v4l2_m2m_buf_done
> is called in mtk_jpeg_dec_stop_streaming to complete all
> input/output buffers. However, some buffers are occupied by other
> hardwares, resulting in errors. It needs to add a counter to
> calculate the used decoding buffer counts, it will increase 1 when
> the buffer set to hardware and decrease to 0 until the all buffers
> decoded and the function could continue to be executed.

More things to fix, also same for other of your commits, please reflow between
70 and 75 as suggested in the guide. It should look even.

> 
> 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>
> ---
>  .../media/platform/mediatek/jpeg/mtk_jpeg_core.c | 16 ++++++++++++++++
>  .../media/platform/mediatek/jpeg/mtk_jpeg_core.h |  2 ++
>  .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c     |  9 +++++++++
>  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c     |  9 +++++++++
>  4 files changed, 36 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index bd0afc93d491..59fd79c89f88 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -857,8 +857,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)
> +		wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
> +

Can't it simply be solved with vb2_wait_for_all_buffers() ?

Nicolas

>  	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
>  		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
>  }
> @@ -866,6 +870,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;
>  
>  	/*
> @@ -873,6 +878,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)
> +		wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
> +
>  	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
>  	    V4L2_TYPE_IS_CAPTURE(q->type)) {
>  		struct mtk_jpeg_src_buf *src_buf;
> @@ -1181,6 +1189,7 @@ static int mtk_jpeg_open(struct file *file)
>  	spin_lock_init(&ctx->done_queue_lock);
>  	v4l2_fh_init(&ctx->fh, vfd);
>  	v4l2_fh_add(&ctx->fh, file);
> +	atomic_set(&ctx->buf_list_cnt, 0);
>  
>  	ctx->jpeg = jpeg;
>  	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> @@ -1563,6 +1572,11 @@ static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
>  	return 0;
>  }
>  
> +static void jpeg_buf_queue_inc(struct mtk_jpeg_ctx *ctx)
> +{
> +	atomic_inc(&ctx->buf_list_cnt);
> +}
> +
>  static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
>  {
>  	struct mtk_jpeg_ctx *ctx;
> @@ -1671,6 +1685,7 @@ 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);
> +	jpeg_buf_queue_inc(ctx);
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
>  
> @@ -1786,6 +1801,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  				&bs,
>  				&fb);
>  	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
> +	jpeg_buf_queue_inc(ctx);
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
>  
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> index 148fd41759b7..33f7fbc4ca5e 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> @@ -288,6 +288,7 @@ struct mtk_jpeg_q_data {
>   * @dst_done_queue:		encoded frame buffer queue
>   * @done_queue_lock:		encoded frame operation spinlock
>   * @last_done_frame_num:	the last encoded frame number
> + * @buf_list_cnt:		the frame buffer count own by jpeg driver
>   */
>  struct mtk_jpeg_ctx {
>  	struct mtk_jpeg_dev		*jpeg;
> @@ -306,6 +307,7 @@ struct mtk_jpeg_ctx {
>  	/* spinlock protecting the encode done buffer */
>  	spinlock_t done_queue_lock;
>  	u32 last_done_frame_num;
> +	atomic_t buf_list_cnt;
>  };
>  
>  #endif /* _MTK_JPEG_CORE_H */
> 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 a1e54715cb7e..84d12eea35f7 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -519,6 +519,11 @@ static void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
>  	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
>  }
>  
> +static void jpeg_buf_queue_dec(struct mtk_jpeg_ctx *ctx)
> +{
> +	atomic_dec(&ctx->buf_list_cnt);
> +}
> +
>  static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  {
>  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> @@ -527,9 +532,11 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  			     job_timeout_work.work);
>  	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
>  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct mtk_jpeg_ctx *ctx;
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> +	ctx = cjpeg->hw_param.curr_ctx;
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>  
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
> @@ -540,6 +547,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	wake_up(&master_jpeg->hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegdec_put_buf(cjpeg);
> +	jpeg_buf_queue_dec(ctx);
>  }
>  
>  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> @@ -580,6 +588,7 @@ 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);
> +	jpeg_buf_queue_dec(ctx);
>  	pm_runtime_put(ctx->jpeg->dev);
>  	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
>  
> 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 28d05909c96f..1862444f35f5 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -248,6 +248,11 @@ static void mtk_jpegenc_put_buf(struct mtk_jpegenc_comp_dev *jpeg)
>  	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
>  }
>  
> +static void jpeg_buf_queue_enc(struct mtk_jpeg_ctx *ctx)
> +{
> +	atomic_dec(&ctx->buf_list_cnt);
> +}
> +
>  static void mtk_jpegenc_timeout_work(struct work_struct *work)
>  {
>  	struct delayed_work *dly_work = to_delayed_work(work);
> @@ -258,9 +263,11 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
>  	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
>  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
>  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct mtk_jpeg_ctx *ctx;
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> +	ctx = cjpeg->hw_param.curr_ctx;
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>  
>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
> @@ -271,6 +278,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
>  	wake_up(&master_jpeg->hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegenc_put_buf(cjpeg);
> +	jpeg_buf_queue_enc(ctx);
>  }
>  
>  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> @@ -304,6 +312,7 @@ 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);
> +	jpeg_buf_queue_enc(ctx);
>  	pm_runtime_put(ctx->jpeg->dev);
>  	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 11777 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> 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  | 75 ++++++++++++++++++-
>  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 75 ++++++++++++++++++-
>  3 files changed, 151 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 59fd79c89f88..e45d7e82b8a0 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1122,6 +1122,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)
> @@ -1130,6 +1133,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);
>  }
> @@ -1658,13 +1664,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);
>  
> @@ -1762,20 +1761,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);
>  
> @@ -1785,7 +1777,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,
> @@ -1807,9 +1799,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 84d12eea35f7..5f1557dafad6 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -540,14 +540,13 @@ static void mtk_jpegdec_timeout_work(struct work_struct
> *work)
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>  
>  	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);
>  	jpeg_buf_queue_dec(ctx);
> +	pm_runtime_put(cjpeg->dev);
>  }
>  
>  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> @@ -589,12 +588,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq,
> void *priv)
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegdec_put_buf(jpeg);
>  	jpeg_buf_queue_dec(ctx);
> -	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;
>  }
> @@ -677,15 +675,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegdec_comp_dev *jpeg)
> +{
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(jpeg->jdec_clk.clk_num,
> +				      jpeg->jdec_clk.clks);
> +	if (ret)
> +		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable
> fail\n",
> +			__func__, __LINE__);
> +}
> +
> +static void mtk_jpeg_clk_off(struct mtk_jpegdec_comp_dev *jpeg)
> +{
> +	clk_bulk_disable_unprepare(jpeg->jdec_clk.clk_num,
> +				   jpeg->jdec_clk.clks);
> +}
> +
> +static __maybe_unused int mtk_jpegdec_pm_suspend(struct device *dev)

Its used its passed to mtk_jpegdec_pm_ops structure. Same below. Unless you had
the intention to ifdef that structure and you forgot.

> +{
> +	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
> +
> +	mtk_jpeg_clk_off(jpeg);
>  
>  	return 0;
>  }
>  
> +static __maybe_unused int mtk_jpegdec_pm_resume(struct device *dev)
> +{
> +	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
> +
> +	mtk_jpeg_clk_on(jpeg);
> +
> +	return 0;
> +}
> +
> +static __maybe_unused 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 __maybe_unused 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 ret;
> +}
> +
> +static const struct dev_pm_ops mtk_jpegdec_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegdec_suspend, mtk_jpegdec_resume)
> +	SET_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 1862444f35f5..785db5ba4770 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -271,14 +271,13 @@ static void mtk_jpegenc_timeout_work(struct work_struct
> *work)
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>  
>  	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);
>  	jpeg_buf_queue_enc(ctx);
> +	pm_runtime_put(cjpeg->dev);
>  }
>  
>  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> @@ -313,12 +312,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq,
> void *priv)
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	mtk_jpegenc_put_buf(jpeg);
>  	jpeg_buf_queue_enc(ctx);
> -	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;
>  }
> @@ -399,15 +397,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegenc_comp_dev *jpeg)
> +{
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(jpeg->venc_clk.clk_num,
> +				      jpeg->venc_clk.clks);
> +	if (ret)
> +		dev_err(jpeg->dev, "%s : %d, jpegenc clk_prepare_enable
> fail\n",
> +			__func__, __LINE__);
> +}
> +
> +static void mtk_jpeg_clk_off(struct mtk_jpegenc_comp_dev *jpeg)
> +{
> +	clk_bulk_disable_unprepare(jpeg->venc_clk.clk_num,
> +				   jpeg->venc_clk.clks);
> +}
> +
> +static __maybe_unused int mtk_jpegenc_pm_suspend(struct device *dev)
> +{
> +	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
> +
> +	mtk_jpeg_clk_off(jpeg);

This is the only call to that function, move the code here.

>  
>  	return 0;
>  }
>  
> +static __maybe_unused int mtk_jpegenc_pm_resume(struct device *dev)
> +{
> +	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
> +
> +	mtk_jpeg_clk_on(jpeg);

This is the only call to that function, move the code here. Decides if the ret
value should be passed or not too, add a comment if not.

Its worrying not too, since usually if the clock are not running, we risk
hanging the CPU writing to registers.

> +
> +	return 0;
> +}
> +
> +static __maybe_unused 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 __maybe_unused 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 ret;
> +}
> +
> +static const struct dev_pm_ops mtk_jpegenc_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegenc_suspend, mtk_jpegenc_resume)
> +	SET_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,
>  	},
>  };
>  

Looks like the right way though,looking forward an update.

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3512 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> move decoding buffer increase code into spinlock
> protecting aera for multi-core

I don't understand this text, please rework.

> 
> Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface")
> 

Drop that blank line.

> 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 e45d7e82b8a0..5ffaee4dcd19 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1746,7 +1746,6 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>  	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)) {
> @@ -1755,11 +1754,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) {
> @@ -1784,6 +1778,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 5f1557dafad6..b3142dc9be85 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 785db5ba4770..82c971936c4d 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;
>  			}
>  		}
>  	}

Normally I'd seek for the context in the code. But I think I'll wait for a
proper commit message, and do a proper review then.

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> 1.add a judgement for src buffer to avoid kernel crash
> in the stop streaming function;
> 2.When a resolution changing occurs, it needs to set new
> resolution parameter immediately and then report this event.
> Otherwise, if the original software process is maintained,
> the resolution change event is reported firstly, the CPU is
> dispatched to the app to process the event, and the driver
> does not set a new resolution, which will cause parameter errors.
> 3.After a resolution change occurred, decoding should not continue,
> needs to wait until new buffers are ready and the state machine
> changed.

I mention this in other patchset, very often, 3 bullets means 3 distinct
changes. Don't use bullets. Reflow this text, rework this text, there is many
syntax error here.

> 
> 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 +++++++--
>  1 file changed, 7 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 5ffaee4dcd19..9233bbfe2d97 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -887,7 +887,8 @@ static void mtk_jpeg_dec_stop_streaming(struct vb2_queue
> *q)
>  
>  		vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>  		src_buf = mtk_jpeg_vb2_to_srcbuf(&vb->vb2_buf);
> -		mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);
> +		if (!IS_ERR_OR_NULL(src_buf))
> +			mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);

The lack of vb2_wait_for_all_buffers() might explains this better, you might not
need to do random null checks like this.

Nicolas

>  		ctx->state = MTK_JPEG_RUNNING;
>  	} else if (V4L2_TYPE_IS_OUTPUT(q->type)) {
>  		ctx->state = MTK_JPEG_INIT;
> @@ -1749,11 +1750,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) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v11 08/12] media: mediatek: jpeg: fix remove buffer operation for multi-core

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> move remove buffer code to spinlock protect area for multi-core

Perhaps:

Move calls to v4l2_m2m_src/dst_buf_remove() outside of the spinlock protected
scope.

And perhaps extend in why this is needed. Deadlock, bad lock ordering?

> 
> Fixes: 86379bd9d399 ("media: mtk-jpeg: Fixes jpeg enc&dec worker sw flow")
> 

No blank.

> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Nicolas

> ---
>  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 9233bbfe2d97..da1ca494ed4b 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1665,9 +1665,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));
>  
> @@ -1685,6 +1682,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);
>  	jpeg_buf_queue_inc(ctx);
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
> @@ -1767,9 +1766,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,
> @@ -1794,6 +1790,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);
>  	jpeg_buf_queue_inc(ctx);
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 9607 bytes --]

Hi,

Le mardi 02 décembre 2025 à 17:48 +0800, Kyrie Wu a écrit :
> Add a configuration to set jpeg dec & enc smmu sid
> 
> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> ---
>  .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 37 +++++++++++++++++++
>  .../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, 98 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 10a588b92e76..625dfa8468e1 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>
> @@ -1613,6 +1614,20 @@ static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
>  	return IRQ_HANDLED;
>  }
>  
> +static void mtk_jpeg_enc_set_smmu_sid(struct mtk_jpegenc_comp_dev *jpeg)
> +{
> +	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
> +
> +	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
> +		return;
> +
> +	regmap_update_bits(jpeg->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];
> @@ -1674,6 +1689,9 @@ 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);
> +
> +	mtk_jpeg_enc_set_smmu_sid(comp_jpeg[hw_id]);
> +
>  	mtk_jpeg_set_enc_dst(ctx,
>  			     comp_jpeg[hw_id]->reg_base,
>  			     &dst_buf->vb2_buf);
> @@ -1701,6 +1719,20 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  }
>  
> +static void mtk_jpeg_dec_set_smmu_sid(struct mtk_jpegdec_comp_dev *jpeg)
> +{
> +	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
> +
> +	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
> +		return;
> +
> +	regmap_update_bits(jpeg->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);
> +}

Can you deduplicate that function. Just make one, and pass the SID as parameter.

> +
>  static void mtk_jpegdec_worker(struct work_struct *work)
>  {
>  	struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx,
> @@ -1784,6 +1816,9 @@ 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);
> +
> +	mtk_jpeg_dec_set_smmu_sid(comp_jpeg[hw_id]);
> +
>  	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
>  				jpeg->variant->support_34bit,
>  				&jpeg_src_buf->dec_param,
> @@ -1943,6 +1978,7 @@ static struct mtk_jpeg_variant mtk8196_jpegenc_drvdata = {
>  	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
>  	.multi_core = true,
>  	.jpeg_worker = mtk_jpegenc_worker,
> +	.support_smmu = true,
>  };
>  
>  static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
> @@ -1969,6 +2005,7 @@ static const struct mtk_jpeg_variant mtk8196_jpegdec_drvdata = {
>  	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
>  	.multi_core = true,
>  	.jpeg_worker = mtk_jpegdec_worker,
> +	.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 33f7fbc4ca5e..6e8304680393 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 e453a1634f33..da753a636eaa 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -624,6 +624,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;
> @@ -677,6 +696,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 b30c728c3712..8a61d5537315 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -348,6 +348,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;
> @@ -399,6 +418,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,

With that minor fix:

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3871 bytes --]

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> 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    | 34 +++++++++++++++++++
>  .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  |  3 ++
>  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  3 ++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index da1ca494ed4b..10a588b92e76 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1932,6 +1932,19 @@ 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,
> +};
> +
>  static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
>  	.formats = mtk_jpeg_dec_formats,
>  	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
> @@ -1945,6 +1958,19 @@ 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,
> +};
> +
>  static const struct of_device_id mtk_jpeg_match[] = {
>  	{
>  		.compatible = "mediatek,mt8173-jpgdec",
> @@ -1966,6 +1992,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 b3142dc9be85..e453a1634f33 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 82c971936c4d..b30c728c3712 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);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v11 01/12] media: mediatek: jpeg: fix jpeg hw count setting

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 15:57 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > Different ICs have different amounts of hardware,
> > use a variable to set the amount of hardware.
> 
> Did you mean different amount of cores ? The use of "hardware" is
> strange in
> this description.
Yes, it's right. I will change it in the next version.

Thanks.
> 
> > 
> > Fixes: 934e8bccac95 ("mtk-jpegenc: support jpegenc multi-hardware")
> > Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-
> > hardware")
> 
> Its not clear from your description if this is in preparation of
> MT8196 or an
> actual fix. If its in preparation of, drop this, if it fixes some
> existing SoC,
> please tell use which one. And tell us how it breaks without this
> fix.
> 
> regards,
> Nicolas

Dear Nicolas,

The macro was first used to represent the amount of cores for MT8195.
However, the cores' quantity of MT8196 is different from that in the
MT8195,
and it may also differ in the coming ICs. Therefore, the definition of
the macro can not used to represent the amount of cores.

But if you think this is normal setting patch rather than a fixed one,
I will change the commit messages in the next version.

Look forward to your further reply.

Thanks.

Regards,
Kyrie.

> 
> > 
> > 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 6268d651bdcf..6a7e01130f1c 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1467,7 +1467,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;
> > @@ -1514,7 +1514,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;
> > @@ -1597,7 +1597,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;
> >  
> > @@ -1692,7 +1692,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 e78e1d11093c..a1e54715cb7e 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 9ab27aee302a..28d05909c96f 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);

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

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:15 -0500, Nicolas Dufresne wrote:
> Hi.
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > For multi-core jpegdec, if one of hws gets the event of resolution
> > changing, the payload size, representing the size of Y/C data,
> 
> -> "gets the resolution change event, ..."
> 
> > needed to change. But others hws are decoding at the same time and
> 
> needed -> needs,               hws -> cores ?
> 
> It this specific to decoders or any type of cores ?
> 
> > it can not be changed immediately, which results that the payload
> 
>                                                     in the playload
> 
> > size is not equal to the real buffer length of the hw's, which
> > occurred
> 
> size to not match the real buffer lenght for the ... hw ?
> 
> > resolution changing and a warnning call trace will print.
> 
> Can't parse. You can probably split that large sentence, and it needs
> to be
> rework.
> 
> > So the setting of payload size must less than the real buffer
> > length
> > to remove the warnning logs.
> > 
> > Fixes: 0fa49df4222f ("media: mtk-jpegdec: support jpegdec multi-
> > hardware")
> > 
> 
> Don't add blank line in tags please.
> 
> > 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 6a7e01130f1c..0cf3dc5407e4 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -709,6 +709,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 = {};
> > +	unsigned long max_size;
> 
> size_t ?
> 
> >  	int i;
> >  
> >  	q_data = mtk_jpeg_get_q_data(ctx, vb->vb2_queue->type);
> > @@ -717,12 +718,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));
> 
> This is still not quite right. sizeimage, straight from s_fmt should
> already
> account for the EXIF headers. If enable_exif is unknown at the
> moment, then you
> should just always include that space. This way, the buffer length
> will only be
> bigger if userspace asked for more at allocation time, and wil never
> be smaller
> then this.
> 
> The point of sizeimage it to allow userspace allocate externally the
> right size,
> this driver failed at that task it seems.
> 
> Stepping back a little, I don't even understand why you set the
> payload size
> like this here. Why isn't the true payload size written once, when
> the buffer is
> encoded ? Are you using bytesused to tell the HW how much space can
> be written
> too ? Just give it the full space, wich is the allocated size
> (length).
> 
> Nicolas

Dear Nicolas,

The value, max_size = plane_fmt.sizeimage, recorded the current size,
but if the resolution changed to a smaller one, the sizeimage will
bigger
than the vb->planes[i].length, in the former sw flow, the bigger value
will set to payload size and caused a warnning call trace printed.

Here are the answers of your questions:
1. The enable_exif is a
determined value, which is set in s_ctrl before 
jpeg decoder/encoder starting.

2. If enable_exif is set as true, the buffer planes length is equal to
sizeimage + MTK_JPEG_MAX_EXIF_SIZE.

3. The reason, Why isn't the true payload size written once,
is due to this setting is used to avoid the condition descripted in
commit message. And it will set the true value in irq handler.

Thanks.

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

Re: [PATCH v11 03/12] media: mediatek: jpeg: fix jpeg buffer layout

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:22 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > For memory alloc operation of jpeg dst buffer: the mallocing
> > memory function interface use vb2_buffer as the base addr.
> > If structure mtk_jpeg_src_buf wants to be allocated to memory,
> > it needs to be placed vb2_v4l2_buffer at the starting position,
> > because structure vb2_buffer is at the starting position of
> > vb2_v4l2_buffer, and the allocated size is set to the size of
> > structure mtk_jpeg_src_buf, so as to ensure that structures
> > mtk_jpeg_src_buf, vb2_v4l2_buffer and vb2_buffer can all be
> > allocated memory.
> 
> Please correct the wording, "mallocing" is not a word, addr ->
> address. I tend
> to do the same, but refrain from giving the code a personality, the
> vb2_buffer
> have no will. This is overall complicated way to simply say:
> 
> 	Fix the size of the allocated mtk_jpeg_src_buf structure.
> 
> > 
> > Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker
> > interface")
> > 
> 
> Drop the blank line.
> 
> > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.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 0cf3dc5407e4..bd0afc93d491 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1099,7 +1099,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;
> 
> I like the change, but this shouldn't be an issue if you use
> container_of, which
> is safer then a plain cast. Please review the code and make sure to
> use it. It
> may be confusing to include a cosmetic change in a fixes.
> 
> Nicolas

Dear Nicolas,

The driver requests memory space for mtk_jpeg_src_buf in the
function of mtk_jpeg_queue_init,
and it used struct vb2_v4l2_buffer b as the starting base address,
the parameter,frame_num, will not get a memory if we keep the old
structure. And a unknown memory would be get if we used container_of
to get the starting address of mtk_jpeg_src_buf.

Thanks.

Regards,
Kyrie.
> 
> >  	struct mtk_jpeg_dec_param dec_param;
> >  
> >  	struct mtk_jpeg_ctx *curr_ctx;

Re: [PATCH v11 04/12] media: mediatek: jpeg: fix stop streaming flow for multi-core

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:27 -0500, Nicolas Dufresne wrote:
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > For jpeg multi-core architecture, if all hardware run at the
> > same time, some input and output buffers are occupied.
> > If one hardware is completed firstly, while other hardwares are
> 
> is compelted -> completes
> harwares -> core, matches mtk_jpeg_core.c file name
> 
> > still running. The decoding completion signal calls
> 
> decoding -> decode.
> 
> > mtk_jpeg_dec_stop_streaming, and the function of v4l2_m2m_buf_done
> > is called in mtk_jpeg_dec_stop_streaming to complete all
> > input/output buffers. However, some buffers are occupied by other
> > hardwares, resulting in errors. It needs to add a counter to
> > calculate the used decoding buffer counts, it will increase 1 when
> > the buffer set to hardware and decrease to 0 until the all buffers
> > decoded and the function could continue to be executed.
> 
> More things to fix, also same for other of your commits, please
> reflow between
> 70 and 75 as suggested in the guide. It should look even.

Thanks a lot. I will refine the commit message.
> 
> > 
> > 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>
> > ---
> >  .../media/platform/mediatek/jpeg/mtk_jpeg_core.c | 16
> > ++++++++++++++++
> >  .../media/platform/mediatek/jpeg/mtk_jpeg_core.h |  2 ++
> >  .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c     |  9 +++++++++
> >  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c     |  9 +++++++++
> >  4 files changed, 36 insertions(+)
> > 
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > index bd0afc93d491..59fd79c89f88 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -857,8 +857,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)
> > +		wait_event(jpeg->hw_wq, (atomic_read(&ctx-
> > >buf_list_cnt) == 0));
> > +
> 
> Can't it simply be solved with vb2_wait_for_all_buffers() ?
> 
> Nicolas

Dear Nicolas,

Thanks for your suggestion. I think it better than mine.
I will change to use it.

Regards,
Kyrie.
> 
> >  	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> >  		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >  }
> > @@ -866,6 +870,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;
> >  
> >  	/*
> > @@ -873,6 +878,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)
> > +		wait_event(jpeg->hw_wq, (atomic_read(&ctx-
> > >buf_list_cnt) == 0));
> > +
> >  	if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
> >  	    V4L2_TYPE_IS_CAPTURE(q->type)) {
> >  		struct mtk_jpeg_src_buf *src_buf;
> > @@ -1181,6 +1189,7 @@ static int mtk_jpeg_open(struct file *file)
> >  	spin_lock_init(&ctx->done_queue_lock);
> >  	v4l2_fh_init(&ctx->fh, vfd);
> >  	v4l2_fh_add(&ctx->fh, file);
> > +	atomic_set(&ctx->buf_list_cnt, 0);
> >  
> >  	ctx->jpeg = jpeg;
> >  	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > @@ -1563,6 +1572,11 @@ static int mtk_jpegdec_set_hw_param(struct
> > mtk_jpeg_ctx *ctx,
> >  	return 0;
> >  }
> >  
> > +static void jpeg_buf_queue_inc(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	atomic_inc(&ctx->buf_list_cnt);
> > +}
> > +
> >  static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
> >  {
> >  	struct mtk_jpeg_ctx *ctx;
> > @@ -1671,6 +1685,7 @@ 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);
> > +	jpeg_buf_queue_inc(ctx);
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
> >  
> > @@ -1786,6 +1801,7 @@ static void mtk_jpegdec_worker(struct
> > work_struct *work)
> >  				&bs,
> >  				&fb);
> >  	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
> > +	jpeg_buf_queue_inc(ctx);
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
> >  
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > index 148fd41759b7..33f7fbc4ca5e 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > @@ -288,6 +288,7 @@ struct mtk_jpeg_q_data {
> >   * @dst_done_queue:		encoded frame buffer queue
> >   * @done_queue_lock:		encoded frame operation
> > spinlock
> >   * @last_done_frame_num:	the last encoded frame number
> > + * @buf_list_cnt:		the frame buffer count own by jpeg
> > driver
> >   */
> >  struct mtk_jpeg_ctx {
> >  	struct mtk_jpeg_dev		*jpeg;
> > @@ -306,6 +307,7 @@ struct mtk_jpeg_ctx {
> >  	/* spinlock protecting the encode done buffer */
> >  	spinlock_t done_queue_lock;
> >  	u32 last_done_frame_num;
> > +	atomic_t buf_list_cnt;
> >  };
> >  
> >  #endif /* _MTK_JPEG_CORE_H */
> > 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 a1e54715cb7e..84d12eea35f7 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > @@ -519,6 +519,11 @@ static void mtk_jpegdec_put_buf(struct
> > mtk_jpegdec_comp_dev *jpeg)
> >  	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> >  }
> >  
> > +static void jpeg_buf_queue_dec(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	atomic_dec(&ctx->buf_list_cnt);
> > +}
> > +
> >  static void mtk_jpegdec_timeout_work(struct work_struct *work)
> >  {
> >  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > @@ -527,9 +532,11 @@ static void mtk_jpegdec_timeout_work(struct
> > work_struct *work)
> >  			     job_timeout_work.work);
> >  	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> >  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_ctx *ctx;
> >  
> >  	src_buf = cjpeg->hw_param.src_buffer;
> >  	dst_buf = cjpeg->hw_param.dst_buffer;
> > +	ctx = cjpeg->hw_param.curr_ctx;
> >  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> >  
> >  	mtk_jpeg_dec_reset(cjpeg->reg_base);
> > @@ -540,6 +547,7 @@ static void mtk_jpegdec_timeout_work(struct
> > work_struct *work)
> >  	wake_up(&master_jpeg->hw_wq);
> >  	v4l2_m2m_buf_done(src_buf, buf_state);
> >  	mtk_jpegdec_put_buf(cjpeg);
> > +	jpeg_buf_queue_dec(ctx);
> >  }
> >  
> >  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> > @@ -580,6 +588,7 @@ 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);
> > +	jpeg_buf_queue_dec(ctx);
> >  	pm_runtime_put(ctx->jpeg->dev);
> >  	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
> >  
> > 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 28d05909c96f..1862444f35f5 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > @@ -248,6 +248,11 @@ static void mtk_jpegenc_put_buf(struct
> > mtk_jpegenc_comp_dev *jpeg)
> >  	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> >  }
> >  
> > +static void jpeg_buf_queue_enc(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	atomic_dec(&ctx->buf_list_cnt);
> > +}
> > +
> >  static void mtk_jpegenc_timeout_work(struct work_struct *work)
> >  {
> >  	struct delayed_work *dly_work = to_delayed_work(work);
> > @@ -258,9 +263,11 @@ static void mtk_jpegenc_timeout_work(struct
> > work_struct *work)
> >  	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> >  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> >  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_ctx *ctx;
> >  
> >  	src_buf = cjpeg->hw_param.src_buffer;
> >  	dst_buf = cjpeg->hw_param.dst_buffer;
> > +	ctx = cjpeg->hw_param.curr_ctx;
> >  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> >  
> >  	mtk_jpeg_enc_reset(cjpeg->reg_base);
> > @@ -271,6 +278,7 @@ static void mtk_jpegenc_timeout_work(struct
> > work_struct *work)
> >  	wake_up(&master_jpeg->hw_wq);
> >  	v4l2_m2m_buf_done(src_buf, buf_state);
> >  	mtk_jpegenc_put_buf(cjpeg);
> > +	jpeg_buf_queue_enc(ctx);
> >  }
> >  
> >  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> > @@ -304,6 +312,7 @@ 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);
> > +	jpeg_buf_queue_enc(ctx);
> >  	pm_runtime_put(ctx->jpeg->dev);
> >  	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
> >  
> 
> 	Could not parse S/MIME message: security library: improperly
> formatted DER-encoded message. (-8183) - Decoder failed

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

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:33 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > 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  | 75
> > ++++++++++++++++++-
> >  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 75
> > ++++++++++++++++++-
> >  3 files changed, 151 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > index 59fd79c89f88..e45d7e82b8a0 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1122,6 +1122,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)
> > @@ -1130,6 +1133,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);
> >  }
> > @@ -1658,13 +1664,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);
> >  
> > @@ -1762,20 +1761,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);
> >  
> > @@ -1785,7 +1777,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,
> > @@ -1807,9 +1799,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 84d12eea35f7..5f1557dafad6 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > @@ -540,14 +540,13 @@ static void mtk_jpegdec_timeout_work(struct
> > work_struct
> > *work)
> >  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> >  
> >  	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);
> >  	jpeg_buf_queue_dec(ctx);
> > +	pm_runtime_put(cjpeg->dev);
> >  }
> >  
> >  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> > @@ -589,12 +588,11 @@ static irqreturn_t
> > mtk_jpegdec_hw_irq_handler(int irq,
> > void *priv)
> >  	v4l2_m2m_buf_done(src_buf, buf_state);
> >  	mtk_jpegdec_put_buf(jpeg);
> >  	jpeg_buf_queue_dec(ctx);
> > -	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;
> >  }
> > @@ -677,15 +675,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegdec_comp_dev *jpeg)
> > +{
> > +	int ret;
> > +
> > +	ret = clk_bulk_prepare_enable(jpeg->jdec_clk.clk_num,
> > +				      jpeg->jdec_clk.clks);
> > +	if (ret)
> > +		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable
> > fail\n",
> > +			__func__, __LINE__);
> > +}
> > +
> > +static void mtk_jpeg_clk_off(struct mtk_jpegdec_comp_dev *jpeg)
> > +{
> > +	clk_bulk_disable_unprepare(jpeg->jdec_clk.clk_num,
> > +				   jpeg->jdec_clk.clks);
> > +}
> > +
> > +static __maybe_unused int mtk_jpegdec_pm_suspend(struct device
> > *dev)
> 
> Its used its passed to mtk_jpegdec_pm_ops structure. Same below.
> Unless you had
> the intention to ifdef that structure and you forgot.
Thanks.
I will change to return clk_bulk_disable_unprepare(jpeg-
>jdec_clk.clk_num, jpeg->jdec_clk.clks);
> 
> > +{
> > +	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_clk_off(jpeg);
> >  
> >  	return 0;
> >  }
> >  
> > +static __maybe_unused int mtk_jpegdec_pm_resume(struct device
> > *dev)
> > +{
> > +	struct mtk_jpegdec_comp_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_clk_on(jpeg);
> > +
> > +	return 0;
> > +}
> > +
> > +static __maybe_unused 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 __maybe_unused 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 ret;
> > +}
> > +
> > +static const struct dev_pm_ops mtk_jpegdec_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegdec_suspend,
> > mtk_jpegdec_resume)
> > +	SET_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 1862444f35f5..785db5ba4770 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > @@ -271,14 +271,13 @@ static void mtk_jpegenc_timeout_work(struct
> > work_struct
> > *work)
> >  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> >  
> >  	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);
> >  	jpeg_buf_queue_enc(ctx);
> > +	pm_runtime_put(cjpeg->dev);
> >  }
> >  
> >  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> > @@ -313,12 +312,11 @@ static irqreturn_t
> > mtk_jpegenc_hw_irq_handler(int irq,
> > void *priv)
> >  	v4l2_m2m_buf_done(src_buf, buf_state);
> >  	mtk_jpegenc_put_buf(jpeg);
> >  	jpeg_buf_queue_enc(ctx);
> > -	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;
> >  }
> > @@ -399,15 +397,84 @@ 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 void mtk_jpeg_clk_on(struct mtk_jpegenc_comp_dev *jpeg)
> > +{
> > +	int ret;
> > +
> > +	ret = clk_bulk_prepare_enable(jpeg->venc_clk.clk_num,
> > +				      jpeg->venc_clk.clks);
> > +	if (ret)
> > +		dev_err(jpeg->dev, "%s : %d, jpegenc clk_prepare_enable
> > fail\n",
> > +			__func__, __LINE__);
> > +}
> > +
> > +static void mtk_jpeg_clk_off(struct mtk_jpegenc_comp_dev *jpeg)
> > +{
> > +	clk_bulk_disable_unprepare(jpeg->venc_clk.clk_num,
> > +				   jpeg->venc_clk.clks);
> > +}
> > +
> > +static __maybe_unused int mtk_jpegenc_pm_suspend(struct device
> > *dev)
> > +{
> > +	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_clk_off(jpeg);
> 
> This is the only call to that function, move the code here.

Thanks, I will change it.
> 
> >  
> >  	return 0;
> >  }
> >  
> > +static __maybe_unused int mtk_jpegenc_pm_resume(struct device
> > *dev)
> > +{
> > +	struct mtk_jpegenc_comp_dev *jpeg = dev_get_drvdata(dev);
> > +
> > +	mtk_jpeg_clk_on(jpeg);
> 
> This is the only call to that function, move the code here. Decides
> if the ret
> value should be passed or not too, add a comment if not.
> 
> Its worrying not too, since usually if the clock are not running, we
> risk
> hanging the CPU writing to registers.

Dear Nicolas,

Thanks for your kind suggestion. I also think the return value should
be handled. I will change it in the next version.

Regards,
Kyrie.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static __maybe_unused 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 __maybe_unused 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 ret;
> > +}
> > +
> > +static const struct dev_pm_ops mtk_jpegenc_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(mtk_jpegenc_suspend,
> > mtk_jpegenc_resume)
> > +	SET_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,
> >  	},
> >  };
> >  
> 
> Looks like the right way though,looking forward an update.
> 
> Nicolas
> 	Could not parse S/MIME message: security library: improperly
> formatted DER-encoded message. (-8183) - Decoder failed

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

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:37 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > move decoding buffer increase code into spinlock
> > protecting aera for multi-core
> 
> I don't understand this text, please rework.
Dear Nicolas,

The dst buf parameters only need to set when the power
set succussed and protect the setting by spinlock.

I will rework the commit message in the coming version.

Thanks.

Regards,
Kyrie.
> 
> > 
> > Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker
> > interface")
> > 
> 
> Drop that blank line.
> 
> > 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 e45d7e82b8a0..5ffaee4dcd19 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1746,7 +1746,6 @@ static void mtk_jpegdec_worker(struct
> > work_struct *work)
> >  
> >  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> >  	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)) 
> > {
> > @@ -1755,11 +1754,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) {
> > @@ -1784,6 +1778,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 5f1557dafad6..b3142dc9be85 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 785db5ba4770..82c971936c4d 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;
> >  			}
> >  		}
> >  	}
> 
> Normally I'd seek for the context in the code. But I think I'll wait
> for a
> proper commit message, and do a proper review then.
> 
> Nicolas

Re: [PATCH v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:40 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > 1.add a judgement for src buffer to avoid kernel crash
> > in the stop streaming function;
> > 2.When a resolution changing occurs, it needs to set new
> > resolution parameter immediately and then report this event.
> > Otherwise, if the original software process is maintained,
> > the resolution change event is reported firstly, the CPU is
> > dispatched to the app to process the event, and the driver
> > does not set a new resolution, which will cause parameter errors.
> > 3.After a resolution change occurred, decoding should not continue,
> > needs to wait until new buffers are ready and the state machine
> > changed.
> 
> I mention this in other patchset, very often, 3 bullets means 3
> distinct
> changes. Don't use bullets. Reflow this text, rework this text, there
> is many
> syntax error here.
> 
Thanks. I will rework it.

Kyrie.
> > 
> > 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 +++++++--
> >  1 file changed, 7 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 5ffaee4dcd19..9233bbfe2d97 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -887,7 +887,8 @@ static void mtk_jpeg_dec_stop_streaming(struct
> > vb2_queue
> > *q)
> >  
> >  		vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >  		src_buf = mtk_jpeg_vb2_to_srcbuf(&vb->vb2_buf);
> > -		mtk_jpeg_set_queue_data(ctx, &src_buf->dec_param);
> > +		if (!IS_ERR_OR_NULL(src_buf))
> > +			mtk_jpeg_set_queue_data(ctx, &src_buf-
> > >dec_param);
> 
> The lack of vb2_wait_for_all_buffers() might explains this better,
> you might not
> need to do random null checks like this.
> 
> Nicolas
> 
> >  		ctx->state = MTK_JPEG_RUNNING;
> >  	} else if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> >  		ctx->state = MTK_JPEG_INIT;
> > @@ -1749,11 +1750,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) {

Re: [PATCH v11 08/12] media: mediatek: jpeg: fix remove buffer operation for multi-core

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:43 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > move remove buffer code to spinlock protect area for multi-core
> 
> Perhaps:
> 
> Move calls to v4l2_m2m_src/dst_buf_remove() outside of the spinlock
> protected
> scope.
> 
> And perhaps extend in why this is needed. Deadlock, bad lock
> ordering?

Dear Nicolas,

move them to protected scope to avoid buffer lost if the worker
scheduled
but it hasn't enable jpeg hw succussed yet.

I will refine the commit message.

Thanks.

Regards,
Kyrie.
> 
> > 
> > Fixes: 86379bd9d399 ("media: mtk-jpeg: Fixes jpeg enc&dec worker sw
> > flow")
> > 
> 
> No blank.
> 
> > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Nicolas
> 
> > ---
> >  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 9233bbfe2d97..da1ca494ed4b 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1665,9 +1665,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));
> >  
> > @@ -1685,6 +1682,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);
> >  	jpeg_buf_queue_inc(ctx);
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >  	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
> > @@ -1767,9 +1766,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,
> > @@ -1794,6 +1790,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);
> >  	jpeg_buf_queue_inc(ctx);
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);

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

[Kyrie Wu (吴晗)]

On Tue, 2025-12-16 at 16:48 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 02 décembre 2025 à 17:48 +0800, Kyrie Wu a écrit :
> > Add a configuration to set jpeg dec & enc smmu sid
> > 
> > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> > ---
> >  .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 37
> > +++++++++++++++++++
> >  .../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, 98 insertions(+)
> > 
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > index 10a588b92e76..625dfa8468e1 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>
> > @@ -1613,6 +1614,20 @@ static irqreturn_t mtk_jpeg_enc_done(struct
> > mtk_jpeg_dev *jpeg)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +static void mtk_jpeg_enc_set_smmu_sid(struct mtk_jpegenc_comp_dev
> > *jpeg)
> > +{
> > +	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
> > +
> > +	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
> > +		return;
> > +
> > +	regmap_update_bits(jpeg->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];
> > @@ -1674,6 +1689,9 @@ 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);
> > +
> > +	mtk_jpeg_enc_set_smmu_sid(comp_jpeg[hw_id]);
> > +
> >  	mtk_jpeg_set_enc_dst(ctx,
> >  			     comp_jpeg[hw_id]->reg_base,
> >  			     &dst_buf->vb2_buf);
> > @@ -1701,6 +1719,20 @@ static void mtk_jpegenc_worker(struct
> > work_struct *work)
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >  }
> >  
> > +static void mtk_jpeg_dec_set_smmu_sid(struct mtk_jpegdec_comp_dev
> > *jpeg)
> > +{
> > +	struct mtk_jpeg_dev *mjpeg = jpeg->master_dev;
> > +
> > +	if (!mjpeg->variant->support_smmu || !jpeg->smmu_regmap)
> > +		return;
> > +
> > +	regmap_update_bits(jpeg->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);
> > +}
> 
> Can you deduplicate that function. Just make one, and pass the SID as
> parameter.

Dear Nicolas,

Thanks for your comments, I will refactor this function.

Regards,
Kyrie.
> 
> > +
> >  static void mtk_jpegdec_worker(struct work_struct *work)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = container_of(work, struct
> > mtk_jpeg_ctx,
> > @@ -1784,6 +1816,9 @@ 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);
> > +
> > +	mtk_jpeg_dec_set_smmu_sid(comp_jpeg[hw_id]);
> > +
> >  	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
> >  				jpeg->variant->support_34bit,
> >  				&jpeg_src_buf->dec_param,
> > @@ -1943,6 +1978,7 @@ static struct mtk_jpeg_variant
> > mtk8196_jpegenc_drvdata = {
> >  	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> >  	.multi_core = true,
> >  	.jpeg_worker = mtk_jpegenc_worker,
> > +	.support_smmu = true,
> >  };
> >  
> >  static const struct mtk_jpeg_variant mtk8195_jpegdec_drvdata = {
> > @@ -1969,6 +2005,7 @@ static const struct mtk_jpeg_variant
> > mtk8196_jpegdec_drvdata = {
> >  	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
> >  	.multi_core = true,
> >  	.jpeg_worker = mtk_jpegdec_worker,
> > +	.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 33f7fbc4ca5e..6e8304680393 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 e453a1634f33..da753a636eaa 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > @@ -624,6 +624,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;
> > @@ -677,6 +696,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 b30c728c3712..8a61d5537315 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > @@ -348,6 +348,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;
> > @@ -399,6 +418,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,
> 
> With that minor fix:
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Re: [PATCH v11 03/12] media: mediatek: jpeg: fix jpeg buffer layout

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 4209 bytes --]

Le jeudi 25 décembre 2025 à 06:05 +0000, Kyrie Wu (吴晗) a écrit :
> On Tue, 2025-12-16 at 16:22 -0500, Nicolas Dufresne wrote:
> > Hi,
> > 
> > Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :
> > > For memory alloc operation of jpeg dst buffer: the mallocing
> > > memory function interface use vb2_buffer as the base addr.
> > > If structure mtk_jpeg_src_buf wants to be allocated to memory,
> > > it needs to be placed vb2_v4l2_buffer at the starting position,
> > > because structure vb2_buffer is at the starting position of
> > > vb2_v4l2_buffer, and the allocated size is set to the size of
> > > structure mtk_jpeg_src_buf, so as to ensure that structures
> > > mtk_jpeg_src_buf, vb2_v4l2_buffer and vb2_buffer can all be
> > > allocated memory.
> > 
> > Please correct the wording, "mallocing" is not a word, addr ->
> > address. I tend
> > to do the same, but refrain from giving the code a personality, the
> > vb2_buffer
> > have no will. This is overall complicated way to simply say:
> > 
> > 	Fix the size of the allocated mtk_jpeg_src_buf structure.
> > 
> > > 
> > > Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker
> > > interface")
> > > 
> > 
> > Drop the blank line.
> > 
> > > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.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 0cf3dc5407e4..bd0afc93d491 100644
> > > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > > @@ -1099,7 +1099,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;
> > 
> > I like the change, but this shouldn't be an issue if you use
> > container_of, which
> > is safer then a plain cast. Please review the code and make sure to
> > use it. It
> > may be confusing to include a cosmetic change in a fixes.
> > 
> > Nicolas
> 
> Dear Nicolas,
> 
> The driver requests memory space for mtk_jpeg_src_buf in the
> function of mtk_jpeg_queue_init,
> and it used struct vb2_v4l2_buffer b as the starting base address,
> the parameter,frame_num, will not get a memory if we keep the old
> structure. And a unknown memory would be get if we used container_of
> to get the starting address of mtk_jpeg_src_buf.

You are right, and the documentation says so:

 *              The first field of the
 *		driver-specific buffer structure must be the subsystem-specific
 *		struct (vb2_v4l2_buffer in the case of V4L2).

This is inconsistent with how other objects are overloaded, notably the request
object, which does not have this limitation. We should probably fix that
someday, meanwhile.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> 
> Thanks.
> 
> Regards,
> Kyrie.
> > 
> > >  	struct mtk_jpeg_dec_param dec_param;
> > >  
> > >  	struct mtk_jpeg_ctx *curr_ctx;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]