2022-06-10 13:13:40

by Nicolas Dufresne

[permalink] [raw]
Subject: [PATCH v1 1/5] media: rkvdec: Disable H.264 error detection

Quite often, the HW get stuck in error condition if a stream error
was detected. As documented, the HW should stop immediately and self
reset. There is likely a problem or a miss-understanding of the self
self reset mechanism, as unless we make a long pause, the next command
will then report an error even if there is no error in it.

Disabling error detection fixes the issue, and let the decoder continue
after an error. This patch is safe for backport into older kernels.

Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
Signed-off-by: Nicolas Dufresne <[email protected]>
---
drivers/staging/media/rkvdec/rkvdec-h264.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index 2992fb87cf72..55596ce6bb6e 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -1175,8 +1175,8 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)

schedule_delayed_work(&rkvdec->watchdog_work, msecs_to_jiffies(2000));

- writel(0xffffffff, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
- writel(0xffffffff, rkvdec->regs + RKVDEC_REG_H264_ERR_E);
+ writel(0, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
+ writel(0, 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);

--
2.36.1


2022-06-10 13:50:32

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/5] media: rkvdec: Disable H.264 error detection

On 6/10/22 15:52, Nicolas Dufresne wrote:
> Quite often, the HW get stuck in error condition if a stream error
> was detected. As documented, the HW should stop immediately and self
> reset. There is likely a problem or a miss-understanding of the self
> self reset mechanism, as unless we make a long pause, the next command
> will then report an error even if there is no error in it.
>
> Disabling error detection fixes the issue, and let the decoder continue
> after an error. This patch is safe for backport into older kernels.
>
> Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> Signed-off-by: Nicolas Dufresne <[email protected]>
> ---

Nit: won't hurt to add the explicit stable tag if you'll make the v2.

Cc: [email protected]

--
Best regards,
Dmitry

2022-06-10 17:22:42

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v1 1/5] media: rkvdec: Disable H.264 error detection

On Fri, Jun 10, 2022 at 08:52:11AM -0400, Nicolas Dufresne wrote:
> Quite often, the HW get stuck in error condition if a stream error
> was detected. As documented, the HW should stop immediately and self
> reset. There is likely a problem or a miss-understanding of the self
> self reset mechanism, as unless we make a long pause, the next command
> will then report an error even if there is no error in it.
>
> Disabling error detection fixes the issue, and let the decoder continue
> after an error. This patch is safe for backport into older kernels.
>
> Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> Signed-off-by: Nicolas Dufresne <[email protected]>

This is effectively how ChromeOS previously was using this hardware for
years. When moving to the upstream/staging driver, this started giving
us problems. This fix is helpful; we'd rather sacrifice error detection
for now, to avoid hanging the hardware in error cases ;)

Reviewed-by: Brian Norris <[email protected]>
Tested-by: Brian Norris <[email protected]>

2022-06-27 17:54:10

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH v1 1/5] media: rkvdec: Disable H.264 error detection

Hi Hans,

On Fri, Jun 10, 2022 at 09:39:19AM -0700, Brian Norris wrote:
> On Fri, Jun 10, 2022 at 08:52:11AM -0400, Nicolas Dufresne wrote:
> > Quite often, the HW get stuck in error condition if a stream error
> > was detected. As documented, the HW should stop immediately and self
> > reset. There is likely a problem or a miss-understanding of the self
> > self reset mechanism, as unless we make a long pause, the next command
> > will then report an error even if there is no error in it.
> >
> > Disabling error detection fixes the issue, and let the decoder continue
> > after an error. This patch is safe for backport into older kernels.
> >
> > Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> > Signed-off-by: Nicolas Dufresne <[email protected]>
>
> This is effectively how ChromeOS previously was using this hardware for
> years. When moving to the upstream/staging driver, this started giving
> us problems. This fix is helpful; we'd rather sacrifice error detection
> for now, to avoid hanging the hardware in error cases ;)
>
> Reviewed-by: Brian Norris <[email protected]>
> Tested-by: Brian Norris <[email protected]>

Reviewed-by: Ezequiel Garcia <[email protected]>

Given this is stable material, looks like we should queue it,
while the rest of the series is still being discussed.

Thanks,
Ezequiel