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

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- 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 --]