Repository navigation
ipc_send_queued_msg() has a hard dependency on the EDF scheduler #4356
Description
I found this IPC scheduling and dependency issue while troubleshooting DMA traces on Zephyr but I suspect it has consequences even without Zephyr. In fact I wonder whether it could maybe explain the very long standing #4333 "lag" in DMA traces too.
In Zephyr the problem is simple (once you know it!): no (legacy) EDF scheduler -> task_main_primary_core does not run -> no IPC TX flush -> no DMA traces.
enum task_state task_main_primary_core(void *data)
{
/* main audio processing loop */
while (1) {
/* sleep until next IPC or DMA */
wait_for_interrupt(0);
if (!ipc->pm_prepare_D3)
ipc_send_queued_msg();
}
}I tried to schedule task_main_primary_core() in Zephyr too but I hit the following circular dependency with the EDF scheduler where the EDF scheduler wants to initialize the main task in the middle of its own initialization and task_main_init() needs the half-initialized EDF scheduler to schedule itself.
int scheduler_init_edf(void)
{
edf_sch = rzalloc(SOF_MEM_ZONE_SYS, 0, SOF_MEM_CAPS_RAM,
sizeof(*edf_sch));
scheduler_init(SOF_SCHEDULE_EDF, &schedule_edf_ops, edf_sch);
/* initialize AND SCHEDULE main task context before enabling interrupt */
task_main_init();
interrupt_enable(edf_sch->irq, edf_sch);
}Here's one super-ugly and probably unreliable workaround. Warning: many other code changes are required to get DMA traces with Zephyr.
--- a/src/trace/dma-trace.c
+++ b/src/trace/dma-trace.c
@@ -379,7 +379,7 @@ static int dma_trace_get_avail_data(struct dma_trace_data *d,
* if no new trace is filled.
*/
if (d->old_host_offset != d->posn.host_offset) {
- ipc_msg_send(d->msg, &d->posn, false);
+ ipc_msg_send(d->msg, &d->posn, true /* try to send immediately */ );
Even in plain SOF, this does not "feel right" and as I wrote above I wonder whether it could explain the very regular DMA traces "lag" exposed in #4333
The circular dependency and all the code above is as old as the entire scheduler itself: Aug 2019 20acc16 in giant PR #1710
There was a small ipc_send_queued_msg() refactor in Feb 2020 commit 9e0b2e5 but I don't think it changed the logic.
Activity
marc-hb commentedon Jun 15, 2021
Thanks @kv2019i and @lyakh for trying to solve this issue offline, can you please share the "best bits" here?
cc: @lgirdwood , @andyross , @plbossart and @nashif
kv2019i commentedon Jun 17, 2021
@marc-hb I think we should use the Zephyr ktimer API (like used in the ll-scheduler in zephyr_domain.c) to create a repeating task to flush out the IPC send queue. This could be registered from IPC code if needed (DMA traces or position updates requested).
lgirdwood commentedon Jun 17, 2021
@marc-hb lets do as @kv2019i says, we either DMA copy the buffer to host on
We have the logic to determine if conditions 1 or 2 are true today, but do not have the thread today.
The thread should be similar to
To really simplify the logic for 2 (and reusing the LL timer) I would attach a counter to the LL thread as the dma_trace period counter and wake the thread from here (instead of creating a separate timer).
lyakh commentedon Jun 17, 2021
@kv2019i
k_timerdirectly? That runs in the interrupt context...kv2019i commentedon Jun 17, 2021
@lyakh a similar k_thread_create woken up by the timer IRQ as in zephyr_domain.
kv2019i commentedon Jun 17, 2021
And a plain k_thread which sleeps, flushes IPCs and sleeps again, would do as well.
lgirdwood commentedon Jun 17, 2021
@lyakh can you link your existing kthread code as an example for @marc-hb
marc-hb commentedon Jun 17, 2021
I think the
trace_work()DMA thread already does that and it even runs in Zephyr already. However it merely adds theposnupdate IPC to a (shared?) IPC queue and it's only that IPC queue that does not get flushed (yet) in Zephyr.marc-hb commentedon Jul 1, 2021
#4428 addressed this differently and temporarily