Miscellaneous fixes for dynamic pipelines and multicore #4390

ranj063 · 2021-06-23T06:44:08Z

This fixes a few issues with multi-core and dynamic pipelines. Still got a few more to tackle.

src/include/sof/ipc/msg.h

src/schedule/edf_schedule.c

lgirdwood

Might be worth splitting the PR as some fixes can be merged today.

src/audio/pipeline/pipeline-graph.c

plbossart · 2021-06-23T19:05:55Z

@ranj063 does this make the combination of dynamic pipelines + multicore work, or are additional fixes needed? Most of the fixes are beyond my understanding but I can offer validation help later this week.

ranj063 · 2021-06-23T20:34:21Z

@ranj063 does this make the combination of dynamic pipelines + multicore work, or are additional fixes needed? Most of the fixes are beyond my understanding but I can offer validation help later this week.

@plbossart no it doesn't. just not yet. I have 20 out of 24 tests failing with dynamic pipelines + multicore. This bring it down to 10 failures :D. I have more fixes coming in the FW along with kernel changes

ranj063 · 2021-06-24T17:05:43Z

SOFCI TEST

ranj063 · 2021-06-25T06:22:14Z

@lgirdwood how should I address unit test falure. pipeline_free is no longer associated with widget free. Should I just remove the test?

src/audio/pipeline/pipeline-graph.c

Check and return error if enabling a core fails instead of overwriting the error with a subsequent successful core power up. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

edf_sh->list is not an item. So calling list_itel_del() on it is wrong in scheduler_free_edf(). remove list_init() from scheduler_free_edf() as the list should already be free and initialized. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

lgirdwood · 2021-06-26T13:28:14Z

@ranj063 btw, this fixed most of my valgrind warning on testbench when we free components and pipeline. I've went down from ~50 to 5 errors now. I'm looking at the following now.

==408966== Invalid read of size 8
==408966==    at 0x49CBE3C: fprintf (stdio2.h:105)
==408966==    by 0x49CBE3C: pipeline_free (pipeline-graph.c:193)
==408966==    by 0x10C289: main (testbench.c:425)
==408966==  Address 0x4be3510 is 64 bytes inside a block of size 128 free'd
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x10C187: free_comps (testbench.c:173)
==408966==    by 0x10C187: main (testbench.c:424)
==408966==  Block was alloc'd at
==408966==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CB7D5: pipeline_new (pipeline-graph.c:119)
==408966==    by 0x49C85EB: ipc_pipeline_new (helper.c:460)
==408966==    by 0x10EE3C: load_pipeline (topology.c:487)
==408966==    by 0x49E1547: load_widget (tplg_parser.c:1325)
==408966==    by 0x10F736: parse_topology (topology.c:779)
==408966==    by 0x10BE56: main (testbench.c:317)
==408966== 
==408966== Invalid read of size 8
==408966==    at 0x49CBD5C: pipeline_free (pipeline-graph.c:203)
==408966==    by 0x10C289: main (testbench.c:425)
==408966==  Address 0x4be3510 is 64 bytes inside a block of size 128 free'd
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x10C187: free_comps (testbench.c:173)
==408966==    by 0x10C187: main (testbench.c:424)
==408966==  Block was alloc'd at
==408966==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CB7D5: pipeline_new (pipeline-graph.c:119)
==408966==    by 0x49C85EB: ipc_pipeline_new (helper.c:460)
==408966==    by 0x10EE3C: load_pipeline (topology.c:487)
==408966==    by 0x49E1547: load_widget (tplg_parser.c:1325)
==408966==    by 0x10F736: parse_topology (topology.c:779)
==408966==    by 0x10BE56: main (testbench.c:317)
==408966== 
==408966== Invalid read of size 8
==408966==    at 0x49CBD65: pipeline_free (pipeline-graph.c:206)
==408966==    by 0x10C289: main (testbench.c:425)
==408966==  Address 0x4be3548 is 120 bytes inside a block of size 128 free'd
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x10C187: free_comps (testbench.c:173)
==408966==    by 0x10C187: main (testbench.c:424)
==408966==  Block was alloc'd at
==408966==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CB7D5: pipeline_new (pipeline-graph.c:119)
==408966==    by 0x49C85EB: ipc_pipeline_new (helper.c:460)
==408966==    by 0x10EE3C: load_pipeline (topology.c:487)
==408966==    by 0x49E1547: load_widget (tplg_parser.c:1325)
==408966==    by 0x10F736: parse_topology (topology.c:779)
==408966==    by 0x10BE56: main (testbench.c:317)
==408966== 
==408966== Invalid read of size 4
==408966==    at 0x49CBDC9: pipeline_free (pipeline-graph.c:208)
==408966==    by 0x10C289: main (testbench.c:425)
==408966==  Address 0x4be3540 is 112 bytes inside a block of size 128 free'd
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x10C187: free_comps (testbench.c:173)
==408966==    by 0x10C187: main (testbench.c:424)
==408966==  Block was alloc'd at
==408966==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CB7D5: pipeline_new (pipeline-graph.c:119)
==408966==    by 0x49C85EB: ipc_pipeline_new (helper.c:460)
==408966==    by 0x10EE3C: load_pipeline (topology.c:487)
==408966==    by 0x49E1547: load_widget (tplg_parser.c:1325)
==408966==    by 0x10F736: parse_topology (topology.c:779)
==408966==    by 0x10BE56: main (testbench.c:317)
==408966== 
==408966== Invalid free() / delete / delete[] / realloc()
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CBDF1: pipeline_free (pipeline-graph.c:211)
==408966==    by 0x10C289: main (testbench.c:425)
==408966==  Address 0x4be34d0 is 0 bytes inside a block of size 128 free'd
==408966==    at 0x484521F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x10C187: free_comps (testbench.c:173)
==408966==    by 0x10C187: main (testbench.c:424)
==408966==  Block was alloc'd at
==408966==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==408966==    by 0x49CB7D5: pipeline_new (pipeline-graph.c:119)
==408966==    by 0x49C85EB: ipc_pipeline_new (helper.c:460)
==408966==    by 0x10EE3C: load_pipeline (topology.c:487)
==408966==    by 0x49E1547: load_widget (tplg_parser.c:1325)
==408966==    by 0x10F736: parse_topology (topology.c:779)
==408966==    by 0x10BE56: main (testbench.c:317)
==408966==

lgirdwood · 2021-06-26T13:39:01Z

@ranj063 update, I've fixed above and got it down to some leaks now, these might all be in testbench.

==409844== 
==409844== HEAP SUMMARY:
==409844==     in use at exit: 5,088 bytes in 13 blocks
==409844==   total heap usage: 101 allocs, 88 frees, 45,835 bytes allocated
==409844== 
==409844== 0 bytes in 2 blocks are definitely lost in loss record 1 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49CAF75: buffer_alloc (buffer.c:48)
==409844==    by 0x49C9211: buffer_new (helper.c:885)
==409844==    by 0x49C9302: ipc_buffer_new (helper.c:617)
==409844==    by 0x10EA9E: load_buffer (topology.c:154)
==409844==    by 0x49E14ED: load_widget (tplg_parser.c:1313)
==409844==    by 0x10F736: parse_topology (topology.c:779)
==409844==    by 0x10BE56: main (testbench.c:317)
==409844== 
==409844== 16 bytes in 1 blocks are still reachable in loss record 2 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49C42FE: scheduler_register (schedule.c:49)
==409844==    by 0x49C42FE: scheduler_init (schedule.c:66)
==409844==    by 0x49C44C8: scheduler_init_edf (edf_schedule.c:83)
==409844==    by 0x10C6E1: tb_pipeline_setup (common_test.c:39)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 16 bytes in 1 blocks are still reachable in loss record 3 of 12
==409844==    at 0x4842839: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49D01F0: platform_ipc_init (ipc.c:48)
==409844==    by 0x10C6F6: tb_pipeline_setup (common_test.c:43)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 24 bytes in 1 blocks are still reachable in loss record 4 of 12
==409844==    at 0x4842839: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49C44AB: scheduler_init_edf (edf_schedule.c:80)
==409844==    by 0x10C6E1: tb_pipeline_setup (common_test.c:39)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 24 bytes in 1 blocks are still reachable in loss record 5 of 12
==409844==    at 0x4842839: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49C44AB: scheduler_init_edf (edf_schedule.c:80)
==409844==    by 0x10C6FF: tb_pipeline_setup (common_test.c:49)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 40 bytes in 1 blocks are still reachable in loss record 6 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49C4290: scheduler_init (schedule.c:60)
==409844==    by 0x49C44C8: scheduler_init_edf (edf_schedule.c:83)
==409844==    by 0x10C6E1: tb_pipeline_setup (common_test.c:39)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 40 bytes in 1 blocks are still reachable in loss record 7 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49C4290: scheduler_init (schedule.c:60)
==409844==    by 0x49C44C8: scheduler_init_edf (edf_schedule.c:83)
==409844==    by 0x10C6FF: tb_pipeline_setup (common_test.c:49)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 120 bytes in 1 blocks are still reachable in loss record 8 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49CF46E: sa_init (agent.c:97)
==409844==    by 0x10C6EE: tb_pipeline_setup (common_test.c:40)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 152 bytes in 1 blocks are still reachable in loss record 9 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49CA09F: ipc_init (ipc-common.c:221)
==409844==    by 0x10C6F6: tb_pipeline_setup (common_test.c:43)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 176 bytes in 1 blocks are still reachable in loss record 10 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49CF247: init_system_notify (notifier.c:192)
==409844==    by 0x10C6DC: tb_pipeline_setup (common_test.c:38)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 384 bytes in 1 blocks are still reachable in loss record 11 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49CA0BC: ipc_init (ipc-common.c:222)
==409844==    by 0x10C6F6: tb_pipeline_setup (common_test.c:43)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== 4,096 bytes in 1 blocks are still reachable in loss record 12 of 12
==409844==    at 0x4847A25: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409844==    by 0x49D0209: platform_ipc_init (ipc.c:52)
==409844==    by 0x10C6F6: tb_pipeline_setup (common_test.c:43)
==409844==    by 0x10BE29: main (testbench.c:311)
==409844== 
==409844== LEAK SUMMARY:
==409844==    definitely lost: 0 bytes in 2 blocks
==409844==    indirectly lost: 0 bytes in 0 blocks
==409844==      possibly lost: 0 bytes in 0 blocks
==409844==    still reachable: 5,088 bytes in 11 blocks
==409844==         suppressed: 0 bytes in 0 blocks
==409844== 
==409844== For lists of detected and suppressed errors, rerun with: -s
==409844== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

lgirdwood · 2021-06-26T20:32:04Z

@ranj063 testbench now running clean with this PR and #4408 and #4407

ranj063 · 2021-06-27T01:41:41Z

@lgirdwood I've updated the PR with the unit test failure update and a couple more fixes to make buffer and pipeline shared along with checking if the core is enabled before scheduling the DAI config IDC

ranj063 · 2021-06-27T01:41:58Z

this could fix the IDC timeout in #4382

…ree() Pipeline components will be freed by the host before the pipeline widget itself is freed. No need to walk the pipeline and free anything during pipeline_free(). Fix the unit tests to remove the tests that are not relevant anymore. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

When a secondary core is disabled and re-enabled without a D3 cycle, the POWER_UP IDC message end up reading the trace point message from the first boot. Reset it during power down to avoid this. Suggested-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Make them shared between cores as they can be shared with other pipelines, trace and possibly even DMA. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

DAI config IPC is propagated to all DAI comps scheduled on all cores but it should only be done if the secondary core is enabled. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 requested review from dbaluta, lbetlej, lgirdwood, mmaka1, mrajwa and plbossart as code owners June 23, 2021 06:44

ranj063 requested a review from lyakh June 23, 2021 06:45

ranj063 force-pushed the fix/multi_core branch from 909a69e to 5876880 Compare June 23, 2021 07:34

lyakh requested changes Jun 23, 2021

View reviewed changes

src/include/sof/ipc/msg.h Outdated Show resolved Hide resolved

src/schedule/edf_schedule.c Outdated Show resolved Hide resolved

lgirdwood reviewed Jun 23, 2021

View reviewed changes

src/audio/pipeline/pipeline-graph.c Outdated Show resolved Hide resolved

ranj063 force-pushed the fix/multi_core branch 2 times, most recently from 52a0bb8 to 9abfa64 Compare June 23, 2021 18:36

ranj063 force-pushed the fix/multi_core branch from 9abfa64 to 7947abe Compare June 23, 2021 20:43

ranj063 requested review from lgirdwood and lyakh June 23, 2021 20:44

lyakh reviewed Jun 25, 2021

View reviewed changes

src/audio/pipeline/pipeline-graph.c Outdated Show resolved Hide resolved

src/audio/pipeline/pipeline-graph.c Outdated Show resolved Hide resolved

lgirdwood approved these changes Jun 25, 2021

View reviewed changes

lgirdwood added the multicore Issues observed when not only core#0 is used. label Jun 25, 2021

ranj063 added 2 commits June 25, 2021 14:03

ipc: ipc3: handler: fix error handling for pm_core_enable

41aa7f8

Check and return error if enabling a core fails instead of overwriting the error with a subsequent successful core power up. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/multi_core branch from 7947abe to 710ea4c Compare June 27, 2021 01:39

ranj063 added 4 commits June 26, 2021 19:48

pipeline/buffer: alloc them in the runtime shared zone

e0ebb33

Make them shared between cores as they can be shared with other pipelines, trace and possibly even DMA. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

dai: only process DAI config for comps on cores that enabled

4bc8362

DAI config IPC is propagated to all DAI comps scheduled on all cores but it should only be done if the secondary core is enabled. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/multi_core branch from 710ea4c to 4bc8362 Compare June 27, 2021 02:49

lgirdwood approved these changes Jun 27, 2021

View reviewed changes

lgirdwood merged commit bccecb1 into thesofproject:main Jun 27, 2021

ranj063 deleted the fix/multi_core branch June 27, 2021 22:41

libinyang mentioned this pull request Jun 29, 2021

[BUG] [TGL Chrome] Sporadic playback failure on speaker, headset and HDMI #4382

Closed

Miscellaneous fixes for dynamic pipelines and multicore #4390

Miscellaneous fixes for dynamic pipelines and multicore #4390

Uh oh!

Conversation

ranj063 commented Jun 23, 2021

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

plbossart commented Jun 23, 2021

Uh oh!

ranj063 commented Jun 23, 2021

Uh oh!

ranj063 commented Jun 24, 2021

Uh oh!

ranj063 commented Jun 25, 2021

Uh oh!

Uh oh!

Uh oh!

lgirdwood commented Jun 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Jun 26, 2021

Uh oh!

lgirdwood commented Jun 26, 2021

Uh oh!

ranj063 commented Jun 27, 2021

Uh oh!

ranj063 commented Jun 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lgirdwood commented Jun 26, 2021 •

edited

Loading