media: verisilicon: Fix CPU stalls on G2 bus error

commit 19c286b755072a22a063052f530a6b1fac8a1f63 upstream.

In some seek stress tests, we are getting IRQ from the G2 decoder where
the dec_bus_int and the dec_e bits are high, meaning the decoder is
still running despite the error.

Fix this by reworking the IRQ handler to only finish the job once we
have reached completion and move the software reset to when our software
watchdog triggers.

This way, we let the hardware continue on errors when it did not self
reset and in worse case scenario the hardware timeout will
automatically stop it. The actual error will be fixed in a follow up
patch.

Fixes: 3385c514ecc5a ("media: hantro: Convert imx8m_vpu_g2_irq to helper")
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/media/platform/verisilicon/hantro_g2.c b/drivers/media/platform/verisilicon/hantro_g2.c
index 5c1d799d861840993166270ece8b24a6fcc136ca..022d16294ae01f83c8d4172789d7d595b7cbf12f 100644 (file)
--- a/drivers/media/platform/verisilicon/hantro_g2.c
+++ b/drivers/media/platform/verisilicon/hantro_g2.c
@@ -5,43 +5,93 @@
  * Copyright (C) 2021 Collabora Ltd, Andrzej Pietrasiewicz <andrzej.p@collabora.com>
  */
 
+#include <linux/delay.h>
 #include "hantro_hw.h"
 #include "hantro_g2_regs.h"
 
 #define G2_ALIGN       16
 
-void hantro_g2_check_idle(struct hantro_dev *vpu)
+static bool hantro_g2_active(struct hantro_ctx *ctx)
 {
-       int i;
-
-       for (i = 0; i < 3; i++) {
-               u32 status;
-
-               /* Make sure the VPU is idle */
-               status = vdpu_read(vpu, G2_REG_INTERRUPT);
-               if (status & G2_REG_INTERRUPT_DEC_E) {
-                       dev_warn(vpu->dev, "device still running, aborting");
-                       status |= G2_REG_INTERRUPT_DEC_ABORT_E | G2_REG_INTERRUPT_DEC_IRQ_DIS;
-                       vdpu_write(vpu, status, G2_REG_INTERRUPT);
-               }
+       struct hantro_dev *vpu = ctx->dev;
+       u32 status;
+
+       status = vdpu_read(vpu, G2_REG_INTERRUPT);
+
+       return (status & G2_REG_INTERRUPT_DEC_E);
+}
+
+/**
+ * hantro_g2_reset:
+ * @ctx: the hantro context
+ *
+ * Emulates a reset using Hantro abort function. Failing this procedure would
+ * results in programming a running IP which leads to CPU hang.
+ *
+ * Using a hard reset procedure instead is prefferred.
+ */
+void hantro_g2_reset(struct hantro_ctx *ctx)
+{
+       struct hantro_dev *vpu = ctx->dev;
+       u32 status;
+
+       status = vdpu_read(vpu, G2_REG_INTERRUPT);
+       if (status & G2_REG_INTERRUPT_DEC_E) {
+               dev_warn_ratelimited(vpu->dev, "device still running, aborting");
+               status |= G2_REG_INTERRUPT_DEC_ABORT_E | G2_REG_INTERRUPT_DEC_IRQ_DIS;
+               vdpu_write(vpu, status, G2_REG_INTERRUPT);
+
+               do {
+                       mdelay(1);
+               } while (hantro_g2_active(ctx));
        }
 }
 
 irqreturn_t hantro_g2_irq(int irq, void *dev_id)
 {
        struct hantro_dev *vpu = dev_id;
-       enum vb2_buffer_state state;
        u32 status;
 
        status = vdpu_read(vpu, G2_REG_INTERRUPT);
-       state = (status & G2_REG_INTERRUPT_DEC_RDY_INT) ?
-                VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
 
-       vdpu_write(vpu, 0, G2_REG_INTERRUPT);
-       vdpu_write(vpu, G2_REG_CONFIG_DEC_CLK_GATE_E, G2_REG_CONFIG);
+       if (!(status & G2_REG_INTERRUPT_DEC_IRQ))
+               return IRQ_NONE;
+
+       hantro_reg_write(vpu, &g2_dec_irq, 0);
+       hantro_reg_write(vpu, &g2_dec_int_stat, 0);
+       hantro_reg_write(vpu, &g2_clk_gate_e, 1);
+
+       if (status & G2_REG_INTERRUPT_DEC_RDY_INT) {
+               hantro_irq_done(vpu, VB2_BUF_STATE_DONE);
+               return IRQ_HANDLED;
+       }
+
+       if (status & G2_REG_INTERRUPT_DEC_ABORT_INT) {
+               /* disabled on abort, though lets be safe and handle it */
+               dev_warn_ratelimited(vpu->dev, "decode operation aborted.");
+               return IRQ_HANDLED;
+       }
+
+       if (status & G2_REG_INTERRUPT_DEC_LAST_SLICE_INT)
+               dev_warn_ratelimited(vpu->dev, "not all macroblocks were decoded.");
+
+       if (status & G2_REG_INTERRUPT_DEC_BUS_INT)
+               dev_warn_ratelimited(vpu->dev, "bus error detected.");
+
+       if (status & G2_REG_INTERRUPT_DEC_ERROR_INT)
+               dev_warn_ratelimited(vpu->dev, "decode error detected.");
+
+       if (status & G2_REG_INTERRUPT_DEC_TIMEOUT)
+               dev_warn_ratelimited(vpu->dev, "frame decode timed out.");
 
-       hantro_irq_done(vpu, state);
+       /**
+        * If the decoding haven't stopped, let it continue. The hardware timeout
+        * will trigger if it is trully stuck.
+        */
+       if (status & G2_REG_INTERRUPT_DEC_E)
+               return IRQ_HANDLED;
 
+       hantro_irq_done(vpu, VB2_BUF_STATE_ERROR);
        return IRQ_HANDLED;
 }
 

diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
index 0e212198dd65b1cc27770ca93b14aa96e2772ac4..f066636e56f98560d9b1c5036691e3c34dd13b1f 100644 (file)
--- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
@@ -582,8 +582,6 @@ int hantro_g2_hevc_dec_run(struct hantro_ctx *ctx)
        struct hantro_dev *vpu = ctx->dev;
        int ret;
 
-       hantro_g2_check_idle(vpu);
-
        /* Prepare HEVC decoder context. */
        ret = hantro_hevc_dec_prepare_run(ctx);
        if (ret)

diff --git a/drivers/media/platform/verisilicon/hantro_g2_regs.h b/drivers/media/platform/verisilicon/hantro_g2_regs.h
index b943b1816db7fd11eee063ecd7b082a5ed6c6f90..c614951121c79cf5c3bd4d086e596f60fba12459 100644 (file)
--- a/drivers/media/platform/verisilicon/hantro_g2_regs.h
+++ b/drivers/media/platform/verisilicon/hantro_g2_regs.h
@@ -22,7 +22,14 @@
 #define G2_REG_VERSION                 G2_SWREG(0)
 
 #define G2_REG_INTERRUPT               G2_SWREG(1)
+#define G2_REG_INTERRUPT_DEC_LAST_SLICE_INT    BIT(19)
+#define G2_REG_INTERRUPT_DEC_TIMEOUT   BIT(18)
+#define G2_REG_INTERRUPT_DEC_ERROR_INT BIT(16)
+#define G2_REG_INTERRUPT_DEC_BUF_INT   BIT(14)
+#define G2_REG_INTERRUPT_DEC_BUS_INT   BIT(13)
 #define G2_REG_INTERRUPT_DEC_RDY_INT   BIT(12)
+#define G2_REG_INTERRUPT_DEC_ABORT_INT BIT(11)
+#define G2_REG_INTERRUPT_DEC_IRQ       BIT(8)
 #define G2_REG_INTERRUPT_DEC_ABORT_E   BIT(5)
 #define G2_REG_INTERRUPT_DEC_IRQ_DIS   BIT(4)
 #define G2_REG_INTERRUPT_DEC_E         BIT(0)
@@ -35,6 +42,9 @@
 #define BUS_WIDTH_128                  2
 #define BUS_WIDTH_256                  3
 
+#define g2_dec_int_stat                G2_DEC_REG(1, 11, 0xf)
+#define g2_dec_irq             G2_DEC_REG(1, 8, 0x1)
+
 #define g2_strm_swap           G2_DEC_REG(2, 28, 0xf)
 #define g2_strm_swap_old       G2_DEC_REG(2, 27, 0x1f)
 #define g2_pic_swap            G2_DEC_REG(2, 22, 0x1f)
@@ -225,6 +235,9 @@
 #define vp9_filt_level_seg5    G2_DEC_REG(19,  8, 0x3f)
 #define vp9_quant_seg5         G2_DEC_REG(19,  0, 0xff)
 
+#define g2_timemout_override_e G2_DEC_REG(45, 31, 0x1)
+#define g2_timemout_cycles     G2_DEC_REG(45, 0, 0x7fffffff)
+
 #define hevc_cur_poc_00                G2_DEC_REG(46, 24, 0xff)
 #define hevc_cur_poc_01                G2_DEC_REG(46, 16, 0xff)
 #define hevc_cur_poc_02                G2_DEC_REG(46, 8,  0xff)

diff --git a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
index 342e543dee4c4cd4d685f7aafa56b83f653fa446..70e7cedc33b81043466bc5b85f4f71d65490edb2 100644 (file)
--- a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
@@ -893,8 +893,6 @@ int hantro_g2_vp9_dec_run(struct hantro_ctx *ctx)
        struct vb2_v4l2_buffer *dst;
        int ret;
 
-       hantro_g2_check_idle(ctx->dev);
-
        ret = start_prepare_run(ctx, &decode_params);
        if (ret) {
                hantro_end_prepare_run(ctx);

diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index c9b6556f8b2b78b94e11c6f1374b395ca02f16ff..5f2011529f02dc6f8260f10a579bf1cd4e6c2edf 100644 (file)
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -583,6 +583,7 @@ void hantro_g2_vp9_dec_done(struct hantro_ctx *ctx);
 int hantro_vp9_dec_init(struct hantro_ctx *ctx);
 void hantro_vp9_dec_exit(struct hantro_ctx *ctx);
 void hantro_g2_check_idle(struct hantro_dev *vpu);
+void hantro_g2_reset(struct hantro_ctx *ctx);
 irqreturn_t hantro_g2_irq(int irq, void *dev_id);
 
 #endif /* HANTRO_HW_H_ */

diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
index f850d8bddef6bc5426e96aa49803f7de37f83908..74fd985a8aad188c442d67e2126e6cb7a5ac3a0e 100644 (file)
--- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
@@ -312,11 +312,13 @@ static const struct hantro_codec_ops imx8mq_vpu_g1_codec_ops[] = {
 static const struct hantro_codec_ops imx8mq_vpu_g2_codec_ops[] = {
        [HANTRO_MODE_HEVC_DEC] = {
                .run = hantro_g2_hevc_dec_run,
+               .reset = hantro_g2_reset,
                .init = hantro_hevc_dec_init,
                .exit = hantro_hevc_dec_exit,
        },
        [HANTRO_MODE_VP9_DEC] = {
                .run = hantro_g2_vp9_dec_run,
+               .reset = hantro_g2_reset,
                .done = hantro_g2_vp9_dec_done,
                .init = hantro_vp9_dec_init,
                .exit = hantro_vp9_dec_exit,