Hi Nicolas,
Great stuff! See below for some ideas how to expose errors.
On Fri, Jun 10, 2022 at 9:52 AM Nicolas Dufresne
<nicolas.dufresne@xxxxxxxxxxxxx> wrote:
>
> This re-enables H.264 error detection, but using the other error mode.
> In that mode, the decoder will skip over the error macro-block or
> slices and complete the decoding. As a side effect, the error status
> is not set in the interrupt status register, and instead errors are
> detected per format. Using this mode workaround the issue that the
> HW get stuck in error stated and allow reporting that some corruption
> may be present in the buffer returned to userland.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> ---
> drivers/staging/media/rkvdec/rkvdec-h264.c | 23 +++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> index 55596ce6bb6e..60a89918e2c1 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -1175,14 +1175,15 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)
>
> schedule_delayed_work(&rkvdec->watchdog_work, msecs_to_jiffies(2000));
>
> - writel(0, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
> - writel(0, rkvdec->regs + RKVDEC_REG_H264_ERR_E);
> + writel(0xffffffff, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
> + writel(0xffffffff, rkvdec->regs + RKVDEC_REG_H264_ERR_E);
> writel(1, rkvdec->regs + RKVDEC_REG_PREF_LUMA_CACHE_COMMAND);
> writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>
> /* Start decoding! */
> writel(RKVDEC_INTERRUPT_DEC_E | RKVDEC_CONFIG_DEC_CLK_GATE_E |
> - RKVDEC_TIMEOUT_E | RKVDEC_BUF_EMPTY_E,
> + RKVDEC_TIMEOUT_E | RKVDEC_BUF_EMPTY_E |
> + RKVDEC_H264ORVP9_ERR_MODE,
> rkvdec->regs + RKVDEC_REG_INTERRUPT);
>
> return 0;
> @@ -1196,10 +1197,26 @@ static int rkvdec_h264_try_ctrl(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl)
> return 0;
> }
>
> +static int rkvdec_h264_check_error_info(struct rkvdec_ctx *ctx)
> +{
> + struct rkvdec_dev *rkvdec = ctx->dev;
> + int err;
> +
> + err = readl(rkvdec->regs + RKVDEC_REG_H264_ERRINFO_NUM);
> + if (err & RKVDEC_STRMD_DECT_ERR_FLAG) {
> + pr_debug("Decoded picture have %i/%i slices with errors.\n",
> + RKVDEC_ERR_PKT_NUM(err), RKVDEC_SLICEDEC_NUM(err));
It's more useful friendly to just keep a counter somewhere. In the past,
we've created a user control, which has the advantage of leveraging
an existing mechanism, and already being per-fd.
See:
commit b2d3bef1aa7858b2ae5e0d01adb214121ba00b9f
"media: coda: Add a V4L2 user for control error macroblocks count".
I would drop the pr_debug, or if you think it's really useful for users
and developers, go with v4l2_dbg. In which case, how do you ensure
a corrupted stream won't flood the logs?
Thanks,
Ezequiel
> + return VB2_BUF_STATE_ERROR;
> + }
> +
> + return VB2_BUF_STATE_DONE;
> +}
> +
> const struct rkvdec_coded_fmt_ops rkvdec_h264_fmt_ops = {
> .adjust_fmt = rkvdec_h264_adjust_fmt,
> .start = rkvdec_h264_start,
> .stop = rkvdec_h264_stop,
> .run = rkvdec_h264_run,
> .try_ctrl = rkvdec_h264_try_ctrl,
> + .check_error_info = rkvdec_h264_check_error_info,
> };
> --
> 2.36.1
>
