[alsa-devel] [PATCH 00/15] ASoC: SOF: Intel: HDaudio fixes and improvements
This is unfortunately a rather large patchset tied to hardware enablement. While productizing HDAudio+DMIC-based platforms, which is a common design starting with CoffeLake platforms, the SOF team encountered a combination of issues with host/link DMA coupling, interrupt handling and other bits not getting properly cleared. The patches in this series provide significant reliability and usage improvements. They are viewed as mandatory for any product based on HDaudio.
There will be a smaller series to fix HDaudio wake. It's almost ready but didn't make the cut for this week and there's no reason to delay reviews on the main changes further.
The remaining topics to complete are autodetection of the number of microphones, for which an ACPI-based solution exists, and autodetection of which platforms need to rely on a signed firmware, for which DMI quirks will be required. This will be provided later in July (after travel+vacation break for a number of contributors).
Keyon Jie (1): ASoC: SOF: Intel: hda-stream: fix a deadlock with bus->reg_lock
Ranjani Sridharan (9): ASoC: SOF: Intel: hda: save handle to sdev in sof_intel_hda_stream ASoC: SOF: Intel: hda: add new macro hstream_to_sof_hda_stream() ASoC: SOF: topology: add cpu_dai_name for DAIs ASoC: SOF: Intel: hda: assign link DMA channel at run-time ASoC: SOF: Intel: hda: reserve host DMA channel for hostless streams ASoC: SOF: Intel: hda: release link DMA for paused streams during suspend ASoC: SOF: Intel: hda: couple host and link DMA during FE hw_free ASoC: SOF: Intel: hda: modify stream interrupt handler ASoC: SOF: Intel: hda: clear stream status and wakests properly
Zhu Yingjiang (5): ASoC: SOF: Intel: hda: use the SOF defined ppcap functions ASoC: SOF: Intel: hda: add function for hda stop chip ASoC: SOF: Intel: hda: use the defined stop chip in suspend ASoC: SOF: Intel: hda: make sure DMA is start/stop by read the RUN bit ASoC: SOF: Intel: hda: make sure RUN bit setting to 0 during clear stream status
sound/soc/sof/intel/apl.c | 1 + sound/soc/sof/intel/cnl.c | 1 + sound/soc/sof/intel/hda-ctrl.c | 77 +++++++- sound/soc/sof/intel/hda-dai.c | 293 ++++++++++++++++++++----------- sound/soc/sof/intel/hda-dsp.c | 50 ++++-- sound/soc/sof/intel/hda-stream.c | 157 ++++++++++++++--- sound/soc/sof/intel/hda.h | 17 +- sound/soc/sof/ops.h | 16 +- sound/soc/sof/pcm.c | 7 + sound/soc/sof/pm.c | 24 ++- sound/soc/sof/sof-priv.h | 9 +- sound/soc/sof/topology.c | 56 +----- 12 files changed, 495 insertions(+), 213 deletions(-)
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 c92006f89499..1cd94e7631a8 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 6c7dee2627d0..502b0a3c2e3c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -409,6 +409,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 */
On Wed, 12 Jun 2019 12:23:33 -0500 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 c92006f89499..1cd94e7631a8 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;
Just one thing, you have two rather big for loops here, one for capture and other for playback, they are almost identical apart for ~2 lines. Maybe share this code and only put the differences into ifs?
/* we always have DSP support */diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 6c7dee2627d0..502b0a3c2e3c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -409,6 +409,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 */
The patch
ASoC: SOF: Intel: hda: save handle to sdev in sof_intel_hda_stream
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 7623ae793c28cc0928c5d1292542dbb92fc2e9e2 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:33 -0500 Subject: [PATCH] ASoC: SOF: Intel: 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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 c92006f89499..1cd94e7631a8 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 6c7dee2627d0..502b0a3c2e3c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -409,6 +409,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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Add a new macro to get sof_intel_hda_stream from hdac_ext_stream.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- 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 502b0a3c2e3c..376b1ca51e2b 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -415,6 +415,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)
The patch
ASoC: SOF: Intel: hda: add new macro hstream_to_sof_hda_stream()
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From f5dbba9fee801f4678a50d92c785f7f24d4ee2c6 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:34 -0500 Subject: [PATCH] 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 ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 502b0a3c2e3c..376b1ca51e2b 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -415,6 +415,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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Add the cpu_dai_name member to snd_sof_dai and save the cpu_dai_name while setting the DAI config.
The internal SOF representation will have to change at a later point as well when we have multiple CPU dais.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 d2dbca7d57d1..10e061a5c39c 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -337,6 +337,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 aaf459af89d0..178256e338b1 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2360,6 +2360,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->cpus->dai_name; + found = 1; } } @@ -2624,6 +2627,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->cpus->dai_name; + /* send message to DSP */ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, size,
The patch
ASoC: SOF: topology: add cpu_dai_name for DAIs
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 1b7e1956860d7566325502651c6bf14f115cd91d Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:35 -0500 Subject: [PATCH] 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.
The internal SOF representation will have to change at a later point as well when we have multiple CPU dais.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 8760a4694d8f..8c3ac149bbb9 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -337,6 +337,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 aaf459af89d0..178256e338b1 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2360,6 +2360,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->cpus->dai_name; + found = 1; } } @@ -2624,6 +2627,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->cpus->dai_name; + /* send message to DSP */ ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, size,
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 e1decf25aeac..c270fd7a0878 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 10e061a5c39c..f6b8522471c9 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 178256e338b1..432ae343f960 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2591,9 +2591,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); @@ -2606,22 +2604,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) @@ -2667,10 +2654,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 */ @@ -2694,22 +2677,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); @@ -2837,17 +2805,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; }
The patch
ASoC: SOF: Intel: hda: assign link DMA channel at run-time
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From bdf4ad3fd01f5dc53c5d6d3b17afc98cd76d8988 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:36 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: 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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 e1decf25aeac..c270fd7a0878 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 8c3ac149bbb9..b80d93e5df2f 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 178256e338b1..432ae343f960 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2591,9 +2591,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); @@ -2606,22 +2604,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) @@ -2667,10 +2654,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 */ @@ -2694,22 +2677,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); @@ -2837,17 +2805,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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 c270fd7a0878..a514f9cf5c9a 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 1cd94e7631a8..a3f7c91469ec 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 376b1ca51e2b..2862b4b3b07c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -413,6 +413,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) \
The patch
ASoC: SOF: Intel: hda: reserve host DMA channel for hostless streams
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 6b2239e3337b4c46b850078a6fc8f1a70ebe1c1f Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:37 -0500 Subject: [PATCH] ASoC: SOF: Intel: 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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 c270fd7a0878..a514f9cf5c9a 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 1cd94e7631a8..a3f7c91469ec 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 376b1ca51e2b..2862b4b3b07c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -413,6 +413,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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 5b73115a0b78..c6eea3079ab7 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; + int stream_tag; +#endif + /* 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; +#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; + stream_tag = hdac_stream(stream)->stream_tag; + 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 2862b4b3b07c..327621ef5cf3 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -451,7 +451,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 80fc3b374c2b..a23297353750 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 b7843f02ef67..8eeb3a1029f2 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 f6b8522471c9..4d8c06eaf571 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 */
The patch
ASoC: SOF: Intel: hda: release link DMA for paused streams during suspend
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 7077a07a72d38a78040873bbc13a77d1e45f8aa0 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:38 -0500 Subject: [PATCH] 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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 5b73115a0b78..c6eea3079ab7 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; + int stream_tag; +#endif + /* 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; +#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; + stream_tag = hdac_stream(stream)->stream_tag; + 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 2862b4b3b07c..327621ef5cf3 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -451,7 +451,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 80fc3b374c2b..a23297353750 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 b7843f02ef67..8eeb3a1029f2 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 b80d93e5df2f..cf1b047f8cb6 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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 f215d80dce2c..43d1c9f31ec4 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 d59305787fc3..3840f81767fa 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -219,6 +219,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 a3f7c91469ec..ff6ab0c45d8e 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 327621ef5cf3..8812dae9cf7a 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -468,6 +468,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 a23297353750..45a3d1091163 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 6dc5f97be0bc..334e9d59b1ba 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 4d8c06eaf571..e04418615129 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,
The patch
ASoC: SOF: Intel: hda: couple host and link DMA during FE hw_free
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 93146bc22f6131abf5161030f259e4b911d859eb Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:39 -0500 Subject: [PATCH] ASoC: SOF: Intel: 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 ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 f215d80dce2c..43d1c9f31ec4 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 d59305787fc3..3840f81767fa 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -219,6 +219,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 a3f7c91469ec..ff6ab0c45d8e 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 327621ef5cf3..8812dae9cf7a 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -468,6 +468,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 a23297353750..45a3d1091163 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 6dc5f97be0bc..334e9d59b1ba 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 cf1b047f8cb6..58621db4fd31 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,
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Modify the stream interrupt handler to always wake up the IRQ thread if the status register is valid. The IRQ thread performs the check for stream interrupts and RIRB interrupts in a loop to handle the case of missed interrupts when an unsolicited response from the codec is received just before the stream interrupt handler is completed.
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-stream.c | 79 +++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 28 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index ff6ab0c45d8e..d44318040948 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -461,57 +461,40 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) { struct hdac_bus *bus = context; - struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); - u32 stream_mask; + int ret = IRQ_WAKE_THREAD; u32 status;
- if (!pm_runtime_active(bus->dev)) - return IRQ_NONE; - spin_lock(&bus->reg_lock);
status = snd_hdac_chip_readl(bus, INTSTS); - stream_mask = GENMASK(sof_hda->stream_max - 1, 0) | AZX_INT_CTRL_EN; - - /* Not stream interrupt or register inaccessible, ignore it.*/ - if (!(status & stream_mask) || status == 0xffffffff) { - spin_unlock(&bus->reg_lock); - return IRQ_NONE; - } + dev_vdbg(bus->dev, "stream irq, INTSTS status: 0x%x\n", status);
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* clear rirb int */ - status = snd_hdac_chip_readb(bus, RIRBSTS); - if (status & RIRB_INT_MASK) { - if (status & RIRB_INT_RESPONSE) - snd_hdac_bus_update_rirb(bus); - snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); - } -#endif + /* Register inaccessible, ignore it.*/ + if (status == 0xffffffff) + ret = IRQ_NONE;
spin_unlock(&bus->reg_lock);
- return snd_hdac_chip_readl(bus, INTSTS) ? IRQ_WAKE_THREAD : IRQ_HANDLED; + return ret; }
-irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) +static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status) { - struct hdac_bus *bus = context; struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); - u32 status = snd_hdac_chip_readl(bus, INTSTS); struct hdac_stream *s; + bool active = false; u32 sd_status;
- /* check streams */ list_for_each_entry(s, &bus->stream_list, list) { - if (status & (1 << s->index) && s->opened) { + if (status & BIT(s->index) && s->opened) { sd_status = snd_hdac_stream_readb(s, SD_STS);
dev_vdbg(bus->dev, "stream %d status 0x%x\n", s->index, sd_status);
- snd_hdac_stream_writeb(s, SD_STS, SD_INT_MASK); + snd_hdac_stream_writeb(s, SD_STS, sd_status);
+ active = true; if (!s->substream || !s->running || (sd_status & SOF_HDA_CL_DMA_SD_INT_COMPLETE) == 0) @@ -520,8 +503,48 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) /* Inform ALSA only in case not do that with IPC */ if (sof_hda->no_ipc_position) snd_sof_pcm_period_elapsed(s->substream); + } + } + + return active; +}
+irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) +{ + struct hdac_bus *bus = context; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + u32 rirb_status; +#endif + bool active; + u32 status; + int i; + + /* + * Loop 10 times to handle missed interrupts caused by + * unsolicited responses from the codec + */ + for (i = 0, active = true; i < 10 && active; i++) { + spin_lock_irq(&bus->reg_lock); + + status = snd_hdac_chip_readl(bus, INTSTS); + + /* check streams */ + active = hda_dsp_stream_check(bus, status); + + /* check and clear RIRB interrupt */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + if (status & AZX_INT_CTRL_EN) { + rirb_status = snd_hdac_chip_readb(bus, RIRBSTS); + if (rirb_status & RIRB_INT_MASK) { + active = true; + if (rirb_status & RIRB_INT_RESPONSE) + snd_hdac_bus_update_rirb(bus); + snd_hdac_chip_writeb(bus, RIRBSTS, + RIRB_INT_MASK); + } } +#endif + spin_unlock_irq(&bus->reg_lock); }
return IRQ_HANDLED;
The patch
ASoC: SOF: Intel: hda: modify stream interrupt handler
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 6297a0dc4c14a62bea5a9137ceef280cb7a80665 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:40 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: modify stream interrupt handler
Modify the stream interrupt handler to always wake up the IRQ thread if the status register is valid. The IRQ thread performs the check for stream interrupts and RIRB interrupts in a loop to handle the case of missed interrupts when an unsolicited response from the codec is received just before the stream interrupt handler is completed.
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-stream.c | 79 +++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 28 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index ff6ab0c45d8e..d44318040948 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -461,57 +461,40 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) { struct hdac_bus *bus = context; - struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); - u32 stream_mask; + int ret = IRQ_WAKE_THREAD; u32 status;
- if (!pm_runtime_active(bus->dev)) - return IRQ_NONE; - spin_lock(&bus->reg_lock);
status = snd_hdac_chip_readl(bus, INTSTS); - stream_mask = GENMASK(sof_hda->stream_max - 1, 0) | AZX_INT_CTRL_EN; - - /* Not stream interrupt or register inaccessible, ignore it.*/ - if (!(status & stream_mask) || status == 0xffffffff) { - spin_unlock(&bus->reg_lock); - return IRQ_NONE; - } + dev_vdbg(bus->dev, "stream irq, INTSTS status: 0x%x\n", status);
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* clear rirb int */ - status = snd_hdac_chip_readb(bus, RIRBSTS); - if (status & RIRB_INT_MASK) { - if (status & RIRB_INT_RESPONSE) - snd_hdac_bus_update_rirb(bus); - snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); - } -#endif + /* Register inaccessible, ignore it.*/ + if (status == 0xffffffff) + ret = IRQ_NONE;
spin_unlock(&bus->reg_lock);
- return snd_hdac_chip_readl(bus, INTSTS) ? IRQ_WAKE_THREAD : IRQ_HANDLED; + return ret; }
-irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) +static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status) { - struct hdac_bus *bus = context; struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); - u32 status = snd_hdac_chip_readl(bus, INTSTS); struct hdac_stream *s; + bool active = false; u32 sd_status;
- /* check streams */ list_for_each_entry(s, &bus->stream_list, list) { - if (status & (1 << s->index) && s->opened) { + if (status & BIT(s->index) && s->opened) { sd_status = snd_hdac_stream_readb(s, SD_STS);
dev_vdbg(bus->dev, "stream %d status 0x%x\n", s->index, sd_status);
- snd_hdac_stream_writeb(s, SD_STS, SD_INT_MASK); + snd_hdac_stream_writeb(s, SD_STS, sd_status);
+ active = true; if (!s->substream || !s->running || (sd_status & SOF_HDA_CL_DMA_SD_INT_COMPLETE) == 0) @@ -520,8 +503,48 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) /* Inform ALSA only in case not do that with IPC */ if (sof_hda->no_ipc_position) snd_sof_pcm_period_elapsed(s->substream); + } + } + + return active; +}
+irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) +{ + struct hdac_bus *bus = context; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + u32 rirb_status; +#endif + bool active; + u32 status; + int i; + + /* + * Loop 10 times to handle missed interrupts caused by + * unsolicited responses from the codec + */ + for (i = 0, active = true; i < 10 && active; i++) { + spin_lock_irq(&bus->reg_lock); + + status = snd_hdac_chip_readl(bus, INTSTS); + + /* check streams */ + active = hda_dsp_stream_check(bus, status); + + /* check and clear RIRB interrupt */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + if (status & AZX_INT_CTRL_EN) { + rirb_status = snd_hdac_chip_readb(bus, RIRBSTS); + if (rirb_status & RIRB_INT_MASK) { + active = true; + if (rirb_status & RIRB_INT_RESPONSE) + snd_hdac_bus_update_rirb(bus); + snd_hdac_chip_writeb(bus, RIRBSTS, + RIRB_INT_MASK); + } } +#endif + spin_unlock_irq(&bus->reg_lock); }
return IRQ_HANDLED;
From: Keyon Jie yang.jie@linux.intel.com
We should use irq disabled mode when read/write hda registers from thread context, as we need to hold the same bus->reg_lock in interrupt context hda_dsp_stream_interrupt(), otherwise, when we are holding the lock in hda_dsp_stream_hw_free() and the interrupt arrives, we will get deadlock in the interrupt handler.
Error logs like this:
[ 5.603606] CPU0 [ 5.603606] ---- [ 5.603607] lock(&(&bus->reg_lock)->rlock); [ 5.603608] <Interrupt> [ 5.603609] lock(&(&bus->reg_lock)->rlock); [ 5.603610] *** DEADLOCK ***
[ 5.603611] 2 locks held by pulseaudio/2329: [ 5.603612] #0: 000000005fcf26c6 (&card->mutex/1){+.+.}, at: dpcm_fe_dai_hw_free+0x2b/0x110 [snd_soc_core] [ 5.603619] #1: 00000000ef369faf (&rtd->pcm_mutex){+.+.}, at: soc_pcm_hw_free+0x2e/0x1c0 [snd_soc_core]
The fix is simple, let's switch to use spin_lock/unlock_irq().
Reported-by: Xun Zhang xun2.zhang@intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-stream.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index d44318040948..23cff5aca007 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -448,12 +448,12 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, struct hdac_bus *bus = sof_to_bus(sdev); u32 mask = 0x1 << stream->index;
- spin_lock(&bus->reg_lock); + spin_lock_irq(&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); + spin_unlock_irq(&bus->reg_lock);
return 0; }
The patch
ASoC: SOF: Intel: hda-stream: fix a deadlock with bus->reg_lock
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 7fd572e7d317fa51049d623badb8b2874bfd0119 Mon Sep 17 00:00:00 2001
From: Keyon Jie yang.jie@linux.intel.com Date: Wed, 12 Jun 2019 12:23:41 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda-stream: fix a deadlock with bus->reg_lock
We should use irq disabled mode when read/write hda registers from thread context, as we need to hold the same bus->reg_lock in interrupt context hda_dsp_stream_interrupt(), otherwise, when we are holding the lock in hda_dsp_stream_hw_free() and the interrupt arrives, we will get deadlock in the interrupt handler.
Error logs like this:
[ 5.603606] CPU0 [ 5.603606] ---- [ 5.603607] lock(&(&bus->reg_lock)->rlock); [ 5.603608] <Interrupt> [ 5.603609] lock(&(&bus->reg_lock)->rlock); [ 5.603610] *** DEADLOCK ***
[ 5.603611] 2 locks held by pulseaudio/2329: [ 5.603612] #0: 000000005fcf26c6 (&card->mutex/1){+.+.}, at: dpcm_fe_dai_hw_free+0x2b/0x110 [snd_soc_core] [ 5.603619] #1: 00000000ef369faf (&rtd->pcm_mutex){+.+.}, at: soc_pcm_hw_free+0x2e/0x1c0 [snd_soc_core]
The fix is simple, let's switch to use spin_lock/unlock_irq().
Reported-by: Xun Zhang xun2.zhang@intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-stream.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index d44318040948..23cff5aca007 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -448,12 +448,12 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, struct hdac_bus *bus = sof_to_bus(sdev); u32 mask = 0x1 << stream->index;
- spin_lock(&bus->reg_lock); + spin_lock_irq(&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); + spin_unlock_irq(&bus->reg_lock);
return 0; }
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com
Unify ppcap function setup by using SOF common functions for both HDA and non-HDA cases.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index c6eea3079ab7..883134d535b3 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -307,18 +307,14 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) return ret; }
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* disable ppcap interrupt */ - snd_hdac_ext_bus_ppcap_int_enable(bus, false); - snd_hdac_ext_bus_ppcap_enable(bus, false); + hda_dsp_ctrl_ppcap_enable(sdev, false); + hda_dsp_ctrl_ppcap_int_enable(sdev, false);
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* disable hda bus irq and i/o */ snd_hdac_bus_stop_chip(bus); #else - /* disable ppcap interrupt */ - hda_dsp_ctrl_ppcap_enable(sdev, false); - hda_dsp_ctrl_ppcap_int_enable(sdev, false); - /* disable hda bus irq */ snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, @@ -370,10 +366,6 @@ static int hda_resume(struct snd_sof_dev *sdev) bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
hda_dsp_ctrl_misc_clock_gating(sdev, true); - - /* enable ppcap interrupt */ - snd_hdac_ext_bus_ppcap_enable(bus, true); - snd_hdac_ext_bus_ppcap_int_enable(bus, true); #else
hda_dsp_ctrl_misc_clock_gating(sdev, false); @@ -400,11 +392,11 @@ static int hda_resume(struct snd_sof_dev *sdev) SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN);
hda_dsp_ctrl_misc_clock_gating(sdev, true); +#endif
/* enable ppcap interrupt */ hda_dsp_ctrl_ppcap_enable(sdev, true); hda_dsp_ctrl_ppcap_int_enable(sdev, true); -#endif
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* turn off the links that were off before suspend */
The patch
ASoC: SOF: Intel: hda: use the SOF defined ppcap functions
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 24b6ff686fce808346c129bce52836c3339733bf Mon Sep 17 00:00:00 2001
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com Date: Wed, 12 Jun 2019 12:23:42 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: use the SOF defined ppcap functions
Unify ppcap function setup by using SOF common functions for both HDA and non-HDA cases.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-dsp.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index c6eea3079ab7..883134d535b3 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -307,18 +307,14 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) return ret; }
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* disable ppcap interrupt */ - snd_hdac_ext_bus_ppcap_int_enable(bus, false); - snd_hdac_ext_bus_ppcap_enable(bus, false); + hda_dsp_ctrl_ppcap_enable(sdev, false); + hda_dsp_ctrl_ppcap_int_enable(sdev, false);
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* disable hda bus irq and i/o */ snd_hdac_bus_stop_chip(bus); #else - /* disable ppcap interrupt */ - hda_dsp_ctrl_ppcap_enable(sdev, false); - hda_dsp_ctrl_ppcap_int_enable(sdev, false); - /* disable hda bus irq */ snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, @@ -370,10 +366,6 @@ static int hda_resume(struct snd_sof_dev *sdev) bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
hda_dsp_ctrl_misc_clock_gating(sdev, true); - - /* enable ppcap interrupt */ - snd_hdac_ext_bus_ppcap_enable(bus, true); - snd_hdac_ext_bus_ppcap_int_enable(bus, true); #else
hda_dsp_ctrl_misc_clock_gating(sdev, false); @@ -400,11 +392,11 @@ static int hda_resume(struct snd_sof_dev *sdev) SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN);
hda_dsp_ctrl_misc_clock_gating(sdev, true); +#endif
/* enable ppcap interrupt */ hda_dsp_ctrl_ppcap_enable(sdev, true); hda_dsp_ctrl_ppcap_int_enable(sdev, true); -#endif
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* turn off the links that were off before suspend */
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com
Add common hda_dsp_ctrl_stop_chip() function to stop controller with the same function handling both HDA and non-HDA cases. This function disables IRQs and clears status masks. When CONFIG_SND_SOC_SOF_HDA is defined, also disables the CORB/RIRB, and stops i/o.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-ctrl.c | 67 ++++++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 07bc123112c9..688ab8d895a9 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -263,3 +263,70 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset)
return ret; } + +void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) +{ + struct hdac_bus *bus = sof_to_bus(sdev); + struct hdac_stream *stream; + int sd_offset; + + if (!bus->chip_init) + return; + + /* disable interrupts in stream descriptor */ + list_for_each_entry(stream, &bus->stream_list, list) { + sd_offset = SOF_STREAM_SD_OFFSET(stream); + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, + sd_offset + + SOF_HDA_ADSP_REG_CL_SD_CTL, + SOF_HDA_CL_DMA_SD_INT_MASK, + 0); + } + + /* disable SIE for all streams */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, + SOF_HDA_INT_ALL_STREAM, 0); + + /* disable controller CIE and GIE */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, + SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, + 0); + + /* clear stream status */ + list_for_each_entry(stream, &bus->stream_list, list) { + sd_offset = SOF_STREAM_SD_OFFSET(stream); + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, + sd_offset + + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK, + SOF_HDA_CL_DMA_SD_INT_MASK); + } + + /* clear WAKESTS */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK, + SOF_HDA_WAKESTS_INT_MASK); + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + /* clear rirb status */ + snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); +#endif + + /* clear interrupt status register */ + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTSTS, + SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_ALL_STREAM); + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + /* disable CORB/RIRB */ + snd_hdac_bus_stop_cmd_io(bus); +#endif + /* disable position buffer */ + if (bus->posbuf.addr) { + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + SOF_HDA_ADSP_DPLBASE, 0); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + SOF_HDA_ADSP_DPUBASE, 0); + } + + bus->chip_init = false; +} diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 8812dae9cf7a..50653859e0a0 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -538,7 +538,7 @@ int hda_dsp_ctrl_link_reset(struct snd_sof_dev *sdev, bool reset); void hda_dsp_ctrl_misc_clock_gating(struct snd_sof_dev *sdev, bool enable); int hda_dsp_ctrl_clock_power_gating(struct snd_sof_dev *sdev, bool enable); int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset); - +void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev); /* * HDA bus operations. */
The patch
ASoC: SOF: Intel: hda: add function for hda stop chip
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 13063a2ccf79d232889491105a7bd9d552e417fc Mon Sep 17 00:00:00 2001
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com Date: Wed, 12 Jun 2019 12:23:43 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: add function for hda stop chip
Add common hda_dsp_ctrl_stop_chip() function to stop controller with the same function handling both HDA and non-HDA cases. This function disables IRQs and clears status masks. When CONFIG_SND_SOC_SOF_HDA is defined, also disables the CORB/RIRB, and stops i/o.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-ctrl.c | 67 ++++++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 07bc123112c9..688ab8d895a9 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -263,3 +263,70 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset)
return ret; } + +void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) +{ + struct hdac_bus *bus = sof_to_bus(sdev); + struct hdac_stream *stream; + int sd_offset; + + if (!bus->chip_init) + return; + + /* disable interrupts in stream descriptor */ + list_for_each_entry(stream, &bus->stream_list, list) { + sd_offset = SOF_STREAM_SD_OFFSET(stream); + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, + sd_offset + + SOF_HDA_ADSP_REG_CL_SD_CTL, + SOF_HDA_CL_DMA_SD_INT_MASK, + 0); + } + + /* disable SIE for all streams */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, + SOF_HDA_INT_ALL_STREAM, 0); + + /* disable controller CIE and GIE */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, + SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, + 0); + + /* clear stream status */ + list_for_each_entry(stream, &bus->stream_list, list) { + sd_offset = SOF_STREAM_SD_OFFSET(stream); + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, + sd_offset + + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK, + SOF_HDA_CL_DMA_SD_INT_MASK); + } + + /* clear WAKESTS */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK, + SOF_HDA_WAKESTS_INT_MASK); + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + /* clear rirb status */ + snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK); +#endif + + /* clear interrupt status register */ + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTSTS, + SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_ALL_STREAM); + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + /* disable CORB/RIRB */ + snd_hdac_bus_stop_cmd_io(bus); +#endif + /* disable position buffer */ + if (bus->posbuf.addr) { + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + SOF_HDA_ADSP_DPLBASE, 0); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + SOF_HDA_ADSP_DPUBASE, 0); + } + + bus->chip_init = false; +} diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 8812dae9cf7a..50653859e0a0 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -538,7 +538,7 @@ int hda_dsp_ctrl_link_reset(struct snd_sof_dev *sdev, bool reset); void hda_dsp_ctrl_misc_clock_gating(struct snd_sof_dev *sdev, bool enable); int hda_dsp_ctrl_clock_power_gating(struct snd_sof_dev *sdev, bool enable); int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset); - +void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev); /* * HDA bus operations. */
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com
Unify suspend code by using SOF common function hda_dsp_ctrl_stop_chip() which can handle both HDA and non-HDA cases.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 883134d535b3..f2c5a12db930 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -311,15 +311,8 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) hda_dsp_ctrl_ppcap_enable(sdev, false); hda_dsp_ctrl_ppcap_int_enable(sdev, false);
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* disable hda bus irq and i/o */ - snd_hdac_bus_stop_chip(bus); -#else - /* disable hda bus irq */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, - SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, - 0); -#endif + /* disable hda bus irq and streams */ + hda_dsp_ctrl_stop_chip(sdev);
/* disable LP retention mode */ snd_sof_pci_update_bits(sdev, PCI_PGCTL,
The patch
ASoC: SOF: Intel: hda: use the defined stop chip in suspend
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 9a50ee58b8b11300788d41cd1c0f370833424823 Mon Sep 17 00:00:00 2001
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com Date: Wed, 12 Jun 2019 12:23:44 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: use the defined stop chip in suspend
Unify suspend code by using SOF common function hda_dsp_ctrl_stop_chip() which can handle both HDA and non-HDA cases.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-dsp.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 883134d535b3..f2c5a12db930 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -311,15 +311,8 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) hda_dsp_ctrl_ppcap_enable(sdev, false); hda_dsp_ctrl_ppcap_int_enable(sdev, false);
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* disable hda bus irq and i/o */ - snd_hdac_bus_stop_chip(bus); -#else - /* disable hda bus irq */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_INTCTL, - SOF_HDA_INT_CTRL_EN | SOF_HDA_INT_GLOBAL_EN, - 0); -#endif + /* disable hda bus irq and streams */ + hda_dsp_ctrl_stop_chip(sdev);
/* disable LP retention mode */ snd_sof_pci_update_bits(sdev, PCI_PGCTL,
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Stream status and WAKESTS registers need to be cleared by writing to them with snd_sof_dsp_write(). snd_sof_dsp_update_bits() only writes if the value is changed and will result in not clearing the status.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-ctrl.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 688ab8d895a9..ea63f83a509b 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -217,17 +217,14 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) /* clear stream status */ list_for_each_entry(stream, &bus->stream_list, list) { sd_offset = SOF_STREAM_SD_OFFSET(stream); - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, - sd_offset + - SOF_HDA_ADSP_REG_CL_SD_STS, - SOF_HDA_CL_DMA_SD_INT_MASK, - SOF_HDA_CL_DMA_SD_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK); }
/* clear WAKESTS */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, - SOF_HDA_WAKESTS_INT_MASK, - SOF_HDA_WAKESTS_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* clear rirb status */ @@ -295,17 +292,14 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) /* clear stream status */ list_for_each_entry(stream, &bus->stream_list, list) { sd_offset = SOF_STREAM_SD_OFFSET(stream); - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, - sd_offset + - SOF_HDA_ADSP_REG_CL_SD_STS, - SOF_HDA_CL_DMA_SD_INT_MASK, - SOF_HDA_CL_DMA_SD_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK); }
/* clear WAKESTS */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, - SOF_HDA_WAKESTS_INT_MASK, - SOF_HDA_WAKESTS_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* clear rirb status */
The patch
ASoC: SOF: Intel: hda: clear stream status and wakests properly
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 3a39e0eae80919a15831f66324d4530454b4f495 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com Date: Wed, 12 Jun 2019 12:23:45 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: clear stream status and wakests properly
Stream status and WAKESTS registers need to be cleared by writing to them with snd_sof_dsp_write(). snd_sof_dsp_update_bits() only writes if the value is changed and will result in not clearing the status.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-ctrl.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 688ab8d895a9..ea63f83a509b 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -217,17 +217,14 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) /* clear stream status */ list_for_each_entry(stream, &bus->stream_list, list) { sd_offset = SOF_STREAM_SD_OFFSET(stream); - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, - sd_offset + - SOF_HDA_ADSP_REG_CL_SD_STS, - SOF_HDA_CL_DMA_SD_INT_MASK, - SOF_HDA_CL_DMA_SD_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK); }
/* clear WAKESTS */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, - SOF_HDA_WAKESTS_INT_MASK, - SOF_HDA_WAKESTS_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* clear rirb status */ @@ -295,17 +292,14 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev) /* clear stream status */ list_for_each_entry(stream, &bus->stream_list, list) { sd_offset = SOF_STREAM_SD_OFFSET(stream); - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, - sd_offset + - SOF_HDA_ADSP_REG_CL_SD_STS, - SOF_HDA_CL_DMA_SD_INT_MASK, - SOF_HDA_CL_DMA_SD_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, + sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, + SOF_HDA_CL_DMA_SD_INT_MASK); }
/* clear WAKESTS */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, - SOF_HDA_WAKESTS_INT_MASK, - SOF_HDA_WAKESTS_INT_MASK); + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, SOF_HDA_WAKESTS, + SOF_HDA_WAKESTS_INT_MASK);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* clear rirb status */
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com
As per the HW recommendation, after setting the RUN bit (start as 1, stop as 0), software must read the bit back to make sure the bit is set right, before modifying related control registers/re-starting the DMA engine.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-stream.c | 22 ++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 6 ++++++ 2 files changed, 28 insertions(+)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 23cff5aca007..13d114993f96 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -217,6 +217,9 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, { struct hdac_stream *hstream = &stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); + u32 dma_start = SOF_HDA_SD_CTL_DMA_START; + int ret; + u32 run;
/* cmd must be for audio stream */ switch (cmd) { @@ -234,6 +237,16 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, SOF_HDA_SD_CTL_DMA_START | SOF_HDA_CL_DMA_SD_INT_MASK);
+ ret = snd_sof_dsp_read_poll_timeout(sdev, + HDA_DSP_HDA_BAR, + sd_offset, run, + ((run & dma_start) == dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + hstream->running = true; break; case SNDRV_PCM_TRIGGER_SUSPEND: @@ -244,6 +257,15 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, SOF_HDA_SD_CTL_DMA_START | SOF_HDA_CL_DMA_SD_INT_MASK, 0x0);
+ ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK); diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 50653859e0a0..73d7cc08afc2 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -159,6 +159,12 @@ #define HDA_DSP_MBOX_UPLINK_OFFSET 0x81000
#define HDA_DSP_STREAM_RESET_TIMEOUT 300 +/* + * Timeout in us, for setting the stream RUN bit, during + * start/stop the stream. The timeout expires if new RUN bit + * value cannot be read back within the specified time. + */ +#define HDA_DSP_STREAM_RUN_TIMEOUT 300 #define HDA_DSP_CL_TRIGGER_TIMEOUT 300
#define HDA_DSP_SPIB_ENABLE 1
The patch
ASoC: SOF: Intel: hda: make sure DMA is start/stop by read the RUN bit
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 7bcaf0f2cdfacca2226eee8895f64bc019d5a8be Mon Sep 17 00:00:00 2001
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com Date: Wed, 12 Jun 2019 12:23:46 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: make sure DMA is start/stop by read the RUN bit
As per the HW recommendation, after setting the RUN bit (start as 1, stop as 0), software must read the bit back to make sure the bit is set right, before modifying related control registers/re-starting the DMA engine.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-stream.c | 22 ++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 6 ++++++ 2 files changed, 28 insertions(+)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 23cff5aca007..13d114993f96 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -217,6 +217,9 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, { struct hdac_stream *hstream = &stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); + u32 dma_start = SOF_HDA_SD_CTL_DMA_START; + int ret; + u32 run;
/* cmd must be for audio stream */ switch (cmd) { @@ -234,6 +237,16 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, SOF_HDA_SD_CTL_DMA_START | SOF_HDA_CL_DMA_SD_INT_MASK);
+ ret = snd_sof_dsp_read_poll_timeout(sdev, + HDA_DSP_HDA_BAR, + sd_offset, run, + ((run & dma_start) == dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + hstream->running = true; break; case SNDRV_PCM_TRIGGER_SUSPEND: @@ -244,6 +257,15 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, SOF_HDA_SD_CTL_DMA_START | SOF_HDA_CL_DMA_SD_INT_MASK, 0x0);
+ ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK); diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 50653859e0a0..73d7cc08afc2 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -159,6 +159,12 @@ #define HDA_DSP_MBOX_UPLINK_OFFSET 0x81000
#define HDA_DSP_STREAM_RESET_TIMEOUT 300 +/* + * Timeout in us, for setting the stream RUN bit, during + * start/stop the stream. The timeout expires if new RUN bit + * value cannot be read back within the specified time. + */ +#define HDA_DSP_STREAM_RUN_TIMEOUT 300 #define HDA_DSP_CL_TRIGGER_TIMEOUT 300
#define HDA_DSP_SPIB_ENABLE 1
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com
Before clearing stream statuses, ensure RUN bit update has taken effect by reading the value back.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-stream.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 13d114993f96..ad8d41f22e92 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -295,7 +295,9 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_stream *hstream = &stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); int ret, timeout = HDA_DSP_STREAM_RESET_TIMEOUT; + u32 dma_start = SOF_HDA_SD_CTL_DMA_START; u32 val, mask; + u32 run;
if (!stream) { dev_err(sdev->dev, "error: no stream available\n"); @@ -316,6 +318,16 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset, SOF_HDA_CL_DMA_SD_INT_MASK | SOF_HDA_SD_CTL_DMA_START, 0); + + ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK, @@ -368,6 +380,16 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset, SOF_HDA_CL_DMA_SD_INT_MASK | SOF_HDA_SD_CTL_DMA_START, 0); + + ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK,
The patch
ASoC: SOF: Intel: hda: make sure RUN bit setting to 0 during clear stream status
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 5b8cc7d17f56257056ed65624dec28dcdf14e87e Mon Sep 17 00:00:00 2001
From: Zhu Yingjiang yingjiang.zhu@linux.intel.com Date: Wed, 12 Jun 2019 12:23:47 -0500 Subject: [PATCH] ASoC: SOF: Intel: hda: make sure RUN bit setting to 0 during clear stream status
Before clearing stream statuses, ensure RUN bit update has taken effect by reading the value back.
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-stream.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 13d114993f96..ad8d41f22e92 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -295,7 +295,9 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_stream *hstream = &stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); int ret, timeout = HDA_DSP_STREAM_RESET_TIMEOUT; + u32 dma_start = SOF_HDA_SD_CTL_DMA_START; u32 val, mask; + u32 run;
if (!stream) { dev_err(sdev->dev, "error: no stream available\n"); @@ -316,6 +318,16 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset, SOF_HDA_CL_DMA_SD_INT_MASK | SOF_HDA_SD_CTL_DMA_START, 0); + + ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK, @@ -368,6 +380,16 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset, SOF_HDA_CL_DMA_SD_INT_MASK | SOF_HDA_SD_CTL_DMA_START, 0); + + ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR, + sd_offset, run, + !(run & dma_start), + HDA_DSP_REG_POLL_INTERVAL_US, + HDA_DSP_STREAM_RUN_TIMEOUT); + + if (ret) + return ret; + snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset + SOF_HDA_ADSP_REG_CL_SD_STS, SOF_HDA_CL_DMA_SD_INT_MASK,
participants (3)
-
Amadeusz Sławiński -
Mark Brown -
Pierre-Louis Bossart