The CSI must be disabled immediately after receiving the last EOF before
stream off (and thus before disabling the IDMA channel). This can be
accomplished by moving upstream stream off to just after receiving the
last EOF completion in prp_stop(). For symmetry also move upstream
stream on to end of prp_start().
This fixes a complete system hard lockup on the SabreAuto when streaming
from the ADV7180, by repeatedly sending a stream off immediately followed
by stream on:
while true; do v4l2-ctl -d1 --stream-mmap --stream-count=3; done
Eventually this either causes the system lockup or EOF timeouts at all
subsequent stream on, until a system reset.
The lockup occurs when disabling the IDMA channel at stream off. Disabling
the CSI before disabling the IDMA channel appears to be a reliable fix for
the hard lockup.
Fixes: f0d9c8924e2c3 ("[media] media: imx: Add IC subdev drivers")
Reported-by: Gaël PORTAY <[email protected]>
Signed-off-by: Steve Longerbeam <[email protected]>
Cc: [email protected]
---
Changes in v2:
- Add Fixes: and Cc: stable
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 26 ++++++++++++++-------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 33ada6612fee..f53cdb608528 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -707,12 +707,23 @@ static int prp_start(struct prp_priv *priv)
goto out_free_nfb4eof_irq;
}
+ /* start upstream */
+ ret = v4l2_subdev_call(priv->src_sd, video, s_stream, 1);
+ ret = (ret && ret != -ENOIOCTLCMD) ? ret : 0;
+ if (ret) {
+ v4l2_err(&ic_priv->sd,
+ "upstream stream on failed: %d\n", ret);
+ goto out_free_eof_irq;
+ }
+
/* start the EOF timeout timer */
mod_timer(&priv->eof_timeout_timer,
jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
return 0;
+out_free_eof_irq:
+ devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
out_free_nfb4eof_irq:
devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
out_unsetup:
@@ -744,6 +755,12 @@ static void prp_stop(struct prp_priv *priv)
if (ret == 0)
v4l2_warn(&ic_priv->sd, "wait last EOF timeout\n");
+ /* stop upstream */
+ ret = v4l2_subdev_call(priv->src_sd, video, s_stream, 0);
+ if (ret && ret != -ENOIOCTLCMD)
+ v4l2_warn(&ic_priv->sd,
+ "upstream stream off failed: %d\n", ret);
+
devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
@@ -1174,15 +1191,6 @@ static int prp_s_stream(struct v4l2_subdev *sd, int enable)
if (ret)
goto out;
- /* start/stop upstream */
- ret = v4l2_subdev_call(priv->src_sd, video, s_stream, enable);
- ret = (ret && ret != -ENOIOCTLCMD) ? ret : 0;
- if (ret) {
- if (enable)
- prp_stop(priv);
- goto out;
- }
-
update_count:
priv->stream_count += enable ? 1 : -1;
if (priv->stream_count < 0)
--
2.17.1
Steve, all,
On 1/17/19 3:49 PM, Steve Longerbeam wrote:
> The CSI must be disabled immediately after receiving the last EOF before
> stream off (and thus before disabling the IDMA channel). This can be
> accomplished by moving upstream stream off to just after receiving the
> last EOF completion in prp_stop(). For symmetry also move upstream
> stream on to end of prp_start().
>
> This fixes a complete system hard lockup on the SabreAuto when streaming
> from the ADV7180, by repeatedly sending a stream off immediately followed
> by stream on:
>
> while true; do v4l2-ctl -d1 --stream-mmap --stream-count=3; done
>
> Eventually this either causes the system lockup or EOF timeouts at all
> subsequent stream on, until a system reset.
>
> The lockup occurs when disabling the IDMA channel at stream off. Disabling
> the CSI before disabling the IDMA channel appears to be a reliable fix for
> the hard lockup.
>
> Fixes: f0d9c8924e2c3 ("[media] media: imx: Add IC subdev drivers")
>
> Reported-by: Gaël PORTAY <[email protected]>
> Signed-off-by: Steve Longerbeam <[email protected]>
> Cc: [email protected]
> ---
> Changes in v2:
> - Add Fixes: and Cc: stable
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 26 ++++++++++++++-------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 33ada6612fee..f53cdb608528 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -707,12 +707,23 @@ static int prp_start(struct prp_priv *priv)
> goto out_free_nfb4eof_irq;
> }
>
> + /* start upstream */
> + ret = v4l2_subdev_call(priv->src_sd, video, s_stream, 1);
> + ret = (ret && ret != -ENOIOCTLCMD) ? ret : 0;
> + if (ret) {
> + v4l2_err(&ic_priv->sd,
> + "upstream stream on failed: %d\n", ret);
> + goto out_free_eof_irq;
> + }
> +
> /* start the EOF timeout timer */
> mod_timer(&priv->eof_timeout_timer,
> jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
>
> return 0;
>
> +out_free_eof_irq:
> + devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
> out_free_nfb4eof_irq:
> devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
> out_unsetup:
> @@ -744,6 +755,12 @@ static void prp_stop(struct prp_priv *priv)
> if (ret == 0)
> v4l2_warn(&ic_priv->sd, "wait last EOF timeout\n");
>
> + /* stop upstream */
> + ret = v4l2_subdev_call(priv->src_sd, video, s_stream, 0);
> + if (ret && ret != -ENOIOCTLCMD)
> + v4l2_warn(&ic_priv->sd,
> + "upstream stream off failed: %d\n", ret);
> +
> devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
> devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
>
> @@ -1174,15 +1191,6 @@ static int prp_s_stream(struct v4l2_subdev *sd, int enable)
> if (ret)
> goto out;
>
> - /* start/stop upstream */
> - ret = v4l2_subdev_call(priv->src_sd, video, s_stream, enable);
> - ret = (ret && ret != -ENOIOCTLCMD) ? ret : 0;
> - if (ret) {
> - if (enable)
> - prp_stop(priv);
> - goto out;
> - }
> -
> update_count:
> priv->stream_count += enable ? 1 : -1;
> if (priv->stream_count < 0)
>
Tested-by: Gaël PORTAY <[email protected]>
Thanks,
Gael