Fixes for dynamic pipelines and multi-core #3019

ranj063 · 2021-06-30T07:29:15Z

No description provided.

sound/soc/sof/sof-priv.h

kv2019i

This is somewhat challenging set to review, but the changes seem correct. The main concern I have are the new ops. It seems a get/put model is the way to go, but now that we introduce it, having the old power_up/down ops seem really out-of-place.

sound/soc/sof/sof-audio.c

sound/soc/sof/ops.h

lyakh

I do think this API makes more sense than the previous one, looks nice! Just some minor clean up. Since none of the comments address real bugs, I'll make it a comment

sound/soc/sof/intel/hda-dai.c

sound/soc/sof/intel/hda.c

sound/soc/sof/intel/shim.h

drivers/soundwire/intel.c

sound/soc/sof/sof-audio.c

lyakh · 2021-07-01T07:11:01Z

sound/soc/sof/sof-audio.c


 	/* reset route setup status for all routes that contain this widget */
 	sof_reset_route_setup_status(sdev, swidget);
 	swidget->complete = 0;
 	dev_dbg(sdev->dev, "widget %s freed\n", swidget->widget->name);

-	return 0;
+	return ret;


you can return a negative error code if the IPC failed but in fact you still performed the rest of the clean up. A failing IPC used to return an error from this before too, so you aren't changing that. There are many places in hda.c and hda-dai.c where this error code is propagated to the caller, hopefully they're all correct and are taking into account that this can be a "soft" failure.

yes, the thing is the core ref_count needs to decremented even if the widget free fails. I think this is correct to do a core_put even if widget free failed. Otherwise, the core ref_count's will be messed up and will prevent D3

sound/soc/sof/sof-audio.c

sound/soc/sof/intel/hda-dai.c

kv2019i

I'll remove my request-changes for now.

I'm not available for re-review

plbossart

I think we should remove SSP and SoundWire patches from this PR, they are controversial and need to be tested separately.

sound/soc/sof/imx/imx8.c

sound/soc/sof/intel/shim.h

drivers/soundwire/intel.c

sound/soc/sof/ops.h

sound/soc/sof/intel/tgl.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/intel/cnl.c

sound/soc/sof/sof-audio.c

Add two fields num_cores and dsp_cores_ref_count to struct snd_sof_dev. These will be used to maintain the ref count for each core to determine when it should be powered up or down. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add ops to get/put a core that will be used to power up/down a core along with incrementing/decrementing its ref_count. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Set core_get/put() ops for TGL. When core_get() is requested for a core, its ref_count is incremented and the PM_CORE_ENABLE IPC sent to the firmware to power up the core if the current ref_count is 1. Conversely, the ref_count is decremented in core_put() and an IPC is sent to the DSP to power off the core if the ref_count is 0. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

sound/soc/sof/intel/hda-dsp.c

ranj063 · 2021-08-02T20:21:36Z

@plbossart @lyakh do you have any more comments/concerns with this PR?

sound/soc/sof/intel/hda-dsp.c

Set core_get/put ops for CNL/ICL platforms. These platforms do not support enabling/disabling secondary cores dynamically. So skip sending the IPC to power off the cores in the core_put op. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Remove the function sof_load_pipeline_ipc() and directly send the IPC instead. The pipeline core is already enabled with the call to sof_pipeline_core_enable() in sof_widget_setup(). Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

…ines Free widgets for static pipelines in sof_tear_down_pipelines(). But this feature is unavailable in older firmware with ABI < 3.19. Just reset widget use_count's for this case. This would ensure that the secondary cores enabled required for topology setup are powered down properly before the primary core is powered off during system suspend. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

The core_power_up/down() ops will be deprecated. Use the HDA platform-specific functions for powering up/down the cores during probe/suspend/remove. The enabled_cores_mask and the core ref_count's are manually updated in each of these functions. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This patch adds support for dynamic pipelines with multi-core by using the platform-specific core_get/put() ops to power up/down a core when a widget is set up/freed. Along with this, a few redundant functions are removed: 1. sof_pipeline_core_enable() is no longer needed as the pipeline core will be set up when the pipeline widget is set up 2. sof_core_enable() is replaced with snd_sof_core_get() 4. core_power_up/down() DSP ops are deprecated and replaced with core get/put ops. 5. Core power down in sof_widget_unload() during topology removal is also removed as it is not really needed. For dynamic pipelines, the cores will be powered off when they are not used. For static pipelines, the cores will be powered off in the device remove callback. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

To keep the widget use_counts balanced, free the DAI widget during suspend and also during the stop trigger. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

bardliao · 2021-08-03T10:35:11Z

There is no regression found when I run run-all-tests.sh on my CML device.

plbossart · 2021-08-03T14:39:29Z

I started Intel test 5682 on this PR as the last verification prior to merge.

plbossart · 2021-08-03T19:20:28Z

test 5682 did not show any new issues, let's merge.

ranj063 requested review from bardliao, dbaluta, jack-cy-yu, juimonen, keyonjie, kv2019i, lgirdwood, libinyang, lyakh, oder-chiou, plbossart, RanderWang, ryans-lee, shumingfan and singalsu as code owners June 30, 2021 07:29

ranj063 force-pushed the fix/multi_core_dyn_pipes branch 4 times, most recently from 82f7381 to a6225cd Compare June 30, 2021 07:56

RanderWang reviewed Jun 30, 2021

View reviewed changes

sound/soc/sof/sof-priv.h Outdated Show resolved Hide resolved

kv2019i previously requested changes Jun 30, 2021

View reviewed changes

sound/soc/sof/sof-audio.c Show resolved Hide resolved

sound/soc/sof/sof-audio.c Outdated Show resolved Hide resolved

sound/soc/sof/ops.h Show resolved Hide resolved

ranj063 force-pushed the fix/multi_core_dyn_pipes branch 4 times, most recently from 9bfcc3f to 3f1488f Compare June 30, 2021 17:53

lyakh reviewed Jul 1, 2021

View reviewed changes

plbossart requested changes Jul 1, 2021

View reviewed changes

sound/soc/sof/intel/hda-dai.c Outdated Show resolved Hide resolved

kv2019i reviewed Jul 2, 2021

View reviewed changes

plbossart requested changes Jul 6, 2021

View reviewed changes

ranj063 added 3 commits July 27, 2021 17:07

ASoC: SOF: Add ops for core_get and core_put

58c970f

Add ops to get/put a core that will be used to power up/down a core along with incrementing/decrementing its ref_count. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/multi_core_dyn_pipes branch 2 times, most recently from 716ff58 to fe4f2a9 Compare July 28, 2021 00:35

lyakh reviewed Jul 28, 2021

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

ranj063 requested review from plbossart and lyakh and removed request for plbossart and lyakh August 2, 2021 20:20

ranj063 force-pushed the fix/multi_core_dyn_pipes branch from fe4f2a9 to 64abd7d Compare August 2, 2021 20:21

plbossart reviewed Aug 2, 2021

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

ranj063 added 6 commits August 2, 2021 15:58

ASoC: SOF: Intel: hda: free DAI widget during stop and suspend

8587c32

To keep the widget use_counts balanced, free the DAI widget during suspend and also during the stop trigger. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/multi_core_dyn_pipes branch from 64abd7d to 8587c32 Compare August 2, 2021 22:59

lyakh approved these changes Aug 3, 2021

View reviewed changes

keyonjie mentioned this pull request Aug 3, 2021

[BUG]ipc timed out when CTX_SAVE on TGLU_RVP_NOCODEC_MULTICORE thesofproject/sof#4553

Closed

plbossart approved these changes Aug 3, 2021

View reviewed changes

plbossart merged commit 16d5c45 into thesofproject:topic/sof-dev Aug 3, 2021

ranj063 deleted the fix/multi_core_dyn_pipes branch August 3, 2021 23:15

XiaoyunWu6666 mentioned this pull request Aug 6, 2021

[BUG]ipc timed out when runtime resume/system-suspend-resume on TGLU_RVP_NOCODEC_CI thesofproject/sof#4552

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for dynamic pipelines and multi-core #3019

Fixes for dynamic pipelines and multi-core #3019

ranj063 commented Jun 30, 2021

kv2019i left a comment

lyakh left a comment

lyakh Jul 1, 2021

ranj063 Jul 20, 2021

kv2019i left a comment

plbossart left a comment

ranj063 commented Aug 2, 2021

bardliao commented Aug 3, 2021

plbossart commented Aug 3, 2021

plbossart commented Aug 3, 2021

Fixes for dynamic pipelines and multi-core #3019

Fixes for dynamic pipelines and multi-core #3019

Conversation

ranj063 commented Jun 30, 2021

kv2019i left a comment

Choose a reason for hiding this comment

lyakh left a comment

Choose a reason for hiding this comment

lyakh Jul 1, 2021

Choose a reason for hiding this comment

ranj063 Jul 20, 2021

Choose a reason for hiding this comment

kv2019i left a comment

Choose a reason for hiding this comment

plbossart left a comment

Choose a reason for hiding this comment

ranj063 commented Aug 2, 2021

bardliao commented Aug 3, 2021

plbossart commented Aug 3, 2021

plbossart commented Aug 3, 2021