Re: [PATCH v2 3/4] media: rkvdec: Re-enable H.264 error detection

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

Le lundi 26 décembre 2022 à 12:17 +0800, Chen-Yu Tsai a écrit :
> On Sat, Dec 24, 2022 at 3:39 AM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> 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@collabora.com>
> > ---
> >  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);
> 
> Use RKVDEC_H264_ERR_EN_HIGHBITS() here? Only lower 30 bits are valid.

I was simply reverting back to the value it was before, I can try that too, you
are likely right.

> 
> >         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;
> 
> u32?

Will do.

> 
> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> 
> > +
> > +       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));
> > +               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
> >