[PATCH] media: verisilicon: vp9: do not arm watchdog on preparation failure
Benjamin Gaignard
benjamin.gaignard at collabora.com
Thu Jul 23 09:05:03 PDT 2026
Le 23/07/2026 à 17:36, Tharit Tangkijwanichakul a écrit :
> hantro_g2_vp9_dec_run() calls hantro_end_prepare_run() when
> start_prepare_run() fails to find one of the required VP9 controls.
>
> hantro_end_prepare_run() completes the control request, but also schedules
> the hardware watchdog. Since the decode operation has not been started on
> this error path, the watchdog should not be scheduled.
>
> Factor the control-request completion out of hantro_end_prepare_run() and
> call it directly from the VP9 preparation error path. This preserves the
> pairing with hantro_start_prepare_run() without scheduling the watchdog.
Nack.
First it schedules a software timer not an hardware timer, which led after
timeout to call hantro_job_finish_no_pm() with VB2_BUF_STATE_ERROR parameter.
We need this call to complete v4l2_m2m job.
If you want to stop the task before the timeout you need to add something like
hantro_end_prepare_run_with_error() where you do the correct sequence.
Regards,
Benjamin
>
> Signed-off-by: Tharit Tangkijwanichakul <tharitt97 at gmail.com>
> ---
> Testing:
>
> - Built with the Verisilicon Hantro driver enabled.
> - Not runtime-tested: the available RK3588 board does not expose a Hantro
> G2 VP9 decoder.
>
> drivers/media/platform/verisilicon/hantro_drv.c | 13 +++++++++----
> .../media/platform/verisilicon/hantro_g2_vp9_dec.c | 14 +++++++++-----
> drivers/media/platform/verisilicon/hantro_hw.h | 1 +
> 3 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 2e81877f640f..04167e4be003 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -147,10 +147,17 @@ void hantro_start_prepare_run(struct hantro_ctx *ctx)
> }
> }
>
> -void hantro_end_prepare_run(struct hantro_ctx *ctx)
> +void hantro_complete_ctrl_request(struct hantro_ctx *ctx)
> {
> struct vb2_v4l2_buffer *src_buf;
>
> + src_buf = hantro_get_src_buf(ctx);
> + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> + &ctx->ctrl_handler);
> +}
> +
> +void hantro_end_prepare_run(struct hantro_ctx *ctx)
> +{
> if (!ctx->is_encoder && ctx->dev->variant->late_postproc) {
> if (hantro_needs_postproc(ctx, ctx->vpu_dst_fmt))
> hantro_postproc_enable(ctx);
> @@ -158,9 +165,7 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
> hantro_postproc_disable(ctx);
> }
>
> - src_buf = hantro_get_src_buf(ctx);
> - v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> - &ctx->ctrl_handler);
> + hantro_complete_ctrl_request(ctx);
>
> /* Kick the watchdog. */
> schedule_delayed_work(&ctx->dev->watchdog_work,
> diff --git a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
> index 56c79e339030..e08db6e3f6f9 100644
> --- a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
> +++ b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
> @@ -36,12 +36,14 @@ static int start_prepare_run(struct hantro_ctx *ctx, const struct v4l2_ctrl_vp9_
>
> ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, V4L2_CID_STATELESS_VP9_FRAME);
> if (WARN_ON(!ctrl))
> - return -EINVAL;
> + goto err_complete_request;
> +
> *dec_params = ctrl->p_cur.p;
>
> ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, V4L2_CID_STATELESS_VP9_COMPRESSED_HDR);
> if (WARN_ON(!ctrl))
> - return -EINVAL;
> + goto err_complete_request;
> +
> prob_updates = ctrl->p_cur.p;
> vp9_ctx->cur.tx_mode = prob_updates->tx_mode;
>
> @@ -86,6 +88,10 @@ static int start_prepare_run(struct hantro_ctx *ctx, const struct v4l2_ctrl_vp9_
> v4l2_vp9_fw_update_probs(&vp9_ctx->probability_tables, prob_updates, *dec_params);
>
> return 0;
> +
> +err_complete_request:
> + hantro_complete_ctrl_request(ctx);
> + return -EINVAL;
> }
>
> static struct hantro_decoded_buffer *
> @@ -894,10 +900,8 @@ int hantro_g2_vp9_dec_run(struct hantro_ctx *ctx)
> int ret;
>
> ret = start_prepare_run(ctx, &decode_params);
> - if (ret) {
> - hantro_end_prepare_run(ctx);
> + if (ret)
> return ret;
> - }
>
> src = hantro_get_src_buf(ctx);
> dst = hantro_get_dst_buf(ctx);
> diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
> index 13e573f1f19d..9d4e858f17c7 100644
> --- a/drivers/media/platform/verisilicon/hantro_hw.h
> +++ b/drivers/media/platform/verisilicon/hantro_hw.h
> @@ -430,6 +430,7 @@ void hantro_watchdog(struct work_struct *work);
> void hantro_irq_done(struct hantro_dev *vpu,
> enum vb2_buffer_state result);
> void hantro_start_prepare_run(struct hantro_ctx *ctx);
> +void hantro_complete_ctrl_request(struct hantro_ctx *ctx);
> void hantro_end_prepare_run(struct hantro_ctx *ctx);
>
> irqreturn_t hantro_g1_irq(int irq, void *dev_id);
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
More information about the Linux-rockchip
mailing list