Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes for dynamic pipelines and multi-core #3019

Merged
merged 11 commits into from
Aug 3, 2021
Merged

Fixes for dynamic pipelines and multi-core #3019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b5d87cb
fixup! ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC
ranj063 Jun 30, 2021
44f79c7
ASoC: SOF: Intel: hda: expose get_chip_info()
ranj063 Jun 30, 2021
5c3d019
ASoC: SOF: Introduce num_cores and ref count per core
ranj063 Jun 30, 2021
58c970f
ASoC: SOF: Add ops for core_get and core_put
ranj063 Jun 30, 2021
b6e4ebb
ASoC: SOF: Intel: TGL: set core_get/put ops
ranj063 Jun 30, 2021
b3f6af8
ASoC: SOF: Intel: CNL/ICL/APL: set core_get/core_put ops
ranj063 Jun 30, 2021
c3077f1
ASoC: SOF: topology: remove sof_load_pipeline_ipc()
ranj063 Jul 21, 2021
595648a
ASoC: SOF: free widgets in sof_tear_down_pipelines() for static pipel…
ranj063 Jul 21, 2021
fc5112c
ASoC: SOF: hda: don't use the core op for power up/power down
ranj063 Jul 22, 2021
6ffd4f0
ASoC: SOF: add support for dynamic pipelines with multi-core
ranj063 Jul 21, 2021
8587c32
ASoC: SOF: Intel: hda: free DAI widget during stop and suspend
ranj063 Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sound/soc/sof/imx/imx8.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ static int imx8_probe(struct snd_sof_dev *sdev)
if (!priv)
return -ENOMEM;

sdev->num_cores = 1;
sdev->pdata->hw_pdata = priv;
priv->dev = sdev->dev;
priv->sdev = sdev;
Expand Down
1 change: 1 addition & 0 deletions sound/soc/sof/imx/imx8m.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ static int imx8m_probe(struct snd_sof_dev *sdev)
if (!priv)
return -ENOMEM;

sdev->num_cores = 1;
sdev->pdata->hw_pdata = priv;
priv->dev = sdev->dev;
priv->sdev = sdev;
Expand Down
6 changes: 3 additions & 3 deletions sound/soc/sof/intel/apl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
/* dsp core get/put */
.core_get = hda_dsp_core_get,
.core_put = hda_dsp_core_put,

/* trace callback */
.trace_init = hda_dsp_trace_init,
Expand Down
9 changes: 9 additions & 0 deletions sound/soc/sof/intel/bdw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,19 @@ static int bdw_probe(struct snd_sof_dev *sdev)
const struct sof_dev_desc *desc = pdata->desc;
struct platform_device *pdev =
container_of(sdev->dev, struct platform_device, dev);
const struct sof_intel_dsp_desc *chip;
struct resource *mmio;
u32 base, size;
int ret;

chip = get_chip_info(sdev->pdata);
if (!chip) {
dev_err(sdev->dev, "error: no such device supported\n");
return -EIO;
}

sdev->num_cores = chip->cores_num;

/* LPE base */
mmio = platform_get_resource(pdev, IORESOURCE_MEM,
desc->resindex_lpe_base);
Expand Down
9 changes: 9 additions & 0 deletions sound/soc/sof/intel/byt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,19 @@ static int byt_acpi_probe(struct snd_sof_dev *sdev)
const struct sof_dev_desc *desc = pdata->desc;
struct platform_device *pdev =
container_of(sdev->dev, struct platform_device, dev);
const struct sof_intel_dsp_desc *chip;
struct resource *mmio;
u32 base, size;
int ret;

chip = get_chip_info(sdev->pdata);
if (!chip) {
dev_err(sdev->dev, "error: no such device supported\n");
return -EIO;
}

sdev->num_cores = chip->cores_num;

/* DSP DMA can only access low 31 bits of host memory */
ret = dma_coerce_mask_and_coherent(sdev->dev, DMA_BIT_MASK(31));
if (ret < 0) {
Expand Down
6 changes: 3 additions & 3 deletions sound/soc/sof/intel/cnl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
/* dsp core get/put */
.core_get = hda_dsp_core_get,
ranj063 marked this conversation as resolved.
Show resolved Hide resolved
.core_put = hda_dsp_core_put,

/* firmware run */
.run = hda_dsp_cl_boot_firmware,
Expand Down
23 changes: 2 additions & 21 deletions sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,24 +183,6 @@ static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widg
return config;
}

static int hda_link_config_ipc(struct sof_intel_hda_stream *hda_stream,
struct snd_soc_dapm_widget *w, int channel)
{
struct snd_sof_dev *sdev = hda_stream->sdev;
struct sof_ipc_dai_config *config;
struct sof_ipc_reply reply;

config = hda_dai_update_config(w, channel);
if (!config) {
dev_err(sdev->dev, "error: no config for DAI %s\n", w->name);
return -ENOENT;
}

/* send DAI_CONFIG IPC */
return sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
&reply, sizeof(reply));
}

static int hda_link_dai_widget_update(struct sof_intel_hda_stream *hda_stream,
struct snd_soc_dapm_widget *w,
int channel, bool widget_setup)
Expand Down Expand Up @@ -354,10 +336,9 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream,
w = dai->capture_widget;

/*
* clear link DMA channel. It will be assigned when
* hw_params is set up again after resume.
* free DAI widget during stop/suspend to keep widget use_count's balanced.
*/
ret = hda_link_config_ipc(hda_stream, w, DMA_CHAN_INVALID);
ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
if (ret < 0)
return ret;

Expand Down
68 changes: 66 additions & 2 deletions sound/soc/sof/intel/hda-dsp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
struct hdac_bus *bus = sof_to_bus(sdev);
#endif
int ret;
int ret, j;

hda_sdw_int_enable(sdev, false);

Expand All @@ -630,13 +630,17 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
#endif

/* power down DSP */
ret = snd_sof_dsp_core_power_down(sdev, chip->host_managed_cores_mask);
ret = hda_dsp_core_reset_power_down(sdev, chip->host_managed_cores_mask);
if (ret < 0) {
dev_err(sdev->dev,
"error: failed to power down core during suspend\n");
return ret;
}

/* reset ref counts for all cores */
for (j = 0; j < chip->cores_num; j++)
sdev->dsp_core_ref_count[j] = 0;
plbossart marked this conversation as resolved.
Show resolved Hide resolved

/* disable ppcap interrupt */
hda_dsp_ctrl_ppcap_enable(sdev, false);
hda_dsp_ctrl_ppcap_int_enable(sdev, false);
Expand Down Expand Up @@ -963,3 +967,63 @@ void hda_dsp_d0i3_work(struct work_struct *work)
"error: failed to set DSP state %d substate %d\n",
target_state.state, target_state.substate);
}

int hda_dsp_core_get(struct snd_sof_dev *sdev, int core)
{
struct sof_ipc_pm_core_config pm_core_config = {
.hdr = {
.cmd = SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CORE_ENABLE,
.size = sizeof(pm_core_config),
},
.enable_mask = sdev->enabled_cores_mask | BIT(core),
};
int ret, ret1;

/* power up core */
ret = hda_dsp_enable_core(sdev, BIT(core));
if (ret < 0) {
dev_err(sdev->dev, "failed to power up core %d with err: %d\n",
core, ret);
return ret;
}

/* No need to send IPC for primary core or if FW boot is not complete */
if (sdev->fw_state != SOF_FW_BOOT_COMPLETE || core == SOF_DSP_PRIMARY_CORE)
return 0;

/* Now notify DSP for secondary cores */
plbossart marked this conversation as resolved.
Show resolved Hide resolved
ret = sof_ipc_tx_message(sdev->ipc, pm_core_config.hdr.cmd,
&pm_core_config, sizeof(pm_core_config),
&pm_core_config, sizeof(pm_core_config));
if (ret < 0) {
dev_err(sdev->dev, "failed to enable secondary core '%d' failed with %d\n",
core, ret);
goto power_down;
}

return ret;

power_down:
/* power down core if it is host managed and return the original error if this fails too */
ret1 = hda_dsp_core_reset_power_down(sdev, BIT(core));
if (ret1 < 0)
dev_err(sdev->dev, "failed to power down core: %d with err: %d\n", core, ret1);

return ret;
}

int hda_dsp_core_put(struct snd_sof_dev *sdev, int core)
{
int ret;

/*
* Due to limitations with powering on/off secondary cores dynamically on some platforms,
* skip sending IPC for secondary core power off
plbossart marked this conversation as resolved.
Show resolved Hide resolved
*/
ret = hda_dsp_core_reset_power_down(sdev, BIT(core));
if (ret < 0)
dev_err(sdev->dev, "failed to power down core: %d with err: %d\n",
core, ret);

return ret;
}
24 changes: 17 additions & 7 deletions sound/soc/sof/intel/hda-loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag)
struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
const struct sof_intel_dsp_desc *chip = hda->desc;
unsigned int status;
u32 flags;
unsigned long mask;
u32 flags, j;
int ret;
int i;

/* step 1: power up corex */
ret = snd_sof_dsp_core_power_up(sdev, chip->host_managed_cores_mask);
ret = hda_dsp_enable_core(sdev, chip->host_managed_cores_mask);
if (ret < 0) {
if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS)
dev_err(sdev->dev, "error: dsp core 0/1 power up failed\n");
Expand Down Expand Up @@ -148,8 +149,8 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag)
chip->ipc_ack_mask);

/* step 5: power down cores that are no longer needed */
ret = snd_sof_dsp_core_power_down(sdev, chip->host_managed_cores_mask &
~(chip->init_core_mask));
ret = hda_dsp_core_reset_power_down(sdev, chip->host_managed_cores_mask &
~(chip->init_core_mask));
if (ret < 0) {
if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS)
dev_err(sdev->dev,
Expand All @@ -168,8 +169,14 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag)
HDA_DSP_REG_POLL_INTERVAL_US,
chip->rom_init_timeout *
USEC_PER_MSEC);
if (!ret)
if (!ret) {
/* set enabled cores mask and increment ref count for cores in init_core_mask */
sdev->enabled_cores_mask |= chip->init_core_mask;
mask = sdev->enabled_cores_mask;
for_each_set_bit(j, &mask, SOF_MAX_DSP_NUM_CORES)
sdev->dsp_core_ref_count[j]++;
plbossart marked this conversation as resolved.
Show resolved Hide resolved
return 0;
}

if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS)
dev_err(sdev->dev,
Expand All @@ -185,7 +192,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag)
flags &= ~SOF_DBG_DUMP_OPTIONAL;

snd_sof_dsp_dbg_dump(sdev, flags);
snd_sof_dsp_core_power_down(sdev, chip->host_managed_cores_mask);
hda_dsp_core_reset_power_down(sdev, chip->host_managed_cores_mask);

return ret;
}
Expand Down Expand Up @@ -503,12 +510,15 @@ int hda_dsp_post_fw_run_icl(struct snd_sof_dev *sdev)
* the host whereas on TGL it is handled by the firmware.
*/
if (!hda->clk_config_lpro) {
ret = snd_sof_dsp_core_power_up(sdev, BIT(3));
ret = hda_dsp_enable_core(sdev, BIT(3));
if (ret < 0) {
dev_err(sdev->dev, "error: dsp core power up failed on core 3\n");
return ret;
}

sdev->enabled_cores_mask |= BIT(3);
sdev->dsp_core_ref_count[3]++;

snd_sof_dsp_stall(sdev, BIT(3));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like we need a macro for this magic 3...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh the comment above explains why 3 is used. I think it's better to keep an explicit 3 that is aligned with the comment, no?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It explains it, yes. And if it were just one (or two) use(s) of it, as it was until now, that would be ok-ish, but now I see 5 occurrences of "3" not counting the comment, and this is generic HDA code... So, I think it would be better to either move it to TGL-specific code - at least make a separate function for it, but even with a separate function, if it's kept in generic code, maybe it would be better to add a platform-specific bitmask and loop over bits in it... But it isn't critical at all, it just begins to look a bit fragile, error prone and too specific to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to leave this as 3 for now. But I will submit a separate PR to move this part of the code to icl.c and maybe replace the 3 with a macro there

}

Expand Down
25 changes: 9 additions & 16 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,27 +119,18 @@ int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w)

ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
&reply, sizeof(reply));
if (ret < 0) {
if (ret < 0)
dev_err(sdev->dev, "error: failed resetting DAI config for %s\n", w->name);
return ret;
}

/*
* Reset the configured_flag and free the widget even if the IPC fails to keep
* the widget use_count balanced
*/
ranj063 marked this conversation as resolved.
Show resolved Hide resolved
sof_dai->configured = false;

return sof_widget_free(sdev, swidget);
}

static const struct sof_intel_dsp_desc
*get_chip_info(struct snd_sof_pdata *pdata)
{
const struct sof_dev_desc *desc = pdata->desc;
const struct sof_intel_dsp_desc *chip_info;

chip_info = desc->chip_info;

return chip_info;
}

#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)

/*
Expand Down Expand Up @@ -899,6 +890,8 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
goto err;
}

sdev->num_cores = chip->cores_num;

hdev = devm_kzalloc(sdev->dev, sizeof(*hdev), GFP_KERNEL);
if (!hdev)
return -ENOMEM;
Expand Down Expand Up @@ -1034,9 +1027,9 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
int hda_dsp_remove(struct snd_sof_dev *sdev)
{
struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
const struct sof_intel_dsp_desc *chip = hda->desc;
struct hdac_bus *bus = sof_to_bus(sdev);
struct pci_dev *pci = to_pci_dev(sdev->dev);
const struct sof_intel_dsp_desc *chip = hda->desc;

/* cancel any attempt for DSP D0I3 */
cancel_delayed_work_sync(&hda->d0i3_work);
Expand All @@ -1061,7 +1054,7 @@ int hda_dsp_remove(struct snd_sof_dev *sdev)

/* disable cores */
if (chip)
snd_sof_dsp_core_power_down(sdev, chip->host_managed_cores_mask);
hda_dsp_core_reset_power_down(sdev, chip->host_managed_cores_mask);

/* disable DSP */
snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL,
Expand Down
2 changes: 2 additions & 0 deletions sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,8 @@ int hda_dsp_core_run(struct snd_sof_dev *sdev, unsigned int core_mask);
int hda_dsp_enable_core(struct snd_sof_dev *sdev, unsigned int core_mask);
int hda_dsp_core_reset_power_down(struct snd_sof_dev *sdev,
unsigned int core_mask);
int hda_dsp_core_get(struct snd_sof_dev *sdev, int core);
int hda_dsp_core_put(struct snd_sof_dev *sdev, int core);
void hda_dsp_ipc_int_enable(struct snd_sof_dev *sdev);
void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev);

Expand Down
6 changes: 3 additions & 3 deletions sound/soc/sof/intel/icl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ const struct snd_sof_dsp_ops sof_icl_ops = {
/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
/* dsp core get/put */
.core_get = hda_dsp_core_get,
.core_put = hda_dsp_core_put,

/* firmware run */
.run = hda_dsp_cl_boot_firmware_iccmax,
Expand Down
9 changes: 9 additions & 0 deletions sound/soc/sof/intel/pci-tng.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,18 @@ static int tangier_pci_probe(struct snd_sof_dev *sdev)
struct snd_sof_pdata *pdata = sdev->pdata;
const struct sof_dev_desc *desc = pdata->desc;
struct pci_dev *pci = to_pci_dev(sdev->dev);
const struct sof_intel_dsp_desc *chip;
u32 base, size;
int ret;

chip = get_chip_info(sdev->pdata);
if (!chip) {
dev_err(sdev->dev, "error: no such device supported\n");
return -EIO;
}

sdev->num_cores = chip->cores_num;

/* DSP DMA can only access low 31 bits of host memory */
ret = dma_coerce_mask_and_coherent(&pci->dev, DMA_BIT_MASK(31));
if (ret < 0) {
Expand Down
7 changes: 7 additions & 0 deletions sound/soc/sof/intel/shim.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,11 @@ struct sof_intel_stream {
size_t posn_offset;
};

static inline const struct sof_intel_dsp_desc *get_chip_info(struct snd_sof_pdata *pdata)
{
const struct sof_dev_desc *desc = pdata->desc;

return desc->chip_info;
}

#endif
Loading