From 37ea566eb8eaca402fa167a378e0bc7fee354298 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 14 Sep 2021 06:47:52 -0500 Subject: [PATCH 01/40] ALSA: HDA: hdac_ext_stream: use consistent prefixes for variables The existing code maximizes confusion by using 'stream' and 'hstream' variables of different types. Examples: struct hdac_stream *stream; struct hdac_ext_stream *stream; struct hdac_stream *hstream; struct hdac_ext_stream *hstream; with some additional copy/paste remains: struct hdac_ext_stream *azx_dev; This patch suggests a consistent naming across all 'hdac_ext_stream' functions. The convention is: struct hdac_stream *hstream; struct hdac_ext_stream *he_stream; No functionality change - just renaming of variables and more consistent indentation. Signed-off-by: Pierre-Louis Bossart --- include/sound/hdaudio_ext.h | 26 ++--- sound/hda/ext/hdac_ext_stream.c | 201 ++++++++++++++++---------------- 2 files changed, 114 insertions(+), 113 deletions(-) diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 375581634143c9..b72c7a3c569db8 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -78,34 +78,34 @@ struct hdac_ext_stream { container_of(s, struct hdac_ext_stream, hstream) void snd_hdac_ext_stream_init(struct hdac_bus *bus, - struct hdac_ext_stream *stream, int idx, - int direction, int tag); + struct hdac_ext_stream *he_stream, int idx, + int direction, int tag); int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx, - int num_stream, int dir); + int num_stream, int dir); void snd_hdac_stream_free_all(struct hdac_bus *bus); void snd_hdac_link_free_all(struct hdac_bus *bus); struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream, int type); -void snd_hdac_ext_stream_release(struct hdac_ext_stream *azx_dev, int type); +void snd_hdac_ext_stream_release(struct hdac_ext_stream *he_stream, int type); void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, - struct hdac_ext_stream *azx_dev, bool decouple); + struct hdac_ext_stream *he_stream, bool decouple); void snd_hdac_ext_stop_streams(struct hdac_bus *bus); int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus, - struct hdac_ext_stream *stream, u32 value); + struct hdac_ext_stream *he_stream, u32 value); int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus, - struct hdac_ext_stream *stream); + struct hdac_ext_stream *he_stream); void snd_hdac_ext_stream_drsm_enable(struct hdac_bus *bus, bool enable, int index); int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, - struct hdac_ext_stream *stream, u32 value); -int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *stream, u32 value); + struct hdac_ext_stream *he_stream, u32 value); +int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *he_stream, u32 value); -void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hstream); -void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hstream); -void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hstream); -int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt); +void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *he_stream); +void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *he_stream); +void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *he_stream); +int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *he_stream, int fmt); struct hdac_ext_link { struct hdac_bus *bus; diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index 0c005d67fa891e..7966d0d9f6880f 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -18,7 +18,7 @@ /** * snd_hdac_ext_stream_init - initialize each stream (aka device) * @bus: HD-audio core bus - * @stream: HD-audio ext core stream object to initialize + * @he_stream: HD-audio ext core stream object to initialize * @idx: stream index number * @direction: stream direction (SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CAPTURE) * @tag: the tag id to assign @@ -27,34 +27,34 @@ * invoke hdac stream initialization routine */ void snd_hdac_ext_stream_init(struct hdac_bus *bus, - struct hdac_ext_stream *stream, - int idx, int direction, int tag) + struct hdac_ext_stream *he_stream, + int idx, int direction, int tag) { if (bus->ppcap) { - stream->pphc_addr = bus->ppcap + AZX_PPHC_BASE + + he_stream->pphc_addr = bus->ppcap + AZX_PPHC_BASE + AZX_PPHC_INTERVAL * idx; - stream->pplc_addr = bus->ppcap + AZX_PPLC_BASE + + he_stream->pplc_addr = bus->ppcap + AZX_PPLC_BASE + AZX_PPLC_MULTI * bus->num_streams + AZX_PPLC_INTERVAL * idx; } if (bus->spbcap) { - stream->spib_addr = bus->spbcap + AZX_SPB_BASE + + he_stream->spib_addr = bus->spbcap + AZX_SPB_BASE + AZX_SPB_INTERVAL * idx + AZX_SPB_SPIB; - stream->fifo_addr = bus->spbcap + AZX_SPB_BASE + + he_stream->fifo_addr = bus->spbcap + AZX_SPB_BASE + AZX_SPB_INTERVAL * idx + AZX_SPB_MAXFIFO; } if (bus->drsmcap) - stream->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE + + he_stream->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE + AZX_DRSM_INTERVAL * idx; - stream->decoupled = false; - snd_hdac_stream_init(bus, &stream->hstream, idx, direction, tag); + he_stream->decoupled = false; + snd_hdac_stream_init(bus, &he_stream->hstream, idx, direction, tag); } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init); @@ -67,18 +67,18 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init); * @dir: direction of streams */ int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx, - int num_stream, int dir) + int num_stream, int dir) { int stream_tag = 0; int i, tag, idx = start_idx; for (i = 0; i < num_stream; i++) { - struct hdac_ext_stream *stream = - kzalloc(sizeof(*stream), GFP_KERNEL); - if (!stream) + struct hdac_ext_stream *he_stream = + kzalloc(sizeof(*he_stream), GFP_KERNEL); + if (!he_stream) return -ENOMEM; tag = ++stream_tag; - snd_hdac_ext_stream_init(bus, stream, idx, dir, tag); + snd_hdac_ext_stream_init(bus, he_stream, idx, dir, tag); idx++; } @@ -95,13 +95,13 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init_all); void snd_hdac_stream_free_all(struct hdac_bus *bus) { struct hdac_stream *s, *_s; - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; list_for_each_entry_safe(s, _s, &bus->stream_list, list) { - stream = stream_to_hdac_ext_stream(s); - snd_hdac_ext_stream_decouple(bus, stream, false); + he_stream = stream_to_hdac_ext_stream(s); + snd_hdac_ext_stream_decouple(bus, he_stream, false); list_del(&s->list); - kfree(stream); + kfree(he_stream); } } EXPORT_SYMBOL_GPL(snd_hdac_stream_free_all); @@ -109,13 +109,13 @@ EXPORT_SYMBOL_GPL(snd_hdac_stream_free_all); /** * snd_hdac_ext_stream_decouple - decouple the hdac stream * @bus: HD-audio core bus - * @stream: HD-audio ext core stream object to initialize + * @he_stream: HD-audio ext core stream object to initialize * @decouple: flag to decouple */ void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, - struct hdac_ext_stream *stream, bool decouple) + struct hdac_ext_stream *he_stream, bool decouple) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; u32 val; int mask = AZX_PPCTL_PROCEN(hstream->index); @@ -127,62 +127,62 @@ void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, else if (!decouple && val) snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, mask, 0); - stream->decoupled = decouple; + he_stream->decoupled = decouple; spin_unlock_irq(&bus->reg_lock); } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple); /** * snd_hdac_ext_link_stream_start - start a stream - * @stream: HD-audio ext core stream to start + * @he_stream: HD-audio ext core stream to start */ -void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *stream) +void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *he_stream) { - snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL, + snd_hdac_updatel(he_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, AZX_PPLCCTL_RUN); } EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_start); /** * snd_hdac_ext_link_stream_clear - stop a stream DMA - * @stream: HD-audio ext core stream to stop + * @he_stream: HD-audio ext core stream to stop */ -void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *stream) +void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *he_stream) { - snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, 0); + snd_hdac_updatel(he_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, 0); } EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_clear); /** * snd_hdac_ext_link_stream_reset - reset a stream - * @stream: HD-audio ext core stream to reset + * @he_stream: HD-audio ext core stream to reset */ -void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *stream) +void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *he_stream) { unsigned char val; int timeout; - snd_hdac_ext_link_stream_clear(stream); + snd_hdac_ext_link_stream_clear(he_stream); - snd_hdac_updatel(stream->pplc_addr, AZX_REG_PPLCCTL, + snd_hdac_updatel(he_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_STRST, AZX_PPLCCTL_STRST); udelay(3); timeout = 50; do { - val = readl(stream->pplc_addr + AZX_REG_PPLCCTL) & + val = readl(he_stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_STRST; if (val) break; udelay(3); } while (--timeout); val &= ~AZX_PPLCCTL_STRST; - writel(val, stream->pplc_addr + AZX_REG_PPLCCTL); + writel(val, he_stream->pplc_addr + AZX_REG_PPLCCTL); udelay(3); timeout = 50; /* waiting for hardware to report that the stream is out of reset */ do { - val = readl(stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_STRST; + val = readl(he_stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_STRST; if (!val) break; udelay(3); @@ -193,24 +193,24 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_reset); /** * snd_hdac_ext_link_stream_setup - set up the SD for streaming - * @stream: HD-audio ext core stream to set up + * @he_stream: HD-audio ext core stream to set up * @fmt: stream format */ -int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt) +int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *he_stream, int fmt) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; unsigned int val; /* make sure the run bit is zero for SD */ - snd_hdac_ext_link_stream_clear(stream); + snd_hdac_ext_link_stream_clear(he_stream); /* program the stream_tag */ - val = readl(stream->pplc_addr + AZX_REG_PPLCCTL); + val = readl(he_stream->pplc_addr + AZX_REG_PPLCCTL); val = (val & ~AZX_PPLCCTL_STRM_MASK) | (hstream->stream_tag << AZX_PPLCCTL_STRM_SHIFT); - writel(val, stream->pplc_addr + AZX_REG_PPLCCTL); + writel(val, he_stream->pplc_addr + AZX_REG_PPLCCTL); /* program the stream format */ - writew(fmt, stream->pplc_addr + AZX_REG_PPLCFMT); + writew(fmt, he_stream->pplc_addr + AZX_REG_PPLCFMT); return 0; } @@ -222,7 +222,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_setup); * @stream: stream id */ void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, - int stream) + int stream) { snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 1 << stream); } @@ -242,32 +242,32 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_clear_stream_id); static struct hdac_ext_stream * hdac_ext_link_stream_assign(struct hdac_bus *bus, - struct snd_pcm_substream *substream) + struct snd_pcm_substream *substream) { struct hdac_ext_stream *res = NULL; - struct hdac_stream *stream = NULL; + struct hdac_stream *hstream = NULL; if (!bus->ppcap) { dev_err(bus->dev, "stream type not supported\n"); return NULL; } - list_for_each_entry(stream, &bus->stream_list, list) { - struct hdac_ext_stream *hstream = container_of(stream, - struct hdac_ext_stream, - hstream); - if (stream->direction != substream->stream) + list_for_each_entry(hstream, &bus->stream_list, list) { + struct hdac_ext_stream *he_stream = container_of(hstream, + struct hdac_ext_stream, + hstream); + if (hstream->direction != substream->stream) continue; /* check if decoupled stream and not in use is available */ - if (hstream->decoupled && !hstream->link_locked) { - res = hstream; + if (he_stream->decoupled && !he_stream->link_locked) { + res = he_stream; break; } - if (!hstream->link_locked) { - snd_hdac_ext_stream_decouple(bus, hstream, true); - res = hstream; + if (!he_stream->link_locked) { + snd_hdac_ext_stream_decouple(bus, he_stream, true); + res = he_stream; break; } } @@ -282,27 +282,27 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus, static struct hdac_ext_stream * hdac_ext_host_stream_assign(struct hdac_bus *bus, - struct snd_pcm_substream *substream) + struct snd_pcm_substream *substream) { struct hdac_ext_stream *res = NULL; - struct hdac_stream *stream = NULL; + struct hdac_stream *hstream = NULL; if (!bus->ppcap) { dev_err(bus->dev, "stream type not supported\n"); return NULL; } - list_for_each_entry(stream, &bus->stream_list, list) { - struct hdac_ext_stream *hstream = container_of(stream, - struct hdac_ext_stream, - hstream); - if (stream->direction != substream->stream) + list_for_each_entry(hstream, &bus->stream_list, list) { + struct hdac_ext_stream *he_stream = container_of(hstream, + struct hdac_ext_stream, + hstream); + if (hstream->direction != substream->stream) continue; - if (!stream->opened) { - if (!hstream->decoupled) - snd_hdac_ext_stream_decouple(bus, hstream, true); - res = hstream; + if (!hstream->opened) { + if (!he_stream->decoupled) + snd_hdac_ext_stream_decouple(bus, he_stream, true); + res = he_stream; break; } } @@ -338,16 +338,17 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream, int type) { - struct hdac_ext_stream *hstream = NULL; - struct hdac_stream *stream = NULL; + struct hdac_ext_stream *he_stream = NULL; + struct hdac_stream *hstream = NULL; switch (type) { case HDAC_EXT_STREAM_TYPE_COUPLED: - stream = snd_hdac_stream_assign(bus, substream); - if (stream) - hstream = container_of(stream, - struct hdac_ext_stream, hstream); - return hstream; + hstream = snd_hdac_stream_assign(bus, substream); + if (hstream) + he_stream = container_of(hstream, + struct hdac_ext_stream, + hstream); + return he_stream; case HDAC_EXT_STREAM_TYPE_HOST: return hdac_ext_host_stream_assign(bus, substream); @@ -363,32 +364,32 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_assign); /** * snd_hdac_ext_stream_release - release the assigned stream - * @stream: HD-audio ext core stream to release + * @he_stream: HD-audio ext core stream to release * @type: type of stream (coupled, host or link stream) * * Release the stream that has been assigned by snd_hdac_ext_stream_assign(). */ -void snd_hdac_ext_stream_release(struct hdac_ext_stream *stream, int type) +void snd_hdac_ext_stream_release(struct hdac_ext_stream *he_stream, int type) { - struct hdac_bus *bus = stream->hstream.bus; + struct hdac_bus *bus = he_stream->hstream.bus; switch (type) { case HDAC_EXT_STREAM_TYPE_COUPLED: - snd_hdac_stream_release(&stream->hstream); + snd_hdac_stream_release(&he_stream->hstream); break; case HDAC_EXT_STREAM_TYPE_HOST: - if (stream->decoupled && !stream->link_locked) - snd_hdac_ext_stream_decouple(bus, stream, false); - snd_hdac_stream_release(&stream->hstream); + if (he_stream->decoupled && !he_stream->link_locked) + snd_hdac_ext_stream_decouple(bus, he_stream, false); + snd_hdac_stream_release(&he_stream->hstream); break; case HDAC_EXT_STREAM_TYPE_LINK: - if (stream->decoupled && !stream->hstream.opened) - snd_hdac_ext_stream_decouple(bus, stream, false); + if (he_stream->decoupled && !he_stream->hstream.opened) + snd_hdac_ext_stream_decouple(bus, he_stream, false); spin_lock_irq(&bus->reg_lock); - stream->link_locked = 0; - stream->link_substream = NULL; + he_stream->link_locked = 0; + he_stream->link_substream = NULL; spin_unlock_irq(&bus->reg_lock); break; @@ -427,11 +428,11 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable); /** * snd_hdac_ext_stream_set_spib - sets the spib value of a stream * @bus: HD-audio core bus - * @stream: hdac_ext_stream + * @he_stream: hdac_ext_stream * @value: spib value to set */ int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus, - struct hdac_ext_stream *stream, u32 value) + struct hdac_ext_stream *he_stream, u32 value) { if (!bus->spbcap) { @@ -439,7 +440,7 @@ int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus, return -EINVAL; } - writel(value, stream->spib_addr); + writel(value, he_stream->spib_addr); return 0; } @@ -448,12 +449,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spib); /** * snd_hdac_ext_stream_get_spbmaxfifo - gets the spib value of a stream * @bus: HD-audio core bus - * @stream: hdac_ext_stream + * @he_stream: hdac_ext_stream * * Return maxfifo for the stream */ int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus, - struct hdac_ext_stream *stream) + struct hdac_ext_stream *he_stream) { if (!bus->spbcap) { @@ -461,7 +462,7 @@ int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus, return -EINVAL; } - return readl(stream->fifo_addr); + return readl(he_stream->fifo_addr); } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_get_spbmaxfifo); @@ -472,11 +473,11 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_get_spbmaxfifo); */ void snd_hdac_ext_stop_streams(struct hdac_bus *bus) { - struct hdac_stream *stream; + struct hdac_stream *hstream; if (bus->chip_init) { - list_for_each_entry(stream, &bus->stream_list, list) - snd_hdac_stream_stop(stream); + list_for_each_entry(hstream, &bus->stream_list, list) + snd_hdac_stream_stop(hstream); snd_hdac_bus_stop_chip(bus); } } @@ -510,11 +511,11 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_drsm_enable); /** * snd_hdac_ext_stream_set_dpibr - sets the dpibr value of a stream * @bus: HD-audio core bus - * @stream: hdac_ext_stream + * @he_stream: hdac_ext_stream * @value: dpib value to set */ int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, - struct hdac_ext_stream *stream, u32 value) + struct hdac_ext_stream *he_stream, u32 value) { if (!bus->drsmcap) { @@ -522,7 +523,7 @@ int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, return -EINVAL; } - writel(value, stream->dpibr_addr); + writel(value, he_stream->dpibr_addr); return 0; } @@ -530,12 +531,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_dpibr); /** * snd_hdac_ext_stream_set_lpib - sets the lpib value of a stream - * @stream: hdac_ext_stream + * @he_stream: hdac_ext_stream * @value: lpib value to set */ -int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *stream, u32 value) +int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *he_stream, u32 value) { - snd_hdac_stream_writel(&stream->hstream, SD_LPIB, value); + snd_hdac_stream_writel(&he_stream->hstream, SD_LPIB, value); return 0; } From 67787ed8fa13c2c59543943df5caac21eb2a9a44 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 14 Sep 2021 07:55:09 -0500 Subject: [PATCH 02/40] ASoC: SOF: Intel: hdac_ext_stream: consistent prefixes for variables/members The existing code maximizes confusion by using 'stream' and 'hstream' variables of different types, e.g: struct hdac_stream *stream; struct hdac_ext_stream *stream; struct hdac_stream *hstream; struct hdac_ext_stream *hstream; This confusion is partly inherited from legacy code but SOF contributors added their own creative spin, e.g. struct hdac_ext_stream *link_dev; struct hdac_ext_stream *dsp_stream; struct hdac_ext_stream hda_stream; and my personal favorite: stream = &hda_stream->hda_stream; This patch suggests a consistent naming across all Intel code related to HDAudio stream management. The convention is - by hierarchical order: struct sof_intel_hda_stream *hda_stream; struct hdac_ext_stream *he_stream; struct hdac_stream *hstream; No functionality change - just renaming of variables/members. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 86 +++++++++++----------- sound/soc/sof/intel/hda-dsp.c | 14 ++-- sound/soc/sof/intel/hda-ipc.c | 10 +-- sound/soc/sof/intel/hda-loader.c | 34 ++++----- sound/soc/sof/intel/hda-pcm.c | 12 ++-- sound/soc/sof/intel/hda-probes.c | 30 ++++---- sound/soc/sof/intel/hda-stream.c | 120 +++++++++++++++---------------- sound/soc/sof/intel/hda-trace.c | 6 +- sound/soc/sof/intel/hda.h | 17 ++--- 9 files changed, 165 insertions(+), 164 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index ba441056bf8ad2..84932d31d75c5a 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -63,7 +63,7 @@ static struct hdac_ext_stream * struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct sof_intel_hda_stream *hda_stream; struct hdac_ext_stream *res = NULL; - struct hdac_stream *stream = NULL; + struct hdac_stream *hstream = NULL; int stream_dir = substream->stream; @@ -72,28 +72,28 @@ static struct hdac_ext_stream * return NULL; } - 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) + list_for_each_entry(hstream, &bus->stream_list, list) { + struct hdac_ext_stream *he_stream = + stream_to_hdac_ext_stream(hstream); + if (hstream->direction != substream->stream) continue; - hda_stream = hstream_to_sof_hda_stream(hstream); + hda_stream = hstream_to_sof_hda_stream(he_stream); /* check if link is available */ - if (!hstream->link_locked) { - if (stream->opened) { + if (!he_stream->link_locked) { + if (hstream->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; + hstream->stream_tag)) { + res = he_stream; break; } } else { - res = hstream; + res = he_stream; /* * This must be a hostless stream. @@ -121,17 +121,17 @@ static struct hdac_ext_stream * return res; } -static int hda_link_dma_params(struct hdac_ext_stream *stream, +static int hda_link_dma_params(struct hdac_ext_stream *he_stream, struct hda_pipe_params *params) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; unsigned char stream_tag = hstream->stream_tag; struct hdac_bus *bus = hstream->bus; struct hdac_ext_link *link; unsigned int format_val; - snd_hdac_ext_stream_decouple(bus, stream, true); - snd_hdac_ext_link_stream_reset(stream); + snd_hdac_ext_stream_decouple(bus, he_stream, true); + snd_hdac_ext_link_stream_reset(he_stream); format_val = snd_hdac_calc_stream_format(params->s_freq, params->ch, params->format, @@ -140,9 +140,9 @@ static int hda_link_dma_params(struct hdac_ext_stream *stream, dev_dbg(bus->dev, "format_val=%d, rate=%d, ch=%d, format=%d\n", format_val, params->s_freq, params->ch, params->format); - snd_hdac_ext_link_stream_setup(stream, format_val); + snd_hdac_ext_link_stream_setup(he_stream, format_val); - if (stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { + if (he_stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { list_for_each_entry(link, &bus->hlink_list, list) { if (link->index == params->link_index) snd_hdac_ext_link_set_stream_id(link, @@ -150,7 +150,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *stream, } } - stream->link_prepared = 1; + he_stream->link_prepared = 1; return 0; } @@ -207,7 +207,7 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, { struct hdac_stream *hstream = substream->runtime->private_data; struct hdac_bus *bus = hstream->bus; - struct hdac_ext_stream *link_dev; + struct hdac_ext_stream *he_stream; struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); struct sof_intel_hda_stream *hda_stream; @@ -218,18 +218,18 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, int ret; /* get stored dma data if resuming from system suspend */ - link_dev = snd_soc_dai_get_dma_data(dai, substream); - if (!link_dev) { - link_dev = hda_link_stream_assign(bus, substream); - if (!link_dev) + he_stream = snd_soc_dai_get_dma_data(dai, substream); + if (!he_stream) { + he_stream = hda_link_stream_assign(bus, substream); + if (!he_stream) return -EBUSY; - snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev); + snd_soc_dai_set_dma_data(dai, substream, (void *)he_stream); } - stream_tag = hdac_stream(link_dev)->stream_tag; + stream_tag = hdac_stream(he_stream)->stream_tag; - hda_stream = hstream_to_sof_hda_stream(link_dev); + hda_stream = hstream_to_sof_hda_stream(he_stream); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) w = dai->playback_widget; @@ -264,20 +264,20 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, else p_params.link_bps = codec_dai->driver->capture.sig_bits; - return hda_link_dma_params(link_dev, &p_params); + return hda_link_dma_params(he_stream, &p_params); } static int hda_link_pcm_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - struct hdac_ext_stream *link_dev = + struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(dai, substream); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component); struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); int stream = substream->stream; - if (link_dev->link_prepared) + if (he_stream->link_prepared) return 0; dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream); @@ -289,7 +289,7 @@ static int hda_link_pcm_prepare(struct snd_pcm_substream *substream, static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { - struct hdac_ext_stream *link_dev = + struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(dai, substream); struct sof_intel_hda_stream *hda_stream; struct snd_soc_pcm_runtime *rtd; @@ -308,7 +308,7 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, if (!link) return -EINVAL; - hda_stream = hstream_to_sof_hda_stream(link_dev); + hda_stream = hstream_to_sof_hda_stream(he_stream); dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd); switch (cmd) { @@ -324,7 +324,7 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, fallthrough; case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - snd_hdac_ext_link_stream_start(link_dev); + snd_hdac_ext_link_stream_start(he_stream); break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: @@ -341,15 +341,15 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, return ret; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - stream_tag = hdac_stream(link_dev)->stream_tag; + stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); } - link_dev->link_prepared = 0; + he_stream->link_prepared = 0; fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - snd_hdac_ext_link_stream_clear(link_dev); + snd_hdac_ext_link_stream_clear(he_stream); break; default: return -EINVAL; @@ -366,22 +366,22 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, struct hdac_ext_link *link; struct hdac_stream *hstream; struct snd_soc_pcm_runtime *rtd; - struct hdac_ext_stream *link_dev; + struct hdac_ext_stream *he_stream; struct snd_soc_dapm_widget *w; int ret; hstream = substream->runtime->private_data; bus = hstream->bus; rtd = asoc_substream_to_rtd(substream); - link_dev = snd_soc_dai_get_dma_data(dai, substream); + he_stream = snd_soc_dai_get_dma_data(dai, substream); - if (!link_dev) { + if (!he_stream) { dev_dbg(dai->dev, - "%s: link_dev is not assigned\n", __func__); + "%s: he_stream is not assigned\n", __func__); return -EINVAL; } - hda_stream = hstream_to_sof_hda_stream(link_dev); + hda_stream = hstream_to_sof_hda_stream(he_stream); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) w = dai->playback_widget; @@ -398,13 +398,13 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, return -EINVAL; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - stream_tag = hdac_stream(link_dev)->stream_tag; + stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); } snd_soc_dai_set_dma_data(dai, substream, NULL); - snd_hdac_ext_stream_release(link_dev, HDAC_EXT_STREAM_TYPE_LINK); - link_dev->link_prepared = 0; + snd_hdac_ext_stream_release(he_stream, HDAC_EXT_STREAM_TYPE_LINK); + he_stream->link_prepared = 0; /* free the host DMA channel reserved by hostless streams */ hda_stream->host_reserved = 0; diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index e03bb455f82a16..bba95ab4c1a31e 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -905,7 +905,7 @@ int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) struct hdac_bus *bus = sof_to_bus(sdev); struct snd_soc_pcm_runtime *rtd; - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; struct hdac_ext_link *link; struct hdac_stream *s; const char *name; @@ -913,7 +913,7 @@ int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) /* set internal flag for BE */ list_for_each_entry(s, &bus->stream_list, list) { - stream = stream_to_hdac_ext_stream(s); + he_stream = stream_to_hdac_ext_stream(s); /* * clear stream. This should already be taken care for running @@ -921,20 +921,20 @@ int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) * streams do not get suspended, so this needs to be done * explicitly during suspend. */ - if (stream->link_substream) { - rtd = asoc_substream_to_rtd(stream->link_substream); + if (he_stream->link_substream) { + rtd = asoc_substream_to_rtd(he_stream->link_substream); name = asoc_rtd_to_codec(rtd, 0)->component->name; link = snd_hdac_ext_bus_get_link(bus, name); if (!link) return -EINVAL; - stream->link_prepared = 0; + he_stream->link_prepared = 0; - if (hdac_stream(stream)->direction == + if (hdac_stream(he_stream)->direction == SNDRV_PCM_STREAM_CAPTURE) continue; - stream_tag = hdac_stream(stream)->stream_tag; + stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); } } diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 2019087a84cecc..976f3f09cd5fb8 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -240,13 +240,13 @@ int hda_ipc_msg_data(struct snd_sof_dev *sdev, hda_stream = container_of(hstream, struct sof_intel_hda_stream, - hda_stream.hstream); + he_stream.hstream); /* The stream might already be closed */ if (!hstream) return -ESTRPIPE; - sof_mailbox_read(sdev, hda_stream->stream.posn_offset, p, sz); + sof_mailbox_read(sdev, hda_stream->sof_intel_stream.posn_offset, p, sz); } return 0; @@ -262,17 +262,17 @@ int hda_ipc_pcm_params(struct snd_sof_dev *sdev, size_t posn_offset = reply->posn_offset; hda_stream = container_of(hstream, struct sof_intel_hda_stream, - hda_stream.hstream); + he_stream.hstream); /* check for unaligned offset or overflow */ if (posn_offset > sdev->stream_box.size || posn_offset % sizeof(struct sof_ipc_stream_posn) != 0) return -EINVAL; - hda_stream->stream.posn_offset = sdev->stream_box.offset + posn_offset; + hda_stream->sof_intel_stream.posn_offset = sdev->stream_box.offset + posn_offset; dev_dbg(sdev->dev, "pcm: stream dir %d, posn mailbox offset is %zu", - substream->stream, hda_stream->stream.posn_offset); + substream->stream, hda_stream->sof_intel_stream.posn_offset); return 0; } diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index bfb0e374ebab6f..a85ebd0b46e723 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -30,18 +30,18 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig unsigned int size, struct snd_dma_buffer *dmab, int direction) { - struct hdac_ext_stream *dsp_stream; + struct hdac_ext_stream *he_stream; struct hdac_stream *hstream; struct pci_dev *pci = to_pci_dev(sdev->dev); int ret; - dsp_stream = hda_dsp_stream_get(sdev, direction, 0); + he_stream = hda_dsp_stream_get(sdev, direction, 0); - if (!dsp_stream) { + if (!he_stream) { dev_err(sdev->dev, "error: no stream available\n"); return ERR_PTR(-ENODEV); } - hstream = &dsp_stream->hstream; + hstream = &he_stream->hstream; hstream->substream = NULL; /* allocate DMA buffer */ @@ -56,21 +56,21 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig hstream->bufsize = size; if (direction == SNDRV_PCM_STREAM_CAPTURE) { - ret = hda_dsp_iccmax_stream_hw_params(sdev, dsp_stream, dmab, NULL); + ret = hda_dsp_iccmax_stream_hw_params(sdev, he_stream, dmab, NULL); if (ret < 0) { dev_err(sdev->dev, "error: iccmax stream prepare failed: %d\n", ret); goto error; } } else { - ret = hda_dsp_stream_hw_params(sdev, dsp_stream, dmab, NULL); + ret = hda_dsp_stream_hw_params(sdev, he_stream, dmab, NULL); if (ret < 0) { dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); goto error; } - hda_dsp_stream_spib_config(sdev, dsp_stream, HDA_DSP_SPIB_ENABLE, size); + hda_dsp_stream_spib_config(sdev, he_stream, HDA_DSP_SPIB_ENABLE, size); } - return dsp_stream; + return he_stream; error: hda_dsp_stream_put(sdev, direction, hstream->stream_tag); @@ -197,9 +197,9 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) } static int cl_trigger(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, int cmd) + struct hdac_ext_stream *he_stream, int cmd) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); /* code loader is special case that reuses stream ops */ @@ -219,19 +219,19 @@ static int cl_trigger(struct snd_sof_dev *sdev, hstream->running = true; return 0; default: - return hda_dsp_stream_trigger(sdev, stream, cmd); + return hda_dsp_stream_trigger(sdev, he_stream, cmd); } } static int cl_cleanup(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, - struct hdac_ext_stream *stream) + struct hdac_ext_stream *he_stream) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); int ret = 0; if (hstream->direction == SNDRV_PCM_STREAM_PLAYBACK) - ret = hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0); + ret = hda_dsp_stream_spib_config(sdev, he_stream, HDA_DSP_SPIB_DISABLE, 0); else snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset, SOF_HDA_SD_CTL_DMA_START, 0); @@ -255,12 +255,12 @@ static int cl_cleanup(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, return ret; } -static int cl_copy_fw(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream) +static int cl_copy_fw(struct snd_sof_dev *sdev, struct hdac_ext_stream *he_stream) { unsigned int reg; int ret, status; - ret = cl_trigger(sdev, stream, SNDRV_PCM_TRIGGER_START); + ret = cl_trigger(sdev, he_stream, SNDRV_PCM_TRIGGER_START); if (ret < 0) { dev_err(sdev->dev, "error: DMA trigger start failed\n"); return ret; @@ -284,7 +284,7 @@ static int cl_copy_fw(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream) __func__); } - ret = cl_trigger(sdev, stream, SNDRV_PCM_TRIGGER_STOP); + ret = cl_trigger(sdev, he_stream, SNDRV_PCM_TRIGGER_STOP); if (ret < 0) { dev_err(sdev->dev, "error: DMA trigger stop failed\n"); if (!status) diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 875350283eacc0..7d16818d3453ec 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -96,7 +96,7 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, struct sof_ipc_stream_params *ipc_params) { struct hdac_stream *hstream = substream->runtime->private_data; - struct hdac_ext_stream *stream = stream_to_hdac_ext_stream(hstream); + struct hdac_ext_stream *he_stream = stream_to_hdac_ext_stream(hstream); struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; struct snd_dma_buffer *dmab; struct sof_ipc_fw_version *v = &sdev->fw_ready.version; @@ -118,7 +118,7 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) && (params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP); - ret = hda_dsp_stream_hw_params(sdev, stream, dmab, params); + ret = hda_dsp_stream_hw_params(sdev, he_stream, dmab, params); if (ret < 0) { dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); return ret; @@ -126,9 +126,9 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, /* enable SPIB when rewinds are disabled */ if (hda_disable_rewinds) - hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_ENABLE, 0); + hda_dsp_stream_spib_config(sdev, he_stream, HDA_DSP_SPIB_ENABLE, 0); else - hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0); + hda_dsp_stream_spib_config(sdev, he_stream, HDA_DSP_SPIB_DISABLE, 0); /* update no_stream_position flag for ipc params */ if (hda && hda->no_ipc_position) { @@ -174,9 +174,9 @@ int hda_dsp_pcm_trigger(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, int cmd) { struct hdac_stream *hstream = substream->runtime->private_data; - struct hdac_ext_stream *stream = stream_to_hdac_ext_stream(hstream); + struct hdac_ext_stream *he_stream = stream_to_hdac_ext_stream(hstream); - return hda_dsp_stream_trigger(sdev, stream, cmd); + return hda_dsp_stream_trigger(sdev, he_stream, cmd); } snd_pcm_uframes_t hda_dsp_pcm_pointer(struct snd_sof_dev *sdev, diff --git a/sound/soc/sof/intel/hda-probes.c b/sound/soc/sof/intel/hda-probes.c index fe2f3f7d236b10..1da526c1e529f8 100644 --- a/sound/soc/sof/intel/hda-probes.c +++ b/sound/soc/sof/intel/hda-probes.c @@ -23,34 +23,34 @@ int hda_probe_compr_assign(struct snd_sof_dev *sdev, struct snd_compr_stream *cstream, struct snd_soc_dai *dai) { - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; - stream = hda_dsp_stream_get(sdev, cstream->direction, 0); - if (!stream) + he_stream = hda_dsp_stream_get(sdev, cstream->direction, 0); + if (!he_stream) return -EBUSY; - hdac_stream(stream)->curr_pos = 0; - hdac_stream(stream)->cstream = cstream; - cstream->runtime->private_data = stream; + hdac_stream(he_stream)->curr_pos = 0; + hdac_stream(he_stream)->cstream = cstream; + cstream->runtime->private_data = he_stream; - return hdac_stream(stream)->stream_tag; + return hdac_stream(he_stream)->stream_tag; } int hda_probe_compr_free(struct snd_sof_dev *sdev, struct snd_compr_stream *cstream, struct snd_soc_dai *dai) { - struct hdac_ext_stream *stream = hda_compr_get_stream(cstream); + struct hdac_ext_stream *he_stream = hda_compr_get_stream(cstream); int ret; ret = hda_dsp_stream_put(sdev, cstream->direction, - hdac_stream(stream)->stream_tag); + hdac_stream(he_stream)->stream_tag); if (ret < 0) { dev_dbg(sdev->dev, "stream put failed: %d\n", ret); return ret; } - hdac_stream(stream)->cstream = NULL; + hdac_stream(he_stream)->cstream = NULL; cstream->runtime->private_data = NULL; return 0; @@ -61,8 +61,8 @@ int hda_probe_compr_set_params(struct snd_sof_dev *sdev, struct snd_compr_params *params, struct snd_soc_dai *dai) { - struct hdac_ext_stream *stream = hda_compr_get_stream(cstream); - struct hdac_stream *hstream = hdac_stream(stream); + struct hdac_ext_stream *he_stream = hda_compr_get_stream(cstream); + struct hdac_stream *hstream = hdac_stream(he_stream); struct snd_dma_buffer *dmab; u32 bits, rate; int bps, ret; @@ -80,7 +80,7 @@ int hda_probe_compr_set_params(struct snd_sof_dev *sdev, hstream->period_bytes = cstream->runtime->fragment_size; hstream->no_period_wakeup = 0; - ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL); + ret = hda_dsp_stream_hw_params(sdev, he_stream, dmab, NULL); if (ret < 0) { dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); return ret; @@ -103,11 +103,11 @@ int hda_probe_compr_pointer(struct snd_sof_dev *sdev, struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai) { - struct hdac_ext_stream *stream = hda_compr_get_stream(cstream); + struct hdac_ext_stream *he_stream = hda_compr_get_stream(cstream); struct snd_soc_pcm_stream *pstream; pstream = &dai->driver->capture; - tstamp->copied_total = hdac_stream(stream)->curr_pos; + tstamp->copied_total = hdac_stream(he_stream)->curr_pos; tstamp->sampling_rate = snd_pcm_rate_bit_to_rate(pstream->rates); return 0; diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 3f9cd734db3d59..61cce8837689cd 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -57,7 +57,7 @@ static char *hda_hstream_dbg_get_stream_info_str(struct hdac_stream *hstream) */ static int hda_setup_bdle(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, - struct hdac_stream *stream, + struct hdac_stream *hstream, struct sof_intel_dsp_bdl **bdlp, int offset, int size, int ioc) { @@ -68,7 +68,7 @@ static int hda_setup_bdle(struct snd_sof_dev *sdev, dma_addr_t addr; int chunk; - if (stream->frags >= HDA_DSP_MAX_BDL_ENTRIES) { + if (hstream->frags >= HDA_DSP_MAX_BDL_ENTRIES) { dev_err(sdev->dev, "error: stream frags exceeded\n"); return -EINVAL; } @@ -91,11 +91,11 @@ static int hda_setup_bdle(struct snd_sof_dev *sdev, size -= chunk; bdl->ioc = (size || !ioc) ? 0 : cpu_to_le32(0x01); bdl++; - stream->frags++; + hstream->frags++; offset += chunk; dev_vdbg(sdev->dev, "bdl, frags:%d, chunk size:0x%x;\n", - stream->frags, chunk); + hstream->frags, chunk); } *bdlp = bdl; @@ -108,47 +108,47 @@ static int hda_setup_bdle(struct snd_sof_dev *sdev, */ int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, - struct hdac_stream *stream) + struct hdac_stream *hstream) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; struct sof_intel_dsp_bdl *bdl; int i, offset, period_bytes, periods; int remain, ioc; - period_bytes = stream->period_bytes; + period_bytes = hstream->period_bytes; dev_dbg(sdev->dev, "%s: period_bytes:0x%x\n", __func__, period_bytes); if (!period_bytes) - period_bytes = stream->bufsize; + period_bytes = hstream->bufsize; - periods = stream->bufsize / period_bytes; + periods = hstream->bufsize / period_bytes; dev_dbg(sdev->dev, "%s: periods:%d\n", __func__, periods); - remain = stream->bufsize % period_bytes; + remain = hstream->bufsize % period_bytes; if (remain) periods++; /* program the initial BDL entries */ - bdl = (struct sof_intel_dsp_bdl *)stream->bdl.area; + bdl = (struct sof_intel_dsp_bdl *)hstream->bdl.area; offset = 0; - stream->frags = 0; + hstream->frags = 0; /* * set IOC if don't use position IPC * and period_wakeup needed. */ ioc = hda->no_ipc_position ? - !stream->no_period_wakeup : 0; + !hstream->no_period_wakeup : 0; for (i = 0; i < periods; i++) { if (i == (periods - 1) && remain) /* set the last small entry */ offset = hda_setup_bdle(sdev, dmab, - stream, &bdl, offset, + hstream, &bdl, offset, remain, 0); else offset = hda_setup_bdle(sdev, dmab, - stream, &bdl, offset, + hstream, &bdl, offset, period_bytes, ioc); } @@ -156,10 +156,10 @@ int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev, } int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, + struct hdac_ext_stream *he_stream, int enable, u32 size) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; u32 mask; if (!sdev->bar[HDA_DSP_SPIB_BAR]) { @@ -175,7 +175,7 @@ int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev, enable << hstream->index); /* set the SPIB value */ - sof_io_write(sdev, stream->spib_addr, size); + sof_io_write(sdev, he_stream->spib_addr, size); return 0; } @@ -186,7 +186,7 @@ hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags) { struct hdac_bus *bus = sof_to_bus(sdev); struct sof_intel_hda_stream *hda_stream; - struct hdac_ext_stream *stream = NULL; + struct hdac_ext_stream *he_stream = NULL; struct hdac_stream *s; spin_lock_irq(&bus->reg_lock); @@ -194,10 +194,10 @@ hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags) /* get an unused stream */ list_for_each_entry(s, &bus->stream_list, list) { if (s->direction == direction && !s->opened) { - stream = stream_to_hdac_ext_stream(s); - hda_stream = container_of(stream, + he_stream = stream_to_hdac_ext_stream(s); + hda_stream = container_of(he_stream, struct sof_intel_hda_stream, - hda_stream); + he_stream); /* check if the host DMA channel is reserved */ if (hda_stream->host_reserved) continue; @@ -210,11 +210,11 @@ hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags) spin_unlock_irq(&bus->reg_lock); /* stream found ? */ - if (!stream) { + if (!he_stream) { dev_err(sdev->dev, "error: no free %s streams\n", direction == SNDRV_PCM_STREAM_PLAYBACK ? "playback" : "capture"); - return stream; + return he_stream; } hda_stream->flags = flags; @@ -229,7 +229,7 @@ hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags) HDA_VS_INTEL_EM2, HDA_VS_INTEL_EM2_L1SEN, 0); - return stream; + return he_stream; } /* free a stream */ @@ -237,7 +237,7 @@ int hda_dsp_stream_put(struct snd_sof_dev *sdev, int direction, int stream_tag) { struct hdac_bus *bus = sof_to_bus(sdev); struct sof_intel_hda_stream *hda_stream; - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; struct hdac_stream *s; bool dmi_l1_enable = true; bool found = false; @@ -249,8 +249,8 @@ int hda_dsp_stream_put(struct snd_sof_dev *sdev, int direction, int stream_tag) * that are DMI L1 incompatible. */ 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); + he_stream = stream_to_hdac_ext_stream(s); + hda_stream = container_of(he_stream, struct sof_intel_hda_stream, he_stream); if (!s->opened) continue; @@ -280,9 +280,9 @@ int hda_dsp_stream_put(struct snd_sof_dev *sdev, int direction, int stream_tag) } int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, int cmd) + struct hdac_ext_stream *he_stream, int cmd) { - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); u32 dma_start = SOF_HDA_SD_CTL_DMA_START; int ret = 0; @@ -358,17 +358,17 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, } /* minimal recommended programming for ICCMAX stream */ -int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream, +int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_stream *he_stream, struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params) { struct hdac_bus *bus = sof_to_bus(sdev); - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); int ret; u32 mask = 0x1 << hstream->index; - if (!stream) { + if (!he_stream) { dev_err(sdev->dev, "error: no stream available\n"); return -ENODEV; } @@ -429,20 +429,20 @@ int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_st * and normal stream. */ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, + struct hdac_ext_stream *he_stream, struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params) { const struct sof_intel_dsp_desc *chip = get_chip_info(sdev->pdata); struct hdac_bus *bus = sof_to_bus(sdev); - struct hdac_stream *hstream = &stream->hstream; + struct hdac_stream *hstream = &he_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) { + if (!he_stream) { dev_err(sdev->dev, "error: no stream available\n"); return -ENODEV; } @@ -647,23 +647,23 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, 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_stream *hstream = substream->runtime->private_data; + struct hdac_ext_stream *he_stream = container_of(hstream, + struct hdac_ext_stream, + hstream); struct hdac_bus *bus = sof_to_bus(sdev); - u32 mask = 0x1 << stream->index; + u32 mask = 0x1 << hstream->index; spin_lock_irq(&bus->reg_lock); /* couple host and link DMA if link DMA channel is idle */ - if (!link_dev->link_locked) + if (!he_stream->link_locked) snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, mask, 0); spin_unlock_irq(&bus->reg_lock); - hda_dsp_stream_spib_config(sdev, link_dev, HDA_DSP_SPIB_DISABLE, 0); + hda_dsp_stream_spib_config(sdev, he_stream, HDA_DSP_SPIB_DISABLE, 0); - stream->substream = NULL; + hstream->substream = NULL; return 0; } @@ -791,7 +791,7 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) int hda_dsp_stream_init(struct snd_sof_dev *sdev) { struct hdac_bus *bus = sof_to_bus(sdev); - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; struct hdac_stream *hstream; struct pci_dev *pci = to_pci_dev(sdev->dev); struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus); @@ -855,27 +855,27 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) hda_stream->sdev = sdev; - stream = &hda_stream->hda_stream; + he_stream = &hda_stream->he_stream; - stream->pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + + he_stream->pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + SOF_HDA_PPHC_BASE + SOF_HDA_PPHC_INTERVAL * i; - stream->pplc_addr = sdev->bar[HDA_DSP_PP_BAR] + + he_stream->pplc_addr = sdev->bar[HDA_DSP_PP_BAR] + SOF_HDA_PPLC_BASE + SOF_HDA_PPLC_MULTI * num_total + SOF_HDA_PPLC_INTERVAL * i; /* do we support SPIB */ if (sdev->bar[HDA_DSP_SPIB_BAR]) { - stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + he_stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_SPIB; - stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + he_stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_MAXFIFO; } - hstream = &stream->hstream; + hstream = &he_stream->hstream; hstream->bus = bus; hstream->sd_int_sta_mask = 1 << i; hstream->index = i; @@ -910,28 +910,28 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) hda_stream->sdev = sdev; - stream = &hda_stream->hda_stream; + he_stream = &hda_stream->he_stream; /* we always have DSP support */ - stream->pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + + he_stream->pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + SOF_HDA_PPHC_BASE + SOF_HDA_PPHC_INTERVAL * i; - stream->pplc_addr = sdev->bar[HDA_DSP_PP_BAR] + + he_stream->pplc_addr = sdev->bar[HDA_DSP_PP_BAR] + SOF_HDA_PPLC_BASE + SOF_HDA_PPLC_MULTI * num_total + SOF_HDA_PPLC_INTERVAL * i; /* do we support SPIB */ if (sdev->bar[HDA_DSP_SPIB_BAR]) { - stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + he_stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_SPIB; - stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + he_stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_MAXFIFO; } - hstream = &stream->hstream; + hstream = &he_stream->hstream; hstream->bus = bus; hstream->sd_int_sta_mask = 1 << i; hstream->index = i; @@ -966,7 +966,7 @@ void hda_dsp_stream_free(struct snd_sof_dev *sdev) { struct hdac_bus *bus = sof_to_bus(sdev); struct hdac_stream *s, *_s; - struct hdac_ext_stream *stream; + struct hdac_ext_stream *he_stream; struct sof_intel_hda_stream *hda_stream; /* free position buffer */ @@ -986,9 +986,9 @@ void hda_dsp_stream_free(struct snd_sof_dev *sdev) if (s->bdl.area) snd_dma_free_pages(&s->bdl); list_del(&s->list); - stream = stream_to_hdac_ext_stream(s); - hda_stream = container_of(stream, struct sof_intel_hda_stream, - hda_stream); + he_stream = stream_to_hdac_ext_stream(s); + hda_stream = container_of(he_stream, struct sof_intel_hda_stream, + he_stream); devm_kfree(sdev->dev, hda_stream); } } diff --git a/sound/soc/sof/intel/hda-trace.c b/sound/soc/sof/intel/hda-trace.c index 29e3da3c63db01..1a87b7f307cb94 100644 --- a/sound/soc/sof/intel/hda-trace.c +++ b/sound/soc/sof/intel/hda-trace.c @@ -22,15 +22,15 @@ static int hda_dsp_trace_prepare(struct snd_sof_dev *sdev) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; - struct hdac_ext_stream *stream = hda->dtrace_stream; - struct hdac_stream *hstream = &stream->hstream; + struct hdac_ext_stream *he_stream = hda->dtrace_stream; + struct hdac_stream *hstream = &he_stream->hstream; struct snd_dma_buffer *dmab = &sdev->dmatb; int ret; hstream->period_bytes = 0;/* initialize period_bytes */ hstream->bufsize = sdev->dmatb.bytes; - ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL); + ret = hda_dsp_stream_hw_params(sdev, he_stream, dmab, NULL); if (ret < 0) dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 5629608dc5b4f2..a018a4e5b82cb5 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -471,14 +471,14 @@ 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; + struct hdac_ext_stream he_stream; + struct sof_intel_stream sof_intel_stream; int host_reserved; /* reserve host DMA channel */ u32 flags; }; #define hstream_to_sof_hda_stream(hstream) \ - container_of(hstream, struct sof_intel_hda_stream, hda_stream) + container_of(hstream, struct sof_intel_hda_stream, he_stream) #define bus_to_sof_hda(bus) \ container_of(bus, struct sof_intel_hda_dev, hbus.core) @@ -544,18 +544,19 @@ int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substrea int hda_dsp_stream_init(struct snd_sof_dev *sdev); void hda_dsp_stream_free(struct snd_sof_dev *sdev); int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, + struct hdac_ext_stream *he_stream, struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params); -int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream, +int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, + struct hdac_ext_stream *he_stream, struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params); int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, int cmd); + struct hdac_ext_stream *he_stream, int cmd); irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context); int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, - struct hdac_stream *stream); + struct hdac_stream *hstream); bool hda_dsp_check_ipc_irq(struct snd_sof_dev *sdev); bool hda_dsp_check_stream_irq(struct snd_sof_dev *sdev); @@ -563,7 +564,7 @@ struct hdac_ext_stream * hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags); int hda_dsp_stream_put(struct snd_sof_dev *sdev, int direction, int stream_tag); int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev, - struct hdac_ext_stream *stream, + struct hdac_ext_stream *he_stream, int enable, u32 size); int hda_ipc_msg_data(struct snd_sof_dev *sdev, From 1e300e79f3074f7eccaf11920e639baa7b8bb0dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 15:38:23 -0500 Subject: [PATCH 03/40] ASoC/SoundWire: dai: expand 'stream' concept beyond SoundWire The HDAudio ASoC support relies on the set_tdm_slots() helper to store the HDaudio stream tag in the tx_mask. This only works because of the pre-existing order in soc-pcm.c, where the hw_params() is handled for codec_dais *before* cpu_dais. When the order is reversed, the stream_tag is used as a mask in the codec fixup functions: /* fixup params based on TDM slot masks */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && codec_dai->tx_mask) soc_pcm_codec_params_fixup(&codec_params, codec_dai->tx_mask); As a result of this confusion, the codec_params_fixup() ends-up generating bad channel masks, depending on what stream_tag was allocated. We could add a flag to state that the tx_mask is really not a mask, but it would be quite ugly to persist in overloading concepts. Instead, this patch suggests a more generic get/set 'stream' API based on the existing model for SoundWire. We can expand the concept to store 'stream' opaque information that is specific to different DAI types. In the case of HDAudio DAIs, we only need to store a stream tag as an unsigned char pointer. The TDM rx_ and tx_masks should really only be used to store masks. Rename get_sdw_stream/set_sdw_stream callbacks and helpers as get_stream/set_stream. No functionality change beyond the rename. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 8 ++++---- drivers/soundwire/qcom.c | 8 ++++---- drivers/soundwire/stream.c | 4 ++-- include/sound/soc-dai.h | 32 ++++++++++++++++---------------- sound/soc/codecs/max98373-sdw.c | 2 +- sound/soc/codecs/rt1308-sdw.c | 2 +- sound/soc/codecs/rt1316-sdw.c | 2 +- sound/soc/codecs/rt5682-sdw.c | 2 +- sound/soc/codecs/rt700.c | 2 +- sound/soc/codecs/rt711-sdca.c | 2 +- sound/soc/codecs/rt711.c | 2 +- sound/soc/codecs/rt715-sdca.c | 2 +- sound/soc/codecs/rt715.c | 2 +- sound/soc/codecs/sdw-mockup.c | 2 +- sound/soc/codecs/wcd938x.c | 2 +- sound/soc/codecs/wsa881x.c | 2 +- sound/soc/intel/boards/sof_sdw.c | 6 +++--- sound/soc/qcom/sdm845.c | 4 ++-- sound/soc/qcom/sm8250.c | 4 ++-- 19 files changed, 45 insertions(+), 45 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index f95c3cc231e2e9..53505e1c0db09b 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1070,8 +1070,8 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .hw_free = intel_hw_free, .trigger = intel_trigger, .shutdown = intel_shutdown, - .set_sdw_stream = intel_pcm_set_sdw_stream, - .get_sdw_stream = intel_get_sdw_stream, + .set_stream = intel_pcm_set_sdw_stream, + .get_stream = intel_get_sdw_stream, }; static const struct snd_soc_dai_ops intel_pdm_dai_ops = { @@ -1081,8 +1081,8 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_free = intel_hw_free, .trigger = intel_trigger, .shutdown = intel_shutdown, - .set_sdw_stream = intel_pdm_set_sdw_stream, - .get_sdw_stream = intel_get_sdw_stream, + .set_stream = intel_pdm_set_sdw_stream, + .get_stream = intel_get_sdw_stream, }; static const struct snd_soc_component_driver dai_component = { diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 0ef79d60e88e6d..a5d375389732bb 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1019,8 +1019,8 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream, ctrl->sruntime[dai->id] = sruntime; for_each_rtd_codec_dais(rtd, i, codec_dai) { - ret = snd_soc_dai_set_sdw_stream(codec_dai, sruntime, - substream->stream); + ret = snd_soc_dai_set_stream(codec_dai, sruntime, + substream->stream); if (ret < 0 && ret != -ENOTSUPP) { dev_err(dai->dev, "Failed to set sdw stream on %s\n", codec_dai->name); @@ -1046,8 +1046,8 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = { .hw_free = qcom_swrm_hw_free, .startup = qcom_swrm_startup, .shutdown = qcom_swrm_shutdown, - .set_sdw_stream = qcom_swrm_set_sdw_stream, - .get_sdw_stream = qcom_swrm_get_sdw_stream, + .set_stream = qcom_swrm_set_sdw_stream, + .get_stream = qcom_swrm_get_sdw_stream, }; static const struct snd_soc_component_driver qcom_swrm_dai_component = { diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 5d4f6b308ef731..980f26d49b66f0 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1863,7 +1863,7 @@ static int set_stream(struct snd_pcm_substream *substream, /* Set stream pointer on all DAIs */ for_each_rtd_dais(rtd, i, dai) { - ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream); + ret = snd_soc_dai_set_stream(dai, sdw_stream, substream->stream); if (ret < 0) { dev_err(rtd->dev, "failed to set stream pointer on dai %s\n", dai->name); break; @@ -1934,7 +1934,7 @@ void sdw_shutdown_stream(void *sdw_substream) /* Find stream from first CPU DAI */ dai = asoc_rtd_to_cpu(rtd, 0); - sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream); + sdw_stream = snd_soc_dai_get_stream(dai, substream->stream); if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s\n", dai->name); diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 0dcb361a98bb34..673a53ee9abc5d 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -295,9 +295,9 @@ struct snd_soc_dai_ops { unsigned int *rx_num, unsigned int *rx_slot); int (*set_tristate)(struct snd_soc_dai *dai, int tristate); - int (*set_sdw_stream)(struct snd_soc_dai *dai, - void *stream, int direction); - void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); + int (*set_stream)(struct snd_soc_dai *dai, + void *stream, int direction); + void *(*get_stream)(struct snd_soc_dai *dai, int direction); /* * DAI digital mute - optional. @@ -515,42 +515,42 @@ static inline void *snd_soc_dai_get_drvdata(struct snd_soc_dai *dai) } /** - * snd_soc_dai_set_sdw_stream() - Configures a DAI for SDW stream operation + * snd_soc_dai_set_stream() - Configures a DAI for stream operation * @dai: DAI - * @stream: STREAM + * @stream: STREAM (opaque structure depending on DAI type * @direction: Stream direction(Playback/Capture) - * SoundWire subsystem doesn't have a notion of direction and we reuse + * Some subsystems, such as SoundWire, don't have a notion of direction and we reuse * the ASoC stream direction to configure sink/source ports. * Playback maps to source ports and Capture for sink ports. * * This should be invoked with NULL to clear the stream set previously. * Returns 0 on success, a negative error code otherwise. */ -static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, - void *stream, int direction) +static inline int snd_soc_dai_set_stream(struct snd_soc_dai *dai, + void *stream, int direction) { - if (dai->driver->ops->set_sdw_stream) - return dai->driver->ops->set_sdw_stream(dai, stream, direction); + if (dai->driver->ops->set_stream) + return dai->driver->ops->set_stream(dai, stream, direction); else return -ENOTSUPP; } /** - * snd_soc_dai_get_sdw_stream() - Retrieves SDW stream from DAI + * snd_soc_dai_get_stream() - Retrieves stream from DAI * @dai: DAI * @direction: Stream direction(Playback/Capture) * * This routine only retrieves that was previously configured - * with snd_soc_dai_get_sdw_stream() + * with snd_soc_dai_get_stream() * * Returns pointer to stream or an ERR_PTR value, e.g. * ERR_PTR(-ENOTSUPP) if callback is not supported; */ -static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, - int direction) +static inline void *snd_soc_dai_get_stream(struct snd_soc_dai *dai, + int direction) { - if (dai->driver->ops->get_sdw_stream) - return dai->driver->ops->get_sdw_stream(dai, direction); + if (dai->driver->ops->get_stream) + return dai->driver->ops->get_stream(dai, direction); else return ERR_PTR(-ENOTSUPP); } diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c index dc520effc61cb8..f47e956d4f55ac 100644 --- a/sound/soc/codecs/max98373-sdw.c +++ b/sound/soc/codecs/max98373-sdw.c @@ -741,7 +741,7 @@ static int max98373_sdw_set_tdm_slot(struct snd_soc_dai *dai, static const struct snd_soc_dai_ops max98373_dai_sdw_ops = { .hw_params = max98373_sdw_dai_hw_params, .hw_free = max98373_pcm_hw_free, - .set_sdw_stream = max98373_set_sdw_stream, + .set_stream = max98373_set_sdw_stream, .shutdown = max98373_shutdown, .set_tdm_slot = max98373_sdw_set_tdm_slot, }; diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index f716668de6400e..149a76075c76a1 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -613,7 +613,7 @@ static const struct snd_soc_component_driver soc_component_sdw_rt1308 = { static const struct snd_soc_dai_ops rt1308_aif_dai_ops = { .hw_params = rt1308_sdw_hw_params, .hw_free = rt1308_sdw_pcm_hw_free, - .set_sdw_stream = rt1308_set_sdw_stream, + .set_stream = rt1308_set_sdw_stream, .shutdown = rt1308_sdw_shutdown, .set_tdm_slot = rt1308_sdw_set_tdm_slot, }; diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c index 09b4914bba1bfe..c66d7b20cb4dda 100644 --- a/sound/soc/codecs/rt1316-sdw.c +++ b/sound/soc/codecs/rt1316-sdw.c @@ -602,7 +602,7 @@ static const struct snd_soc_component_driver soc_component_sdw_rt1316 = { static const struct snd_soc_dai_ops rt1316_aif_dai_ops = { .hw_params = rt1316_sdw_hw_params, .hw_free = rt1316_sdw_pcm_hw_free, - .set_sdw_stream = rt1316_set_sdw_stream, + .set_stream = rt1316_set_sdw_stream, .shutdown = rt1316_sdw_shutdown, }; diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 31a4f286043e46..248257a2e4e0f3 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -272,7 +272,7 @@ static int rt5682_sdw_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt5682_sdw_ops = { .hw_params = rt5682_sdw_hw_params, .hw_free = rt5682_sdw_hw_free, - .set_sdw_stream = rt5682_set_sdw_stream, + .set_stream = rt5682_set_sdw_stream, .shutdown = rt5682_sdw_shutdown, }; diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c index 921382724f9cdb..e61a8257bf6470 100644 --- a/sound/soc/codecs/rt700.c +++ b/sound/soc/codecs/rt700.c @@ -1005,7 +1005,7 @@ static int rt700_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt700_ops = { .hw_params = rt700_pcm_hw_params, .hw_free = rt700_pcm_hw_free, - .set_sdw_stream = rt700_set_sdw_stream, + .set_stream = rt700_set_sdw_stream, .shutdown = rt700_shutdown, }; diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c index 2e992589f1e420..bdb1375f033885 100644 --- a/sound/soc/codecs/rt711-sdca.c +++ b/sound/soc/codecs/rt711-sdca.c @@ -1358,7 +1358,7 @@ static int rt711_sdca_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt711_sdca_ops = { .hw_params = rt711_sdca_pcm_hw_params, .hw_free = rt711_sdca_pcm_hw_free, - .set_sdw_stream = rt711_sdca_set_sdw_stream, + .set_stream = rt711_sdca_set_sdw_stream, .shutdown = rt711_sdca_shutdown, }; diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c index a7c5608a0ef879..6770825d037a8a 100644 --- a/sound/soc/codecs/rt711.c +++ b/sound/soc/codecs/rt711.c @@ -1089,7 +1089,7 @@ static int rt711_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt711_ops = { .hw_params = rt711_pcm_hw_params, .hw_free = rt711_pcm_hw_free, - .set_sdw_stream = rt711_set_sdw_stream, + .set_stream = rt711_set_sdw_stream, .shutdown = rt711_shutdown, }; diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c index 66e166568c5087..bfa536bd71960c 100644 --- a/sound/soc/codecs/rt715-sdca.c +++ b/sound/soc/codecs/rt715-sdca.c @@ -938,7 +938,7 @@ static int rt715_sdca_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt715_sdca_ops = { .hw_params = rt715_sdca_pcm_hw_params, .hw_free = rt715_sdca_pcm_hw_free, - .set_sdw_stream = rt715_sdca_set_sdw_stream, + .set_stream = rt715_sdca_set_sdw_stream, .shutdown = rt715_sdca_shutdown, }; diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index 1352869cc08670..a64d11a7475136 100644 --- a/sound/soc/codecs/rt715.c +++ b/sound/soc/codecs/rt715.c @@ -909,7 +909,7 @@ static int rt715_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops rt715_ops = { .hw_params = rt715_pcm_hw_params, .hw_free = rt715_pcm_hw_free, - .set_sdw_stream = rt715_set_sdw_stream, + .set_stream = rt715_set_sdw_stream, .shutdown = rt715_shutdown, }; diff --git a/sound/soc/codecs/sdw-mockup.c b/sound/soc/codecs/sdw-mockup.c index 8ea13cfa9f8ede..7c612aaf31c75c 100644 --- a/sound/soc/codecs/sdw-mockup.c +++ b/sound/soc/codecs/sdw-mockup.c @@ -138,7 +138,7 @@ static int sdw_mockup_pcm_hw_free(struct snd_pcm_substream *substream, static const struct snd_soc_dai_ops sdw_mockup_ops = { .hw_params = sdw_mockup_pcm_hw_params, .hw_free = sdw_mockup_pcm_hw_free, - .set_sdw_stream = sdw_mockup_set_sdw_stream, + .set_stream = sdw_mockup_set_sdw_stream, .shutdown = sdw_mockup_shutdown, }; diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c index f0daf8defcf1ed..105fc89304287d 100644 --- a/sound/soc/codecs/wcd938x.c +++ b/sound/soc/codecs/wcd938x.c @@ -4284,7 +4284,7 @@ static int wcd938x_codec_set_sdw_stream(struct snd_soc_dai *dai, static const struct snd_soc_dai_ops wcd938x_sdw_dai_ops = { .hw_params = wcd938x_codec_hw_params, .hw_free = wcd938x_codec_free, - .set_sdw_stream = wcd938x_codec_set_sdw_stream, + .set_stream = wcd938x_codec_set_sdw_stream, }; static struct snd_soc_dai_driver wcd938x_dais[] = { diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c index 2da4a5fa7a18d8..ffc025e01bce47 100644 --- a/sound/soc/codecs/wsa881x.c +++ b/sound/soc/codecs/wsa881x.c @@ -1018,7 +1018,7 @@ static const struct snd_soc_dai_ops wsa881x_dai_ops = { .hw_params = wsa881x_hw_params, .hw_free = wsa881x_hw_free, .mute_stream = wsa881x_digital_mute, - .set_sdw_stream = wsa881x_set_sdw_stream, + .set_stream = wsa881x_set_sdw_stream, }; static struct snd_soc_dai_driver wsa881x_dais[] = { diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 3515d18b9c6243..25e8a255064e24 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -291,7 +291,7 @@ int sdw_prepare(struct snd_pcm_substream *substream) /* Find stream from first CPU DAI */ dai = asoc_rtd_to_cpu(rtd, 0); - sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream); + sdw_stream = snd_soc_dai_get_stream(dai, substream->stream); if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s", dai->name); @@ -311,7 +311,7 @@ int sdw_trigger(struct snd_pcm_substream *substream, int cmd) /* Find stream from first CPU DAI */ dai = asoc_rtd_to_cpu(rtd, 0); - sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream); + sdw_stream = snd_soc_dai_get_stream(dai, substream->stream); if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s", dai->name); @@ -350,7 +350,7 @@ int sdw_hw_free(struct snd_pcm_substream *substream) /* Find stream from first CPU DAI */ dai = asoc_rtd_to_cpu(rtd, 0); - sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream); + sdw_stream = snd_soc_dai_get_stream(dai, substream->stream); if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s", dai->name); diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c index 0adfc570894928..4da5ad609fcea9 100644 --- a/sound/soc/qcom/sdm845.c +++ b/sound/soc/qcom/sdm845.c @@ -56,8 +56,8 @@ static int sdm845_slim_snd_hw_params(struct snd_pcm_substream *substream, int ret = 0, i; for_each_rtd_codec_dais(rtd, i, codec_dai) { - sruntime = snd_soc_dai_get_sdw_stream(codec_dai, - substream->stream); + sruntime = snd_soc_dai_get_stream(codec_dai, + substream->stream); if (sruntime != ERR_PTR(-ENOTSUPP)) pdata->sruntime[cpu_dai->id] = sruntime; diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c index fe8fd7367e21b2..8f3e6953df30ef 100644 --- a/sound/soc/qcom/sm8250.c +++ b/sound/soc/qcom/sm8250.c @@ -70,8 +70,8 @@ static int sm8250_snd_hw_params(struct snd_pcm_substream *substream, switch (cpu_dai->id) { case WSA_CODEC_DMA_RX_0: for_each_rtd_codec_dais(rtd, i, codec_dai) { - sruntime = snd_soc_dai_get_sdw_stream(codec_dai, - substream->stream); + sruntime = snd_soc_dai_get_stream(codec_dai, + substream->stream); if (sruntime != ERR_PTR(-ENOTSUPP)) pdata->sruntime[cpu_dai->id] = sruntime; } From c18456ecabe1847bd93bd8bb6e72c7d23c39cd3a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 16:17:21 -0500 Subject: [PATCH 04/40] ASoC: Intel/SOF: use set_stream() instead of set_tdm_slots() for HDAudio Overloading the tx_mask with a linear value is asking for trouble and only works because the codec_dai hw_params() is called before the cpu_dai hw_params(). Move to the more generic set_stream() API to pass the hdac_stream information. Signed-off-by: Pierre-Louis Bossart --- sound/soc/codecs/hdac_hda.c | 22 +++++++++++----------- sound/soc/intel/skylake/skl-pcm.c | 7 ++----- sound/soc/sof/intel/hda-dai.c | 7 ++----- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c index 390dd6c7f6a506..de5955db0a5f02 100644 --- a/sound/soc/codecs/hdac_hda.c +++ b/sound/soc/codecs/hdac_hda.c @@ -46,9 +46,8 @@ static int hdac_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai); static int hdac_hda_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai); -static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai, - unsigned int tx_mask, unsigned int rx_mask, - int slots, int slot_width); +static int hdac_hda_dai_set_stream(struct snd_soc_dai *dai, void *stream, + int direction); static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt, struct snd_soc_dai *dai); @@ -58,7 +57,7 @@ static const struct snd_soc_dai_ops hdac_hda_dai_ops = { .prepare = hdac_hda_dai_prepare, .hw_params = hdac_hda_dai_hw_params, .hw_free = hdac_hda_dai_hw_free, - .set_tdm_slot = hdac_hda_dai_set_tdm_slot, + .set_stream = hdac_hda_dai_set_stream, }; static struct snd_soc_dai_driver hdac_hda_dais[] = { @@ -180,21 +179,22 @@ static struct snd_soc_dai_driver hdac_hda_dais[] = { }; -static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai, - unsigned int tx_mask, unsigned int rx_mask, - int slots, int slot_width) +static int hdac_hda_dai_set_stream(struct snd_soc_dai *dai, + void *stream, int direction) { struct snd_soc_component *component = dai->component; struct hdac_hda_priv *hda_pvt; struct hdac_hda_pcm *pcm; + struct hdac_stream *hstream; + + if (!stream) + return -EINVAL; hda_pvt = snd_soc_component_get_drvdata(component); pcm = &hda_pvt->pcm[dai->id]; + hstream = (struct hdac_stream *)stream; - if (tx_mask) - pcm->stream_tag[SNDRV_PCM_STREAM_PLAYBACK] = tx_mask; - else - pcm->stream_tag[SNDRV_PCM_STREAM_CAPTURE] = rx_mask; + pcm->stream_tag[direction] = hstream->stream_tag; return 0; } diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 9ecaf6a1e8475d..8378c187959fb7 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -562,11 +562,8 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream, stream_tag = hdac_stream(link_dev)->stream_tag; - /* 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 - snd_soc_dai_set_tdm_slot(codec_dai, 0, stream_tag, 0, 0); + /* set the hdac_stream in the codec dai */ + snd_soc_dai_set_stream(codec_dai, hdac_stream(link_dev), substream->stream); p_params.s_fmt = snd_pcm_format_width(params_format(params)); p_params.ch = params_channels(params); diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 84932d31d75c5a..049f52be54cead 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -245,11 +245,8 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, if (!link) return -EINVAL; - /* 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 - snd_soc_dai_set_tdm_slot(codec_dai, 0, stream_tag, 0, 0); + /* set the hdac_stream in the codec dai */ + snd_soc_dai_set_stream(codec_dai, hdac_stream(link_dev), substream->stream); p_params.s_fmt = snd_pcm_format_width(params_format(params)); p_params.ch = params_channels(params); From 2fa550d4edd84899b53654290cf1a9f9f45dbec1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 08:50:07 -0500 Subject: [PATCH 05/40] ASOC: SOF: Intel: use snd_soc_dai_get_widget() We have a helper, use it to simplify widget lookup Suggested-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 20 ++++---------------- sound/soc/sof/intel/hda.c | 10 ++-------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 049f52be54cead..748220cba6d98e 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -231,10 +231,7 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, hda_stream = hstream_to_sof_hda_stream(he_stream); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = dai->playback_widget; - else - w = dai->capture_widget; + w = snd_soc_dai_get_widget(dai, substream->stream); /* set up the DAI widget and send the DAI_CONFIG with the new tag */ ret = hda_link_dai_widget_update(hda_stream, w, stream_tag - 1, true); @@ -325,10 +322,7 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = dai->playback_widget; - else - w = dai->capture_widget; + w = snd_soc_dai_get_widget(dai, substream->stream); /* * free DAI widget during stop/suspend to keep widget use_count's balanced. @@ -380,10 +374,7 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, hda_stream = hstream_to_sof_hda_stream(he_stream); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = dai->playback_widget; - else - w = dai->capture_widget; + w = snd_soc_dai_get_widget(dai, substream->stream); /* free the link DMA channel in the FW and the DAI widget */ ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); @@ -432,10 +423,7 @@ static int ssp_dai_setup_or_free(struct snd_pcm_substream *substream, struct snd struct sof_ipc_fw_version *v; struct snd_sof_dev *sdev; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = dai->playback_widget; - else - w = dai->capture_widget; + w = snd_soc_dai_get_widget(dai, substream->stream); swidget = w->dobj.private; component = swidget->scomp; diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 6a8c699ac52c7f..7476d106f3cdcb 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -186,10 +186,7 @@ static int sdw_params_stream(struct device *dev, struct snd_soc_dai *d = params_data->dai; struct snd_soc_dapm_widget *w; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = d->playback_widget; - else - w = d->capture_widget; + w = snd_soc_dai_get_widget(d, substream->stream); return sdw_dai_config_ipc(sdev, w, params_data->link_id, params_data->alh_stream_id, d->id, true); @@ -203,10 +200,7 @@ static int sdw_free_stream(struct device *dev, struct snd_soc_dai *d = free_data->dai; struct snd_soc_dapm_widget *w; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - w = d->playback_widget; - else - w = d->capture_widget; + w = snd_soc_dai_get_widget(d, substream->stream); /* send invalid stream_id */ return sdw_dai_config_ipc(sdev, w, free_data->link_id, 0xFFFF, d->id, false); From c2656d12873c3d97d4331123fb0d28e6206c50b2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 21:53:03 -0500 Subject: [PATCH 06/40] ASoC: SOF: Intel: hda-dai: remove support for TRIGGER_RESUME TRIGGER_RESUME is not supported on Intel platforms, let's remove untested/dead code. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 748220cba6d98e..f5da77f786c646 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -306,16 +306,6 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd); switch (cmd) { - case SNDRV_PCM_TRIGGER_RESUME: - /* set up hw_params */ - ret = hda_link_pcm_prepare(substream, dai); - if (ret < 0) { - dev_err(dai->dev, - "error: setting up hw_params during resume\n"); - return ret; - } - - fallthrough; case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: snd_hdac_ext_link_stream_start(he_stream); From fb422e77004a8c5924f6d62a82a0bb432d7353c3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 21:41:35 -0500 Subject: [PATCH 07/40] ASOC: SOF: Intel: hda-dai: consistent naming for HDA DAI and HDA link DMA The Intel documentation refers to the concepts of 'HDAudio host DMA' (system memory <--> DSP) and 'HDaudio link DMA' (DSP <--> peripherals). We currently use the prefix 'hda_link' to describe DAI operations, which can be confused for dailink operations. Since the topology tokens refer unambiguously to the 'HDA' dai, let's drop the link prefix for dai-related ops/callbacks. Conversely let's use 'hda_link_dma' for routines related to the DMA management. In a follow-up patch we will introduce the 'hda_dai_link' prefix for dailink ops/callbacks. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 61 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index f5da77f786c646..e46f76f8fb6cbc 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -57,8 +57,8 @@ static bool hda_check_fes(struct snd_soc_pcm_runtime *rtd, } static struct hdac_ext_stream * - hda_link_stream_assign(struct hdac_bus *bus, - struct snd_pcm_substream *substream) +hda_link_dma_stream_assign(struct hdac_bus *bus, + struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct sof_intel_hda_stream *hda_stream; @@ -181,9 +181,9 @@ static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widg return config; } -static int hda_link_dai_widget_update(struct sof_intel_hda_stream *hda_stream, - struct snd_soc_dapm_widget *w, - int channel, bool widget_setup) +static int hda_dai_widget_update(struct sof_intel_hda_stream *hda_stream, + struct snd_soc_dapm_widget *w, + int channel, bool widget_setup) { struct snd_sof_dev *sdev = hda_stream->sdev; struct sof_ipc_dai_config *config; @@ -201,9 +201,9 @@ static int hda_link_dai_widget_update(struct sof_intel_hda_stream *hda_stream, return hda_ctrl_dai_widget_free(w); } -static int hda_link_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) +static int hda_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) { struct hdac_stream *hstream = substream->runtime->private_data; struct hdac_bus *bus = hstream->bus; @@ -234,7 +234,7 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, w = snd_soc_dai_get_widget(dai, substream->stream); /* set up the DAI widget and send the DAI_CONFIG with the new tag */ - ret = hda_link_dai_widget_update(hda_stream, w, stream_tag - 1, true); + ret = hda_dai_widget_update(hda_stream, w, stream_tag - 1, true); if (ret < 0) return ret; @@ -261,8 +261,8 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, return hda_link_dma_params(he_stream, &p_params); } -static int hda_link_pcm_prepare(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) +static int hda_dai_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) { struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(dai, substream); @@ -276,12 +276,11 @@ static int hda_link_pcm_prepare(struct snd_pcm_substream *substream, dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream); - return hda_link_hw_params(substream, &rtd->dpcm[stream].hw_params, - dai); + return hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, dai); } -static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_dai *dai) +static int hda_dai_trigger(struct snd_pcm_substream *substream, + int cmd, struct snd_soc_dai *dai) { struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(dai, substream); @@ -317,7 +316,7 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, /* * free DAI widget during stop/suspend to keep widget use_count's balanced. */ - ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); + ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); if (ret < 0) return ret; @@ -338,8 +337,8 @@ static int hda_link_pcm_trigger(struct snd_pcm_substream *substream, return 0; } -static int hda_link_hw_free(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) +static int hda_dai_hw_free(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) { unsigned int stream_tag; struct sof_intel_hda_stream *hda_stream; @@ -367,7 +366,7 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, w = snd_soc_dai_get_widget(dai, substream->stream); /* free the link DMA channel in the FW and the DAI widget */ - ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); + ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); if (ret < 0) return ret; @@ -390,11 +389,11 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, return 0; } -static const struct snd_soc_dai_ops hda_link_dai_ops = { - .hw_params = hda_link_hw_params, - .hw_free = hda_link_hw_free, - .trigger = hda_link_pcm_trigger, - .prepare = hda_link_pcm_prepare, +static const struct snd_soc_dai_ops hda_dai_ops = { + .hw_params = hda_dai_hw_params, + .hw_free = hda_dai_hw_free, + .trigger = hda_dai_trigger, + .prepare = hda_dai_prepare, }; #endif @@ -616,7 +615,7 @@ struct snd_soc_dai_driver skl_dai[] = { #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) { .name = "iDisp1 Pin", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 8, @@ -624,7 +623,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "iDisp2 Pin", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 8, @@ -632,7 +631,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "iDisp3 Pin", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 8, @@ -640,7 +639,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "iDisp4 Pin", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 8, @@ -648,7 +647,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "Analog CPU DAI", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 16, @@ -660,7 +659,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "Digital CPU DAI", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 16, @@ -672,7 +671,7 @@ struct snd_soc_dai_driver skl_dai[] = { }, { .name = "Alt Analog CPU DAI", - .ops = &hda_link_dai_ops, + .ops = &hda_dai_ops, .playback = { .channels_min = 1, .channels_max = 16, From 3c6ad2ce4cdfbbe5c4798496c4db24df6905e3ea Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 22:36:56 -0500 Subject: [PATCH 08/40] ASoC: SOF: Intel: hda-dai: simplify hda_dai_widget_update() prototype We don't need to pass a hda_stream to find the sdev to find a device for dev_dbg. Just pass the device. While we're at it, also update error log Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index e46f76f8fb6cbc..1ebba08c89bc79 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -181,16 +181,15 @@ static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widg return config; } -static int hda_dai_widget_update(struct sof_intel_hda_stream *hda_stream, +static int hda_dai_widget_update(struct device *dev, struct snd_soc_dapm_widget *w, int channel, bool widget_setup) { - struct snd_sof_dev *sdev = hda_stream->sdev; struct sof_ipc_dai_config *config; config = hda_dai_update_config(w, channel); if (!config) { - dev_err(sdev->dev, "error: no config for DAI %s\n", w->name); + dev_err(dev, "%s: no config for DAI %s\n", __func__, w->name); return -ENOENT; } @@ -234,7 +233,7 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream, w = snd_soc_dai_get_widget(dai, substream->stream); /* set up the DAI widget and send the DAI_CONFIG with the new tag */ - ret = hda_dai_widget_update(hda_stream, w, stream_tag - 1, true); + ret = hda_dai_widget_update(dai->dev, w, stream_tag - 1, true); if (ret < 0) return ret; @@ -316,7 +315,7 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, /* * free DAI widget during stop/suspend to keep widget use_count's balanced. */ - ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); if (ret < 0) return ret; @@ -366,7 +365,7 @@ static int hda_dai_hw_free(struct snd_pcm_substream *substream, w = snd_soc_dai_get_widget(dai, substream->stream); /* free the link DMA channel in the FW and the DAI widget */ - ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false); + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); if (ret < 0) return ret; From 72330f9273622c983c2474ca6a14f1c4a08b4471 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 22:33:18 -0500 Subject: [PATCH 09/40] ASoC: SOF: Intel: hda-dai: split dailink and dai operations First split the dailink and dai operations in separate functions, that will still be called from the dai callbacks. In an ideal world, these dailink routines would be moved and invoked directly from the dailink callbacks. This would be quite invasive and impact multiple machine drivers, so for now we keep the split internal to the SOF driver. The main intent here is to split 'link management' from 'SOF IPC'. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 217 +++++++++++++++++++++------------- 1 file changed, 136 insertions(+), 81 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 1ebba08c89bc79..a36579001a03cb 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -200,49 +200,37 @@ static int hda_dai_widget_update(struct device *dev, return hda_ctrl_dai_widget_free(w); } -static int hda_dai_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) +static int hda_dai_link_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) { struct hdac_stream *hstream = substream->runtime->private_data; - struct hdac_bus *bus = hstream->bus; - struct hdac_ext_stream *he_stream; struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); - struct sof_intel_hda_stream *hda_stream; + struct hdac_ext_stream *he_stream; struct hda_pipe_params p_params = {0}; - struct snd_soc_dapm_widget *w; + struct hdac_bus *bus = hstream->bus; struct hdac_ext_link *link; int stream_tag; - int ret; /* get stored dma data if resuming from system suspend */ - he_stream = snd_soc_dai_get_dma_data(dai, substream); + he_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); if (!he_stream) { - he_stream = hda_link_stream_assign(bus, substream); + he_stream = hda_link_dma_stream_assign(bus, substream); if (!he_stream) return -EBUSY; - snd_soc_dai_set_dma_data(dai, substream, (void *)he_stream); + snd_soc_dai_set_dma_data(cpu_dai, substream, (void *)he_stream); } stream_tag = hdac_stream(he_stream)->stream_tag; - hda_stream = hstream_to_sof_hda_stream(he_stream); - - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* set up the DAI widget and send the DAI_CONFIG with the new tag */ - ret = hda_dai_widget_update(dai->dev, w, stream_tag - 1, true); - if (ret < 0) - return ret; - link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); if (!link) return -EINVAL; /* set the hdac_stream in the codec dai */ - snd_soc_dai_set_stream(codec_dai, hdac_stream(link_dev), substream->stream); + snd_soc_dai_set_stream(codec_dai, hdac_stream(he_stream), substream->stream); p_params.s_fmt = snd_pcm_format_width(params_format(params)); p_params.ch = params_channels(params); @@ -260,49 +248,88 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream, return hda_link_dma_params(he_stream, &p_params); } -static int hda_dai_prepare(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) +static int hda_dai_hw_params_update(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct hdac_ext_stream *he_stream; + struct snd_soc_dapm_widget *w; + int stream_tag; + + he_stream = snd_soc_dai_get_dma_data(dai, substream); + if (!he_stream) + return -EINVAL; + + stream_tag = hdac_stream(he_stream)->stream_tag; + + w = snd_soc_dai_get_widget(dai, substream->stream); + + /* set up the DAI widget and send the DAI_CONFIG with the new tag */ + return hda_dai_widget_update(dai->dev, w, stream_tag - 1, true); +} + +static int hda_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + int ret; + + ret = hda_dai_link_hw_params(substream, params); + if (ret < 0) + return ret; + + return hda_dai_hw_params_update(substream, params, dai); +} + +static int hda_dai_link_prepare(struct snd_pcm_substream *substream) { - struct hdac_ext_stream *he_stream = - snd_soc_dai_get_dma_data(dai, substream); - struct snd_sof_dev *sdev = - snd_soc_component_get_drvdata(dai->component); struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); + struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); + struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(cpu_dai->component); int stream = substream->stream; if (he_stream->link_prepared) return 0; - dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream); + dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream); - return hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, dai); + return hda_dai_link_hw_params(substream, &rtd->dpcm[stream].hw_params); } -static int hda_dai_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_dai *dai) +static int hda_dai_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) { - struct hdac_ext_stream *he_stream = - snd_soc_dai_get_dma_data(dai, substream); - struct sof_intel_hda_stream *hda_stream; - struct snd_soc_pcm_runtime *rtd; - struct snd_soc_dapm_widget *w; - struct hdac_ext_link *link; - struct hdac_stream *hstream; - struct hdac_bus *bus; - int stream_tag; + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component); + int stream = substream->stream; int ret; - hstream = substream->runtime->private_data; - bus = hstream->bus; - rtd = asoc_substream_to_rtd(substream); + dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream); - link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name); + ret = hda_dai_link_prepare(substream); + if (ret < 0) + return ret; + + return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai); +} + +static int hda_dai_link_trigger(struct snd_pcm_substream *substream, int cmd) +{ + struct hdac_stream *hstream = substream->runtime->private_data; + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); + struct hdac_ext_stream *he_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); + struct hdac_ext_link *link; + struct hdac_bus *bus = hstream->bus; + int stream_tag; + + link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); if (!link) return -EINVAL; - hda_stream = hstream_to_sof_hda_stream(he_stream); - - dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd); + dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd); switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: @@ -310,15 +337,6 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* - * free DAI widget during stop/suspend to keep widget use_count's balanced. - */ - ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); - if (ret < 0) - return ret; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); @@ -336,40 +354,56 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, return 0; } -static int hda_dai_hw_free(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) +static int hda_dai_trigger(struct snd_pcm_substream *substream, + int cmd, struct snd_soc_dai *dai) { - 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 snd_soc_pcm_runtime *rtd; - struct hdac_ext_stream *he_stream; struct snd_soc_dapm_widget *w; int ret; - hstream = substream->runtime->private_data; - bus = hstream->bus; - rtd = asoc_substream_to_rtd(substream); - he_stream = snd_soc_dai_get_dma_data(dai, substream); + ret = hda_dai_link_trigger(substream, cmd); + if (ret < 0) + return ret; + + dev_dbg(dai->dev, "%s: cmd=%d\n", __func__, cmd); + switch (cmd) { + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_STOP: + w = snd_soc_dai_get_widget(dai, substream->stream); + + /* + * free DAI widget during stop/suspend to keep widget use_count's balanced. + */ + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); + if (ret < 0) + return ret; + + break; + default: + break; + } + return 0; +} + +static int hda_dai_link_hw_free(struct snd_pcm_substream *substream) +{ + struct hdac_stream *hstream = substream->runtime->private_data; + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); + struct sof_intel_hda_stream *hda_stream; + struct hdac_bus *bus = hstream->bus; + struct hdac_ext_stream *he_stream; + struct hdac_ext_link *link; + int stream_tag; + he_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); if (!he_stream) { - dev_dbg(dai->dev, + dev_dbg(cpu_dai->dev, "%s: he_stream is not assigned\n", __func__); return -EINVAL; } - hda_stream = hstream_to_sof_hda_stream(he_stream); - - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* free the link DMA channel in the FW and the DAI widget */ - ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); - if (ret < 0) - return ret; - - link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name); + link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); if (!link) return -EINVAL; @@ -378,16 +412,37 @@ static int hda_dai_hw_free(struct snd_pcm_substream *substream, snd_hdac_ext_link_clear_stream_id(link, stream_tag); } - snd_soc_dai_set_dma_data(dai, substream, NULL); + snd_soc_dai_set_dma_data(cpu_dai, substream, NULL); snd_hdac_ext_stream_release(he_stream, HDAC_EXT_STREAM_TYPE_LINK); he_stream->link_prepared = 0; /* free the host DMA channel reserved by hostless streams */ + hda_stream = hstream_to_sof_hda_stream(he_stream); hda_stream->host_reserved = 0; return 0; } +static int hda_dai_hw_free(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_dapm_widget *w; + int ret; + + ret = hda_dai_link_hw_free(substream); + if (ret < 0) + return ret; + + w = snd_soc_dai_get_widget(dai, substream->stream); + + /* free the link DMA channel in the FW and the DAI widget */ + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); + if (ret < 0) + return ret; + + return 0; +} + static const struct snd_soc_dai_ops hda_dai_ops = { .hw_params = hda_dai_hw_params, .hw_free = hda_dai_hw_free, From 5fd0931f3d8aee07180df848570747c7efe2c3dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 8 Sep 2021 15:06:43 -0500 Subject: [PATCH 10/40] ASoC: SOF: Intel: hda-dai: regroup dai and dailink operations Just code move with no functionality change, to clearly separate out the 'dai' operation from the 'dailink' ones. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 252 +++++++++++++++++----------------- 1 file changed, 127 insertions(+), 125 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index a36579001a03cb..6bae01b846876d 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -155,51 +155,6 @@ static int hda_link_dma_params(struct hdac_ext_stream *he_stream, return 0; } -/* Update config for the DAI widget */ -static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w, - int channel) -{ - struct snd_sof_widget *swidget = w->dobj.private; - struct sof_ipc_dai_config *config; - struct snd_sof_dai *sof_dai; - - if (!swidget) - return NULL; - - sof_dai = swidget->private; - - if (!sof_dai || !sof_dai->dai_config) { - dev_err(swidget->scomp->dev, "error: No config for DAI %s\n", w->name); - return NULL; - } - - config = &sof_dai->dai_config[sof_dai->current_config]; - - /* update config with stream tag */ - config->hda.link_dma_ch = channel; - - return config; -} - -static int hda_dai_widget_update(struct device *dev, - struct snd_soc_dapm_widget *w, - int channel, bool widget_setup) -{ - struct sof_ipc_dai_config *config; - - config = hda_dai_update_config(w, channel); - if (!config) { - dev_err(dev, "%s: no config for DAI %s\n", __func__, w->name); - return -ENOENT; - } - - /* set up/free DAI widget and send DAI_CONFIG IPC */ - if (widget_setup) - return hda_ctrl_dai_widget_setup(w); - - return hda_ctrl_dai_widget_free(w); -} - static int hda_dai_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -248,39 +203,6 @@ static int hda_dai_link_hw_params(struct snd_pcm_substream *substream, return hda_link_dma_params(he_stream, &p_params); } -static int hda_dai_hw_params_update(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) -{ - struct hdac_ext_stream *he_stream; - struct snd_soc_dapm_widget *w; - int stream_tag; - - he_stream = snd_soc_dai_get_dma_data(dai, substream); - if (!he_stream) - return -EINVAL; - - stream_tag = hdac_stream(he_stream)->stream_tag; - - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* set up the DAI widget and send the DAI_CONFIG with the new tag */ - return hda_dai_widget_update(dai->dev, w, stream_tag - 1, true); -} - -static int hda_dai_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) -{ - int ret; - - ret = hda_dai_link_hw_params(substream, params); - if (ret < 0) - return ret; - - return hda_dai_hw_params_update(substream, params, dai); -} - static int hda_dai_link_prepare(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); @@ -297,23 +219,6 @@ static int hda_dai_link_prepare(struct snd_pcm_substream *substream) return hda_dai_link_hw_params(substream, &rtd->dpcm[stream].hw_params); } -static int hda_dai_prepare(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component); - int stream = substream->stream; - int ret; - - dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream); - - ret = hda_dai_link_prepare(substream); - if (ret < 0) - return ret; - - return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai); -} - static int hda_dai_link_trigger(struct snd_pcm_substream *substream, int cmd) { struct hdac_stream *hstream = substream->runtime->private_data; @@ -354,36 +259,6 @@ static int hda_dai_link_trigger(struct snd_pcm_substream *substream, int cmd) return 0; } -static int hda_dai_trigger(struct snd_pcm_substream *substream, - int cmd, struct snd_soc_dai *dai) -{ - struct snd_soc_dapm_widget *w; - int ret; - - ret = hda_dai_link_trigger(substream, cmd); - if (ret < 0) - return ret; - - dev_dbg(dai->dev, "%s: cmd=%d\n", __func__, cmd); - switch (cmd) { - case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_STOP: - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* - * free DAI widget during stop/suspend to keep widget use_count's balanced. - */ - ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); - if (ret < 0) - return ret; - - break; - default: - break; - } - return 0; -} - static int hda_dai_link_hw_free(struct snd_pcm_substream *substream) { struct hdac_stream *hstream = substream->runtime->private_data; @@ -423,6 +298,133 @@ static int hda_dai_link_hw_free(struct snd_pcm_substream *substream) return 0; } +/* Update config for the DAI widget */ +static struct sof_ipc_dai_config *hda_dai_update_config(struct snd_soc_dapm_widget *w, + int channel) +{ + struct snd_sof_widget *swidget = w->dobj.private; + struct sof_ipc_dai_config *config; + struct snd_sof_dai *sof_dai; + + if (!swidget) { + dev_err(swidget->scomp->dev, "error: No private data for widget %s\n", w->name); + return NULL; + } + + sof_dai = swidget->private; + + if (!sof_dai || !sof_dai->dai_config) { + dev_err(swidget->scomp->dev, "error: No config for DAI %s\n", w->name); + return NULL; + } + + config = &sof_dai->dai_config[sof_dai->current_config]; + + /* update config with stream tag */ + config->hda.link_dma_ch = channel; + + return config; +} + +static int hda_dai_widget_update(struct device *dev, + struct snd_soc_dapm_widget *w, + int channel, bool widget_setup) +{ + struct sof_ipc_dai_config *config; + + config = hda_dai_update_config(w, channel); + if (!config) { + dev_err(dev, "%s: no config for DAI %s\n", __func__, w->name); + return -ENOENT; + } + + /* set up/free DAI widget and send DAI_CONFIG IPC */ + if (widget_setup) + return hda_ctrl_dai_widget_setup(w); + + return hda_ctrl_dai_widget_free(w); +} + +static int hda_dai_hw_params_update(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct hdac_ext_stream *he_stream; + struct snd_soc_dapm_widget *w; + int stream_tag; + + he_stream = snd_soc_dai_get_dma_data(dai, substream); + if (!he_stream) + return -EINVAL; + + stream_tag = hdac_stream(he_stream)->stream_tag; + + w = snd_soc_dai_get_widget(dai, substream->stream); + + /* set up the DAI widget and send the DAI_CONFIG with the new tag */ + return hda_dai_widget_update(dai->dev, w, stream_tag - 1, true); +} + +static int hda_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + int ret; + + ret = hda_dai_link_hw_params(substream, params); + if (ret < 0) + return ret; + + return hda_dai_hw_params_update(substream, params, dai); +} + +static int hda_dai_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component); + int stream = substream->stream; + int ret; + + dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream); + + ret = hda_dai_link_prepare(substream); + if (ret < 0) + return ret; + + return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai); +} + +static int hda_dai_trigger(struct snd_pcm_substream *substream, + int cmd, struct snd_soc_dai *dai) +{ + struct snd_soc_dapm_widget *w; + int ret; + + ret = hda_dai_link_trigger(substream, cmd); + if (ret < 0) + return ret; + + dev_dbg(dai->dev, "%s: cmd=%d\n", __func__, cmd); + switch (cmd) { + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_STOP: + w = snd_soc_dai_get_widget(dai, substream->stream); + + /* + * free DAI widget during stop/suspend to keep widget use_count's balanced. + */ + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); + if (ret < 0) + return ret; + + break; + default: + break; + } + return 0; +} + static int hda_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { From 9ebf97b7f3d9543e349247e22e57022ec5ae76e5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 15:28:14 -0500 Subject: [PATCH 11/40] soundwire: intel: remove unnecessary initialization cppcheck warning: drivers/soundwire/intel.c:1557:10: style: Variable 'ret' is assigned a value that is never used. [unreadVariable] int ret = 0; ^ Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 53505e1c0db09b..19f7d1e797eea7 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1554,7 +1554,7 @@ static int __maybe_unused intel_pm_prepare(struct device *dev) struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_bus *bus = &cdns->bus; u32 clock_stop_quirks; - int ret = 0; + int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", From f35a506c6604ae7116d2ecdc32dbd6a91931d33c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 27 Aug 2021 16:19:52 -0500 Subject: [PATCH 12/40] soundwire: stream: remove unused parameter in sdw_stream_add_slave The stream parameter is not used, remove before further simplifications. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 980f26d49b66f0..a30d0fb4871bde 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -968,14 +968,12 @@ static struct sdw_master_runtime * * @slave: Slave handle * @stream_config: Stream configuration - * @stream: Stream runtime handle * * This function is to be called with bus_lock held. */ static struct sdw_slave_runtime *sdw_alloc_slave_rt(struct sdw_slave *slave, - struct sdw_stream_config *stream_config, - struct sdw_stream_runtime *stream) + struct sdw_stream_config *stream_config) { struct sdw_slave_runtime *s_rt; @@ -1367,7 +1365,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, goto error; } - s_rt = sdw_alloc_slave_rt(slave, stream_config, stream); + s_rt = sdw_alloc_slave_rt(slave, stream_config); if (!s_rt) { dev_err(&slave->dev, "Slave runtime config failed for stream:%s\n", From b028f65e9702a1d9bb139c9b0ff2c34bc481008c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 27 Aug 2021 16:34:24 -0500 Subject: [PATCH 13/40] soundwire: stream: add slave runtime to list earlier sdw_config_stream() only verifies the compatibility between information provided by the Slave driver and the stream configuration. There is no problem if we add the slave runtime to the list earlier. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index a30d0fb4871bde..a75d3576bfcf4d 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1373,20 +1373,11 @@ int sdw_stream_add_slave(struct sdw_slave *slave, ret = -ENOMEM; goto stream_error; } + list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); ret = sdw_config_stream(&slave->dev, stream, stream_config, true); - if (ret) { - /* - * sdw_release_master_stream will release s_rt in slave_rt_list in - * stream_error case, but s_rt is only added to slave_rt_list - * when sdw_config_stream is successful, so free s_rt explicitly - * when sdw_config_stream is failed. - */ - kfree(s_rt); + if (ret) goto stream_error; - } - - list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports); if (ret) From 2a546d4eab2d6a4a3e7c84cfa7688d29c85aefb7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:08:25 -0500 Subject: [PATCH 14/40] soundwire: stream: simplify check on port range Pass the index directly to sdw_is_valid_port_range(), this will be useful for further simplifications. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index a75d3576bfcf4d..03024e7ab21260 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1177,12 +1177,11 @@ static int sdw_config_stream(struct device *dev, return 0; } -static int sdw_is_valid_port_range(struct device *dev, - struct sdw_port_runtime *p_rt) +static int sdw_is_valid_port_range(struct device *dev, int num) { - if (!SDW_VALID_PORT_RANGE(p_rt->num)) { + if (!SDW_VALID_PORT_RANGE(num)) { dev_err(dev, - "SoundWire: Invalid port number :%d\n", p_rt->num); + "SoundWire: Invalid port number :%d\n", num); return -EINVAL; } @@ -1249,7 +1248,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave, * TODO: Check valid port range as defined by DisCo/ * slave */ - ret = sdw_is_valid_port_range(&slave->dev, p_rt); + ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num); if (ret < 0) { kfree(p_rt); return ret; From 62fd00f4812ae6b54d6fa0827cd8b663e86dc913 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 8 Sep 2021 15:58:08 -0500 Subject: [PATCH 15/40] soundwire: stream: add alloc/config/free helpers for ports The existing code only has a config helper that allocates memory, start adding alloc/config/free for ports, as a first step in the simplification of the stream API. This change removes a kfree() on a configuration error, this should have not impact on existing platforms and error handling will be revisited in follow-up patches to make sure invalid configurations have not impact on memory allocation. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 83 +++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 03024e7ab21260..a8c58144f1d241 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -865,6 +865,39 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) return ret; } +static struct sdw_port_runtime *sdw_port_alloc(struct list_head *port_list) +{ + struct sdw_port_runtime *p_rt; + + p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL); + if (!p_rt) + return NULL; + + list_add_tail(&p_rt->port_node, port_list); + + return p_rt; +} + +static int sdw_port_config(struct sdw_port_runtime *p_rt, + struct sdw_port_config *port_config, + int port_index) +{ + p_rt->ch_mask = port_config[port_index].ch_mask; + p_rt->num = port_config[port_index].num; + + /* + * TODO: Check port capabilities for requested configuration + */ + + return 0; +} + +static void sdw_port_free(struct sdw_port_runtime *p_rt) +{ + list_del(&p_rt->port_node); + kfree(p_rt); +} + /** * sdw_release_stream() - Free the assigned stream runtime * @@ -995,8 +1028,7 @@ static void sdw_master_port_release(struct sdw_bus *bus, struct sdw_port_runtime *p_rt, *_p_rt; list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) { - list_del(&p_rt->port_node); - kfree(p_rt); + sdw_port_free(p_rt); } } @@ -1015,8 +1047,7 @@ static void sdw_slave_port_release(struct sdw_bus *bus, list_for_each_entry_safe(p_rt, _p_rt, &s_rt->port_list, port_node) { - list_del(&p_rt->port_node); - kfree(p_rt); + sdw_port_free(p_rt); } } } @@ -1188,43 +1219,24 @@ static int sdw_is_valid_port_range(struct device *dev, int num) return 0; } -static struct sdw_port_runtime -*sdw_port_alloc(struct device *dev, - struct sdw_port_config *port_config, - int port_index) -{ - struct sdw_port_runtime *p_rt; - - p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL); - if (!p_rt) - return NULL; - - p_rt->ch_mask = port_config[port_index].ch_mask; - p_rt->num = port_config[port_index].num; - - return p_rt; -} - static int sdw_master_port_config(struct sdw_bus *bus, struct sdw_master_runtime *m_rt, struct sdw_port_config *port_config, unsigned int num_ports) { struct sdw_port_runtime *p_rt; + int ret; int i; /* Iterate for number of ports to perform initialization */ for (i = 0; i < num_ports; i++) { - p_rt = sdw_port_alloc(bus->dev, port_config, i); + p_rt = sdw_port_alloc(&m_rt->port_list); if (!p_rt) return -ENOMEM; - /* - * TODO: Check port capabilities for requested - * configuration (audio mode support) - */ - - list_add_tail(&p_rt->port_node, &m_rt->port_list); + ret = sdw_port_config(p_rt, port_config, i); + if (ret < 0) + return ret; } return 0; @@ -1240,7 +1252,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave, /* Iterate for number of ports to perform initialization */ for (i = 0; i < num_config; i++) { - p_rt = sdw_port_alloc(&slave->dev, port_config, i); + p_rt = sdw_port_alloc(&s_rt->port_list); if (!p_rt) return -ENOMEM; @@ -1249,17 +1261,12 @@ static int sdw_slave_port_config(struct sdw_slave *slave, * slave */ ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num); - if (ret < 0) { - kfree(p_rt); + if (ret < 0) return ret; - } - - /* - * TODO: Check port capabilities for requested - * configuration (audio mode support) - */ - list_add_tail(&p_rt->port_node, &s_rt->port_list); + ret = sdw_port_config(p_rt, port_config, i); + if (ret < 0) + return ret; } return 0; From 191e7f18f1128b4dc561f15a3a87137937614c2f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:39:50 -0500 Subject: [PATCH 16/40] soundwire: stream: split port allocation and configuration loops Split loops before moving the allocation and configuration to separate functions. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index a8c58144f1d241..478f27358f085b 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1233,10 +1233,14 @@ static int sdw_master_port_config(struct sdw_bus *bus, p_rt = sdw_port_alloc(&m_rt->port_list); if (!p_rt) return -ENOMEM; + } + i = 0; + list_for_each_entry(p_rt, &m_rt->port_list, port_node) { ret = sdw_port_config(p_rt, port_config, i); if (ret < 0) return ret; + i++; } return 0; @@ -1255,7 +1259,10 @@ static int sdw_slave_port_config(struct sdw_slave *slave, p_rt = sdw_port_alloc(&s_rt->port_list); if (!p_rt) return -ENOMEM; + } + i = 0; + list_for_each_entry(p_rt, &s_rt->port_list, port_node) { /* * TODO: Check valid port range as defined by DisCo/ * slave @@ -1267,6 +1274,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave, ret = sdw_port_config(p_rt, port_config, i); if (ret < 0) return ret; + i++; } return 0; From 528ed46ea2917aa367303c7bae062a088451ff46 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:56:18 -0500 Subject: [PATCH 17/40] soundwire: stream: split alloc and config in two functions Continue the split with two functions for master and slave, and remove unused arguments. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 48 ++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 478f27358f085b..6a985e24d55437 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1219,13 +1219,10 @@ static int sdw_is_valid_port_range(struct device *dev, int num) return 0; } -static int sdw_master_port_config(struct sdw_bus *bus, - struct sdw_master_runtime *m_rt, - struct sdw_port_config *port_config, - unsigned int num_ports) +static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt, + unsigned int num_ports) { struct sdw_port_runtime *p_rt; - int ret; int i; /* Iterate for number of ports to perform initialization */ @@ -1235,6 +1232,16 @@ static int sdw_master_port_config(struct sdw_bus *bus, return -ENOMEM; } + return 0; +} + +static int sdw_master_port_config(struct sdw_master_runtime *m_rt, + struct sdw_port_config *port_config) +{ + struct sdw_port_runtime *p_rt; + int ret; + int i; + i = 0; list_for_each_entry(p_rt, &m_rt->port_list, port_node) { ret = sdw_port_config(p_rt, port_config, i); @@ -1246,13 +1253,12 @@ static int sdw_master_port_config(struct sdw_bus *bus, return 0; } -static int sdw_slave_port_config(struct sdw_slave *slave, - struct sdw_slave_runtime *s_rt, - struct sdw_port_config *port_config, - unsigned int num_config) +static int sdw_slave_port_alloc(struct sdw_slave *slave, + struct sdw_slave_runtime *s_rt, + unsigned int num_config) { struct sdw_port_runtime *p_rt; - int i, ret; + int i; /* Iterate for number of ports to perform initialization */ for (i = 0; i < num_config; i++) { @@ -1261,6 +1267,16 @@ static int sdw_slave_port_config(struct sdw_slave *slave, return -ENOMEM; } + return 0; +} + +static int sdw_slave_port_config(struct sdw_slave *slave, + struct sdw_slave_runtime *s_rt, + struct sdw_port_config *port_config) +{ + struct sdw_port_runtime *p_rt; + int i, ret; + i = 0; list_for_each_entry(p_rt, &s_rt->port_list, port_node) { /* @@ -1325,7 +1341,11 @@ int sdw_stream_add_master(struct sdw_bus *bus, if (ret) goto stream_error; - ret = sdw_master_port_config(bus, m_rt, port_config, num_ports); + ret = sdw_master_port_alloc(m_rt, num_ports); + if (ret) + goto stream_error; + + ret = sdw_master_port_config(m_rt, port_config); if (ret) goto stream_error; @@ -1393,7 +1413,11 @@ int sdw_stream_add_slave(struct sdw_slave *slave, if (ret) goto stream_error; - ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports); + ret = sdw_slave_port_alloc(slave, s_rt, num_ports); + if (ret) + goto stream_error; + + ret = sdw_slave_port_config(slave, s_rt, port_config); if (ret) goto stream_error; From 1f7c030e2a575a74f38c03ee5e97589cefedbe7d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:02:51 -0500 Subject: [PATCH 18/40] soundwire: stream: add 'slave' prefix for port range checks We can only check for Slave port ranges, the ports are not defined at the Master level. Also move the function to the 'slave port' block. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 6a985e24d55437..f563b002393904 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1208,17 +1208,6 @@ static int sdw_config_stream(struct device *dev, return 0; } -static int sdw_is_valid_port_range(struct device *dev, int num) -{ - if (!SDW_VALID_PORT_RANGE(num)) { - dev_err(dev, - "SoundWire: Invalid port number :%d\n", num); - return -EINVAL; - } - - return 0; -} - static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt, unsigned int num_ports) { @@ -1270,6 +1259,17 @@ static int sdw_slave_port_alloc(struct sdw_slave *slave, return 0; } +static int sdw_slave_port_is_valid_range(struct device *dev, int num) +{ + if (!SDW_VALID_PORT_RANGE(num)) { + dev_err(dev, + "SoundWire: Invalid port number :%d\n", num); + return -EINVAL; + } + + return 0; +} + static int sdw_slave_port_config(struct sdw_slave *slave, struct sdw_slave_runtime *s_rt, struct sdw_port_config *port_config) @@ -1283,7 +1283,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave, * TODO: Check valid port range as defined by DisCo/ * slave */ - ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num); + ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num); if (ret < 0) return ret; From a7addd1d2caeb7127126740eb4475a5a18d99b41 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:11:31 -0500 Subject: [PATCH 19/40] soundwire: stream: group sdw_port and sdw_master/slave_port functions re-group all the helpers in one location with a code move. For consistency the 'slave' helpers are placed before the 'master' helpers. Also remove unused arguments and rename the 'release' function to 'free' for consistency. No functional change in this patch. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 242 ++++++++++++++++++------------------- 1 file changed, 120 insertions(+), 122 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index f563b002393904..b9a76e79dff061 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -898,6 +898,123 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt) kfree(p_rt); } +static void sdw_slave_port_free(struct sdw_slave *slave, + struct sdw_stream_runtime *stream) +{ + struct sdw_port_runtime *p_rt, *_p_rt; + struct sdw_master_runtime *m_rt; + struct sdw_slave_runtime *s_rt; + + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { + if (s_rt->slave != slave) + continue; + + list_for_each_entry_safe(p_rt, _p_rt, + &s_rt->port_list, port_node) { + sdw_port_free(p_rt); + } + } + } +} + +static int sdw_slave_port_alloc(struct sdw_slave *slave, + struct sdw_slave_runtime *s_rt, + unsigned int num_config) +{ + struct sdw_port_runtime *p_rt; + int i; + + /* Iterate for number of ports to perform initialization */ + for (i = 0; i < num_config; i++) { + p_rt = sdw_port_alloc(&s_rt->port_list); + if (!p_rt) + return -ENOMEM; + } + + return 0; +} + +static int sdw_slave_port_is_valid_range(struct device *dev, int num) +{ + if (!SDW_VALID_PORT_RANGE(num)) { + dev_err(dev, + "SoundWire: Invalid port number :%d\n", num); + return -EINVAL; + } + + return 0; +} + +static int sdw_slave_port_config(struct sdw_slave *slave, + struct sdw_slave_runtime *s_rt, + struct sdw_port_config *port_config) +{ + struct sdw_port_runtime *p_rt; + int i, ret; + + i = 0; + list_for_each_entry(p_rt, &s_rt->port_list, port_node) { + /* + * TODO: Check valid port range as defined by DisCo/ + * slave + */ + ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num); + if (ret < 0) + return ret; + + ret = sdw_port_config(p_rt, port_config, i); + if (ret < 0) + return ret; + i++; + } + + return 0; +} + +static void sdw_master_port_free(struct sdw_master_runtime *m_rt) +{ + struct sdw_port_runtime *p_rt, *_p_rt; + + list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) { + sdw_port_free(p_rt); + } +} + +static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt, + unsigned int num_ports) +{ + struct sdw_port_runtime *p_rt; + int i; + + /* Iterate for number of ports to perform initialization */ + for (i = 0; i < num_ports; i++) { + p_rt = sdw_port_alloc(&m_rt->port_list); + if (!p_rt) + return -ENOMEM; + } + + return 0; +} + +static int sdw_master_port_config(struct sdw_master_runtime *m_rt, + struct sdw_port_config *port_config) +{ + struct sdw_port_runtime *p_rt; + int ret; + int i; + + i = 0; + list_for_each_entry(p_rt, &m_rt->port_list, port_node) { + ret = sdw_port_config(p_rt, port_config, i); + if (ret < 0) + return ret; + i++; + } + + return 0; +} + /** * sdw_release_stream() - Free the assigned stream runtime * @@ -1022,37 +1139,6 @@ static struct sdw_slave_runtime return s_rt; } -static void sdw_master_port_release(struct sdw_bus *bus, - struct sdw_master_runtime *m_rt) -{ - struct sdw_port_runtime *p_rt, *_p_rt; - - list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) { - sdw_port_free(p_rt); - } -} - -static void sdw_slave_port_release(struct sdw_bus *bus, - struct sdw_slave *slave, - struct sdw_stream_runtime *stream) -{ - struct sdw_port_runtime *p_rt, *_p_rt; - struct sdw_master_runtime *m_rt; - struct sdw_slave_runtime *s_rt; - - list_for_each_entry(m_rt, &stream->master_list, stream_node) { - list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { - if (s_rt->slave != slave) - continue; - - list_for_each_entry_safe(p_rt, _p_rt, - &s_rt->port_list, port_node) { - sdw_port_free(p_rt); - } - } - } -} - /** * sdw_release_slave_stream() - Free Slave(s) runtime handle * @@ -1097,7 +1183,7 @@ static void sdw_release_master_stream(struct sdw_master_runtime *m_rt, struct sdw_slave_runtime *s_rt, *_s_rt; list_for_each_entry_safe(s_rt, _s_rt, &m_rt->slave_rt_list, m_rt_node) { - sdw_slave_port_release(s_rt->slave->bus, s_rt->slave, stream); + sdw_slave_port_free(s_rt->slave, stream); sdw_release_slave_stream(s_rt->slave, stream); } @@ -1126,7 +1212,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus, if (m_rt->bus != bus) continue; - sdw_master_port_release(bus, m_rt); + sdw_master_port_free(m_rt); sdw_release_master_stream(m_rt, stream); stream->m_rt_count--; } @@ -1153,7 +1239,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave, { mutex_lock(&slave->bus->bus_lock); - sdw_slave_port_release(slave->bus, slave, stream); + sdw_slave_port_free(slave, stream); sdw_release_slave_stream(slave, stream); mutex_unlock(&slave->bus->bus_lock); @@ -1208,94 +1294,6 @@ static int sdw_config_stream(struct device *dev, return 0; } -static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt, - unsigned int num_ports) -{ - struct sdw_port_runtime *p_rt; - int i; - - /* Iterate for number of ports to perform initialization */ - for (i = 0; i < num_ports; i++) { - p_rt = sdw_port_alloc(&m_rt->port_list); - if (!p_rt) - return -ENOMEM; - } - - return 0; -} - -static int sdw_master_port_config(struct sdw_master_runtime *m_rt, - struct sdw_port_config *port_config) -{ - struct sdw_port_runtime *p_rt; - int ret; - int i; - - i = 0; - list_for_each_entry(p_rt, &m_rt->port_list, port_node) { - ret = sdw_port_config(p_rt, port_config, i); - if (ret < 0) - return ret; - i++; - } - - return 0; -} - -static int sdw_slave_port_alloc(struct sdw_slave *slave, - struct sdw_slave_runtime *s_rt, - unsigned int num_config) -{ - struct sdw_port_runtime *p_rt; - int i; - - /* Iterate for number of ports to perform initialization */ - for (i = 0; i < num_config; i++) { - p_rt = sdw_port_alloc(&s_rt->port_list); - if (!p_rt) - return -ENOMEM; - } - - return 0; -} - -static int sdw_slave_port_is_valid_range(struct device *dev, int num) -{ - if (!SDW_VALID_PORT_RANGE(num)) { - dev_err(dev, - "SoundWire: Invalid port number :%d\n", num); - return -EINVAL; - } - - return 0; -} - -static int sdw_slave_port_config(struct sdw_slave *slave, - struct sdw_slave_runtime *s_rt, - struct sdw_port_config *port_config) -{ - struct sdw_port_runtime *p_rt; - int i, ret; - - i = 0; - list_for_each_entry(p_rt, &s_rt->port_list, port_node) { - /* - * TODO: Check valid port range as defined by DisCo/ - * slave - */ - ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num); - if (ret < 0) - return ret; - - ret = sdw_port_config(p_rt, port_config, i); - if (ret < 0) - return ret; - i++; - } - - return 0; -} - /** * sdw_stream_add_master() - Allocate and add master runtime to a stream * From e674f47a4954d9afe8fb977855c68183b09dc470 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:30:19 -0500 Subject: [PATCH 20/40] soundwire: stream: simplify sdw_alloc_master_rt() Only do the allocation in that function, and move check for allocation in the caller. This will it easier to split allocation and configuration. No functionality change in this patch. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index b9a76e79dff061..379b68249a5e81 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1085,14 +1085,6 @@ static struct sdw_master_runtime { struct sdw_master_runtime *m_rt; - /* - * check if Master is already allocated (as a result of Slave adding - * it first), if so skip allocation and go to configure - */ - m_rt = sdw_find_master_rt(bus, stream); - if (m_rt) - goto stream_config; - m_rt = kzalloc(sizeof(*m_rt), GFP_KERNEL); if (!m_rt) return NULL; @@ -1104,7 +1096,6 @@ static struct sdw_master_runtime list_add_tail(&m_rt->bus_node, &bus->m_rt_list); -stream_config: m_rt->ch_count = stream_config->ch_count; m_rt->bus = bus; m_rt->stream = stream; @@ -1326,6 +1317,14 @@ int sdw_stream_add_master(struct sdw_bus *bus, goto unlock; } + /* + * check if Master is already allocated (e.g. as a result of Slave adding + * it first), if so skip allocation and go to configuration + */ + m_rt = sdw_find_master_rt(bus, stream); + if (m_rt) + goto skip_alloc_master_rt; + m_rt = sdw_alloc_master_rt(bus, stream_config, stream); if (!m_rt) { dev_err(bus->dev, @@ -1335,6 +1334,7 @@ int sdw_stream_add_master(struct sdw_bus *bus, goto unlock; } +skip_alloc_master_rt: ret = sdw_config_stream(bus->dev, stream, stream_config, false); if (ret) goto stream_error; @@ -1384,6 +1384,14 @@ int sdw_stream_add_slave(struct sdw_slave *slave, mutex_lock(&slave->bus->bus_lock); + /* + * check if Master is already allocated, if so skip allocation + * and go to configuration + */ + m_rt = sdw_find_master_rt(slave->bus, stream); + if (m_rt) + goto skip_alloc_master_rt; + /* * If this API is invoked by Slave first then m_rt is not valid. * So, allocate m_rt and add Slave to it. @@ -1397,6 +1405,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, goto error; } +skip_alloc_master_rt: s_rt = sdw_alloc_slave_rt(slave, stream_config); if (!s_rt) { dev_err(&slave->dev, From d0cb229e8fc62dfb1f75b29b71cede43a407c96b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:47:52 -0500 Subject: [PATCH 21/40] soundwire: stream: split sdw_alloc_master_rt() in alloc and config Split the two parts so that we can do multiple configurations during ALSA/ASoC hw_params stage. Also follow existing convention sdw__ used at lower level. No functionality change here. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 43 +++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 379b68249a5e81..1b55c543c51dc3 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1055,7 +1055,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) EXPORT_SYMBOL(sdw_alloc_stream); static struct sdw_master_runtime -*sdw_find_master_rt(struct sdw_bus *bus, +*sdw_master_rt_find(struct sdw_bus *bus, struct sdw_stream_runtime *stream) { struct sdw_master_runtime *m_rt; @@ -1070,17 +1070,15 @@ static struct sdw_master_runtime } /** - * sdw_alloc_master_rt() - Allocates and initialize Master runtime handle + * sdw_master_rt_alloc() - Allocates a Master runtime handle * * @bus: SDW bus instance - * @stream_config: Stream configuration * @stream: Stream runtime handle. * * This function is to be called with bus_lock held. */ static struct sdw_master_runtime -*sdw_alloc_master_rt(struct sdw_bus *bus, - struct sdw_stream_config *stream_config, +*sdw_master_rt_alloc(struct sdw_bus *bus, struct sdw_stream_runtime *stream) { struct sdw_master_runtime *m_rt; @@ -1096,14 +1094,30 @@ static struct sdw_master_runtime list_add_tail(&m_rt->bus_node, &bus->m_rt_list); - m_rt->ch_count = stream_config->ch_count; m_rt->bus = bus; m_rt->stream = stream; - m_rt->direction = stream_config->direction; return m_rt; } +/** + * sdw_master_rt_config() - Configure Master runtime handle + * + * @m_rt: Master runtime handle + * @stream_config: Stream configuration + * + * This function is to be called with bus_lock held. + */ + +static int sdw_master_rt_config(struct sdw_master_runtime *m_rt, + struct sdw_stream_config *stream_config) +{ + m_rt->ch_count = stream_config->ch_count; + m_rt->direction = stream_config->direction; + + return 0; +} + /** * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle. * @@ -1321,19 +1335,23 @@ int sdw_stream_add_master(struct sdw_bus *bus, * check if Master is already allocated (e.g. as a result of Slave adding * it first), if so skip allocation and go to configuration */ - m_rt = sdw_find_master_rt(bus, stream); + m_rt = sdw_master_rt_find(bus, stream); if (m_rt) goto skip_alloc_master_rt; - m_rt = sdw_alloc_master_rt(bus, stream_config, stream); + m_rt = sdw_master_rt_alloc(bus, stream); if (!m_rt) { dev_err(bus->dev, - "Master runtime config failed for stream:%s\n", + "Master runtime alloc failed for stream:%s\n", stream->name); ret = -ENOMEM; goto unlock; } + ret = sdw_master_rt_config(m_rt, stream_config); + if (ret < 0) + goto unlock; + skip_alloc_master_rt: ret = sdw_config_stream(bus->dev, stream, stream_config, false); if (ret) @@ -1388,7 +1406,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * check if Master is already allocated, if so skip allocation * and go to configuration */ - m_rt = sdw_find_master_rt(slave->bus, stream); + m_rt = sdw_master_rt_find(slave->bus, stream); if (m_rt) goto skip_alloc_master_rt; @@ -1396,7 +1414,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * If this API is invoked by Slave first then m_rt is not valid. * So, allocate m_rt and add Slave to it. */ - m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream); + m_rt = sdw_master_rt_alloc(slave->bus, stream); if (!m_rt) { dev_err(&slave->dev, "alloc master runtime failed for stream:%s\n", @@ -1404,6 +1422,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, ret = -ENOMEM; goto error; } + ret = sdw_master_rt_config(m_rt, stream_config); skip_alloc_master_rt: s_rt = sdw_alloc_slave_rt(slave, stream_config); From a375efc59ddc677330df551ff7a05b7673a29a08 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 13:48:33 -0500 Subject: [PATCH 22/40] soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers Code move before splitting the function in two. No functionality change. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 52 +++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 1b55c543c51dc3..d01e538e58c678 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1054,6 +1054,32 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) } EXPORT_SYMBOL(sdw_alloc_stream); +/** + * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle. + * + * @slave: Slave handle + * @stream_config: Stream configuration + * + * This function is to be called with bus_lock held. + */ +static struct sdw_slave_runtime +*sdw_alloc_slave_rt(struct sdw_slave *slave, + struct sdw_stream_config *stream_config) +{ + struct sdw_slave_runtime *s_rt; + + s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL); + if (!s_rt) + return NULL; + + INIT_LIST_HEAD(&s_rt->port_list); + s_rt->ch_count = stream_config->ch_count; + s_rt->direction = stream_config->direction; + s_rt->slave = slave; + + return s_rt; +} + static struct sdw_master_runtime *sdw_master_rt_find(struct sdw_bus *bus, struct sdw_stream_runtime *stream) @@ -1118,32 +1144,6 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt, return 0; } -/** - * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle. - * - * @slave: Slave handle - * @stream_config: Stream configuration - * - * This function is to be called with bus_lock held. - */ -static struct sdw_slave_runtime -*sdw_alloc_slave_rt(struct sdw_slave *slave, - struct sdw_stream_config *stream_config) -{ - struct sdw_slave_runtime *s_rt; - - s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL); - if (!s_rt) - return NULL; - - INIT_LIST_HEAD(&s_rt->port_list); - s_rt->ch_count = stream_config->ch_count; - s_rt->direction = stream_config->direction; - s_rt->slave = slave; - - return s_rt; -} - /** * sdw_release_slave_stream() - Free Slave(s) runtime handle * From cc1a16e6facab3f3ed9f7d0744988cef272c2182 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 12:00:01 -0500 Subject: [PATCH 23/40] soundwire: stream: split sdw_alloc_slave_rt() in alloc and config Split the two parts so that we can do multiple configurations during ALSA/ASoC hw_params stage. Also follow existing convention sdw__ used at lower level. No functionality change here. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index d01e538e58c678..fc3d7139cf9f43 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1055,16 +1055,14 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) EXPORT_SYMBOL(sdw_alloc_stream); /** - * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle. + * sdw_slave_rt_alloc() - Allocate a Slave runtime handle. * * @slave: Slave handle - * @stream_config: Stream configuration * * This function is to be called with bus_lock held. */ static struct sdw_slave_runtime -*sdw_alloc_slave_rt(struct sdw_slave *slave, - struct sdw_stream_config *stream_config) +*sdw_slave_rt_alloc(struct sdw_slave *slave) { struct sdw_slave_runtime *s_rt; @@ -1073,13 +1071,28 @@ static struct sdw_slave_runtime return NULL; INIT_LIST_HEAD(&s_rt->port_list); - s_rt->ch_count = stream_config->ch_count; - s_rt->direction = stream_config->direction; s_rt->slave = slave; return s_rt; } +/** + * sdw_slave_rt_config() - Configure a Slave runtime handle. + * + * @s_rt: Slave runtime handle + * @stream_config: Stream configuration + * + * This function is to be called with bus_lock held. + */ +static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt, + struct sdw_stream_config *stream_config) +{ + s_rt->ch_count = stream_config->ch_count; + s_rt->direction = stream_config->direction; + + return 0; +} + static struct sdw_master_runtime *sdw_master_rt_find(struct sdw_bus *bus, struct sdw_stream_runtime *stream) @@ -1425,16 +1438,20 @@ int sdw_stream_add_slave(struct sdw_slave *slave, ret = sdw_master_rt_config(m_rt, stream_config); skip_alloc_master_rt: - s_rt = sdw_alloc_slave_rt(slave, stream_config); + s_rt = sdw_slave_rt_alloc(slave); if (!s_rt) { dev_err(&slave->dev, - "Slave runtime config failed for stream:%s\n", + "Slave runtime alloc failed for stream:%s\n", stream->name); ret = -ENOMEM; goto stream_error; } list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); + ret = sdw_slave_rt_config(s_rt, stream_config); + if (ret) + goto stream_error; + ret = sdw_config_stream(&slave->dev, stream, stream_config, true); if (ret) goto stream_error; From bc54f169077be2a17395e73a5a8925173062a858 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 13:32:37 -0500 Subject: [PATCH 24/40] soundwire: stream: group sdw_stream_ functions Group all exported functions prior to split of add in alloc/config stages necessary for support of multiple calls to hw_params() by ALSA/ASoC core. Pure code move, no functionality change. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 679 +++++++++++++++++++------------------ 1 file changed, 340 insertions(+), 339 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index fc3d7139cf9f43..0bd08015a587b4 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1015,45 +1015,6 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt, return 0; } -/** - * sdw_release_stream() - Free the assigned stream runtime - * - * @stream: SoundWire stream runtime - * - * sdw_release_stream should be called only once per stream - */ -void sdw_release_stream(struct sdw_stream_runtime *stream) -{ - kfree(stream); -} -EXPORT_SYMBOL(sdw_release_stream); - -/** - * sdw_alloc_stream() - Allocate and return stream runtime - * - * @stream_name: SoundWire stream name - * - * Allocates a SoundWire stream runtime instance. - * sdw_alloc_stream should be called only once per stream. Typically - * invoked from ALSA/ASoC machine/platform driver. - */ -struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) -{ - struct sdw_stream_runtime *stream; - - stream = kzalloc(sizeof(*stream), GFP_KERNEL); - if (!stream) - return NULL; - - stream->name = stream_name; - INIT_LIST_HEAD(&stream->master_list); - stream->state = SDW_STREAM_ALLOCATED; - stream->m_rt_count = 0; - - return stream; -} -EXPORT_SYMBOL(sdw_alloc_stream); - /** * sdw_slave_rt_alloc() - Allocate a Slave runtime handle. * @@ -1210,62 +1171,6 @@ static void sdw_release_master_stream(struct sdw_master_runtime *m_rt, kfree(m_rt); } -/** - * sdw_stream_remove_master() - Remove master from sdw_stream - * - * @bus: SDW Bus instance - * @stream: SoundWire stream - * - * This removes and frees port_rt and master_rt from a stream - */ -int sdw_stream_remove_master(struct sdw_bus *bus, - struct sdw_stream_runtime *stream) -{ - struct sdw_master_runtime *m_rt, *_m_rt; - - mutex_lock(&bus->bus_lock); - - list_for_each_entry_safe(m_rt, _m_rt, - &stream->master_list, stream_node) { - if (m_rt->bus != bus) - continue; - - sdw_master_port_free(m_rt); - sdw_release_master_stream(m_rt, stream); - stream->m_rt_count--; - } - - if (list_empty(&stream->master_list)) - stream->state = SDW_STREAM_RELEASED; - - mutex_unlock(&bus->bus_lock); - - return 0; -} -EXPORT_SYMBOL(sdw_stream_remove_master); - -/** - * sdw_stream_remove_slave() - Remove slave from sdw_stream - * - * @slave: SDW Slave instance - * @stream: SoundWire stream - * - * This removes and frees port_rt and slave_rt from a stream - */ -int sdw_stream_remove_slave(struct sdw_slave *slave, - struct sdw_stream_runtime *stream) -{ - mutex_lock(&slave->bus->bus_lock); - - sdw_slave_port_free(slave, stream); - sdw_release_slave_stream(slave, stream); - - mutex_unlock(&slave->bus->bus_lock); - - return 0; -} -EXPORT_SYMBOL(sdw_stream_remove_slave); - /** * sdw_config_stream() - Configure the allocated stream * @@ -1313,273 +1218,100 @@ static int sdw_config_stream(struct device *dev, } /** - * sdw_stream_add_master() - Allocate and add master runtime to a stream + * sdw_get_slave_dpn_prop() - Get Slave port capabilities * - * @bus: SDW Bus instance - * @stream_config: Stream configuration for audio stream - * @port_config: Port configuration for audio stream - * @num_ports: Number of ports - * @stream: SoundWire stream + * @slave: Slave handle + * @direction: Data direction. + * @port_num: Port number */ -int sdw_stream_add_master(struct sdw_bus *bus, - struct sdw_stream_config *stream_config, - struct sdw_port_config *port_config, - unsigned int num_ports, - struct sdw_stream_runtime *stream) +struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave, + enum sdw_data_direction direction, + unsigned int port_num) { - struct sdw_master_runtime *m_rt; - int ret; - - mutex_lock(&bus->bus_lock); + struct sdw_dpn_prop *dpn_prop; + u8 num_ports; + int i; - /* - * For multi link streams, add the second master only if - * the bus supports it. - * Check if bus->multi_link is set - */ - if (!bus->multi_link && stream->m_rt_count > 0) { - dev_err(bus->dev, - "Multilink not supported, link %d\n", bus->link_id); - ret = -EINVAL; - goto unlock; + if (direction == SDW_DATA_DIR_TX) { + num_ports = hweight32(slave->prop.source_ports); + dpn_prop = slave->prop.src_dpn_prop; + } else { + num_ports = hweight32(slave->prop.sink_ports); + dpn_prop = slave->prop.sink_dpn_prop; } - /* - * check if Master is already allocated (e.g. as a result of Slave adding - * it first), if so skip allocation and go to configuration - */ - m_rt = sdw_master_rt_find(bus, stream); - if (m_rt) - goto skip_alloc_master_rt; - - m_rt = sdw_master_rt_alloc(bus, stream); - if (!m_rt) { - dev_err(bus->dev, - "Master runtime alloc failed for stream:%s\n", - stream->name); - ret = -ENOMEM; - goto unlock; + for (i = 0; i < num_ports; i++) { + if (dpn_prop[i].num == port_num) + return &dpn_prop[i]; } - ret = sdw_master_rt_config(m_rt, stream_config); - if (ret < 0) - goto unlock; - -skip_alloc_master_rt: - ret = sdw_config_stream(bus->dev, stream, stream_config, false); - if (ret) - goto stream_error; - - ret = sdw_master_port_alloc(m_rt, num_ports); - if (ret) - goto stream_error; - - ret = sdw_master_port_config(m_rt, port_config); - if (ret) - goto stream_error; + return NULL; +} - stream->m_rt_count++; +/** + * sdw_acquire_bus_lock: Acquire bus lock for all Master runtime(s) + * + * @stream: SoundWire stream + * + * Acquire bus_lock for each of the master runtime(m_rt) part of this + * stream to reconfigure the bus. + * NOTE: This function is called from SoundWire stream ops and is + * expected that a global lock is held before acquiring bus_lock. + */ +static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream) +{ + struct sdw_master_runtime *m_rt; + struct sdw_bus *bus; - goto unlock; + /* Iterate for all Master(s) in Master list */ + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + bus = m_rt->bus; -stream_error: - sdw_release_master_stream(m_rt, stream); -unlock: - mutex_unlock(&bus->bus_lock); - return ret; + mutex_lock(&bus->bus_lock); + } } -EXPORT_SYMBOL(sdw_stream_add_master); /** - * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream + * sdw_release_bus_lock: Release bus lock for all Master runtime(s) * - * @slave: SDW Slave instance - * @stream_config: Stream configuration for audio stream * @stream: SoundWire stream - * @port_config: Port configuration for audio stream - * @num_ports: Number of ports - * - * It is expected that Slave is added before adding Master - * to the Stream. * + * Release the previously held bus_lock after reconfiguring the bus. + * NOTE: This function is called from SoundWire stream ops and is + * expected that a global lock is held before releasing bus_lock. */ -int sdw_stream_add_slave(struct sdw_slave *slave, - struct sdw_stream_config *stream_config, - struct sdw_port_config *port_config, - unsigned int num_ports, - struct sdw_stream_runtime *stream) +static void sdw_release_bus_lock(struct sdw_stream_runtime *stream) +{ + struct sdw_master_runtime *m_rt; + struct sdw_bus *bus; + + /* Iterate for all Master(s) in Master list */ + list_for_each_entry_reverse(m_rt, &stream->master_list, stream_node) { + bus = m_rt->bus; + mutex_unlock(&bus->bus_lock); + } +} + +static int _sdw_prepare_stream(struct sdw_stream_runtime *stream, + bool update_params) { - struct sdw_slave_runtime *s_rt; struct sdw_master_runtime *m_rt; + struct sdw_bus *bus = NULL; + struct sdw_master_prop *prop; + struct sdw_bus_params params; int ret; - mutex_lock(&slave->bus->bus_lock); + /* Prepare Master(s) and Slave(s) port(s) associated with stream */ + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + bus = m_rt->bus; + prop = &bus->prop; + memcpy(¶ms, &bus->params, sizeof(params)); - /* - * check if Master is already allocated, if so skip allocation - * and go to configuration - */ - m_rt = sdw_master_rt_find(slave->bus, stream); - if (m_rt) - goto skip_alloc_master_rt; - - /* - * If this API is invoked by Slave first then m_rt is not valid. - * So, allocate m_rt and add Slave to it. - */ - m_rt = sdw_master_rt_alloc(slave->bus, stream); - if (!m_rt) { - dev_err(&slave->dev, - "alloc master runtime failed for stream:%s\n", - stream->name); - ret = -ENOMEM; - goto error; - } - ret = sdw_master_rt_config(m_rt, stream_config); - -skip_alloc_master_rt: - s_rt = sdw_slave_rt_alloc(slave); - if (!s_rt) { - dev_err(&slave->dev, - "Slave runtime alloc failed for stream:%s\n", - stream->name); - ret = -ENOMEM; - goto stream_error; - } - list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); - - ret = sdw_slave_rt_config(s_rt, stream_config); - if (ret) - goto stream_error; - - ret = sdw_config_stream(&slave->dev, stream, stream_config, true); - if (ret) - goto stream_error; - - ret = sdw_slave_port_alloc(slave, s_rt, num_ports); - if (ret) - goto stream_error; - - ret = sdw_slave_port_config(slave, s_rt, port_config); - if (ret) - goto stream_error; - - /* - * Change stream state to CONFIGURED on first Slave add. - * Bus is not aware of number of Slave(s) in a stream at this - * point so cannot depend on all Slave(s) to be added in order to - * change stream state to CONFIGURED. - */ - stream->state = SDW_STREAM_CONFIGURED; - goto error; - -stream_error: - /* - * we hit error so cleanup the stream, release all Slave(s) and - * Master runtime - */ - sdw_release_master_stream(m_rt, stream); -error: - mutex_unlock(&slave->bus->bus_lock); - return ret; -} -EXPORT_SYMBOL(sdw_stream_add_slave); - -/** - * sdw_get_slave_dpn_prop() - Get Slave port capabilities - * - * @slave: Slave handle - * @direction: Data direction. - * @port_num: Port number - */ -struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave, - enum sdw_data_direction direction, - unsigned int port_num) -{ - struct sdw_dpn_prop *dpn_prop; - u8 num_ports; - int i; - - if (direction == SDW_DATA_DIR_TX) { - num_ports = hweight32(slave->prop.source_ports); - dpn_prop = slave->prop.src_dpn_prop; - } else { - num_ports = hweight32(slave->prop.sink_ports); - dpn_prop = slave->prop.sink_dpn_prop; - } - - for (i = 0; i < num_ports; i++) { - if (dpn_prop[i].num == port_num) - return &dpn_prop[i]; - } - - return NULL; -} - -/** - * sdw_acquire_bus_lock: Acquire bus lock for all Master runtime(s) - * - * @stream: SoundWire stream - * - * Acquire bus_lock for each of the master runtime(m_rt) part of this - * stream to reconfigure the bus. - * NOTE: This function is called from SoundWire stream ops and is - * expected that a global lock is held before acquiring bus_lock. - */ -static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream) -{ - struct sdw_master_runtime *m_rt; - struct sdw_bus *bus; - - /* Iterate for all Master(s) in Master list */ - list_for_each_entry(m_rt, &stream->master_list, stream_node) { - bus = m_rt->bus; - - mutex_lock(&bus->bus_lock); - } -} - -/** - * sdw_release_bus_lock: Release bus lock for all Master runtime(s) - * - * @stream: SoundWire stream - * - * Release the previously held bus_lock after reconfiguring the bus. - * NOTE: This function is called from SoundWire stream ops and is - * expected that a global lock is held before releasing bus_lock. - */ -static void sdw_release_bus_lock(struct sdw_stream_runtime *stream) -{ - struct sdw_master_runtime *m_rt; - struct sdw_bus *bus; - - /* Iterate for all Master(s) in Master list */ - list_for_each_entry_reverse(m_rt, &stream->master_list, stream_node) { - bus = m_rt->bus; - mutex_unlock(&bus->bus_lock); - } -} - -static int _sdw_prepare_stream(struct sdw_stream_runtime *stream, - bool update_params) -{ - struct sdw_master_runtime *m_rt; - struct sdw_bus *bus = NULL; - struct sdw_master_prop *prop; - struct sdw_bus_params params; - int ret; - - /* Prepare Master(s) and Slave(s) port(s) associated with stream */ - list_for_each_entry(m_rt, &stream->master_list, stream_node) { - bus = m_rt->bus; - prop = &bus->prop; - memcpy(¶ms, &bus->params, sizeof(params)); - - /* TODO: Support Asynchronous mode */ - if ((prop->max_clk_freq % stream->params.rate) != 0) { - dev_err(bus->dev, "Async mode not supported\n"); - return -EINVAL; - } + /* TODO: Support Asynchronous mode */ + if ((prop->max_clk_freq % stream->params.rate) != 0) { + dev_err(bus->dev, "Async mode not supported\n"); + return -EINVAL; + } if (!update_params) goto program_params; @@ -1943,6 +1675,32 @@ static int set_stream(struct snd_pcm_substream *substream, return ret; } +/** + * sdw_alloc_stream() - Allocate and return stream runtime + * + * @stream_name: SoundWire stream name + * + * Allocates a SoundWire stream runtime instance. + * sdw_alloc_stream should be called only once per stream. Typically + * invoked from ALSA/ASoC machine/platform driver. + */ +struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) +{ + struct sdw_stream_runtime *stream; + + stream = kzalloc(sizeof(*stream), GFP_KERNEL); + if (!stream) + return NULL; + + stream->name = stream_name; + INIT_LIST_HEAD(&stream->master_list); + stream->state = SDW_STREAM_ALLOCATED; + stream->m_rt_count = 0; + + return stream; +} +EXPORT_SYMBOL(sdw_alloc_stream); + /** * sdw_startup_stream() - Startup SoundWire stream * @@ -2019,3 +1777,246 @@ void sdw_shutdown_stream(void *sdw_substream) set_stream(substream, NULL); } EXPORT_SYMBOL(sdw_shutdown_stream); + +/** + * sdw_release_stream() - Free the assigned stream runtime + * + * @stream: SoundWire stream runtime + * + * sdw_release_stream should be called only once per stream + */ +void sdw_release_stream(struct sdw_stream_runtime *stream) +{ + kfree(stream); +} +EXPORT_SYMBOL(sdw_release_stream); + +/** + * sdw_stream_add_master() - Allocate and add master runtime to a stream + * + * @bus: SDW Bus instance + * @stream_config: Stream configuration for audio stream + * @port_config: Port configuration for audio stream + * @num_ports: Number of ports + * @stream: SoundWire stream + */ +int sdw_stream_add_master(struct sdw_bus *bus, + struct sdw_stream_config *stream_config, + struct sdw_port_config *port_config, + unsigned int num_ports, + struct sdw_stream_runtime *stream) +{ + struct sdw_master_runtime *m_rt; + int ret; + + mutex_lock(&bus->bus_lock); + + /* + * For multi link streams, add the second master only if + * the bus supports it. + * Check if bus->multi_link is set + */ + if (!bus->multi_link && stream->m_rt_count > 0) { + dev_err(bus->dev, + "Multilink not supported, link %d\n", bus->link_id); + ret = -EINVAL; + goto unlock; + } + + /* + * check if Master is already allocated (e.g. as a result of Slave adding + * it first), if so skip allocation and go to configuration + */ + m_rt = sdw_master_rt_find(bus, stream); + if (m_rt) + goto skip_alloc_master_rt; + + m_rt = sdw_master_rt_alloc(bus, stream); + if (!m_rt) { + dev_err(bus->dev, + "Master runtime alloc failed for stream:%s\n", + stream->name); + ret = -ENOMEM; + goto unlock; + } + + ret = sdw_master_rt_config(m_rt, stream_config); + if (ret < 0) + goto unlock; + +skip_alloc_master_rt: + ret = sdw_config_stream(bus->dev, stream, stream_config, false); + if (ret) + goto stream_error; + + ret = sdw_master_port_alloc(m_rt, num_ports); + if (ret) + goto stream_error; + + ret = sdw_master_port_config(m_rt, port_config); + if (ret) + goto stream_error; + + stream->m_rt_count++; + + goto unlock; + +stream_error: + sdw_release_master_stream(m_rt, stream); +unlock: + mutex_unlock(&bus->bus_lock); + return ret; +} +EXPORT_SYMBOL(sdw_stream_add_master); + +/** + * sdw_stream_remove_master() - Remove master from sdw_stream + * + * @bus: SDW Bus instance + * @stream: SoundWire stream + * + * This removes and frees port_rt and master_rt from a stream + */ +int sdw_stream_remove_master(struct sdw_bus *bus, + struct sdw_stream_runtime *stream) +{ + struct sdw_master_runtime *m_rt, *_m_rt; + + mutex_lock(&bus->bus_lock); + + list_for_each_entry_safe(m_rt, _m_rt, + &stream->master_list, stream_node) { + if (m_rt->bus != bus) + continue; + + sdw_master_port_free(m_rt); + sdw_release_master_stream(m_rt, stream); + stream->m_rt_count--; + } + + if (list_empty(&stream->master_list)) + stream->state = SDW_STREAM_RELEASED; + + mutex_unlock(&bus->bus_lock); + + return 0; +} +EXPORT_SYMBOL(sdw_stream_remove_master); + +/** + * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream + * + * @slave: SDW Slave instance + * @stream_config: Stream configuration for audio stream + * @stream: SoundWire stream + * @port_config: Port configuration for audio stream + * @num_ports: Number of ports + * + * It is expected that Slave is added before adding Master + * to the Stream. + * + */ +int sdw_stream_add_slave(struct sdw_slave *slave, + struct sdw_stream_config *stream_config, + struct sdw_port_config *port_config, + unsigned int num_ports, + struct sdw_stream_runtime *stream) +{ + struct sdw_slave_runtime *s_rt; + struct sdw_master_runtime *m_rt; + int ret; + + mutex_lock(&slave->bus->bus_lock); + + /* + * check if Master is already allocated, if so skip allocation + * and go to configuration + */ + m_rt = sdw_master_rt_find(slave->bus, stream); + if (m_rt) + goto skip_alloc_master_rt; + + /* + * If this API is invoked by Slave first then m_rt is not valid. + * So, allocate m_rt and add Slave to it. + */ + m_rt = sdw_master_rt_alloc(slave->bus, stream); + if (!m_rt) { + dev_err(&slave->dev, + "alloc master runtime failed for stream:%s\n", + stream->name); + ret = -ENOMEM; + goto error; + } + ret = sdw_master_rt_config(m_rt, stream_config); + +skip_alloc_master_rt: + s_rt = sdw_slave_rt_alloc(slave); + if (!s_rt) { + dev_err(&slave->dev, + "Slave runtime alloc failed for stream:%s\n", + stream->name); + ret = -ENOMEM; + goto stream_error; + } + list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); + + ret = sdw_slave_rt_config(s_rt, stream_config); + if (ret) + goto stream_error; + + ret = sdw_config_stream(&slave->dev, stream, stream_config, true); + if (ret) + goto stream_error; + + ret = sdw_slave_port_alloc(slave, s_rt, num_ports); + if (ret) + goto stream_error; + + ret = sdw_slave_port_config(slave, s_rt, port_config); + if (ret) + goto stream_error; + + /* + * Change stream state to CONFIGURED on first Slave add. + * Bus is not aware of number of Slave(s) in a stream at this + * point so cannot depend on all Slave(s) to be added in order to + * change stream state to CONFIGURED. + */ + stream->state = SDW_STREAM_CONFIGURED; + goto error; + +stream_error: + /* + * we hit error so cleanup the stream, release all Slave(s) and + * Master runtime + */ + sdw_release_master_stream(m_rt, stream); +error: + mutex_unlock(&slave->bus->bus_lock); + return ret; +} +EXPORT_SYMBOL(sdw_stream_add_slave); + +/** + * sdw_stream_remove_slave() - Remove slave from sdw_stream + * + * @slave: SDW Slave instance + * @stream: SoundWire stream + * + * This removes and frees port_rt and slave_rt from a stream + */ +int sdw_stream_remove_slave(struct sdw_slave *slave, + struct sdw_stream_runtime *stream) +{ + mutex_lock(&slave->bus->bus_lock); + + sdw_slave_port_free(slave, stream); + sdw_release_slave_stream(slave, stream); + + mutex_unlock(&slave->bus->bus_lock); + + return 0; +} +EXPORT_SYMBOL(sdw_stream_remove_slave); + From 2332f41de6e6bc9158d88ba88a647fb62b16a16f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:02:56 -0500 Subject: [PATCH 25/40] soundwire: stream: rename and move master/slave_rt_free routines The naming is rather inconsistent, use the sdw__ convention, and move the free routine after alloc/config. No functionality change beyond rename/move. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 72 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 0bd08015a587b4..9fd22dafb7ea62 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1054,6 +1054,33 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt, return 0; } +/** + * sdw_slave_rt_free() - Free Slave(s) runtime handle + * + * @slave: Slave handle. + * @stream: Stream runtime handle. + * + * This function is to be called with bus_lock held. + */ +static void sdw_slave_rt_free(struct sdw_slave *slave, + struct sdw_stream_runtime *stream) +{ + struct sdw_slave_runtime *s_rt, *_s_rt; + struct sdw_master_runtime *m_rt; + + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + /* Retrieve Slave runtime handle */ + list_for_each_entry_safe(s_rt, _s_rt, + &m_rt->slave_rt_list, m_rt_node) { + if (s_rt->slave == slave) { + list_del(&s_rt->m_rt_node); + kfree(s_rt); + return; + } + } + } +} + static struct sdw_master_runtime *sdw_master_rt_find(struct sdw_bus *bus, struct sdw_stream_runtime *stream) @@ -1119,51 +1146,24 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt, } /** - * sdw_release_slave_stream() - Free Slave(s) runtime handle - * - * @slave: Slave handle. - * @stream: Stream runtime handle. - * - * This function is to be called with bus_lock held. - */ -static void sdw_release_slave_stream(struct sdw_slave *slave, - struct sdw_stream_runtime *stream) -{ - struct sdw_slave_runtime *s_rt, *_s_rt; - struct sdw_master_runtime *m_rt; - - list_for_each_entry(m_rt, &stream->master_list, stream_node) { - /* Retrieve Slave runtime handle */ - list_for_each_entry_safe(s_rt, _s_rt, - &m_rt->slave_rt_list, m_rt_node) { - if (s_rt->slave == slave) { - list_del(&s_rt->m_rt_node); - kfree(s_rt); - return; - } - } - } -} - -/** - * sdw_release_master_stream() - Free Master runtime handle + * sdw_master_rt_free() - Free Master runtime handle * * @m_rt: Master runtime node * @stream: Stream runtime handle. * * This function is to be called with bus_lock held * It frees the Master runtime handle and associated Slave(s) runtime - * handle. If this is called first then sdw_release_slave_stream() will have + * handle. If this is called first then sdw_slave_rt_free() will have * no effect as Slave(s) runtime handle would already be freed up. */ -static void sdw_release_master_stream(struct sdw_master_runtime *m_rt, - struct sdw_stream_runtime *stream) +static void sdw_master_rt_free(struct sdw_master_runtime *m_rt, + struct sdw_stream_runtime *stream) { struct sdw_slave_runtime *s_rt, *_s_rt; list_for_each_entry_safe(s_rt, _s_rt, &m_rt->slave_rt_list, m_rt_node) { sdw_slave_port_free(s_rt->slave, stream); - sdw_release_slave_stream(s_rt->slave, stream); + sdw_slave_rt_free(s_rt->slave, stream); } list_del(&m_rt->stream_node); @@ -1862,7 +1862,7 @@ int sdw_stream_add_master(struct sdw_bus *bus, goto unlock; stream_error: - sdw_release_master_stream(m_rt, stream); + sdw_master_rt_free(m_rt, stream); unlock: mutex_unlock(&bus->bus_lock); return ret; @@ -1890,7 +1890,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus, continue; sdw_master_port_free(m_rt); - sdw_release_master_stream(m_rt, stream); + sdw_master_rt_free(m_rt, stream); stream->m_rt_count--; } @@ -1991,7 +1991,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * we hit error so cleanup the stream, release all Slave(s) and * Master runtime */ - sdw_release_master_stream(m_rt, stream); + sdw_master_rt_free(m_rt, stream); error: mutex_unlock(&slave->bus->bus_lock); return ret; @@ -2012,7 +2012,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave, mutex_lock(&slave->bus->bus_lock); sdw_slave_port_free(slave, stream); - sdw_release_slave_stream(slave, stream); + sdw_slave_rt_free(slave, stream); mutex_unlock(&slave->bus->bus_lock); From 71d2c27d19cb799782673fa250a13cb75bea7f08 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:08:10 -0500 Subject: [PATCH 26/40] soundwire: stream: move list addition to sdw_slave_alloc_rt() Simplify sdw_stream_add_slave() by moving the linked list management inside of the sdw_slave_alloc_rt_free() helper, this also makes the alloc/free helpers more symmetrical. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 9fd22dafb7ea62..b62e51fbf04a63 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1019,11 +1019,13 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt, * sdw_slave_rt_alloc() - Allocate a Slave runtime handle. * * @slave: Slave handle + * @m_rt: Master runtime handle * * This function is to be called with bus_lock held. */ static struct sdw_slave_runtime -*sdw_slave_rt_alloc(struct sdw_slave *slave) +*sdw_slave_rt_alloc(struct sdw_slave *slave, + struct sdw_master_runtime *m_rt) { struct sdw_slave_runtime *s_rt; @@ -1034,6 +1036,8 @@ static struct sdw_slave_runtime INIT_LIST_HEAD(&s_rt->port_list); s_rt->slave = slave; + list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); + return s_rt; } @@ -1951,7 +1955,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, ret = sdw_master_rt_config(m_rt, stream_config); skip_alloc_master_rt: - s_rt = sdw_slave_rt_alloc(slave); + s_rt = sdw_slave_rt_alloc(slave, m_rt); if (!s_rt) { dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", @@ -1959,7 +1963,6 @@ int sdw_stream_add_slave(struct sdw_slave *slave, ret = -ENOMEM; goto stream_error; } - list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); ret = sdw_slave_rt_config(s_rt, stream_config); if (ret) From a19f1910240509b3f9950adcc980135d1c23d826 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:12:46 -0500 Subject: [PATCH 27/40] soundwire: stream: separate alloc and config within sdw_stream_add_xxx() Separate alloc and config parts so that follow-up patches can allow for multiple calls to sdw_stream_add_slave/master. This is a feature from the ALSA/ASoC frameworks which is not supported today. This is an invasive patch which modifies the error handling flow, with cleanups only done when an allocation fails. Configuration failures only return an error code. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 66 +++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index b62e51fbf04a63..6fcef24123f0e0 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1843,29 +1843,27 @@ int sdw_stream_add_master(struct sdw_bus *bus, ret = -ENOMEM; goto unlock; } +skip_alloc_master_rt: + + ret = sdw_master_port_alloc(m_rt, num_ports); + if (ret) + goto alloc_error; + + stream->m_rt_count++; ret = sdw_master_rt_config(m_rt, stream_config); if (ret < 0) goto unlock; -skip_alloc_master_rt: ret = sdw_config_stream(bus->dev, stream, stream_config, false); if (ret) - goto stream_error; - - ret = sdw_master_port_alloc(m_rt, num_ports); - if (ret) - goto stream_error; + goto unlock; ret = sdw_master_port_config(m_rt, port_config); - if (ret) - goto stream_error; - - stream->m_rt_count++; goto unlock; -stream_error: +alloc_error: sdw_master_rt_free(m_rt, stream); unlock: mutex_unlock(&bus->bus_lock); @@ -1928,6 +1926,9 @@ int sdw_stream_add_slave(struct sdw_slave *slave, { struct sdw_slave_runtime *s_rt; struct sdw_master_runtime *m_rt; + bool alloc_master_rt = true; + bool alloc_slave_rt = true; + int ret; mutex_lock(&slave->bus->bus_lock); @@ -1937,8 +1938,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * and go to configuration */ m_rt = sdw_master_rt_find(slave->bus, stream); - if (m_rt) + if (m_rt) { + alloc_master_rt = false; goto skip_alloc_master_rt; + } /* * If this API is invoked by Slave first then m_rt is not valid. @@ -1950,9 +1953,8 @@ int sdw_stream_add_slave(struct sdw_slave *slave, "alloc master runtime failed for stream:%s\n", stream->name); ret = -ENOMEM; - goto error; + goto unlock; } - ret = sdw_master_rt_config(m_rt, stream_config); skip_alloc_master_rt: s_rt = sdw_slave_rt_alloc(slave, m_rt); @@ -1960,25 +1962,30 @@ int sdw_stream_add_slave(struct sdw_slave *slave, dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name); + alloc_slave_rt = false; ret = -ENOMEM; - goto stream_error; + goto alloc_error; } - ret = sdw_slave_rt_config(s_rt, stream_config); + ret = sdw_slave_port_alloc(slave, s_rt, num_ports); if (ret) - goto stream_error; + goto alloc_error; - ret = sdw_config_stream(&slave->dev, stream, stream_config, true); + ret = sdw_master_rt_config(m_rt, stream_config); if (ret) - goto stream_error; + goto unlock; - ret = sdw_slave_port_alloc(slave, s_rt, num_ports); + ret = sdw_slave_rt_config(s_rt, stream_config); if (ret) - goto stream_error; + goto unlock; + + ret = sdw_config_stream(&slave->dev, stream, stream_config, true); + if (ret) + goto unlock; ret = sdw_slave_port_config(slave, s_rt, port_config); if (ret) - goto stream_error; + goto unlock; /* * Change stream state to CONFIGURED on first Slave add. @@ -1987,15 +1994,17 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * change stream state to CONFIGURED. */ stream->state = SDW_STREAM_CONFIGURED; - goto error; + goto unlock; -stream_error: +alloc_error: /* - * we hit error so cleanup the stream, release all Slave(s) and - * Master runtime + * we only cleanup what was allocated in this routine */ - sdw_master_rt_free(m_rt, stream); -error: + if (alloc_master_rt) + sdw_master_rt_free(m_rt, stream); + else if (alloc_slave_rt) + sdw_slave_rt_free(slave, stream); +unlock: mutex_unlock(&slave->bus->bus_lock); return ret; } @@ -2022,4 +2031,3 @@ int sdw_stream_remove_slave(struct sdw_slave *slave, return 0; } EXPORT_SYMBOL(sdw_stream_remove_slave); - From 88c2376642089906622073b1d8060f36d991de04 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:54:40 -0500 Subject: [PATCH 28/40] soundwire: stream: introduce sdw_slave_rt_find() helper Before we split the alloc and config steps, we need a helper to find the Slave runtime for a stream. The helper is based on the search loop in sdw_slave_rt_free(), which can now be simplified. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 6fcef24123f0e0..4f5607173b60a4 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1058,6 +1058,23 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt, return 0; } +static struct sdw_slave_runtime *sdw_slave_rt_find(struct sdw_slave *slave, + struct sdw_stream_runtime *stream) +{ + struct sdw_slave_runtime *s_rt, *_s_rt; + struct sdw_master_runtime *m_rt; + + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + /* Retrieve Slave runtime handle */ + list_for_each_entry_safe(s_rt, _s_rt, + &m_rt->slave_rt_list, m_rt_node) { + if (s_rt->slave == slave) + return s_rt; + } + } + return NULL; +} + /** * sdw_slave_rt_free() - Free Slave(s) runtime handle * @@ -1069,19 +1086,12 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt, static void sdw_slave_rt_free(struct sdw_slave *slave, struct sdw_stream_runtime *stream) { - struct sdw_slave_runtime *s_rt, *_s_rt; - struct sdw_master_runtime *m_rt; + struct sdw_slave_runtime *s_rt; - list_for_each_entry(m_rt, &stream->master_list, stream_node) { - /* Retrieve Slave runtime handle */ - list_for_each_entry_safe(s_rt, _s_rt, - &m_rt->slave_rt_list, m_rt_node) { - if (s_rt->slave == slave) { - list_del(&s_rt->m_rt_node); - kfree(s_rt); - return; - } - } + s_rt = sdw_slave_rt_find(slave, stream); + if (s_rt) { + list_del(&s_rt->m_rt_node); + kfree(s_rt); } } From c645243f10753838398bc552d7d266a39fc3ac4c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 17:27:33 -0500 Subject: [PATCH 29/40] soundwire: stream: sdw_stream_add_ functions can be called multiple times The sdw_stream_add_slave/master() functions are called from the .hw_params stage. We need to make sure the functions can be called multiple times. In this version, we assume that only 'audio' parameters provide in the hw_params() can change. If the number of ports could change dynamically depending on the stream configuration (number of channels, etc), we would need to free-up all the stream resources and reallocate them. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 4f5607173b60a4..4279b44dd7d84b 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -898,6 +898,11 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt) kfree(p_rt); } +static bool sdw_slave_port_allocated(struct sdw_slave_runtime *s_rt) +{ + return !list_empty(&s_rt->port_list); +} + static void sdw_slave_port_free(struct sdw_slave *slave, struct sdw_stream_runtime *stream) { @@ -972,6 +977,11 @@ static int sdw_slave_port_config(struct sdw_slave *slave, return 0; } +static bool sdw_master_port_allocated(struct sdw_master_runtime *m_rt) +{ + return !list_empty(&m_rt->port_list); +} + static void sdw_master_port_free(struct sdw_master_runtime *m_rt) { struct sdw_port_runtime *p_rt, *_p_rt; @@ -1855,12 +1865,17 @@ int sdw_stream_add_master(struct sdw_bus *bus, } skip_alloc_master_rt: + if (sdw_master_port_allocated(m_rt)) + goto skip_alloc_master_port; + ret = sdw_master_port_alloc(m_rt, num_ports); if (ret) goto alloc_error; stream->m_rt_count++; +skip_alloc_master_port: + ret = sdw_master_rt_config(m_rt, stream_config); if (ret < 0) goto unlock; @@ -1967,6 +1982,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave, } skip_alloc_master_rt: + s_rt = sdw_slave_rt_find(slave, stream); + if (s_rt) + goto skip_alloc_slave_rt; + s_rt = sdw_slave_rt_alloc(slave, m_rt); if (!s_rt) { dev_err(&slave->dev, @@ -1977,10 +1996,15 @@ int sdw_stream_add_slave(struct sdw_slave *slave, goto alloc_error; } +skip_alloc_slave_rt: + if (sdw_slave_port_allocated(s_rt)) + goto skip_port_alloc; + ret = sdw_slave_port_alloc(slave, s_rt, num_ports); if (ret) goto alloc_error; +skip_port_alloc: ret = sdw_master_rt_config(m_rt, stream_config); if (ret) goto unlock; From ad0cab291c5fed207f724ab499f4812307cc40cf Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 13 Aug 2021 12:03:59 -0500 Subject: [PATCH 30/40] soundwire: stream: make enable/disable/deprepare idempotent The stream management currently flags an 'inconsistent state' error when a change is requested multiple times. This was added on purpose to identify programming mistakes. In hindsight, there was no real reason to fail if the logic at the ASoC-DPCM level invokes the same callback multiple times. It's perfectly acceptable to just return and not flag an error when there is nothing to do. The main concern with the state management is to trap errors such as trying to enable a stream that was not prepared first. This patch suggests allowing the stream functions to be idempotent, i.e. they can be called multiple times. Note that the prepare case was already handling multiple calls, this was added in commit c32464c9393d ("soundwire: stream: only prepare stream when it is configured.") Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 4279b44dd7d84b..90a513eb069352 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1505,6 +1505,11 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_ENABLED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_PREPARED && stream->state != SDW_STREAM_DISABLED) { pr_err("%s: %s: inconsistent state state %d\n", @@ -1588,6 +1593,11 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_DISABLED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_ENABLED) { pr_err("%s: %s: inconsistent state state %d\n", __func__, stream->name, stream->state); @@ -1663,6 +1673,11 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_DEPREPARED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_PREPARED && stream->state != SDW_STREAM_DISABLED) { pr_err("%s: %s: inconsistent state state %d\n", From 4a0a02eb2401b26fa6b92d0033cf9a07aa80dcaa Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 25 Aug 2021 14:23:24 -0500 Subject: [PATCH 31/40] ASoC/soundwire: intel: simplify callbacks for params/hw_free We don't really need to pass a substream to the callback, we only need the direction. No functionality change, only simplification to enable improve suspend with paused streams. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 14 +++++++------- include/linux/soundwire/sdw_intel.h | 4 ++-- sound/soc/sof/intel/hda.c | 6 ++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 19f7d1e797eea7..d6dea7a0662c8d 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -711,7 +711,7 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) } static int intel_params_stream(struct sdw_intel *sdw, - struct snd_pcm_substream *substream, + int stream, struct snd_soc_dai *dai, struct snd_pcm_hw_params *hw_params, int link_id, int alh_stream_id) @@ -719,7 +719,7 @@ static int intel_params_stream(struct sdw_intel *sdw, struct sdw_intel_link_res *res = sdw->link_res; struct sdw_intel_stream_params_data params_data; - params_data.substream = substream; + params_data.stream = stream; /* direction */ params_data.dai = dai; params_data.hw_params = hw_params; params_data.link_id = link_id; @@ -732,14 +732,14 @@ static int intel_params_stream(struct sdw_intel *sdw, } static int intel_free_stream(struct sdw_intel *sdw, - struct snd_pcm_substream *substream, + int stream, struct snd_soc_dai *dai, int link_id) { struct sdw_intel_link_res *res = sdw->link_res; struct sdw_intel_stream_free_data free_data; - free_data.substream = substream; + free_data.stream = stream; /* direction */ free_data.dai = dai; free_data.link_id = link_id; @@ -876,7 +876,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream, dma->hw_params = params; /* Inform DSP about PDI stream number */ - ret = intel_params_stream(sdw, substream, dai, params, + ret = intel_params_stream(sdw, substream->stream, dai, params, sdw->instance, pdi->intel_alh_id); if (ret) @@ -953,7 +953,7 @@ static int intel_prepare(struct snd_pcm_substream *substream, sdw_cdns_config_stream(cdns, ch, dir, dma->pdi); /* Inform DSP about PDI stream number */ - ret = intel_params_stream(sdw, substream, dai, + ret = intel_params_stream(sdw, substream->stream, dai, dma->hw_params, sdw->instance, dma->pdi->intel_alh_id); @@ -987,7 +987,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) return ret; } - ret = intel_free_stream(sdw, substream, dai, sdw->instance); + ret = intel_free_stream(sdw, substream->stream, dai, sdw->instance); if (ret < 0) { dev_err(dai->dev, "intel_free_stream: failed %d\n", ret); return ret; diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 8a463b8fc12ad9..67e0d3e750b5c9 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -92,7 +92,7 @@ * firmware. */ struct sdw_intel_stream_params_data { - struct snd_pcm_substream *substream; + int stream; struct snd_soc_dai *dai; struct snd_pcm_hw_params *hw_params; int link_id; @@ -105,7 +105,7 @@ struct sdw_intel_stream_params_data { * firmware. */ struct sdw_intel_stream_free_data { - struct snd_pcm_substream *substream; + int stream; struct snd_soc_dai *dai; int link_id; }; diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 7476d106f3cdcb..f64c071e841bba 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -181,12 +181,11 @@ static int sdw_dai_config_ipc(struct snd_sof_dev *sdev, static int sdw_params_stream(struct device *dev, struct sdw_intel_stream_params_data *params_data) { - struct snd_pcm_substream *substream = params_data->substream; struct snd_sof_dev *sdev = dev_get_drvdata(dev); struct snd_soc_dai *d = params_data->dai; struct snd_soc_dapm_widget *w; - w = snd_soc_dai_get_widget(d, substream->stream); + w = snd_soc_dai_get_widget(d, params_data->stream); return sdw_dai_config_ipc(sdev, w, params_data->link_id, params_data->alh_stream_id, d->id, true); @@ -195,12 +194,11 @@ static int sdw_params_stream(struct device *dev, static int sdw_free_stream(struct device *dev, struct sdw_intel_stream_free_data *free_data) { - struct snd_pcm_substream *substream = free_data->substream; struct snd_sof_dev *sdev = dev_get_drvdata(dev); struct snd_soc_dai *d = free_data->dai; struct snd_soc_dapm_widget *w; - w = snd_soc_dai_get_widget(d, substream->stream); + w = snd_soc_dai_get_widget(d, free_data->stream); /* send invalid stream_id */ return sdw_dai_config_ipc(sdev, w, free_data->link_id, 0xFFFF, d->id, false); From f8ae301677c08ab57b5e89b735c3ab3e95ef754a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 17:42:27 -0500 Subject: [PATCH 32/40] Revert "soundwire: intel: trap TRIGGER_SUSPEND in .trigger callback" This reverts commit 048ecd7251508b51a27c54a298a16f3b6cbdff99. A better implementation will be provided in follow-up patches. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 53 ++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index d6dea7a0662c8d..532ad5b7214d9b 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1008,6 +1008,29 @@ static void intel_shutdown(struct snd_pcm_substream *substream, pm_runtime_put_autosuspend(cdns->dev); } +static int intel_component_dais_suspend(struct snd_soc_component *component) +{ + struct sdw_cdns_dma_data *dma; + struct snd_soc_dai *dai; + + for_each_component_dais(component, dai) { + /* + * we don't have a .suspend dai_ops, and we don't have access + * to the substream, so let's mark both capture and playback + * DMA contexts as suspended + */ + dma = dai->playback_dma_data; + if (dma) + dma->suspended = true; + + dma = dai->capture_dma_data; + if (dma) + dma->suspended = true; + } + + return 0; +} + static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction) { @@ -1036,39 +1059,11 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai, return dma->stream; } -static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) -{ - struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); - struct sdw_intel *sdw = cdns_to_intel(cdns); - struct sdw_cdns_dma_data *dma; - - /* - * The .prepare callback is used to deal with xruns and resume operations. In the case - * of xruns, the DMAs and SHIM registers cannot be touched, but for resume operations the - * DMAs and SHIM registers need to be initialized. - * the .trigger callback is used to track the suspend case only. - */ - if (cmd != SNDRV_PCM_TRIGGER_SUSPEND) - return 0; - - dma = snd_soc_dai_get_dma_data(dai, substream); - if (!dma) { - dev_err(dai->dev, "failed to get dma data in %s\n", - __func__); - return -EIO; - } - - dma->suspended = true; - - return intel_free_stream(sdw, substream, dai, sdw->instance); -} - static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free, - .trigger = intel_trigger, .shutdown = intel_shutdown, .set_stream = intel_pcm_set_sdw_stream, .get_stream = intel_get_sdw_stream, @@ -1079,7 +1074,6 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free, - .trigger = intel_trigger, .shutdown = intel_shutdown, .set_stream = intel_pdm_set_sdw_stream, .get_stream = intel_get_sdw_stream, @@ -1087,6 +1081,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { static const struct snd_soc_component_driver dai_component = { .name = "soundwire", + .suspend = intel_component_dais_suspend }; static int intel_create_dai(struct sdw_cdns *cdns, From a52031ba1f40a3eb6ca13249a7f678b608f7ae4e Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 29 Jun 2021 22:13:04 -0700 Subject: [PATCH 33/40] soundwire: intel: improve suspend flows This patch provides both a simplification of the suspend flows and a better balanced operation during suspend/resume transition, as part of the transition of Sound Open Firmware (SOF) to dynamic pipelines: the DSP resources are only enabled when required instead of enabled on startup. The exiting code relies on a convoluted way of dealing with suspend signals. Since there is no .suspend DAI callback, we used the component .suspend and marked all the component DAI dmas as 'suspended'. The information was used in the .prepare stage to differentiate resume operations from xrun handling, and only reinitialize SHIM registers and DMA in the former case. While this solution has been working reliably for about 2 years, there is a much better solution consisting in trapping the TRIGGER_SUSPEND in the .trigger DAI ops. The DMA is still marked in the same way for the .prepare op to run, but in addition the callbacks sent to DSP firmware are now balanced. Normal operation: hw_params -> intel_params_stream hw_free -> intel_free_stream suspend -> intel_free_stream prepare -> intel_params_stream This balanced operation was not required with existing SOF firmware relying on static pipelines instantiated at every boot. With the on-going transition to dynamic pipelines, it's however a requirement to keep the use count for the DAI widget balanced across all transitions. The component suspend is not removed but instead modified to deal with a corner case: when a substream is PAUSED, the ALSA core does not throw the TRIGGER_SUSPEND. This is problematic since the refcount for all pipelines and widgets is not balanced, leading to issues on resume. The trigger callback keeps track of the 'paused' state with a new flag, which is tested during the component suspend called later to release the remaining DSP resources. These resources will be re-enabled in the .prepare step. The IPC used in the TRIGGER_SUSPEND to release DSP resources is not a problem since the BE dailink is already marked as non-atomic. Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Signed-off-by: Ranjani Sridharan --- drivers/soundwire/cadence_master.h | 2 + drivers/soundwire/intel.c | 113 +++++++++++++++++++++++------ 2 files changed, 91 insertions(+), 24 deletions(-) diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index e587aede63bf05..aa4b9b0eb2a893 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -86,6 +86,7 @@ struct sdw_cdns_stream_config { * @link_id: Master link id * @hw_params: hw_params to be applied in .prepare step * @suspended: status set when suspended, to be used in .prepare + * @paused: status set in .trigger, to be used in suspend */ struct sdw_cdns_dma_data { char *name; @@ -96,6 +97,7 @@ struct sdw_cdns_dma_data { int link_id; struct snd_pcm_hw_params *hw_params; bool suspended; + bool paused; }; /** diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 532ad5b7214d9b..6474cc9c2282b3 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -871,6 +871,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream, sdw_cdns_config_stream(cdns, ch, dir, pdi); /* store pdi and hw_params, may be needed in prepare step */ + dma->paused = false; dma->suspended = false; dma->pdi = pdi; dma->hw_params = params; @@ -1008,29 +1009,6 @@ static void intel_shutdown(struct snd_pcm_substream *substream, pm_runtime_put_autosuspend(cdns->dev); } -static int intel_component_dais_suspend(struct snd_soc_component *component) -{ - struct sdw_cdns_dma_data *dma; - struct snd_soc_dai *dai; - - for_each_component_dais(component, dai) { - /* - * we don't have a .suspend dai_ops, and we don't have access - * to the substream, so let's mark both capture and playback - * DMA contexts as suspended - */ - dma = dai->playback_dma_data; - if (dma) - dma->suspended = true; - - dma = dai->capture_dma_data; - if (dma) - dma->suspended = true; - } - - return 0; -} - static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction) { @@ -1059,11 +1037,97 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai, return dma->stream; } +static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) +{ + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + struct sdw_intel *sdw = cdns_to_intel(cdns); + struct sdw_cdns_dma_data *dma; + int ret; + + dma = snd_soc_dai_get_dma_data(dai, substream); + if (!dma) { + dev_err(dai->dev, "failed to get dma data in %s\n", + __func__); + return -EIO; + } + + switch (cmd) { + case SNDRV_PCM_TRIGGER_SUSPEND: + + /* + * The .prepare callback is used to deal with xruns and resume operations. + * In the case of xruns, the DMAs and SHIM registers cannot be touched, + * but for resume operations the DMAs and SHIM registers need to be initialized. + * the .trigger callback is used to track the suspend case only. + */ + + dma->suspended = true; + + ret = intel_free_stream(sdw, substream->stream, dai, sdw->instance); + break; + + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + dma->paused = true; + break; + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + dma->paused = false; + break; + default: + break; + } + + return ret; +} + +static int intel_component_dais_suspend(struct snd_soc_component *component) +{ + struct snd_soc_dai *dai; + + /* + * In the corner case where a SUSPEND happens during a PAUSE, the ALSA core + * does not throw the TRIGGER_SUSPEND. This leaves the DAIs in an unbalanced state. + * Since the component suspend is called last, we can trap this corner case + * and force the DAIs to release their resources. + */ + for_each_component_dais(component, dai) { + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + struct sdw_intel *sdw = cdns_to_intel(cdns); + struct sdw_cdns_dma_data *dma; + int stream; + int ret; + + dma = dai->playback_dma_data; + stream = SNDRV_PCM_STREAM_PLAYBACK; + if (!dma) { + dma = dai->capture_dma_data; + stream = SNDRV_PCM_STREAM_CAPTURE; + } + + if (!dma) + continue; + + if (dma->suspended) + continue; + + if (dma->paused) { + dma->suspended = true; + + ret = intel_free_stream(sdw, stream, dai, sdw->instance); + if (ret < 0) + return ret; + } + } + + return 0; +} + static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free, + .trigger = intel_trigger, .shutdown = intel_shutdown, .set_stream = intel_pcm_set_sdw_stream, .get_stream = intel_get_sdw_stream, @@ -1074,6 +1138,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free, + .trigger = intel_trigger, .shutdown = intel_shutdown, .set_stream = intel_pdm_set_sdw_stream, .get_stream = intel_get_sdw_stream, @@ -1081,7 +1146,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { static const struct snd_soc_component_driver dai_component = { .name = "soundwire", - .suspend = intel_component_dais_suspend + .suspend = intel_component_dais_suspend, }; static int intel_create_dai(struct sdw_cdns *cdns, From 5b7fe99f39a471a05dfe3bafd30fb51e715265a8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 13 Aug 2021 13:33:57 -0500 Subject: [PATCH 34/40] ASoC: soc-pcm: protect BE dailink state changes in trigger When more than one FE is connected to a BE, e.g. in a mixing use case, the BE can be triggered multiple times when the FE are opened/started concurrently. This race condition is problematic in the case of SoundWire BE dailinks, and this is not desirable in a general case. The code carefully checks when the BE can be stopped or hw_free'ed, but the trigger code does not use any mutual exclusion. Fix by using the same spinlock already used to check FE states, and set the state before the trigger. In case of errors, the initial state will be restored. This patch does not change how the triggers are handled, it only makes sure the states are handled in critical sections. Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 103 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 18 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index fc1854e3e43fd8..86c13b784a403a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1998,6 +1998,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, { struct snd_soc_pcm_runtime *be; struct snd_soc_dpcm *dpcm; + enum snd_soc_dpcm_state state; + unsigned long flags; int ret = 0; for_each_dpcm_be(fe, stream, dpcm) { @@ -2015,76 +2017,141 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, switch (cmd) { case SNDRV_PCM_TRIGGER_START: + spin_lock_irqsave(&fe->card->dpcm_lock, flags); if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) && (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) && - (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) + (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; case SNDRV_PCM_TRIGGER_RESUME: - if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND)) + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; case SNDRV_PCM_TRIGGER_STOP: + spin_lock_irqsave(&fe->card->dpcm_lock, flags); if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) && - (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) + (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP; break; case SNDRV_PCM_TRIGGER_SUSPEND: - if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_SUSPEND; break; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) { + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; + } + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + state = be->dpcm[stream].state; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + spin_lock_irqsave(&fe->card->dpcm_lock, flags); + be->dpcm[stream].state = state; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; + } - be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED; break; } } From e2ce2162cc7f2b6120df6d99faadfbc898c5b2d0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 16 Aug 2021 16:40:18 -0500 Subject: [PATCH 35/40] ASoC: soc-pcm: test refcount before triggering On start/pause_release/resume, when more than one FE is connected to the same BE, it's possible that the trigger is sent more than once. This is not desirable, we only want to trigger a BE once, which is straightforward to implement with a refcount. For stop/pause/suspend, the problem is more complicated: the check implemented in snd_soc_dpcm_can_be_free_stop() may fail due to a conceptual deadlock when we trigger the BE before the FE. In this case, the FE states have not yet changed, so there are corner cases where the TRIGGER_STOP is never sent - the dual case of start where multiple triggers might be sent. This patch suggests an unconditional trigger in all cases, without checking the FE states, using a refcount protected by a spinlock. Signed-off-by: Pierre-Louis Bossart --- include/sound/soc-dpcm.h | 2 ++ sound/soc/soc-pcm.c | 56 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index bc7af90099a8d5..2fec5d91f0d091 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -101,6 +101,8 @@ struct snd_soc_dpcm_runtime { enum snd_soc_dpcm_state state; int trigger_pending; /* trigger cmd + 1 if pending, 0 if not */ + + int be_start; /* refcount protected by dpcm_lock */ }; #define for_each_dpcm_fe(be, stream, _dpcm) \ diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 86c13b784a403a..616489d985475f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1534,7 +1534,7 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; goto unwind; } - + be->dpcm[stream].be_start = 0; be->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN; count++; } @@ -2000,6 +2000,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dpcm *dpcm; enum snd_soc_dpcm_state state; unsigned long flags; + bool do_trigger; int ret = 0; for_each_dpcm_be(fe, stream, dpcm) { @@ -2015,23 +2016,33 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, dev_dbg(be->dev, "ASoC: trigger BE %s cmd %d\n", be->dai_link->name, cmd); + do_trigger = false; switch (cmd) { case SNDRV_PCM_TRIGGER_START: spin_lock_irqsave(&fe->card->dpcm_lock, flags); - if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) && + if (!be->dpcm[stream].be_start && + (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) && (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) && (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) { spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; } state = be->dpcm[stream].state; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start == 1) + do_trigger = true; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + if (!do_trigger) + continue; + ret = soc_pcm_trigger(be_substream, cmd); if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + be->dpcm[stream].be_start--; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } @@ -2045,13 +2056,21 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, } state = be->dpcm[stream].state; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start == 1) + do_trigger = true; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + if (!do_trigger) + continue; + ret = soc_pcm_trigger(be_substream, cmd); if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + be->dpcm[stream].be_start--; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } @@ -2065,13 +2084,21 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, } state = be->dpcm[stream].state; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start == 1) + do_trigger = true; + be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + if (!do_trigger) + continue; + ret = soc_pcm_trigger(be_substream, cmd); if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + be->dpcm[stream].be_start--; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } @@ -2084,9 +2111,15 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; } + if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_START) + be->dpcm[stream].be_start--; + + if (be->dpcm[stream].be_start == 0) + do_trigger = true; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!do_trigger) continue; spin_lock_irqsave(&fe->card->dpcm_lock, flags); @@ -2098,6 +2131,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_START) + be->dpcm[stream].be_start++; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } @@ -2109,9 +2144,14 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; } + + be->dpcm[stream].be_start--; + if (be->dpcm[stream].be_start == 0) + do_trigger = true; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!do_trigger) continue; spin_lock_irqsave(&fe->card->dpcm_lock, flags); @@ -2123,6 +2163,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + be->dpcm[stream].be_start++; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } @@ -2134,9 +2175,13 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; } + be->dpcm[stream].be_start--; + if (be->dpcm[stream].be_start == 0) + do_trigger = true; + spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!do_trigger) continue; spin_lock_irqsave(&fe->card->dpcm_lock, flags); @@ -2148,6 +2193,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (ret) { spin_lock_irqsave(&fe->card->dpcm_lock, flags); be->dpcm[stream].state = state; + be->dpcm[stream].be_start++; spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); goto end; } From 2a3f977d5a55910949258e226a5a582ae4255ec0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 24 Aug 2021 10:21:55 -0500 Subject: [PATCH 36/40] ASoC: soc-pcm: fix BE handling of PAUSE_RELEASE A BE connected to more than one FE, e.g. in a mixer case, can go through the following transitions. play FE1 -> BE state is START pause FE1 -> BE state is PAUSED play FE2 -> BE state is START stop FE2 -> BE state is STOP release FE1 -> BE state is START stop FE1 -> BE state is STOP play FE1 -> BE state is START pause FE1 -> BE state is PAUSED play FE2 -> BE state is START release FE1 -> BE state is START stop FE2 -> BE state is START stop FE1 -> BE state is STOP The existing code for PAUSE_RELEASE only allows for the case where the BE is paused, which clearly would not work in the sequences above. Extend the allowed states to restart the BE when PAUSE_RELEASE is received. Reported-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 616489d985475f..d736e8cfa5c1a9 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2078,7 +2078,9 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: spin_lock_irqsave(&fe->card->dpcm_lock, flags); - if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED) { + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START && + be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP && + be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED) { spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); continue; } From b99224c16400abffa533bea32d309a9f6b1df1d4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 17 Aug 2021 18:02:14 -0500 Subject: [PATCH 37/40] ASoC: SOF: sof-audio: flag errors on pipeline teardown Before suspending, walk through all the widgets to make sure all refcounts are zero. If not, the resume will not work and random errors will be reported. Adding this paranoia check will help identify leaks and broken sequences. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/sof-audio.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index cdad453bf9c1e3..8388a767a4e622 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -827,6 +827,18 @@ int sof_tear_down_pipelines(struct snd_sof_dev *sdev, bool verify) list_for_each_entry(sroute, &sdev->route_list, list) sroute->setup = false; + /* + * before suspending, make sure the refcounts are all zeroed out. There's no way + * to recover at this point but this will help root cause bad sequences leading to + * more issues on resume + */ + list_for_each_entry(swidget, &sdev->widget_list, list) { + if (swidget->use_count != 0) { + dev_err(sdev->dev, "%s: widget %s is still in use: count %d\n", + __func__, swidget->widget->name, swidget->use_count); + } + } + return 0; } From 5e9c75395668162b1dc6d6b080affb27c3a443e2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 13 Sep 2021 19:02:10 -0500 Subject: [PATCH 38/40] ASOC: SOF: Intel: hda-dai: add hda_dai_hw_free_ipc() helper We do the same thing from different places, let's use a helper. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 6bae01b846876d..2dadc597671ccd 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -395,10 +395,25 @@ static int hda_dai_prepare(struct snd_pcm_substream *substream, return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai); } +static int hda_dai_hw_free_ipc(int stream, /* direction */ + struct snd_soc_dai *dai) +{ + struct snd_soc_dapm_widget *w; + int ret; + + w = snd_soc_dai_get_widget(dai, stream); + + /* free the link DMA channel in the FW and the DAI widget */ + ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); + if (ret < 0) + return ret; + + return 0; +} + static int hda_dai_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { - struct snd_soc_dapm_widget *w; int ret; ret = hda_dai_link_trigger(substream, cmd); @@ -409,12 +424,10 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, switch (cmd) { case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - w = snd_soc_dai_get_widget(dai, substream->stream); - /* * free DAI widget during stop/suspend to keep widget use_count's balanced. */ - ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); + ret = hda_dai_hw_free_ipc(substream->stream, dai); if (ret < 0) return ret; @@ -428,21 +441,13 @@ static int hda_dai_trigger(struct snd_pcm_substream *substream, static int hda_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - struct snd_soc_dapm_widget *w; int ret; ret = hda_dai_link_hw_free(substream); if (ret < 0) return ret; - w = snd_soc_dai_get_widget(dai, substream->stream); - - /* free the link DMA channel in the FW and the DAI widget */ - ret = hda_dai_widget_update(dai->dev, w, DMA_CHAN_INVALID, false); - if (ret < 0) - return ret; - - return 0; + return hda_dai_hw_free_ipc(substream->stream, dai); } static const struct snd_soc_dai_ops hda_dai_ops = { From 22e8209cc4ec2b92945556f5d3134bd5ae623892 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 13 Sep 2021 19:03:40 -0500 Subject: [PATCH 39/40] ASoC: SOF: Intel: hda-dai: move code to deal with hda dai/dailink suspend The location of the code was not optimal and prevents us from using helpers, let's move it to hda-dai.c, add comments and re-align with the TRIGGER_SUSPEND case with an additional call to hda_dai_hw_free_ipc() to free-up resources. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 76 +++++++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda-dsp.c | 42 +++---------------- sound/soc/sof/intel/hda.h | 1 + 3 files changed, 83 insertions(+), 36 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 2dadc597671ccd..0c5603c0eb2c0d 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -457,6 +457,63 @@ static const struct snd_soc_dai_ops hda_dai_ops = { .prepare = hda_dai_prepare, }; +static int hda_dai_suspend(struct hdac_bus *bus) +{ + struct snd_soc_pcm_runtime *rtd; + struct hdac_ext_stream *he_stream; + struct hdac_ext_link *link; + struct hdac_stream *s; + const char *name; + int stream_tag; + int ret; + + /* set internal flag for BE */ + list_for_each_entry(s, &bus->stream_list, list) { + he_stream = stream_to_hdac_ext_stream(s); + + if (!he_stream) + return -EINVAL; + + /* + * clear 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 (he_stream->link_substream) { + struct snd_soc_dai *cpu_dai; + struct snd_soc_dai *codec_dai; + + rtd = asoc_substream_to_rtd(he_stream->link_substream); + cpu_dai = asoc_rtd_to_cpu(rtd, 0); + codec_dai = asoc_rtd_to_codec(rtd, 0); + name = codec_dai->component->name; + link = snd_hdac_ext_bus_get_link(bus, name); + if (!link) + return -EINVAL; + + if (hdac_stream(he_stream)->direction == SNDRV_PCM_STREAM_PLAYBACK) { + stream_tag = hdac_stream(he_stream)->stream_tag; + snd_hdac_ext_link_clear_stream_id(link, stream_tag); + } + + he_stream->link_prepared = 0; + + /* + * we don't need to call snd_hdac_ext_link_stream_clear(he_stream) + * since we can only reach this case in the pause_push state, and + * the TRIGGER_PAUSE_PUSH already stops the DMA + */ + + /* for consistency with TRIGGER_SUSPEND we free DAI resources */ + ret = hda_dai_hw_free_ipc(hdac_stream(he_stream)->direction, cpu_dai); + if (ret < 0) + return ret; + } + } + + return 0; +} #endif /* only one flag used so far to harden hw_params/hw_free/trigger/prepare */ @@ -759,3 +816,22 @@ struct snd_soc_dai_driver skl_dai[] = { #endif #endif }; + +int hda_dsp_dais_suspend(struct snd_sof_dev *sdev) +{ + /* + * In the corner case where a SUSPEND happens during a PAUSE, the ALSA core + * does not throw the TRIGGER_SUSPEND. This leaves the DAIs in an unbalanced state. + * Since the component suspend is called last, we can trap this corner case + * and force the DAIs to release their resources. + */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + int ret; + + ret = hda_dai_suspend(sof_to_bus(sdev)); + if (ret < 0) + return ret; +#endif + + return 0; +} diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index bba95ab4c1a31e..bfe5dc035eacaa 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -902,44 +902,14 @@ int hda_dsp_shutdown(struct snd_sof_dev *sdev) int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev) { -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - struct hdac_bus *bus = sof_to_bus(sdev); - struct snd_soc_pcm_runtime *rtd; - struct hdac_ext_stream *he_stream; - struct hdac_ext_link *link; - struct hdac_stream *s; - const char *name; - int stream_tag; - - /* set internal flag for BE */ - list_for_each_entry(s, &bus->stream_list, list) { - he_stream = stream_to_hdac_ext_stream(s); - - /* - * clear 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 (he_stream->link_substream) { - rtd = asoc_substream_to_rtd(he_stream->link_substream); - name = asoc_rtd_to_codec(rtd, 0)->component->name; - link = snd_hdac_ext_bus_get_link(bus, name); - if (!link) - return -EINVAL; - - he_stream->link_prepared = 0; + int ret; - if (hdac_stream(he_stream)->direction == - SNDRV_PCM_STREAM_CAPTURE) - continue; + /* make sure all DAI resources are freed */ + ret = hda_dsp_dais_suspend(sdev); + if (ret < 0) + dev_warn(sdev->dev, "%s: failure in hda_dsp_dais_suspend\n", __func__); - stream_tag = hdac_stream(he_stream)->stream_tag; - snd_hdac_ext_link_clear_stream_id(link, stream_tag); - } - } -#endif - return 0; + return ret; } void hda_dsp_d0i3_work(struct work_struct *work) diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index a018a4e5b82cb5..7e4dac77d563fa 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -708,6 +708,7 @@ static inline bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev) /* common dai driver */ extern struct snd_soc_dai_driver skl_dai[]; +int hda_dsp_dais_suspend(struct snd_sof_dev *sdev); /* * Platform Specific HW abstraction Ops. From 929aeaab7684e14ca7593bcc5dfeaf27e25746c7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 16 Sep 2021 14:59:03 -0500 Subject: [PATCH 40/40] ASoC: SOF: Intel: hda-dai: reset dma_data and release stream The sequences are missing a call to snd_soc_dai_set_dma_data() when the stream is cleared, as well as a release of the stream, and tests to avoid pointer dereferences. This fixes an underflow issue in a corner case with two streams paused before a suspend-resume cycle. After resume, the pause_release of the last stream causes an underflow due to an invalid sequence. This problem probably existed since the beginning and is only see with prototypes of a 'deep-buffer' capability, which depends on additional ASoC fixes, so there's is no Fixes: tag and no real requirement to backport this patch. BugLink: https://github.com/thesofproject/linux/issues/3151 Co-developed-by: Ranjani Sridharan Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 0c5603c0eb2c0d..98c86100f22248 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -211,7 +211,7 @@ static int hda_dai_link_prepare(struct snd_pcm_substream *substream) struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(cpu_dai->component); int stream = substream->stream; - if (he_stream->link_prepared) + if (he_stream && he_stream->link_prepared) return 0; dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream); @@ -235,6 +235,9 @@ static int hda_dai_link_trigger(struct snd_pcm_substream *substream, int cmd) return -EINVAL; dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd); + if (!he_stream) + return 0; + switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: @@ -246,7 +249,8 @@ static int hda_dai_link_trigger(struct snd_pcm_substream *substream, int cmd) stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); } - + snd_soc_dai_set_dma_data(cpu_dai, substream, NULL); + snd_hdac_ext_stream_release(he_stream, HDAC_EXT_STREAM_TYPE_LINK); he_stream->link_prepared = 0; fallthrough; @@ -469,6 +473,8 @@ static int hda_dai_suspend(struct hdac_bus *bus) /* set internal flag for BE */ list_for_each_entry(s, &bus->stream_list, list) { + struct sof_intel_hda_stream *hda_stream; + he_stream = stream_to_hdac_ext_stream(s); if (!he_stream) @@ -496,7 +502,8 @@ static int hda_dai_suspend(struct hdac_bus *bus) stream_tag = hdac_stream(he_stream)->stream_tag; snd_hdac_ext_link_clear_stream_id(link, stream_tag); } - + snd_soc_dai_set_dma_data(cpu_dai, he_stream->link_substream, NULL); + snd_hdac_ext_stream_release(he_stream, HDAC_EXT_STREAM_TYPE_LINK); he_stream->link_prepared = 0; /* @@ -510,6 +517,10 @@ static int hda_dai_suspend(struct hdac_bus *bus) if (ret < 0) return ret; } + + /* free the host DMA channel reserved by hostless streams */ + hda_stream = hstream_to_sof_hda_stream(he_stream); + hda_stream->host_reserved = 0; } return 0;