ASoC: sun4i-i2s: Updates to the driver

PATCH Superseded [email protected] 3 participants 13 messages Aug 26, 2019 – Aug 27, 2019

Subsystems (3)

Sound ASoC Arm/Allwinner sunXi SoC support

lore

Share & tools

lore

[email protected]

author

2019-08-26 18:07 UTC #0 lore raw

v6 cover ASoC: sun4i-i2s: Updates to the driver

From: Marcus Cooper <[email protected]>

Hi All,

here is a patch series to add some improvements to the sun4i-i2s driver which is enough to get HDMI audio working on the A83T, A64, H3 and H5 platforms.

I've dropped a lot of the functionality that was presented earlier in favour of getting initial HDMI audio delivered. H6 and multi-channel HDMI will follow shortly.

My test branch for this can be found at
https://github.com/codekipper/linux-sunxi/commits/upstream-i2s , I've been using a Pine64 to test with; validating the new SoC block with HDMI audio and ensuring that I've not broken the old block by making sure that the audio codec still works.

BR,
CK

---
v6 changes compared to v5 are:
- removed patches for multi-channel and H6 HDMI audio.
- removed patch for 20, 24 and 32 bit (will push support for just 20 and 24bit)
- ditched tdm patches as support has already been added.
- added fix for A83T reg map.

v5 changes compared to v4 are:
- removed delivered patches.
- Added more details to commit messages.
- replaced some reg fields with function calls.
- Added DSP_A and DSP_B support for H3 and later SoCs.
- Added support for the Allwinner H6.

v4 changes compared to v3 are:
- Moved patches around so that the more controversial of patches are at the top of the stack.
- Added more details to commit messages.
- Fixed 20bit audio PCM format to use 4 bytes.
- Reduced number of flags used to indicate a new SoC.

v3 changes compared to v2 are:
- added back slave mode changes
- added back the use of tdm properties
- changes to regmap and caching
- removed loopback functionality
- fixes to the channel offset mask

v2 changes compared to v1 are:
- removed slave mode changes which didn't set mclk and bclk div.
- removed use of tdm and now use a dedicated property.
- fix commit message to better explain reason for sign extending
- add divider calculations for newer SoCs.
- add support for multi-lane i2s data output.
- add support for 20, 24 and 32 bit samples.
- add loopback property so blocks can be tested without a codec.

---

Marcus Cooper (3):
ASoC: sun4i-i2s: incorrect regmap for A83T
ASoC: sun4i-i2s: Add regmap field to sign extend sample
ASoC: sun4i-i2s: Adjust LRCLK width

 sound/soc/sunxi/sun4i-i2s.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Signature · 1 line

2.23.0

[email protected]

author

2019-08-26 18:07 UTC #1 lore raw

PATCH v6 1/3 ASoC: sun4i-i2s: incorrect regmap for A83T

From: Marcus Cooper <[email protected]>

The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.

Fixes 21faaea1343f ("ASoC: sun4i-i2s: Add support for A83T")

Signed-off-by Marcus Cooper <[email protected]>

---
 sound/soc/sunxi/sun4i-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/soc/sunxi/sun4i-i2s.c +1 -1

index 57bf2a33753e..34575a8aa9f6 100644
@@ -1100,7 +1100,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
 	.has_reset		= true,
 	.reg_offset_txdata	= SUN8I_I2S_FIFO_TX_REG,
-	.sun4i_i2s_regmap	= &sun4i_i2s_regmap_config,
+	.sun4i_i2s_regmap	= &sun8i_i2s_regmap_config,
 	.field_clkdiv_mclk_en	= REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
 	.field_fmt_wss		= REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
 	.field_fmt_sr		= REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),

Signature · 1 line

2.23.0

[email protected]

author

2019-08-26 18:07 UTC #7 lore raw

PATCH v6 2/3 ASoC: sun4i-i2s: Add regmap field to sign extend sample

In reply to [email protected]

From: Marcus Cooper <[email protected]>

On the newer SoCs such as the H3 and A64 this is set by default to transfer a 0 after each sample in each slot. However the A10 and A20 SoCs that this driver was developed on had a default setting where it padded the audio gain with zeros.

This isn't a problem whilst we have only support for 16bit audio but with larger sample resolution rates in the pipeline then SEXT bits should be cleared so that they also pad at the LSB. Without this the audio gets distorted.

Signed-off-by Marcus Cooper <[email protected]>

---
 sound/soc/sunxi/sun4i-i2s.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

sound/soc/sunxi/sun4i-i2s.c +17 -0

index 34575a8aa9f6..056a299c03fb 100644
@@ -135,6 +135,7 @@ struct sun4i_i2s;
  * @field_clkdiv_mclk_en: regmap field to enable mclk output.
  * @field_fmt_wss: regmap field to set word select size.
  * @field_fmt_sr: regmap field to set sample resolution.
+ * @field_fmt_sext: regmap field to set the sign extension.
  */
 struct sun4i_i2s_quirks {
 	bool				has_reset;

@@ -145,6 +146,7 @@ struct sun4i_i2s_quirks {
 	struct reg_field		field_clkdiv_mclk_en;
 	struct reg_field		field_fmt_wss;
 	struct reg_field		field_fmt_sr;
+	struct reg_field		field_fmt_sext;
 
 	const struct sun4i_i2s_clk_div	*bclk_dividers;
 	unsigned int			num_bclk_dividers;

@@ -177,6 +179,7 @@ struct sun4i_i2s {
 	struct regmap_field	*field_clkdiv_mclk_en;
 	struct regmap_field	*field_fmt_wss;
 	struct regmap_field	*field_fmt_sr;
+	struct regmap_field	*field_fmt_sext;
 
 	const struct sun4i_i2s_quirks	*variant;
 };

@@ -354,6 +357,10 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 
 	regmap_field_write(i2s->field_clkdiv_mclk_en, 1);
 
+
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_field_write(i2s->field_fmt_sext, 0);
+
 	return 0;
 }

@@ -1073,6 +1080,7 @@ static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
 	.mclk_dividers		= sun4i_i2s_mclk_div,
 	.num_mclk_dividers	= ARRAY_SIZE(sun4i_i2s_mclk_div),
 	.get_bclk_parent_rate	= sun4i_i2s_get_bclk_parent_rate,
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
 	.get_sr			= sun4i_i2s_get_sr,
 	.get_wss		= sun4i_i2s_get_wss,
 	.set_chan_cfg		= sun4i_i2s_set_chan_cfg,

@@ -1091,6 +1099,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 	.mclk_dividers		= sun4i_i2s_mclk_div,
 	.num_mclk_dividers	= ARRAY_SIZE(sun4i_i2s_mclk_div),
 	.get_bclk_parent_rate	= sun4i_i2s_get_bclk_parent_rate,
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
 	.get_sr			= sun4i_i2s_get_sr,
 	.get_wss		= sun4i_i2s_get_wss,
 	.set_chan_cfg		= sun4i_i2s_set_chan_cfg,

@@ -1109,6 +1118,7 @@ static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
 	.mclk_dividers		= sun8i_i2s_clk_div,
 	.num_mclk_dividers	= ARRAY_SIZE(sun8i_i2s_clk_div),
 	.get_bclk_parent_rate	= sun8i_i2s_get_bclk_parent_rate,
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 4, 5),
 	.get_sr			= sun8i_i2s_get_sr_wss,
 	.get_wss		= sun8i_i2s_get_sr_wss,
 	.set_chan_cfg		= sun8i_i2s_set_chan_cfg,

@@ -1127,6 +1137,7 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
 	.mclk_dividers		= sun4i_i2s_mclk_div,
 	.num_mclk_dividers	= ARRAY_SIZE(sun4i_i2s_mclk_div),
 	.get_bclk_parent_rate	= sun4i_i2s_get_bclk_parent_rate,
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
 	.get_sr			= sun4i_i2s_get_sr,
 	.get_wss		= sun4i_i2s_get_wss,
 	.set_chan_cfg		= sun4i_i2s_set_chan_cfg,

@@ -1154,6 +1165,12 @@ static int sun4i_i2s_init_regmap_fields(struct device *dev,
 	if (IS_ERR(i2s->field_fmt_sr))
 		return PTR_ERR(i2s->field_fmt_sr);
 
+	i2s->field_fmt_sext =
+			devm_regmap_field_alloc(dev, i2s->regmap,
+						i2s->variant->field_fmt_sext);
+	if (IS_ERR(i2s->field_fmt_sext))
+		return PTR_ERR(i2s->field_fmt_sext);
+
 	return 0;
 }

Signature · 1 line

2.23.0

[email protected]

author

2019-08-26 18:07 UTC #10 lore raw

PATCH v6 3/3 ASoC: sun4i-i2s: Adjust LRCLK width

In reply to [email protected]

From: Marcus Cooper <[email protected]>

Some codecs such as i2s based HDMI audio and the Pine64 DAC require a different amount of bit clocks per frame than what is calculated by the sample width. Use the values obtained by the tdm slot bindings to adjust the LRCLK width accordingly.

Signed-off-by Marcus Cooper <[email protected]>

---
 sound/soc/sunxi/sun4i-i2s.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

sound/soc/sunxi/sun4i-i2s.c +4 -1

index 056a299c03fb..0965a97c96e5 100644
@@ -455,7 +455,10 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 		break;
 
 	case SND_SOC_DAIFMT_I2S:
-		lrck_period = params_physical_width(params);
+		if (i2s->slot_width)
+			lrck_period = i2s->slot_width;
+		else
+			lrck_period = params_physical_width(params);
 		break;
 
 	default:

Signature · 1 line

2.23.0

Chen-Yu Tsai

2019-08-27 04:13 UTC #2 lore raw

Re: [linux-sunxi] [PATCH v6 1/3] ASoC: sun4i-i2s: incorrect regmap for A83T

In reply to [email protected]

Quoted 5 lines · On Tue, Aug 27, 2019 at 2:07 AM <[email protected]> wrote:

From: Marcus Cooper <[email protected]>
The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.

Looking at the code Allwinner previously released [1], that doesn't seem to be the case. Keep in mind that the register map shown in the user manual is for the TDM interface, which we don't actually support right now.

The file shows the base address as 0x01c22800, and the last defined register is SUNXI_RXCHMAP at 0x3c.

The I2S driver [2] also shows that it is the old register map size, but with TX_FIFO and INT_STA swapped around. This might mean that it would need a separate regmap_config, as the read/write callbacks need to be changed to fit the swapped registers.

Finally, the TDM driver [3], which matches the TDM section in the manual, shows a larger register map.

A83T is SUN8IW6, while SUN8IW7 refers to the H3.

ChenYu

[1] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/hdmiaudio/sunxi-hdmipcm.h
[2] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/i2s0/sunxi-i2s0.h
[3] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/daudio0/sunxi-daudio0.h

Quoted 26 lines

Fixes 21faaea1343f ("ASoC: sun4i-i2s: Add support for A83T")
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +1 -2
index 57bf2a33753e..34575a8aa9f6 100644
@@ -1100,7 +1100,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
        .has_reset              = true,
        .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
-       .sun4i_i2s_regmap       = &sun4i_i2s_regmap_config,
+       .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
        .field_clkdiv_mclk_en   = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
        .field_fmt_wss          = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
        .field_fmt_sr           = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
--
2.23.0
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190826180734.15801-2-codekipper%40gmail.com.

Code Kipper

author

2019-08-27 05:54 UTC #3 lore raw

Re: [linux-sunxi] [PATCH v6 1/3] ASoC: sun4i-i2s: incorrect regmap for A83T

Quoted 11 lines · Chen-Yu Tsai

Quoted 5 lines · On Tue, Aug 27, 2019 at 2:07 AM <[email protected]…
From: Marcus Cooper <[email protected]>
The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.
Looking at the code Allwinner previously released [1], that doesn't seem to be the case. Keep in mind that the register map shown in the user manual is for the TDM interface, which we don't actually support right now.

Should it matter what we support right now?, the block according to the user manual shows the bigger range. I don't have a A83T device and from what I gather not many users do. But the compatible for the H3 has been removed and replaced with the settings for the A83T which also has default settings in registers further up than SUNXI_RXCHMAP.

Quoted 9 lines

The file shows the base address as 0x01c22800, and the last defined register is SUNXI_RXCHMAP at 0x3c.
The I2S driver [2] also shows that it is the old register map size, but with TX_FIFO and INT_STA swapped around. This might mean that it would need a separate regmap_config, as the read/write callbacks need to be changed to fit the swapped registers.

Quoted 4 lines

Finally, the TDM driver [3], which matches the TDM section in the manual, shows a larger register map.
A83T is SUN8IW6, while SUN8IW7 refers to the H3.

Since when have we trusted Allwinner code?, the TDM labelled block clearly supports
I2S. The biggest use case for this block is getting HDMI audio working on the newer
devices(LibreELEC nightlies has a user base of over 300) and I've tested this on numerous set ups over the last couple of years.

Failing that reverting (3e9acd7ac693: "ASoC: sun4i-i2s: Remove duplicated quirks structure")
would help.

BR,
CK

Quoted 33 lines

ChenYu
[1] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/hdmiaudio/sunxi-hdmipcm.h
[2] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/i2s0/sunxi-i2s0.h
[3] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/daudio0/sunxi-daudio0.h
Quoted 26 lines
Fixes 21faaea1343f ("ASoC: sun4i-i2s: Add support for A83T")
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +1 -2
index 57bf2a33753e..34575a8aa9f6 100644
@@ -1100,7 +1100,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
        .has_reset              = true,
        .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
-       .sun4i_i2s_regmap       = &sun4i_i2s_regmap_config,
+       .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
        .field_clkdiv_mclk_en   = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
        .field_fmt_wss          = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
        .field_fmt_sr           = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
--
2.23.0
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190826180734.15801-2-codekipper%40gmail.com.

Maxime Ripard

2019-08-27 07:01 UTC #11 lore raw

Re: [PATCH v6 3/3] ASoC: sun4i-i2s: Adjust LRCLK width

In reply to [email protected]

Quoted 17 lines · On Mon, Aug 26, 2019 at 08:07:34PM +0200, [email protected] wrote:

From: Marcus Cooper <[email protected]>
Some codecs such as i2s based HDMI audio and the Pine64 DAC require a different amount of bit clocks per frame than what is calculated by the sample width. Use the values obtained by the tdm slot bindings to adjust the LRCLK width accordingly.
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +0 -0
index 056a299c03fb..0965a97c96e5 100644

sound/soc/sunxi/sun4i-i2s.c static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s, · line 462

Quoted 9 lines

sound/soc/sunxi/sun4i-i2s.c +4 -1

@@ -455,7 +455,10 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 		break;
 
 	case SND_SOC_DAIFMT_I2S:
-		lrck_period = params_physical_width(params);
+		if (i2s->slot_width)
+			lrck_period = i2s->slot_width;
+		else
+			lrck_period = params_physical_width(params);
 		break;

That would be the case with the DSP formats too, right?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Chen-Yu Tsai

2019-08-27 08:01 UTC #4 lore raw

Re: [linux-sunxi] [PATCH v6 1/3] ASoC: sun4i-i2s: incorrect regmap for A83T

In reply to Code Kipper

Quoted 10 lines · Code Kipper

Quoted 8 lines · Chen-Yu Tsai
Quoted 5 lines · On Tue, Aug 27, 2019 at 2:07 AM <[email protected]…
From: Marcus Cooper <[email protected]>
The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.

Quoted 6 lines

Quoted 3 lines
Looking at the code Allwinner previously released [1], that doesn't seem to be the case. Keep in mind that the register map shown in the user manual is for the TDM interface, which we don't actually support right now.
Should it matter what we support right now?, the block according to the user manual shows the bigger range. I don't have a A83T device and from what I

There are a total of four I2S controllers on the A83T. Currently three of them are listed in the dtsi file, which are _not_ the one shown in the user manual. The one shown is the fourth one, which is the TDM controller.

It's not like we haven't seen this before. IIRC the A64 also had two variants of the I2S interface. The one coupled with the audio codec was different from the others.

Quoted 3 lines

gather not many users do. But the compatible for the H3 has been removed and replaced with the settings for the A83T which also has default settings in registers further up than SUNXI_RXCHMAP.

I'll sync up with Maxime on this.

Quoted 9 lines

Quoted 9 lines
The file shows the base address as 0x01c22800, and the last defined register is SUNXI_RXCHMAP at 0x3c.
The I2S driver [2] also shows that it is the old register map size, but with TX_FIFO and INT_STA swapped around. This might mean that it would need a separate regmap_config, as the read/write callbacks need to be changed to fit the swapped registers.

Quoted 7 lines

Quoted 4 lines
Finally, the TDM driver [3], which matches the TDM section in the manual, shows a larger register map.
A83T is SUN8IW6, while SUN8IW7 refers to the H3.
Since when have we trusted Allwinner code?, the TDM labelled block clearly supports

Since they haven't listed the I2S block in the user manual, so that is what we have to go by.

The TDM section in the user manual only lists the block at 0x1c23000. The memory map says DAUDIO-[012] for addresses 0x1c22000, 0x1c22400, 0x1c22800, and TDM for address 0x1c23000. One would assume this meant these are somewhat different.

Quoted 2 lines

I2S. The biggest use case for this block is getting HDMI audio working on the newer

I understand that.

Quoted 2 lines

devices(LibreELEC nightlies has a user base of over 300) and I've tested this on numerous set ups over the last couple of years.

Tested on the H3, correct?

Quoted 3 lines

Failing that reverting (3e9acd7ac693: "ASoC: sun4i-i2s: Remove duplicated quirks structure")
would help.

I'll take a look. IIRC it worked with the old layout, with the two registers swapped, playing standard 48 KHz / 16 bit audio when I added supported for the A83T. Then again maybe the stars were perfectly aligned. At the very least we could separate A83T and H3 as you suggested.

ChenYu

Quoted 40 lines

BR,
CK
Quoted 29 lines
ChenYu
[1] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/hdmiaudio/sunxi-hdmipcm.h
[2] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/i2s0/sunxi-i2s0.h
[3] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/daudio0/sunxi-daudio0.h
Quoted 22 lines
Fixes 21faaea1343f ("ASoC: sun4i-i2s: Add support for A83T")
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +1 -2
index 57bf2a33753e..34575a8aa9f6 100644
@@ -1100,7 +1100,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
        .has_reset              = true,
        .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
-       .sun4i_i2s_regmap       = &sun4i_i2s_regmap_config,
+       .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
        .field_clkdiv_mclk_en   = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
        .field_fmt_wss          = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
        .field_fmt_sr           = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
--
2.23.0
Quoted 4 lines
Quoted 4 lines
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190826180734.15801-2-codekipper%40gmail.com.
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/CAEKpxBmCg4AkqKM-O3C76gto%2BmPWyEdDbviAmRJ8PxLOOMTJ7w%40mail.gmail.com.

Code Kipper

author

2019-08-27 08:35 UTC #5 lore raw

Re: [linux-sunxi] [PATCH v6 1/3] ASoC: sun4i-i2s: incorrect regmap for A83T

Quoted 16 lines · Chen-Yu Tsai

Quoted 14 lines · Code Kipper
Quoted 11 lines · Chen-Yu Tsai
Quoted 5 lines · On Tue, Aug 27, 2019 at 2:07 AM <[email protected]…
From: Marcus Cooper <[email protected]>
The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.
Looking at the code Allwinner previously released [1], that doesn't seem to be the case. Keep in mind that the register map shown in the user manual is for the TDM interface, which we don't actually support right now.

Quoted 6 lines

Quoted 2 lines
Should it matter what we support right now?, the block according to the user manual shows the bigger range. I don't have a A83T device and from what I
There are a total of four I2S controllers on the A83T. Currently three of them are listed in the dtsi file, which are _not_ the one shown in the user manual. The one shown is the fourth one, which is the TDM controller.

The configuration for the A83T suggests that it's a mixture of old and new which I don't
think is the case considering it was released around the same time as the H3. There
is enough similarity between the blocks for it to still work. For example on the H6
we referenced by mistake the H3 block and we still got audio (with only slight distortion).
I would suggest to validate all of the i2s blocks we need to test using the internal loopback
as that will also cover capture.

Quoted 4 lines

It's not like we haven't seen this before. IIRC the A64 also had two variants of the I2S interface. The one coupled with the audio codec was different from the others.

Yes...but the i2s of the audio codec was documented in the audio codec section. I've used
this device to ensure that I've not broken anything in the old block with these new changes.

Quoted 34 lines

Quoted 4 lines
gather not many users do. But the compatible for the H3 has been removed and replaced with the settings for the A83T which also has default settings in registers further up than SUNXI_RXCHMAP.
I'll sync up with Maxime on this.
Quoted 9 lines
Quoted 9 lines
The file shows the base address as 0x01c22800, and the last defined register is SUNXI_RXCHMAP at 0x3c.
The I2S driver [2] also shows that it is the old register map size, but with TX_FIFO and INT_STA swapped around. This might mean that it would need a separate regmap_config, as the read/write callbacks need to be changed to fit the swapped registers.
Quoted 7 lines
Quoted 4 lines
Finally, the TDM driver [3], which matches the TDM section in the manual, shows a larger register map.
A83T is SUN8IW6, while SUN8IW7 refers to the H3.
Since when have we trusted Allwinner code?, the TDM labelled block clearly supports
Since they haven't listed the I2S block in the user manual, so that is what we have to go by.
The TDM section in the user manual only lists the block at 0x1c23000. The memory map says DAUDIO-[012] for addresses 0x1c22000, 0x1c22400, 0x1c22800, and TDM for address 0x1c23000. One would assume this meant these are somewhat different.
Quoted 2 lines
I2S. The biggest use case for this block is getting HDMI audio working on the newer

Quoted 6 lines

I understand that.
Quoted 2 lines
devices(LibreELEC nightlies has a user base of over 300) and I've tested this on numerous set ups over the last couple of years.
Tested on the H3, correct?

Yes....but only with the additional changes for multi-channel with my LibreELEC build.
These changes I tested on my pine64 before pushing upstream.

Quoted 9 lines

Quoted 4 lines
Failing that reverting (3e9acd7ac693: "ASoC: sun4i-i2s: Remove duplicated quirks structure")
would help.
I'll take a look. IIRC it worked with the old layout, with the two registers swapped, playing standard 48 KHz / 16 bit audio when I added supported for the A83T. Then again maybe the stars were perfectly aligned. At the very least we could separate A83T and H3 as you suggested.

Thanks,
CK

Quoted 44 lines

ChenYu
Quoted 40 lines
BR,
CK
Quoted 29 lines
ChenYu
[1] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/hdmiaudio/sunxi-hdmipcm.h
[2] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/i2s0/sunxi-i2s0.h
[3] https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/daudio0/sunxi-daudio0.h
Quoted 22 lines
Fixes 21faaea1343f ("ASoC: sun4i-i2s: Add support for A83T")
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +1 -2
index 57bf2a33753e..34575a8aa9f6 100644
@@ -1100,7 +1100,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
        .has_reset              = true,
        .reg_offset_txdata      = SUN8I_I2S_FIFO_TX_REG,
-       .sun4i_i2s_regmap       = &sun4i_i2s_regmap_config,
+       .sun4i_i2s_regmap       = &sun8i_i2s_regmap_config,
        .field_clkdiv_mclk_en   = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
        .field_fmt_wss          = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 2),
        .field_fmt_sr           = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
--
2.23.0
Quoted 4 lines
Quoted 4 lines
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190826180734.15801-2-codekipper%40gmail.com.
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/CAEKpxBmCg4AkqKM-O3C76gto%2BmPWyEdDbviAmRJ8PxLOOMTJ7w%40mail.gmail.com.

Code Kipper

author

2019-08-27 08:36 UTC #12 lore raw

Re: [PATCH v6 3/3] ASoC: sun4i-i2s: Adjust LRCLK width

In reply to Maxime Ripard

Quoted 21 lines · Maxime Ripard

Quoted 19 lines · On Mon, Aug 26, 2019 at 08:07:34PM +0200…
From: Marcus Cooper <[email protected]>
Some codecs such as i2s based HDMI audio and the Pine64 DAC require a different amount of bit clocks per frame than what is calculated by the sample width. Use the values obtained by the tdm slot bindings to adjust the LRCLK width accordingly.
Signed-off-by Marcus Cooper <[email protected]>
---
 sound/soc/sunxi/sun4i-i2s.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
sound/soc/sunxi/sun4i-i2s.c +0 -0
index 056a299c03fb..0965a97c96e5 100644
@@ -455,7 +455,10 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
              break;
 

Quoted 9 lines

sound/soc/sunxi/sun4i-i2s.c static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s, · line 462
Quoted 7 lines
sound/soc/sunxi/sun4i-i2s.c +4 -1
      case SND_SOC_DAIFMT_I2S:
-             lrck_period = params_physical_width(params);
+             if (i2s->slot_width)
+                     lrck_period = i2s->slot_width;
+             else
+                     lrck_period = params_physical_width(params);
              break;
That would be the case with the DSP formats too, right?

Maybe....but I need a TDM test volunteer!,
CK

Quoted 7 lines

Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Maxime Ripard

2019-08-27 09:34 UTC #8 lore raw

Re: [PATCH v6 2/3] ASoC: sun4i-i2s: Add regmap field to sign extend sample

In reply to [email protected]

Quoted 7 lines · On Mon, Aug 26, 2019 at 08:07:33PM +0200, [email protected] wrote:

From: Marcus Cooper <[email protected]>
On the newer SoCs such as the H3 and A64 this is set by default to transfer a 0 after each sample in each slot. However the A10 and A20 SoCs that this driver was developed on had a default setting where it padded the audio gain with zeros.

Quoted 6 lines

This isn't a problem whilst we have only support for 16bit audio but with larger sample resolution rates in the pipeline then SEXT bits should be cleared so that they also pad at the LSB. Without this the audio gets distorted.
Signed-off-by Marcus Cooper <[email protected]>

If anything, I'd like to have less regmap_fields rather than more of them. This is pretty easy to add to one of the callbacks, especially since the field itself has been completely reworked from one generation to the other.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Chen-Yu Tsai

2019-08-27 10:28 UTC #6 lore raw

Re: [linux-sunxi] [PATCH v6 1/3] ASoC: sun4i-i2s: incorrect regmap for A83T

In reply to Code Kipper

Quoted 25 lines · Code Kipper

Quoted 16 lines · Chen-Yu Tsai
Quoted 14 lines · Code Kipper
Quoted 11 lines · Chen-Yu Tsai
Quoted 5 lines · On Tue, Aug 27, 2019 at 2:07 AM <[email protected]> wrote:
From: Marcus Cooper <[email protected]>
The regmap configuration is set up for the legacy block on the A83T whereas it uses the new block with a larger register map.
Looking at the code Allwinner previously released [1], that doesn't seem to be the case. Keep in mind that the register map shown in the user manual is for the TDM interface, which we don't actually support right now.
Quoted 6 lines
Quoted 2 lines
Should it matter what we support right now?, the block according to the user manual shows the bigger range. I don't have a A83T device and from what I
There are a total of four I2S controllers on the A83T. Currently three of them are listed in the dtsi file, which are _not_ the one shown in the user manual. The one shown is the fourth one, which is the TDM controller.

Quoted 8 lines

The configuration for the A83T suggests that it's a mixture of old and new which I don't
think is the case considering it was released around the same time as the H3. There
is enough similarity between the blocks for it to still work. For example on the H6
we referenced by mistake the H3 block and we still got audio (with only slight distortion).

The difference with the A83T here is large enough that if you play anything it will simply stall. I already reported it as broken and Maxime has sent fixes.

Quoted 50 lines

I would suggest to validate all of the i2s blocks we need to test using the internal loopback
as that will also cover capture.
Quoted 4 lines
It's not like we haven't seen this before. IIRC the A64 also had two variants of the I2S interface. The one coupled with the audio codec was different from the others.
Yes...but the i2s of the audio codec was documented in the audio codec section. I've used
this device to ensure that I've not broken anything in the old block with these new changes.
Quoted 36 lines
Quoted 4 lines
gather not many users do. But the compatible for the H3 has been removed and replaced with the settings for the A83T which also has default settings in registers further up than SUNXI_RXCHMAP.
I'll sync up with Maxime on this.
Quoted 9 lines
Quoted 9 lines
The file shows the base address as 0x01c22800, and the last defined register is SUNXI_RXCHMAP at 0x3c.
The I2S driver [2] also shows that it is the old register map size, but with TX_FIFO and INT_STA swapped around. This might mean that it would need a separate regmap_config, as the read/write callbacks need to be changed to fit the swapped registers.
Quoted 7 lines
Quoted 4 lines
Finally, the TDM driver [3], which matches the TDM section in the manual, shows a larger register map.
A83T is SUN8IW6, while SUN8IW7 refers to the H3.
Since when have we trusted Allwinner code?, the TDM labelled block clearly supports
Since they haven't listed the I2S block in the user manual, so that is what we have to go by.
The TDM section in the user manual only lists the block at 0x1c23000. The memory map says DAUDIO-[012] for addresses 0x1c22000, 0x1c22400, 0x1c22800, and TDM for address 0x1c23000. One would assume this meant these are somewhat different.
Quoted 2 lines
I2S. The biggest use case for this block is getting HDMI audio working on the newer
I understand that.

Quoted 18 lines

Quoted 4 lines
Quoted 2 lines
devices(LibreELEC nightlies has a user base of over 300) and I've tested this on numerous set ups over the last couple of years.
Tested on the H3, correct?
Yes....but only with the additional changes for multi-channel with my LibreELEC build.
These changes I tested on my pine64 before pushing upstream.
Quoted 9 lines
Quoted 4 lines
Failing that reverting (3e9acd7ac693: "ASoC: sun4i-i2s: Remove duplicated quirks structure")
would help.
I'll take a look. IIRC it worked with the old layout, with the two registers swapped, playing standard 48 KHz / 16 bit audio when I added supported for the A83T. Then again maybe the stars were perfectly aligned. At the very least we could separate A83T and H3 as you suggested.

Maxime has sent a patch reverting the merger.

ChenYu

Code Kipper

author

2019-08-27 10:55 UTC #9 lore raw

Re: [PATCH v6 2/3] ASoC: sun4i-i2s: Add regmap field to sign extend sample

In reply to Maxime Ripard

Quoted 14 lines · Maxime Ripard

Quoted 12 lines · On Mon, Aug 26, 2019 at 08:07:33PM +0200…
From: Marcus Cooper <[email protected]>
On the newer SoCs such as the H3 and A64 this is set by default to transfer a 0 after each sample in each slot. However the A10 and A20 SoCs that this driver was developed on had a default setting where it padded the audio gain with zeros.
This isn't a problem whilst we have only support for 16bit audio but with larger sample resolution rates in the pipeline then SEXT bits should be cleared so that they also pad at the LSB. Without this the audio gets distorted.

Quoted 7 lines

Quoted 1 line
Signed-off-by Marcus Cooper <[email protected]>
If anything, I'd like to have less regmap_fields rather than more of them. This is pretty easy to add to one of the callbacks, especially since the field itself has been completely reworked from one generation to the other.

ACK
That's fine....I've been doing that with the patches which follow this. CK

Quoted 6 lines

Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com