Pipeline scheduling #4471

lyakh · 2021-07-09T15:06:49Z

Run trigger START and RELEASE in the pipeline task context

lyakh · 2021-07-09T15:17:54Z

this survived "run-all-tests.sh -l 1" in a Zephyr build with only expected failures: xrun-inject, ipc-flood, kmod and suspend-resume-with-*.

lgirdwood

Just need some more comments describing the flows and transitions and rules.

lgirdwood · 2021-07-13T15:24:04Z

src/drivers/intel/cavs/ipc.c

Is this patch in another PR ?

yes, I closed #4469

@lgirdwood what do you think about this change - does it seem safe to you? @ranj063 expressed some concerns in #4469 (comment)

lgirdwood · 2021-07-13T15:25:18Z

src/drivers/intel/cavs/ipc.c

This also looks similar to the other PR ?

src/audio/pipeline/pipeline-schedule.c

ranj063 · 2021-07-13T17:19:42Z

src/drivers/intel/cavs/ipc.c

@lyakh how about moving the power_off to be the first thing in platform_complete_cmd(). That way we'd still have the same sequence as before ie power off and then send the host notification?

@ranj063 but then what's the point of this patch? :-) The whole idea was to make sure we acknowledge the completion of the IPC to the host, so that it doesn't run onto an IPC timeout.

I think this patch is not critical. CTX_SAVE IPC is always executed on core 0 anyway

ranj063 · 2021-07-14T14:40:00Z

src/idc/idc.c

BTW @lyakh I assume you have tested this change with xtos too and not just zephyr? It looks correct for sure but we dont have a topology in CI with multi-core. So just wanted to make sure

@ranj063 We don't, really?? Oh... I thought PR CI was testing multicore? @keyonjie

@lyakh I tested this PR with my multi-core topology and it passed with flying colors! good to go!

lgirdwood · 2021-07-14T20:41:08Z

@lyakh looks good on all platforms except legacy ? can you check, it could be topology is using DMA driver, whereas they can support timer driven too.

lyakh · 2021-07-15T10:37:34Z

@lyakh looks good on all platforms except legacy ? can you check, it could be topology is using DMA driver, whereas they can support timer driven too.

@lgirdwood I'm wondering: with this PR TRIG_START doesn't trigger components from the IPC, instead when the pipeline task is scheduled for the first time, then TRIG_START is sent to all components. But when using the DMA domain, isn't DMA scheduling started from scheduling component's TRIG_START? So, the task never gets scheduled? I can of course make this decision to postpone TRIG_START depend on timer or Zephyr LL-scheduling domain...

lyakh · 2021-07-19T08:12:36Z

This PR now has 2 issues:

power down in IPC completion seems to be wrong. SOF has assembly code to acknowledge the power-down IPC, which in XTOS builds is also synchronised with clock management. In Zephyr builds clock is managed by Zephyr so the SOF powers down hardware asynchronously to clock halting. Supposedly a clock interrupt can trigger at that time, trying to access RAM. But this raises a question: @lgirdwood why don't we just disable interrupts when powering down? All this should be running on the primary core.
it breaks the DMA scheduling domain. To fix that I'll try to only perform synchronous triggering for timer-domain tasks, not for DMA-domain tasks

lgirdwood · 2021-07-19T08:32:28Z

@lgirdwood why don't we just disable interrupts when powering down? All this should be running on the primary core.

Doesn't work, the clock IP is still ON and asserts an IRQ. The clock must be OFF so no IRQ is asserted.

Btw, not following why power OFF is related to this PR for LL triggering ?

Host should stop all streams prior to sending PM OFF IPC (otherwise FW can reply to PM off with -EBUSY)
We should only reply to stream trigger IPCs when the LL scheduler has performed the trigger op. i.e. this means the IPC reply can take up to 1ms.

lyakh · 2021-07-19T09:40:39Z

@lgirdwood why don't we just disable interrupts when powering down? All this should be running on the primary core.

Doesn't work, the clock IP is still ON and asserts an IRQ. The clock must be OFF so no IRQ is asserted.

I understand, but IRQs will be off at the DSP level, so the DSP won't be interrupted. Or do you mean it still somehow breaks at the hardware level?

Btw, not following why power OFF is related to this PR for LL triggering ?

Not to LL triggering, but this PR also contains a patch for power off sequence. Yes, it is unrelated to the PR, I can drop it from here.

lgirdwood · 2021-07-19T12:18:42Z

@lgirdwood why don't we just disable interrupts when powering down? All this should be running on the primary core.

Doesn't work, the clock IP is still ON and asserts an IRQ. The clock must be OFF so no IRQ is asserted.

I understand, but IRQs will be off at the DSP level, so the DSP won't be interrupted. Or do you mean it still somehow breaks at the hardware level?

Yes, HW level blocker.

keyonjie · 2021-07-20T02:24:47Z

This PR now has 2 issues:

power down in IPC completion seems to be wrong. SOF has assembly code to acknowledge the power-down IPC, which in XTOS builds is also synchronised with clock management. In Zephyr builds clock is managed by Zephyr so the SOF powers down hardware asynchronously to clock halting. Supposedly a clock interrupt can trigger at that time, trying to access RAM. But this raises a question: @lgirdwood why don't we just disable interrupts when powering down? All this should be running on the primary core.

it breaks the DMA scheduling domain. To fix that I'll try to only perform synchronous triggering for timer-domain tasks, not for DMA-domain tasks

Don't have time to review details yet, but yes, let's NOT merge this until the DMA scheduling domain issues are fixed.

keyonjie

Let's not merge it before regressions on DMA domain addressed.

pipeline_task_init() is always called with pipeline_task as its last parameter. Remove the parameter and use the function explicitly. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Use bool type for boolean flags, split some too long lines, merge some needlessly split lines. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh · 2021-07-27T09:42:47Z

Let's not merge it before regressions on DMA domain addressed.

@keyonjie this version should leave DMA domain pipelines unchanged.

When the firmware receives a START or RELEASE IPC message, it immediately triggers all involved components, which starts DMA. Then it schedules the pipeline task, but since the scheduler can be already running at that time, the task might be scheduled when DMA data isn't available yet or has already overflowed. To fix this change the control flow to also trigger all components from the task during its first run. Actual data processing then begins with the next period. Note, that this is currently only possible with pipelines, using timer-based scheduling. Pipelines, using DMA interrupts for scheduling are unaffected. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lgirdwood · 2021-07-27T13:55:00Z

CI shows unrelated CML kernel PM failure.

lyakh force-pushed the pipe-sched branch from 1c0e7ed to dd1c182 Compare July 9, 2021 15:15

lyakh force-pushed the pipe-sched branch from dd1c182 to fa48bc1 Compare July 12, 2021 15:50

lgirdwood requested changes Jul 13, 2021

View reviewed changes

lyakh mentioned this pull request Jul 13, 2021

Multicore IPC #4469

Closed

lyakh changed the title ~~[WiP] Pipeline scheduling~~ Pipeline scheduling Jul 13, 2021

lyakh marked this pull request as ready for review July 13, 2021 16:11

lyakh requested review from bardliao, dbaluta, lbetlej, mmaka1 and plbossart as code owners July 13, 2021 16:11

lyakh force-pushed the pipe-sched branch from fa48bc1 to 1e142c7 Compare July 13, 2021 16:31

lyakh requested a review from lgirdwood July 13, 2021 16:31

ranj063 reviewed Jul 13, 2021

View reviewed changes

ranj063 reviewed Jul 14, 2021

View reviewed changes

ranj063 approved these changes Jul 18, 2021

View reviewed changes

keyonjie suggested changes Jul 20, 2021

View reviewed changes

This was referenced Jul 21, 2021

[BUG] glitch in test 02_02_TestPtpSimpleLoopbackMasterRecord16000Hz32b32b4ch #4080

Closed

[BUG] THD value is larger than acceptable value #4180

Closed

zephyr: sof-logger #4503

Merged

marc-hb mentioned this pull request Jul 22, 2021

[FEATURE] Extend zephyr_ll to get its LL system tick from the DMA IRQ #4523

Closed

lyakh mentioned this pull request Jul 26, 2021

ipc: let secondary cores acknowledge IPC messages #4549

Merged

lyakh added 2 commits July 27, 2021 08:15

pipeline: (cosmetic) simplify pipeline_task_init()

5d48109

pipeline_task_init() is always called with pipeline_task as its last parameter. Remove the parameter and use the function explicitly. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

pipeline: (cosmetic) use bool type, adjust line lengths

47b12d3

Use bool type for boolean flags, split some too long lines, merge some needlessly split lines. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh force-pushed the pipe-sched branch 2 times, most recently from 1a9f528 to 6e44891 Compare July 27, 2021 09:41

lyakh force-pushed the pipe-sched branch from 6e44891 to b6776ec Compare July 27, 2021 10:33

lyakh requested a review from keyonjie July 27, 2021 13:38

lgirdwood approved these changes Jul 27, 2021

View reviewed changes

lgirdwood merged commit 9a7a5ce into thesofproject:main Jul 27, 2021

lyakh deleted the pipe-sched branch July 27, 2021 14:10

lyakh mentioned this pull request Jul 28, 2021

stop all connected pipelines on XRUNs #4562

Merged

Pipeline scheduling #4471

Pipeline scheduling #4471

Uh oh!

Conversation

lyakh commented Jul 9, 2021

Uh oh!

lyakh commented Jul 9, 2021

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ranj063 Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jul 14, 2021

Uh oh!

lyakh commented Jul 15, 2021

Uh oh!

lyakh commented Jul 19, 2021

Uh oh!

lgirdwood commented Jul 19, 2021

Uh oh!

lyakh commented Jul 19, 2021

Uh oh!

lgirdwood commented Jul 19, 2021

Uh oh!

keyonjie commented Jul 20, 2021

Uh oh!

keyonjie left a comment

Choose a reason for hiding this comment

Uh oh!

lyakh commented Jul 27, 2021

Uh oh!

lgirdwood commented Jul 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ranj063 Jul 13, 2021 •

edited

Loading