From 13da0021155100b398b1115846fab709525999e8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 14 Sep 2021 06:47:52 -0500 Subject: [PATCH 01/49] 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 56ea5cde5e63ad..acf711a914f037 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -78,35 +78,35 @@ 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_locked(struct hdac_bus *bus, - struct hdac_ext_stream *azx_dev, bool decouple); + struct hdac_ext_stream *he_stream, bool decouple); void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, struct hdac_ext_stream *azx_dev, bool decouple); 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 c09652da43ffdd..973c17099f25aa 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,22 +95,22 @@ 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); void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus, - struct hdac_ext_stream *stream, + 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); @@ -121,76 +121,76 @@ void snd_hdac_ext_stream_decouple_locked(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; } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple_locked); /** * 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) { spin_lock_irq(&bus->reg_lock); - snd_hdac_ext_stream_decouple_locked(bus, stream, decouple); + snd_hdac_ext_stream_decouple_locked(bus, he_stream, 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); @@ -201,24 +201,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; } @@ -230,7 +230,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); } @@ -250,10 +250,10 @@ 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"); @@ -261,22 +261,22 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus, } spin_lock_irq(&bus->reg_lock); - 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_locked(bus, hstream, true); - res = hstream; + if (!he_stream->link_locked) { + snd_hdac_ext_stream_decouple_locked(bus, he_stream, true); + res = he_stream; break; } } @@ -290,10 +290,10 @@ 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"); @@ -301,17 +301,17 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus, } spin_lock_irq(&bus->reg_lock); - 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_locked(bus, hstream, true); - res = hstream; + if (!hstream->opened) { + if (!he_stream->decoupled) + snd_hdac_ext_stream_decouple_locked(bus, he_stream, true); + res = he_stream; break; } } @@ -346,16 +346,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); @@ -371,34 +372,34 @@ 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: spin_lock_irq(&bus->reg_lock); - if (stream->decoupled && !stream->link_locked) - snd_hdac_ext_stream_decouple_locked(bus, stream, false); - spin_unlock_irq(&bus->reg_lock); - snd_hdac_stream_release(&stream->hstream); + if (he_stream->decoupled && !he_stream->link_locked) + snd_hdac_ext_stream_decouple_locked(bus, he_stream, false); + spin_lock_irq(&bus->reg_lock); + snd_hdac_stream_release(&he_stream->hstream); break; case HDAC_EXT_STREAM_TYPE_LINK: spin_lock_irq(&bus->reg_lock); - if (stream->decoupled && !stream->hstream.opened) - snd_hdac_ext_stream_decouple_locked(bus, stream, false); - stream->link_locked = 0; - stream->link_substream = NULL; + if (he_stream->decoupled && !he_stream->hstream.opened) + snd_hdac_ext_stream_decouple_locked(bus, he_stream, false); + he_stream->link_locked = 0; + he_stream->link_substream = NULL; spin_unlock_irq(&bus->reg_lock); break; @@ -437,11 +438,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) { @@ -449,7 +450,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; } @@ -458,12 +459,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) { @@ -471,7 +472,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); @@ -503,11 +504,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) { @@ -515,7 +516,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; } @@ -523,12 +524,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 2396cc0c59999d49363a6021999e51134ee71a98 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 14 Sep 2021 07:55:09 -0500 Subject: [PATCH 02/49] 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 f81af21b4f4778..a8cdaaffffb473 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -60,7 +60,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; @@ -70,28 +70,28 @@ static struct hdac_ext_stream * } spin_lock_irq(&bus->reg_lock); - 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. @@ -119,17 +119,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, @@ -138,9 +138,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, @@ -148,7 +148,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *stream, } } - stream->link_prepared = 1; + he_stream->link_prepared = 1; return 0; } @@ -205,7 +205,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; @@ -216,18 +216,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; @@ -261,20 +261,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); @@ -286,7 +286,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; @@ -305,7 +305,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) { @@ -321,7 +321,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: @@ -338,15 +338,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; @@ -363,22 +363,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; @@ -395,13 +395,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 b3f78639c30238..859105c1f216b9 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 4502a6133735ca..cf0b017824c69b 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; } @@ -648,23 +648,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; } @@ -792,7 +792,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); @@ -856,27 +856,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; @@ -911,28 +911,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; @@ -967,7 +967,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 */ @@ -987,9 +987,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 b60c77cd0d8f62..2b9ed8aedf50df 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 b096ecc7797b4fdce0a1283d85f8b8182739fdca Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 15:38:23 -0500 Subject: [PATCH 03/49] 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 e5da9fc1b6d807b909718f2b9612abba66f7d639 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 16:17:21 -0500 Subject: [PATCH 04/49] 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 a8cdaaffffb473..7a80bc2ce4668c 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -243,11 +243,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 d71ca5132d4fe5ff4ef7f318a0c7a18bf06f6758 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 31 Aug 2021 08:50:07 -0500 Subject: [PATCH 05/49] 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 7a80bc2ce4668c..19687f368405ec 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -229,10 +229,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); @@ -322,10 +319,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. @@ -377,10 +371,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); @@ -429,10 +420,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 b2e99d35053ea9..6d6fdd9959c1c8 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 532860c4318ba0823240d594eaefafec3ca83702 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 21:53:03 -0500 Subject: [PATCH 06/49] 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 19687f368405ec..471da2302879e9 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -303,16 +303,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 2966f4d76f8795a9a8a95948f291ff2bddcfde3f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 21:41:35 -0500 Subject: [PATCH 07/49] 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 471da2302879e9..ad6316e6127ff8 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -54,8 +54,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; @@ -179,9 +179,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; @@ -199,9 +199,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; @@ -232,7 +232,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; @@ -258,8 +258,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); @@ -273,12 +273,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); @@ -314,7 +313,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; @@ -335,8 +334,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; @@ -364,7 +363,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; @@ -387,11 +386,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 @@ -613,7 +612,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, @@ -621,7 +620,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, @@ -629,7 +628,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, @@ -637,7 +636,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, @@ -645,7 +644,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, @@ -657,7 +656,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, @@ -669,7 +668,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 77a3cf9e3e8adb2ca9e7fa2e6adf952166fbdbdb Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 22:36:56 -0500 Subject: [PATCH 08/49] 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 ad6316e6127ff8..1f1818631f53bf 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -179,16 +179,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; } @@ -232,7 +231,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; @@ -313,7 +312,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; @@ -363,7 +362,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 71301fcacdcf807e290edf30e48382932471a525 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Aug 2021 22:33:18 -0500 Subject: [PATCH 09/49] 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 | 220 +++++++++++++++++++++------------- 1 file changed, 136 insertions(+), 84 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 1f1818631f53bf..21211f200bca62 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -198,49 +198,34 @@ 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); @@ -257,49 +242,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); + + ret = hda_dai_link_prepare(substream); + if (ret < 0) + return ret; + + return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai); +} - link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name); +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: @@ -307,15 +331,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); @@ -333,40 +348,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; @@ -375,16 +406,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 269c3c27f5d5179152e39cee5b3f7629cfd5c37f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 8 Sep 2021 15:06:43 -0500 Subject: [PATCH 10/49] 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 21211f200bca62..4d33d935135058 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -153,51 +153,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) { @@ -242,39 +197,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); @@ -291,23 +213,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; @@ -348,36 +253,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; @@ -417,6 +292,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 f90211141286e87e54c9aa8ec325253b1f67c667 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 15:28:14 -0500 Subject: [PATCH 11/49] 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 70d78d3ac1bae5ad050fe4878b417b672b624d6a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 27 Aug 2021 16:19:52 -0500 Subject: [PATCH 12/49] 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 5490a5ac22f4d2e576dfd935d63d6323f25ca0e0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 27 Aug 2021 16:34:24 -0500 Subject: [PATCH 13/49] 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 7a1dcb70d5404eea9c8f260fb6d8f5a3f1f67924 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:08:25 -0500 Subject: [PATCH 14/49] 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 c6ce5537f7aea45687ad086dc04b4f11ec52f9a0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 8 Sep 2021 15:58:08 -0500 Subject: [PATCH 15/49] 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 ec8d21ffe540d34af2361573c5f1e87af5a15f9c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:39:50 -0500 Subject: [PATCH 16/49] 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 7f134d68e2c604084955d04e11ff45a238edaa0c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 10:56:18 -0500 Subject: [PATCH 17/49] 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 86bba9d05ba9f806484e2cc4351240ea68dff591 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:02:51 -0500 Subject: [PATCH 18/49] 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 aad6f1e3e121e0a04c83cab2a8a39ae04ee0fa61 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:11:31 -0500 Subject: [PATCH 19/49] 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 78b7f69d4788ad3b043c444fdf71218c224409cf Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:30:19 -0500 Subject: [PATCH 20/49] 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 15a7633bb247b2751abb3c79338486d1af4678d0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 11:47:52 -0500 Subject: [PATCH 21/49] 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 cc559b121e250d9e9b4411ee09ffe960cdbcb656 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 13:48:33 -0500 Subject: [PATCH 22/49] 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 62de7f5af15063b0126204996e3ee4c1ee449f52 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 12:00:01 -0500 Subject: [PATCH 23/49] 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 b9e7394225121d7162f7a7eabe29ddd930d869e1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 13:32:37 -0500 Subject: [PATCH 24/49] 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 e85b0a470299c80557cde163f808ba2c81d2691d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:02:56 -0500 Subject: [PATCH 25/49] 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 96a1736cc3d2c3f3e12feb6a5b81867a69cb59b7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:08:10 -0500 Subject: [PATCH 26/49] 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 793b7b0cdd9f71db4dc8e90d8766b732e8b5f52c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:12:46 -0500 Subject: [PATCH 27/49] 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 87adb12a253dde2b0607a8031253a03482bcb896 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 14:54:40 -0500 Subject: [PATCH 28/49] 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 d3af764c46070cc6f083f5d40c1679c84697ad44 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 17:27:33 -0500 Subject: [PATCH 29/49] 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 1bc7109fc58158de506fdefc27266b0340656c7e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 13 Aug 2021 12:03:59 -0500 Subject: [PATCH 30/49] 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 aa4d479c39b990762718d169f9fd7f9563dc25e2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 25 Aug 2021 14:23:24 -0500 Subject: [PATCH 31/49] 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 6d6fdd9959c1c8..de44a52baf6e27 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 2527e308d2e00dec98a0e12cc377228261115e03 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 9 Sep 2021 17:42:27 -0500 Subject: [PATCH 32/49] 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 63c98d0a02fbbd18db017559ba9b90fe85ff3b9a Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 29 Jun 2021 22:13:04 -0700 Subject: [PATCH 33/49] 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 031ac4bcb9a9f4c92da71e8170f497bd574afc53 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 17 Aug 2021 18:02:14 -0500 Subject: [PATCH 34/49] 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 d3ce86bb8031531fbbf9c1b39b066783ddfac083 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 13 Sep 2021 19:02:10 -0500 Subject: [PATCH 35/49] 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 4d33d935135058..d3601579e7d119 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -389,10 +389,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); @@ -403,12 +418,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; @@ -422,21 +435,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 ca8b267a2a6eca1f0e06e0318b39ae5d9c4cf7ac Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 13 Sep 2021 19:03:40 -0500 Subject: [PATCH 36/49] 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 d3601579e7d119..72d360b8648281 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -451,6 +451,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 */ @@ -753,3 +810,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 2b9ed8aedf50df..22b63e6be9998b 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 f5edd38ab388a86be2a69521fdf8b43d80d7bc12 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 16 Sep 2021 14:59:03 -0500 Subject: [PATCH 37/49] 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 72d360b8648281..8619171005a60b 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -205,7 +205,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); @@ -229,6 +229,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: @@ -240,7 +243,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; @@ -463,6 +467,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) @@ -490,7 +496,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; /* @@ -504,6 +511,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; From d4f82c0391783d218b2dbd3360e352a6db2e059e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 30 Sep 2021 13:14:21 -0500 Subject: [PATCH 38/49] ASoC: soc-pcm: remove snd_soc_dpcm_fe_can_update() This function is not used anywhere, including soc-pcm.c Remove dead code. Signed-off-by: Pierre-Louis Bossart --- include/sound/soc-dpcm.h | 3 --- sound/soc/soc-pcm.c | 9 --------- 2 files changed, 12 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index bc7af90099a8d5..72d45ad47ee3aa 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -121,9 +121,6 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); -/* is the current PCM operation for this FE ? */ -int snd_soc_dpcm_fe_can_update(struct snd_soc_pcm_runtime *fe, int stream); - /* is the current PCM operation for this BE ? */ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 0aa0ae9703d13d..2790379015ca3c 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2815,15 +2815,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) return ret; } -/* is the current PCM operation for this FE ? */ -int snd_soc_dpcm_fe_can_update(struct snd_soc_pcm_runtime *fe, int stream) -{ - if (fe->dpcm[stream].runtime_update == SND_SOC_DPCM_UPDATE_FE) - return 1; - return 0; -} -EXPORT_SYMBOL_GPL(snd_soc_dpcm_fe_can_update); - /* is the current PCM operation for this BE ? */ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) From 2ff7d5be4e34f77caf8cf8b98e073d6c4780f45e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 30 Sep 2021 13:06:05 -0500 Subject: [PATCH 39/49] ASoC: soc-pcm: don't export local functions, use static No one uses the following functions outside of soc-pcm.c snd_soc_dpcm_can_be_free_stop() snd_soc_dpcm_can_be_params() snd_soc_dpcm_be_can_update() In preparation for locking changes, move them to static functions and remove the EXPORT_SYMBOL_GPL() Signed-off-by: Pierre-Louis Bossart --- include/sound/soc-dpcm.h | 12 ------------ sound/soc/soc-pcm.c | 21 +++++++++++++++------ 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 72d45ad47ee3aa..9c00118603e743 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -113,18 +113,6 @@ struct snd_soc_dpcm_runtime { #define for_each_dpcm_be_rollback(fe, stream, _dpcm) \ list_for_each_entry_continue_reverse(_dpcm, &(fe)->dpcm[stream].be_clients, list_be) -/* can this BE stop and free */ -int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); - -/* can this BE perform a hw_params() */ -int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); - -/* is the current PCM operation for this BE ? */ -int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); - /* get the substream for this BE */ struct snd_pcm_substream * snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream); diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 2790379015ca3c..d428a8815d3929 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -29,6 +29,18 @@ #define DPCM_MAX_BE_USERS 8 +/* can this BE stop and free */ +static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream); + +/* can this BE perform a hw_params() */ +static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream); + +/* is the current PCM operation for this BE ? */ +static int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream); + static inline const char *soc_cpu_dai_name(struct snd_soc_pcm_runtime *rtd) { return (rtd)->num_cpus == 1 ? asoc_rtd_to_cpu(rtd, 0)->name : "multicpu"; @@ -2816,7 +2828,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) } /* is the current PCM operation for this BE ? */ -int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, +static int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { if ((fe->dpcm[stream].runtime_update == SND_SOC_DPCM_UPDATE_FE) || @@ -2825,7 +2837,6 @@ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, return 1; return 0; } -EXPORT_SYMBOL_GPL(snd_soc_dpcm_be_can_update); /* get the substream for this BE */ struct snd_pcm_substream * @@ -2871,7 +2882,7 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE * are not running, paused or suspended for the specified stream direction. */ -int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, +static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { const enum snd_soc_dpcm_state state[] = { @@ -2882,13 +2893,12 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); } -EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop); /* * We can only change hw params a BE DAI if any of it's FE are not prepared, * running, paused or suspended for the specified stream direction. */ -int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, +static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { const enum snd_soc_dpcm_state state[] = { @@ -2900,4 +2910,3 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); } -EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params); From 142815a7f7f3f5732896d9c6cda44f8dad229033 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 14:01:07 -0500 Subject: [PATCH 40/49] ASoC: soc-pcm: use proper indentation on 'continue' Minor realignment before more changes. Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d428a8815d3929..19539300d94dc1 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1856,7 +1856,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) /* only free hw when no longer used - check all FEs */ if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) - continue; + continue; /* do not free hw if this BE is used by other FE */ if (be->dpcm[stream].users > 1) From eb0cba23a6012485d073b71c8737dc78bcfc8e7c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 29 Sep 2021 11:30:19 -0500 Subject: [PATCH 41/49] ASoC: soc-pcm: introduce snd_soc_dpcm_lock/unlock() In preparation for more changes, add two new helpers to hide the actual type of lock being used for DPCM. Since DPCM functions are not used from interrupt handlers, use spin_lock_irq instead of the spin_lock_irqsave version. While most of the uses of DPCM are internal to soc-pcm.c, some drivers in soc/fsl and soc/sh do make use of DPCM-related loops that will require protection, adding EXPORT_SYMBOL_GPL() is needed for those drivers. Signed-off-by: Pierre-Louis Bossart --- include/sound/soc-dpcm.h | 3 +++ sound/soc/soc-pcm.c | 42 +++++++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 9c00118603e743..3202f3230c4e00 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -151,4 +151,7 @@ bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget, enum snd_soc_dapm_d #define dpcm_be_dai_startup_unwind(fe, stream) dpcm_be_dai_stop(fe, stream, 0, NULL) #define dpcm_be_dai_shutdown(fe, stream) dpcm_be_dai_stop(fe, stream, 1, NULL) +void snd_soc_dpcm_lock(struct snd_soc_pcm_runtime *fe); +void snd_soc_dpcm_unlock(struct snd_soc_pcm_runtime *fe); + #endif diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 19539300d94dc1..86c771d69d4dee 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -29,6 +29,18 @@ #define DPCM_MAX_BE_USERS 8 +void snd_soc_dpcm_lock(struct snd_soc_pcm_runtime *fe) +{ + spin_lock_irq(&fe->card->dpcm_lock); +} +EXPORT_SYMBOL_GPL(snd_soc_dpcm_lock); + +void snd_soc_dpcm_unlock(struct snd_soc_pcm_runtime *fe) +{ + spin_unlock_irq(&fe->card->dpcm_lock); +} +EXPORT_SYMBOL_GPL(snd_soc_dpcm_unlock); + /* can this BE stop and free */ static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); @@ -85,7 +97,6 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, struct snd_pcm_hw_params *params = &fe->dpcm[stream].hw_params; struct snd_soc_dpcm *dpcm; ssize_t offset = 0; - unsigned long flags; /* FE state */ offset += scnprintf(buf + offset, size - offset, @@ -113,7 +124,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, goto out; } - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; params = &dpcm->hw_params; @@ -134,7 +145,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, params_channels(params), params_rate(params)); } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); out: return offset; } @@ -1141,7 +1152,6 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; /* only add new dpcms */ for_each_dpcm_be(fe, stream, dpcm) { @@ -1157,10 +1167,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1203,7 +1213,6 @@ static void dpcm_be_reparent(struct snd_soc_pcm_runtime *fe, void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; - unsigned long flags; for_each_dpcm_be_safe(fe, stream, dpcm, d) { dev_dbg(fe->dev, "ASoC: BE %s disconnect check for %s\n", @@ -1222,10 +1231,10 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) dpcm_remove_debugfs_state(dpcm); - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); kfree(dpcm); } } @@ -1451,12 +1460,11 @@ int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) dpcm_set_be_update_state(dpcm->be, stream, SND_SOC_DPCM_UPDATE_NO); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); } void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, @@ -2374,7 +2382,6 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream]; int ret = 0; - unsigned long flags; dev_dbg(fe->dev, "ASoC: runtime %s open on FE %s\n", stream ? "capture" : "playback", fe->dai_link->name); @@ -2443,7 +2450,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) dpcm_be_dai_shutdown(fe, stream); disconnect: /* disconnect any pending BEs */ - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -2455,7 +2462,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state == SND_SOC_DPCM_STATE_NEW) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret); @@ -2855,10 +2862,9 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; int state; int ret = 1; - unsigned long flags; int i; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_lock(fe); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -2872,7 +2878,7 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, } } } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + snd_soc_dpcm_unlock(fe); /* it's safe to do this BE DAI */ return ret; From 78efd2c409b14ea0b44dabab8d0027b1ffa251eb Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 14:51:06 -0500 Subject: [PATCH 42/49] ASoC: soc-pcm: protect for_each_dpcm_be() loops The D in DPCM stands for 'dynamic', which means that connections between FE and BE can evolve. Commit a97648697790 ("ASoC: dpcm: prevent snd_soc_dpcm use after free") started to protect some of the for_each_dpcm_be() loops, but there are still many cases that were not modified. This patch adds protection for all the loops, with the notable exception of the dpcm_be_dai_trigger(), where additional changes are required. Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 68 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 86c771d69d4dee..d641839b7435a7 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(snd_soc_dpcm_unlock); static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); +static int snd_soc_dpcm_can_be_free_stop_unlocked(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream); + /* can this BE perform a hw_params() */ static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); @@ -332,6 +335,7 @@ int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, { struct snd_soc_dpcm *dpcm; + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, dir, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -345,6 +349,8 @@ int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, snd_soc_dapm_stream_event(be, dir, event); } + snd_soc_dpcm_unlock(fe); + snd_soc_dapm_stream_event(fe, dir, event); @@ -1154,10 +1160,14 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; /* only add new dpcms */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { - if (dpcm->be == be && dpcm->fe == fe) + if (dpcm->be == be && dpcm->fe == fe) { + snd_soc_dpcm_unlock(fe); return 0; + } } + snd_soc_dpcm_unlock(fe); dpcm = kzalloc(sizeof(struct snd_soc_dpcm), GFP_KERNEL); if (!dpcm) @@ -1214,6 +1224,7 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; + snd_soc_dpcm_lock(fe); for_each_dpcm_be_safe(fe, stream, dpcm, d) { dev_dbg(fe->dev, "ASoC: BE %s disconnect check for %s\n", stream ? "capture" : "playback", @@ -1231,12 +1242,11 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) dpcm_remove_debugfs_state(dpcm); - snd_soc_dpcm_lock(fe); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - snd_soc_dpcm_unlock(fe); kfree(dpcm); } + snd_soc_dpcm_unlock(fe); } /* get BE for DAI widget and stream */ @@ -1363,6 +1373,7 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, int prune = 0; /* Destroy any old FE <--> BE connections */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { if (dpcm_be_is_active(dpcm, stream, *list_)) continue; @@ -1374,6 +1385,7 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, dpcm_set_be_update_state(dpcm->be, stream, SND_SOC_DPCM_UPDATE_BE); prune++; } + snd_soc_dpcm_unlock(fe); dev_dbg(fe->dev, "ASoC: found %d old BE paths for pruning\n", prune); return prune; @@ -1473,13 +1485,16 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dpcm *dpcm; /* disable any enabled and non active backends */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *be_substream = snd_soc_dpcm_get_substream(be, stream); - if (dpcm == last) + if (dpcm == last) { + snd_soc_dpcm_unlock(fe); return; + } /* is this op for this BE ? */ if (!snd_soc_dpcm_be_can_update(fe, be, stream)) @@ -1509,6 +1524,7 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, be_substream->runtime = NULL; be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; } + snd_soc_dpcm_unlock(fe); } int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) @@ -1518,6 +1534,7 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) int err, count = 0; /* only startup BE DAIs that are either sinks or sources to this FE DAI */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_pcm_substream *be_substream; @@ -1568,11 +1585,13 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN; count++; } + snd_soc_dpcm_unlock(fe); return count; unwind: dpcm_be_dai_startup_rollback(fe, stream, dpcm); + snd_soc_dpcm_unlock(fe); dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n", __func__, be->dai_link->name, err); @@ -1627,6 +1646,7 @@ static void dpcm_runtime_setup_be_format(struct snd_pcm_substream *substream) * if FE want to use it (= dpcm_merged_format) */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *codec_stream; @@ -1645,6 +1665,7 @@ static void dpcm_runtime_setup_be_format(struct snd_pcm_substream *substream) soc_pcm_hw_update_format(hw, codec_stream); } } + snd_soc_dpcm_unlock(fe); } static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) @@ -1662,7 +1683,7 @@ static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) * It returns merged BE codec channel; * if FE want to use it (= dpcm_merged_chan) */ - + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *cpu_stream; @@ -1693,6 +1714,7 @@ static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) soc_pcm_hw_update_chan(hw, codec_stream); } } + snd_soc_dpcm_unlock(fe); } static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) @@ -1710,7 +1732,7 @@ static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) * It returns merged BE codec channel; * if FE want to use it (= dpcm_merged_chan) */ - + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *pcm; @@ -1730,6 +1752,7 @@ static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) soc_pcm_hw_update_rate(hw, pcm); } } + snd_soc_dpcm_unlock(fe); } static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, @@ -1752,6 +1775,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, } /* apply symmetry for BE */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *be_substream = @@ -1777,6 +1801,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, } } error: + snd_soc_dpcm_unlock(fe); if (err < 0) dev_err(fe->dev, "ASoC: %s failed (%d)\n", __func__, err); @@ -1852,6 +1877,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) /* only hw_params backends that are either sinks or sources * to this frontend DAI */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -1885,6 +1911,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE; } + snd_soc_dpcm_unlock(fe); } static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream) @@ -1918,6 +1945,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; int ret; + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { be = dpcm->be; be_substream = snd_soc_dpcm_get_substream(be, stream); @@ -1957,6 +1985,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS; } + snd_soc_dpcm_unlock(fe); return 0; unwind: @@ -1983,6 +2012,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) soc_pcm_hw_free(be_substream); } + snd_soc_dpcm_unlock(fe); return ret; } @@ -2081,7 +2111,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2094,7 +2124,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2107,7 +2137,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2267,6 +2297,7 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; int ret = 0; + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -2292,6 +2323,7 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE; } + snd_soc_dpcm_unlock(fe); if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret); @@ -2565,8 +2597,10 @@ static void dpcm_fe_dai_cleanup(struct snd_pcm_substream *fe_substream) int stream = fe_substream->stream; /* mark FE's links ready to prune */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; + snd_soc_dpcm_unlock(fe); dpcm_be_disconnect(fe, stream); @@ -2853,6 +2887,7 @@ struct snd_pcm_substream * } EXPORT_SYMBOL_GPL(snd_soc_dpcm_get_substream); +/* This must be called with snd_soc_dpcm_lock() held. */ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream, @@ -2864,7 +2899,6 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, int ret = 1; int i; - snd_soc_dpcm_lock(fe); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -2878,7 +2912,6 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, } } } - snd_soc_dpcm_unlock(fe); /* it's safe to do this BE DAI */ return ret; @@ -2887,6 +2920,7 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, /* * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE * are not running, paused or suspended for the specified stream direction. + * This must be called with snd_soc_dpcm_lock() held. */ static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) @@ -2900,6 +2934,18 @@ static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); } +static int snd_soc_dpcm_can_be_free_stop_unlocked(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream) +{ + int ret; + + snd_soc_dpcm_lock(fe); + ret = snd_soc_dpcm_can_be_free_stop(fe, be, stream); + snd_soc_dpcm_unlock(fe); + + return ret; +} + /* * We can only change hw params a BE DAI if any of it's FE are not prepared, * running, paused or suspended for the specified stream direction. From 4ef2c8ed24ba33fb962e0ba554874c42e130f0b2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 15:54:43 -0500 Subject: [PATCH 43/49] ASoC: soc-compress: protect for_each_dpcm_be() loops Follow the locking model used within soc-pcm.c Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-compress.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 8e2494a9f3a7f0..632e7e4c85be73 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -158,8 +158,10 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) ret = dpcm_be_dai_startup(fe, stream); if (ret < 0) { /* clean up all links */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; + snd_soc_dpcm_unlock(fe); dpcm_be_disconnect(fe, stream); fe->dpcm[stream].runtime = NULL; @@ -224,8 +226,10 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream) dpcm_be_dai_shutdown(fe, stream); /* mark FE's links ready to prune */ + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; + snd_soc_dpcm_unlock(fe); dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP); From baa9f3eac3609a98054fb074c87c1618158e378b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 15:57:55 -0500 Subject: [PATCH 44/49] ASoC: sh: rcar: protect for_each_dpcm_be() loops Follow the locking model used within soc-pcm.c Signed-off-by: Pierre-Louis Bossart --- sound/soc/sh/rcar/core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 978bd0406729ae..8f180f00ce40ab 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -1511,6 +1511,7 @@ static int rsnd_hw_params(struct snd_soc_component *component, struct snd_soc_dpcm *dpcm; int stream = substream->stream; + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_pcm_hw_params *be_params = &dpcm->hw_params; @@ -1519,6 +1520,7 @@ static int rsnd_hw_params(struct snd_soc_component *component, if (params_rate(hw_params) != params_rate(be_params)) io->converted_rate = params_rate(be_params); } + snd_soc_dpcm_unlock(fe); if (io->converted_chan) dev_dbg(dev, "convert channels = %d\n", io->converted_chan); if (io->converted_rate) { From 4c03c19f2739f844c11a0d7f36a50691a25da7a5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 16:00:24 -0500 Subject: [PATCH 45/49] ASoC: fsl: asrc_dma: protect for_each_dpcm_be() loops Follow the locking model used within soc-pcm.c Signed-off-by: Pierre-Louis Bossart --- sound/soc/fsl/fsl_asrc_dma.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c index cd9b36ec0ecb92..2997d22a231bb6 100644 --- a/sound/soc/fsl/fsl_asrc_dma.c +++ b/sound/soc/fsl/fsl_asrc_dma.c @@ -151,6 +151,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component, int ret, width; /* Fetch the Back-End dma_data from DPCM */ + snd_soc_dpcm_lock(rtd); for_each_dpcm_be(rtd, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *substream_be; @@ -164,6 +165,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component, dev_be = dai->dev; break; } + snd_soc_dpcm_unlock(rtd); if (!dma_params_be) { dev_err(dev, "failed to get the substream of Back-End\n"); From 01ffb83f83c8c081d93f327f7371412497f7fcc9 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 15:35:23 -0500 Subject: [PATCH 46/49] ASoC: soc-pcm: extend snd_soc_dpcm_lock() for non-atomic cases Follow the PCM stream example and use either a spin-lock or a mutex. This assumes that all FEs in a card have the same 'atomicity'. Signed-off-by: Pierre-Louis Bossart --- include/sound/soc.h | 3 ++- sound/soc/soc-core.c | 1 + sound/soc/soc-pcm.c | 10 ++++++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index 8e6dd8a257c567..5a804af248b49c 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -893,7 +893,8 @@ struct snd_soc_card { struct mutex pcm_mutex; enum snd_soc_pcm_subclass pcm_subclass; - spinlock_t dpcm_lock; + struct mutex dpcm_mutex; /* protects DPCM operations with non-atomic FEs */ + spinlock_t dpcm_lock; /* protects DPCM operations with atomic FEs */ int (*probe)(struct snd_soc_card *card); int (*late_probe)(struct snd_soc_card *card); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c830e96afba244..de4c0a3f14bd4e 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2339,6 +2339,7 @@ int snd_soc_register_card(struct snd_soc_card *card) mutex_init(&card->mutex); mutex_init(&card->dapm_mutex); mutex_init(&card->pcm_mutex); + mutex_init(&card->dpcm_mutex); spin_lock_init(&card->dpcm_lock); return snd_soc_bind_card(card); diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d641839b7435a7..3ce6c47ab2d0b7 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -31,13 +31,19 @@ void snd_soc_dpcm_lock(struct snd_soc_pcm_runtime *fe) { - spin_lock_irq(&fe->card->dpcm_lock); + if (fe->dai_link->nonatomic) + mutex_lock(&fe->card->dpcm_mutex); + else + spin_lock_irq(&fe->card->dpcm_lock); } EXPORT_SYMBOL_GPL(snd_soc_dpcm_lock); void snd_soc_dpcm_unlock(struct snd_soc_pcm_runtime *fe) { - spin_unlock_irq(&fe->card->dpcm_lock); + if (fe->dai_link->nonatomic) + mutex_unlock(&fe->card->dpcm_mutex); + else + spin_unlock_irq(&fe->card->dpcm_lock); } EXPORT_SYMBOL_GPL(snd_soc_dpcm_unlock); From ddbfd9d9cb1bbb3cfa05846dfad5aa48411b81b5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 7 Oct 2021 15:41:44 -0500 Subject: [PATCH 47/49] ASoC: soc-pcm: serialize BE triggers 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. This can be fixed by using the snd_soc_dpcm_lock/unlock() helpers which internally rely on a spinlock or mutex, depending on the dailink nonatomic nature. This locking solves another problem discussed on the alsa-mailing list [1], where BEs can be disconnected dynamically, leading to potential accesses to freed memory. [1] https://lore.kernel.org/alsa-devel/002f01d7b4f5$c030f4a0$4092dde0$@samsung.com/ Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 3ce6c47ab2d0b7..7e51905454e627 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -51,9 +51,6 @@ EXPORT_SYMBOL_GPL(snd_soc_dpcm_unlock); static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); -static int snd_soc_dpcm_can_be_free_stop_unlocked(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); - /* can this BE perform a hw_params() */ static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); @@ -2066,6 +2063,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dpcm *dpcm; int ret = 0; + snd_soc_dpcm_lock(fe); for_each_dpcm_be(fe, stream, dpcm) { struct snd_pcm_substream *be_substream; @@ -2117,7 +2115,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) continue; - if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2130,7 +2128,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2143,7 +2141,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop_unlocked(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) continue; ret = soc_pcm_trigger(be_substream, cmd); @@ -2155,6 +2153,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, } } end: + snd_soc_dpcm_unlock(fe); + if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n", __func__, be->dai_link->name, ret); @@ -2940,18 +2940,6 @@ static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); } -static int snd_soc_dpcm_can_be_free_stop_unlocked(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream) -{ - int ret; - - snd_soc_dpcm_lock(fe); - ret = snd_soc_dpcm_can_be_free_stop(fe, be, stream); - snd_soc_dpcm_unlock(fe); - - return ret; -} - /* * We can only change hw params a BE DAI if any of it's FE are not prepared, * running, paused or suspended for the specified stream direction. From 8a4bd381a638582a0a4bf7c75728b083cc2e0bc8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 16 Aug 2021 16:40:18 -0500 Subject: [PATCH 48/49] 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 mutex. Signed-off-by: Pierre-Louis Bossart --- include/sound/soc-dpcm.h | 2 ++ sound/soc/soc-pcm.c | 53 +++++++++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 3202f3230c4e00..89608d43661f22 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 card's dpcm_mutex */ }; #define for_each_dpcm_fe(be, stream, _dpcm) \ diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 7e51905454e627..a6e744e57cc85d 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1584,7 +1584,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++; } @@ -2079,14 +2079,21 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, switch (cmd) { case SNDRV_PCM_TRIGGER_START: - 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)) continue; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start != 1) + continue; + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + be->dpcm[stream].be_start--; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; @@ -2094,9 +2101,15 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND)) continue; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start != 1) + continue; + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + be->dpcm[stream].be_start--; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; @@ -2104,9 +2117,15 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) continue; + be->dpcm[stream].be_start++; + if (be->dpcm[stream].be_start != 1) + continue; + ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + be->dpcm[stream].be_start--; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; break; @@ -2115,12 +2134,18 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_START) + be->dpcm[stream].be_start--; + + if (be->dpcm[stream].be_start != 0) continue; ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_START) + be->dpcm[stream].be_start++; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP; break; @@ -2128,12 +2153,15 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + be->dpcm[stream].be_start--; + if (be->dpcm[stream].be_start != 0) continue; ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + be->dpcm[stream].be_start++; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_SUSPEND; break; @@ -2141,12 +2169,15 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) continue; - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + be->dpcm[stream].be_start--; + if (be->dpcm[stream].be_start != 0) continue; ret = soc_pcm_trigger(be_substream, cmd); - if (ret) + if (ret) { + be->dpcm[stream].be_start++; goto end; + } be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED; break; From 0ac4b26f02a3490773d225c58c79798d51b51302 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 24 Aug 2021 10:21:55 -0500 Subject: [PATCH 49/49] 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 (see note [1] below) 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. [1] the existing logic does not move the BE state back to PAUSED when the FE2 is stopped. This patch does not change the logic; it would be painful to keep a history of changes on the FE side, the state machine is already rather complicated with transitions based on the last BE state and the trigger type. 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 a6e744e57cc85d..dff512119b9b4e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2114,7 +2114,9 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, 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)) + 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)) continue; be->dpcm[stream].be_start++;