2021-09-17 15:22:11

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 00/12] Add support for on demand pipeline setup/destroy

From: Daniel Baluta <[email protected]>

This patchseries implements the new feature to setup/teardown pipeline
as needed when a PCM is open/closed.

This only only support single cores, with support for multicore arriving
with a future patchseries.

Review with SOF community at
https://github.com/thesofproject/linux/pull/2794

Changes since v1:
- added my own Signed-off-by tag.

Ranjani Sridharan (12):
ASoC: topology: change the complete op in snd_soc_tplg_ops to return
int
ASoC: SOF: control: Add access field in struct snd_sof_control
ASoC: SOF: topology: Add new token for dynamic pipeline
ASoC: SOF: sof-audio: add helpers for widgets, kcontrols and dai
config set up
AsoC: dapm: export a couple of functions
ASoC: SOF: Add new fields to snd_sof_route
ASoC: SOF: restore kcontrols for widget during set up
ASoC: SOF: Don't set up widgets during topology parsing
ASoC: SOF: Introduce widget use_count
ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC
ASoC: SOF: Add support for dynamic pipelines
ASoC: SOF: topology: Add kernel parameter for topology verification

include/sound/soc-dpcm.h | 1 +
include/sound/soc-topology.h | 2 +-
include/uapi/sound/sof/tokens.h | 1 +
sound/soc/intel/skylake/skl-topology.c | 6 +-
sound/soc/soc-dapm.c | 2 +
sound/soc/soc-pcm.c | 4 +-
sound/soc/soc-topology.c | 10 +-
sound/soc/sof/intel/hda-dai.c | 176 +++---
sound/soc/sof/intel/hda.c | 177 ++++--
sound/soc/sof/intel/hda.h | 5 +
sound/soc/sof/ipc.c | 22 +
sound/soc/sof/pcm.c | 58 +-
sound/soc/sof/pm.c | 4 +-
sound/soc/sof/sof-audio.c | 709 +++++++++++++++++++------
sound/soc/sof/sof-audio.h | 32 +-
sound/soc/sof/sof-priv.h | 1 +
sound/soc/sof/topology.c | 362 +++++--------
17 files changed, 1034 insertions(+), 538 deletions(-)

--
2.27.0


2021-09-17 15:22:46

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 07/12] ASoC: SOF: restore kcontrols for widget during set up

From: Ranjani Sridharan <[email protected]>

Restore kcontrols for each widget after it has been set up
successfully.

Signed-off-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Kai Vehmanen <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Seppo Ingalsuo <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/sof/sof-audio.c | 59 ++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index b52a453ae9d7..b27760208a4b 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -64,6 +64,25 @@ static int sof_dai_config_setup(struct snd_sof_dev *sdev, struct snd_sof_dai *da
return ret;
}

+static int sof_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
+{
+ struct snd_sof_control *scontrol;
+ int ret;
+
+ /* set up all controls for the widget */
+ list_for_each_entry(scontrol, &sdev->kcontrol_list, list)
+ if (scontrol->comp_id == swidget->comp_id) {
+ ret = sof_kcontrol_setup(sdev, scontrol);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: fail to set up kcontrols for widget %s\n",
+ swidget->widget->name);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
{
struct sof_ipc_pipe_new *pipeline;
@@ -113,10 +132,20 @@ static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swi
&r, sizeof(r));
break;
}
- if (ret < 0)
+ if (ret < 0) {
dev_err(sdev->dev, "error: failed to load widget %s\n", swidget->widget->name);
- else
- dev_dbg(sdev->dev, "widget %s setup complete\n", swidget->widget->name);
+ return ret;
+ }
+
+ /* restore kcontrols for widget */
+ ret = sof_widget_kcontrol_setup(sdev, swidget);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: failed to restore kcontrols for widget %s\n",
+ swidget->widget->name);
+ return ret;
+ }
+
+ dev_dbg(sdev->dev, "widget %s setup complete\n", swidget->widget->name);

return ret;
}
@@ -203,22 +232,6 @@ int sof_set_hw_params_upon_resume(struct device *dev)
return snd_sof_dsp_hw_params_upon_resume(sdev);
}

-static int sof_restore_kcontrols(struct device *dev)
-{
- struct snd_sof_dev *sdev = dev_get_drvdata(dev);
- struct snd_sof_control *scontrol;
- int ret = 0;
-
- /* restore kcontrol values */
- list_for_each_entry(scontrol, &sdev->kcontrol_list, list) {
- ret = sof_kcontrol_setup(sdev, scontrol);
- if (ret < 0)
- return ret;
- }
-
- return 0;
-}
-
const struct sof_ipc_pipe_new *snd_sof_pipeline_find(struct snd_sof_dev *sdev,
int pipeline_id)
{
@@ -309,13 +322,7 @@ int sof_restore_pipelines(struct device *dev)
}
}

- /* restore pipeline kcontrols */
- ret = sof_restore_kcontrols(dev);
- if (ret < 0)
- dev_err(dev,
- "error: restoring kcontrols after resume\n");
-
- return ret;
+ return 0;
}

/* This function doesn't free widgets. It only resets the set up status for all routes */
--
2.27.0

2021-09-17 21:43:10

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 04/12] ASoC: SOF: sof-audio: add helpers for widgets, kcontrols and dai config set up

From: Ranjani Sridharan <[email protected]>

Refactor the existing code to use helper functions to
set up/free widgets, send dai config and set up kcontrols for
widgets. These will be reused later on for setting up widgets in
the connected DAPM widgets list for a particular PCM when the
dynamic pipeline feature is implemented.

Signed-off-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/sof/sof-audio.c | 234 +++++++++++++++++++-------------------
1 file changed, 116 insertions(+), 118 deletions(-)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 989912f2b739..a4b9bb99bced 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -11,6 +11,116 @@
#include "sof-audio.h"
#include "ops.h"

+static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control *scontrol)
+{
+ int ipc_cmd, ctrl_type;
+ int ret;
+
+ /* reset readback offset for scontrol */
+ scontrol->readback_offset = 0;
+
+ /* notify DSP of kcontrol values */
+ switch (scontrol->cmd) {
+ case SOF_CTRL_CMD_VOLUME:
+ case SOF_CTRL_CMD_ENUM:
+ case SOF_CTRL_CMD_SWITCH:
+ ipc_cmd = SOF_IPC_COMP_SET_VALUE;
+ ctrl_type = SOF_CTRL_TYPE_VALUE_CHAN_SET;
+ break;
+ case SOF_CTRL_CMD_BINARY:
+ ipc_cmd = SOF_IPC_COMP_SET_DATA;
+ ctrl_type = SOF_CTRL_TYPE_DATA_SET;
+ break;
+ default:
+ return 0;
+ }
+
+ ret = snd_sof_ipc_set_get_comp_data(scontrol, ipc_cmd, ctrl_type, scontrol->cmd, true);
+ if (ret < 0)
+ dev_err(sdev->dev, "error: failed kcontrol value set for widget: %d\n",
+ scontrol->comp_id);
+
+ return ret;
+}
+
+static int sof_dai_config_setup(struct snd_sof_dev *sdev, struct snd_sof_dai *dai)
+{
+ struct sof_ipc_dai_config *config;
+ struct sof_ipc_reply reply;
+ int ret;
+
+ config = &dai->dai_config[dai->current_config];
+ if (!config) {
+ dev_err(sdev->dev, "error: no config for DAI %s\n", dai->name);
+ return -EINVAL;
+ }
+
+ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
+ &reply, sizeof(reply));
+
+ if (ret < 0)
+ dev_err(sdev->dev, "error: failed to set dai config for %s\n", dai->name);
+
+ return ret;
+}
+
+static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
+{
+ struct sof_ipc_pipe_new *pipeline;
+ struct sof_ipc_comp_reply r;
+ struct sof_ipc_cmd_hdr *hdr;
+ struct sof_ipc_comp *comp;
+ struct snd_sof_dai *dai;
+ size_t ipc_size;
+ int ret;
+
+ /* skip if there is no private data */
+ if (!swidget->private)
+ return 0;
+
+ ret = sof_pipeline_core_enable(sdev, swidget);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: failed to enable target core: %d for widget %s\n",
+ ret, swidget->widget->name);
+ return ret;
+ }
+
+ switch (swidget->id) {
+ case snd_soc_dapm_dai_in:
+ case snd_soc_dapm_dai_out:
+ ipc_size = sizeof(struct sof_ipc_comp_dai) + sizeof(struct sof_ipc_comp_ext);
+ comp = kzalloc(ipc_size, GFP_KERNEL);
+ if (!comp)
+ return -ENOMEM;
+
+ dai = swidget->private;
+ memcpy(comp, &dai->comp_dai, sizeof(struct sof_ipc_comp_dai));
+
+ /* append extended data to the end of the component */
+ memcpy((u8 *)comp + sizeof(struct sof_ipc_comp_dai), &swidget->comp_ext,
+ sizeof(swidget->comp_ext));
+
+ ret = sof_ipc_tx_message(sdev->ipc, comp->hdr.cmd, comp, ipc_size, &r, sizeof(r));
+ kfree(comp);
+ break;
+ case snd_soc_dapm_scheduler:
+ pipeline = swidget->private;
+ ret = sof_load_pipeline_ipc(sdev->dev, pipeline, &r);
+ break;
+ default:
+ hdr = swidget->private;
+ ret = sof_ipc_tx_message(sdev->ipc, hdr->cmd, swidget->private, hdr->size,
+ &r, sizeof(r));
+ break;
+ }
+ if (ret < 0)
+ dev_err(sdev->dev, "error: failed to load widget %s\n", swidget->widget->name);
+ else
+ dev_dbg(sdev->dev, "widget %s setup complete\n", swidget->widget->name);
+
+ return ret;
+}
+
/*
* helper to determine if there are only D0i3 compatible
* streams active
@@ -97,46 +207,13 @@ static int sof_restore_kcontrols(struct device *dev)
{
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
struct snd_sof_control *scontrol;
- int ipc_cmd, ctrl_type;
int ret = 0;

/* restore kcontrol values */
list_for_each_entry(scontrol, &sdev->kcontrol_list, list) {
- /* reset readback offset for scontrol after resuming */
- scontrol->readback_offset = 0;
-
- /* notify DSP of kcontrol values */
- switch (scontrol->cmd) {
- case SOF_CTRL_CMD_VOLUME:
- case SOF_CTRL_CMD_ENUM:
- case SOF_CTRL_CMD_SWITCH:
- ipc_cmd = SOF_IPC_COMP_SET_VALUE;
- ctrl_type = SOF_CTRL_TYPE_VALUE_CHAN_SET;
- ret = snd_sof_ipc_set_get_comp_data(scontrol,
- ipc_cmd, ctrl_type,
- scontrol->cmd,
- true);
- break;
- case SOF_CTRL_CMD_BINARY:
- ipc_cmd = SOF_IPC_COMP_SET_DATA;
- ctrl_type = SOF_CTRL_TYPE_DATA_SET;
- ret = snd_sof_ipc_set_get_comp_data(scontrol,
- ipc_cmd, ctrl_type,
- scontrol->cmd,
- true);
- break;
-
- default:
- break;
- }
-
- if (ret < 0) {
- dev_err(dev,
- "error: failed kcontrol value set for widget: %d\n",
- scontrol->comp_id);
-
+ ret = sof_kcontrol_setup(sdev, scontrol);
+ if (ret < 0)
return ret;
- }
}

return 0;
@@ -163,77 +240,14 @@ int sof_restore_pipelines(struct device *dev)
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
struct snd_sof_widget *swidget;
struct snd_sof_route *sroute;
- struct sof_ipc_pipe_new *pipeline;
struct snd_sof_dai *dai;
- struct sof_ipc_cmd_hdr *hdr;
- struct sof_ipc_comp *comp;
- size_t ipc_size;
int ret;

/* restore pipeline components */
list_for_each_entry_reverse(swidget, &sdev->widget_list, list) {
- struct sof_ipc_comp_reply r;
-
- /* skip if there is no private data */
- if (!swidget->private)
- continue;
-
- ret = sof_pipeline_core_enable(sdev, swidget);
- if (ret < 0) {
- dev_err(dev,
- "error: failed to enable target core: %d\n",
- ret);
-
- return ret;
- }
-
- switch (swidget->id) {
- case snd_soc_dapm_dai_in:
- case snd_soc_dapm_dai_out:
- ipc_size = sizeof(struct sof_ipc_comp_dai) +
- sizeof(struct sof_ipc_comp_ext);
- comp = kzalloc(ipc_size, GFP_KERNEL);
- if (!comp)
- return -ENOMEM;
-
- dai = swidget->private;
- memcpy(comp, &dai->comp_dai,
- sizeof(struct sof_ipc_comp_dai));
-
- /* append extended data to the end of the component */
- memcpy((u8 *)comp + sizeof(struct sof_ipc_comp_dai),
- &swidget->comp_ext, sizeof(swidget->comp_ext));
-
- ret = sof_ipc_tx_message(sdev->ipc, comp->hdr.cmd,
- comp, ipc_size,
- &r, sizeof(r));
- kfree(comp);
- break;
- case snd_soc_dapm_scheduler:
-
- /*
- * During suspend, all DSP cores are powered off.
- * Therefore upon resume, create the pipeline comp
- * and power up the core that the pipeline is
- * scheduled on.
- */
- pipeline = swidget->private;
- ret = sof_load_pipeline_ipc(dev, pipeline, &r);
- break;
- default:
- hdr = swidget->private;
- ret = sof_ipc_tx_message(sdev->ipc, hdr->cmd,
- swidget->private, hdr->size,
- &r, sizeof(r));
- break;
- }
- if (ret < 0) {
- dev_err(dev,
- "error: failed to load widget type %d with ID: %d\n",
- swidget->widget->id, swidget->comp_id);
-
+ ret = sof_widget_setup(sdev, swidget);
+ if (ret < 0)
return ret;
- }
}

/* restore pipeline connections */
@@ -266,15 +280,8 @@ int sof_restore_pipelines(struct device *dev)

/* restore dai links */
list_for_each_entry_reverse(dai, &sdev->dai_list, list) {
- struct sof_ipc_reply reply;
struct sof_ipc_dai_config *config = &dai->dai_config[dai->current_config];

- if (!config) {
- dev_err(dev, "error: no config for DAI %s\n",
- dai->name);
- continue;
- }
-
/*
* The link DMA channel would be invalidated for running
* streams but not for streams that were in the PAUSED
@@ -284,18 +291,9 @@ int sof_restore_pipelines(struct device *dev)
if (config->type == SOF_DAI_INTEL_HDA)
config->hda.link_dma_ch = DMA_CHAN_INVALID;

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

/* complete pipeline */
--
2.27.0

2021-09-17 21:44:59

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 10/12] ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC

From: Ranjani Sridharan <[email protected]>

With the implementation of the dynamic pipeline feature, widgets
will only be setup when a PCM is opened during the
hw_params ioctl. The BE hw_params callback is responsible for
sending the DAI_CONFIG for the DAI widgets in the DSP.
With dynamic pipelines, the DAI widgets will need to set up
first before sending the DAI_CONFIG IPC in the BE hw_params.

Update the BE hw_params/hw_free callbacks for all ALH, HDA and SSP
DAIs to set up/free the DAI widget before/after DAI_CONFIG IPC.

Signed-off-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/sof/intel/hda-dai.c | 176 ++++++++++++++++++++-------------
sound/soc/sof/intel/hda.c | 177 +++++++++++++++++++++++++---------
sound/soc/sof/intel/hda.h | 5 +
sound/soc/sof/sof-audio.c | 1 +
sound/soc/sof/sof-audio.h | 2 +-
sound/soc/sof/topology.c | 3 -
6 files changed, 245 insertions(+), 119 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 2f54a659b78a..d1ec8bfb6002 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -155,49 +155,70 @@ static int hda_link_dma_params(struct hdac_ext_stream *stream,
return 0;
}

-/* Send DAI_CONFIG IPC to the DAI that matches the dai_name and direction */
-static int hda_link_config_ipc(struct sof_intel_hda_stream *hda_stream,
- const char *dai_name, int channel, int dir)
+/* Update config for the DAI widget */
+static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w,
+ int channel)
{
+ struct snd_sof_widget *swidget = w->dobj.private;
struct sof_ipc_dai_config *config;
struct snd_sof_dai *sof_dai;
- struct sof_ipc_reply reply;
- int ret = 0;

- list_for_each_entry(sof_dai, &hda_stream->sdev->dai_list, list) {
- if (!sof_dai->cpu_dai_name)
- continue;
+ if (!swidget) {
+ dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name);
+ return NULL;
+ }

- if (!strcmp(dai_name, sof_dai->cpu_dai_name) &&
- dir == sof_dai->comp_dai.direction) {
- config = sof_dai->dai_config;
+ sof_dai = swidget->private;

- if (!config) {
- dev_err(hda_stream->sdev->dev,
- "error: no config for DAI %s\n",
- sof_dai->name);
- return -EINVAL;
- }
+ if (!sof_dai || !sof_dai->dai_config) {
+ dev_err(swidget->scomp->dev, "error: No config for DAI %s\n", w->name);
+ return NULL;
+ }

- /* update config with stream tag */
- config->hda.link_dma_ch = channel;
+ config = &sof_dai->dai_config[sof_dai->current_config];

- /* send IPC */
- ret = sof_ipc_tx_message(hda_stream->sdev->ipc,
- config->hdr.cmd,
- config,
- config->hdr.size,
- &reply, sizeof(reply));
+ /* update config with stream tag */
+ config->hda.link_dma_ch = channel;

- if (ret < 0)
- dev_err(hda_stream->sdev->dev,
- "error: failed to set dai config for %s\n",
- sof_dai->name);
- return ret;
- }
+ 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)
+{
+ struct snd_sof_dev *sdev = hda_stream->sdev;
+ struct sof_ipc_dai_config *config;
+
+ config = hda_dai_update_config(w, channel);
+ if (!config) {
+ dev_err(sdev->dev, "error: no config for DAI %s\n", w->name);
+ return -ENOENT;
}

- return -EINVAL;
+ /* set up/free DAI widget and send DAI_CONFIG IPC */
+ if (widget_setup)
+ return hda_ctrl_dai_widget_setup(w);
+
+ return hda_ctrl_dai_widget_free(w);
}

static int hda_link_hw_params(struct snd_pcm_substream *substream,
@@ -211,6 +232,7 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
struct sof_intel_hda_stream *hda_stream;
struct hda_pipe_params p_params = {0};
+ struct snd_soc_dapm_widget *w;
struct hdac_ext_link *link;
int stream_tag;
int ret;
@@ -229,9 +251,13 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,

hda_stream = hstream_to_sof_hda_stream(link_dev);

- /* update the DSP with the new tag */
- ret = hda_link_config_ipc(hda_stream, dai->name, stream_tag - 1,
- substream->stream);
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = dai->playback_widget;
+ else
+ w = dai->capture_widget;
+
+ /* set up the DAI widget and send the DAI_CONFIG with the new tag */
+ ret = hda_link_dai_widget_update(hda_stream, w, stream_tag - 1, true);
if (ret < 0)
return ret;

@@ -287,6 +313,7 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream,
snd_soc_dai_get_dma_data(dai, substream);
struct sof_intel_hda_stream *hda_stream;
struct snd_soc_pcm_runtime *rtd;
+ struct snd_soc_dapm_widget *w;
struct hdac_ext_link *link;
struct hdac_stream *hstream;
struct hdac_bus *bus;
@@ -321,12 +348,16 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream,
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = dai->playback_widget;
+ else
+ w = dai->capture_widget;
+
/*
* clear link DMA channel. It will be assigned when
* hw_params is set up again after resume.
*/
- ret = hda_link_config_ipc(hda_stream, dai->name,
- DMA_CHAN_INVALID, substream->stream);
+ ret = hda_link_config_ipc(hda_stream, w, DMA_CHAN_INVALID);
if (ret < 0)
return ret;

@@ -357,6 +388,7 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream,
struct hdac_stream *hstream;
struct snd_soc_pcm_runtime *rtd;
struct hdac_ext_stream *link_dev;
+ struct snd_soc_dapm_widget *w;
int ret;

hstream = substream->runtime->private_data;
@@ -372,9 +404,13 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream,

hda_stream = hstream_to_sof_hda_stream(link_dev);

- /* free the link DMA channel in the FW */
- ret = hda_link_config_ipc(hda_stream, dai->name, DMA_CHAN_INVALID,
- substream->stream);
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = dai->playback_widget;
+ else
+ w = dai->capture_widget;
+
+ /* free the link DMA channel in the FW and the DAI widget */
+ ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
if (ret < 0)
return ret;

@@ -406,47 +442,51 @@ static const struct snd_soc_dai_ops hda_link_dai_ops = {

#endif

-static int ssp_dai_hw_params(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *params,
- struct snd_soc_dai *dai)
+static int ssp_dai_setup_or_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai,
+ bool setup)
{
- struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
- struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME);
- struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
- struct sof_ipc_fw_version *v = &sdev->fw_ready.version;
- struct sof_ipc_dai_config *config;
- struct snd_sof_dai *sof_dai;
- struct sof_ipc_reply reply;
- int ret;
+ struct snd_soc_component *component;
+ struct snd_sof_widget *swidget;
+ struct snd_soc_dapm_widget *w;
+ struct sof_ipc_fw_version *v;
+ struct snd_sof_dev *sdev;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = dai->playback_widget;
+ else
+ w = dai->capture_widget;
+
+ swidget = w->dobj.private;
+ component = swidget->scomp;
+ sdev = snd_soc_component_get_drvdata(component);
+ v = &sdev->fw_ready.version;

/* DAI_CONFIG IPC during hw_params is not supported in older firmware */
if (v->abi_version < SOF_ABI_VER(3, 18, 0))
return 0;

- list_for_each_entry(sof_dai, &sdev->dai_list, list) {
- if (!sof_dai->cpu_dai_name || !sof_dai->dai_config)
- continue;
-
- if (!strcmp(dai->name, sof_dai->cpu_dai_name) &&
- substream->stream == sof_dai->comp_dai.direction) {
- config = &sof_dai->dai_config[sof_dai->current_config];
+ if (setup)
+ return hda_ctrl_dai_widget_setup(w);

- /* send IPC */
- ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config,
- config->hdr.size, &reply, sizeof(reply));
+ return hda_ctrl_dai_widget_free(w);
+}

- if (ret < 0)
- dev_err(sdev->dev, "error: failed to set DAI config for %s\n",
- sof_dai->name);
- return ret;
- }
- }
+static int ssp_dai_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ return ssp_dai_setup_or_free(substream, dai, true);
+}

- return 0;
+static int ssp_dai_hw_free(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ return ssp_dai_setup_or_free(substream, dai, false);
}

static const struct snd_soc_dai_ops ssp_dai_ops = {
.hw_params = ssp_dai_hw_params,
+ .hw_free = ssp_dai_hw_free,
};

/*
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index c11e4c14d875..93305d389ff6 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -52,6 +52,86 @@ static const struct sof_intel_dsp_desc
return chip_info;
}

+int hda_ctrl_dai_widget_setup(struct snd_soc_dapm_widget *w)
+{
+ struct snd_sof_widget *swidget = w->dobj.private;
+ struct snd_soc_component *component = swidget->scomp;
+ struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
+ struct sof_ipc_dai_config *config;
+ struct snd_sof_dai *sof_dai;
+ struct sof_ipc_reply reply;
+ int ret;
+
+ sof_dai = swidget->private;
+
+ if (!sof_dai || !sof_dai->dai_config) {
+ dev_err(sdev->dev, "No config for DAI %s\n", w->name);
+ return -EINVAL;
+ }
+
+ config = &sof_dai->dai_config[sof_dai->current_config];
+
+ /*
+ * For static pipelines, the DAI widget would already be set up and calling
+ * sof_widget_setup() simply returns without doing anything.
+ * For dynamic pipelines, the DAI widget will be set up now.
+ */
+ ret = sof_widget_setup(sdev, swidget);
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: failed setting up DAI widget %s\n", w->name);
+ return ret;
+ }
+
+ /* send DAI_CONFIG IPC */
+ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
+ &reply, sizeof(reply));
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: failed setting DAI config for %s\n", w->name);
+ return ret;
+ }
+
+ sof_dai->configured = true;
+
+ return 0;
+}
+
+int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w)
+{
+ struct snd_sof_widget *swidget = w->dobj.private;
+ struct snd_soc_component *component = swidget->scomp;
+ struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
+ struct sof_ipc_dai_config *config;
+ struct snd_sof_dai *sof_dai;
+ struct sof_ipc_reply reply;
+ int ret;
+
+ sof_dai = swidget->private;
+
+ if (!sof_dai || !sof_dai->dai_config) {
+ dev_err(sdev->dev, "error: No config to free DAI %s\n", w->name);
+ return -EINVAL;
+ }
+
+ /* nothing to do if hw_free() is called without restarting the stream after resume. */
+ if (!sof_dai->configured)
+ return 0;
+
+ config = &sof_dai->dai_config[sof_dai->current_config];
+
+ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
+ &reply, sizeof(reply));
+ if (ret < 0)
+ dev_err(sdev->dev, "error: failed resetting DAI config for %s\n", w->name);
+
+ /*
+ * Reset the configured_flag and free the widget even if the IPC fails to keep
+ * the widget use_count balanced
+ */
+ sof_dai->configured = false;
+
+ return sof_widget_free(sdev, swidget);
+}
+
#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)

/*
@@ -64,67 +144,70 @@ static int sdw_clock_stop_quirks = SDW_INTEL_CLK_STOP_BUS_RESET;
module_param(sdw_clock_stop_quirks, int, 0444);
MODULE_PARM_DESC(sdw_clock_stop_quirks, "SOF SoundWire clock stop quirks");

+static int sdw_dai_config_ipc(struct snd_sof_dev *sdev,
+ struct snd_soc_dapm_widget *w,
+ int link_id, int alh_stream_id, int dai_id, bool setup)
+{
+ struct snd_sof_widget *swidget = w->dobj.private;
+ struct sof_ipc_dai_config *config;
+ struct snd_sof_dai *sof_dai;
+
+ if (!swidget) {
+ dev_err(sdev->dev, "error: No private data for widget %s\n", w->name);
+ return -EINVAL;
+ }
+
+ sof_dai = swidget->private;
+
+ if (!sof_dai || !sof_dai->dai_config) {
+ dev_err(sdev->dev, "error: No config for DAI %s\n", w->name);
+ return -EINVAL;
+ }
+
+ config = &sof_dai->dai_config[sof_dai->current_config];
+
+ /* update config with link and stream ID */
+ config->dai_index = (link_id << 8) | dai_id;
+ config->alh.stream_id = alh_stream_id;
+
+ if (setup)
+ return hda_ctrl_dai_widget_setup(w);
+
+ return hda_ctrl_dai_widget_free(w);
+}
+
static int sdw_params_stream(struct device *dev,
struct sdw_intel_stream_params_data *params_data)
{
+ struct snd_pcm_substream *substream = params_data->substream;
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
struct snd_soc_dai *d = params_data->dai;
- struct sof_ipc_dai_config config;
- struct sof_ipc_reply reply;
- int link_id = params_data->link_id;
- int alh_stream_id = params_data->alh_stream_id;
- int ret;
- u32 size = sizeof(config);
-
- memset(&config, 0, size);
- config.hdr.size = size;
- config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG;
- config.type = SOF_DAI_INTEL_ALH;
- config.dai_index = (link_id << 8) | (d->id);
- config.alh.stream_id = alh_stream_id;
-
- /* send message to DSP */
- ret = sof_ipc_tx_message(sdev->ipc,
- config.hdr.cmd, &config, size, &reply,
- sizeof(reply));
- if (ret < 0) {
- dev_err(sdev->dev,
- "error: failed to set DAI hw_params for link %d dai->id %d ALH %d\n",
- link_id, d->id, alh_stream_id);
- }
+ struct snd_soc_dapm_widget *w;

- return ret;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = d->playback_widget;
+ else
+ w = d->capture_widget;
+
+ return sdw_dai_config_ipc(sdev, w, params_data->link_id, params_data->alh_stream_id,
+ d->id, true);
}

static int sdw_free_stream(struct device *dev,
struct sdw_intel_stream_free_data *free_data)
{
+ struct snd_pcm_substream *substream = free_data->substream;
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
struct snd_soc_dai *d = free_data->dai;
- struct sof_ipc_dai_config config;
- struct sof_ipc_reply reply;
- int link_id = free_data->link_id;
- int ret;
- u32 size = sizeof(config);
-
- memset(&config, 0, size);
- config.hdr.size = size;
- config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG;
- config.type = SOF_DAI_INTEL_ALH;
- config.dai_index = (link_id << 8) | d->id;
- config.alh.stream_id = 0xFFFF; /* invalid value on purpose */
-
- /* send message to DSP */
- ret = sof_ipc_tx_message(sdev->ipc,
- config.hdr.cmd, &config, size, &reply,
- sizeof(reply));
- if (ret < 0) {
- dev_err(sdev->dev,
- "error: failed to free stream for link %d dai->id %d\n",
- link_id, d->id);
- }
+ struct snd_soc_dapm_widget *w;

- return ret;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ w = d->playback_widget;
+ else
+ w = d->capture_widget;
+
+ /* send invalid stream_id */
+ return sdw_dai_config_ipc(sdev, w, free_data->link_id, 0xFFFF, d->id, false);
}

static const struct sdw_intel_ops sdw_callback = {
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 087fa06d5210..9da8ba0ed38d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -733,4 +733,9 @@ void hda_set_mach_params(const struct snd_soc_acpi_mach *mach,
/* PCI driver selection and probe */
int hda_pci_intel_probe(struct pci_dev *pci, const struct pci_device_id *pci_id);

+struct snd_sof_dai;
+struct sof_ipc_dai_config;
+int hda_ctrl_dai_widget_setup(struct snd_soc_dapm_widget *w);
+int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w);
+
#endif
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 7a4aaabf091e..bf5e7c7019a5 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -163,6 +163,7 @@ int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
return -ENOMEM;

dai = swidget->private;
+ dai->configured = false;
memcpy(comp, &dai->comp_dai, sizeof(struct sof_ipc_comp_dai));

/* append extended data to the end of the component */
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index 6ac623137026..d358d455da1e 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -130,11 +130,11 @@ struct snd_sof_route {
struct snd_sof_dai {
struct snd_soc_component *scomp;
const char *name;
- const char *cpu_dai_name;

struct sof_ipc_comp_dai comp_dai;
int number_configs;
int current_config;
+ bool configured; /* DAI configured during BE hw_params */
struct sof_ipc_dai_config *dai_config;
struct list_head list; /* list in sdev dai list */
};
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index b996b89f2920..d8eb238e5229 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2756,9 +2756,6 @@ static int sof_set_dai_config_multi(struct snd_sof_dev *sdev, u32 size,
if (!dai->dai_config)
return -ENOMEM;

- /* set cpu_dai_name */
- dai->cpu_dai_name = link->cpus->dai_name;
-
found = 1;
}
}
--
2.27.0

2021-09-17 22:28:56

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 03/12] ASoC: SOF: topology: Add new token for dynamic pipeline

From: Ranjani Sridharan <[email protected]>

Today, we set up all widgets required for all PCM streams
at the time of topology parsing even if they are not
used. An optimization would be to only set up the widgets
required for currently active PCM streams. This would give
the FW the opportunity to power gate unused memory blocks,
thereby saving power.

For dynamic pipelines, the widgets in the connected DAPM path
for each PCM will need to be set up at runtime. This patch
introduces a new token, DYNAMIC_PIPELINE, for scheduler type
widgets that indicate whether a pipeline should be set up
statically during topology load or at runtime when the PCM is
opened. Introduce a new field called dynamic_pipeline_widget
in struct snd_sof_widget to save the value of the parsed token.

The token is set only for the pipeline (scheduler type)
widget and must be propagated to all widgets in the same
pipeline during topology load. Introduce another field called
pipe_widget in struct snd_sof_widget that saves the pointer to
the scheduler widget with the same pipeline ID as that of the
widget. This field is populated when the pipeline completion
callback is invoked during topology loading.

Signed-off-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
include/uapi/sound/sof/tokens.h | 1 +
sound/soc/sof/sof-audio.h | 13 +++++++
sound/soc/sof/topology.c | 62 ++++++++++++++++++++++++++++++++-
3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/include/uapi/sound/sof/tokens.h b/include/uapi/sound/sof/tokens.h
index a642bf30c027..02b71a8deea4 100644
--- a/include/uapi/sound/sof/tokens.h
+++ b/include/uapi/sound/sof/tokens.h
@@ -51,6 +51,7 @@
#define SOF_TKN_SCHED_CORE 203
#define SOF_TKN_SCHED_FRAMES 204
#define SOF_TKN_SCHED_TIME_DOMAIN 205
+#define SOF_TKN_SCHED_DYNAMIC_PIPELINE 206

/* volume */
#define SOF_TKN_VOLUME_RAMP_STEP_TYPE 250
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index 78a4a0c90a29..4a1c38c5618d 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -81,6 +81,8 @@ struct snd_sof_control {
bool comp_data_dirty;
};

+struct snd_sof_widget;
+
/* ASoC SOF DAPM widget */
struct snd_sof_widget {
struct snd_soc_component *scomp;
@@ -90,8 +92,19 @@ struct snd_sof_widget {
int core;
int id;

+ /*
+ * Flag indicating if the widget should be set up dynamically when a PCM is opened.
+ * This flag is only set for the scheduler type widget in topology. During topology
+ * loading, this flag is propagated to all the widgets belonging to the same pipeline.
+ * When this flag is not set, a widget is set up at the time of topology loading
+ * and retained until the DSP enters D3. It will need to be set up again when resuming
+ * from D3.
+ */
+ bool dynamic_pipeline_widget;
+
struct snd_soc_dapm_widget *widget;
struct list_head list; /* list in sdev widget list */
+ struct snd_sof_widget *pipe_widget;

/* extended data for UUID components */
struct sof_ipc_comp_ext comp_ext;
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index d8f7b1edefc3..60d1db6a9193 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -572,6 +572,12 @@ static const struct sof_topology_token sched_tokens[] = {
offsetof(struct sof_ipc_pipe_new, time_domain), 0},
};

+static const struct sof_topology_token pipeline_tokens[] = {
+ {SOF_TKN_SCHED_DYNAMIC_PIPELINE, SND_SOC_TPLG_TUPLE_TYPE_BOOL, get_token_u16,
+ offsetof(struct snd_sof_widget, dynamic_pipeline_widget), 0},
+
+};
+
/* volume */
static const struct sof_topology_token volume_tokens[] = {
{SOF_TKN_VOLUME_RAMP_STEP_TYPE, SND_SOC_TPLG_TUPLE_TYPE_WORD,
@@ -1765,6 +1771,15 @@ static int sof_widget_load_pipeline(struct snd_soc_component *scomp, int index,
goto err;
}

+ ret = sof_parse_tokens(scomp, swidget, pipeline_tokens,
+ ARRAY_SIZE(pipeline_tokens), private->array,
+ le32_to_cpu(private->size));
+ if (ret != 0) {
+ dev_err(scomp->dev, "error: parse dynamic pipeline token failed %d\n",
+ private->size);
+ goto err;
+ }
+
dev_dbg(scomp->dev, "pipeline %s: period %d pri %d mips %d core %d frames %d\n",
swidget->widget->name, pipeline->period, pipeline->priority,
pipeline->period_mips, pipeline->core, pipeline->frames_per_sched);
@@ -3567,11 +3582,45 @@ int snd_sof_complete_pipeline(struct device *dev,
return 1;
}

+/**
+ * sof_set_pipe_widget - Set pipe_widget for a component
+ * @sdev: pointer to struct snd_sof_dev
+ * @pipe_widget: pointer to struct snd_sof_widget of type snd_soc_dapm_scheduler
+ * @swidget: pointer to struct snd_sof_widget that has the same pipeline ID as @pipe_widget
+ *
+ * Return: 0 if successful, -EINVAL on error.
+ * The function checks if @swidget is associated with any volatile controls. If so, setting
+ * the dynamic_pipeline_widget is disallowed.
+ */
+static int sof_set_pipe_widget(struct snd_sof_dev *sdev, struct snd_sof_widget *pipe_widget,
+ struct snd_sof_widget *swidget)
+{
+ struct snd_sof_control *scontrol;
+
+ if (pipe_widget->dynamic_pipeline_widget) {
+ /* dynamic widgets cannot have volatile kcontrols */
+ list_for_each_entry(scontrol, &sdev->kcontrol_list, list)
+ if (scontrol->comp_id == swidget->comp_id &&
+ (scontrol->access & SNDRV_CTL_ELEM_ACCESS_VOLATILE)) {
+ dev_err(sdev->dev,
+ "error: volatile control found for dynamic widget %s\n",
+ swidget->widget->name);
+ return -EINVAL;
+ }
+ }
+
+ /* set the pipe_widget and apply the dynamic_pipeline_widget_flag */
+ swidget->pipe_widget = pipe_widget;
+ swidget->dynamic_pipeline_widget = pipe_widget->dynamic_pipeline_widget;
+
+ return 0;
+}
+
/* completion - called at completion of firmware loading */
static int sof_complete(struct snd_soc_component *scomp)
{
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
- struct snd_sof_widget *swidget;
+ struct snd_sof_widget *swidget, *comp_swidget;
int ret;

/* some widget types require completion notificattion */
@@ -3586,6 +3635,17 @@ static int sof_complete(struct snd_soc_component *scomp)
return ret;

swidget->complete = ret;
+
+ /*
+ * Apply the dynamic_pipeline_widget flag and set the pipe_widget field
+ * for all widgets that have the same pipeline ID as the scheduler widget
+ */
+ list_for_each_entry_reverse(comp_swidget, &sdev->widget_list, list)
+ if (comp_swidget->pipeline_id == swidget->pipeline_id) {
+ ret = sof_set_pipe_widget(sdev, swidget, comp_swidget);
+ if (ret < 0)
+ return ret;
+ }
break;
default:
break;
--
2.27.0

2021-09-17 22:30:09

by Daniel Baluta (OSS)

[permalink] [raw]
Subject: [PATCH v2 09/12] ASoC: SOF: Introduce widget use_count

From: Ranjani Sridharan <[email protected]>

Add a new field, use_count to struct snd_sof_widget to keep track
of the usage count for each widget. Since widgets can belong to
multiple pipelines, this field will ensure that the widget
is setup only when the first pipeline that needs it is started
and freed when the last pipeline that needs it is stopped. There is
no need to protect the widget use_count access as the core already
handles mutual exclusion at the PCM level.
Add a new helper sof_widget_free() to handle freeing the SOF
widgets and export the sof_widget_setup/free() functions.

Signed-off-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Guennadi Liakhovetski <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/sof/ipc.c | 22 +++++++++++
sound/soc/sof/sof-audio.c | 77 ++++++++++++++++++++++++++++++++++++---
sound/soc/sof/sof-audio.h | 4 ++
3 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index 5d41924f37a6..ecb424161e80 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -719,9 +719,31 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
struct sof_ipc_fw_version *v = &ready->version;
struct sof_ipc_ctrl_data_params sparams;
+ struct snd_sof_widget *swidget;
+ bool widget_found = false;
size_t send_bytes;
int err;

+ list_for_each_entry(swidget, &sdev->widget_list, list) {
+ if (swidget->comp_id == scontrol->comp_id) {
+ widget_found = true;
+ break;
+ }
+ }
+
+ if (!widget_found) {
+ dev_err(sdev->dev, "error: can't find widget with id %d\n", scontrol->comp_id);
+ return -EINVAL;
+ }
+
+ /*
+ * Volatile controls should always be part of static pipelines and the widget use_count
+ * would always be > 0 in this case. For the others, just return the cached value if the
+ * widget is not set up.
+ */
+ if (!swidget->use_count)
+ return 0;
+
/* read or write firmware volume */
if (scontrol->readback_offset != 0) {
/* write/read value header via mmaped region */
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 4bed50847f1d..7a4aaabf091e 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -83,7 +83,53 @@ static int sof_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_wi
return 0;
}

-static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
+int sof_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
+{
+ struct sof_ipc_free ipc_free = {
+ .hdr = {
+ .size = sizeof(ipc_free),
+ .cmd = SOF_IPC_GLB_TPLG_MSG,
+ },
+ .id = swidget->comp_id,
+ };
+ struct sof_ipc_reply reply;
+ int ret;
+
+ if (!swidget->private)
+ return 0;
+
+ /* only free when use_count is 0 */
+ if (--swidget->use_count)
+ return 0;
+
+ switch (swidget->id) {
+ case snd_soc_dapm_scheduler:
+ ipc_free.hdr.cmd |= SOF_IPC_TPLG_PIPE_FREE;
+ break;
+ case snd_soc_dapm_buffer:
+ ipc_free.hdr.cmd |= SOF_IPC_TPLG_BUFFER_FREE;
+ break;
+ default:
+ ipc_free.hdr.cmd |= SOF_IPC_TPLG_COMP_FREE;
+ break;
+ }
+
+ ret = sof_ipc_tx_message(sdev->ipc, ipc_free.hdr.cmd, &ipc_free, sizeof(ipc_free),
+ &reply, sizeof(reply));
+ if (ret < 0) {
+ dev_err(sdev->dev, "error: failed to free widget %s\n", swidget->widget->name);
+ swidget->use_count++;
+ return ret;
+ }
+
+ swidget->complete = 0;
+ dev_dbg(sdev->dev, "widget %s freed\n", swidget->widget->name);
+
+ return 0;
+}
+EXPORT_SYMBOL(sof_widget_free);
+
+int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
{
struct sof_ipc_pipe_new *pipeline;
struct sof_ipc_comp_reply r;
@@ -97,11 +143,15 @@ static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swi
if (!swidget->private)
return 0;

+ /* widget already set up */
+ if (++swidget->use_count > 1)
+ return 0;
+
ret = sof_pipeline_core_enable(sdev, swidget);
if (ret < 0) {
dev_err(sdev->dev, "error: failed to enable target core: %d for widget %s\n",
ret, swidget->widget->name);
- return ret;
+ goto use_count_dec;
}

switch (swidget->id) {
@@ -134,7 +184,7 @@ static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swi
}
if (ret < 0) {
dev_err(sdev->dev, "error: failed to load widget %s\n", swidget->widget->name);
- return ret;
+ goto use_count_dec;
}

/* restore kcontrols for widget */
@@ -147,8 +197,13 @@ static int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swi

dev_dbg(sdev->dev, "widget %s setup complete\n", swidget->widget->name);

+ return 0;
+
+use_count_dec:
+ swidget->use_count--;
return ret;
}
+EXPORT_SYMBOL(sof_widget_setup);

/*
* helper to determine if there are only D0i3 compatible
@@ -258,6 +313,9 @@ int sof_set_up_pipelines(struct device *dev)

/* restore pipeline components */
list_for_each_entry_reverse(swidget, &sdev->widget_list, list) {
+ /* reset widget use_count after resuming */
+ swidget->use_count = 0;
+
ret = sof_widget_setup(sdev, swidget);
if (ret < 0)
return ret;
@@ -325,16 +383,23 @@ int sof_set_up_pipelines(struct device *dev)
return 0;
}

-/* This function doesn't free widgets. It only resets the set up status for all routes */
+/*
+ * This function doesn't free widgets. It only resets the set up status for all routes and
+ * use_count for all widgets.
+ */
void sof_tear_down_pipelines(struct device *dev)
{
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+ struct snd_sof_widget *swidget;
struct snd_sof_route *sroute;

/*
- * No need to protect sroute->setup as this function is called only during the suspend
- * callback and all streams should be suspended by then
+ * No need to protect swidget->use_count and sroute->setup as this function is called only
+ * during the suspend callback and all streams should be suspended by then
*/
+ list_for_each_entry(swidget, &sdev->widget_list, list)
+ swidget->use_count = 0;
+
list_for_each_entry(sroute, &sdev->route_list, list)
sroute->setup = false;
}
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index f1f630028c21..6ac623137026 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -89,6 +89,7 @@ struct snd_sof_widget {
int comp_id;
int pipeline_id;
int complete;
+ int use_count; /* use_count will be protected by the PCM mutex held by the core */
int core;
int id;

@@ -252,4 +253,7 @@ bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev);
int sof_machine_register(struct snd_sof_dev *sdev, void *pdata);
void sof_machine_unregister(struct snd_sof_dev *sdev, void *pdata);

+int sof_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget);
+int sof_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget);
+
#endif
--
2.27.0

2021-09-23 13:01:49

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC


>> +static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w,
>> + int channel)
>> {
>> + struct snd_sof_widget *swidget = w->dobj.private;
>> struct sof_ipc_dai_config *config;
>> struct snd_sof_dai *sof_dai;
>> - struct sof_ipc_reply reply;
>> - int ret = 0;
>>
>> - list_for_each_entry(sof_dai, &hda_stream->sdev->dai_list, list) {
>> - if (!sof_dai->cpu_dai_name)
>> - continue;
>> + if (!swidget) {
>> + dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name);
>
> NULL pointer dereference, just return NULL without the print. The caller
> is printing anyways.

yes good catch, we need a v3 with the fixes suggested by Peter in
https://github.com/thesofproject/linux/pull/3171/ applied.

2021-09-23 13:03:01

by Péter Ujfalusi

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC



On 23/09/2021 15:58, Pierre-Louis Bossart wrote:
>
>>> +static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w,
>>> + int channel)
>>> {
>>> + struct snd_sof_widget *swidget = w->dobj.private;
>>> struct sof_ipc_dai_config *config;
>>> struct snd_sof_dai *sof_dai;
>>> - struct sof_ipc_reply reply;
>>> - int ret = 0;
>>>
>>> - list_for_each_entry(sof_dai, &hda_stream->sdev->dai_list, list) {
>>> - if (!sof_dai->cpu_dai_name)
>>> - continue;
>>> + if (!swidget) {
>>> + dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name);
>>
>> NULL pointer dereference, just return NULL without the print. The caller
>> is printing anyways.
>
> yes good catch, we need a v3 with the fixes suggested by Peter in
> https://github.com/thesofproject/linux/pull/3171/ applied.

Only the second patch in the PR is applicable for upstream, but it
should be squashed in for v3.

--
Péter

2021-09-24 07:45:22

by Daniel Baluta

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC

On Thu, Sep 23, 2021 at 4:04 PM Péter Ujfalusi
<[email protected]> wrote:
>
>
>
> On 23/09/2021 15:58, Pierre-Louis Bossart wrote:
> >
> >>> +static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w,
> >>> + int channel)
> >>> {
> >>> + struct snd_sof_widget *swidget = w->dobj.private;
> >>> struct sof_ipc_dai_config *config;
> >>> struct snd_sof_dai *sof_dai;
> >>> - struct sof_ipc_reply reply;
> >>> - int ret = 0;
> >>>
> >>> - list_for_each_entry(sof_dai, &hda_stream->sdev->dai_list, list) {
> >>> - if (!sof_dai->cpu_dai_name)
> >>> - continue;
> >>> + if (!swidget) {
> >>> + dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name);
> >>
> >> NULL pointer dereference, just return NULL without the print. The caller
> >> is printing anyways.
> >
> > yes good catch, we need a v3 with the fixes suggested by Peter in
> > https://github.com/thesofproject/linux/pull/3171/ applied.
>
> Only the second patch in the PR is applicable for upstream, but it
> should be squashed in for v3.

Thanks Peter, will squash this in and send v3.

2021-09-27 09:11:42

by Péter Ujfalusi

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] ASoC: SOF: Intel: hda: make sure DAI widget is set up before IPC

Hi Daniel,

On 24/09/2021 10:42, Daniel Baluta wrote:
> On Thu, Sep 23, 2021 at 4:04 PM Péter Ujfalusi
> <[email protected]> wrote:
>>
>>
>>
>> On 23/09/2021 15:58, Pierre-Louis Bossart wrote:
>>>
>>>>> +static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w,
>>>>> + int channel)
>>>>> {
>>>>> + struct snd_sof_widget *swidget = w->dobj.private;
>>>>> struct sof_ipc_dai_config *config;
>>>>> struct snd_sof_dai *sof_dai;
>>>>> - struct sof_ipc_reply reply;
>>>>> - int ret = 0;
>>>>>
>>>>> - list_for_each_entry(sof_dai, &hda_stream->sdev->dai_list, list) {
>>>>> - if (!sof_dai->cpu_dai_name)
>>>>> - continue;
>>>>> + if (!swidget) {
>>>>> + dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name);
>>>>
>>>> NULL pointer dereference, just return NULL without the print. The caller
>>>> is printing anyways.
>>>
>>> yes good catch, we need a v3 with the fixes suggested by Peter in
>>> https://github.com/thesofproject/linux/pull/3171/ applied.
>>
>> Only the second patch in the PR is applicable for upstream, but it
>> should be squashed in for v3.
>
> Thanks Peter, will squash this in and send v3.

As we discussed, I'll send the v3 with the fix.

Thanks for sending the initial versions upstream!

--
Péter