[PATCH v2] media: mtk-jpeg: drain hardware completion before freeing context

[PATCH v2] media: mtk-jpeg: drain hardware completion before freeing context

[Guangshuo Li]

The change referenced by the Fixes tag cancels ctx->jpeg_work before
freeing the JPEG context.

That prevents the worker itself from accessing the context after it has
been freed. However, the multi-core workers return after programming a
hardware instance. The IRQ handler or the per-hardware timeout work can
then continue to access the context through hw_param.curr_ctx after the
worker has completed.

If userspace closes the file while a hardware job is still pending,
cancel_work_sync() can return and mtk_jpeg_release() can free the
context before the IRQ or timeout path has finished using it.

Track the number of in-flight hardware jobs for each context and wait
for them to complete in mtk_jpeg_release().

Use a per-hardware active flag to ensure that only the IRQ handler or
the timeout work completes each job. Clear hw_param.curr_ctx and drop
the in-flight count only after the completion path has finished all
accesses to the context.

Fixes: 34c519feef3e ("media: mtk-jpeg: fix use-after-free in release path due to uncancelled work")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Replace the undefined mtk_jpeg_release_hw() call with per-context
    in-flight hardware job tracking.
  - Ensure that only the IRQ or timeout path completes each hardware job.
  - Clear hw_param.curr_ctx after the completion path stops using the
    context.
  - Keep hardware instances busy until the timeout path has finished
    accessing their saved context and buffers.

 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 12 ++++++++-
 .../platform/mediatek/jpeg/mtk_jpeg_core.h    | 14 +++++++++++
 .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 25 +++++++++++++------
 .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 25 +++++++++++++------
 4 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d147ec483081..d41c0e0516b9 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1161,6 +1161,8 @@ static int mtk_jpeg_open(struct file *file)
 	}
 
 	INIT_WORK(&ctx->jpeg_work, jpeg->variant->jpeg_worker);
+	atomic_set(&ctx->hw_jobs, 0);
+	init_waitqueue_head(&ctx->hw_jobs_wq);
 	INIT_LIST_HEAD(&ctx->dst_done_queue);
 	spin_lock_init(&ctx->done_queue_lock);
 	v4l2_fh_init(&ctx->fh, vfd);
@@ -1202,8 +1204,12 @@ static int mtk_jpeg_release(struct file *file)
 	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
 	struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
 
-	if (jpeg->variant->jpeg_worker)
+	if (jpeg->variant->jpeg_worker) {
 		cancel_work_sync(&ctx->jpeg_work);
+		wait_event(ctx->hw_jobs_wq,
+			   atomic_read(&ctx->hw_jobs) == 0);
+	}
+
 	mutex_lock(&jpeg->lock);
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
@@ -1640,6 +1646,8 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
+	atomic_inc(&ctx->hw_jobs);
+	atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
 	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
 			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
@@ -1759,6 +1767,8 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 		goto setdst_end;
 	}
 
+	atomic_inc(&ctx->hw_jobs);
+	atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
 	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
 			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 02ed0ed5b736..be8ada60782a 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
@@ -103,6 +103,7 @@ struct mtk_jpeg_hw_param {
 	struct vb2_v4l2_buffer *src_buffer;
 	struct vb2_v4l2_buffer *dst_buffer;
 	struct mtk_jpeg_ctx *curr_ctx;
+	atomic_t job_active;
 };
 
 enum mtk_jpegenc_hw_id {
@@ -282,6 +283,8 @@ struct mtk_jpeg_q_data {
  * @restart_interval:		jpeg encoder restart interval
  * @ctrl_hdl:			controls handler
  * @jpeg_work:			jpeg encoder workqueue
+ * @hw_jobs:			number of hardware jobs still referencing this context
+ * @hw_jobs_wq:			wait queue for hardware job completion
  * @total_frame_num:		encoded frame number
  * @dst_done_queue:		encoded frame buffer queue
  * @done_queue_lock:		encoded frame operation spinlock
@@ -299,6 +302,8 @@ struct mtk_jpeg_ctx {
 	struct v4l2_ctrl_handler ctrl_hdl;
 
 	struct work_struct jpeg_work;
+	atomic_t hw_jobs;
+	wait_queue_head_t hw_jobs_wq;
 	u32 total_frame_num;
 	struct list_head dst_done_queue;
 	/* spinlock protecting the encode done buffer */
@@ -306,4 +311,13 @@ struct mtk_jpeg_ctx {
 	u32 last_done_frame_num;
 };
 
+static inline void
+mtk_jpeg_hw_job_done(struct mtk_jpeg_hw_param *hw_param,
+		     struct mtk_jpeg_ctx *ctx)
+{
+	WRITE_ONCE(hw_param->curr_ctx, NULL);
+	if (atomic_dec_and_test(&ctx->hw_jobs))
+		wake_up(&ctx->hw_jobs_wq);
+}
+
 #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 32372781daf5..c2d40653891a 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -527,7 +527,12 @@ 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;
+
+	if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
+		return;
 
+	ctx = cjpeg->hw_param.curr_ctx;
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
@@ -535,11 +540,13 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
 	pm_runtime_put(cjpeg->dev);
+	v4l2_m2m_buf_done(src_buf, buf_state);
+	mtk_jpegdec_put_buf(cjpeg);
+	mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
+
 	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);
 }
 
 static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
@@ -555,12 +562,10 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	struct mtk_jpegdec_comp_dev *jpeg = priv;
 	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
 
-	cancel_delayed_work(&jpeg->job_timeout_work);
+	if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
+		return IRQ_HANDLED;
 
-	ctx = jpeg->hw_param.curr_ctx;
-	src_buf = jpeg->hw_param.src_buffer;
-	dst_buf = jpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
+	cancel_delayed_work(&jpeg->job_timeout_work);
 
 	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
 	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
@@ -570,6 +575,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE)
 		dev_warn(jpeg->dev, "Jpg Dec occurs unknown Err.");
 
+	ctx = jpeg->hw_param.curr_ctx;
+	src_buf = jpeg->hw_param.src_buffer;
+	dst_buf = jpeg->hw_param.dst_buffer;
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
+
 	jpeg_src_buf =
 		container_of(src_buf, struct mtk_jpeg_src_buf, b);
 
@@ -582,6 +592,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	mtk_jpegdec_put_buf(jpeg);
 	pm_runtime_put(ctx->jpeg->dev);
 	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
+	mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
 
 	jpeg->hw_state = MTK_JPEG_HW_IDLE;
 	wake_up(&master_jpeg->hw_wq);
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 b312a15d707b..b08f94845613 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -257,7 +257,12 @@ 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;
+
+	if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
+		return;
 
+	ctx = cjpeg->hw_param.curr_ctx;
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
@@ -265,11 +270,13 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	mtk_jpeg_enc_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
 	pm_runtime_put(cjpeg->dev);
+	v4l2_m2m_buf_done(src_buf, buf_state);
+	mtk_jpegenc_put_buf(cjpeg);
+	mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
+
 	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);
 }
 
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
@@ -283,12 +290,10 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	struct mtk_jpegenc_comp_dev *jpeg = priv;
 	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
 
-	cancel_delayed_work(&jpeg->job_timeout_work);
+	if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
+		return IRQ_HANDLED;
 
-	ctx = jpeg->hw_param.curr_ctx;
-	src_buf = jpeg->hw_param.src_buffer;
-	dst_buf = jpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
+	cancel_delayed_work(&jpeg->job_timeout_work);
 
 	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
 		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
@@ -297,6 +302,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
 		dev_warn(jpeg->dev, "Jpg Enc occurs unknown Err.");
 
+	ctx = jpeg->hw_param.curr_ctx;
+	src_buf = jpeg->hw_param.src_buffer;
+	dst_buf = jpeg->hw_param.dst_buffer;
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
+
 	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base,
 						 ctx->jpeg->variant->support_34bit);
 	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
@@ -305,6 +315,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	mtk_jpegenc_put_buf(jpeg);
 	pm_runtime_put(ctx->jpeg->dev);
 	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
+	mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
 
 	jpeg->hw_state = MTK_JPEG_HW_IDLE;
 	wake_up(&master_jpeg->hw_wq);
-- 
2.43.0

Re: [PATCH v2] media: mtk-jpeg: drain hardware completion before freeing context

[Nicolas Dufresne]

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

Le samedi 18 juillet 2026 à 19:24 +0800, Guangshuo Li a écrit :
> The change referenced by the Fixes tag cancels ctx->jpeg_work before
> freeing the JPEG context.
> 
> That prevents the worker itself from accessing the context after it has
> been freed. However, the multi-core workers return after programming a
> hardware instance. The IRQ handler or the per-hardware timeout work can
> then continue to access the context through hw_param.curr_ctx after the
> worker has completed.
> 
> If userspace closes the file while a hardware job is still pending,
> cancel_work_sync() can return and mtk_jpeg_release() can free the
> context before the IRQ or timeout path has finished using it.
> 
> Track the number of in-flight hardware jobs for each context and wait
> for them to complete in mtk_jpeg_release().
> 
> Use a per-hardware active flag to ensure that only the IRQ handler or
> the timeout work completes each job. Clear hw_param.curr_ctx and drop
> the in-flight count only after the completion path has finished all
> accesses to the context.
> 
> Fixes: 34c519feef3e ("media: mtk-jpeg: fix use-after-free in release path due to uncancelled work")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
>   - Replace the undefined mtk_jpeg_release_hw() call with per-context
>     in-flight hardware job tracking.
>   - Ensure that only the IRQ or timeout path completes each hardware job.
>   - Clear hw_param.curr_ctx after the completion path stops using the
>     context.
>   - Keep hardware instances busy until the timeout path has finished
>     accessing their saved context and buffers.
> 
>  .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 12 ++++++++-
>  .../platform/mediatek/jpeg/mtk_jpeg_core.h    | 14 +++++++++++
>  .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 25 +++++++++++++------
>  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 25 +++++++++++++------
>  4 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec483081..d41c0e0516b9 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1161,6 +1161,8 @@ static int mtk_jpeg_open(struct file *file)
>  	}
>  
>  	INIT_WORK(&ctx->jpeg_work, jpeg->variant->jpeg_worker);
> +	atomic_set(&ctx->hw_jobs, 0);
> +	init_waitqueue_head(&ctx->hw_jobs_wq);
>  	INIT_LIST_HEAD(&ctx->dst_done_queue);
>  	spin_lock_init(&ctx->done_queue_lock);
>  	v4l2_fh_init(&ctx->fh, vfd);
> @@ -1202,8 +1204,12 @@ static int mtk_jpeg_release(struct file *file)
>  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
>  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
>  
> -	if (jpeg->variant->jpeg_worker)
> +	if (jpeg->variant->jpeg_worker) {
>  		cancel_work_sync(&ctx->jpeg_work);
> +		wait_event(ctx->hw_jobs_wq,
> +			   atomic_read(&ctx->hw_jobs) == 0);
> +	}
> +
>  	mutex_lock(&jpeg->lock);
>  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);

The things is that we already paid the price of this complexity in the m2m
framework by synchronously waiting for the last v4l2_m2m_job_finish() to be
called from the driver. I think we should not add a mechanism on top, but rather
figure-out why this is not working for your usage of this driver.

Nicolas

>  	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> @@ -1640,6 +1646,8 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>  	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>  
> +	atomic_inc(&ctx->hw_jobs);
> +	atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
>  	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
>  			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
> @@ -1759,6 +1767,8 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  		goto setdst_end;
>  	}
>  
> +	atomic_inc(&ctx->hw_jobs);
> +	atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
>  	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
>  			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> index 02ed0ed5b736..be8ada60782a 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> @@ -103,6 +103,7 @@ struct mtk_jpeg_hw_param {
>  	struct vb2_v4l2_buffer *src_buffer;
>  	struct vb2_v4l2_buffer *dst_buffer;
>  	struct mtk_jpeg_ctx *curr_ctx;
> +	atomic_t job_active;
>  };
>  
>  enum mtk_jpegenc_hw_id {
> @@ -282,6 +283,8 @@ struct mtk_jpeg_q_data {
>   * @restart_interval:		jpeg encoder restart interval
>   * @ctrl_hdl:			controls handler
>   * @jpeg_work:			jpeg encoder workqueue
> + * @hw_jobs:			number of hardware jobs still referencing this context
> + * @hw_jobs_wq:			wait queue for hardware job completion
>   * @total_frame_num:		encoded frame number
>   * @dst_done_queue:		encoded frame buffer queue
>   * @done_queue_lock:		encoded frame operation spinlock
> @@ -299,6 +302,8 @@ struct mtk_jpeg_ctx {
>  	struct v4l2_ctrl_handler ctrl_hdl;
>  
>  	struct work_struct jpeg_work;
> +	atomic_t hw_jobs;
> +	wait_queue_head_t hw_jobs_wq;
>  	u32 total_frame_num;
>  	struct list_head dst_done_queue;
>  	/* spinlock protecting the encode done buffer */
> @@ -306,4 +311,13 @@ struct mtk_jpeg_ctx {
>  	u32 last_done_frame_num;
>  };
>  
> +static inline void
> +mtk_jpeg_hw_job_done(struct mtk_jpeg_hw_param *hw_param,
> +		     struct mtk_jpeg_ctx *ctx)
> +{
> +	WRITE_ONCE(hw_param->curr_ctx, NULL);
> +	if (atomic_dec_and_test(&ctx->hw_jobs))
> +		wake_up(&ctx->hw_jobs_wq);
> +}
> +
>  #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 32372781daf5..c2d40653891a 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -527,7 +527,12 @@ 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;
> +
> +	if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
> +		return;
>  
> +	ctx = cjpeg->hw_param.curr_ctx;
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> @@ -535,11 +540,13 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
>  	pm_runtime_put(cjpeg->dev);
> +	v4l2_m2m_buf_done(src_buf, buf_state);
> +	mtk_jpegdec_put_buf(cjpeg);
> +	mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
> +
>  	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);
>  }
>  
>  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> @@ -555,12 +562,10 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	struct mtk_jpegdec_comp_dev *jpeg = priv;
>  	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
>  
> -	cancel_delayed_work(&jpeg->job_timeout_work);
> +	if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
> +		return IRQ_HANDLED;
>  
> -	ctx = jpeg->hw_param.curr_ctx;
> -	src_buf = jpeg->hw_param.src_buffer;
> -	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> +	cancel_delayed_work(&jpeg->job_timeout_work);
>  
>  	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
>  	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> @@ -570,6 +575,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE)
>  		dev_warn(jpeg->dev, "Jpg Dec occurs unknown Err.");
>  
> +	ctx = jpeg->hw_param.curr_ctx;
> +	src_buf = jpeg->hw_param.src_buffer;
> +	dst_buf = jpeg->hw_param.dst_buffer;
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> +
>  	jpeg_src_buf =
>  		container_of(src_buf, struct mtk_jpeg_src_buf, b);
>  
> @@ -582,6 +592,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	mtk_jpegdec_put_buf(jpeg);
>  	pm_runtime_put(ctx->jpeg->dev);
>  	clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
> +	mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
>  
>  	jpeg->hw_state = MTK_JPEG_HW_IDLE;
>  	wake_up(&master_jpeg->hw_wq);
> 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 b312a15d707b..b08f94845613 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -257,7 +257,12 @@ 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;
> +
> +	if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
> +		return;
>  
> +	ctx = cjpeg->hw_param.curr_ctx;
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> @@ -265,11 +270,13 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
>  	pm_runtime_put(cjpeg->dev);
> +	v4l2_m2m_buf_done(src_buf, buf_state);
> +	mtk_jpegenc_put_buf(cjpeg);
> +	mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
> +
>  	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);
>  }
>  
>  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> @@ -283,12 +290,10 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
>  	struct mtk_jpegenc_comp_dev *jpeg = priv;
>  	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
>  
> -	cancel_delayed_work(&jpeg->job_timeout_work);
> +	if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
> +		return IRQ_HANDLED;
>  
> -	ctx = jpeg->hw_param.curr_ctx;
> -	src_buf = jpeg->hw_param.src_buffer;
> -	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> +	cancel_delayed_work(&jpeg->job_timeout_work);
>  
>  	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>  		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> @@ -297,6 +302,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
>  	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
>  		dev_warn(jpeg->dev, "Jpg Enc occurs unknown Err.");
>  
> +	ctx = jpeg->hw_param.curr_ctx;
> +	src_buf = jpeg->hw_param.src_buffer;
> +	dst_buf = jpeg->hw_param.dst_buffer;
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> +
>  	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base,
>  						 ctx->jpeg->variant->support_34bit);
>  	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
> @@ -305,6 +315,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
>  	mtk_jpegenc_put_buf(jpeg);
>  	pm_runtime_put(ctx->jpeg->dev);
>  	clk_disable_unprepare(jpeg->venc_clk.clks->clk);
> +	mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
>  
>  	jpeg->hw_state = MTK_JPEG_HW_IDLE;
>  	wake_up(&master_jpeg->hw_wq);

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

Re: [PATCH v2] media: mtk-jpeg: drain hardware completion before freeing context

[Guangshuo Li]

Hi Nicolas,

On Sat, 18 Jul 2026 at 22:56, Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Le samedi 18 juillet 2026 à 19:24 +0800, Guangshuo Li a écrit :
> > The change referenced by the Fixes tag cancels ctx->jpeg_work before
> > freeing the JPEG context.
> >
> > That prevents the worker itself from accessing the context after it has
> > been freed. However, the multi-core workers return after programming a
> > hardware instance. The IRQ handler or the per-hardware timeout work can
> > then continue to access the context through hw_param.curr_ctx after the
> > worker has completed.
> >
> > If userspace closes the file while a hardware job is still pending,
> > cancel_work_sync() can return and mtk_jpeg_release() can free the
> > context before the IRQ or timeout path has finished using it.
> >
> > Track the number of in-flight hardware jobs for each context and wait
> > for them to complete in mtk_jpeg_release().
> >
> > Use a per-hardware active flag to ensure that only the IRQ handler or
> > the timeout work completes each job. Clear hw_param.curr_ctx and drop
> > the in-flight count only after the completion path has finished all
> > accesses to the context.
> >
> > Fixes: 34c519feef3e ("media: mtk-jpeg: fix use-after-free in release path due to uncancelled work")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> > v2:
> >   - Replace the undefined mtk_jpeg_release_hw() call with per-context
> >     in-flight hardware job tracking.
> >   - Ensure that only the IRQ or timeout path completes each hardware job.
> >   - Clear hw_param.curr_ctx after the completion path stops using the
> >     context.
> >   - Keep hardware instances busy until the timeout path has finished
> >     accessing their saved context and buffers.
> >
> >  .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 12 ++++++++-
> >  .../platform/mediatek/jpeg/mtk_jpeg_core.h    | 14 +++++++++++
> >  .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 25 +++++++++++++------
> >  .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 25 +++++++++++++------
> >  4 files changed, 61 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > index d147ec483081..d41c0e0516b9 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1161,6 +1161,8 @@ static int mtk_jpeg_open(struct file *file)
> >       }
> >
> >       INIT_WORK(&ctx->jpeg_work, jpeg->variant->jpeg_worker);
> > +     atomic_set(&ctx->hw_jobs, 0);
> > +     init_waitqueue_head(&ctx->hw_jobs_wq);
> >       INIT_LIST_HEAD(&ctx->dst_done_queue);
> >       spin_lock_init(&ctx->done_queue_lock);
> >       v4l2_fh_init(&ctx->fh, vfd);
> > @@ -1202,8 +1204,12 @@ static int mtk_jpeg_release(struct file *file)
> >       struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >       struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
> >
> > -     if (jpeg->variant->jpeg_worker)
> > +     if (jpeg->variant->jpeg_worker) {
> >               cancel_work_sync(&ctx->jpeg_work);
> > +             wait_event(ctx->hw_jobs_wq,
> > +                        atomic_read(&ctx->hw_jobs) == 0);
> > +     }
> > +
> >       mutex_lock(&jpeg->lock);
> >       v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>
> The things is that we already paid the price of this complexity in the m2m
> framework by synchronously waiting for the last v4l2_m2m_job_finish() to be
> called from the driver. I think we should not add a mechanism on top, but rather
> figure-out why this is not working for your usage of this driver.
>
> Nicolas
>
> >       v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > @@ -1640,6 +1646,8 @@ static void mtk_jpegenc_worker(struct work_struct *work)
> >       v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> >       v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> >
> > +     atomic_inc(&ctx->hw_jobs);
> > +     atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
> >       schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
> >                             msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
> >
> > @@ -1759,6 +1767,8 @@ static void mtk_jpegdec_worker(struct work_struct *work)
> >               goto setdst_end;
> >       }
> >
> > +     atomic_inc(&ctx->hw_jobs);
> > +     atomic_set_release(&comp_jpeg[hw_id]->hw_param.job_active, 1);
> >       schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
> >                             msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
> >
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > index 02ed0ed5b736..be8ada60782a 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > @@ -103,6 +103,7 @@ struct mtk_jpeg_hw_param {
> >       struct vb2_v4l2_buffer *src_buffer;
> >       struct vb2_v4l2_buffer *dst_buffer;
> >       struct mtk_jpeg_ctx *curr_ctx;
> > +     atomic_t job_active;
> >  };
> >
> >  enum mtk_jpegenc_hw_id {
> > @@ -282,6 +283,8 @@ struct mtk_jpeg_q_data {
> >   * @restart_interval:                jpeg encoder restart interval
> >   * @ctrl_hdl:                        controls handler
> >   * @jpeg_work:                       jpeg encoder workqueue
> > + * @hw_jobs:                 number of hardware jobs still referencing this context
> > + * @hw_jobs_wq:                      wait queue for hardware job completion
> >   * @total_frame_num:         encoded frame number
> >   * @dst_done_queue:          encoded frame buffer queue
> >   * @done_queue_lock:         encoded frame operation spinlock
> > @@ -299,6 +302,8 @@ struct mtk_jpeg_ctx {
> >       struct v4l2_ctrl_handler ctrl_hdl;
> >
> >       struct work_struct jpeg_work;
> > +     atomic_t hw_jobs;
> > +     wait_queue_head_t hw_jobs_wq;
> >       u32 total_frame_num;
> >       struct list_head dst_done_queue;
> >       /* spinlock protecting the encode done buffer */
> > @@ -306,4 +311,13 @@ struct mtk_jpeg_ctx {
> >       u32 last_done_frame_num;
> >  };
> >
> > +static inline void
> > +mtk_jpeg_hw_job_done(struct mtk_jpeg_hw_param *hw_param,
> > +                  struct mtk_jpeg_ctx *ctx)
> > +{
> > +     WRITE_ONCE(hw_param->curr_ctx, NULL);
> > +     if (atomic_dec_and_test(&ctx->hw_jobs))
> > +             wake_up(&ctx->hw_jobs_wq);
> > +}
> > +
> >  #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 32372781daf5..c2d40653891a 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> > @@ -527,7 +527,12 @@ 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;
> > +
> > +     if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
> > +             return;
> >
> > +     ctx = cjpeg->hw_param.curr_ctx;
> >       src_buf = cjpeg->hw_param.src_buffer;
> >       dst_buf = cjpeg->hw_param.dst_buffer;
> >       v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > @@ -535,11 +540,13 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
> >       mtk_jpeg_dec_reset(cjpeg->reg_base);
> >       clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
> >       pm_runtime_put(cjpeg->dev);
> > +     v4l2_m2m_buf_done(src_buf, buf_state);
> > +     mtk_jpegdec_put_buf(cjpeg);
> > +     mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
> > +
> >       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);
> >  }
> >
> >  static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> > @@ -555,12 +562,10 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> >       struct mtk_jpegdec_comp_dev *jpeg = priv;
> >       struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
> >
> > -     cancel_delayed_work(&jpeg->job_timeout_work);
> > +     if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
> > +             return IRQ_HANDLED;
> >
> > -     ctx = jpeg->hw_param.curr_ctx;
> > -     src_buf = jpeg->hw_param.src_buffer;
> > -     dst_buf = jpeg->hw_param.dst_buffer;
> > -     v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > +     cancel_delayed_work(&jpeg->job_timeout_work);
> >
> >       irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
> >       dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> > @@ -570,6 +575,11 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> >       if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE)
> >               dev_warn(jpeg->dev, "Jpg Dec occurs unknown Err.");
> >
> > +     ctx = jpeg->hw_param.curr_ctx;
> > +     src_buf = jpeg->hw_param.src_buffer;
> > +     dst_buf = jpeg->hw_param.dst_buffer;
> > +     v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > +
> >       jpeg_src_buf =
> >               container_of(src_buf, struct mtk_jpeg_src_buf, b);
> >
> > @@ -582,6 +592,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> >       mtk_jpegdec_put_buf(jpeg);
> >       pm_runtime_put(ctx->jpeg->dev);
> >       clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
> > +     mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
> >
> >       jpeg->hw_state = MTK_JPEG_HW_IDLE;
> >       wake_up(&master_jpeg->hw_wq);
> > 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 b312a15d707b..b08f94845613 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> > @@ -257,7 +257,12 @@ 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;
> > +
> > +     if (atomic_cmpxchg(&cjpeg->hw_param.job_active, 1, 0) != 1)
> > +             return;
> >
> > +     ctx = cjpeg->hw_param.curr_ctx;
> >       src_buf = cjpeg->hw_param.src_buffer;
> >       dst_buf = cjpeg->hw_param.dst_buffer;
> >       v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > @@ -265,11 +270,13 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
> >       mtk_jpeg_enc_reset(cjpeg->reg_base);
> >       clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
> >       pm_runtime_put(cjpeg->dev);
> > +     v4l2_m2m_buf_done(src_buf, buf_state);
> > +     mtk_jpegenc_put_buf(cjpeg);
> > +     mtk_jpeg_hw_job_done(&cjpeg->hw_param, ctx);
> > +
> >       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);
> >  }
> >
> >  static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> > @@ -283,12 +290,10 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> >       struct mtk_jpegenc_comp_dev *jpeg = priv;
> >       struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
> >
> > -     cancel_delayed_work(&jpeg->job_timeout_work);
> > +     if (atomic_cmpxchg(&jpeg->hw_param.job_active, 1, 0) != 1)
> > +             return IRQ_HANDLED;
> >
> > -     ctx = jpeg->hw_param.curr_ctx;
> > -     src_buf = jpeg->hw_param.src_buffer;
> > -     dst_buf = jpeg->hw_param.dst_buffer;
> > -     v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > +     cancel_delayed_work(&jpeg->job_timeout_work);
> >
> >       irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
> >               JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> > @@ -297,6 +302,11 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> >       if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
> >               dev_warn(jpeg->dev, "Jpg Enc occurs unknown Err.");
> >
> > +     ctx = jpeg->hw_param.curr_ctx;
> > +     src_buf = jpeg->hw_param.src_buffer;
> > +     dst_buf = jpeg->hw_param.dst_buffer;
> > +     v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> > +
> >       result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base,
> >                                                ctx->jpeg->variant->support_34bit);
> >       vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
> > @@ -305,6 +315,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> >       mtk_jpegenc_put_buf(jpeg);
> >       pm_runtime_put(ctx->jpeg->dev);
> >       clk_disable_unprepare(jpeg->venc_clk.clks->clk);
> > +     mtk_jpeg_hw_job_done(&jpeg->hw_param, ctx);
> >
> >       jpeg->hw_state = MTK_JPEG_HW_IDLE;
> >       wake_up(&master_jpeg->hw_wq);

Thanks. I agree that the v4l2-m2m framework should normally provide
the required lifetime synchronization.

The reason it does not appear to work for the multi-core path is that
mtk_jpegenc_worker() and mtk_jpegdec_worker() call
v4l2_m2m_job_finish() immediately after programming the hardware, before
the IRQ or timeout completion path has finished using the context.

That clears TRANS_RUNNING, so a later v4l2_m2m_ctx_release() has no
running m2m job to wait for, while hw_param.curr_ctx can still point to
the context and the IRQ or timeout path can still dereference it.

I added the private in-flight tracking because the early job_finish()
also appears to be what allows the m2m scheduler to dispatch additional
jobs to the other hardware instances. Deferring job_finish() until the
IRQ or timeout path would restore the normal framework lifetime
contract, but since v4l2-m2m tracks a single current transaction per
m2m_dev, it also appears to serialize the multi-core variant.

Would you prefer a correctness-first change that moves job_finish() to
the hardware completion paths, accepting that serialization, or is
there an existing v4l2-m2m mechanism for representing several
concurrent hardware jobs that I have missed?

Thanks,
Guangshuo

Re: [PATCH v2] media: mtk-jpeg: drain hardware completion before freeing context

[Nicolas Dufresne]

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

Hi,

Le mardi 21 juillet 2026 à 17:15 +0800, Guangshuo Li a écrit :
> Would you prefer a correctness-first change that moves job_finish() to
> the hardware completion paths, accepting that serialization, or is
> there an existing v4l2-m2m mechanism for representing several
> concurrent hardware jobs that I have missed?

This one predates me as a maintainer, so I'm a bit in catchup mode. I think your
work shows how much problems/complexity this method introduce. I'm currently
tracking two proposal, an RKVDEC and an RGA2 proposal. I think the second method
is worth looking into. The first one might have had the same issue as this one,
as it called job_finish() immediatly in device_run() (which is equivalent of
doing it in a worker like MTK codecs do in general**. 

https://lore.kernel.org/all/20260606-spu-rga3multicore-v1-0-3ec2b15675f7@pengutronix.de/

And you'll find the patch:

[PATCH 05/17] media: v4l2-mem2mem: support running multiple jobs in parallel

That adds parallel job in the mem2mem, so that you can have a single m2m context
for multiple cores, handling multiple jobs, and the tear down is happening on
all cores. Now, I'm not forcing you to do that path, but I think its a better
solution long term, as open coding custom tear down is error prone. Let me know
your thought, of course I'd like if you could work with Sven on a common
solution.

Nicolas


** Most of MTK codecs worker are not needed, since the m2m framework already has
a worker and is re-entrent safe for triggering the next job. But MTK code uses a
lot of condition wait, turning async operation into synchronous, and once multi-
core, it just break apart.

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