[PATCH V2 0/2] ASoC: audio_graph_card2: Support variable slot widths
This adds support for I2S/TDM links where the slot width varies depending on the sample width, in a way that cannot be guessed by component hw_params().
A typical example is:
- 16-bit samples use 16-bit slots - 24-bit samples use 32-bit slots
There is no way for a component hw_params() to deduce from the information it is passed that 24-bit samples will be in 32-bit slots.
Some audio hardware cannot support a fixed slot width or a slot width equal to the sample width in all cases. This is usually due either to limitations of the audio serial port or system clocking restrictions.
These two patches add support for defining a mapping between sample widths and sample slots. This allows audio_graph_card2 to be used in these situations instead of having to write a custom machine driver.
Richard Fitzgerald (2): ASoC: dt-bindings: audio-graph-port: Add dai-tdm-slot-width-map ASoC: audio_graph_card2: Add support for variable slot widths
.../bindings/sound/audio-graph-port.yaml | 7 ++ include/sound/simple_card_utils.h | 10 ++ sound/soc/generic/audio-graph-card2.c | 4 + sound/soc/generic/simple-card-utils.c | 93 +++++++++++++++++++ 4 files changed, 114 insertions(+)
Some audio hardware cannot support a fixed slot width or a slot width equal to the sample width in all cases. This is usually due either to limitations of the audio serial port or system clocking restrictions.
This property allows setting a mapping of sample widths and the corresponding tdm slot widths.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- .../devicetree/bindings/sound/audio-graph-port.yaml | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml index 476dcb49ece6..420adad49382 100644 --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml @@ -71,4 +71,11 @@ patternProperties: description: CPU to Codec rate channels. $ref: /schemas/types.yaml#/definitions/uint32
+ dai-tdm-slot-width-map: + description: Mapping of sample widths to slot widths. For hardware that + cannot support a fixed slot width or a slot width equal to sample + width. An array containing one or more pairs of values. Each pair + of values is a sample_width and the corresponding slot_width. + $ref: /schemas/types.yaml#/definitions/uint32-array + additionalProperties: true
On Thu, Feb 17, 2022 at 01:48:34PM +0000, Richard Fitzgerald wrote:
Some audio hardware cannot support a fixed slot width or a slot width equal to the sample width in all cases. This is usually due either to limitations of the audio serial port or system clocking restrictions.
This property allows setting a mapping of sample widths and the corresponding tdm slot widths.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com
.../devicetree/bindings/sound/audio-graph-port.yaml | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml index 476dcb49ece6..420adad49382 100644 --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml @@ -71,4 +71,11 @@ patternProperties: description: CPU to Codec rate channels. $ref: /schemas/types.yaml#/definitions/uint32
dai-tdm-slot-width-map:description: Mapping of sample widths to slot widths. For hardware thatcannot support a fixed slot width or a slot width equal to sample
A variable slot width sounds like a feature, not a limitation.
width. An array containing one or more pairs of values. Each pairof values is a sample_width and the corresponding slot_width.
That sounds like a matrix, not an array. N entries of 2 cells each.
$ref: /schemas/types.yaml#/definitions/uint32-array
I'd think there are some constraints on the values? Slots should be at least 8 bits, right? A max of 2x32 bits or is there more than stereo within a slot? In any case, it's for sure no where near 2^32 max.
Is there a need for specifying where in the slot the data is?
Rob
On 24/02/2022 22:10, Rob Herring wrote:
On Thu, Feb 17, 2022 at 01:48:34PM +0000, Richard Fitzgerald wrote:
Some audio hardware cannot support a fixed slot width or a slot width equal to the sample width in all cases. This is usually due either to limitations of the audio serial port or system clocking restrictions.
This property allows setting a mapping of sample widths and the corresponding tdm slot widths.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com
.../devicetree/bindings/sound/audio-graph-port.yaml | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml index 476dcb49ece6..420adad49382 100644 --- a/Documentation/devicetree/bindings/sound/audio-graph-port.yaml +++ b/Documentation/devicetree/bindings/sound/audio-graph-port.yaml @@ -71,4 +71,11 @@ patternProperties: description: CPU to Codec rate channels. $ref: /schemas/types.yaml#/definitions/uint32
dai-tdm-slot-width-map:description: Mapping of sample widths to slot widths. For hardware thatcannot support a fixed slot width or a slot width equal to sampleA variable slot width sounds like a feature, not a limitation.
Depends on point of view. Most interfaces allow setting a fixed slot width but in some cases that's not possible so it is more likely to be seen as a limitation. It is however a feature in the sense that it can avoid using higher frequencies that are necessary.
width. An array containing one or more pairs of values. Each pairof values is a sample_width and the corresponding slot_width.That sounds like a matrix, not an array. N entries of 2 cells each.
$ref: /schemas/types.yaml#/definitions/uint32-arrayI'd think there are some constraints on the values? Slots should be at least 8 bits, right? A max of 2x32 bits or is there more
True. I didn't think it was appropriate for a generic binding to enforce a range when that depends on the exact hardware. But if you want I can enforce a range that's likely to be true for all hardware.
than stereo within a slot? In any case, it's for sure no where near 2^32 max.
One sample per slot.
Is there a need for specifying where in the slot the data is?
I don't believe so, all the protocols I know of have the data bits transmitted first followed by padding. There's no harm adding a reserved field to allow for this info if it is ever needed, but it would be unused at present as there's no kernel API to do this.
Rob
On Fri, Feb 25, 2022 at 12:08:00PM +0000, Richard Fitzgerald wrote:
On 24/02/2022 22:10, Rob Herring wrote:
Is there a need for specifying where in the slot the data is?
I don't believe so, all the protocols I know of have the data bits transmitted first followed by padding. There's no harm adding a reserved field to allow for this info if it is ever needed, but it would be unused at present as there's no kernel API to do this.
It's part of the general format for the bus I'd say.
Some audio hardware cannot support a fixed slot width or a slot width equal to the sample width in all cases. This is usually due either to limitations of the audio serial port or system clocking restrictions. A typical example would be:
- 16-bit samples in 16-bit slots - 24-bit samples in 32-bit slots
The new dai-tdm-slot-width-map property allows setting a mapping of sample widths and the corresponding tdm slot widths. The content is an array of integers. Each pair of values are the sample_width and the corresponding slot width.
The property is added to each endpoint node that needs the restriction.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- include/sound/simple_card_utils.h | 10 +++ sound/soc/generic/audio-graph-card2.c | 4 ++ sound/soc/generic/simple-card-utils.c | 93 +++++++++++++++++++++++++++ 3 files changed, 107 insertions(+)
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index 5ee269c59aac..ee0374b0c7d1 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -16,6 +16,11 @@ #define asoc_simple_init_mic(card, sjack, prefix) \ asoc_simple_init_jack(card, sjack, 0, prefix, NULL)
+struct asoc_simple_tdm_width_map { + int sample_bits; + int slot_width; +}; + struct asoc_simple_dai { const char *name; unsigned int sysclk; @@ -26,6 +31,8 @@ struct asoc_simple_dai { unsigned int rx_slot_mask; struct clk *clk; bool clk_fixed; + struct asoc_simple_tdm_width_map *tdm_width_map; + int n_tdm_widths; };
struct asoc_simple_data { @@ -132,6 +139,9 @@ int asoc_simple_parse_daifmt(struct device *dev, struct device_node *codec, char *prefix, unsigned int *retfmt); +int asoc_simple_parse_tdm_width_map(struct device *dev, struct device_node *np, + struct asoc_simple_dai *dai); + __printf(3, 4) int asoc_simple_set_dailink_name(struct device *dev, struct snd_soc_dai_link *dai_link, diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c index c3947347dda3..c0f3907a01fd 100644 --- a/sound/soc/generic/audio-graph-card2.c +++ b/sound/soc/generic/audio-graph-card2.c @@ -503,6 +503,10 @@ static int __graph_parse_node(struct asoc_simple_priv *priv, if (ret < 0) return ret;
+ ret = asoc_simple_parse_tdm_width_map(dev, ep, dai); + if (ret < 0) + return ret; + ret = asoc_simple_parse_clk(dev, ep, dai, dlc); if (ret < 0) return ret; diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index a4babfb63175..8ff7966b7632 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -12,6 +12,7 @@ #include <linux/of_gpio.h> #include <linux/of_graph.h> #include <sound/jack.h> +#include <sound/pcm_params.h> #include <sound/simple_card_utils.h>
void asoc_simple_convert_fixup(struct asoc_simple_data *data, @@ -87,6 +88,49 @@ int asoc_simple_parse_daifmt(struct device *dev, } EXPORT_SYMBOL_GPL(asoc_simple_parse_daifmt);
+int asoc_simple_parse_tdm_width_map(struct device *dev, struct device_node *np, + struct asoc_simple_dai *dai) +{ + u32 *array_values; + int n, i, ret; + + if (!of_property_read_bool(np, "dai-tdm-slot-width-map")) + return 0; + + n = of_property_count_elems_of_size(np, "dai-tdm-slot-width-map", sizeof(u32)); + if (n % 1) { + dev_err(dev, "Invalid number of cells for dai-tdm-slot-width-map\n"); + return -EINVAL; + } + + dai->tdm_width_map = devm_kcalloc(dev, n, sizeof(*dai->tdm_width_map), GFP_KERNEL); + if (!dai->tdm_width_map) + return -ENOMEM; + + array_values = kcalloc(n, sizeof(*array_values), GFP_KERNEL); + if (!array_values) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "dai-tdm-slot-width-map", array_values, n); + if (ret < 0) { + dev_err(dev, "Could not read dai-tdm-slot-width-map: %d\n", ret); + goto out; + } + + for (i = 0; i < n; i += 2) { + dai->tdm_width_map[i / 2].sample_bits = array_values[i]; + dai->tdm_width_map[i / 2].slot_width = array_values[i + 1]; + } + + dai->n_tdm_widths = n / 2; + ret = 0; +out: + kfree(array_values); + + return ret; +} +EXPORT_SYMBOL_GPL(asoc_simple_parse_tdm_width_map); + int asoc_simple_set_dailink_name(struct device *dev, struct snd_soc_dai_link *dai_link, const char *fmt, ...) @@ -309,6 +353,40 @@ static int asoc_simple_set_clk_rate(struct device *dev, return clk_set_rate(simple_dai->clk, rate); }
+static int asoc_simple_set_tdm(struct snd_soc_dai *dai, + struct asoc_simple_dai *simple_dai, + struct snd_pcm_hw_params *params) +{ + int slot_width = params_width(params); + int sample_bits = params_width(params); + int i, ret; + + if (!simple_dai || !simple_dai->tdm_width_map) + return 0; + + if (simple_dai->slot_width) + slot_width = simple_dai->slot_width; + + for (i = 0; i < simple_dai->n_tdm_widths; ++i) { + if (simple_dai->tdm_width_map[i].sample_bits == sample_bits) { + slot_width = simple_dai->tdm_width_map[i].slot_width; + break; + } + } + + ret = snd_soc_dai_set_tdm_slot(dai, + simple_dai->tx_slot_mask, + simple_dai->rx_slot_mask, + simple_dai->slots, + slot_width); + if (ret && ret != -ENOTSUPP) { + dev_err(dai->dev, "simple-card: set_tdm_slot error: %d\n", ret); + return ret; + } + + return 0; +} + int asoc_simple_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -362,6 +440,21 @@ int asoc_simple_hw_params(struct snd_pcm_substream *substream, return ret; } } + + for_each_prop_dai_codec(props, i, pdai) { + sdai = asoc_rtd_to_codec(rtd, i); + ret = asoc_simple_set_tdm(sdai, pdai, params); + if (ret < 0) + return ret; + } + + for_each_prop_dai_cpu(props, i, pdai) { + sdai = asoc_rtd_to_cpu(rtd, i); + ret = asoc_simple_set_tdm(sdai, pdai, params); + if (ret < 0) + return ret; + } + return 0; } EXPORT_SYMBOL_GPL(asoc_simple_hw_params);
On Thu, 17 Feb 2022 13:48:33 +0000, Richard Fitzgerald wrote:
This adds support for I2S/TDM links where the slot width varies depending on the sample width, in a way that cannot be guessed by component hw_params().
A typical example is:
- 16-bit samples use 16-bit slots
- 24-bit samples use 32-bit slots
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: dt-bindings: audio-graph-port: Add dai-tdm-slot-width-map commit: 26e5366dd30569a469e1a87998b866b814deccf8 [2/2] ASoC: audio_graph_card2: Add support for variable slot widths commit: 1e974e5b82b3d75069b50445cd248cee0199654e
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
participants (3)
-
Mark Brown -
Richard Fitzgerald -
Rob Herring