Hi there,
A while back I put out this thread:
How do I set up multiple codecs on one I2S - without TDM?"
https://lore.kernel.org/all/ZMBRMnv0GQF4wyfQ@titan/
This is my specific use case and motivation for this patch series.
I posted how I managed to configure the audio-graph-card2 to allow
doing this in conjunction with some register hacking, but I now have
an initial patch series to support this in the sun4i-i2s driver.
Now I've tested this on the T113-s3 with 3 wm8782s and it works well,
but I'm marking this as RFC as I think I'm out of my depth here and
I'd like to know the direction I need to take this: I'm new to
kernel development and I'm genuinely unsure what are best practices
and which are bad practices design-wise.
My main concerns are the following:
First, I split up channel-dins and channel-slots. This is mainly
because I implemented one first but both of them only make sense
together. The registers themselves use a format of a byte per
channel with the upper nibble being the din and the lower being
the slot. Perhaps this is a better format to copy?
Second, I use u8 arrays on the device tree. This is done so I can
just read the array easily, but it also means I can't make this
property contingent on a compatible string in the schema as $ref
doesn't seem to be allowed there.
Third, channel-slots is available on all sun4i-i2s controllers,
but I only have it implemented on the R329 variant for now when
there are multiple din pins.
I could add support for this on older controllers but there's not
really a use case for manual configuration as there's no DIN
and I don't have hardware to test it on.
Fourth, I don't limit the readable channels to the channels
listed. Reading more channels than you have currently results in
the controller assuming you want to use TDM, but that's not the
case here and you can have strange duplicate channels show up.
Fifth, it might be a good idea to increase the maximum channels
from 8 to 16, especially if people are going to be running
multiple TDM streams on one controller.
Sixth, the channel-slots only apply to capture, not playback.
This is something I just realized now when writing and forgot
to document in the patch set.
Thanks for your time,
John.
John Watts (7):
ASoC: sunxi: sun4i-i2s: Prepare for runtime DIN pin selection
ASoC: sunxi: sun4i-i2s: Use channel-dins device tree property
ASoC: sunxi: sun4i-i2s: Prepare for runtime channel slot selection
ASoC: sunxi: sun4i-i2s: Use channel-slots device tree property
ASoC: sunxi: sun4i-i2s: Detect TDM slots based on channel slots
dt-bindings: sound: sun4i-i2s: Add channel-dins property
dt-bindings: sound: sun4i-i2s: Add channel-slots property
.../sound/allwinner,sun4i-a10-i2s.yaml | 30 +++++
sound/soc/sunxi/sun4i-i2s.c | 106 +++++++++++++++++-
2 files changed, 132 insertions(+), 4 deletions(-)
--
2.41.0
The sun4i I2S controller supports mapping channels to specific TDM
slots. This is usually done in a 1:1 mapping because there's not
really a reason to re-order the TDM slots in software.
On the R329 and D1 multiplexing can be done using pins as well as
TDM. Here's an example of using 3 pins and 2 slots (left and right):
Channel 0 -> DIN0 -> ADC0, TDM slot 0
Channel 1 -> DIN0 -> ADC0, TDM slot 1
Channel 2 -> DIN1 -> ADC1, TDM slot 0
Channel 3 -> DIN1 -> ADC1, TDM slot 1
Channel 4 -> DIN2 -> ADC2, TDM slot 0
Channel 5 -> DIN2 -> ADC2, TDM slot 1
This connects 3 separate TDM-unaware ADCs to the I2S controller.
Likewise, multiple TDM slots could be used to run two sets of
TDM-aware ADCs on one I2S controller.
Prepare for configurable slot selection by adding a channel to slot
mapping array and using that in the R329 code if we multiple DIN pins.
Signed-off-by: John Watts <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index cf66f21646a4..627bf319e1cc 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -217,6 +217,7 @@ struct sun4i_i2s {
unsigned int slots;
unsigned int slot_width;
u8 channel_dins[16];
+ u8 channel_slots[16];
struct snd_dmaengine_dai_dma_data capture_dma_data;
struct snd_dmaengine_dai_dma_data playback_dma_data;
@@ -266,6 +267,17 @@ static int sun4i_i2s_read_channel_dins(struct device *dev, struct sun4i_i2s *i2s
return 0;
}
+static int sun4i_i2s_read_channel_slots(struct device *dev, struct sun4i_i2s *i2s)
+{
+ int max_channels = ARRAY_SIZE(i2s->channel_dins);
+
+ /* Use a 1:1 mapping by default */
+ for (int i = 0; i < max_channels; ++i)
+ i2s->channel_slots[i] = i;
+
+ return 0;
+}
+
static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
{ .div = 2, .val = 0 },
{ .div = 4, .val = 1 },
@@ -570,7 +582,7 @@ static void sun50i_h6_write_channel_map(const struct sun4i_i2s *i2s,
for (int i = 3; i >= 0; i--) {
int channel = channel_start + i;
u8 din = i2s->channel_dins[channel];
- u8 slot = channel; /* Map slot to channel */
+ u8 slot = i2s->channel_slots[channel];
reg_value <<= 8;
reg_value |= (din << 4) | slot;
@@ -1616,6 +1628,11 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
return -EINVAL;
}
+ if (sun4i_i2s_read_channel_slots(&pdev->dev, i2s)) {
+ dev_err(&pdev->dev, "Invalid channel slots\n");
+ return -EINVAL;
+ }
+
i2s->playback_dma_data.addr = res->start +
i2s->variant->reg_offset_txdata;
i2s->playback_dma_data.maxburst = 8;
--
2.41.0
The R329 variant of the sun4i I2S controller supports multiple
data input pins (din pins) for receiving data. Each channel can have
its data input pin configured.
Allow this to be configured using a new channel-dins property.
Signed-off-by: John Watts <[email protected]>
---
.../sound/allwinner,sun4i-a10-i2s.yaml | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 739114fb6549..402549f9941c 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
@@ -52,6 +52,13 @@ properties:
- const: apb
- const: mod
+ channel-dins:
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ description:
+ This is a list of DIN pin numbers, each used for a receiving I2S
+ channel. Pins are mapped to channels based on array index.
+ Channel 0 is the first number, then channel 1, and so on.
+
# Even though it only applies to subschemas under the conditionals,
# not listing them here will trigger a warning because of the
# additionalsProperties set to false.
@@ -144,4 +151,19 @@ examples:
dma-names = "rx", "tx";
};
+ - |
+ i2s0_d1: i2s@2032000 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun20i-d1-i2s",
+ "allwinner,sun50i-r329-i2s";
+ reg = <0x2032000 0x1000>;
+ interrupts = <0 26 0>;
+ clocks = <&ccu 86>, <&ccu 82>;
+ clock-names = "apb", "mod";
+ resets = <&ccu 34>;
+ dmas = <&dma 3>, <&dma 3>;
+ dma-names = "rx", "tx";
+ channel-dins = /bits/ 8 <0 0 1 1 2 2>;
+ };
+
...
--
2.41.0
The current controller code assumes a 1:1 relationship between audio
channel and TDM slot. This may not be the case when slots are set
explicitly. Instead figure out how many slots we need based on the
number of slots used in the channel map.
This allows the case of reading multiple data pins on a single slot.
Signed-off-by: John Watts <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 019a4856c90b..6347aaaed016 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -271,6 +271,7 @@ static int sun4i_i2s_read_channel_slots(struct device *dev, struct sun4i_i2s *i2
{
struct device_node *np = dev->of_node;
int max_channels = ARRAY_SIZE(i2s->channel_dins);
+ int slot_max;
int ret;
/* Use a 1:1 mapping by default */
@@ -290,6 +291,16 @@ static int sun4i_i2s_read_channel_slots(struct device *dev, struct sun4i_i2s *i2
if (ret < 0)
return ret;
+ for (int i = 0; i < ret; ++i) {
+ int slot = i2s->channel_slots[i];
+
+ if (slot_max < slot)
+ slot_max = slot;
+ }
+
+ /* Add 1 to be inclusive of slot 0 */
+ i2s->slots = slot_max + 1;
+
return 0;
}
--
2.41.0
On Sat, Aug 12, 2023 at 06:13:59AM +1000, John Watts wrote:
> First, I split up channel-dins and channel-slots. This is mainly
> because I implemented one first but both of them only make sense
> together. The registers themselves use a format of a byte per
> channel with the upper nibble being the din and the lower being
> the slot. Perhaps this is a better format to copy?
I think this is fine.
> Third, channel-slots is available on all sun4i-i2s controllers,
> but I only have it implemented on the R329 variant for now when
> there are multiple din pins.
> I could add support for this on older controllers but there's not
> really a use case for manual configuration as there's no DIN
> and I don't have hardware to test it on.
It's fine to leave this for someone who cares about that hardware to
implement, might be nice to add a warning if the properties are set but
not supported but it's not essential.
> Fourth, I don't limit the readable channels to the channels
> listed. Reading more channels than you have currently results in
> the controller assuming you want to use TDM, but that's not the
> case here and you can have strange duplicate channels show up.
It would be better to have constraints which prevent userspace doing the
wrong thing here.
> Fifth, it might be a good idea to increase the maximum channels
> from 8 to 16, especially if people are going to be running
> multiple TDM streams on one controller.
If there's no reason not to...