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

Kyrie Wu posted 12 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation
Posted by Kyrie Wu 2 months, 1 week ago
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
Re: [PATCH v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation
Posted by Nicolas Dufresne 1 month, 3 weeks ago
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) {
Re: [PATCH v11 07/12] media: mediatek: jpeg: fix decoding resolution change operation
Posted by Kyrie Wu (吴晗) 1 month, 2 weeks ago
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) {