Re: [PATCH 0/4] ASoC: fsl_asrc: allow selecting arbitrary clocks
Hi Nic, Le 02/07/2020 à 20:42, Nicolin Chen a écrit : > Hi Arnaud, > > On Thu, Jul 02, 2020 at 04:22:31PM +0200, Arnaud Ferraris wrote: >> The current ASRC driver hardcodes the input and output clocks used for >> sample rate conversions. In order to allow greater flexibility and to >> cover more use cases, it would be preferable to select the clocks using >> device-tree properties. > > We recent just merged a new change that auto-selecting internal > clocks based on sample rates as the first option -- ideal ratio > mode is the fallback mode now. Please refer to: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200702&id=d0250cf4f2abfbea64ed247230f08f5ae23979f0 While working on fixing the automatic clock selection (see my v3), I came across another potential issue, which would be better explained with an example: - Input has sample rate 8kHz and uses clock SSI1 with rate 512kHz - Output has sample rate 16kHz and uses clock SSI2 with rate 1024kHz Let's say my v3 patch is merged, then the selected input clock will be SSI1, while the selected output clock will be SSI2. In that case, it's all good, as the driver will calculate the dividers right. Now, suppose a similar board has the input wired to SSI2 and output to SSI1, meaning we're now in the following case: - Input has sample rate 8kHz and uses clock SSI2 with rate 512kHz - Output has sample rate 16kHz and uses clock SSI1 with rate 1024kHz (the same result is achieved during capture with the initial example setup, as input and output properties are then swapped) In that case, the selected clocks will still be SSI1 for input (just because it appears first in the clock table), and SSI2 for output, meaning the calculated dividers will be: - input: 512 / 16 => 32 (should be 64) - output: 1024 / 8 => 128 (should be 64 here too) --- I can't see how the clock selection algorithm could be made smart enough to cover cases such as this one, as it would need to be aware of the exact relationship between the sample rate and the clock rate (my example demonstrates a case where the "sample rate to clock rate" multiplier is identical for both input and output, but this can't be assumed to be always the case). Therefore, I still believe being able to force clock selection using optional DT properties would make sense, while still using the auto-selection by default. Regards, Arnaud > > Having a quick review at your changes, I think the DT part may > not be necessary as it's more likely a software configuration. > I personally like the new auto-selecting solution more. > >> This series also fix register configuration and clock assignment so >> conversion can be conducted effectively in both directions with a good >> quality. > > If there's any further change that you feel you can improve on > the top of mentioned change after rebasing, I'd like to review. > > Thanks > Nic >
Re: [PATCH 0/4] ASoC: fsl_asrc: allow selecting arbitrary clocks
Hi Nic, Le 02/07/2020 à 20:42, Nicolin Chen a écrit : > Hi Arnaud, > > On Thu, Jul 02, 2020 at 04:22:31PM +0200, Arnaud Ferraris wrote: >> The current ASRC driver hardcodes the input and output clocks used for >> sample rate conversions. In order to allow greater flexibility and to >> cover more use cases, it would be preferable to select the clocks using >> device-tree properties. > > We recent just merged a new change that auto-selecting internal > clocks based on sample rates as the first option -- ideal ratio > mode is the fallback mode now. Please refer to: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200702&id=d0250cf4f2abfbea64ed247230f08f5ae23979f0 That looks interesting, thanks for pointing this out! I'll rebase and see how it works for my use-case, will keep you informed. Regards, Arnaud
Re: [PATCH 1/2] dt-bindings: sound: fsl-asoc-card: add new compatible for I2S slave
Le 02/07/2020 à 17:42, Mark Brown a écrit : > On Thu, Jul 02, 2020 at 05:28:03PM +0200, Arnaud Ferraris wrote: >> Le 02/07/2020 à 16:31, Mark Brown a écrit : > >>> Why require that the CODEC be clock master here - why not make this >>> configurable, reusing the properties from the generic and audio graph >>> cards? > >> This is partly because I'm not sure how to do it (yet), but mostly >> because I don't have the hardware to test this (the 2 CODECs present on >> my only i.MX6 board are both clock master) > > Take a look at what the generic cards are doing, it's a library function > asoc_simple_parse_daifmt(). It's not the end of the world if you can't > test it properly - if it turns out it's buggy somehow someone can always > fix the code later but an ABI is an ABI so we can't change it. > Thanks for the hints, I'll look into it. Regards, Arnaud
Re: [PATCH 1/2] dt-bindings: sound: fsl-asoc-card: add new compatible for I2S slave
Hi Mark, Le 02/07/2020 à 16:31, Mark Brown a écrit : > On Thu, Jul 02, 2020 at 04:11:14PM +0200, Arnaud Ferraris wrote: >> fsl-asoc-card currently doesn't support generic codecs with the SoC >> acting as I2S slave. >> >> This commit adds a new `fsl,imx-audio-i2s-slave` for this use-case, as >> well as the following mandatory properties: > > Why require that the CODEC be clock master here - why not make this > configurable, reusing the properties from the generic and audio graph > cards? This is partly because I'm not sure how to do it (yet), but mostly because I don't have the hardware to test this (the 2 CODECs present on my only i.MX6 board are both clock master) Regards, Arnaud
[PATCH 4/4] ASoC: fsl_asrc: swap input and output clocks in capture mode
The input clock is the reference clock we need to convert the stream to,
which therefore has to be the clock of the origin stream/device.
When the stream is bi-directional and we want ASRC to act on both
directions, we need to swap the input and output clocks between the
playback and capture streams.
As some of the clocks have different ID's depending on whether they are
used as input or output, this requires adding a new function to find the
output clock ID corresponding to a given input clock.
Signed-off-by: Arnaud Ferraris
---
sound/soc/fsl/fsl_asrc.c | 50 ++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 65e7307a3df0..5aeab1fbcdd9 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -506,6 +506,50 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
*pair, bool use_ideal_rate)
return fsl_asrc_set_ideal_ratio(pair, inrate, outrate);
}
+/**
+ * Select the output clock corresponding to a given input clock (and
vice-versa)
+ *
+ * If we want to setup a capture channel, the input and output clocks have to
+ * be swapped.
+ * However, even if most of the clocks have the same index when used as input
+ * or output, some of them (ESAI, SSI* and SPDIF) are different:
+ * - the TX output clock has the index of the corresponding RX input clock
+ * - the RX output clock has the index of the corresponding TX input clock
+ *
+ * This function makes sure that we use the proper clock index when swapping
+ * the input and output clocks.
+ */
+static enum asrc_outclk fsl_asrc_get_capture_clock(enum asrc_inclk inclk)
+{
+ enum asrc_outclk outclk;
+
+ switch (inclk) {
+ case INCLK_ESAI_RX:
+ case INCLK_SSI1_RX:
+ case INCLK_SSI2_RX:
+ case INCLK_SPDIF_RX:
+ outclk = inclk + 0x8;
+ break;
+ case INCLK_SSI3_RX:
+ outclk = OUTCLK_SSI3_RX;
+ break;
+ case INCLK_ESAI_TX:
+ case INCLK_SSI1_TX:
+ case INCLK_SSI2_TX:
+ case INCLK_SPDIF_TX:
+ outclk = inclk - 0x8;
+ break;
+ case INCLK_SSI3_TX:
+ outclk = OUTCLK_SSI3_TX;
+ break;
+ default:
+ outclk = inclk;
+ break;
+ }
+
+ return outclk;
+}
+
/**
* Start the assigned ASRC pair
*
@@ -604,15 +648,17 @@ static int fsl_asrc_dai_hw_params(struct
snd_pcm_substream *substream,
config.pair = pair->index;
config.channel_num = channels;
- config.inclk = asrc->inclk;
- config.outclk = asrc->outclk;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ config.inclk = asrc->inclk;
+ config.outclk = asrc->outclk;
config.input_format = params_format(params);
config.output_format = asrc->asrc_format;
config.input_sample_rate = rate;
config.output_sample_rate = asrc->asrc_rate;
} else {
+ config.inclk = fsl_asrc_get_capture_clock(asrc->outclk);
+ config.outclk = fsl_asrc_get_capture_clock(asrc->inclk);
config.input_format = asrc->asrc_format;
config.output_format = params_format(params);
config.input_sample_rate = asrc->asrc_rate;
--
2.27.0
[PATCH 3/4] ASoC: fsl_asrc: always use ratio for conversion
Even when not in "Ideal Ratio" mode, ASRC can use an internally measured ratio, which greatly improves the conversion quality. This patch ensures we always use at least the internal ratio. Signed-off-by: Arnaud Ferraris --- sound/soc/fsl/fsl_asrc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 75df220e4b51..65e7307a3df0 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -451,7 +451,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) regmap_update_bits(asrc->regmap, REG_ASRCTR, ASRCTR_ATSi_MASK(index), ASRCTR_ATS(index)); regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_USRi_MASK(index), 0); + ASRCTR_USRi_MASK(index), ASRCTR_USR(index)); /* Set the input and output clock sources */ regmap_update_bits(asrc->regmap, REG_ASRCSR, @@ -493,8 +493,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) /* Enable Ideal Ratio mode */ regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index), - ASRCTR_IDR(index) | ASRCTR_USR(index)); + ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index)); fsl_asrc_sel_proc(inrate, outrate, &pre_proc, &post_proc); -- 2.27.0
[PATCH 2/4] ASoC: fsl_asrc: allow using arbitrary input and output clocks
fsl_asrc currently uses hardcoded input and output clocks, preventing
its use for anything other than S/PDIF output.
This patch adds the ability to select any clock as input or output (by
using new DT properties), making it possible to use this peripheral in a
more advanced way.
Signed-off-by: Arnaud Ferraris
---
sound/soc/fsl/fsl_asrc.c| 18 --
sound/soc/fsl/fsl_asrc_common.h | 3 +++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 95f6a9617b0b..75df220e4b51 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -605,8 +605,8 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream
*substream,
config.pair = pair->index;
config.channel_num = channels;
- config.inclk = INCLK_NONE;
- config.outclk = OUTCLK_ASRCK1_CLK;
+ config.inclk = asrc->inclk;
+ config.outclk = asrc->outclk;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
config.input_format = params_format(params);
@@ -1067,6 +1067,20 @@ static int fsl_asrc_probe(struct platform_device *pdev)
asrc->channel_avail = 10;
+ ret = of_property_read_u32(np, "fsl,asrc-input-clock",
+ &asrc->inclk);
+ if (ret) {
+ dev_info(&pdev->dev, "no input clock specified, using none\n");
+ asrc->inclk = INCLK_NONE;
+ }
+
+ ret = of_property_read_u32(np, "fsl,asrc-output-clock",
+ &asrc->outclk);
+ if (ret) {
+ dev_info(&pdev->dev, "no output clock specified, using
default\n");
+ asrc->outclk = OUTCLK_ASRCK1_CLK;
+ }
+
ret = of_property_read_u32(np, "fsl,asrc-rate",
&asrc->asrc_rate);
if (ret) {
diff --git a/sound/soc/fsl/fsl_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
index 7e1c13ca37f1..1468878fbaca 100644
--- a/sound/soc/fsl/fsl_asrc_common.h
+++ b/sound/soc/fsl/fsl_asrc_common.h
@@ -89,6 +89,9 @@ struct fsl_asrc {
struct fsl_asrc_pair *pair[PAIR_CTX_NUM];
unsigned int channel_avail;
+ enum asrc_inclk inclk;
+ enum asrc_outclk outclk;
+
int asrc_rate;
snd_pcm_format_t asrc_format;
bool use_edma;
--
2.27.0
[PATCH 1/4] dt-bindings: sound: fsl, asrc: add properties to select in/out clocks
The ASRC peripheral accepts a wide range of input and output clocks, but
no mechanism exists at the moment to define those as they are currently
hardcoded in the driver.
This commit adds new properties allowing selection of arbitrary input
and output clocks.
Signed-off-by: Arnaud Ferraris
---
Documentation/devicetree/bindings/sound/fsl,asrc.txt | 8
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
index 998b4c8a7f78..e26ce9bad617 100644
--- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
@@ -55,6 +55,12 @@ Optional properties:
Ends, which can replace the fsl,asrc-width.
The value is 2 (S16_LE), or 6 (S24_LE).
+ - fsl,asrc-input-clock : Input clock ID, defaults to INCLK_NONE
+ (see enum asrc_inclk in fsl_asrc.h)
+
+ - fsl,asrc-output-clock : Output clock ID, defaults to OUTCLK_ASRCK1_CLK
+ (see enum asrc_outclk in fsl_asrc.h)
+
Example:
asrc: asrc@2034000 {
@@ -77,4 +83,6 @@ asrc: asrc@2034000 {
"txa", "txb", "txc";
fsl,asrc-rate = <48000>;
fsl,asrc-width = <16>;
+ fsl,asrc-input-clock = <0x3>;
+ fsl,asrc-output-clock = <0xf>;
};
--
2.27.0
[PATCH 0/4] ASoC: fsl_asrc: allow selecting arbitrary clocks
The current ASRC driver hardcodes the input and output clocks used for sample rate conversions. In order to allow greater flexibility and to cover more use cases, it would be preferable to select the clocks using device-tree properties. This series also fix register configuration and clock assignment so conversion can be conducted effectively in both directions with a good quality. Arnaud Ferraris (4): dt-bindings: sound: fsl,asrc: add properties to select in/out clocks ASoC: fsl_asrc: allow using arbitrary input and output clocks ASoC: fsl_asrc: always use ratio for conversion ASoC: fsl_asrc: swap input and output clocks in capture mode Documentation/devicetree/bindings/sound/fsl,asrc.txt | 8 sound/soc/fsl/fsl_asrc.c | 69 - sound/soc/fsl/fsl_asrc_common.h | 3 +++ 3 files changed, 75 insertions(+), 5 deletions(-)
[PATCH 2/2] ASoC: fsl-asoc-card: add support for generic I2S slave use-case
This commit implements support for generic codecs with the SoC acting as
I2S slave, by implementing the new `fsl,imx-audio-i2s-slave` compatible
and related properties.
This is particularly useful when using a Bluetooth controller acting as
I2S master, but other simple or dummy codecs might benefit from it too.
Signed-off-by: Arnaud Ferraris
---
sound/soc/fsl/fsl-asoc-card.c | 46 ++-
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 57ea1b072326..6076b963c873 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -166,12 +166,15 @@ static int fsl_asoc_card_hw_params(struct
snd_pcm_substream *substream,
return 0;
/* Specific configurations of DAIs starts from here */
- ret = snd_soc_dai_set_sysclk(asoc_rtd_to_cpu(rtd, 0),
cpu_priv->sysclk_id[tx],
-cpu_priv->sysclk_freq[tx],
-cpu_priv->sysclk_dir[tx]);
- if (ret && ret != -ENOTSUPP) {
- dev_err(dev, "failed to set sysclk for cpu dai\n");
- return ret;
+ if (cpu_priv->sysclk_freq[tx] > 0) {
+ ret = snd_soc_dai_set_sysclk(asoc_rtd_to_cpu(rtd, 0),
+cpu_priv->sysclk_id[tx],
+cpu_priv->sysclk_freq[tx],
+cpu_priv->sysclk_dir[tx]);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dev, "failed to set sysclk for cpu dai\n");
+ return ret;
+ }
}
if (cpu_priv->slot_width) {
@@ -475,11 +478,14 @@ static int fsl_asoc_card_late_probe(struct snd_soc_card
*card)
return 0;
}
- ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id,
-codec_priv->mclk_freq, SND_SOC_CLOCK_IN);
- if (ret && ret != -ENOTSUPP) {
- dev_err(dev, "failed to set sysclk in %s\n", __func__);
- return ret;
+ if (codec_priv->mclk_freq > 0) {
+ ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id,
+codec_priv->mclk_freq,
+SND_SOC_CLOCK_IN);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dev, "failed to set sysclk in %s\n", __func__);
+ return ret;
+ }
}
return 0;
@@ -620,6 +626,23 @@ static int fsl_asoc_card_probe(struct platform_device
*pdev)
priv->cpu_priv.slot_width = 32;
priv->card.dapm_routes = audio_map_tx;
priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
+ } else if (of_device_is_compatible(np, "fsl,imx-audio-i2s-slave")) {
+ ret = of_property_read_string(np, "audio-codec-dai-name",
+ &codec_dai_name);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get codec DAI name\n");
+ ret = -EINVAL;
+ goto asrc_fail;
+ }
+ ret = of_property_read_u32(np, "audio-slot-width",
+ &priv->cpu_priv.slot_width);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get slot width\n");
+ ret = -EINVAL;
+ goto asrc_fail;
+ }
+ priv->card.set_bias_level = NULL;
+ priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
} else {
dev_err(&pdev->dev, "unknown Device Tree compatible\n");
ret = -EINVAL;
@@ -763,6 +786,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
static const struct of_device_id fsl_asoc_card_dt_ids[] = {
{ .compatible = "fsl,imx-audio-ac97", },
+ { .compatible = "fsl,imx-audio-i2s-slave", },
{ .compatible = "fsl,imx-audio-cs42888", },
{ .compatible = "fsl,imx-audio-cs427x", },
{ .compatible = "fsl,imx-audio-sgtl5000", },
--
2.27.0
[PATCH 1/2] dt-bindings: sound: fsl-asoc-card: add new compatible for I2S slave
fsl-asoc-card currently doesn't support generic codecs with the SoC
acting as I2S slave.
This commit adds a new `fsl,imx-audio-i2s-slave` for this use-case, as
well as the following mandatory properties:
- `audio-codec-dai-name` for specifying the codec DAI to be used
- `audio-slot-width`
Signed-off-by: Arnaud Ferraris
---
.../bindings/sound/fsl-asoc-card.txt | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index 133d7e14a4d0..694a138df462 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -22,6 +22,8 @@ Note: The card is initially designed for those sound cards
who use AC'97, I2S
The compatible list for this generic sound card currently:
"fsl,imx-audio-ac97"
+ "fsl,imx-audio-i2s-slave"
+
"fsl,imx-audio-cs42888"
"fsl,imx-audio-cs427x"
@@ -75,7 +77,13 @@ Optional unless SSI is selected as a CPU DAI:
- mux-ext-port : The external port of the i.MX audio muxer
-Example:
+Optional unless compatible is "fsl,imx-audio-i2s-slave":
+
+ - audio-codec-dai-name: The name of the DAI provided by the codec
+
+ - audio-slot-width : The audio sample format
+
+Examples:
sound-cs42888 {
compatible = "fsl,imx-audio-cs42888";
model = "cs42888-audio";
@@ -96,3 +104,16 @@ sound-cs42888 {
"AIN2L", "Line In Jack",
"AIN2R", "Line In Jack";
};
+
+sound-bluetooth {
+ compatible = "fsl,imx-audio-i2s-slave";
+ audio-cpu = <&ssi1>;
+ audio-codec = <&codec_bluetooth>;
+ audio-codec-dai-name = "bt-sco-pcm-wb";
+ audio-slot-width = <16>;
+ audio-routing =
+ "RX", "Mic Jack",
+ "Headphone Jack", "TX";
+ mux-int-port = <1>;
+ mux-ext-port = <4>;
+};
--
2.27.0
[PATCH 0/2] ASoC: fsl-asoc-card: add support for generic codecs
fsl-asoc-card currently only works with AC97 or a selection of codecs, although the hardware is capable of more. Supporting generic codecs when acting as I2S slave (codec is master) would be useful, especially when using Bluetooth audio, as these are generally simple I2S devices not controlled by the sound subsystem. This will allow using simple/dummy codecs along with ASRC. Arnaud Ferraris (2): dt-bindings: sound: fsl-asoc-card: add new compatible for I2S slave ASoC: fsl-asoc-card: add support for generic I2S slave use-case Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 23 ++- sound/soc/fsl/fsl-asoc-card.c | 46 +++--- 2 files changed, 57 insertions(+), 12 deletions(-)
