* [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches
@ 2020-03-09 17:07 Guennadi Liakhovetski
2020-03-09 17:07 ` [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() Guennadi Liakhovetski
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Guennadi Liakhovetski @ 2020-03-09 17:07 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware
This is the first set of patches for the SOF virtualisation work. We
send these patches first because they touch the ASoC core. 2 of them
are mostly cosmetic with no functional changes, but patch 2/3 might
cause some discussions. Please review and comment.
Thanks
Guennadi
Guennadi Liakhovetski (3):
ASoC: (cosmetic) simplify dpcm_prune_paths()
ASoC: add function parameters to enable forced path pruning
ASoC: export DPCM runtime update functions
include/sound/soc-dpcm.h | 30 ++++++++---
sound/soc/soc-compress.c | 2 +-
sound/soc/soc-dapm.c | 8 +--
sound/soc/soc-pcm.c | 130 +++++++++++++++++++++++++++++------------------
4 files changed, 109 insertions(+), 61 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() 2020-03-09 17:07 [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Guennadi Liakhovetski @ 2020-03-09 17:07 ` Guennadi Liakhovetski 2020-03-09 21:37 ` Pierre-Louis Bossart 2020-03-09 17:07 ` [PATCH 2/3] ASoC: add function parameters to enable forced path pruning Guennadi Liakhovetski ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-09 17:07 UTC (permalink / raw) To: alsa-devel; +Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware Currently dpcm_prune_paths() has up to 4 nested condition and loop levels, which forces the code to use flags for flow control. Extracting widget status verification code from dpcm_prune_paths() into a separate function simplifies the code. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> --- sound/soc/soc-pcm.c | 63 +++++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e7915ada..8732cf1 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1626,45 +1626,46 @@ void dpcm_path_put(struct snd_soc_dapm_widget_list **list) snd_soc_dapm_dai_free_widgets(list); } -static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, - struct snd_soc_dapm_widget_list **list_) +static bool dpcm_be_is_active(struct snd_soc_dpcm *dpcm, int stream, + struct snd_soc_dapm_widget_list *list) { - struct snd_soc_dpcm *dpcm; - struct snd_soc_dapm_widget_list *list = *list_; struct snd_soc_dapm_widget *widget; struct snd_soc_dai *dai; - int prune = 0; - int do_prune; + unsigned int i; - /* Destroy any old FE <--> BE connections */ - for_each_dpcm_be(fe, stream, dpcm) { - unsigned int i; + /* is there a valid CPU DAI widget for this BE */ + for_each_rtd_cpu_dai(dpcm->be, i, dai) { + widget = snd_soc_dai_get_widget(dai, stream); - /* is there a valid CPU DAI widget for this BE */ - do_prune = 1; - for_each_rtd_cpu_dai(dpcm->be, i, dai) { - widget = snd_soc_dai_get_widget(dai, stream); + /* + * The BE is pruned only if none of the cpu_dai + * widgets are in the active list. + */ + if (widget && widget_in_list(list, widget)) + return true; + } - /* - * The BE is pruned only if none of the cpu_dai - * widgets are in the active list. - */ - if (widget && widget_in_list(list, widget)) - do_prune = 0; - } - if (!do_prune) - continue; + /* is there a valid CODEC DAI widget for this BE */ + for_each_rtd_codec_dai(dpcm->be, i, dai) { + widget = snd_soc_dai_get_widget(dai, stream); - /* is there a valid CODEC DAI widget for this BE */ - do_prune = 1; - for_each_rtd_codec_dai(dpcm->be, i, dai) { - widget = snd_soc_dai_get_widget(dai, stream); + /* prune the BE if it's no longer in our active list */ + if (widget && widget_in_list(list, widget)) + return true; + } - /* prune the BE if it's no longer in our active list */ - if (widget && widget_in_list(list, widget)) - do_prune = 0; - } - if (!do_prune) + return false; +} + +static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, + struct snd_soc_dapm_widget_list **list_) +{ + struct snd_soc_dpcm *dpcm; + int prune = 0; + + /* Destroy any old FE <--> BE connections */ + for_each_dpcm_be(fe, stream, dpcm) { + if (dpcm_be_is_active(dpcm, stream, *list_)) continue; dev_dbg(fe->dev, "ASoC: pruning %s BE %s for %s\n", -- 1.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() 2020-03-09 17:07 ` [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() Guennadi Liakhovetski @ 2020-03-09 21:37 ` Pierre-Louis Bossart 0 siblings, 0 replies; 20+ messages in thread From: Pierre-Louis Bossart @ 2020-03-09 21:37 UTC (permalink / raw) To: Guennadi Liakhovetski, alsa-devel Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware On 3/9/20 12:07 PM, Guennadi Liakhovetski wrote: > Currently dpcm_prune_paths() has up to 4 nested condition and loop > levels, which forces the code to use flags for flow control. > Extracting widget status verification code from dpcm_prune_paths() > into a separate function simplifies the code. > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> The diff format makes it hard to review, but with --patience it looks fine indeed and a nice code refactoring. Minor comment below: > + /* is there a valid CPU DAI widget for this BE */ > + for_each_rtd_cpu_dai(dpcm->be, i, dai) { this will conflict with Morimoto-san's cleanup step6 shared today. > + /* is there a valid CODEC DAI widget for this BE */ > + for_each_rtd_codec_dai(dpcm->be, i, dai) { and this as well, will be _dais, not _dai. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-09 17:07 [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Guennadi Liakhovetski 2020-03-09 17:07 ` [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() Guennadi Liakhovetski @ 2020-03-09 17:07 ` Guennadi Liakhovetski 2020-03-09 21:48 ` Pierre-Louis Bossart 2020-03-10 12:45 ` Mark Brown 2020-03-09 17:07 ` [PATCH 3/3] ASoC: export DPCM runtime update functions Guennadi Liakhovetski 2020-03-09 22:05 ` [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Pierre-Louis Bossart 3 siblings, 2 replies; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-09 17:07 UTC (permalink / raw) To: alsa-devel; +Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware This is a preparation for the host part of a virtualised VirtIO audio host-guest driver pair. It adds a "mode" parameter to soc_dpcm_runtime_update() to allow it to be used when stopping streaming in a virtual machine, which requires forced DPCM audio path pruning. For audio virtualisation the host side driver will be using the vhost API, i.e. it will run completely in the kernel. When a guest begins to stream audio, the vhost calls snd_soc_runtime_activate() and soc_dpcm_runtime_update() to activate an audio path and update audio routing. When streaming is stopped, the vhost driver calls soc_dpcm_runtime_update() and snd_soc_runtime_deactivate(). The latter doesn't work at the moment, because the DPCM doesn't recognise the path as inactive. We address this by adding a "mode" parameter to soc_dpcm_runtime_update(). If virtualisation isn't used, the current behaviour isn't affected. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> --- include/sound/soc-dpcm.h | 28 ++++++++++++++++---- sound/soc/soc-compress.c | 2 +- sound/soc/soc-dapm.c | 8 +++--- sound/soc/soc-pcm.c | 68 +++++++++++++++++++++++++++++++++--------------- 4 files changed, 75 insertions(+), 31 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 4022357..aa56118 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -61,6 +61,23 @@ enum snd_soc_dpcm_trigger { SND_SOC_DPCM_TRIGGER_BESPOKE, }; +/** + * enum snd_soc_dpcm_update_mode - mode for calling soc_dpcm_runtime_update() + * + * @SND_SOC_DPCM_UPDATE_FULL: default mode, used for mux, mixer, and + * volume widgets + * @SND_SOC_DPCM_UPDATE_NEW_ONLY: a pipeline is starting. Skip checking + * for old paths. + * @SND_SOC_DPCM_UPDATE_OLD_ONLY: a pipeline is shutting down. Skip + * checking for new paths, force old path + * pruning. + */ +enum snd_soc_dpcm_update_mode { + SND_SOC_DPCM_UPDATE_FULL, + SND_SOC_DPCM_UPDATE_NEW_ONLY, + SND_SOC_DPCM_UPDATE_OLD_ONLY, +}; + /* * Dynamic PCM link * This links together a FE and BE DAI at runtime and stores the link @@ -133,7 +150,8 @@ struct snd_pcm_substream * snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream); /* internal use only */ -int soc_dpcm_runtime_update(struct snd_soc_card *); +int soc_dpcm_runtime_update(struct snd_soc_card *card, + enum snd_soc_dpcm_update_mode mode); #ifdef CONFIG_DEBUG_FS void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd); @@ -143,11 +161,11 @@ static inline void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd) } #endif -int dpcm_path_get(struct snd_soc_pcm_runtime *fe, - int stream, struct snd_soc_dapm_widget_list **list_); +int dpcm_path_get(struct snd_soc_pcm_runtime *fe, int stream, + struct snd_soc_dapm_widget_list **list_); void dpcm_path_put(struct snd_soc_dapm_widget_list **list); -int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, - int stream, struct snd_soc_dapm_widget_list **list, int new); +int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, int stream, + struct snd_soc_dapm_widget_list **list, bool new, bool force_prune); int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream); int dpcm_be_dai_shutdown(struct snd_soc_pcm_runtime *fe, int stream); void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream); diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 50062eb..66d7e78 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -162,7 +162,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) dev_dbg(fe->dev, "Compress ASoC: %s no valid %s route\n", fe->dai_link->name, stream ? "capture" : "playback"); /* calculate valid and active FE <-> BE dpcms */ - dpcm_process_paths(fe, stream, &list, 1); + dpcm_process_paths(fe, stream, &list, true, false); fe->dpcm[stream].runtime = fe_substream->runtime; fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 09fa437..f496c53 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2292,7 +2292,7 @@ int snd_soc_dapm_mux_update_power(struct snd_soc_dapm_context *dapm, card->update = NULL; mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card); + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_mux_update_power); @@ -2357,7 +2357,7 @@ int snd_soc_dapm_mixer_update_power(struct snd_soc_dapm_context *dapm, card->update = NULL; mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card); + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_mixer_update_power); @@ -3397,7 +3397,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card); + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return change; } @@ -3502,7 +3502,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card); + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return change; } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8732cf1..5b3bc3f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1658,14 +1658,14 @@ static bool dpcm_be_is_active(struct snd_soc_dpcm *dpcm, int stream, } static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, - struct snd_soc_dapm_widget_list **list_) + struct snd_soc_dapm_widget_list **list_, bool force) { struct snd_soc_dpcm *dpcm; int prune = 0; /* Destroy any old FE <--> BE connections */ for_each_dpcm_be(fe, stream, dpcm) { - if (dpcm_be_is_active(dpcm, stream, *list_)) + if (!force && dpcm_be_is_active(dpcm, stream, *list_)) continue; dev_dbg(fe->dev, "ASoC: pruning %s BE %s for %s\n", @@ -1740,12 +1740,13 @@ static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream, * FE substream. */ int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, - int stream, struct snd_soc_dapm_widget_list **list, int new) + int stream, struct snd_soc_dapm_widget_list **list, + bool new, bool force_prune) { if (new) return dpcm_add_paths(fe, stream, list); else - return dpcm_prune_paths(fe, stream, list); + return dpcm_prune_paths(fe, stream, list, force_prune); } void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) @@ -2770,11 +2771,13 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) return ret; } -static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream) +static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream, + bool force) { struct snd_pcm_substream *substream = snd_soc_dpcm_get_substream(fe, stream); enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream]; + int event = force ? SND_SOC_DAPM_STREAM_STOP : SND_SOC_DAPM_STREAM_NOP; int err; dev_dbg(fe->dev, "ASoC: runtime %s close on FE %s\n", @@ -2806,7 +2809,7 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream) dev_err(fe->dev,"ASoC: shutdown FE failed %d\n", err); /* run the stream event for each BE */ - dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_NOP); + dpcm_dapm_stream_event(fe, stream, event); return 0; } @@ -2899,7 +2902,8 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) return ret; } -static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) +static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, bool new, + bool force_prune) { struct snd_soc_dapm_widget_list *list; int stream; @@ -2945,13 +2949,14 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) } /* update any playback/capture paths */ - count = dpcm_process_paths(fe, stream, &list, new); + count = dpcm_process_paths(fe, stream, &list, new, force_prune); if (count) { dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_BE); if (new) ret = dpcm_run_update_startup(fe, stream); else - ret = dpcm_run_update_shutdown(fe, stream); + ret = dpcm_run_update_shutdown(fe, stream, + force_prune); if (ret < 0) dev_err(fe->dev, "ASoC: failed to shutdown some BEs\n"); dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO); @@ -2969,25 +2974,46 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) /* Called by DAPM mixer/mux changes to update audio routing between PCMs and * any DAI links. */ -int soc_dpcm_runtime_update(struct snd_soc_card *card) +int soc_dpcm_runtime_update(struct snd_soc_card *card, + enum snd_soc_dpcm_update_mode mode) { struct snd_soc_pcm_runtime *fe; int ret = 0; mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_RUNTIME); + /* shutdown all old paths first */ - for_each_card_rtds(card, fe) { - ret = soc_dpcm_fe_runtime_update(fe, 0); - if (ret) - goto out; - } + if (mode != SND_SOC_DPCM_UPDATE_NEW_ONLY) + /* + * This is entered if mode == FULL or OLD_ONLY. In both cases we + * have to call soc_dpcm_fe_runtime_update() but only in the + * OLD_ONLY case we have to set the "force" (last) parameter to + * "true." + */ + for_each_card_rtds(card, fe) { + /* + * check "old" paths (new = false), only force for + * shutting down. + */ + ret = soc_dpcm_fe_runtime_update(fe, false, + mode == SND_SOC_DPCM_UPDATE_OLD_ONLY); + if (ret) + goto out; + } /* bring new paths up */ - for_each_card_rtds(card, fe) { - ret = soc_dpcm_fe_runtime_update(fe, 1); - if (ret) - goto out; - } + if (mode != SND_SOC_DPCM_UPDATE_OLD_ONLY) + /* + * This is entered if mode == FULL or NEW_ONLY. In both cases we + * have to call soc_dpcm_fe_runtime_update() with the "force" + * (last) parameter set to "false" + */ + for_each_card_rtds(card, fe) { + /* check "new" paths (new = true), no forcing */ + ret = soc_dpcm_fe_runtime_update(fe, true, false); + if (ret) + goto out; + } out: mutex_unlock(&card->mutex); @@ -3042,7 +3068,7 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) } /* calculate valid and active FE <-> BE dpcms */ - dpcm_process_paths(fe, stream, &list, 1); + dpcm_process_paths(fe, stream, &list, true, false); ret = dpcm_fe_dai_startup(fe_substream); if (ret < 0) -- 1.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-09 17:07 ` [PATCH 2/3] ASoC: add function parameters to enable forced path pruning Guennadi Liakhovetski @ 2020-03-09 21:48 ` Pierre-Louis Bossart 2020-03-10 12:45 ` Mark Brown 1 sibling, 0 replies; 20+ messages in thread From: Pierre-Louis Bossart @ 2020-03-09 21:48 UTC (permalink / raw) To: Guennadi Liakhovetski, alsa-devel Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware On 3/9/20 12:07 PM, Guennadi Liakhovetski wrote: > This is a preparation for the host part of a virtualised VirtIO audio > host-guest driver pair. It adds a "mode" parameter to > soc_dpcm_runtime_update() to allow it to be used when stopping > streaming in a virtual machine, which requires forced DPCM audio path > pruning. > > For audio virtualisation the host side driver will be using the vhost > API, i.e. it will run completely in the kernel. When a guest begins to > stream audio, the vhost calls snd_soc_runtime_activate() and > soc_dpcm_runtime_update() to activate an audio path and update audio > routing. When streaming is stopped, the vhost driver calls > soc_dpcm_runtime_update() and snd_soc_runtime_deactivate(). The latter > doesn't work at the moment, because the DPCM doesn't recognise the > path as inactive. We address this by adding a "mode" parameter to > soc_dpcm_runtime_update(). If virtualisation isn't used, the current > behaviour isn't affected. > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Late feedback I know, but would it make sense to split this patch in two where a) first dpcm_process_paths() take a new force_prune parameter, set to false always b) the prune parameter is modified depending on the update_mode Besides making it simpler to review it would also allow to check for non-regressions/bisect. > --- > include/sound/soc-dpcm.h | 28 ++++++++++++++++---- > sound/soc/soc-compress.c | 2 +- > sound/soc/soc-dapm.c | 8 +++--- > sound/soc/soc-pcm.c | 68 +++++++++++++++++++++++++++++++++--------------- > 4 files changed, 75 insertions(+), 31 deletions(-) > > diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h > index 4022357..aa56118 100644 > --- a/include/sound/soc-dpcm.h > +++ b/include/sound/soc-dpcm.h > @@ -61,6 +61,23 @@ enum snd_soc_dpcm_trigger { > SND_SOC_DPCM_TRIGGER_BESPOKE, > }; > > +/** > + * enum snd_soc_dpcm_update_mode - mode for calling soc_dpcm_runtime_update() > + * > + * @SND_SOC_DPCM_UPDATE_FULL: default mode, used for mux, mixer, and > + * volume widgets > + * @SND_SOC_DPCM_UPDATE_NEW_ONLY: a pipeline is starting. Skip checking > + * for old paths. > + * @SND_SOC_DPCM_UPDATE_OLD_ONLY: a pipeline is shutting down. Skip > + * checking for new paths, force old path > + * pruning. > + */ > +enum snd_soc_dpcm_update_mode { > + SND_SOC_DPCM_UPDATE_FULL, > + SND_SOC_DPCM_UPDATE_NEW_ONLY, > + SND_SOC_DPCM_UPDATE_OLD_ONLY, > +}; > + > /* > * Dynamic PCM link > * This links together a FE and BE DAI at runtime and stores the link > @@ -133,7 +150,8 @@ struct snd_pcm_substream * > snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream); > > /* internal use only */ > -int soc_dpcm_runtime_update(struct snd_soc_card *); > +int soc_dpcm_runtime_update(struct snd_soc_card *card, > + enum snd_soc_dpcm_update_mode mode); > > #ifdef CONFIG_DEBUG_FS > void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd); > @@ -143,11 +161,11 @@ static inline void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd) > } > #endif > > -int dpcm_path_get(struct snd_soc_pcm_runtime *fe, > - int stream, struct snd_soc_dapm_widget_list **list_); > +int dpcm_path_get(struct snd_soc_pcm_runtime *fe, int stream, > + struct snd_soc_dapm_widget_list **list_); > void dpcm_path_put(struct snd_soc_dapm_widget_list **list); > -int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, > - int stream, struct snd_soc_dapm_widget_list **list, int new); > +int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, int stream, > + struct snd_soc_dapm_widget_list **list, bool new, bool force_prune); > int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream); > int dpcm_be_dai_shutdown(struct snd_soc_pcm_runtime *fe, int stream); > void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream); > diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c > index 50062eb..66d7e78 100644 > --- a/sound/soc/soc-compress.c > +++ b/sound/soc/soc-compress.c > @@ -162,7 +162,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) > dev_dbg(fe->dev, "Compress ASoC: %s no valid %s route\n", > fe->dai_link->name, stream ? "capture" : "playback"); > /* calculate valid and active FE <-> BE dpcms */ > - dpcm_process_paths(fe, stream, &list, 1); > + dpcm_process_paths(fe, stream, &list, true, false); > fe->dpcm[stream].runtime = fe_substream->runtime; > > fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c > index 09fa437..f496c53 100644 > --- a/sound/soc/soc-dapm.c > +++ b/sound/soc/soc-dapm.c > @@ -2292,7 +2292,7 @@ int snd_soc_dapm_mux_update_power(struct snd_soc_dapm_context *dapm, > card->update = NULL; > mutex_unlock(&card->dapm_mutex); > if (ret > 0) > - soc_dpcm_runtime_update(card); > + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > return ret; > } > EXPORT_SYMBOL_GPL(snd_soc_dapm_mux_update_power); > @@ -2357,7 +2357,7 @@ int snd_soc_dapm_mixer_update_power(struct snd_soc_dapm_context *dapm, > card->update = NULL; > mutex_unlock(&card->dapm_mutex); > if (ret > 0) > - soc_dpcm_runtime_update(card); > + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > return ret; > } > EXPORT_SYMBOL_GPL(snd_soc_dapm_mixer_update_power); > @@ -3397,7 +3397,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, > mutex_unlock(&card->dapm_mutex); > > if (ret > 0) > - soc_dpcm_runtime_update(card); > + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > > return change; > } > @@ -3502,7 +3502,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, > mutex_unlock(&card->dapm_mutex); > > if (ret > 0) > - soc_dpcm_runtime_update(card); > + soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > > return change; > } > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > index 8732cf1..5b3bc3f 100644 > --- a/sound/soc/soc-pcm.c > +++ b/sound/soc/soc-pcm.c > @@ -1658,14 +1658,14 @@ static bool dpcm_be_is_active(struct snd_soc_dpcm *dpcm, int stream, > } > > static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, > - struct snd_soc_dapm_widget_list **list_) > + struct snd_soc_dapm_widget_list **list_, bool force) > { > struct snd_soc_dpcm *dpcm; > int prune = 0; > > /* Destroy any old FE <--> BE connections */ > for_each_dpcm_be(fe, stream, dpcm) { > - if (dpcm_be_is_active(dpcm, stream, *list_)) > + if (!force && dpcm_be_is_active(dpcm, stream, *list_)) > continue; > > dev_dbg(fe->dev, "ASoC: pruning %s BE %s for %s\n", > @@ -1740,12 +1740,13 @@ static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream, > * FE substream. > */ > int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, > - int stream, struct snd_soc_dapm_widget_list **list, int new) > + int stream, struct snd_soc_dapm_widget_list **list, > + bool new, bool force_prune) > { > if (new) > return dpcm_add_paths(fe, stream, list); > else > - return dpcm_prune_paths(fe, stream, list); > + return dpcm_prune_paths(fe, stream, list, force_prune); > } > > void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) > @@ -2770,11 +2771,13 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) > return ret; > } > > -static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream) > +static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream, > + bool force) > { > struct snd_pcm_substream *substream = > snd_soc_dpcm_get_substream(fe, stream); > enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream]; > + int event = force ? SND_SOC_DAPM_STREAM_STOP : SND_SOC_DAPM_STREAM_NOP; > int err; > > dev_dbg(fe->dev, "ASoC: runtime %s close on FE %s\n", > @@ -2806,7 +2809,7 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream) > dev_err(fe->dev,"ASoC: shutdown FE failed %d\n", err); > > /* run the stream event for each BE */ > - dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_NOP); > + dpcm_dapm_stream_event(fe, stream, event); > > return 0; > } > @@ -2899,7 +2902,8 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) > return ret; > } > > -static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) > +static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, bool new, > + bool force_prune) > { > struct snd_soc_dapm_widget_list *list; > int stream; > @@ -2945,13 +2949,14 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) > } > > /* update any playback/capture paths */ > - count = dpcm_process_paths(fe, stream, &list, new); > + count = dpcm_process_paths(fe, stream, &list, new, force_prune); > if (count) { > dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_BE); > if (new) > ret = dpcm_run_update_startup(fe, stream); > else > - ret = dpcm_run_update_shutdown(fe, stream); > + ret = dpcm_run_update_shutdown(fe, stream, > + force_prune); > if (ret < 0) > dev_err(fe->dev, "ASoC: failed to shutdown some BEs\n"); > dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO); > @@ -2969,25 +2974,46 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) > /* Called by DAPM mixer/mux changes to update audio routing between PCMs and > * any DAI links. > */ > -int soc_dpcm_runtime_update(struct snd_soc_card *card) > +int soc_dpcm_runtime_update(struct snd_soc_card *card, > + enum snd_soc_dpcm_update_mode mode) > { > struct snd_soc_pcm_runtime *fe; > int ret = 0; > > mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_RUNTIME); > + > /* shutdown all old paths first */ > - for_each_card_rtds(card, fe) { > - ret = soc_dpcm_fe_runtime_update(fe, 0); > - if (ret) > - goto out; > - } > + if (mode != SND_SOC_DPCM_UPDATE_NEW_ONLY) > + /* > + * This is entered if mode == FULL or OLD_ONLY. In both cases we > + * have to call soc_dpcm_fe_runtime_update() but only in the > + * OLD_ONLY case we have to set the "force" (last) parameter to > + * "true." > + */ > + for_each_card_rtds(card, fe) { > + /* > + * check "old" paths (new = false), only force for > + * shutting down. > + */ > + ret = soc_dpcm_fe_runtime_update(fe, false, > + mode == SND_SOC_DPCM_UPDATE_OLD_ONLY); > + if (ret) > + goto out; > + } > > /* bring new paths up */ > - for_each_card_rtds(card, fe) { > - ret = soc_dpcm_fe_runtime_update(fe, 1); > - if (ret) > - goto out; > - } > + if (mode != SND_SOC_DPCM_UPDATE_OLD_ONLY) > + /* > + * This is entered if mode == FULL or NEW_ONLY. In both cases we > + * have to call soc_dpcm_fe_runtime_update() with the "force" > + * (last) parameter set to "false" > + */ > + for_each_card_rtds(card, fe) { > + /* check "new" paths (new = true), no forcing */ > + ret = soc_dpcm_fe_runtime_update(fe, true, false); > + if (ret) > + goto out; > + } > > out: > mutex_unlock(&card->mutex); > @@ -3042,7 +3068,7 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) > } > > /* calculate valid and active FE <-> BE dpcms */ > - dpcm_process_paths(fe, stream, &list, 1); > + dpcm_process_paths(fe, stream, &list, true, false); > > ret = dpcm_fe_dai_startup(fe_substream); > if (ret < 0) > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-09 17:07 ` [PATCH 2/3] ASoC: add function parameters to enable forced path pruning Guennadi Liakhovetski 2020-03-09 21:48 ` Pierre-Louis Bossart @ 2020-03-10 12:45 ` Mark Brown 2020-03-11 7:41 ` Guennadi Liakhovetski 1 sibling, 1 reply; 20+ messages in thread From: Mark Brown @ 2020-03-10 12:45 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 640 bytes --] On Mon, Mar 09, 2020 at 06:07:48PM +0100, Guennadi Liakhovetski wrote: > soc_dpcm_runtime_update() to activate an audio path and update audio > routing. When streaming is stopped, the vhost driver calls > soc_dpcm_runtime_update() and snd_soc_runtime_deactivate(). The latter > doesn't work at the moment, because the DPCM doesn't recognise the > path as inactive. We address this by adding a "mode" parameter to > soc_dpcm_runtime_update(). If virtualisation isn't used, the current > behaviour isn't affected. So why doesn't DPCM recognize that the path is inactive and why is it better to do this than fix whatever the issue is there? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-10 12:45 ` Mark Brown @ 2020-03-11 7:41 ` Guennadi Liakhovetski 2020-03-11 12:25 ` Mark Brown 0 siblings, 1 reply; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-11 7:41 UTC (permalink / raw) To: Mark Brown; +Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware Hi Mark, On Tue, Mar 10, 2020 at 12:45:44PM +0000, Mark Brown wrote: > On Mon, Mar 09, 2020 at 06:07:48PM +0100, Guennadi Liakhovetski wrote: > > > soc_dpcm_runtime_update() to activate an audio path and update audio > > routing. When streaming is stopped, the vhost driver calls > > soc_dpcm_runtime_update() and snd_soc_runtime_deactivate(). The latter > > doesn't work at the moment, because the DPCM doesn't recognise the > > path as inactive. We address this by adding a "mode" parameter to > > soc_dpcm_runtime_update(). If virtualisation isn't used, the current > > behaviour isn't affected. > > So why doesn't DPCM recognize that the path is inactive and why is it > better to do this than fix whatever the issue is there? Of course that would be better abd I'd much prefer that. Unfortunately I haven't been able to find a single scenario in which those paths would be exercised. As far as I understand path pruning should take place e.g. when a mixer modifies audio routing and as a result disables a certain pipeline, which is then pruned. If I could reproduce such a scenario I would be able to first check whether it's working, then see exactly how it is working and then see how best to add my use case to it. Since I wasn't able to find such a scenario, my only option was to preserve the current state and add my own path "on top." I'd be happy to try the other path too, I just need a use case, that I can reproduce. Thanks Guennadi ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-11 7:41 ` Guennadi Liakhovetski @ 2020-03-11 12:25 ` Mark Brown 2020-03-11 12:36 ` Guennadi Liakhovetski 0 siblings, 1 reply; 20+ messages in thread From: Mark Brown @ 2020-03-11 12:25 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 1335 bytes --] On Wed, Mar 11, 2020 at 08:41:27AM +0100, Guennadi Liakhovetski wrote: > On Tue, Mar 10, 2020 at 12:45:44PM +0000, Mark Brown wrote: > > So why doesn't DPCM recognize that the path is inactive and why is it > > better to do this than fix whatever the issue is there? > Of course that would be better abd I'd much prefer that. Unfortunately I > haven't been able to find a single scenario in which those paths would be > exercised. As far as I understand path pruning should take place e.g. > when a mixer modifies audio routing and as a result disables a certain > pipeline, which is then pruned. If I could reproduce such a scenario I > would be able to first check whether it's working, then see exactly how > it is working and then see how best to add my use case to it. Since I > wasn't able to find such a scenario, my only option was to preserve > the current state and add my own path "on top." I'd be happy to try the > other path too, I just need a use case, that I can reproduce. It's still not clear to me what the issue is here. If something is making a modification to the graph which needs a recheck or update I'd expect that these things happen along with that modification. I don't understand what you're saying about not being able to reproduce scenarios or adding things "on top". [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-11 12:25 ` Mark Brown @ 2020-03-11 12:36 ` Guennadi Liakhovetski 2020-03-11 12:41 ` Mark Brown 0 siblings, 1 reply; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-11 12:36 UTC (permalink / raw) To: Mark Brown; +Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware Hi Mark, On Wed, Mar 11, 2020 at 12:25:31PM +0000, Mark Brown wrote: > On Wed, Mar 11, 2020 at 08:41:27AM +0100, Guennadi Liakhovetski wrote: > > On Tue, Mar 10, 2020 at 12:45:44PM +0000, Mark Brown wrote: > > > > So why doesn't DPCM recognize that the path is inactive and why is it > > > better to do this than fix whatever the issue is there? > > > Of course that would be better abd I'd much prefer that. Unfortunately I > > haven't been able to find a single scenario in which those paths would be > > exercised. As far as I understand path pruning should take place e.g. > > when a mixer modifies audio routing and as a result disables a certain > > pipeline, which is then pruned. If I could reproduce such a scenario I > > would be able to first check whether it's working, then see exactly how > > it is working and then see how best to add my use case to it. Since I > > wasn't able to find such a scenario, my only option was to preserve > > the current state and add my own path "on top." I'd be happy to try the > > other path too, I just need a use case, that I can reproduce. > > It's still not clear to me what the issue is here. If something is > making a modification to the graph which needs a recheck or update I'd > expect that these things happen along with that modification. I don't > understand what you're saying about not being able to reproduce > scenarios or adding things "on top". I mean, that I don't have a test-case to test dpcm_prune_paths() without my VirtIO support. That function currently exists in the kernel, but I don't have a test-case to verify its work, to see it called and actually perform the pruning. So I don't know how it is supposed to work. And because of that I cannot fix my VirtIO use case to use that function properly, without forcing it with an additional parameter. Thanks Guennadi ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-11 12:36 ` Guennadi Liakhovetski @ 2020-03-11 12:41 ` Mark Brown 2020-03-11 13:26 ` Guennadi Liakhovetski 0 siblings, 1 reply; 20+ messages in thread From: Mark Brown @ 2020-03-11 12:41 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 893 bytes --] On Wed, Mar 11, 2020 at 01:36:17PM +0100, Guennadi Liakhovetski wrote: > On Wed, Mar 11, 2020 at 12:25:31PM +0000, Mark Brown wrote: > > It's still not clear to me what the issue is here. If something is > > making a modification to the graph which needs a recheck or update I'd > > expect that these things happen along with that modification. I don't > > understand what you're saying about not being able to reproduce > > scenarios or adding things "on top". > VirtIO support. That function currently exists in the kernel, but I don't > have a test-case to verify its work, to see it called and actually perform > the pruning. So I don't know how it is supposed to work. And because of > that I cannot fix my VirtIO use case to use that function properly, without > forcing it with an additional parameter. I think you need to include this with your VirtIO series. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ASoC: add function parameters to enable forced path pruning 2020-03-11 12:41 ` Mark Brown @ 2020-03-11 13:26 ` Guennadi Liakhovetski 0 siblings, 0 replies; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-11 13:26 UTC (permalink / raw) To: Mark Brown; +Cc: Liam Girdwood, Takashi Iwai, alsa-devel, sound-open-firmware On Wed, Mar 11, 2020 at 12:41:02PM +0000, Mark Brown wrote: > On Wed, Mar 11, 2020 at 01:36:17PM +0100, Guennadi Liakhovetski wrote: > > On Wed, Mar 11, 2020 at 12:25:31PM +0000, Mark Brown wrote: > > > > It's still not clear to me what the issue is here. If something is > > > making a modification to the graph which needs a recheck or update I'd > > > expect that these things happen along with that modification. I don't > > > understand what you're saying about not being able to reproduce > > > scenarios or adding things "on top". > > > VirtIO support. That function currently exists in the kernel, but I don't > > have a test-case to verify its work, to see it called and actually perform > > the pruning. So I don't know how it is supposed to work. And because of > > that I cannot fix my VirtIO use case to use that function properly, without > > forcing it with an additional parameter. > > I think you need to include this with your VirtIO series. That was also my original intention, personally I also would find it difficult to review this kind of work without context. So, would you prefer me to drop this series completely and re-send it with the whole series or should I just drop #2 and resend #1 and #3? Thanks Guennadi ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] ASoC: export DPCM runtime update functions 2020-03-09 17:07 [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Guennadi Liakhovetski 2020-03-09 17:07 ` [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() Guennadi Liakhovetski 2020-03-09 17:07 ` [PATCH 2/3] ASoC: add function parameters to enable forced path pruning Guennadi Liakhovetski @ 2020-03-09 17:07 ` Guennadi Liakhovetski 2020-03-09 21:54 ` [Sound-open-firmware] " Pierre-Louis Bossart 2020-03-09 22:05 ` [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Pierre-Louis Bossart 3 siblings, 1 reply; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-09 17:07 UTC (permalink / raw) To: alsa-devel; +Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware This makes DPCM runtime update functions available for external calling. As an example, virtualised ASoC component drivers may need to call these when managing shared DAPM routes that are used by more than one driver (i.e. when host driver and guest drivers have a DAPM path from guest PCM to host DAI where some parts are owned by host driver and others by guest driver). Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> --- include/sound/soc-dpcm.h | 6 +++--- sound/soc/soc-dapm.c | 8 ++++---- sound/soc/soc-pcm.c | 7 +++++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index aa56118..b961c06 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -149,9 +149,9 @@ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, struct snd_pcm_substream * snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream); -/* internal use only */ -int soc_dpcm_runtime_update(struct snd_soc_card *card, - enum snd_soc_dpcm_update_mode mode); +/* update audio routing between PCMs and any DAI links */ +int snd_soc_dpcm_runtime_update(struct snd_soc_card *card, + enum snd_soc_dpcm_update_mode mode); #ifdef CONFIG_DEBUG_FS void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd); diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index f496c53..3b91d7c 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2292,7 +2292,7 @@ int snd_soc_dapm_mux_update_power(struct snd_soc_dapm_context *dapm, card->update = NULL; mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_mux_update_power); @@ -2357,7 +2357,7 @@ int snd_soc_dapm_mixer_update_power(struct snd_soc_dapm_context *dapm, card->update = NULL; mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_mixer_update_power); @@ -3397,7 +3397,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return change; } @@ -3502,7 +3502,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, mutex_unlock(&card->dapm_mutex); if (ret > 0) - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); return change; } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 5b3bc3f..d373148 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -295,6 +295,7 @@ void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream) { snd_soc_runtime_action(rtd, stream, 1); } +EXPORT_SYMBOL_GPL(snd_soc_runtime_activate); /** * snd_soc_runtime_deactivate() - Decrement active count for PCM runtime components @@ -310,6 +311,7 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream) { snd_soc_runtime_action(rtd, stream, -1); } +EXPORT_SYMBOL_GPL(snd_soc_runtime_deactivate); /** * snd_soc_runtime_ignore_pmdown_time() - Check whether to ignore the power down delay @@ -2974,8 +2976,8 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, bool new, /* Called by DAPM mixer/mux changes to update audio routing between PCMs and * any DAI links. */ -int soc_dpcm_runtime_update(struct snd_soc_card *card, - enum snd_soc_dpcm_update_mode mode) +int snd_soc_dpcm_runtime_update(struct snd_soc_card *card, + enum snd_soc_dpcm_update_mode mode) { struct snd_soc_pcm_runtime *fe; int ret = 0; @@ -3019,6 +3021,7 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card, mutex_unlock(&card->mutex); return ret; } +EXPORT_SYMBOL_GPL(snd_soc_dpcm_runtime_update); static void dpcm_fe_dai_cleanup(struct snd_pcm_substream *fe_substream) { -- 1.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Sound-open-firmware] [PATCH 3/3] ASoC: export DPCM runtime update functions 2020-03-09 17:07 ` [PATCH 3/3] ASoC: export DPCM runtime update functions Guennadi Liakhovetski @ 2020-03-09 21:54 ` Pierre-Louis Bossart 0 siblings, 0 replies; 20+ messages in thread From: Pierre-Louis Bossart @ 2020-03-09 21:54 UTC (permalink / raw) To: Guennadi Liakhovetski, alsa-devel Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware On 3/9/20 12:07 PM, Guennadi Liakhovetski wrote: > This makes DPCM runtime update functions available for external > calling. As an example, virtualised ASoC component drivers may need > to call these when managing shared DAPM routes that are used by more > than one driver (i.e. when host driver and guest drivers have a DAPM > path from guest PCM to host DAI where some parts are owned by host > driver and others by guest driver). Exporting the functions for uses by vhost drivers looks fine, but your 'as an example' prose is rather confusing. It's not really that the vhost drivers 'may' use these functions but rather that they *shall* do so so that guest-initiated events are propagated into the host-managed graph which handles 'physical' widgets and DAIs. Or did I completely miss the idea? > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> > --- > include/sound/soc-dpcm.h | 6 +++--- > sound/soc/soc-dapm.c | 8 ++++---- > sound/soc/soc-pcm.c | 7 +++++-- > 3 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h > index aa56118..b961c06 100644 > --- a/include/sound/soc-dpcm.h > +++ b/include/sound/soc-dpcm.h > @@ -149,9 +149,9 @@ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, > struct snd_pcm_substream * > snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream); > > -/* internal use only */ > -int soc_dpcm_runtime_update(struct snd_soc_card *card, > - enum snd_soc_dpcm_update_mode mode); > +/* update audio routing between PCMs and any DAI links */ > +int snd_soc_dpcm_runtime_update(struct snd_soc_card *card, > + enum snd_soc_dpcm_update_mode mode); > > #ifdef CONFIG_DEBUG_FS > void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd); > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c > index f496c53..3b91d7c 100644 > --- a/sound/soc/soc-dapm.c > +++ b/sound/soc/soc-dapm.c > @@ -2292,7 +2292,7 @@ int snd_soc_dapm_mux_update_power(struct snd_soc_dapm_context *dapm, > card->update = NULL; > mutex_unlock(&card->dapm_mutex); > if (ret > 0) > - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > return ret; > } > EXPORT_SYMBOL_GPL(snd_soc_dapm_mux_update_power); > @@ -2357,7 +2357,7 @@ int snd_soc_dapm_mixer_update_power(struct snd_soc_dapm_context *dapm, > card->update = NULL; > mutex_unlock(&card->dapm_mutex); > if (ret > 0) > - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > return ret; > } > EXPORT_SYMBOL_GPL(snd_soc_dapm_mixer_update_power); > @@ -3397,7 +3397,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, > mutex_unlock(&card->dapm_mutex); > > if (ret > 0) > - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > > return change; > } > @@ -3502,7 +3502,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, > mutex_unlock(&card->dapm_mutex); > > if (ret > 0) > - soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > + snd_soc_dpcm_runtime_update(card, SND_SOC_DPCM_UPDATE_FULL); > > return change; > } > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > index 5b3bc3f..d373148 100644 > --- a/sound/soc/soc-pcm.c > +++ b/sound/soc/soc-pcm.c > @@ -295,6 +295,7 @@ void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream) > { > snd_soc_runtime_action(rtd, stream, 1); > } > +EXPORT_SYMBOL_GPL(snd_soc_runtime_activate); > > /** > * snd_soc_runtime_deactivate() - Decrement active count for PCM runtime components > @@ -310,6 +311,7 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream) > { > snd_soc_runtime_action(rtd, stream, -1); > } > +EXPORT_SYMBOL_GPL(snd_soc_runtime_deactivate); > > /** > * snd_soc_runtime_ignore_pmdown_time() - Check whether to ignore the power down delay > @@ -2974,8 +2976,8 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, bool new, > /* Called by DAPM mixer/mux changes to update audio routing between PCMs and > * any DAI links. > */ > -int soc_dpcm_runtime_update(struct snd_soc_card *card, > - enum snd_soc_dpcm_update_mode mode) > +int snd_soc_dpcm_runtime_update(struct snd_soc_card *card, > + enum snd_soc_dpcm_update_mode mode) > { > struct snd_soc_pcm_runtime *fe; > int ret = 0; > @@ -3019,6 +3021,7 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card, > mutex_unlock(&card->mutex); > return ret; > } > +EXPORT_SYMBOL_GPL(snd_soc_dpcm_runtime_update); > > static void dpcm_fe_dai_cleanup(struct snd_pcm_substream *fe_substream) > { > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-09 17:07 [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Guennadi Liakhovetski ` (2 preceding siblings ...) 2020-03-09 17:07 ` [PATCH 3/3] ASoC: export DPCM runtime update functions Guennadi Liakhovetski @ 2020-03-09 22:05 ` Pierre-Louis Bossart 2020-03-10 12:50 ` Mark Brown 3 siblings, 1 reply; 20+ messages in thread From: Pierre-Louis Bossart @ 2020-03-09 22:05 UTC (permalink / raw) To: Guennadi Liakhovetski, alsa-devel Cc: Liam Girdwood, Takashi Iwai, Mark Brown, sound-open-firmware On 3/9/20 12:07 PM, Guennadi Liakhovetski wrote: > This is the first set of patches for the SOF virtualisation work. We > send these patches first because they touch the ASoC core. 2 of them > are mostly cosmetic with no functional changes, but patch 2/3 might > cause some discussions. Please review and comment. Sorry about the delay in reviewing. To get a better picture of the directions, reviewers are invited to take a look at the in-depth documentation written by Guennadi since the initial patches were shared. This documentation was reviewed by Liam and me and is really required to understand the concepts: https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html I added a couple of comments on the three patches, this looks good to me. > Thanks > Guennadi > > Guennadi Liakhovetski (3): > ASoC: (cosmetic) simplify dpcm_prune_paths() > ASoC: add function parameters to enable forced path pruning > ASoC: export DPCM runtime update functions > > include/sound/soc-dpcm.h | 30 ++++++++--- > sound/soc/soc-compress.c | 2 +- > sound/soc/soc-dapm.c | 8 +-- > sound/soc/soc-pcm.c | 130 +++++++++++++++++++++++++++++------------------ > 4 files changed, 109 insertions(+), 61 deletions(-) > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-09 22:05 ` [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Pierre-Louis Bossart @ 2020-03-10 12:50 ` Mark Brown 2020-03-10 20:13 ` Pierre-Louis Bossart 2020-03-12 11:45 ` Guennadi Liakhovetski 0 siblings, 2 replies; 20+ messages in thread From: Mark Brown @ 2020-03-10 12:50 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Liam Girdwood, Guennadi Liakhovetski, alsa-devel, Takashi Iwai, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 708 bytes --] On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote: > To get a better picture of the directions, reviewers are invited to take a > look at the in-depth documentation written by Guennadi since the initial > patches were shared. This documentation was reviewed by Liam and me and is > really required to understand the concepts: > https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html How does this relate to the virtio audio spec that's currently under review? It looks to be doing something much lower level than that. I am concerned that this looks to be exposing DPCM as a virtio ABI, we're trying to replace it as an internal API never mind ABI. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-10 12:50 ` Mark Brown @ 2020-03-10 20:13 ` Pierre-Louis Bossart 2020-03-11 12:16 ` Mark Brown 2020-03-12 11:45 ` Guennadi Liakhovetski 1 sibling, 1 reply; 20+ messages in thread From: Pierre-Louis Bossart @ 2020-03-10 20:13 UTC (permalink / raw) To: Mark Brown Cc: Liam Girdwood, Guennadi Liakhovetski, alsa-devel, Takashi Iwai, sound-open-firmware On 3/10/20 7:50 AM, Mark Brown wrote: > On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote: > >> To get a better picture of the directions, reviewers are invited to take a >> look at the in-depth documentation written by Guennadi since the initial >> patches were shared. This documentation was reviewed by Liam and me and is >> really required to understand the concepts: > >> https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html > > How does this relate to the virtio audio spec that's currently under > review? It looks to be doing something much lower level than that. Mark, to avoid any ambiguity, what 'virtio audio spec that's currently under review' are you referring to? > I am concerned that this looks to be exposing DPCM as a virtio ABI, > we're trying to replace it as an internal API never mind ABI. Valid point indeed. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-10 20:13 ` Pierre-Louis Bossart @ 2020-03-11 12:16 ` Mark Brown 0 siblings, 0 replies; 20+ messages in thread From: Mark Brown @ 2020-03-11 12:16 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Liam Girdwood, Guennadi Liakhovetski, alsa-devel, Takashi Iwai, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 439 bytes --] On Tue, Mar 10, 2020 at 03:13:12PM -0500, Pierre-Louis Bossart wrote: > On 3/10/20 7:50 AM, Mark Brown wrote: > > How does this relate to the virtio audio spec that's currently under > > review? It looks to be doing something much lower level than that. > Mark, to avoid any ambiguity, what 'virtio audio spec that's currently under > review' are you referring to? https://lists.oasis-open.org/archives/virtio-dev/202003/msg00045.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-10 12:50 ` Mark Brown 2020-03-10 20:13 ` Pierre-Louis Bossart @ 2020-03-12 11:45 ` Guennadi Liakhovetski 2020-03-12 12:15 ` Mark Brown 1 sibling, 1 reply; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-12 11:45 UTC (permalink / raw) To: Mark Brown Cc: Liam Girdwood, Takashi Iwai, alsa-devel, Pierre-Louis Bossart, sound-open-firmware Hi, On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote: > On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote: > > > To get a better picture of the directions, reviewers are invited to take a > > look at the in-depth documentation written by Guennadi since the initial > > patches were shared. This documentation was reviewed by Liam and me and is > > really required to understand the concepts: > > > https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html > > How does this relate to the virtio audio spec that's currently under > review? The spec under discussion is only for simple audio virtualisation with fixed roles and topologies. With our approach guests get access to advanced DSP capabilities. The virtualisation approach under discussion can be a fallback for cases when no DSP has been detected on the host. > It looks to be doing something much lower level than that. > I am concerned that this looks to be exposing DPCM as a virtio ABI, > we're trying to replace it as an internal API never mind ABI. You mean that our approach works at the widget level, which is a part of the DPCM API? Well there is a translation layer between our ABI and DPCM. And by virtue of the same argument don't we already have DPCM as an ABI on the opposite side of SOF - in its IPC ABI? Largely this virtualisation approach doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of its advantages. Thanks Guennadi ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-12 11:45 ` Guennadi Liakhovetski @ 2020-03-12 12:15 ` Mark Brown 2020-03-12 13:09 ` Guennadi Liakhovetski 0 siblings, 1 reply; 20+ messages in thread From: Mark Brown @ 2020-03-12 12:15 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Liam Girdwood, Takashi Iwai, alsa-devel, Pierre-Louis Bossart, sound-open-firmware [-- Attachment #1: Type: text/plain, Size: 1425 bytes --] On Thu, Mar 12, 2020 at 12:45:37PM +0100, Guennadi Liakhovetski wrote: > On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote: > > How does this relate to the virtio audio spec that's currently under > > review? > The spec under discussion is only for simple audio virtualisation with fixed > roles and topologies. With our approach guests get access to advanced DSP > capabilities. The virtualisation approach under discussion can be a fallback > for cases when no DSP has been detected on the host. So they're orthogonal :/ Have you proposed your spec yet? > > It looks to be doing something much lower level than that. > > I am concerned that this looks to be exposing DPCM as a virtio ABI, > > we're trying to replace it as an internal API never mind ABI. > You mean that our approach works at the widget level, which is a part of the > DPCM API? Well there is a translation layer between our ABI and DPCM. And by > virtue of the same argument don't we already have DPCM as an ABI on the > opposite side of SOF - in its IPC ABI? Largely this virtualisation approach > doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of > its advantages. Please bear in mind that the page you linked to is very high level and I've not seen the actual spec or anything. The page and your mails both talk about DPCM so it sounds like that's a part of the interface. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches 2020-03-12 12:15 ` Mark Brown @ 2020-03-12 13:09 ` Guennadi Liakhovetski 0 siblings, 0 replies; 20+ messages in thread From: Guennadi Liakhovetski @ 2020-03-12 13:09 UTC (permalink / raw) To: Mark Brown Cc: Liam Girdwood, Takashi Iwai, alsa-devel, Pierre-Louis Bossart, sound-open-firmware On Thu, Mar 12, 2020 at 12:15:49PM +0000, Mark Brown wrote: > On Thu, Mar 12, 2020 at 12:45:37PM +0100, Guennadi Liakhovetski wrote: > > On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote: > > > > How does this relate to the virtio audio spec that's currently under > > > review? > > > The spec under discussion is only for simple audio virtualisation with fixed > > roles and topologies. With our approach guests get access to advanced DSP > > capabilities. The virtualisation approach under discussion can be a fallback > > for cases when no DSP has been detected on the host. > > So they're orthogonal :/ Have you proposed your spec yet? > > > > It looks to be doing something much lower level than that. > > > I am concerned that this looks to be exposing DPCM as a virtio ABI, > > > we're trying to replace it as an internal API never mind ABI. > > > You mean that our approach works at the widget level, which is a part of the > > DPCM API? Well there is a translation layer between our ABI and DPCM. And by > > virtue of the same argument don't we already have DPCM as an ABI on the > > opposite side of SOF - in its IPC ABI? Largely this virtualisation approach > > doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of > > its advantages. > > Please bear in mind that the page you linked to is very high level and > I've not seen the actual spec or anything. The page and your mails both > talk about DPCM so it sounds like that's a part of the interface. Let me rebase my current stack and post it here, stay tuned :-) Thanks Guennadi ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-03-12 13:10 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-09 17:07 [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Guennadi Liakhovetski 2020-03-09 17:07 ` [PATCH 1/3] ASoC: (cosmetic) simplify dpcm_prune_paths() Guennadi Liakhovetski 2020-03-09 21:37 ` Pierre-Louis Bossart 2020-03-09 17:07 ` [PATCH 2/3] ASoC: add function parameters to enable forced path pruning Guennadi Liakhovetski 2020-03-09 21:48 ` Pierre-Louis Bossart 2020-03-10 12:45 ` Mark Brown 2020-03-11 7:41 ` Guennadi Liakhovetski 2020-03-11 12:25 ` Mark Brown 2020-03-11 12:36 ` Guennadi Liakhovetski 2020-03-11 12:41 ` Mark Brown 2020-03-11 13:26 ` Guennadi Liakhovetski 2020-03-09 17:07 ` [PATCH 3/3] ASoC: export DPCM runtime update functions Guennadi Liakhovetski 2020-03-09 21:54 ` [Sound-open-firmware] " Pierre-Louis Bossart 2020-03-09 22:05 ` [PATCH 0/3] [RESEND] ASoC: SOF: preparatory patches Pierre-Louis Bossart 2020-03-10 12:50 ` Mark Brown 2020-03-10 20:13 ` Pierre-Louis Bossart 2020-03-11 12:16 ` Mark Brown 2020-03-12 11:45 ` Guennadi Liakhovetski 2020-03-12 12:15 ` Mark Brown 2020-03-12 13:09 ` Guennadi Liakhovetski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.