Hi Nicolas,
Thanks for the patch.
On Fri, Dec 23, 2022 at 02:38:03PM -0500, Nicolas Dufresne wrote:
> This optional internal ops allow each codec to do their own
> error status checking. The presence of an error is reported
> using the ERROR buffer state. This patch have no functional
> changes.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> ---
> drivers/staging/media/rkvdec/rkvdec.c | 10 ++++++----
> drivers/staging/media/rkvdec/rkvdec.h | 2 ++
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index 7bab7586918c1..7e76f8b728854 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -950,6 +950,7 @@ static void rkvdec_v4l2_cleanup(struct rkvdec_dev *rkvdec)
> static irqreturn_t rkvdec_irq_handler(int irq, void *priv)
> {
> struct rkvdec_dev *rkvdec = priv;
> + struct rkvdec_ctx *ctx;
> enum vb2_buffer_state state;
> u32 status;
>
> @@ -958,12 +959,13 @@ static irqreturn_t rkvdec_irq_handler(int irq, void *priv)
> VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>
> writel(0, rkvdec->regs + RKVDEC_REG_INTERRUPT);
> - if (cancel_delayed_work(&rkvdec->watchdog_work)) {
> - struct rkvdec_ctx *ctx;
> + ctx = v4l2_m2m_get_curr_priv(rkvdec->m2m_dev);
>
> - ctx = v4l2_m2m_get_curr_priv(rkvdec->m2m_dev);
> + if (ctx->coded_fmt_desc->ops->check_error_info)
> + state = ctx->coded_fmt_desc->ops->check_error_info(ctx);
> +
> + if (cancel_delayed_work(&rkvdec->watchdog_work))
> rkvdec_job_finish(ctx, state);
> - }
>
> return IRQ_HANDLED;
> }
> diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
> index 633335ebb9c49..4ae8e6c6b03c9 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.h
> +++ b/drivers/staging/media/rkvdec/rkvdec.h
> @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops {
> struct vb2_v4l2_buffer *dst_buf,
> enum vb2_buffer_state result);
> int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl);
> + /* called from IRQ handler */
> + int (*check_error_info)(struct rkvdec_ctx *ctx);
I don't think it's a good idea to mix the return of this internal API
with enum vb2_buffer_state.
Please make the return type of this function a boolean or an integer
type that is decoupled from the VB2 buffer state.
Thanks!
Ezequiel
> };
>
> struct rkvdec_coded_fmt_desc {
> --
> 2.38.1
>
