Re: [v2,1/2] media: imx: csi: Disable CSI immediately after last EOF

[Steve Longerbeam <slongerbeam@gmail.com>]

Hi Peter,

Thanks for the suggestion. My patch introduces some inconsistency in the 
code, and your solution fixes those issues and is much simpler. It seems 
the fix is to silence the video data stream entering the IDMAC channel 
before disabling the channel, and that can be done either by disabling 
the SMFC or the CSI.

I'm testing your patch on the SabreAuto reference board. If it looks 
good, I can either make you the author or the Suggested-by:, whichever 
you prefer, let me know.

Steve


On 1/18/19 9:03 AM, Peter Seiderer wrote:
> Hello Steve,
>
> On Thu, Jan 17, 2019 at 6:15 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> 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")
> [...]
>
> Similar lockup observed on a custom i.mx6 board, fixed locally by the following
> patch/workaround:
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 555aa45e02e3..f04d1695f7a4 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -560,8 +560,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   static void csi_idmac_unsetup(struct csi_priv *priv,
>                                enum vb2_buffer_state state)
>   {
> -       ipu_idmac_disable_channel(priv->idmac_ch);
>          ipu_smfc_disable(priv->smfc);
> +       ipu_idmac_disable_channel(priv->idmac_ch);
>
>          csi_idmac_unsetup_vb2_buf(priv, state);
>   }
>
> Will test your patch the next days...
>
> Regards,
> Peter
>
> [Sorry for missing some of the CC and Message-Id, not fully subscribed to linux-media]