Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/7] Add H6 I2S support
@ 2020-04-26 10:41 Clément Péron
  2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, Clément Péron, linux-kernel,
	linux-arm-kernel

Hi,

This is a sequel of Marcus Cooper serie[0], where remarks made by Maxime
have been fixed.

I have tested it on my Beelink GS1 board.

Thanks,
Clement

0: https://lore.kernel.org/patchwork/cover/1139949/

Changes since v2 (thanks Maxime):
 - Add details in commit log about sign extend sample
 - Only set FIFO regs as volatile in regmap
 - Missing a space (detected by checkpatch)

Changes since v1:
 - Fix missing header in set sign extend sample

Jernej Skrabec (3):
  dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
  ASoC: sun4i-i2s: Add support for H6 I2S
  arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI

Marcus Cooper (4):
  ASoC: sun4i-i2s: Adjust LRCLK width
  ASoC: sun4i-i2s: Set sign extend sample
  ASoc: sun4i-i2s: Add 20 and 24 bit support
  ASoC: sun4i-i2s: Adjust regmap settings

 .../sound/allwinner,sun4i-a10-i2s.yaml        |   2 +
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  31 ++
 sound/soc/sunxi/sun4i-i2s.c                   | 265 +++++++++++++++++-
 3 files changed, 296 insertions(+), 2 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-28  8:02   ` Maxime Ripard
  2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, linux-kernel, Marcus Cooper,
	Clément Péron, linux-arm-kernel

From: Marcus Cooper <codekipper@gmail.com>

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 <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index d0a8d5810c0a..4198a5410bf9 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -455,6 +455,9 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 		return -EINVAL;
 	}
 
+	if (i2s->slot_width)
+		lrck_period = i2s->slot_width;
+
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
 			   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
 			   SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
  2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-28  8:08   ` Maxime Ripard
  2020-05-11 22:26   ` Rob Herring
  2020-04-26 10:41 ` [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S Clément Péron
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, Jernej Skrabec, linux-kernel,
	Marcus Cooper, Clément Péron, linux-arm-kernel

From: Jernej Skrabec <jernej.skrabec@siol.net>

H6 I2S is very similar to H3, except that it supports up to 16 channels
and thus few registers have fields on different position.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml      | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
index 112ae00d63c1..606ad2d884a8 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml
@@ -24,6 +24,7 @@ properties:
       - items:
           - const: allwinner,sun50i-a64-i2s
           - const: allwinner,sun8i-h3-i2s
+      - const: allwinner,sun50i-h6-i2s
 
   reg:
     maxItems: 1
@@ -59,6 +60,7 @@ allOf:
               - allwinner,sun8i-a83t-i2s
               - allwinner,sun8i-h3-i2s
               - allwinner,sun50i-a64-codec-i2s
+              - allwinner,sun50i-h6-i2s
 
     then:
       required:
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
  2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
  2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-28  8:13   ` Maxime Ripard
  2020-04-26 10:41 ` [PATCH v3 4/7] ASoC: sun4i-i2s: Set sign extend sample Clément Péron
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, Jernej Skrabec, linux-kernel,
	Marcus Cooper, Clément Péron, linux-arm-kernel

From: Jernej Skrabec <jernej.skrabec@siol.net>

H6 I2S is very similar to that in H3, except it supports up to 16
channels.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 227 ++++++++++++++++++++++++++++++++++++
 1 file changed, 227 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 4198a5410bf9..a23c9f2a3f8c 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -124,6 +124,21 @@
 #define SUN8I_I2S_RX_CHAN_SEL_REG	0x54
 #define SUN8I_I2S_RX_CHAN_MAP_REG	0x58
 
+/* Defines required for sun50i-h6 support */
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK	GENMASK(21, 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset)	((offset) << 20)
+#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK		GENMASK(19, 16)
+#define SUN50I_H6_I2S_TX_CHAN_SEL(chan)		((chan - 1) << 16)
+#define SUN50I_H6_I2S_TX_CHAN_EN_MASK		GENMASK(15, 0)
+#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan)	(((1 << num_chan) - 1))
+
+#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG	0x44
+#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG	0x48
+
+#define SUN50I_H6_I2S_RX_CHAN_SEL_REG	0x64
+#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG	0x68
+#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG	0x6C
+
 struct sun4i_i2s;
 
 /**
@@ -469,6 +484,65 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 	return 0;
 }
 
+static int sun50i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
+				   const struct snd_pcm_hw_params *params)
+{
+	unsigned int channels = params_channels(params);
+	unsigned int slots = channels;
+	unsigned int lrck_period;
+
+	if (i2s->slots)
+		slots = i2s->slots;
+
+	/* Map the channels for playback and capture */
+	regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
+	regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);
+
+	/* Configure the channels */
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
+			   SUN50I_H6_I2S_TX_CHAN_SEL(channels));
+	regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
+			   SUN50I_H6_I2S_TX_CHAN_SEL(channels));
+
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+			   SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
+			   SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+			   SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
+			   SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
+
+	switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_DSP_A:
+	case SND_SOC_DAIFMT_DSP_B:
+	case SND_SOC_DAIFMT_LEFT_J:
+	case SND_SOC_DAIFMT_RIGHT_J:
+		lrck_period = params_physical_width(params) * slots;
+		break;
+
+	case SND_SOC_DAIFMT_I2S:
+		lrck_period = params_physical_width(params);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (i2s->slot_width)
+		lrck_period = i2s->slot_width;
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
+			   SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
+
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+			   SUN50I_H6_I2S_TX_CHAN_EN_MASK,
+			   SUN50I_H6_I2S_TX_CHAN_EN(channels));
+
+	return 0;
+}
+
 static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 			       struct snd_pcm_hw_params *params,
 			       struct snd_soc_dai *dai)
@@ -694,6 +768,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
 	return 0;
 }
 
+static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
+				 unsigned int fmt)
+{
+	u32 mode, val;
+	u8 offset;
+
+	/*
+	 * DAI clock polarity
+	 *
+	 * The setup for LRCK contradicts the datasheet, but under a
+	 * scope it's clear that the LRCK polarity is reversed
+	 * compared to the expected polarity on the bus.
+	 */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_IB_IF:
+		/* Invert both clocks */
+		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		/* Invert bit clock */
+		val = SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED |
+		      SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		/* Invert frame clock */
+		val = 0;
+		break;
+	case SND_SOC_DAIFMT_NB_NF:
+		val = SUN8I_I2S_FMT0_LRCLK_POLARITY_INVERTED;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
+			   SUN8I_I2S_FMT0_LRCLK_POLARITY_MASK |
+			   SUN8I_I2S_FMT0_BCLK_POLARITY_MASK,
+			   val);
+
+	/* DAI Mode */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_DSP_A:
+		mode = SUN8I_I2S_CTRL_MODE_PCM;
+		offset = 1;
+		break;
+
+	case SND_SOC_DAIFMT_DSP_B:
+		mode = SUN8I_I2S_CTRL_MODE_PCM;
+		offset = 0;
+		break;
+
+	case SND_SOC_DAIFMT_I2S:
+		mode = SUN8I_I2S_CTRL_MODE_LEFT;
+		offset = 1;
+		break;
+
+	case SND_SOC_DAIFMT_LEFT_J:
+		mode = SUN8I_I2S_CTRL_MODE_LEFT;
+		offset = 0;
+		break;
+
+	case SND_SOC_DAIFMT_RIGHT_J:
+		mode = SUN8I_I2S_CTRL_MODE_RIGHT;
+		offset = 0;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN8I_I2S_CTRL_MODE_MASK, mode);
+	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset));
+	regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
+			   SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset));
+
+	/* DAI clock master masks */
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		/* BCLK and LRCLK master */
+		val = SUN8I_I2S_CTRL_BCLK_OUT |	SUN8I_I2S_CTRL_LRCK_OUT;
+		break;
+
+	case SND_SOC_DAIFMT_CBM_CFM:
+		/* BCLK and LRCLK slave */
+		val = 0;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
+			   val);
+
+	return 0;
+}
+
 static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -974,6 +1150,22 @@ static const struct reg_default sun8i_i2s_reg_defaults[] = {
 	{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
 };
 
+static const struct reg_default sun50i_i2s_reg_defaults[] = {
+	{ SUN4I_I2S_CTRL_REG, 0x00060000 },
+	{ SUN4I_I2S_FMT0_REG, 0x00000033 },
+	{ SUN4I_I2S_FMT1_REG, 0x00000030 },
+	{ SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
+	{ SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
+	{ SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
+	{ SUN8I_I2S_CHAN_CFG_REG, 0x00000000 },
+	{ SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
+	{ SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0x00000000 },
+	{ SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x00000000 },
+	{ SUN50I_H6_I2S_RX_CHAN_SEL_REG, 0x00000000 },
+	{ SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0x00000000 },
+	{ SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x00000000 },
+};
+
 static const struct regmap_config sun4i_i2s_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
@@ -1001,6 +1193,19 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
 	.volatile_reg	= sun8i_i2s_volatile_reg,
 };
 
+static const struct regmap_config sun50i_i2s_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= SUN50I_H6_I2S_RX_CHAN_MAP1_REG,
+	.cache_type	= REGCACHE_FLAT,
+	.reg_defaults	= sun50i_i2s_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(sun50i_i2s_reg_defaults),
+	.writeable_reg	= sun4i_i2s_wr_reg,
+	.readable_reg	= sun8i_i2s_rd_reg,
+	.volatile_reg	= sun8i_i2s_volatile_reg,
+};
+
 static int sun4i_i2s_runtime_resume(struct device *dev)
 {
 	struct sun4i_i2s *i2s = dev_get_drvdata(dev);
@@ -1159,6 +1364,24 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
 	.set_fmt		= sun4i_i2s_set_soc_fmt,
 };
 
+static const struct sun4i_i2s_quirks sun50i_h6_i2s_quirks = {
+	.has_reset		= true,
+	.reg_offset_txdata	= SUN8I_I2S_FIFO_TX_REG,
+	.sun4i_i2s_regmap	= &sun50i_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),
+	.bclk_dividers		= sun8i_i2s_clk_div,
+	.num_bclk_dividers	= ARRAY_SIZE(sun8i_i2s_clk_div),
+	.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,
+	.get_sr			= sun8i_i2s_get_sr_wss,
+	.get_wss		= sun8i_i2s_get_sr_wss,
+	.set_chan_cfg		= sun50i_i2s_set_chan_cfg,
+	.set_fmt		= sun50i_i2s_set_soc_fmt,
+};
+
 static int sun4i_i2s_init_regmap_fields(struct device *dev,
 					struct sun4i_i2s *i2s)
 {
@@ -1328,6 +1551,10 @@ static const struct of_device_id sun4i_i2s_match[] = {
 		.compatible = "allwinner,sun50i-a64-codec-i2s",
 		.data = &sun50i_a64_codec_i2s_quirks,
 	},
+	{
+		.compatible = "allwinner,sun50i-h6-i2s",
+		.data = &sun50i_h6_i2s_quirks,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 4/7] ASoC: sun4i-i2s: Set sign extend sample
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
                   ` (2 preceding siblings ...)
  2020-04-26 10:41 ` [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-26 10:41 ` [PATCH v3 5/7] ASoc: sun4i-i2s: Add 20 and 24 bit support Clément Péron
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, linux-kernel, Marcus Cooper,
	Clément Péron, linux-arm-kernel

From: Marcus Cooper <codekipper@gmail.com>

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 while 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.

Set sign extend sample for all the sunxi generations even if they
are not affected. This will keep coherency and avoid relying on
default.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index a23c9f2a3f8c..8ea08b49e7fe 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -48,6 +48,9 @@
 #define SUN4I_I2S_FMT0_FMT_I2S				(0 << 0)
 
 #define SUN4I_I2S_FMT1_REG		0x08
+#define SUN4I_I2S_FMT1_REG_SEXT_MASK		BIT(8)
+#define SUN4I_I2S_FMT1_REG_SEXT(sext)			((sext) << 8)
+
 #define SUN4I_I2S_FIFO_TX_REG		0x0c
 #define SUN4I_I2S_FIFO_RX_REG		0x10
 
@@ -105,6 +108,9 @@
 #define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED		(1 << 7)
 #define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL		(0 << 7)
 
+#define SUN8I_I2S_FMT1_REG_SEXT_MASK		GENMASK(5, 4)
+#define SUN8I_I2S_FMT1_REG_SEXT(sext)			((sext) << 4)
+
 #define SUN8I_I2S_INT_STA_REG		0x0c
 #define SUN8I_I2S_FIFO_TX_REG		0x20
 
@@ -663,6 +669,12 @@ static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
 	}
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
 			   SUN4I_I2S_CTRL_MODE_MASK, val);
+
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+			   SUN4I_I2S_FMT1_REG_SEXT_MASK,
+			   SUN4I_I2S_FMT1_REG_SEXT(0));
+
 	return 0;
 }
 
@@ -765,6 +777,11 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
 			   SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
 			   val);
 
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+			   SUN8I_I2S_FMT1_REG_SEXT_MASK,
+			   SUN8I_I2S_FMT1_REG_SEXT(0));
+
 	return 0;
 }
 
@@ -867,6 +884,11 @@ static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
 			   SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT,
 			   val);
 
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG,
+			   SUN8I_I2S_FMT1_REG_SEXT_MASK,
+			   SUN8I_I2S_FMT1_REG_SEXT(0));
+
 	return 0;
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 5/7] ASoc: sun4i-i2s: Add 20 and 24 bit support
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
                   ` (3 preceding siblings ...)
  2020-04-26 10:41 ` [PATCH v3 4/7] ASoC: sun4i-i2s: Set sign extend sample Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-26 10:41 ` [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings Clément Péron
  2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
  6 siblings, 0 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, linux-kernel, Marcus Cooper,
	Clément Péron, linux-arm-kernel

From: Marcus Cooper <codekipper@gmail.com>

Extend the functionality of the driver to include support of 20 and
24 bits per sample.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 8ea08b49e7fe..277bf566c154 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -577,6 +577,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	case 16:
 		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		break;
+	case 32:
+		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
 	default:
 		dev_err(dai->dev, "Unsupported physical sample width: %d\n",
 			params_physical_width(params));
@@ -1063,6 +1066,10 @@ static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
 	return 0;
 }
 
+#define SUN4I_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
+			 SNDRV_PCM_FMTBIT_S20_LE | \
+			 SNDRV_PCM_FMTBIT_S24_LE)
+
 static struct snd_soc_dai_driver sun4i_i2s_dai = {
 	.probe = sun4i_i2s_dai_probe,
 	.capture = {
@@ -1070,14 +1077,14 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {
 		.channels_min = 1,
 		.channels_max = 8,
 		.rates = SNDRV_PCM_RATE_8000_192000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SUN4I_FORMATS,
 	},
 	.playback = {
 		.stream_name = "Playback",
 		.channels_min = 1,
 		.channels_max = 8,
 		.rates = SNDRV_PCM_RATE_8000_192000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SUN4I_FORMATS,
 	},
 	.ops = &sun4i_i2s_dai_ops,
 	.symmetric_rates = 1,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
                   ` (4 preceding siblings ...)
  2020-04-26 10:41 ` [PATCH v3 5/7] ASoc: sun4i-i2s: Add 20 and 24 bit support Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-27 11:03   ` Chen-Yu Tsai
  2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
  6 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, linux-kernel, Marcus Cooper,
	Clément Péron, linux-arm-kernel

From: Marcus Cooper <codekipper@gmail.com>

Bypass the regmap cache when flushing or reading the i2s FIFOs.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 277bf566c154..b5ab25483a9b 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1121,6 +1121,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case SUN4I_I2S_FIFO_RX_REG:
+	case SUN4I_I2S_FIFO_TX_REG:
+	case SUN4I_I2S_FIFO_STA_REG:
 	case SUN4I_I2S_INT_STA_REG:
 	case SUN4I_I2S_RX_CNT_REG:
 	case SUN4I_I2S_TX_CNT_REG:
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI
  2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
                   ` (5 preceding siblings ...)
  2020-04-26 10:41 ` [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings Clément Péron
@ 2020-04-26 10:41 ` Clément Péron
  2020-04-26 11:51   ` Clément Péron
  2020-04-28  8:14   ` Maxime Ripard
  6 siblings, 2 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 10:41 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, alsa-devel, Jernej Skrabec, linux-kernel,
	Marcus Cooper, Clément Péron, linux-arm-kernel

From: Jernej Skrabec <jernej.skrabec@siol.net>

Add a simple-soundcard to link audio between HDMI and I2S.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index a5ee68388bd3..558fe63739cb 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -88,6 +88,24 @@
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 	};
 
+	sound_hdmi: sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "allwinner-hdmi";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,frame-inversion;
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s1>;
+			dai-tdm-slot-num = <2>;
+			dai-tdm-slot-width = <32>;
+		};
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -581,6 +599,18 @@
 			};
 		};
 
+		i2s1: i2s@5091000 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun50i-h6-i2s";
+			reg = <0x05091000 0x1000>;
+			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;
+			clock-names = "apb", "mod";
+			dmas = <&dma 4>, <&dma 4>;
+			resets = <&ccu RST_BUS_I2S1>;
+			dma-names = "rx", "tx";
+		};
+
 		spdif: spdif@5093000 {
 			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-h6-spdif";
@@ -711,6 +741,7 @@
 		};
 
 		hdmi: hdmi@6000000 {
+			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-h6-dw-hdmi";
 			reg = <0x06000000 0x10000>;
 			reg-io-width = <1>;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI
  2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
@ 2020-04-26 11:51   ` Clément Péron
  2020-04-28  8:14   ` Maxime Ripard
  1 sibling, 0 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-26 11:51 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jaroslav Kysela, Takashi Iwai
  Cc: devicetree, Linux-ALSA, Jernej Skrabec, linux-kernel,
	Marcus Cooper, linux-arm-kernel

Hi,

On Sun, 26 Apr 2020 at 12:41, Clément Péron <peron.clem@gmail.com> wrote:
>
> From: Jernej Skrabec <jernej.skrabec@siol.net>
>
> Add a simple-soundcard to link audio between HDMI and I2S.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 31 ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index a5ee68388bd3..558fe63739cb 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -88,6 +88,24 @@
>                         (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>         };
>
> +       sound_hdmi: sound {

I will rename this to
hdmi_sound: hdmi-sound {

in the next version

> +               compatible = "simple-audio-card";
> +               simple-audio-card,format = "i2s";
> +               simple-audio-card,name = "allwinner-hdmi";
> +               simple-audio-card,mclk-fs = <128>;
> +               simple-audio-card,frame-inversion;

And I will add a status="disabled" here as not all board have an hdmi connector

Regards,
Clement

> +
> +               simple-audio-card,codec {
> +                       sound-dai = <&hdmi>;
> +               };
> +
> +               simple-audio-card,cpu {
> +                       sound-dai = <&i2s1>;
> +                       dai-tdm-slot-num = <2>;
> +                       dai-tdm-slot-width = <32>;
> +               };
> +       };
> +
>         soc {
>                 compatible = "simple-bus";
>                 #address-cells = <1>;
> @@ -581,6 +599,18 @@
>                         };
>                 };
>
> +               i2s1: i2s@5091000 {
> +                       #sound-dai-cells = <0>;
> +                       compatible = "allwinner,sun50i-h6-i2s";
> +                       reg = <0x05091000 0x1000>;
> +                       interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;
> +                       clock-names = "apb", "mod";
> +                       dmas = <&dma 4>, <&dma 4>;
> +                       resets = <&ccu RST_BUS_I2S1>;
> +                       dma-names = "rx", "tx";
> +               };
> +
>                 spdif: spdif@5093000 {
>                         #sound-dai-cells = <0>;
>                         compatible = "allwinner,sun50i-h6-spdif";
> @@ -711,6 +741,7 @@
>                 };
>
>                 hdmi: hdmi@6000000 {
> +                       #sound-dai-cells = <0>;
>                         compatible = "allwinner,sun50i-h6-dw-hdmi";
>                         reg = <0x06000000 0x10000>;
>                         reg-io-width = <1>;
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings
  2020-04-26 10:41 ` [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings Clément Péron
@ 2020-04-27 11:03   ` Chen-Yu Tsai
  2020-05-03 11:42     ` Clément Péron
  0 siblings, 1 reply; 26+ messages in thread
From: Chen-Yu Tsai @ 2020-04-27 11:03 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, Linux-ALSA, Marcus Cooper, linux-kernel,
	Takashi Iwai, Maxime Ripard, Liam Girdwood, Rob Herring,
	Mark Brown, linux-arm-kernel

On Sun, Apr 26, 2020 at 6:41 PM Clément Péron <peron.clem@gmail.com> wrote:
>
> From: Marcus Cooper <codekipper@gmail.com>
>
> Bypass the regmap cache when flushing or reading the i2s FIFOs.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width
  2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
@ 2020-04-28  8:02   ` Maxime Ripard
  0 siblings, 0 replies; 26+ messages in thread
From: Maxime Ripard @ 2020-04-28  8:02 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, alsa-devel, linux-kernel, Takashi Iwai, Rob Herring,
	Liam Girdwood, Marcus Cooper, Chen-Yu Tsai, Mark Brown,
	linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

On Sun, Apr 26, 2020 at 12:41:09PM +0200, Clément Péron wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> 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 <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index d0a8d5810c0a..4198a5410bf9 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -455,6 +455,9 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>  		return -EINVAL;
>  	}
>  
> +	if (i2s->slot_width)
> +		lrck_period = i2s->slot_width;
> +

Acked-by: Maxime Ripard <maxime@cerno.tech>

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
  2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
@ 2020-04-28  8:08   ` Maxime Ripard
  2020-05-11 22:26   ` Rob Herring
  1 sibling, 0 replies; 26+ messages in thread
From: Maxime Ripard @ 2020-04-28  8:08 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, alsa-devel, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

On Sun, Apr 26, 2020 at 12:41:10PM +0200, Clément Péron wrote:
> From: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> H6 I2S is very similar to H3, except that it supports up to 16 channels
> and thus few registers have fields on different position.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-26 10:41 ` [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S Clément Péron
@ 2020-04-28  8:13   ` Maxime Ripard
  2020-04-28  8:55     ` Clément Péron
  0 siblings, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-04-28  8:13 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, alsa-devel, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 4406 bytes --]

Hi,

On Sun, Apr 26, 2020 at 12:41:11PM +0200, Clément Péron wrote:
> From: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> H6 I2S is very similar to that in H3, except it supports up to 16
> channels.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 227 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 227 insertions(+)
> 
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 4198a5410bf9..a23c9f2a3f8c 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -124,6 +124,21 @@
>  #define SUN8I_I2S_RX_CHAN_SEL_REG	0x54
>  #define SUN8I_I2S_RX_CHAN_MAP_REG	0x58
>  
> +/* Defines required for sun50i-h6 support */
> +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK	GENMASK(21, 20)
> +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset)	((offset) << 20)
> +#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK		GENMASK(19, 16)
> +#define SUN50I_H6_I2S_TX_CHAN_SEL(chan)		((chan - 1) << 16)
> +#define SUN50I_H6_I2S_TX_CHAN_EN_MASK		GENMASK(15, 0)
> +#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan)	(((1 << num_chan) - 1))
> +
> +#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG	0x44
> +#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG	0x48
> +
> +#define SUN50I_H6_I2S_RX_CHAN_SEL_REG	0x64
> +#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG	0x68
> +#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG	0x6C
> +
>  struct sun4i_i2s;
>  
>  /**
> @@ -469,6 +484,65 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>  	return 0;
>  }
>  
> +static int sun50i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> +				   const struct snd_pcm_hw_params *params)
> +{
> +	unsigned int channels = params_channels(params);
> +	unsigned int slots = channels;
> +	unsigned int lrck_period;
> +
> +	if (i2s->slots)
> +		slots = i2s->slots;
> +
> +	/* Map the channels for playback and capture */
> +	regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
> +	regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);
> +
> +	/* Configure the channels */
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> +			   SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> +			   SUN50I_H6_I2S_TX_CHAN_SEL(channels));
> +	regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
> +			   SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> +			   SUN50I_H6_I2S_TX_CHAN_SEL(channels));
> +
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
> +			   SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
> +			   SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
> +			   SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
> +			   SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
> +
> +	switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_DSP_A:
> +	case SND_SOC_DAIFMT_DSP_B:
> +	case SND_SOC_DAIFMT_LEFT_J:
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		lrck_period = params_physical_width(params) * slots;
> +		break;
> +
> +	case SND_SOC_DAIFMT_I2S:
> +		lrck_period = params_physical_width(params);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (i2s->slot_width)
> +		lrck_period = i2s->slot_width;
> +
> +	regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> +			   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> +			   SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
> +
> +	regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> +			   SUN50I_H6_I2S_TX_CHAN_EN_MASK,
> +			   SUN50I_H6_I2S_TX_CHAN_EN(channels));
> +
> +	return 0;
> +}
> +
>  static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
>  			       struct snd_pcm_hw_params *params,
>  			       struct snd_soc_dai *dai)
> @@ -694,6 +768,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
>  	return 0;
>  }
>  
> +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> +				 unsigned int fmt)

The alignment is off here

> +{
> +	u32 mode, val;
> +	u8 offset;
> +
> +	/*
> +	 * DAI clock polarity
> +	 *
> +	 * The setup for LRCK contradicts the datasheet, but under a
> +	 * scope it's clear that the LRCK polarity is reversed
> +	 * compared to the expected polarity on the bus.
> +	 */

Did you check this or has it been copy-pasted?

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI
  2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
  2020-04-26 11:51   ` Clément Péron
@ 2020-04-28  8:14   ` Maxime Ripard
  2020-04-28 14:36     ` Clément Péron
  1 sibling, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-04-28  8:14 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, alsa-devel, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

On Sun, Apr 26, 2020 at 12:41:15PM +0200, Clément Péron wrote:
> From: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> Add a simple-soundcard to link audio between HDMI and I2S.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 31 ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index a5ee68388bd3..558fe63739cb 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -88,6 +88,24 @@
>  			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>  	};
>  
> +	sound_hdmi: sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,name = "allwinner-hdmi";

It doesn't seem to be on purpose, but the name is different from the other
series you sent.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-28  8:13   ` Maxime Ripard
@ 2020-04-28  8:55     ` Clément Péron
  2020-04-29 12:35       ` Maxime Ripard
  0 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-28  8:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

Hi Maxime,

On Tue, 28 Apr 2020 at 10:13, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Sun, Apr 26, 2020 at 12:41:11PM +0200, Clément Péron wrote:
> > From: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > H6 I2S is very similar to that in H3, except it supports up to 16
> > channels.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  sound/soc/sunxi/sun4i-i2s.c | 227 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 227 insertions(+)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 4198a5410bf9..a23c9f2a3f8c 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -124,6 +124,21 @@
> >  #define SUN8I_I2S_RX_CHAN_SEL_REG    0x54
> >  #define SUN8I_I2S_RX_CHAN_MAP_REG    0x58
> >
> > +/* Defines required for sun50i-h6 support */
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK        GENMASK(21, 20)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset)     ((offset) << 20)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK               GENMASK(19, 16)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL(chan)              ((chan - 1) << 16)
> > +#define SUN50I_H6_I2S_TX_CHAN_EN_MASK                GENMASK(15, 0)
> > +#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan)   (((1 << num_chan) - 1))
> > +
> > +#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG       0x44
> > +#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG       0x48
> > +
> > +#define SUN50I_H6_I2S_RX_CHAN_SEL_REG        0x64
> > +#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG       0x68
> > +#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG       0x6C
> > +
> >  struct sun4i_i2s;
> >
> >  /**
> > @@ -469,6 +484,65 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> >       return 0;
> >  }
> >
> > +static int sun50i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > +                                const struct snd_pcm_hw_params *params)
> > +{
> > +     unsigned int channels = params_channels(params);
> > +     unsigned int slots = channels;
> > +     unsigned int lrck_period;
> > +
> > +     if (i2s->slots)
> > +             slots = i2s->slots;
> > +
> > +     /* Map the channels for playback and capture */
> > +     regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
> > +     regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);
> > +
> > +     /* Configure the channels */
> > +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> > +                        SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> > +                        SUN50I_H6_I2S_TX_CHAN_SEL(channels));
> > +     regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG,
> > +                        SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> > +                        SUN50I_H6_I2S_TX_CHAN_SEL(channels));
> > +
> > +     regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
> > +                        SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
> > +                        SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
> > +     regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
> > +                        SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
> > +                        SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
> > +
> > +     switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) {
> > +     case SND_SOC_DAIFMT_DSP_A:
> > +     case SND_SOC_DAIFMT_DSP_B:
> > +     case SND_SOC_DAIFMT_LEFT_J:
> > +     case SND_SOC_DAIFMT_RIGHT_J:
> > +             lrck_period = params_physical_width(params) * slots;
> > +             break;
> > +
> > +     case SND_SOC_DAIFMT_I2S:
> > +             lrck_period = params_physical_width(params);
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (i2s->slot_width)
> > +             lrck_period = i2s->slot_width;
> > +
> > +     regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> > +                        SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> > +                        SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period));
> > +
> > +     regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> > +                        SUN50I_H6_I2S_TX_CHAN_EN_MASK,
> > +                        SUN50I_H6_I2S_TX_CHAN_EN(channels));
> > +
> > +     return 0;
> > +}
> > +
> >  static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> >                              struct snd_pcm_hw_params *params,
> >                              struct snd_soc_dai *dai)
> > @@ -694,6 +768,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> >       return 0;
> >  }
> >
> > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > +                              unsigned int fmt)
>
> The alignment is off here
>
> > +{
> > +     u32 mode, val;
> > +     u8 offset;
> > +
> > +     /*
> > +      * DAI clock polarity
> > +      *
> > +      * The setup for LRCK contradicts the datasheet, but under a
> > +      * scope it's clear that the LRCK polarity is reversed
> > +      * compared to the expected polarity on the bus.
> > +      */
>
> Did you check this or has it been copy-pasted?

copy-pasted, I will check this.

Thanks,
Clement

>
> Thanks!
> Maxime

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI
  2020-04-28  8:14   ` Maxime Ripard
@ 2020-04-28 14:36     ` Clément Péron
  0 siblings, 0 replies; 26+ messages in thread
From: Clément Péron @ 2020-04-28 14:36 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

Hi Maxime,

On Tue, 28 Apr 2020 at 10:14, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Sun, Apr 26, 2020 at 12:41:15PM +0200, Clément Péron wrote:
> > From: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > Add a simple-soundcard to link audio between HDMI and I2S.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 31 ++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > index a5ee68388bd3..558fe63739cb 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -88,6 +88,24 @@
> >                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> >       };
> >
> > +     sound_hdmi: sound {
> > +             compatible = "simple-audio-card";
> > +             simple-audio-card,format = "i2s";
> > +             simple-audio-card,name = "allwinner-hdmi";
>
> It doesn't seem to be on purpose, but the name is different from the other
> series you sent.

Indeed, I have sent this serie before looking at the other.

I will change this to keep coherency, once we agree on the correct card name.

Thanks for the review,
Clement

>
> Maxime

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-28  8:55     ` Clément Péron
@ 2020-04-29 12:35       ` Maxime Ripard
  2020-04-29 16:33         ` Clément Péron
  0 siblings, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-04-29 12:35 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > +                              unsigned int fmt)
> >
> > The alignment is off here
> >
> > > +{
> > > +     u32 mode, val;
> > > +     u8 offset;
> > > +
> > > +     /*
> > > +      * DAI clock polarity
> > > +      *
> > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > +      * scope it's clear that the LRCK polarity is reversed
> > > +      * compared to the expected polarity on the bus.
> > > +      */
> >
> > Did you check this or has it been copy-pasted?
> 
> copy-pasted, I will check this.

It's not going to be easy to do this if you only have a board with HDMI. If you
can't test that easily, just remove the comment (or make it explicit that you
copy pasted it?), no comment is better than a wrong one.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-29 12:35       ` Maxime Ripard
@ 2020-04-29 16:33         ` Clément Péron
  2020-04-30  8:46           ` Maxime Ripard
  0 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-29 16:33 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

Hi Maxime,

On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > +                              unsigned int fmt)
> > >
> > > The alignment is off here
> > >
> > > > +{
> > > > +     u32 mode, val;
> > > > +     u8 offset;
> > > > +
> > > > +     /*
> > > > +      * DAI clock polarity
> > > > +      *
> > > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > > +      * scope it's clear that the LRCK polarity is reversed
> > > > +      * compared to the expected polarity on the bus.
> > > > +      */
> > >
> > > Did you check this or has it been copy-pasted?
> >
> > copy-pasted, I will check this.
>
> It's not going to be easy to do this if you only have a board with HDMI. If you
> can't test that easily, just remove the comment (or make it explicit that you
> copy pasted it?), no comment is better than a wrong one.

I have talked with Marcus Cooper it may be able to test this this week-end.
Also this can explain why we need the "
simple-audio-card,frame-inversion;" in the device-tree.

If think this fix has been introduced by you, correct? Could you say
on which SoC did you see this issue?

Thanks
Clement

>
> Maxime

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-29 16:33         ` Clément Péron
@ 2020-04-30  8:46           ` Maxime Ripard
  2020-04-30 14:00             ` Clément Péron
  0 siblings, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-04-30  8:46 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

Hi,

On Wed, Apr 29, 2020 at 06:33:00PM +0200, Clément Péron wrote:
> On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > > +                              unsigned int fmt)
> > > >
> > > > The alignment is off here
> > > >
> > > > > +{
> > > > > +     u32 mode, val;
> > > > > +     u8 offset;
> > > > > +
> > > > > +     /*
> > > > > +      * DAI clock polarity
> > > > > +      *
> > > > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > > > +      * scope it's clear that the LRCK polarity is reversed
> > > > > +      * compared to the expected polarity on the bus.
> > > > > +      */
> > > >
> > > > Did you check this or has it been copy-pasted?
> > >
> > > copy-pasted, I will check this.
> >
> > It's not going to be easy to do this if you only have a board with HDMI. If you
> > can't test that easily, just remove the comment (or make it explicit that you
> > copy pasted it?), no comment is better than a wrong one.
> 
> I have talked with Marcus Cooper it may be able to test this this week-end.
> Also this can explain why we need the "
> simple-audio-card,frame-inversion;" in the device-tree.
> 
> If think this fix has been introduced by you, correct? Could you say
> on which SoC did you see this issue?

This was seen on an H3

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-30  8:46           ` Maxime Ripard
@ 2020-04-30 14:00             ` Clément Péron
  2020-05-04 12:09               ` Maxime Ripard
  0 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-04-30 14:00 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

Hi Maxime,

On Thu, 30 Apr 2020 at 10:46, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Wed, Apr 29, 2020 at 06:33:00PM +0200, Clément Péron wrote:
> > On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > > > +                              unsigned int fmt)
> > > > >
> > > > > The alignment is off here
> > > > >
> > > > > > +{
> > > > > > +     u32 mode, val;
> > > > > > +     u8 offset;
> > > > > > +
> > > > > > +     /*
> > > > > > +      * DAI clock polarity
> > > > > > +      *
> > > > > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > > > > +      * scope it's clear that the LRCK polarity is reversed
> > > > > > +      * compared to the expected polarity on the bus.
> > > > > > +      */
> > > > >
> > > > > Did you check this or has it been copy-pasted?
> > > >
> > > > copy-pasted, I will check this.
> > >
> > > It's not going to be easy to do this if you only have a board with HDMI. If you
> > > can't test that easily, just remove the comment (or make it explicit that you
> > > copy pasted it?), no comment is better than a wrong one.
> >
> > I have talked with Marcus Cooper it may be able to test this this week-end.
> > Also this can explain why we need the "
> > simple-audio-card,frame-inversion;" in the device-tree.
> >
> > If think this fix has been introduced by you, correct? Could you say
> > on which SoC did you see this issue?
>
> This was seen on an H3

Just two more questions:
- Did you observe this issue on both TDM and I2S mode?
- On which DAI node?

Since recent change in sun4i-i2s.c, we had to introduce the
"simple-audio-card,frame-inversion" in LibreElec tree.
H3 boards are quite common in LibreElec community so I think:
 - This fix is only needed in TDM mode
 - Or this fix is not required for the HDMI DAI node (HDMI DAI is a
little bit different compare to other DAI but I think the first guess
is more likely)

Regards,
Clement

>
> Maxime

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings
  2020-04-27 11:03   ` Chen-Yu Tsai
@ 2020-05-03 11:42     ` Clément Péron
  0 siblings, 0 replies; 26+ messages in thread
From: Clément Péron @ 2020-05-03 11:42 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: devicetree, Linux-ALSA, Marcus Cooper, linux-kernel,
	Takashi Iwai, Maxime Ripard, Liam Girdwood, Rob Herring,
	Mark Brown, linux-arm-kernel

Hi,

On Mon, 27 Apr 2020 at 13:03, Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Sun, Apr 26, 2020 at 6:41 PM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > Bypass the regmap cache when flushing or reading the i2s FIFOs.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
>
> Acked-by: Chen-Yu Tsai <wens@csie.org>

The  SUN4I_I2S_FIFO_CTRL_REG is also missing.
As some bits can self-clear by themselves.

I will fix this in v4.

Regards,
Clement

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-04-30 14:00             ` Clément Péron
@ 2020-05-04 12:09               ` Maxime Ripard
  2020-05-04 19:43                 ` Clément Péron
  0 siblings, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-05-04 12:09 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 3223 bytes --]

Hi Clement,

On Thu, Apr 30, 2020 at 04:00:14PM +0200, Clément Péron wrote:
> On Thu, 30 Apr 2020 at 10:46, Maxime Ripard <maxime@cerno.tech> wrote:
> > On Wed, Apr 29, 2020 at 06:33:00PM +0200, Clément Péron wrote:
> > > On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
> > > >
> > > > On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > > > > +                              unsigned int fmt)
> > > > > >
> > > > > > The alignment is off here
> > > > > >
> > > > > > > +{
> > > > > > > +     u32 mode, val;
> > > > > > > +     u8 offset;
> > > > > > > +
> > > > > > > +     /*
> > > > > > > +      * DAI clock polarity
> > > > > > > +      *
> > > > > > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > > > > > +      * scope it's clear that the LRCK polarity is reversed
> > > > > > > +      * compared to the expected polarity on the bus.
> > > > > > > +      */
> > > > > >
> > > > > > Did you check this or has it been copy-pasted?
> > > > >
> > > > > copy-pasted, I will check this.
> > > >
> > > > It's not going to be easy to do this if you only have a board with HDMI. If you
> > > > can't test that easily, just remove the comment (or make it explicit that you
> > > > copy pasted it?), no comment is better than a wrong one.
> > >
> > > I have talked with Marcus Cooper it may be able to test this this week-end.
> > > Also this can explain why we need the "
> > > simple-audio-card,frame-inversion;" in the device-tree.
> > >
> > > If think this fix has been introduced by you, correct? Could you say
> > > on which SoC did you see this issue?
> >
> > This was seen on an H3
> 
> Just two more questions:
> - Did you observe this issue on both TDM and I2S mode?
> - On which DAI node?

This is fairly fuzzy now and I don't remember if I tested it in I2S. Let's
assume I didn't. And similarly, I'm not sure what the exact controller was, but
it was one of the regular controllers (so not either connected to the codec or
the HDMI, one that goes out of the SoC).

We had pretty much the same issue on the A33 in I2S for the codec though:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/sunxi/sun8i-codec.c?id=18c1bf35c1c09bca05cf70bc984a4764e0b0372b

I didn't think of it that way then, but it might very well have been the i2s
controller suffering the same issue.

> Since recent change in sun4i-i2s.c, we had to introduce the
> "simple-audio-card,frame-inversion" in LibreElec tree.

Do you have a link to that commit ? I couldn't find anything on libreelec's
github repo.

> H3 boards are quite common in LibreElec community so I think:
>  - This fix is only needed in TDM mode
>  - Or this fix is not required for the HDMI DAI node (HDMI DAI is a
> little bit different compare to other DAI but I think the first guess
> is more likely)

Given what we know about the A33, I'd be inclined to say the latter. I'd don't
have the tools to check anymore, but if you have even a cheap logical analyzer,
i2s being pretty slow you can definitely see it.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-05-04 12:09               ` Maxime Ripard
@ 2020-05-04 19:43                 ` Clément Péron
  2020-07-29 14:39                   ` Maxime Ripard
  0 siblings, 1 reply; 26+ messages in thread
From: Clément Péron @ 2020-05-04 19:43 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

Hi Maxime,

On Mon, 4 May 2020 at 14:09, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Clement,
>
> On Thu, Apr 30, 2020 at 04:00:14PM +0200, Clément Péron wrote:
> > On Thu, 30 Apr 2020 at 10:46, Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Wed, Apr 29, 2020 at 06:33:00PM +0200, Clément Péron wrote:
> > > > On Wed, 29 Apr 2020 at 14:35, Maxime Ripard <maxime@cerno.tech> wrote:
> > > > >
> > > > > On Tue, Apr 28, 2020 at 10:55:47AM +0200, Clément Péron wrote:
> > > > > > > > +static int sun50i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > > > > > > > +                              unsigned int fmt)
> > > > > > >
> > > > > > > The alignment is off here
> > > > > > >
> > > > > > > > +{
> > > > > > > > +     u32 mode, val;
> > > > > > > > +     u8 offset;
> > > > > > > > +
> > > > > > > > +     /*
> > > > > > > > +      * DAI clock polarity
> > > > > > > > +      *
> > > > > > > > +      * The setup for LRCK contradicts the datasheet, but under a
> > > > > > > > +      * scope it's clear that the LRCK polarity is reversed
> > > > > > > > +      * compared to the expected polarity on the bus.
> > > > > > > > +      */
> > > > > > >
> > > > > > > Did you check this or has it been copy-pasted?
> > > > > >
> > > > > > copy-pasted, I will check this.
> > > > >
> > > > > It's not going to be easy to do this if you only have a board with HDMI. If you
> > > > > can't test that easily, just remove the comment (or make it explicit that you
> > > > > copy pasted it?), no comment is better than a wrong one.
> > > >
> > > > I have talked with Marcus Cooper it may be able to test this this week-end.
> > > > Also this can explain why we need the "
> > > > simple-audio-card,frame-inversion;" in the device-tree.
> > > >
> > > > If think this fix has been introduced by you, correct? Could you say
> > > > on which SoC did you see this issue?
> > >
> > > This was seen on an H3
> >
> > Just two more questions:
> > - Did you observe this issue on both TDM and I2S mode?
> > - On which DAI node?
>
> This is fairly fuzzy now and I don't remember if I tested it in I2S. Let's
> assume I didn't. And similarly, I'm not sure what the exact controller was, but
> it was one of the regular controllers (so not either connected to the codec or
> the HDMI, one that goes out of the SoC).
>
> We had pretty much the same issue on the A33 in I2S for the codec though:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/sunxi/sun8i-codec.c?id=18c1bf35c1c09bca05cf70bc984a4764e0b0372b
>
> I didn't think of it that way then, but it might very well have been the i2s
> controller suffering the same issue.
>
> > Since recent change in sun4i-i2s.c, we had to introduce the
> > "simple-audio-card,frame-inversion" in LibreElec tree.
>
> Do you have a link to that commit ? I couldn't find anything on libreelec's
> github repo.

These commits:
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/A64/patches/linux/04-Add-frame-inversion-to-correct-multi-channel-audio.patch
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/H3/patches/linux/17-Add-frame-inversion-to-correct-multi-channel-audio.patch
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Allwinner/devices/H6/patches/linux/16-Add-frame-inversion-to-correct-multi-channel-audio.patch

>
> > H3 boards are quite common in LibreElec community so I think:
> >  - This fix is only needed in TDM mode
> >  - Or this fix is not required for the HDMI DAI node (HDMI DAI is a
> > little bit different compare to other DAI but I think the first guess
> > is more likely)
>
> Given what we know about the A33, I'd be inclined to say the latter. I'd don't
> have the tools to check anymore, but if you have even a cheap logical analyzer,
> i2s being pretty slow you can definitely see it.

Me neither but maybe Marcus will be able to check this.
Thanks for all these informations.

>
> Maxime

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
  2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
  2020-04-28  8:08   ` Maxime Ripard
@ 2020-05-11 22:26   ` Rob Herring
  1 sibling, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-05-11 22:26 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, alsa-devel, Liam Girdwood, Mark Brown,
	Jernej Skrabec, Takashi Iwai, Maxime Ripard, linux-kernel,
	Marcus Cooper, Chen-Yu Tsai, Rob Herring, linux-arm-kernel

On Sun, 26 Apr 2020 12:41:10 +0200, Clément Péron wrote:
> From: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> H6 I2S is very similar to H3, except that it supports up to 16 channels
> and thus few registers have fields on different position.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml      | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-05-04 19:43                 ` Clément Péron
@ 2020-07-29 14:39                   ` Maxime Ripard
  2020-07-29 15:15                     ` Mark Brown
  0 siblings, 1 reply; 26+ messages in thread
From: Maxime Ripard @ 2020-07-29 14:39 UTC (permalink / raw)
  To: Clément Péron
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Rob Herring, Liam Girdwood, Marcus Cooper,
	Chen-Yu Tsai, Mark Brown, linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 1421 bytes --]

On Mon, May 04, 2020 at 09:43:05PM +0200, Clément Péron wrote:
> > > H3 boards are quite common in LibreElec community so I think:
> > >  - This fix is only needed in TDM mode
> > >  - Or this fix is not required for the HDMI DAI node (HDMI DAI is a
> > > little bit different compare to other DAI but I think the first guess
> > > is more likely)
> >
> > Given what we know about the A33, I'd be inclined to say the latter. I'd don't
> > have the tools to check anymore, but if you have even a cheap logical analyzer,
> > i2s being pretty slow you can definitely see it.
> 
> Me neither but maybe Marcus will be able to check this.
> Thanks for all these informations.

I finally got my hangs on a logical analyzer and gave it a try.

You'll find the start of the playback on H6's i2s0 attached.

It really looks like the polarity of LRCK is fine though. The first word
is sent with LRCK low, and then high, so we have channel 0 and then
channel 1 which seems to be the proper ordering?

Something that is fairly odd though is that there's a delay of about 8ms
(about 387 LRCK periods) between the time where SCK and LRCK start to
output something, and then when the data starts being output.

However, the signal seems to be dephased and will start being output on
the channel 1, which might lead to the behaviour that is seen? you can
see that on the second screenshot attached.

Maxime

[-- Attachment #1.2: h6-i2s-start-data.png --]
[-- Type: image/png, Size: 183824 bytes --]

[-- Attachment #1.3: i2s-h6.png --]
[-- Type: image/png, Size: 135413 bytes --]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S
  2020-07-29 14:39                   ` Maxime Ripard
@ 2020-07-29 15:15                     ` Mark Brown
  0 siblings, 0 replies; 26+ messages in thread
From: Mark Brown @ 2020-07-29 15:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree, Linux-ALSA, linux-kernel, Jernej Skrabec,
	Takashi Iwai, Liam Girdwood, Marcus Cooper, Chen-Yu Tsai,
	Rob Herring, Clément Péron, linux-arm-kernel


[-- Attachment #1: Type: text/plain, Size: 281 bytes --]

On Wed, Jul 29, 2020 at 04:39:27PM +0200, Maxime Ripard wrote:

> It really looks like the polarity of LRCK is fine though. The first word
> is sent with LRCK low, and then high, so we have channel 0 and then
> channel 1 which seems to be the proper ordering?

Yes, that's normal.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, back to index

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-26 10:41 [PATCH v3 0/7] Add H6 I2S support Clément Péron
2020-04-26 10:41 ` [PATCH v3 1/7] ASoC: sun4i-i2s: Adjust LRCLK width Clément Péron
2020-04-28  8:02   ` Maxime Ripard
2020-04-26 10:41 ` [PATCH v3 2/7] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible Clément Péron
2020-04-28  8:08   ` Maxime Ripard
2020-05-11 22:26   ` Rob Herring
2020-04-26 10:41 ` [PATCH v3 3/7] ASoC: sun4i-i2s: Add support for H6 I2S Clément Péron
2020-04-28  8:13   ` Maxime Ripard
2020-04-28  8:55     ` Clément Péron
2020-04-29 12:35       ` Maxime Ripard
2020-04-29 16:33         ` Clément Péron
2020-04-30  8:46           ` Maxime Ripard
2020-04-30 14:00             ` Clément Péron
2020-05-04 12:09               ` Maxime Ripard
2020-05-04 19:43                 ` Clément Péron
2020-07-29 14:39                   ` Maxime Ripard
2020-07-29 15:15                     ` Mark Brown
2020-04-26 10:41 ` [PATCH v3 4/7] ASoC: sun4i-i2s: Set sign extend sample Clément Péron
2020-04-26 10:41 ` [PATCH v3 5/7] ASoc: sun4i-i2s: Add 20 and 24 bit support Clément Péron
2020-04-26 10:41 ` [PATCH v3 6/7] ASoC: sun4i-i2s: Adjust regmap settings Clément Péron
2020-04-27 11:03   ` Chen-Yu Tsai
2020-05-03 11:42     ` Clément Péron
2020-04-26 10:41 ` [PATCH v3 7/7] arm64: dts: sun50i-h6: Add HDMI audio to H6 DTSI Clément Péron
2020-04-26 11:51   ` Clément Péron
2020-04-28  8:14   ` Maxime Ripard
2020-04-28 14:36     ` Clément Péron

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git