Hi Nicolas,
I'm still unsure about this patchset.
It sounds like a good approach and a nice
improvement, but I want to make sure I think through it.
Meanwhile, a small comment...
On Fri, Dec 23, 2022 at 02:38:05PM -0500, Nicolas Dufresne wrote:
> This re-enable 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 state, and allow reporting that some corruption
> may be present in the buffer 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 4fc167b42cf0c..dfe3e235f099a 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -1162,14 +1162,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;
> @@ -1183,10 +1184,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",
... still uses pr_debug. I would change it so it uses v4l2_dbg,
and can be controlled using the same debug parameter
as you use in patch 4/4.
> + RKVDEC_ERR_PKT_NUM(err), RKVDEC_SLICEDEC_NUM(err));
> + 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.38.1
>
