media: imx: Disable CSI immediately after last EOF
Subsystems (5)
Steve Longerbeam
maintainerv2 cover media: imx: Disable CSI immediately after last EOF
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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.
History:
v2:
- Whitespace fixes
- Add Fixes: and Cc: [email protected]
- No functional changes.
Steve Longerbeam (2):
media: imx: csi: Disable CSI immediately after last EOF
media: imx: prpencvf: Disable CSI immediately after last EOF
drivers/staging/media/imx/imx-ic-prpencvf.c | 26 ++++++++++++++------- drivers/staging/media/imx/imx-media-csi.c | 6 +++-- 2 files changed, 21 insertions(+), 11 deletions(-)
Signature · 1 line
2.17.1
Steve Longerbeam
maintainerPATCH v2 1/2 media: imx: csi: Disable CSI immediately after last EOF
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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.
---
Changes in v2:
- restore an empty line
- Add Fixes: and Cc: stable
--- drivers/staging/media/imx/imx-media-csi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/media/imx/imx-media-csi.c +4 -2
@@ -681,6 +681,8 @@ static void csi_idmac_stop(struct csi_priv *priv)if (ret ==0 )v4l2_warn (&priv->sd,"wait last EOF timeout\n" ); +ipu_csi_disable (priv->csi); +devm_free_irq (priv->dev, priv->eof_irq, priv);devm_free_irq (priv->dev, priv->nfb4eof_irq, priv);
@@ -793,9 +795,9 @@ static void csi_stop(struct csi_priv *priv)/* stop the frame interval monitor */ if (priv->fim)imx_media_fim_set_stream (priv->fim,NULL , false); + }else { +ipu_csi_disable (priv->csi); } - -ipu_csi_disable (priv->csi); }static const struct csi_skip_desc csi_skip[12 ] = {
Signature · 1 line
2.17.1
Re: [PATCH v2 1/2] media: imx: csi: Disable CSI immediately after last EOF
Quoted 34 lines · Steve Longerbeam
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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 4a34ec8e470cb ("[media] media: imx: Add CSI subdev driver")Reported-by Gaël PORTAY <[email protected]>Signed-off-by Steve Longerbeam <[email protected]>---
Changes in v2:
- restore an empty line
- Add Fixes: and Cc: stable--- drivers/staging/media/imx/imx-media-csi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/staging/media/imx/imx-media-csi.c +0
-0
drivers/staging/media/imx/imx-media-csi.c static void csi_idmac_stop(struct csi_priv *priv) · line 685
Quoted 5 lines
drivers/staging/media/imx/imx-media-csi.c +2
-0@@ -681,6 +681,8 @@ static void csi_idmac_stop(struct csi_priv *priv)if (ret ==0 )v4l2_warn (&priv->sd,"wait last EOF timeout\n" ); +ipu_csi_disable (priv->csi); +
Can you add a short comment why this call is here? Since now csi_idmac_stop is kind of a misnomer and symmetry with csi(_idmac)_start is broken, I think this is a bit un-obvious.
Also note that now the error path of csi_start() will now call ipu_csi_disable() while the CSI is disabled. This happens to work because that just calls ipu_module_disable(), which is not refcounted.
drivers/staging/media/imx/imx-media-csi.c static void csi_stop(struct csi_priv *priv) · line 798
Quoted 12 lines
drivers/staging/media/imx/imx-media-csi.c +2
-2devm_free_irq (priv->dev, priv->eof_irq, priv);devm_free_irq (priv->dev, priv->nfb4eof_irq, priv);@@ -793,9 +795,9 @@ static void csi_stop(struct csi_priv *priv)/* stop the frame interval monitor */ if (priv->fim)imx_media_fim_set_stream (priv->fim,NULL , false); + }else { +ipu_csi_disable (priv->csi); } - -ipu_csi_disable (priv->csi);
regards
Philipp
Steve Longerbeam
maintainerRe: [PATCH v2 1/2] media: imx: csi: Disable CSI immediately after last EOF
Quoted 38 lines · Philipp Zabel
Quoted 37 lines · Steve Longerbeam
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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 4a34ec8e470cb ("[media] media: imx: Add CSI subdev driver")Reported-by Gaël PORTAY <[email protected]>Signed-off-by Steve Longerbeam <[email protected]>---
Changes in v2:
- restore an empty line
- Add Fixes: and Cc: stable--- drivers/staging/media/imx/imx-media-csi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/staging/media/imx/imx-media-csi.c +0
-0@@ -681,6 +681,8 @@ static void csi_idmac_stop(struct csi_priv *priv)if (ret ==0 )v4l2_warn (&priv->sd,"wait last EOF timeout\n" );
Quoted 5 lines
drivers/staging/media/imx/imx-media-csi.c static void csi_idmac_stop(struct csi_priv *priv) · line 685
Quoted 2 lines
drivers/staging/media/imx/imx-media-csi.c +2
-0+ipu_csi_disable (priv->csi); +Can you add a short comment why this call is here? Since now
csi_idmac_stopis kind of a misnomer and symmetry with csi(_idmac)_start is broken, I think this is a bit un-obvious.
Yeah. I think a cleaner, more symmetric solution would be to split up csi_idmac_stop.
Quoted 4 lines
Also note that now the error path of
csi_start()will now callipu_csi_disable()while the CSI is disabled. This happens to work because that just callsipu_module_disable(), which is not refcounted.
Thanks for catching. Splitting up csi_idmac_stop will fix this. Working on that.
Steve
Quoted 17 lines
drivers/staging/media/imx/imx-media-csi.c static void csi_stop(struct csi_priv *priv) · line 798
Quoted 13 lines
drivers/staging/media/imx/imx-media-csi.c +2
-2devm_free_irq (priv->dev, priv->eof_irq, priv);devm_free_irq (priv->dev, priv->nfb4eof_irq, priv);@@ -793,9 +795,9 @@ static void csi_stop(struct csi_priv *priv)/* stop the frame interval monitor */ if (priv->fim)imx_media_fim_set_stream (priv->fim,NULL , false); + }else { +ipu_csi_disable (priv->csi); } - -ipu_csi_disable (priv->csi);Reviewed-by Philipp Zabel <[email protected]>regards
Philipp
Steve Longerbeam
maintainerRe: [PATCH v2 1/2] media: imx: csi: Disable CSI immediately after last EOF
Quoted 46 lines · Steve Longerbeam
Quoted 30 lines · Philipp Zabel
Quoted 29 lines · Steve Longerbeam
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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 4a34ec8e470cb ("[media] media: imx: Add CSI subdev driver")Reported-by Gaël PORTAY <[email protected]>Signed-off-by Steve Longerbeam <[email protected]>---
Changes in v2:
- restore an empty line
- Add Fixes: and Cc: stable
---
drivers/staging/media/imx/imx-media-csi.c| 6 ++++--1 file changed, 4 insertions(+), 2 deletions(-)Quoted 12 lines
drivers/staging/media/imx/imx-media-csi.c static void csi_idmac_stop(struct csi_priv *priv) · line 684
Quoted 9 lines
drivers/staging/media/imx/imx-media-csi.c +1
-0@@ -681,6 +681,8 @@ static void csi_idmac_stop(struct csi_priv *priv) if (ret == 0) v4l2_warn(&priv->sd, "wait last EOF timeout\n"); + ipu_csi_disable(priv->csi); +Can you add a short comment why this call is here? Since now
csi_idmac_stopis kind of a misnomer and symmetry with csi(_idmac)_start is broken, I think this is a bit un-obvious.
Quoted 8 lines
Yeah. I think a cleaner, more symmetric solution would be to split up
csi_idmac_stop.Quoted 4 lines
Also note that now the error path of
csi_start()will now callipu_csi_disable()while the CSI is disabled. This happens to work because that just callsipu_module_disable(), which is not refcounted.Thanks for catching. Splitting up
csi_idmac_stopwill fix this. Working on that.
Well turns out Peter Seiderer kindly provided a solution they have been using for some time on custom imx6 hardware. His solution is to disable the SMFC before the IDMA channel, which I have tested on the SabreAuto and also works. This is a much simpler change, so I will post v3 that does this instead.
Steve
Re: [PATCH v2 1/2] media: imx: csi: Disable CSI immediately after last EOF
In reply to Steve LongerbeamSteeve, all,
Quoted 45 lines · Steve Longerbeam
Disable the CSI immediately after receiving the last EOF before stream off (and thus before disabling the IDMA channel).
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 -d4 --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 4a34ec8e470cb ("[media] media: imx: Add CSI subdev driver")Reported-by Gaël PORTAY <[email protected]>Signed-off-by Steve Longerbeam <[email protected]>---
Changes in v2:
- restore an empty line
- Add Fixes: and Cc: stable--- drivers/staging/media/imx/imx-media-csi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/staging/media/imx/imx-media-csi.c +2
-0@@ -681,6 +681,8 @@ static void csi_idmac_stop(struct csi_priv *priv)if (ret ==0 )v4l2_warn (&priv->sd,"wait last EOF timeout\n" ); +ipu_csi_disable (priv->csi); +devm_free_irq (priv->dev, priv->eof_irq, priv);devm_free_irq (priv->dev, priv->nfb4eof_irq, priv);@@ -793,9 +795,9 @@ static void csi_stop(struct csi_priv *priv)/* stop the frame interval monitor */
drivers/staging/media/imx/imx-media-csi.c static void csi_stop(struct csi_priv *priv) · line 803
Quoted 10 lines
drivers/staging/media/imx/imx-media-csi.c +2
-2if (priv->fim)imx_media_fim_set_stream (priv->fim,NULL , false); + }else { +ipu_csi_disable (priv->csi); } - -ipu_csi_disable (priv->csi); }static const struct csi_skip_desc csi_skip[12 ] = {
Thanks,
Gael
Steve Longerbeam
maintainerPATCH v2 2/2 media: imx: prpencvf: Disable CSI immediately after last EOF
In reply to Steve LongerbeamThe 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.
---
Changes in v2:
- Add Fixes: and Cc: stable
--- drivers/staging/media/imx/imx-ic-prpencvf.c | 26 ++++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-)
drivers/staging/media/imx/imx-ic-prpencvf.c +17 -9
@@ -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 )
Signature · 1 line
2.17.1
Re: [PATCH v2 2/2] media: imx: prpencvf: Disable CSI immediately after last EOF
Steve, all,
Quoted 79 lines · Steve Longerbeam
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 ofprp_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]>---
Changes in v2:
- Add Fixes: and Cc: stable--- drivers/staging/media/imx/imx-ic-prpencvf.c | 26 ++++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-)drivers/staging/media/imx/imx-ic-prpencvf.c +17
-2@@ -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);
drivers/staging/media/imx/imx-ic-prpencvf.c static int prp_s_stream(struct v4l2_subdev *sd, int enable) · line 1196
Quoted 10 lines
drivers/staging/media/imx/imx-ic-prpencvf.c +0
-7- 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 )
Thanks,
Gael