Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: verisilicon: vp9: do not arm watchdog on preparation failure
@ 2026-07-23 15:36 Tharit Tangkijwanichakul
  2026-07-23 16:05 ` Benjamin Gaignard
  0 siblings, 1 reply; 2+ messages in thread
From: Tharit Tangkijwanichakul @ 2026-07-23 15:36 UTC (permalink / raw)
  To: Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel,
	Mauro Carvalho Chehab, linux-media, linux-rockchip, linux-kernel
  Cc: linux-kernel-mentees, skhan, me, jkoolstra,
	Tharit Tangkijwanichakul

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.

Signed-off-by: Tharit Tangkijwanichakul <tharitt97@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
-- 
2.47.3


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] media: verisilicon: vp9: do not arm watchdog on preparation failure
  2026-07-23 15:36 [PATCH] media: verisilicon: vp9: do not arm watchdog on preparation failure Tharit Tangkijwanichakul
@ 2026-07-23 16:05 ` Benjamin Gaignard
  0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Gaignard @ 2026-07-23 16:05 UTC (permalink / raw)
  To: Tharit Tangkijwanichakul, Nicolas Dufresne, Philipp Zabel,
	Mauro Carvalho Chehab, linux-media, linux-rockchip, linux-kernel
  Cc: linux-kernel-mentees, skhan, me, jkoolstra


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@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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-23 16:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 15:36 [PATCH] media: verisilicon: vp9: do not arm watchdog on preparation failure Tharit Tangkijwanichakul
2026-07-23 16:05 ` Benjamin Gaignard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox