From 4de1c79020f99ebb4abb09c074c89fb0b74e2960 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 10 May 2019 18:06:32 -0700 Subject: [PATCH 1/7] ASoC: SOF: hda: save handle to sdev in sof_intel_hda_stream Add a snd_sof_dev member to sof_intel_hda_stream. This will be used to access the snd_sof_dev during link hw_params callback. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda-stream.c | 4 ++++ sound/soc/sof/intel/hda.h | 1 + 2 files changed, 5 insertions(+) diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index c92006f894992a..1cd94e7631a844 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -564,6 +564,8 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) if (!hda_stream) return -ENOMEM; + hda_stream->sdev = sdev; + stream = &hda_stream->hda_stream; stream->pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + @@ -617,6 +619,8 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) if (!hda_stream) return -ENOMEM; + hda_stream->sdev = sdev; + stream = &hda_stream->hda_stream; /* we always have DSP support */ diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 455046612b9494..aae568f01c07c4 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -408,6 +408,7 @@ static inline struct hda_bus *sof_to_hbus(struct snd_sof_dev *s) } struct sof_intel_hda_stream { + struct snd_sof_dev *sdev; struct hdac_ext_stream hda_stream; struct sof_intel_stream stream; int hw_params_upon_resume; /* set up hw_params upon resume */ From b8e2520696403f8d639a1d0688d873ab917752ff Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 10 May 2019 18:08:07 -0700 Subject: [PATCH 2/7] ASoC: SOF: topology: add cpu_dai_name for DAIs Add the cpu_dai_name member to snd_sof_dai and save the cpu_dai_name while setting the DAI config. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/sof-priv.h | 1 + sound/soc/sof/topology.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 563623bcaad6e2..ba6556ecc8bab3 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -335,6 +335,7 @@ struct snd_sof_route { struct snd_sof_dai { struct snd_sof_dev *sdev; const char *name; + const char *cpu_dai_name; struct sof_ipc_comp_dai comp_dai; struct sof_ipc_dai_config *dai_config; diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index d0179e797d60a9..b1bc186d4e9066 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2342,6 +2342,9 @@ static int sof_set_dai_config(struct snd_sof_dev *sdev, u32 size, if (!dai->dai_config) return -ENOMEM; + /* set cpu_dai_name */ + dai->cpu_dai_name = link->cpu_dai_name; + found = 1; } } @@ -2606,6 +2609,8 @@ static int sof_link_hda_process(struct snd_sof_dev *sdev, if (!sof_dai->dai_config) return -ENOMEM; + sof_dai->cpu_dai_name = link->cpu_dai_name; + /* send message to DSP */ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, size, From a90ad77dc0edb4db7e14170d5868f46608df7b20 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 28 May 2019 14:42:01 -0700 Subject: [PATCH 3/7] ASoC: SOF: Intel: hda: add new macro hstream_to_sof_hda_stream() Add a new macro to get sof_intel_hda_stream from hdac_ext_stream. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index aae568f01c07c4..53f9cdad809892 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -414,6 +414,9 @@ struct sof_intel_hda_stream { int hw_params_upon_resume; /* set up hw_params upon resume */ }; +#define hstream_to_sof_hda_stream(hstream) \ + container_of(hstream, struct sof_intel_hda_stream, hda_stream) + #define bus_to_sof_hda(bus) \ container_of(bus, struct sof_intel_hda_dev, hbus.core) From 4801d3e934d019691fdd6f8b1bb8284b114723d3 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 10 May 2019 18:12:31 -0700 Subject: [PATCH 4/7] ASoC: SOF: assign link DMA channel at run-time The recommended HDA HW programming sequence for setting the DMA format requires that the link DMA and host DMA channels be coupled before setting the format. This change means that host DMA or link DMA channels be reserved even if only one is used. Statically assigned link DMA channels would mean that all the corresponding host DMA channels will need to be reserved, leaving only a few channels available at run-time. So, the suggestion here is to switch to dynamically assigning both host DMA channels and link DMA channels are run-time. The host DMA channel is assigned when the pcm is opened as before. While choosing the link DMA channel, if the host DMA channel corresponding to the link DMA channel is already taken, the proposed method checks to make sure that the BE is connected to the FE that has been assigned this host DMA channel. Once the link DMA channel is assigned, an IPC is sent to the DSP to set the link DMA channel. The link DMA channel is freed during hw_free() and also in the SUSPEND trigger callback. It will be re-assigned when hw_params are set upon resume. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda-dai.c | 284 ++++++++++++++++++++++------------ sound/soc/sof/sof-priv.h | 2 + sound/soc/sof/topology.c | 51 +----- 3 files changed, 189 insertions(+), 148 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index e1decf25aeace3..c270fd7a08784c 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -30,62 +30,84 @@ struct hda_pipe_params { }; /* - * Unlike GP dma, there is a set of stream registers in hda controller - * to control the link dma channels. Each register controls one link - * dma channel and the relation is fixed. To make sure FW uses correct - * link dma channels, host allocates stream registers and sends the - * corresponding link dma channels to FW to allocate link dma channel - * - * FIXME: this API is abused in the sense that tx_num and rx_num are - * passed as arguments, not returned. We need to find a better way to - * retrieve the stream tag allocated for the link DMA + * This function checks if the host dma channel corresponding + * to the link DMA stream_tag argument is assigned to one + * of the FEs connected to the BE DAI. */ -static int hda_link_dma_get_channels(struct snd_soc_dai *dai, - unsigned int *tx_num, - unsigned int *tx_slot, - unsigned int *rx_num, - unsigned int *rx_slot) +static bool hda_check_fes(struct snd_soc_pcm_runtime *rtd, + int dir, int stream_tag) { - struct hdac_bus *bus; - struct hdac_ext_stream *stream; - struct snd_pcm_substream substream; - struct snd_sof_dev *sdev = - snd_soc_component_get_drvdata(dai->component); - - bus = sof_to_bus(sdev); - - memset(&substream, 0, sizeof(substream)); - if (*tx_num == 1) { - substream.stream = SNDRV_PCM_STREAM_PLAYBACK; - stream = snd_hdac_ext_stream_assign(bus, &substream, - HDAC_EXT_STREAM_TYPE_LINK); - if (!stream) { - dev_err(bus->dev, "error: failed to find a free hda ext stream for playback"); - return -EBUSY; - } + struct snd_pcm_substream *fe_substream; + struct hdac_stream *fe_hstream; + struct snd_soc_dpcm *dpcm; + + for_each_dpcm_fe(rtd, dir, dpcm) { + fe_substream = snd_soc_dpcm_get_substream(dpcm->fe, dir); + fe_hstream = fe_substream->runtime->private_data; + if (fe_hstream->stream_tag == stream_tag) + return true; + } - snd_soc_dai_set_dma_data(dai, &substream, stream); - *tx_slot = hdac_stream(stream)->stream_tag - 1; + return false; +} + +static struct hdac_ext_stream * + hda_link_stream_assign(struct hdac_bus *bus, + struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct sof_intel_hda_stream *hda_stream; + struct hdac_ext_stream *res = NULL; + struct hdac_stream *stream = NULL; - dev_dbg(bus->dev, "link dma channel %d for playback", *tx_slot); + int stream_dir = substream->stream; + + if (!bus->ppcap) { + dev_err(bus->dev, "stream type not supported\n"); + return NULL; } - if (*rx_num == 1) { - substream.stream = SNDRV_PCM_STREAM_CAPTURE; - stream = snd_hdac_ext_stream_assign(bus, &substream, - HDAC_EXT_STREAM_TYPE_LINK); - if (!stream) { - dev_err(bus->dev, "error: failed to find a free hda ext stream for capture"); - return -EBUSY; + list_for_each_entry(stream, &bus->stream_list, list) { + struct hdac_ext_stream *hstream = + stream_to_hdac_ext_stream(stream); + if (stream->direction != substream->stream) + continue; + + hda_stream = hstream_to_sof_hda_stream(hstream); + + /* check if available */ + if (!hstream->link_locked) { + if (stream->opened) { + /* + * check if the stream tag matches the stream + * tag of one of the connected FEs + */ + if (hda_check_fes(rtd, stream_dir, + stream->stream_tag)) { + res = hstream; + break; + } + } else { + res = hstream; + break; + } } + } - snd_soc_dai_set_dma_data(dai, &substream, stream); - *rx_slot = hdac_stream(stream)->stream_tag - 1; - - dev_dbg(bus->dev, "link dma channel %d for capture", *rx_slot); + if (res) { + /* + * Decouple host and link DMA. The decoupled flag + * is updated in snd_hdac_ext_stream_decouple(). + */ + if (!res->decoupled) + snd_hdac_ext_stream_decouple(bus, res, true); + spin_lock_irq(&bus->reg_lock); + res->link_locked = 1; + res->link_substream = substream; + spin_unlock_irq(&bus->reg_lock); } - return 0; + return res; } static int hda_link_dma_params(struct hdac_ext_stream *stream, @@ -122,6 +144,51 @@ 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) +{ + 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 (!strcmp(dai_name, sof_dai->cpu_dai_name) && + dir == sof_dai->comp_dai.direction) { + config = sof_dai->dai_config; + + if (!config) { + dev_err(hda_stream->sdev->dev, + "error: no config for DAI %s\n", + sof_dai->name); + return -EINVAL; + } + + /* update config with stream tag */ + config->hda.link_dma_ch = channel; + + /* send IPC */ + ret = sof_ipc_tx_message(hda_stream->sdev->ipc, + config->hdr.cmd, + config, + config->hdr.size, + &reply, sizeof(reply)); + + if (ret < 0) + dev_err(hda_stream->sdev->dev, + "error: failed to set dai config for %s\n", + sof_dai->name); + return ret; + } + } + + return -EINVAL; +} + static int hda_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -135,20 +202,31 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, struct hda_pipe_params p_params = {0}; struct hdac_ext_link *link; int stream_tag; + int ret; - link_dev = snd_soc_dai_get_dma_data(dai, substream); + link_dev = hda_link_stream_assign(bus, substream); + if (!link_dev) + return -EBUSY; + + stream_tag = hdac_stream(link_dev)->stream_tag; + + 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 (ret < 0) + return ret; + + snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev); - hda_stream = container_of(link_dev, struct sof_intel_hda_stream, - hda_stream); hda_stream->hw_params_upon_resume = 0; link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); if (!link) return -EINVAL; - stream_tag = hdac_stream(link_dev)->stream_tag; - - /* set the stream tag in the codec dai dma params */ + /* set the stream tag in the codec dai dma params */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) snd_soc_dai_set_tdm_slot(codec_dai, stream_tag, 0, 0, 0); else @@ -181,8 +259,7 @@ static int hda_link_pcm_prepare(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream); int stream = substream->stream; - hda_stream = container_of(link_dev, struct sof_intel_hda_stream, - hda_stream); + hda_stream = hstream_to_sof_hda_stream(link_dev); /* setup hw_params again only if resuming from system suspend */ if (!hda_stream->hw_params_upon_resume) @@ -199,8 +276,24 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, { struct hdac_ext_stream *link_dev = snd_soc_dai_get_dma_data(dai, substream); + struct sof_intel_hda_stream *hda_stream; + struct snd_soc_pcm_runtime *rtd; + struct hdac_ext_link *link; + struct hdac_stream *hstream; + struct hdac_bus *bus; + int stream_tag; int ret; + hstream = substream->runtime->private_data; + bus = hstream->bus; + rtd = snd_pcm_substream_chip(substream); + + link = snd_hdac_ext_bus_get_link(bus, rtd->codec_dai->component->name); + if (!link) + return -EINVAL; + + hda_stream = hstream_to_sof_hda_stream(link_dev); + dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd); switch (cmd) { case SNDRV_PCM_TRIGGER_RESUME: @@ -217,8 +310,22 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: snd_hdac_ext_link_stream_start(link_dev); break; - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: + /* + * clear and release 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); + if (ret < 0) + return ret; + stream_tag = hdac_stream(link_dev)->stream_tag; + snd_hdac_ext_link_clear_stream_id(link, stream_tag); + snd_hdac_ext_stream_release(link_dev, + HDAC_EXT_STREAM_TYPE_LINK); + + /* fallthrough */ + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_STOP: snd_hdac_ext_link_stream_clear(link_dev); break; @@ -228,62 +335,38 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, return 0; } -/* - * FIXME: This API is also abused since it's used for two purposes. - * when the substream argument is NULL this function is used for cleanups - * that aren't necessarily required, and called explicitly by handling - * ASoC core structures, which is not recommended. - * This part will be reworked in follow-up patches. - */ static int hda_link_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - const char *name; unsigned int stream_tag; + struct sof_intel_hda_stream *hda_stream; struct hdac_bus *bus; struct hdac_ext_link *link; struct hdac_stream *hstream; - struct hdac_ext_stream *stream; struct snd_soc_pcm_runtime *rtd; struct hdac_ext_stream *link_dev; - struct snd_pcm_substream pcm_substream; - - memset(&pcm_substream, 0, sizeof(pcm_substream)); - if (substream) { - hstream = substream->runtime->private_data; - bus = hstream->bus; - rtd = snd_pcm_substream_chip(substream); - link_dev = snd_soc_dai_get_dma_data(dai, substream); - snd_hdac_ext_stream_decouple(bus, link_dev, false); - name = rtd->codec_dai->component->name; - link = snd_hdac_ext_bus_get_link(bus, name); - if (!link) - return -EINVAL; - - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - stream_tag = hdac_stream(link_dev)->stream_tag; - snd_hdac_ext_link_clear_stream_id(link, stream_tag); - } + int ret; - link_dev->link_prepared = 0; - } else { - /* release all hda streams when dai link is unloaded */ - pcm_substream.stream = SNDRV_PCM_STREAM_PLAYBACK; - stream = snd_soc_dai_get_dma_data(dai, &pcm_substream); - if (stream) { - snd_soc_dai_set_dma_data(dai, &pcm_substream, NULL); - snd_hdac_ext_stream_release(stream, - HDAC_EXT_STREAM_TYPE_LINK); - } + hstream = substream->runtime->private_data; + bus = hstream->bus; + rtd = snd_pcm_substream_chip(substream); + link_dev = snd_soc_dai_get_dma_data(dai, substream); + hda_stream = hstream_to_sof_hda_stream(link_dev); - pcm_substream.stream = SNDRV_PCM_STREAM_CAPTURE; - stream = snd_soc_dai_get_dma_data(dai, &pcm_substream); - if (stream) { - snd_soc_dai_set_dma_data(dai, &pcm_substream, NULL); - snd_hdac_ext_stream_release(stream, - HDAC_EXT_STREAM_TYPE_LINK); - } - } + /* free the link DMA channel in the FW */ + ret = hda_link_config_ipc(hda_stream, dai->name, DMA_CHAN_INVALID, + substream->stream); + if (ret < 0) + return ret; + + link = snd_hdac_ext_bus_get_link(bus, rtd->codec_dai->component->name); + if (!link) + return -EINVAL; + + stream_tag = hdac_stream(link_dev)->stream_tag; + snd_hdac_ext_link_clear_stream_id(link, stream_tag); + snd_hdac_ext_stream_release(link_dev, HDAC_EXT_STREAM_TYPE_LINK); + link_dev->link_prepared = 0; return 0; } @@ -293,7 +376,6 @@ static const struct snd_soc_dai_ops hda_link_dai_ops = { .hw_free = hda_link_hw_free, .trigger = hda_link_pcm_trigger, .prepare = hda_link_pcm_prepare, - .get_channel_map = hda_link_dma_get_channels, }; #endif diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index ba6556ecc8bab3..0ef8d88db00682 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -60,6 +60,8 @@ (IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) || \ IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)) +#define DMA_CHAN_INVALID 0xFFFFFFFF + struct snd_sof_dev; struct snd_sof_ipc_msg; struct snd_sof_ipc; diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index b1bc186d4e9066..51eb500b5d00d4 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2573,9 +2573,7 @@ static int sof_link_dmic_load(struct snd_soc_component *scomp, int index, */ static int sof_link_hda_process(struct snd_sof_dev *sdev, struct snd_soc_dai_link *link, - struct sof_ipc_dai_config *config, - int tx_slot, - int rx_slot) + struct sof_ipc_dai_config *config) { struct sof_ipc_reply reply; u32 size = sizeof(*config); @@ -2588,22 +2586,11 @@ static int sof_link_hda_process(struct snd_sof_dev *sdev, continue; if (strcmp(link->name, sof_dai->name) == 0) { - if (sof_dai->comp_dai.direction == - SNDRV_PCM_STREAM_PLAYBACK) { - if (!link->dpcm_playback) - return -EINVAL; - - config->hda.link_dma_ch = tx_slot; - } else { - if (!link->dpcm_capture) - return -EINVAL; - - config->hda.link_dma_ch = rx_slot; - } - config->dai_index = sof_dai->comp_dai.dai_index; found = 1; + config->hda.link_dma_ch = DMA_CHAN_INVALID; + /* save config in dai component */ sof_dai->dai_config = kmemdup(config, size, GFP_KERNEL); if (!sof_dai->dai_config) @@ -2650,10 +2637,6 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index, struct snd_soc_tplg_private *private = &cfg->priv; struct snd_soc_dai *dai; u32 size = sizeof(*config); - u32 tx_num = 0; - u32 tx_slot = 0; - u32 rx_num = 0; - u32 rx_slot = 0; int ret; /* init IPC */ @@ -2679,22 +2662,7 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index, return -EINVAL; } - if (link->dpcm_playback) - tx_num = 1; - - if (link->dpcm_capture) - rx_num = 1; - - ret = snd_soc_dai_get_channel_map(dai, &tx_num, &tx_slot, - &rx_num, &rx_slot); - if (ret < 0) { - dev_err(sdev->dev, "error: failed to get dma channel for HDA%d\n", - config->dai_index); - - return ret; - } - - ret = sof_link_hda_process(sdev, link, config, tx_slot, rx_slot); + ret = sof_link_hda_process(sdev, link, config); if (ret < 0) dev_err(sdev->dev, "error: failed to process hda dai link %s", link->name); @@ -2821,17 +2789,6 @@ static int sof_link_hda_unload(struct snd_sof_dev *sdev, return -EINVAL; } - /* - * FIXME: this call to hw_free is mainly to release the link DMA ID. - * This is abusing the API and handling SOC internals is not - * recommended. This part will be reworked. - */ - if (dai->driver->ops->hw_free) - ret = dai->driver->ops->hw_free(NULL, dai); - if (ret < 0) - dev_err(sdev->dev, "error: failed to free hda resource for %s\n", - link->name); - return ret; } From fb05cd241f14af8970f4889197d441eae458a0c8 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 10 May 2019 18:13:38 -0700 Subject: [PATCH 5/7] ASoC: SOF: hda: reserve host DMA channel for hostless streams Due to the HW programming sequence requirement that the host and link DMA channels need to be coupled/decoupled during pcm hw_params, the host DMA channel corresponding to the link DMA channel in use for hostless streams needs to be reserved. This is achieved by adding a host_reserved flag in the sof_intel_hda_stream structure which is checked when assigning a host DMA channel. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda-dai.c | 11 ++++++++++- sound/soc/sof/intel/hda-stream.c | 10 +++++++++- sound/soc/sof/intel/hda.h | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c270fd7a08784c..a514f9cf5c9a1b 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -75,7 +75,7 @@ static struct hdac_ext_stream * hda_stream = hstream_to_sof_hda_stream(hstream); - /* check if available */ + /* check if link is available */ if (!hstream->link_locked) { if (stream->opened) { /* @@ -89,6 +89,12 @@ static struct hdac_ext_stream * } } else { res = hstream; + + /* + * This must be a hostless stream. + * So reserve the host DMA channel. + */ + hda_stream->host_reserved = 1; break; } } @@ -368,6 +374,9 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, snd_hdac_ext_stream_release(link_dev, HDAC_EXT_STREAM_TYPE_LINK); link_dev->link_prepared = 0; + /* free the host DMA channel reserved by hostless streams */ + hda_stream->host_reserved = 0; + return 0; } diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 1cd94e7631a844..a3f7c91469ece9 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -155,6 +155,7 @@ struct hdac_ext_stream * hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction) { struct hdac_bus *bus = sof_to_bus(sdev); + struct sof_intel_hda_stream *hda_stream; struct hdac_ext_stream *stream = NULL; struct hdac_stream *s; @@ -163,8 +164,15 @@ hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction) /* get an unused stream */ list_for_each_entry(s, &bus->stream_list, list) { if (s->direction == direction && !s->opened) { - s->opened = true; stream = stream_to_hdac_ext_stream(s); + hda_stream = container_of(stream, + struct sof_intel_hda_stream, + hda_stream); + /* check if the host DMA channel is reserved */ + if (hda_stream->host_reserved) + continue; + + s->opened = true; break; } } diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 53f9cdad809892..d50db46d98b5d9 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -412,6 +412,7 @@ struct sof_intel_hda_stream { struct hdac_ext_stream hda_stream; struct sof_intel_stream stream; int hw_params_upon_resume; /* set up hw_params upon resume */ + int host_reserved; /* reserve host DMA channel */ }; #define hstream_to_sof_hda_stream(hstream) \ From 6a6404575808863d2f929cbe06564a5cd83b6b4a Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 20 May 2019 11:16:38 -0700 Subject: [PATCH 6/7] ASoC: SOF: Intel: hda: release link DMA for paused streams during suspend Paused streams do not get suspended when the system enters S3. So, clear and release link DMA channel for such streams in the hda_dsp_set_hw_params_upon_resume() callback. Also, invalidate the link DMA channel in the DAI config before restoring the dai config upon resume. Also, modify the signature for the set_hw_params_upon_resume() op to return an int. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda-dsp.c | 29 ++++++++++++++++++++++++++++- sound/soc/sof/intel/hda.h | 2 +- sound/soc/sof/ops.h | 5 +++-- sound/soc/sof/pm.c | 24 ++++++++++++++++++++---- sound/soc/sof/sof-priv.h | 2 +- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 5b73115a0b7873..17f6054d3d68bd 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -454,18 +454,45 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state) return 0; } -void hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) +int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) { struct hdac_bus *bus = sof_to_bus(sdev); struct sof_intel_hda_stream *hda_stream; struct hdac_ext_stream *stream; struct hdac_stream *s; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + struct snd_soc_pcm_runtime *rtd; + struct hdac_ext_link *link; + const char *name; +#endif + int stream_tag; + /* set internal flag for BE */ list_for_each_entry(s, &bus->stream_list, list) { stream = stream_to_hdac_ext_stream(s); hda_stream = container_of(stream, struct sof_intel_hda_stream, hda_stream); hda_stream->hw_params_upon_resume = 1; + stream_tag = hdac_stream(stream)->stream_tag; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + /* + * clear and release stream. This should already be taken care + * for running streams when the SUSPEND trigger is called. + * But paused streams do not get suspended, so this needs to be + * done explicitly during suspend. + */ + if (stream->link_substream) { + rtd = snd_pcm_substream_chip(stream->link_substream); + name = rtd->codec_dai->component->name; + link = snd_hdac_ext_bus_get_link(bus, name); + if (!link) + return -EINVAL; + snd_hdac_ext_link_clear_stream_id(link, stream_tag); + snd_hdac_ext_stream_release(stream, + HDAC_EXT_STREAM_TYPE_LINK); + } +#endif } + return 0; } diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index d50db46d98b5d9..9126757aa4323a 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -450,7 +450,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state); int hda_dsp_resume(struct snd_sof_dev *sdev); int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev, int state); int hda_dsp_runtime_resume(struct snd_sof_dev *sdev); -void hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev); +int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev); void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags); void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags); void hda_ipc_dump(struct snd_sof_dev *sdev); diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index 80fc3b374c2b3b..a2329735375096 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -134,10 +134,11 @@ static inline int snd_sof_dsp_runtime_suspend(struct snd_sof_dev *sdev, return 0; } -static inline void snd_sof_dsp_hw_params_upon_resume(struct snd_sof_dev *sdev) +static inline int snd_sof_dsp_hw_params_upon_resume(struct snd_sof_dev *sdev) { if (sof_ops(sdev)->set_hw_params_upon_resume) - sof_ops(sdev)->set_hw_params_upon_resume(sdev); + return sof_ops(sdev)->set_hw_params_upon_resume(sdev); + return 0; } static inline int snd_sof_dsp_set_clk(struct snd_sof_dev *sdev, u32 freq) diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 8ef1d51025d8bb..a42f9a45d21b0f 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -153,6 +153,15 @@ static int sof_restore_pipelines(struct snd_sof_dev *sdev) continue; } + /* + * The link DMA channel would be invalidated for running + * streams but not for streams that were in the PAUSED + * state during suspend. So invalidate it here before setting + * the dai config in the DSP. + */ + 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, @@ -204,7 +213,7 @@ static int sof_send_pm_ipc(struct snd_sof_dev *sdev, int cmd) sizeof(pm_ctx), &reply, sizeof(reply)); } -static void sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev) +static int sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev) { struct snd_pcm_substream *substream; struct snd_sof_pcm *spcm; @@ -229,7 +238,7 @@ static void sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev) } /* set internal flag for BE */ - snd_sof_dsp_hw_params_upon_resume(sdev); + return snd_sof_dsp_hw_params_upon_resume(sdev); } #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) @@ -333,8 +342,15 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) snd_sof_release_trace(sdev); /* set restore_stream for all streams during system suspend */ - if (!runtime_suspend) - sof_set_hw_params_upon_resume(sdev); + if (!runtime_suspend) { + ret = sof_set_hw_params_upon_resume(sdev); + if (ret < 0) { + dev_err(sdev->dev, + "error: setting hw_params flag during suspend %d\n", + ret); + return ret; + } + } #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) /* cache debugfs contents during runtime suspend */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 0ef8d88db00682..b4afe7a9bc415a 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -172,7 +172,7 @@ struct snd_sof_dsp_ops { int (*runtime_suspend)(struct snd_sof_dev *sof_dev, int state); /* optional */ int (*runtime_resume)(struct snd_sof_dev *sof_dev); /* optional */ - void (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */ + int (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */ /* DSP clocking */ int (*set_clk)(struct snd_sof_dev *sof_dev, u32 freq); /* optional */ From c1574ab31b04db0766d1752dd248964abc3508d3 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 21 May 2019 00:08:15 -0700 Subject: [PATCH 7/7] ASoC: SOF: hda: couple host and link DMA during FE hw_free Host and link DMA are decoupled during FE hw_params. So, they must be coupled in hw_free if the link DMA channel is idle. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/apl.c | 1 + sound/soc/sof/intel/cnl.c | 1 + sound/soc/sof/intel/hda-stream.c | 20 ++++++++++++++++++++ sound/soc/sof/intel/hda.h | 2 ++ sound/soc/sof/ops.h | 11 +++++++++++ sound/soc/sof/pcm.c | 7 +++++++ sound/soc/sof/sof-priv.h | 4 ++++ 7 files changed, 46 insertions(+) diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index f215d80dce2c3e..43d1c9f31ec4c8 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -61,6 +61,7 @@ const struct snd_sof_dsp_ops sof_apl_ops = { .pcm_open = hda_dsp_pcm_open, .pcm_close = hda_dsp_pcm_close, .pcm_hw_params = hda_dsp_pcm_hw_params, + .pcm_hw_free = hda_dsp_stream_hw_free, .pcm_trigger = hda_dsp_pcm_trigger, .pcm_pointer = hda_dsp_pcm_pointer, diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 9a4927b6b6ae54..35f854d33f0695 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -217,6 +217,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = { .pcm_open = hda_dsp_pcm_open, .pcm_close = hda_dsp_pcm_close, .pcm_hw_params = hda_dsp_pcm_hw_params, + .pcm_hw_free = hda_dsp_stream_hw_free, .pcm_trigger = hda_dsp_pcm_trigger, .pcm_pointer = hda_dsp_pcm_pointer, diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index a3f7c91469ece9..ff6ab0c45d8ea2 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -438,6 +438,26 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, return ret; } +int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream) +{ + struct hdac_stream *stream = substream->runtime->private_data; + struct hdac_ext_stream *link_dev = container_of(stream, + struct hdac_ext_stream, + hstream); + struct hdac_bus *bus = sof_to_bus(sdev); + u32 mask = 0x1 << stream->index; + + spin_lock(&bus->reg_lock); + /* couple host and link DMA if link DMA channel is idle */ + if (!link_dev->link_locked) + snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, + SOF_HDA_REG_PP_PPCTL, mask, 0); + spin_unlock(&bus->reg_lock); + + return 0; +} + irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) { struct hdac_bus *bus = context; diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 9126757aa4323a..430e84e6c8c608 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -466,6 +466,8 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct sof_ipc_stream_params *ipc_params); +int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream); int hda_dsp_pcm_trigger(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, int cmd); snd_pcm_uframes_t hda_dsp_pcm_pointer(struct snd_sof_dev *sdev, diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index a2329735375096..45a3d10911634a 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -287,6 +287,17 @@ snd_sof_pcm_platform_hw_params(struct snd_sof_dev *sdev, return 0; } +/* host stream hw free */ +static inline int +snd_sof_pcm_platform_hw_free(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream) +{ + if (sof_ops(sdev) && sof_ops(sdev)->pcm_hw_free) + return sof_ops(sdev)->pcm_hw_free(sdev, substream); + + return 0; +} + /* host stream trigger */ static inline int snd_sof_pcm_platform_trigger(struct snd_sof_dev *sdev, diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 6dc5f97be0bced..334e9d59b1bafa 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -251,6 +251,13 @@ static int sof_pcm_hw_free(struct snd_pcm_substream *substream) cancel_work_sync(&spcm->stream[substream->stream].period_elapsed_work); + if (ret < 0) + return ret; + + ret = snd_sof_pcm_platform_hw_free(sdev, substream); + if (ret < 0) + dev_err(sdev->dev, "error: platform hw free failed\n"); + return ret; } diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index b4afe7a9bc415a..95be40f9c79dca 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -143,6 +143,10 @@ struct snd_sof_dsp_ops { struct snd_pcm_hw_params *params, struct sof_ipc_stream_params *ipc_params); /* optional */ + /* host stream hw_free */ + int (*pcm_hw_free)(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream); /* optional */ + /* host stream trigger */ int (*pcm_trigger)(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream,