[BUG IMX] Input/output error when do simultaneous playback/capture test #3690

Activity

1. dbaluta

   added

   bugSomething isn't working as expectedSomething isn't working as expected

   on Dec 15, 2020
2. dbaluta

   self-assigned this

   on Dec 15, 2020
3. dbaluta commented on Dec 15, 2020

   

   dbaluta

   on Dec 15, 2020

   Last edited by dbaluta

   CollaboratorAuthor

   More actions

   @lyakh @iuliana-prodan

   Regression introduced by #3339

   This happens because we reach https://github.com/thesofproject/sof/blob/master/src/schedule/dma_multi_chan_domain.c#L296

   				*comp = dma_domain->data[i][j].task->sched_comp;
   			} else if (dma_domain->data[i][j].task->sched_comp != *comp) {
   				continue;
   			}
   

   with dma_domain->data[i][j].task = NULL

   Our DMA controller has 32 channels but only two are used for this scenario so having dma_domain->data->data[i][j].task = NULL is legit.

   I wonder why this is not happening on Intel platforms. Are always all the channels started?

   We have two possible solutions for this problem:

   1. Check for dma_domain->data[i][j].task

   --- a/src/schedule/dma_multi_chan_domain.c
   +++ b/src/schedule/dma_multi_chan_domain.c
   @@ -293,7 +293,8 @@ static bool dma_multi_chan_domain_is_pending(struct ll_schedule_domain *domain,
                                           continue;
    
                                   *comp = dma_domain->data[i][j].task->sched_comp;
   -                       } else if (dma_domain->data[i][j].task->sched_comp != *comp) {
   +                       } else if (!dma_domain->data[i][j].task ||
   +                                  dma_domain->data[i][j].task->sched_comp != *comp) {
                                   continue;
                           }
   

   2. Check for channel status

   --- a/src/schedule/dma_multi_chan_domain.c
   +++ b/src/schedule/dma_multi_chan_domain.c
   @@ -286,6 +286,11 @@ static bool dma_multi_chan_domain_is_pending(struct ll_schedule_domain *domain,
    
           for (i = 0; i < dma_domain->num_dma; ++i) {
                   for (j = 0; j < dmas[i].plat_data.channels; ++j) {
   +
   +                       /* channel not running */
   +                       if (dmas[i].chan[j].status != COMP_STATE_ACTIVE)
   +                               continue;
   

   I would go for option 2) but need some guidance on this.
4. dbaluta

   added this to the v1.6 milestone on Dec 15, 2020
5. dbaluta

   assigned

   dbaluta

   and unassigned

   dbaluta

   on Dec 15, 2020
6. lgirdwood commented on Dec 17, 2020

   

   lgirdwood

   on Dec 17, 2020

   Member

   More actions

   Our DMA controller has 32 channels but only two are used for this scenario so having dma_domain->data->data[i][j].task = NULL is legit.

   I wonder why this is not happening on Intel platforms. Are always all the channels started?

   No - only 1 channel is used per stream on Intel HW. However, on some platforms the DMA IRQ is shared by all the channels.
   I would also prefer 2.
   @dbaluta btw, any ETA for CI for IMX ? I guess you have CI for Zephyr, so you could get CI for free when your Zephyr port is done ?
7. iuliana-prodan commented on Dec 17, 2020

   

   iuliana-prodan

   on Dec 17, 2020

   Contributor

   More actions

   @lgirdwood you mean that when doing a dma irq clear on a channel, this might change the status on other channels too?

   One more thing, regarding #3339 - when reading the commit message I see that it 's possible to have multiple tasks on the same channel, but in the code (ll_schedule.c and dma_multi_chan_domain.c) we only look for one task per channel. Am I missing something here?
8. dbaluta commented on Dec 17, 2020

   

   dbaluta

   on Dec 17, 2020

   CollaboratorAuthor

   More actions

   Our DMA controller has 32 channels but only two are used for this scenario so having dma_domain->data->data[i][j].task = NULL is legit.
   I wonder why this is not happening on Intel platforms. Are always all the channels started?

   No - only 1 channel is used per stream on Intel HW. However, on some platforms the DMA IRQ is shared by all the channels.
   I would also prefer 2.
   @dbaluta btw, any ETA for CI for IMX ? I guess you have CI for Zephyr, so you could get CI for free when your Zephyr port is done ?

   Best we could do is to setup an internal CI. And now we switched to 5.10 and stable-v1.6. I will talk with Zephyr guys to see what they have in terms of CI.

   Biggest stopper is that there are some architecture patches on imxes which are not yet upstream so using topic/sof-dev is out of the question for now.
9. iuliana-prodan

   added a commit that references this issue on Dec 18, 2020

   schedule: dma_multi_chan: fix null pointer when scanning for tasks

   ...

   This issue will close once commit 849a878 is merged into the 'main' branch.

   849a878
10. lgirdwood

    closed this as completedin #3696on Dec 18, 2020
11. lgirdwood

    added a commit that references this issue on Dec 18, 2020

    schedule: dma_multi_chan: fix null pointer when scanning for tasks

    ...

    This issue will close once commit 07ef8ad is merged into the 'main' branch.

    07ef8ad
12. nashif

    added a commit that references this issue on Jan 20, 2021

    schedule: dma_multi_chan: fix null pointer when scanning for tasks

    ...

    This issue will close once commit 154b054 is merged into the 'master' branch.

    154b054