[alsa-devel] [PATCH 0/4] simple-audio-card codec2codec support
We are currently using simple-audio-card on the Allwinner A64 SoC. The digital audio codec there (sun8i-codec) has 3 AIFs, one each for the CPU, the modem, and Bluetooth. Adding support for the secondary AIFs requires adding codec2codec DAI links.
Since the modem and bt-sco codec DAI drivers only have one set of possible PCM parameters (namely, 8kHz mono S16LE), there's no real need for a machine driver to specify the DAI link configuration. The parameters for these "simple" DAI links can be chosen automatically.
This series adds a single "codec-to-codec" property to the simple-audio-card binding, which does exactly what it says. It works out rather nicely without making the device tree binding too complicated.
The first patch fixes a bug I found while implementing this feature.
I tried to reuse as much code as possible, so the middle two patches refactor a couple of helper functions to be more generic.
Finally, the last patch adds the new feature and its documentation.
Samuel Holland (4): ASoC: codec2codec: avoid invalid/double-free of pcm runtime ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly ASoC: pcm: Export parameter intersection logic ASoC: simple-card: Add support for codec-to-codec dai_links
.../devicetree/bindings/sound/simple-card.txt | 1 + Documentation/sound/soc/codec-to-codec.rst | 9 ++- .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 2 +- include/sound/pcm.h | 2 +- include/sound/simple_card_utils.h | 1 + include/sound/soc.h | 3 + sound/arm/aaci.c | 2 +- sound/arm/pxa2xx-ac97.c | 2 +- sound/core/pcm_misc.c | 14 ++--- sound/firewire/dice/dice-pcm.c | 2 +- sound/firewire/digi00x/digi00x-pcm.c | 2 +- sound/firewire/fireworks/fireworks_pcm.c | 2 +- sound/firewire/motu/motu-pcm.c | 2 +- sound/firewire/tascam/tascam-pcm.c | 2 +- sound/pci/atiixp.c | 2 +- sound/pci/cs5535audio/cs5535audio_pcm.c | 4 +- sound/pci/hda/hda_controller.c | 4 +- sound/pci/intel8x0.c | 2 +- sound/pci/sis7019.c | 2 +- sound/pci/via82xx.c | 4 +- sound/soc/generic/simple-card-utils.c | 39 +++++++++++++ sound/soc/generic/simple-card.c | 12 ++++ sound/soc/soc-dapm.c | 3 - sound/soc/soc-pcm.c | 57 +++++++++++++------ sound/usb/caiaq/audio.c | 4 +- 25 files changed, 130 insertions(+), 49 deletions(-)
The PCM runtime was freed during PMU in the case that the event hook encountered an error. However, it is also unconditionally freed during PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.
Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables") Cc: stable@vger.kernel.org Signed-off-by: Samuel Holland samuel@sholland.org --- sound/soc/soc-dapm.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index b6378f025836..935b5375ecc5 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, runtime->rate = params_rate(params);
out: - if (ret < 0) - kfree(runtime); - kfree(params); return ret; }
On Thu 13 Feb 2020 at 07:11, Samuel Holland samuel@sholland.org wrote:
The PCM runtime was freed during PMU in the case that the event hook encountered an error. However, it is also unconditionally freed during PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.
Oh ... Thanks for finding this.
I thought that a widget which has failed PMU would not go through PMD, but It seems the return value dapm_seq_check_event is not checked.
This brings another question/problem: A link which has failed in PMU, could try in PMD to hw_free/shutdown a dai which has not gone through startup/hw_params, right ?
Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables") Cc: stable@vger.kernel.org Signed-off-by: Samuel Holland samuel@sholland.org
sound/soc/soc-dapm.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index b6378f025836..935b5375ecc5 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, runtime->rate = params_rate(params);
out:
- if (ret < 0)
kfree(runtime);- kfree(params); return ret;
}
On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:
This brings another question/problem: A link which has failed in PMU, could try in PMD to hw_free/shutdown a dai which has not gone through startup/hw_params, right ?
I think so, yes.
On Thu 13 Feb 2020 at 12:37, Mark Brown broonie@kernel.org wrote:
On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:
This brings another question/problem: A link which has failed in PMU, could try in PMD to hw_free/shutdown a dai which has not gone through startup/hw_params, right ?
I think so, yes.
Maybe this can be solved using the dai active counts which the codec-to-codec event is not updating. I'll try to come up with something.
The patch
ASoC: codec2codec: avoid invalid/double-free of pcm runtime
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From b6570fdb96edf45bcf71884bd2644bd73d348d1a Mon Sep 17 00:00:00 2001
From: Samuel Holland samuel@sholland.org Date: Thu, 13 Feb 2020 00:11:44 -0600 Subject: [PATCH] ASoC: codec2codec: avoid invalid/double-free of pcm runtime
The PCM runtime was freed during PMU in the case that the event hook encountered an error. However, it is also unconditionally freed during PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.
Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables") Cc: stable@vger.kernel.org Signed-off-by: Samuel Holland samuel@sholland.org Link: https://lore.kernel.org/r/20200213061147.29386-2-samuel@sholland.org Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-dapm.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index bc20ad9abf8b..8b24396675ec 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3916,9 +3916,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w, runtime->rate = params_rate(params);
out: - if (ret < 0) - kfree(runtime); - kfree(params); return ret; }
It can be useful to derive min/max rates of a snd_pcm_hardware without having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually need the snd_pcm_runtime. Modify it to take a pointer to hw directly.
Signed-off-by: Samuel Holland samuel@sholland.org --- .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 2 +- include/sound/pcm.h | 2 +- sound/arm/aaci.c | 2 +- sound/arm/pxa2xx-ac97.c | 2 +- sound/core/pcm_misc.c | 14 +++++++------- sound/firewire/dice/dice-pcm.c | 2 +- sound/firewire/digi00x/digi00x-pcm.c | 2 +- sound/firewire/fireworks/fireworks_pcm.c | 2 +- sound/firewire/motu/motu-pcm.c | 2 +- sound/firewire/tascam/tascam-pcm.c | 2 +- sound/pci/atiixp.c | 2 +- sound/pci/cs5535audio/cs5535audio_pcm.c | 4 ++-- sound/pci/hda/hda_controller.c | 4 ++-- sound/pci/intel8x0.c | 2 +- sound/pci/sis7019.c | 2 +- sound/pci/via82xx.c | 4 ++-- sound/soc/soc-pcm.c | 4 ++-- sound/usb/caiaq/audio.c | 4 ++-- 18 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c index 2b7539701b42..33f7bcf992a4 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c @@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream) if (ret < 0) return ret;
- ret = snd_pcm_limit_hw_rates(runtime); + ret = snd_pcm_limit_hw_rates(&runtime->hw); if (ret < 0) return ret;
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 8a89fa6fdd5e..203b79d2712a 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream, return __snd_pcm_lib_xfer(substream, bufs, false, frames, true); }
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime); +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw); unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate); unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit); unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a, diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index b5399b0090a7..5052689247f9 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream) runtime->private_data = aacirun; runtime->hw = aaci_hw_info; runtime->hw.rates = aacirun->pcm->rates; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { runtime->hw.channels_max = 6; diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c index acfaf1d4ec25..cfb23073471e 100644 --- a/sound/arm/pxa2xx-ac97.c +++ b/sound/arm/pxa2xx-ac97.c @@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream) i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? AC97_RATES_FRONT_DAC : AC97_RATES_ADC; runtime->hw.rates = pxa2xx_ac97_ac97->rates[i]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw);
platform_ops = substream->pcm->card->dev->platform_data; if (platform_ops && platform_ops->startup) { diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c index c4eb561d2008..435688855ed0 100644 --- a/sound/core/pcm_misc.c +++ b/sound/core/pcm_misc.c @@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
/** * snd_pcm_limit_hw_rates - determine rate_min/rate_max fields - * @runtime: the runtime instance + * @runtime: the pcm hw instance * * Determines the rate_min and rate_max fields from the rates bits of - * the given runtime->hw. + * the given hw. * * Return: Zero if successful. */ -int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime) +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw) { int i; for (i = 0; i < (int)snd_pcm_known_rates.count; i++) { - if (runtime->hw.rates & (1 << i)) { - runtime->hw.rate_min = snd_pcm_known_rates.list[i]; + if (hw->rates & (1 << i)) { + hw->rate_min = snd_pcm_known_rates.list[i]; break; } } for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) { - if (runtime->hw.rates & (1 << i)) { - runtime->hw.rate_max = snd_pcm_known_rates.list[i]; + if (hw->rates & (1 << i)) { + hw->rate_max = snd_pcm_known_rates.list[i]; break; } } diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c index be79d659eedf..85941d945067 100644 --- a/sound/firewire/dice/dice-pcm.c +++ b/sound/firewire/dice/dice-pcm.c @@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice, hw->channels_max = max(hw->channels_max, channels); }
- snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(hw);
return 0; } diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c index 57cbce4fd836..0709070ab5af 100644 --- a/sound/firewire/digi00x/digi00x-pcm.c +++ b/sound/firewire/digi00x/digi00x-pcm.c @@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x, SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(hw);
err = snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c index e69896d748df..2ee8e98ea2b6 100644 --- a/sound/firewire/fireworks/fireworks_pcm.c +++ b/sound/firewire/fireworks/fireworks_pcm.c @@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
/* limit rates */ runtime->hw.rates = efw->supported_sampling_rate, - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw);
limit_channels(&runtime->hw, pcm_channels);
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c index 005970931030..338eb0572890 100644 --- a/sound/firewire/motu/motu-pcm.c +++ b/sound/firewire/motu/motu-pcm.c @@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu, hw->channels_max = max(hw->channels_max, pcm_channels); }
- snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(hw); }
static int init_hw_info(struct snd_motu *motu, diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c index 8e9b444c8bff..6722c1a65a42 100644 --- a/sound/firewire/tascam/tascam-pcm.c +++ b/sound/firewire/tascam/tascam-pcm.c @@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm, SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(hw);
return amdtp_tscm_add_pcm_hw_constraints(stream, runtime); } diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c index 1e1ededf8eb2..d7af407306b7 100644 --- a/sound/pci/atiixp.c +++ b/sound/pci/atiixp.c @@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream, dma->ac97_pcm_type = pcm_type; if (pcm_type >= 0) { runtime->hw.rates = chip->pcms[pcm_type]->rates; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); } else { /* direct SPDIF */ runtime->hw.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE; diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c index 4642e5384e83..7ce1664a8148 100644 --- a/sound/pci/cs5535audio/cs5535audio_pcm.c +++ b/sound/pci/cs5535audio/cs5535audio_pcm.c @@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
runtime->hw = snd_cs5535audio_playback; runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); cs5535au->playback_substream = substream; runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]); if ((err = snd_pcm_hw_constraint_integer(runtime, @@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
runtime->hw = snd_cs5535audio_capture; runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); cs5535au->capture_substream = substream; runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]); if ((err = snd_pcm_hw_constraint_integer(runtime, diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index ba56b59b3e17..3728dbfae7b0 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) runtime->hw.channels_max = hinfo->channels_max; runtime->hw.formats = hinfo->formats; runtime->hw.rates = hinfo->rates; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
/* avoid wrap-around with wall-clock */ @@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) azx_release_device(azx_dev); goto powerdown; } - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); /* sanity check */ if (snd_BUG_ON(!runtime->hw.channels_min) || snd_BUG_ON(!runtime->hw.channels_max) || diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 12374ba08ca2..7f85a30291a1 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich ichdev->substream = substream; runtime->hw = snd_intel8x0_stream; runtime->hw.rates = ichdev->pcm->rates; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); if (chip->device_type == DEVICE_SIS) { runtime->hw.buffer_bytes_max = 64*1024; runtime->hw.period_bytes_max = 64*1024; diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c index ef7dd290ae05..4d8e852d438f 100644 --- a/sound/pci/sis7019.c +++ b/sound/pci/sis7019.c @@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream) runtime->private_data = voice; runtime->hw = sis_capture_hw_info; runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 9, 0xfff9); snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c index 30c817b6b635..8a55153a6ea3 100644 --- a/sound/pci/via82xx.c +++ b/sound/pci/via82xx.c @@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev, if (chip->spdif_on && viadev->reg_offset == 0x30) { /* DXS#3 and spdif is on */ runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); } else if (chip->dxs_fixed && viadev->reg_offset < 0x40) { /* fixed DXS playback rate */ runtime->hw.rates = SNDRV_PCM_RATE_48000; @@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev, } else if (! ratep->rate) { int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC; runtime->hw.rates = chip->ac97->rates[idx]; - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw); } else { /* a fixed rate */ runtime->hw.rates = SNDRV_PCM_RATE_KNOT; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 01e7bc03d92f..bd7b3cfcfa62 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) hw->formats = formats & cpu_stream->formats; hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
- snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(hw);
hw->rate_min = max(hw->rate_min, cpu_stream->rate_min); hw->rate_min = max(hw->rate_min, rate_min); @@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
dpcm_set_fe_runtime(fe_substream); - snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw);
ret = dpcm_apply_symmetry(fe_substream, stream); if (ret < 0) { diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index 970eb0865ba3..242f50ebc63a 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
dev_dbg(dev, "%s(%p)\n", __func__, substream); substream->runtime->hw = cdev->pcm_info; - snd_pcm_limit_hw_rates(substream->runtime); + snd_pcm_limit_hw_rates(&substream->runtime->hw);
return 0; } @@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream) if (runtime->rate == rates[i]) cdev->pcm_info.rates = 1 << i;
- snd_pcm_limit_hw_rates(runtime); + snd_pcm_limit_hw_rates(&runtime->hw);
bytes_per_sample = BYTES_PER_SAMPLE; if (cdev->spec.data_alignment >= 2)
On Thu, 13 Feb 2020 07:11:45 +0100, Samuel Holland wrote:
It can be useful to derive min/max rates of a snd_pcm_hardware without having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually need the snd_pcm_runtime. Modify it to take a pointer to hw directly.
I prefer adding a new function and change snd_pcm_limit_hw_rates() to static inline just calling the new one with &runtime->hw, instead of touching so many callers site.
thanks,
Takashi
Signed-off-by: Samuel Holland samuel@sholland.org
.../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 2 +- include/sound/pcm.h | 2 +- sound/arm/aaci.c | 2 +- sound/arm/pxa2xx-ac97.c | 2 +- sound/core/pcm_misc.c | 14 +++++++------- sound/firewire/dice/dice-pcm.c | 2 +- sound/firewire/digi00x/digi00x-pcm.c | 2 +- sound/firewire/fireworks/fireworks_pcm.c | 2 +- sound/firewire/motu/motu-pcm.c | 2 +- sound/firewire/tascam/tascam-pcm.c | 2 +- sound/pci/atiixp.c | 2 +- sound/pci/cs5535audio/cs5535audio_pcm.c | 4 ++-- sound/pci/hda/hda_controller.c | 4 ++-- sound/pci/intel8x0.c | 2 +- sound/pci/sis7019.c | 2 +- sound/pci/via82xx.c | 4 ++-- sound/soc/soc-pcm.c | 4 ++-- sound/usb/caiaq/audio.c | 4 ++-- 18 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c index 2b7539701b42..33f7bcf992a4 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c @@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream) if (ret < 0) return ret;
- ret = snd_pcm_limit_hw_rates(runtime);
- ret = snd_pcm_limit_hw_rates(&runtime->hw); if (ret < 0) return ret;
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 8a89fa6fdd5e..203b79d2712a 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream, return __snd_pcm_lib_xfer(substream, bufs, false, frames, true); }
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime); +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw); unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate); unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit); unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a, diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index b5399b0090a7..5052689247f9 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream) runtime->private_data = aacirun; runtime->hw = aaci_hw_info; runtime->hw.rates = aacirun->pcm->rates;
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { runtime->hw.channels_max = 6;
diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c index acfaf1d4ec25..cfb23073471e 100644 --- a/sound/arm/pxa2xx-ac97.c +++ b/sound/arm/pxa2xx-ac97.c @@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream) i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? AC97_RATES_FRONT_DAC : AC97_RATES_ADC; runtime->hw.rates = pxa2xx_ac97_ac97->rates[i];
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);
platform_ops = substream->pcm->card->dev->platform_data; if (platform_ops && platform_ops->startup) {
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c index c4eb561d2008..435688855ed0 100644 --- a/sound/core/pcm_misc.c +++ b/sound/core/pcm_misc.c @@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
/**
- snd_pcm_limit_hw_rates - determine rate_min/rate_max fields
- @runtime: the runtime instance
- @runtime: the pcm hw instance
- Determines the rate_min and rate_max fields from the rates bits of
- the given runtime->hw.
*/
- the given hw.
- Return: Zero if successful.
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime) +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw) { int i; for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
if (runtime->hw.rates & (1 << i)) {runtime->hw.rate_min = snd_pcm_known_rates.list[i];
if (hw->rates & (1 << i)) {hw->rate_min = snd_pcm_known_rates.list[i]; break;
if (runtime->hw.rates & (1 << i)) {runtime->hw.rate_max = snd_pcm_known_rates.list[i];
if (hw->rates & (1 << i)) {hw->rate_max = snd_pcm_known_rates.list[i]; break;diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c index be79d659eedf..85941d945067 100644 --- a/sound/firewire/dice/dice-pcm.c +++ b/sound/firewire/dice/dice-pcm.c @@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice, hw->channels_max = max(hw->channels_max, channels); }
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(hw);
return 0;
} diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c index 57cbce4fd836..0709070ab5af 100644 --- a/sound/firewire/digi00x/digi00x-pcm.c +++ b/sound/firewire/digi00x/digi00x-pcm.c @@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x, SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000;
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(hw);
err = snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c index e69896d748df..2ee8e98ea2b6 100644 --- a/sound/firewire/fireworks/fireworks_pcm.c +++ b/sound/firewire/fireworks/fireworks_pcm.c @@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
/* limit rates */ runtime->hw.rates = efw->supported_sampling_rate,
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);
limit_channels(&runtime->hw, pcm_channels);
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c index 005970931030..338eb0572890 100644 --- a/sound/firewire/motu/motu-pcm.c +++ b/sound/firewire/motu/motu-pcm.c @@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu, hw->channels_max = max(hw->channels_max, pcm_channels); }
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(hw);
}
static int init_hw_info(struct snd_motu *motu, diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c index 8e9b444c8bff..6722c1a65a42 100644 --- a/sound/firewire/tascam/tascam-pcm.c +++ b/sound/firewire/tascam/tascam-pcm.c @@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm, SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000;
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(hw);
return amdtp_tscm_add_pcm_hw_constraints(stream, runtime);
} diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c index 1e1ededf8eb2..d7af407306b7 100644 --- a/sound/pci/atiixp.c +++ b/sound/pci/atiixp.c @@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream, dma->ac97_pcm_type = pcm_type; if (pcm_type >= 0) { runtime->hw.rates = chip->pcms[pcm_type]->rates;
snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c index 4642e5384e83..7ce1664a8148 100644 --- a/sound/pci/cs5535audio/cs5535audio_pcm.c +++ b/sound/pci/cs5535audio/cs5535audio_pcm.c @@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
runtime->hw = snd_cs5535audio_playback; runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC];
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(&runtime->hw); cs5535au->playback_substream = substream; runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]); if ((err = snd_pcm_hw_constraint_integer(runtime,
@@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
runtime->hw = snd_cs5535audio_capture; runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC];
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(&runtime->hw); cs5535au->capture_substream = substream; runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]); if ((err = snd_pcm_hw_constraint_integer(runtime,
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index ba56b59b3e17..3728dbfae7b0 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) runtime->hw.channels_max = hinfo->channels_max; runtime->hw.formats = hinfo->formats; runtime->hw.rates = hinfo->rates;
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw); snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
/* avoid wrap-around with wall-clock */
@@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) azx_release_device(azx_dev); goto powerdown; }
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(&runtime->hw); /* sanity check */ if (snd_BUG_ON(!runtime->hw.channels_min) || snd_BUG_ON(!runtime->hw.channels_max) ||
diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 12374ba08ca2..7f85a30291a1 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich ichdev->substream = substream; runtime->hw = snd_intel8x0_stream; runtime->hw.rates = ichdev->pcm->rates;
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(&runtime->hw); if (chip->device_type == DEVICE_SIS) { runtime->hw.buffer_bytes_max = 64*1024; runtime->hw.period_bytes_max = 64*1024;
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c index ef7dd290ae05..4d8e852d438f 100644 --- a/sound/pci/sis7019.c +++ b/sound/pci/sis7019.c @@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream) runtime->private_data = voice; runtime->hw = sis_capture_hw_info; runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC];
- snd_pcm_limit_hw_rates(runtime);
- snd_pcm_limit_hw_rates(&runtime->hw); snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 9, 0xfff9); snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c index 30c817b6b635..8a55153a6ea3 100644 --- a/sound/pci/via82xx.c +++ b/sound/pci/via82xx.c @@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev, if (chip->spdif_on && viadev->reg_offset == 0x30) { /* DXS#3 and spdif is on */ runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF];
snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);@@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev, } else if (! ratep->rate) { int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC; runtime->hw.rates = chip->ac97->rates[idx];
snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 01e7bc03d92f..bd7b3cfcfa62 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) hw->formats = formats & cpu_stream->formats; hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(hw);
hw->rate_min = max(hw->rate_min, cpu_stream->rate_min); hw->rate_min = max(hw->rate_min, rate_min);
@@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
dpcm_set_fe_runtime(fe_substream);
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);
ret = dpcm_apply_symmetry(fe_substream, stream); if (ret < 0) {
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index 970eb0865ba3..242f50ebc63a 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
dev_dbg(dev, "%s(%p)\n", __func__, substream); substream->runtime->hw = cdev->pcm_info;
- snd_pcm_limit_hw_rates(substream->runtime);
snd_pcm_limit_hw_rates(&substream->runtime->hw);
return 0;
} @@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream) if (runtime->rate == rates[i]) cdev->pcm_info.rates = 1 << i;
- snd_pcm_limit_hw_rates(runtime);
snd_pcm_limit_hw_rates(&runtime->hw);
bytes_per_sample = BYTES_PER_SAMPLE; if (cdev->spec.data_alignment >= 2)
-- 2.24.1
On 2/13/20 12:30 AM, Takashi Iwai wrote:
On Thu, 13 Feb 2020 07:11:45 +0100, Samuel Holland wrote:
It can be useful to derive min/max rates of a snd_pcm_hardware without having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually need the snd_pcm_runtime. Modify it to take a pointer to hw directly.
I prefer adding a new function and change snd_pcm_limit_hw_rates() to static inline just calling the new one with &runtime->hw, instead of touching so many callers site.
I agree. I will definitely do that for v2.
Thanks, Samuel
thanks,
Takashi
Signed-off-by: Samuel Holland samuel@sholland.org
The logic to calculate the subset of stream parameters supported by all DAIs associated with a PCM stream is nontrivial. Export a helper function so it can be used to set up simple codec2codec DAI links.
Signed-off-by: Samuel Holland samuel@sholland.org --- include/sound/soc.h | 3 +++ sound/soc/soc-pcm.c | 53 ++++++++++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 262896799826..578a8e3e08ca 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -473,6 +473,9 @@ bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd); void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream); void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream);
+int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd, + struct snd_pcm_hardware *hw, int stream); + int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, unsigned int dai_fmt);
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index bd7b3cfcfa62..562af1d3f24e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -348,11 +348,18 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream) soc_pcm_set_msb(substream, cpu_bits); }
-static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) +/** + * snd_soc_runtime_calc_hw() - Calculate hw limits for a PCM stream + * @rtd: ASoC PCM runtime + * @hw: PCM hardware parameters (output) + * @stream: Direction of the PCM stream + * + * Calculates the subset of stream parameters supported by all DAIs + * associated with the PCM stream. + */ +int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd, + struct snd_pcm_hardware *hw, int stream) { - struct snd_pcm_runtime *runtime = substream->runtime; - struct snd_pcm_hardware *hw = &runtime->hw; - struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *codec_dai; struct snd_soc_dai_driver *cpu_dai_drv = rtd->cpu_dai->driver; struct snd_soc_dai_driver *codec_dai_drv; @@ -364,7 +371,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) u64 formats = ULLONG_MAX; int i;
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + if (stream == SNDRV_PCM_STREAM_PLAYBACK) cpu_stream = &cpu_dai_drv->playback; else cpu_stream = &cpu_dai_drv->capture; @@ -377,16 +384,12 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) * Otherwise, since the rate, channel, and format values will * zero in that case, we would have no usable settings left, * causing the resulting setup to fail. - * At least one CODEC should match, otherwise we should have - * bailed out on a higher level, since there would be no - * CODEC to support the transfer direction in that case. */ - if (!snd_soc_dai_stream_valid(codec_dai, - substream->stream)) + if (!snd_soc_dai_stream_valid(codec_dai, stream)) continue;
codec_dai_drv = codec_dai->driver; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + if (stream == SNDRV_PCM_STREAM_PLAYBACK) codec_stream = &codec_dai_drv->playback; else codec_stream = &codec_dai_drv->capture; @@ -398,6 +401,9 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) rates = snd_pcm_rate_mask_intersect(codec_stream->rates, rates); }
+ if (!chan_min) + return -EINVAL; + /* * chan min/max cannot be enforced if there are multiple CODEC DAIs * connected to a single CPU DAI, use CPU DAI's directly and let @@ -410,10 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
hw->channels_min = max(chan_min, cpu_stream->channels_min); hw->channels_max = min(chan_max, cpu_stream->channels_max); - if (hw->formats) - hw->formats &= formats & cpu_stream->formats; - else - hw->formats = formats & cpu_stream->formats; + hw->formats = formats & cpu_stream->formats; hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
snd_pcm_limit_hw_rates(hw); @@ -422,6 +425,26 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) hw->rate_min = max(hw->rate_min, rate_min); hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max); hw->rate_max = min_not_zero(hw->rate_max, rate_max); + + return 0; +} +EXPORT_SYMBOL_GPL(snd_soc_runtime_calc_hw); + +static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) +{ + struct snd_pcm_hardware *hw = &substream->runtime->hw; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + u64 formats = hw->formats; + + /* + * At least one CODEC should match, otherwise we should have + * bailed out on a higher level, since there would be no + * CODEC to support the transfer direction in that case. + */ + snd_soc_runtime_calc_hw(rtd, hw, substream->stream); + + if (formats) + hw->formats &= formats; }
static int soc_pcm_components_open(struct snd_pcm_substream *substream,
For now we assume there are only a few sets of valid PCM stream parameters, to avoid needing to specify them in the device tree. We also assume they are the same in both directions. We calculate the common subset of parameters, and then the existing code automatically chooses the highest quality of the remaining values.
Signed-off-by: Samuel Holland samuel@sholland.org --- .../devicetree/bindings/sound/simple-card.txt | 1 + Documentation/sound/soc/codec-to-codec.rst | 9 ++++- include/sound/simple_card_utils.h | 1 + sound/soc/generic/simple-card-utils.c | 39 +++++++++++++++++++ sound/soc/generic/simple-card.c | 12 ++++++ 5 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 79954cd6e37b..18a62e404a30 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -63,6 +63,7 @@ Optional dai-link subnode properties: - mclk-fs : Multiplication factor between stream rate and codec mclk, applied only for the dai-link. +- codec-to-codec : Indicates a codec-to-codec dai-link.
For backward compatibility the frame-master and bitclock-master properties can be used as booleans in codec subnode to indicate if the diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst index 810109d7500d..efe0a8c07933 100644 --- a/Documentation/sound/soc/codec-to-codec.rst +++ b/Documentation/sound/soc/codec-to-codec.rst @@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture dai names ending with "Playback" and "Capture" respectively as dapm core will link and power those dais based on the name.
-Note that in current device tree there is no way to mark a dai_link -as codec to codec. However, it may change in future. +A dai_link in a "simple-audio-card" can be marked as codec to codec in +the device tree by adding the "codec-to-codec" property. The dai_link +will be initialized with the subset of stream parameters (channels, +format, sample rate) supported by all DAIs on the link. Since there is +no way to provide these parameters in the device tree, this is mostly +useful for communication with simple fixed-function codecs, such as a +Bluetooth controller or cellular modem. diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index bbdd1542d6f1..80b60237b3cd 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -49,6 +49,7 @@ struct asoc_simple_priv { struct asoc_simple_data adata; struct snd_soc_codec_conf *codec_conf; unsigned int mclk_fs; + bool codec_to_codec; } *dai_props; struct asoc_simple_jack hp_jack; struct asoc_simple_jack mic_jack; diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 9b794775df53..2de4cfead790 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai, return 0; }
+static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd, + struct simple_dai_props *dai_props) +{ + struct snd_soc_dai_link *dai_link = rtd->dai_link; + struct snd_soc_pcm_stream *params; + struct snd_pcm_hardware hw; + int ret; + + if (!dai_props->codec_to_codec) + return 0; + + /* Assumes the hardware capabilities are the same in both directions */ + ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK); + if (ret < 0) { + dev_err(rtd->dev, "simple-card: no valid dai_link params\n"); + return ret; + } + + params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL); + if (!params) + return -ENOMEM; + + params->formats = hw.formats; + params->rates = hw.rates; + params->rate_min = hw.rate_min; + params->rate_max = hw.rate_max; + params->channels_min = hw.channels_min; + params->channels_max = hw.channels_max; + + dai_link->params = params; + dai_link->num_params = 1; + + return 0; +} + int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) { struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card); @@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) if (ret < 0) return ret;
+ ret = asoc_simple_init_dai_link_params(rtd, dai_props); + if (ret < 0) + return ret; + return 0; } EXPORT_SYMBOL_GPL(asoc_simple_dai_init); diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 55e9f8800b3e..179ab4482d10 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node, return 0; }
+static void simple_parse_codec_to_codec(struct device_node *node, + struct simple_dai_props *props, + char *prefix) +{ + char prop[128]; + + snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix); + props->codec_to_codec = of_property_read_bool(node, prop); +} + static void simple_parse_convert(struct device *dev, struct device_node *np, struct asoc_simple_data *adata) @@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv, "prefix"); }
+ simple_parse_codec_to_codec(node, dai_props, prefix); simple_parse_convert(dev, np, &dai_props->adata); simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
@@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv, if (ret < 0) goto dai_link_of_err;
+ simple_parse_codec_to_codec(node, dai_props, prefix); simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);
On Thu 13 Feb 2020 at 07:11, Samuel Holland samuel@sholland.org wrote:
For now we assume there are only a few sets of valid PCM stream parameters, to avoid needing to specify them in the device tree. We also assume they are the same in both directions. We calculate the common subset of parameters, and then the existing code automatically chooses the highest quality of the remaining values.
Signed-off-by: Samuel Holland samuel@sholland.org
.../devicetree/bindings/sound/simple-card.txt | 1 + Documentation/sound/soc/codec-to-codec.rst | 9 ++++- include/sound/simple_card_utils.h | 1 + sound/soc/generic/simple-card-utils.c | 39 +++++++++++++++++++ sound/soc/generic/simple-card.c | 12 ++++++ 5 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 79954cd6e37b..18a62e404a30 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -63,6 +63,7 @@ Optional dai-link subnode properties:
- mclk-fs : Multiplication factor between stream rate and codec mclk, applied only for the dai-link.
+- codec-to-codec : Indicates a codec-to-codec dai-link.
I wonder if such property could be viewed as a Linux implementation detail ?
Similar discussion around DPCM:
* https://lore.kernel.org/linux-devicetree/20191014115635.GB4826@sirena.co.uk/... * https://alsa-devel.alsa-project.narkive.com/NCs4MsqL/simle-card-dt-style-for...
Should the card figure out the codec-to-codec links on its own or is it something generic enough to put in DT ?
In the Amlogic card driver, I'm using the compatible string of the device to figure out if CPU is a CODEC. It is ad-hoc and, to be honest, I'm not entirely happy with this solution but I could not figure out a better one yet.
For backward compatibility the frame-master and bitclock-master properties can be used as booleans in codec subnode to indicate if the diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst index 810109d7500d..efe0a8c07933 100644 --- a/Documentation/sound/soc/codec-to-codec.rst +++ b/Documentation/sound/soc/codec-to-codec.rst @@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture dai names ending with "Playback" and "Capture" respectively as dapm core will link and power those dais based on the name.
-Note that in current device tree there is no way to mark a dai_link -as codec to codec. However, it may change in future. +A dai_link in a "simple-audio-card" can be marked as codec to codec in +the device tree by adding the "codec-to-codec" property. The dai_link +will be initialized with the subset of stream parameters (channels, +format, sample rate) supported by all DAIs on the link. Since there is +no way to provide these parameters in the device tree, this is mostly +useful for communication with simple fixed-function codecs, such as a +Bluetooth controller or cellular modem. diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index bbdd1542d6f1..80b60237b3cd 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -49,6 +49,7 @@ struct asoc_simple_priv { struct asoc_simple_data adata; struct snd_soc_codec_conf *codec_conf; unsigned int mclk_fs;
bool codec_to_codec;diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 9b794775df53..2de4cfead790 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai, return 0; }
+static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd,
struct simple_dai_props *dai_props)+{
- struct snd_soc_dai_link *dai_link = rtd->dai_link;
- struct snd_soc_pcm_stream *params;
- struct snd_pcm_hardware hw;
- int ret;
- if (!dai_props->codec_to_codec)
return 0;- /* Assumes the hardware capabilities are the same in both directions */
- ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK);
- if (ret < 0) {
dev_err(rtd->dev, "simple-card: no valid dai_link params\n");return ret;- }
- params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL);
- if (!params)
return -ENOMEM;- params->formats = hw.formats;
- params->rates = hw.rates;
- params->rate_min = hw.rate_min;
- params->rate_max = hw.rate_max;
- params->channels_min = hw.channels_min;
- params->channels_max = hw.channels_max;
- dai_link->params = params;
- dai_link->num_params = 1;
- return 0;
+}
int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) { struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card); @@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) if (ret < 0) return ret;
- ret = asoc_simple_init_dai_link_params(rtd, dai_props);
- if (ret < 0)
return ret;- return 0;
} EXPORT_SYMBOL_GPL(asoc_simple_dai_init); diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 55e9f8800b3e..179ab4482d10 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node, return 0; }
+static void simple_parse_codec_to_codec(struct device_node *node,
struct simple_dai_props *props,char *prefix)+{
- char prop[128];
- snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix);
- props->codec_to_codec = of_property_read_bool(node, prop);
+}
static void simple_parse_convert(struct device *dev, struct device_node *np, struct asoc_simple_data *adata) @@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv, "prefix"); }
- simple_parse_codec_to_codec(node, dai_props, prefix); simple_parse_convert(dev, np, &dai_props->adata); simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
@@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv, if (ret < 0) goto dai_link_of_err;
simple_parse_codec_to_codec(node, dai_props, prefix); simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);
On Thu, Feb 13, 2020 at 10:18:53AM +0100, Jerome Brunet wrote:
On Thu 13 Feb 2020 at 07:11, Samuel Holland samuel@sholland.org wrote:
+- codec-to-codec : Indicates a codec-to-codec dai-link.
I wonder if such property could be viewed as a Linux implementation detail ?
Yes, it is.
Should the card figure out the codec-to-codec links on its own or is it something generic enough to put in DT ?
We should be able to figure it out by ourselves, we already have a link between two CODECs - we should be able to infer that it is in fact a link between two CODECs with the information we already have, probably by looking at the two devices that are referenced.
participants (4)
-
Jerome Brunet -
Mark Brown -
Samuel Holland -
Takashi Iwai