From 3635bf09a89cf92b80ac44198c5c8f0989624ea6 Mon Sep 17 00:00:00 2001 From: Nicolin Chen Date: Wed, 13 Nov 2013 18:56:24 +0800 Subject: [PATCH 1/7] ASoC: soc-pcm: add symmetry for channels and sample bits Some SoCs can only work in mono or stereo mode at one time. So if we let them capture a mono stream while playing a stereo stream, there might be a problem occur to one of these two streams: double paced or slowed down. In soc-pcm.c, we have soc_pcm_apply_symmetry() to apply the rate symmetry. But we don't have one for channels. Likewise, we can treat symmetric_rate as a solution for those SoCs or CODECs which can not handle asymmetrical LRCLK. But it's also impossible for them to handle asymmetrical BCLK. And accodring to BCLK = LRCLK * channel number * slot size(fixed or sample bits), sample bits might also be a problem if they are not using a fixed slot size. Thus, this patch applys symmetry for channels and sample bits. Meanwhile, there might be a race between two substreams if starting simultaneously. Previously, we only added warning to compalin but still using conservative way to let it carry on. However, this patch rejects the second stream with any unmatched parameter to make sure the first existing stream won't be broken. Signed-off-by: Nicolin Chen Signed-off-by: Mark Brown --- include/sound/soc-dai.h | 6 ++ include/sound/soc.h | 2 + sound/soc/soc-pcm.c | 130 +++++++++++++++++++++++++++++++++------- 3 files changed, 115 insertions(+), 23 deletions(-) diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 800c101bb096..243d3b689699 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -220,6 +220,8 @@ struct snd_soc_dai_driver { struct snd_soc_pcm_stream capture; struct snd_soc_pcm_stream playback; unsigned int symmetric_rates:1; + unsigned int symmetric_channels:1; + unsigned int symmetric_samplebits:1; /* probe ordering - for components with runtime dependencies */ int probe_order; @@ -244,6 +246,8 @@ struct snd_soc_dai { unsigned int capture_active:1; /* stream is in use */ unsigned int playback_active:1; /* stream is in use */ unsigned int symmetric_rates:1; + unsigned int symmetric_channels:1; + unsigned int symmetric_samplebits:1; struct snd_pcm_runtime *runtime; unsigned int active; unsigned char probed:1; @@ -258,6 +262,8 @@ struct snd_soc_dai { /* Symmetry data - only valid if symmetry is being enforced */ unsigned int rate; + unsigned int channels; + unsigned int sample_bits; /* parent platform/codec */ struct snd_soc_platform *platform; diff --git a/include/sound/soc.h b/include/sound/soc.h index 1f741cb24f33..1cda7d343d16 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -879,6 +879,8 @@ struct snd_soc_dai_link { /* Symmetry requirements */ unsigned int symmetric_rates:1; + unsigned int symmetric_channels:1; + unsigned int symmetric_samplebits:1; /* Do not create a PCM for this DAI link (Backend link) */ unsigned int no_pcm:1; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 42782c01e413..ed1e077114a2 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -84,30 +84,97 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = substream->private_data; int ret; - if (!soc_dai->driver->symmetric_rates && - !rtd->dai_link->symmetric_rates) - return 0; + if (soc_dai->rate && (soc_dai->driver->symmetric_rates || + rtd->dai_link->symmetric_rates)) { + dev_dbg(soc_dai->dev, "ASoC: Symmetry forces %dHz rate\n", + soc_dai->rate); - /* This can happen if multiple streams are starting simultaneously - - * the second can need to get its constraints before the first has - * picked a rate. Complain and allow the application to carry on. - */ - if (!soc_dai->rate) { - dev_warn(soc_dai->dev, - "ASoC: Not enforcing symmetric_rates due to race\n"); - return 0; + ret = snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_RATE, + soc_dai->rate, soc_dai->rate); + if (ret < 0) { + dev_err(soc_dai->dev, + "ASoC: Unable to apply rate constraint: %d\n", + ret); + return ret; + } } - dev_dbg(soc_dai->dev, "ASoC: Symmetry forces %dHz rate\n", soc_dai->rate); + if (soc_dai->channels && (soc_dai->driver->symmetric_channels || + rtd->dai_link->symmetric_channels)) { + dev_dbg(soc_dai->dev, "ASoC: Symmetry forces %d channel(s)\n", + soc_dai->channels); - ret = snd_pcm_hw_constraint_minmax(substream->runtime, - SNDRV_PCM_HW_PARAM_RATE, - soc_dai->rate, soc_dai->rate); - if (ret < 0) { - dev_err(soc_dai->dev, - "ASoC: Unable to apply rate symmetry constraint: %d\n", - ret); - return ret; + ret = snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_CHANNELS, + soc_dai->channels, + soc_dai->channels); + if (ret < 0) { + dev_err(soc_dai->dev, + "ASoC: Unable to apply channel symmetry constraint: %d\n", + ret); + return ret; + } + } + + if (soc_dai->sample_bits && (soc_dai->driver->symmetric_samplebits || + rtd->dai_link->symmetric_samplebits)) { + dev_dbg(soc_dai->dev, "ASoC: Symmetry forces %d sample bits\n", + soc_dai->sample_bits); + + ret = snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_SAMPLE_BITS, + soc_dai->sample_bits, + soc_dai->sample_bits); + if (ret < 0) { + dev_err(soc_dai->dev, + "ASoC: Unable to apply sample bits symmetry constraint: %d\n", + ret); + return ret; + } + } + + return 0; +} + +static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + unsigned int rate, channels, sample_bits, symmetry; + + rate = params_rate(params); + channels = params_channels(params); + sample_bits = snd_pcm_format_physical_width(params_format(params)); + + /* reject unmatched parameters when applying symmetry */ + symmetry = cpu_dai->driver->symmetric_rates || + codec_dai->driver->symmetric_rates || + rtd->dai_link->symmetric_rates; + if (symmetry && cpu_dai->rate && cpu_dai->rate != rate) { + dev_err(rtd->dev, "ASoC: unmatched rate symmetry: %d - %d\n", + cpu_dai->rate, rate); + return -EINVAL; + } + + symmetry = cpu_dai->driver->symmetric_channels || + codec_dai->driver->symmetric_channels || + rtd->dai_link->symmetric_channels; + if (symmetry && cpu_dai->channels && cpu_dai->channels != channels) { + dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n", + cpu_dai->channels, channels); + return -EINVAL; + } + + symmetry = cpu_dai->driver->symmetric_samplebits || + codec_dai->driver->symmetric_samplebits || + rtd->dai_link->symmetric_samplebits; + if (symmetry && cpu_dai->sample_bits && cpu_dai->sample_bits != sample_bits) { + dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n", + cpu_dai->sample_bits, sample_bits); + return -EINVAL; } return 0; @@ -384,11 +451,17 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) codec->active--; /* clear the corresponding DAIs rate when inactive */ - if (!cpu_dai->active) + if (!cpu_dai->active) { cpu_dai->rate = 0; + cpu_dai->channels = 0; + cpu_dai->sample_bits = 0; + } - if (!codec_dai->active) + if (!codec_dai->active) { codec_dai->rate = 0; + codec_dai->channels = 0; + codec_dai->sample_bits = 0; + } /* Muting the DAC suppresses artifacts caused during digital * shutdown, for example from stopping clocks. @@ -525,6 +598,10 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); + ret = soc_pcm_params_symmetry(substream, params); + if (ret) + goto out; + if (rtd->dai_link->ops && rtd->dai_link->ops->hw_params) { ret = rtd->dai_link->ops->hw_params(substream, params); if (ret < 0) { @@ -561,9 +638,16 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } } - /* store the rate for each DAIs */ + /* store the parameters for each DAIs */ cpu_dai->rate = params_rate(params); + cpu_dai->channels = params_channels(params); + cpu_dai->sample_bits = + snd_pcm_format_physical_width(params_format(params)); + codec_dai->rate = params_rate(params); + codec_dai->channels = params_channels(params); + codec_dai->sample_bits = + snd_pcm_format_physical_width(params_format(params)); out: mutex_unlock(&rtd->pcm_mutex); From d3383420c969c25deffd33270ebe321e8401191a Mon Sep 17 00:00:00 2001 From: Nicolin Chen Date: Wed, 20 Nov 2013 18:37:09 +0800 Subject: [PATCH 2/7] ASoC: soc-pcm: move DAIs parameters cleaning into hw_free() We're now applying soc_hw_params_symmetry() to reject unmatched parameters while we clear parameters in soc_pcm_close(). So here's a use case might be broken by this mechanism: aplay -Dhw:0 44100.wav 48000.wav 32000.wav In this case, we call soc_pcm_open()->soc_pcm_hw_params()->soc_pcm_hw_free() ->soc_pcm_hw_params()->soc_pcm_hw_free()->soc_pcm_close() in order. As we only clear parameters in soc_pcm_close(). The parameters would be remained in the system even if the playback of 44100.wav is finished. Thus, this patch is trying to move parameters cleaning into hw_free() so that the system can continue to serve this kind of use case. Also, since we set them in hw_params(), it should be better to clear them in hw_free() for symmetry. Signed-off-by: Nicolin Chen Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ed1e077114a2..170ff9695753 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -450,19 +450,6 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) codec_dai->active--; codec->active--; - /* clear the corresponding DAIs rate when inactive */ - if (!cpu_dai->active) { - cpu_dai->rate = 0; - cpu_dai->channels = 0; - cpu_dai->sample_bits = 0; - } - - if (!codec_dai->active) { - codec_dai->rate = 0; - codec_dai->channels = 0; - codec_dai->sample_bits = 0; - } - /* Muting the DAC suppresses artifacts caused during digital * shutdown, for example from stopping clocks. */ @@ -682,6 +669,19 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); + /* clear the corresponding DAIs parameters when going to be inactive */ + if (cpu_dai->active == 1) { + cpu_dai->rate = 0; + cpu_dai->channels = 0; + cpu_dai->sample_bits = 0; + } + + if (codec_dai->active == 1) { + codec_dai->rate = 0; + codec_dai->channels = 0; + codec_dai->sample_bits = 0; + } + /* apply codec digital mute */ if (!codec->active) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); From 4d9127faa864e7068d7e06527dfdf099ad06f64a Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Wed, 20 Nov 2013 15:37:42 -0200 Subject: [PATCH 3/7] ASoC: soc-io: Use IS_ENABLED() macro Using the IS_ENABLED() macro can make the code shorter and simpler. Signed-off-by: Fabio Estevam Signed-off-by: Mark Brown --- sound/soc/soc-io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c index 4f11d23f2062..aa886cca3ecf 100644 --- a/sound/soc/soc-io.c +++ b/sound/soc/soc-io.c @@ -99,14 +99,14 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec, config.val_bits = data_bits; switch (control) { -#if defined(CONFIG_REGMAP_I2C) || defined(CONFIG_REGMAP_I2C_MODULE) +#if IS_ENABLED(CONFIG_REGMAP_I2C) case SND_SOC_I2C: codec->control_data = regmap_init_i2c(to_i2c_client(codec->dev), &config); break; #endif -#if defined(CONFIG_REGMAP_SPI) || defined(CONFIG_REGMAP_SPI_MODULE) +#if IS_ENABLED(CONFIG_REGMAP_SPI) case SND_SOC_SPI: codec->control_data = regmap_init_spi(to_spi_device(codec->dev), &config); From 62e5f676f6a063e1ab0d6b8fcaef2feb026ee00e Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 30 Nov 2013 17:38:58 +0100 Subject: [PATCH 4/7] ASoC: Set SNDRV_PCM_INFO_JOINT_DUPLEX for PCMs with symmetry constraints If there are symmetry constraints between the playback and the capture channel set the SNDRV_PCM_INFO_JOINT_DUPLEX flag to let userspace know about this. Signed-off-by: Lars-Peter Clausen Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 170ff9695753..f3592f142832 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -180,6 +180,21 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream, return 0; } +static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai_driver *cpu_driver = rtd->cpu_dai->driver; + struct snd_soc_dai_driver *codec_driver = rtd->codec_dai->driver; + struct snd_soc_dai_link *link = rtd->dai_link; + + return cpu_driver->symmetric_rates || codec_driver->symmetric_rates || + link->symmetric_rates || cpu_driver->symmetric_channels || + codec_driver->symmetric_channels || link->symmetric_channels || + cpu_driver->symmetric_samplebits || + codec_driver->symmetric_samplebits || + link->symmetric_samplebits; +} + /* * List of sample sizes that might go over the bus for parameter * application. There ought to be a wildcard sample size for things @@ -309,6 +324,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) &cpu_dai_drv->capture); } + if (soc_pcm_has_symmetry(substream)) + runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + ret = -EINVAL; snd_pcm_limit_hw_rates(runtime); if (!runtime->hw.rates) { From 0b4bbae85e046042af76a65920db4bb5509c97bd Mon Sep 17 00:00:00 2001 From: Nicolin Chen Date: Wed, 4 Dec 2013 11:18:37 +0800 Subject: [PATCH 5/7] ASoC: soc-pcm: Drop the redundant snd_soc_dai_digital_mute() in soc_pcm_close() This patch removed the redundant snd_soc_dai_digital_mute() in close() since it's better to mute in hw_free() which's slightly earlier and symmetrical for the case of reconfiguration: 'aplay 44k1.wav 48k.wav', for example, will be open()->hw_params()->prepare(unmute)->playi1ng->hw_free(mute)->hw_params()-> parepare(unmute)->playing->hw_free(mute)->close() Signed-off-by: Nicolin Chen Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 42782c01e413..89d594138773 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -390,11 +390,6 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) if (!codec_dai->active) codec_dai->rate = 0; - /* Muting the DAC suppresses artifacts caused during digital - * shutdown, for example from stopping clocks. - */ - snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); - if (cpu_dai->driver->ops->shutdown) cpu_dai->driver->ops->shutdown(substream, cpu_dai); From 95ab1297e7b1a57c2b777f210b74f3a2bd2ac269 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Fri, 20 Dec 2013 14:20:25 +0100 Subject: [PATCH 6/7] ASoC: soc-utils: Don't set unused struct snd_pcm_hardware fields The ASoC core assumes that the PCM component of the ASoC card transparently moves data around and does not impose any restrictions on the memory layout or the transfer speed. It ignores all fields from the snd_pcm_hardware struct for the PCM driver that are related to this. Setting these fields in the PCM driver might suggest otherwise though, so rather not set them. Signed-off-by: Lars-Peter Clausen Signed-off-by: Mark Brown --- sound/soc/soc-utils.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 5e633659c1b3..6ebdfd9a1a1d 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -59,10 +59,6 @@ int snd_soc_params_to_bclk(struct snd_pcm_hw_params *params) EXPORT_SYMBOL_GPL(snd_soc_params_to_bclk); static const struct snd_pcm_hardware dummy_dma_hardware = { - .formats = 0xffffffff, - .channels_min = 1, - .channels_max = UINT_MAX, - /* Random values to keep userspace happy when checking constraints */ .info = SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER, From e41975edc73d2c16d0784e5fa87a6162e2fcab80 Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Fri, 20 Dec 2013 14:39:51 +0800 Subject: [PATCH 7/7] ASoC: core: Fix the DAI name getting. From "ASoC: make snd_soc_dai_link more symmetrical", can we see that the name of CPU DAI maybe omitted. If the DAI name is omitted, try to use the component name instead. Signed-off-by: Xiubo Li Signed-off-by: Mark Brown --- sound/soc/soc-core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4e53d87e881d..03c779ebd729 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -4617,10 +4617,14 @@ int snd_soc_of_get_dai_name(struct device_node *of_node, if (id < 0 || id >= pos->num_dai) { ret = -EINVAL; - } else { - *dai_name = pos->dai_drv[id].name; - ret = 0; + break; } + + ret = 0; + + *dai_name = pos->dai_drv[id].name; + if (!*dai_name) + *dai_name = pos->name; } break;