Re: [PATCH v1 5/7] media: chips-media: wave5: Defer job_finish() only when a DEC_PIC was queued

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> Decoder instances sharing a VPU also share one v4l2_m2m job slot, released
> when the running context calls v4l2_m2m_job_finish(). While draining,
> device_run() defers job_finish() once EOS is sent (sent_eos), expecting a
> later finish_decode() (from a DEC_PIC completion IRQ) to release the slot.
> 
> But the m2m core checks job_ready() only when a job is queued, not when it
> is dispatched. A job queued while draining can run after finish_decode()
> has already moved the instance to STOP and sent EOS. device_run() then runs
> in STOP, issues no DEC_PIC, yet still skips job_finish() - so no IRQ, no
> finish_decode(), and the shared slot is leaked, stalling every instance.
> With several v4l2h264dec instances in parallel, GStreamer hangs at EOS.
> 
> Track whether the run actually queued a DEC_PIC (cmd_issued) and defer
> job_finish() only then. Otherwise finish the job immediately
> 
> Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

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

> ---
>  .../media/platform/chips-media/wave5/wave5-vpu-dec.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 93f7b724d86c..f33c00cb801b 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -1655,6 +1655,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>  	struct queue_status_info q_status;
>  	u32 fail_res = 0;
>  	int ret = 0;
> +	bool cmd_issued = false;
>  
>  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
>  	pm_runtime_resume_and_get(inst->dev->dev);
> @@ -1752,6 +1753,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>  			inst->retry = false;
>  			if (!inst->eos)
>  				inst->queuing_num--;
> +			cmd_issued = true;
>  		}
>  		break;
>  	default:
> @@ -1769,8 +1771,16 @@ static void wave5_vpu_dec_device_run(void *priv)
>  	 * in power and CPU time.
>  	 * If EOS is passed, device_run will not call job_finish no more, it is called
>  	 * only if HW is idle status in order to reduce overhead.
> +	 *
> +	 * Deferring job_finish() is only safe when this run actually queued a
> +	 * DEC_PIC command (cmd_issued): that guarantees a completion IRQ, and
> +	 * thus a later finish_decode(), will release the shared job slot. When
> +	 * device_run() is entered with no command to issue (e.g. a job that was
> +	 * queued while draining but reached the STOP state by the time it ran),
> +	 * no IRQ follows, so finish the job here to avoid leaking the slot and
> +	 * stalling every instance sharing the VPU.
>  	 */
> -	if (!inst->sent_eos)
> +	if (!inst->sent_eos || !cmd_issued)
>  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>  

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