TDA4VM: CSIRX unwanted stream enable

Hi Nikita Yushchenko,

CsirxDrv_resetStream should check only a specific stream ID?

Do you have logs of the error or have you stepped through?

Best,
Jared

I was researching situation that, while using CSIRX over TIOVX capture node that only enables stream 0 (to memory) and never enables stream 1 (to CSITX), still sometimes, once per several 10s of executions, I am getting CSIRX_EVENT_TYPE_ERR_FIFO_OVERFLOW_STRM1 error.

TIOVX capture node never sets instCfg.enableStrm for streams other than 0, so stream 1 shall be never started and shall never get an overflow.

So I was looking for a path in CSIRX driver that could cause stream 1 to be started despite of driver was never requested to do that. And, I found such a path.

It looks valid to reset any running streams in case of ECC error. So looping over all streams and calling CsirxDrv_resetStream() looks ok. However, there shall be a condition to either not call CsirxDrv_resetStream() for streams that are not running, or to make CsirxDrv_resetStream() not start stream if it was not running at call time. My patch does the second.

Hi Nikita Yushchenko,

The stream shouldn't be reset/started if the stream is not enabled.

    /* check if stream is enabled */
    if (CDN_EOK != CSIRX_GetStreamStatus(&instObj->cslObj.cslCfgData,
                              &strmStatus,
                              strmIdx))
    {
        retVal = FVID2_EFAIL;
    }
    ...
    /* Re-start stream */
    if (FVID2_SOK == retVal)
    {
        strmCtrlParams.softRst = 0U;
        strmCtrlParams.abrt    = 0U;
        strmCtrlParams.stop    = 0U;
        strmCtrlParams.start   = 1U;
        status = CSIRX_SetStreamCtrl(&instObj->cslObj.cslCfgData,
                                     &strmCtrlParams,
                                     strmIdx);
        if (CDN_EOK != status)
        {
            retVal = FVID2_EBADARGS;
        }
    }

Have you noticed any CSIRX_EVENT_TYPE_ERR_FIFO_OVERFLOW_STRM1 after your patch?

Have you placed a breakpoint or print statement within the function to see if the disabled streams are being started even when disabled?

Best,
Jared

For stream that was not started, CSIRX_GetStreamStatus() returns 0 (the non-started flag is returned inside strmStatus).  So the CSIRX_SetStreamCtrl() enabling the stream is executed.

Hi Nikita Yushchenko,

I see. I will file a bug.

Looking at the current code, it doesn't look like the stream will be reset if the stream is not running. It's just the the restarting that occurs.

    /* Reset stream */
    if (FVID2_SOK == retVal)
    {
        if (1U == strmStatus.running)
        {
            /* Stream is enabled */
            strmCtrlParams.softRst = 1U;
            strmCtrlParams.abrt    = 0U;

I moved the restarting logic into the resetting block.

diff --git a/packages/ti/drv/csirx/src/csirx_event.c b/./csirx_event.c
index d2bc7ff..640381d 100755
--- a/packages/ti/drv/csirx/src/csirx_event.c
+++ b/./csirx_event.c
@@ -1199,21 +1199,22 @@ static int32_t CsirxDrv_resetStream(const CsirxDrv_InstObj *instObj,
                     currTimeout++;
                 }
             }
-        }
-    }
-    /* Re-start stream */
-    if (FVID2_SOK == retVal)
-    {
-        strmCtrlParams.softRst = 0U;
-        strmCtrlParams.abrt    = 0U;
-        strmCtrlParams.stop    = 0U;
-        strmCtrlParams.start   = 1U;
-        status = CSIRX_SetStreamCtrl(&instObj->cslObj.cslCfgData,
-                                     &strmCtrlParams,
-                                     strmIdx);
-        if (CDN_EOK != status)
-        {
-            retVal = FVID2_EBADARGS;
+
+            /* Re-start stream */
+            if (FVID2_SOK == retVal)
+            {
+                strmCtrlParams.softRst = 0U;
+                strmCtrlParams.abrt    = 0U;
+                strmCtrlParams.stop    = 0U;
+                strmCtrlParams.start   = 1U;
+                status = CSIRX_SetStreamCtrl(&instObj->cslObj.cslCfgData,
+                                            &strmCtrlParams,
+                                            strmIdx);
+                if (CDN_EOK != status)
+                {
+                    retVal = FVID2_EBADARGS;
+                }
+            }
         }
     }
 

Best,
Jared

Hmm shouldn't you put the start call into else clause if the 'if' operation?  In your current version, you restart if error happened...

Hi Nikita Yushchenko,

It just appears that way due to the way the diff looks.

    /* Reset stream */
    if (FVID2_SOK == retVal)
    {
        if (1U == strmStatus.running)
        {
            /* Stream is enabled */
            strmCtrlParams.softRst = 1U;
            strmCtrlParams.abrt    = 0U;
            strmCtrlParams.stop    = 0U;
            strmCtrlParams.start   = 0U;
            status = CSIRX_SetStreamCtrl(&instObj->cslObj.cslCfgData,
                                         &strmCtrlParams,
                                         strmIdx);
            if (CDN_EOK != status)
            {
                retVal = FVID2_EBADARGS;
            }
            currTimeout = 0U;
            status = CSIRX_GetStreamStatus(&instObj->cslObj.cslCfgData,
                                           &strmStatus,
                                           strmIdx);
            if (CDN_EOK != status)
            {
                retVal = FVID2_EBADARGS;
            }
            /* Wait for stream to become IDLE */
            while ((0x0U != strmStatus.readyState) &&
                   (0x0U != strmStatus.streamFsm)  &&
                   (FVID2_SOK == retVal))
            {
                 status = CSIRX_GetStreamStatus(&instObj->cslObj.cslCfgData,
                                           &strmStatus,
                                           strmIdx);
                if (CDN_EOK != status)
                {
                    retVal = FVID2_EBADARGS;
                    break;
                }
                if (CSIRX_TIMEOUT_VAL_MS < currTimeout)
                {
                    retVal = FVID2_ETIMEOUT;
                }
                else
                {
                    (void)Osal_delay(1U);
                    currTimeout++;
                }
            }

            /* Re-start stream */
            if (FVID2_SOK == retVal)
            {
                strmCtrlParams.softRst = 0U;
                strmCtrlParams.abrt    = 0U;
                strmCtrlParams.stop    = 0U;
                strmCtrlParams.start   = 1U;
                status = CSIRX_SetStreamCtrl(&instObj->cslObj.cslCfgData,
                                            &strmCtrlParams,
                                            strmIdx);
                if (CDN_EOK != status)
                {
                    retVal = FVID2_EBADARGS;
                }
            }
        }
    }

Best,
Jared

This version looks ok