[PATCH v5 00/20] Add Allwinner H3/H5/H6/A64 HDMI audio

[PATCH v5 00/20] Add Allwinner H3/H5/H6/A64 HDMI audio

[Clément Péron]

Hi,

This is exactly the same as v4 but with more details in some commit log
and also device-tree soundcard and DAI node have been merged.

Regards,
Clement

Change since v4;
- add more comment on get_wss() and set_channel_cfg() patch
- merge soundcard and DAI HDMI patches

Change since v3:
- add Samuel Holland patch to reconfigure FIFO_TX_REG when suspend is enabled
- readd inversion to H6 LRCK sun50i_h6_i2s_set_soc_fmt()
- Fix get_wss() for sun4i
- Add a commit to fix checkpatch warning

Change since v2:
- rebase on next-20200918
- drop revert LRCK polarity patch
- readd simple-audio-card,frame-inversion in dts
- Add patch for changing set_chan_cfg params

Change since v1:
- rebase on next-20200828
- add revert LRCK polarity
- remove all simple-audio-card,frame-inversion in dts
- add Ondrej patches for Orange Pi board
- Add arm64 defconfig patch

Clément Péron (6):
  ASoC: sun4i-i2s: Change set_chan_cfg() params
  ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
  ASoC: sun4i-i2s: Fix sun8i volatile regs
  arm64: dts: allwinner: h6: Enable HDMI sound for Beelink GS1
  arm64: defconfig: Enable Allwinner i2s driver
  ASoC: sun4i-i2s: fix coding-style for callback definition

Jernej Skrabec (3):
  ASoC: sun4i-i2s: Add support for H6 I2S
  dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
  arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

Marcus Cooper (7):
  ASoC: sun4i-i2s: Set sign extend sample
  ASoc: sun4i-i2s: Add 20 and 24 bit support
  arm: dts: sunxi: h3/h5: Add DAI node and soundcard for HDMI
  arm64: dts: allwinner: a64: Add DAI node and soundcard for HDMI
  arm: sun8i: h3: Add HDMI audio to Orange Pi 2
  arm: sun8i: h3: Add HDMI audio to Beelink X2
  arm64: dts: allwinner: a64: Add HDMI audio to Pine64

Ondrej Jirman (3):
  arm64: dts: allwinner: Enable HDMI audio on Orange Pi PC 2
  ARM: dts: sun8i-h3: Enable HDMI audio on Orange Pi PC/One
  arm64: dts: sun50i-h6-orangepi-3: Enable HDMI audio

Samuel Holland (1):
  ASoC: sun4i-i2s: Fix setting of FIFO modes

 .../sound/allwinner,sun4i-a10-i2s.yaml        |   2 +
 arch/arm/boot/dts/sun8i-h3-beelink-x2.dts     |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts     |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts   |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts    |   8 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  33 ++
 .../boot/dts/allwinner/sun50i-a64-pine64.dts  |   8 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  34 ++
 .../dts/allwinner/sun50i-h5-orangepi-pc2.dts  |   8 +
 .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |   8 +
 .../dts/allwinner/sun50i-h6-orangepi-3.dts    |   8 +
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 ++
 arch/arm64/configs/defconfig                  |   1 +
 sound/soc/sunxi/sun4i-i2s.c                   | 374 +++++++++++++++---
 14 files changed, 487 insertions(+), 54 deletions(-)

-- 
2.25.1

[PATCH v5 01/20] ASoC: sun4i-i2s: Add support for H6 I2S

[Clément Péron]

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 | 224 ++++++++++++++++++++++++++++++++++++
 1 file changed, 224 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index f23ff29e7c1d..2baf6c276280 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;
 
 /**
@@ -474,6 +489,62 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 	return 0;
 }
 
+static int sun50i_h6_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;
+	}
+
+	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)
@@ -699,6 +770,108 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
 	return 0;
 }
 
+static int sun50i_h6_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);
@@ -979,6 +1152,22 @@ static const struct reg_default sun8i_i2s_reg_defaults[] = {
 	{ SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
 };
 
+static const struct reg_default sun50i_h6_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,
@@ -1006,6 +1195,19 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
 	.volatile_reg	= sun8i_i2s_volatile_reg,
 };
 
+static const struct regmap_config sun50i_h6_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_h6_i2s_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(sun50i_h6_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);
@@ -1164,6 +1366,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_h6_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_h6_i2s_set_chan_cfg,
+	.set_fmt		= sun50i_h6_i2s_set_soc_fmt,
+};
+
 static int sun4i_i2s_init_regmap_fields(struct device *dev,
 					struct sun4i_i2s *i2s)
 {
@@ -1333,6 +1553,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.25.1

[PATCH v5 02/20] ASoC: sun4i-i2s: Change set_chan_cfg() params

[Clément Péron]

As slots and slot_width can be overwritter in case set_tdm() is
called. Avoid to have this logic in set_chan_cfg().

Instead pass the required values as params to set_chan_cfg().

This will also avoid a bug when we will enable 20/24bits support,
i2s->slot_width is not actually used in the lrck_period computation.

Suggested-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 2baf6c276280..0633b9fba3d7 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -177,8 +177,9 @@ struct sun4i_i2s_quirks {
 	unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
 	s8	(*get_sr)(const struct sun4i_i2s *, int);
 	s8	(*get_wss)(const struct sun4i_i2s *, int);
-	int	(*set_chan_cfg)(const struct sun4i_i2s *,
-				const struct snd_pcm_hw_params *);
+	int	(*set_chan_cfg)(const struct sun4i_i2s *i2s,
+				unsigned int channels,	unsigned int slots,
+				unsigned int slot_width);
 	int	(*set_fmt)(const struct sun4i_i2s *, unsigned int);
 };
 
@@ -414,10 +415,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
 }
 
 static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
-				  const struct snd_pcm_hw_params *params)
+				  unsigned int channels, unsigned int slots,
+				  unsigned int slot_width)
 {
-	unsigned int channels = params_channels(params);
-
 	/* Map the channels for playback and capture */
 	regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
 	regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);
@@ -434,15 +434,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 }
 
 static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
-				  const struct snd_pcm_hw_params *params)
+				  unsigned int channels, unsigned int slots,
+				  unsigned int slot_width)
 {
-	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, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
 	regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
@@ -467,11 +463,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 	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;
+		lrck_period = slot_width * slots;
 		break;
 
 	case SND_SOC_DAIFMT_I2S:
-		lrck_period = params_physical_width(params);
+		lrck_period = slot_width;
 		break;
 
 	default:
@@ -490,15 +486,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 }
 
 static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
-				      const struct snd_pcm_hw_params *params)
+				      unsigned int channels, unsigned int slots,
+				      unsigned int slot_width)
 {
-	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);
@@ -523,11 +515,11 @@ static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
 	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;
+		lrck_period = slot_width * slots;
 		break;
 
 	case SND_SOC_DAIFMT_I2S:
-		lrck_period = params_physical_width(params);
+		lrck_period = slot_width;
 		break;
 
 	default:
@@ -563,7 +555,7 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	if (i2s->slot_width)
 		slot_width = i2s->slot_width;
 
-	ret = i2s->variant->set_chan_cfg(i2s, params);
+	ret = i2s->variant->set_chan_cfg(i2s, channels, slots, slot_width);
 	if (ret < 0) {
 		dev_err(dai->dev, "Invalid channel configuration\n");
 		return ret;
-- 
2.25.1

[PATCH v5 03/20] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit

[Clément Péron]

We are actually using a complex formula to just return a bunch of
simple values. Also this formula is wrong for sun4i when calling
get_wss() the function return 4 instead of 3.

Replace this with a simpler switch case.

Also drop the i2s params which is unused and return a simple int as
returning an error code could be out of range for an s8 and there is
no optim to return a s8 here.

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

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 0633b9fba3d7..11bbcbe24d6b 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -175,8 +175,8 @@ struct sun4i_i2s_quirks {
 	unsigned int			num_mclk_dividers;
 
 	unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
-	s8	(*get_sr)(const struct sun4i_i2s *, int);
-	s8	(*get_wss)(const struct sun4i_i2s *, int);
+	int	(*get_sr)(unsigned int width);
+	int	(*get_wss)(unsigned int width);
 	int	(*set_chan_cfg)(const struct sun4i_i2s *i2s,
 				unsigned int channels,	unsigned int slots,
 				unsigned int slot_width);
@@ -381,37 +381,56 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 	return 0;
 }
 
-static s8 sun4i_i2s_get_sr(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_sr(unsigned int width)
 {
-	if (width < 16 || width > 24)
-		return -EINVAL;
-
-	if (width % 4)
-		return -EINVAL;
+	switch (width) {
+	case 16:
+		return 0x0;
+	case 20:
+		return 0x1;
+	case 24:
+		return 0x2;
+	}
 
-	return (width - 16) / 4;
+	return -EINVAL;
 }
 
-static s8 sun4i_i2s_get_wss(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_wss(unsigned int width)
 {
-	if (width < 16 || width > 32)
-		return -EINVAL;
-
-	if (width % 4)
-		return -EINVAL;
+	switch (width) {
+	case 16:
+		return 0x0;
+	case 20:
+		return 0x1;
+	case 24:
+		return 0x2;
+	case 32:
+		return 0x3;
+	}
 
-	return (width - 16) / 4;
+	return -EINVAL;
 }
 
-static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
+static int sun8i_i2s_get_sr_wss(unsigned int width)
 {
-	if (width % 4)
-		return -EINVAL;
-
-	if (width < 8 || width > 32)
-		return -EINVAL;
+	switch (width) {
+	case 8:
+		return 0x1;
+	case 12:
+		return 0x2;
+	case 16:
+		return 0x3;
+	case 20:
+		return 0x4;
+	case 24:
+		return 0x5;
+	case 28:
+		return 0x6;
+	case 32:
+		return 0x7;
+	}
 
-	return (width - 8) / 4 + 1;
+	return -EINVAL;
 }
 
 static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
@@ -572,11 +591,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 	i2s->playback_dma_data.addr_width = width;
 
-	sr = i2s->variant->get_sr(i2s, word_size);
+	sr = i2s->variant->get_sr(word_size);
 	if (sr < 0)
 		return -EINVAL;
 
-	wss = i2s->variant->get_wss(i2s, slot_width);
+	wss = i2s->variant->get_wss(slot_width);
 	if (wss < 0)
 		return -EINVAL;
 
-- 
2.25.1

[PATCH v5 04/20] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible

[Clément Péron]

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>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../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.25.1

[PATCH v5 05/20] ASoC: sun4i-i2s: Set sign extend sample

[Clément Péron]

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 consistency 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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 11bbcbe24d6b..b31454eee43c 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
 
@@ -676,6 +682,7 @@ 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);
+
 	return 0;
 }
 
@@ -778,6 +785,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;
 }
 
@@ -880,6 +892,11 @@ static int sun50i_h6_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.25.1

[PATCH v5 06/20] ASoc: sun4i-i2s: Add 20 and 24 bit support

[Clément Péron]

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>
Acked-by: Maxime Ripard <mripard@kernel.org>
---
 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 b31454eee43c..60b2fec1bbe9 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -590,6 +590,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));
@@ -1071,6 +1074,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 = {
@@ -1078,14 +1085,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.25.1

[PATCH v5 07/20] ASoC: sun4i-i2s: Fix sun8i volatile regs

[Clément Péron]

The FIFO TX reg is volatile and sun8i i2s register
mapping is different from sun4i.

Even if in this case it's doesn't create an issue,
Avoid setting some regs that are undefined in sun8i.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
---
 sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 60b2fec1bbe9..3f9110d70941 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1152,12 +1152,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
 
 static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
 {
-	if (reg == SUN8I_I2S_INT_STA_REG)
+	switch (reg) {
+	case SUN4I_I2S_FIFO_CTRL_REG:
+	case SUN4I_I2S_FIFO_RX_REG:
+	case SUN4I_I2S_FIFO_STA_REG:
+	case SUN4I_I2S_RX_CNT_REG:
+	case SUN4I_I2S_TX_CNT_REG:
+	case SUN8I_I2S_FIFO_TX_REG:
+	case SUN8I_I2S_INT_STA_REG:
 		return true;
-	if (reg == SUN8I_I2S_FIFO_TX_REG)
-		return false;
 
-	return sun4i_i2s_volatile_reg(dev, reg);
+	default:
+		return false;
+	}
 }
 
 static const struct reg_default sun4i_i2s_reg_defaults[] = {
-- 
2.25.1

[PATCH v5 08/20] ASoC: sun4i-i2s: Fix setting of FIFO modes

[Clément Péron]

From: Samuel Holland <samuel@sholland.org>

Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
regmap is cache-only are ignored. To work around this, move the
configuration to a callback that runs while the ASoC core has a
runtime PM reference to the device.

Signed-off-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 3f9110d70941..9cd6cd1cd284 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -586,6 +586,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
+	/* Set significant bits in our FIFOs */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+			   SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+			   SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+			   SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+			   SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
 	switch (params_physical_width(params)) {
 	case 16:
 		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -914,13 +921,6 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		return ret;
 	}
 
-	/* Set significant bits in our FIFOs */
-	regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
-			   SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
-			   SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
-			   SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
-			   SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
-
 	i2s->format = fmt;
 
 	return 0;
-- 
2.25.1

[PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Clément Péron]

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

Add the I2S node used by the HDMI and a simple-soundcard to
link audio between HDMI and I2S.

Note that the HDMI codec requires an inverted frame clock and
a fixed I2S width. As there is no such option for I2S we use
TDM property of the simple-soundcard to do that.

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 | 33 ++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 28c77d6872f6..a8853ee7885a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -67,6 +67,25 @@ de: display-engine {
 		status = "disabled";
 	};
 
+	hdmi_sound: hdmi-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "sun50i-h6-hdmi";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,frame-inversion;
+		status = "disabled";
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s1>;
+			dai-tdm-slot-num = <2>;
+			dai-tdm-slot-width = <32>;
+		};
+	};
+
 	osc24M: osc24M_clk {
 		#clock-cells = <0>;
 		compatible = "fixed-clock";
@@ -609,6 +628,19 @@ mdio: mdio {
 			};
 		};
 
+		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";
+			status = "disabled";
+		};
+
 		spdif: spdif@5093000 {
 			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-h6-spdif";
@@ -739,6 +771,7 @@ ohci3: usb@5311400 {
 		};
 
 		hdmi: hdmi@6000000 {
+			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-h6-dw-hdmi";
 			reg = <0x06000000 0x10000>;
 			reg-io-width = <1>;
-- 
2.25.1

[PATCH v5 10/20] arm64: dts: allwinner: h6: Enable HDMI sound for Beelink GS1

[Clément Péron]

Now that HDMI sound node is available in the SoC dtsi.
Enable it for this board.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
index 3f7ceeb1a767..049c21718846 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -118,6 +118,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s1 {
+	status = "okay";
+};
+
 &mdio {
 	ext_rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.25.1

[PATCH v5 11/20] arm: dts: sunxi: h3/h5: Add DAI node and soundcard for HDMI

[Clément Péron]

From: Marcus Cooper <codekipper@gmail.com>

Add the new DAI block for I2S2 which is used for HDMI audio and
a simple-soundcard to link audio between HDMI and I2S.

Note that the HDMI codec requires an inverted frame clock and
a fixed I2S width. As there is no such option for I2S we use
TDM property of the simple-soundcard to do that.

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/arm/boot/dts/sunxi-h3-h5.dtsi | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 22d533d18992..7168e9412410 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -105,6 +105,25 @@ de: display-engine {
 		status = "disabled";
 	};
 
+	hdmi_sound: hdmi-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "sun8i-h3-hdmi";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,frame-inversion;
+		status = "disabled";
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s2>;
+			dai-tdm-slot-num = <2>;
+			dai-tdm-slot-width = <32>;
+		};
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -662,6 +681,19 @@ i2s1: i2s@1c22400 {
 			status = "disabled";
 		};
 
+		i2s2: i2s@1c22800 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun8i-h3-i2s";
+			reg = <0x01c22800 0x400>;
+			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+			clock-names = "apb", "mod";
+			dmas = <&dma 27>, <&dma 27>;
+			resets = <&ccu RST_BUS_I2S2>;
+			dma-names = "rx", "tx";
+			status = "disabled";
+		};
+
 		codec: codec@1c22c00 {
 			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun8i-h3-codec";
@@ -793,6 +825,7 @@ csi: camera@1cb0000 {
 		};
 
 		hdmi: hdmi@1ee0000 {
+			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun8i-h3-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
 			reg = <0x01ee0000 0x10000>;
-- 
2.25.1

[PATCH v5 12/20] arm64: dts: allwinner: a64: Add DAI node and soundcard for HDMI

[Clément Péron]

From: Marcus Cooper <codekipper@gmail.com>

Add the new DAI block for I2S2 which is used for HDMI audio
and a simple-soundcard to link audio between HDMI and I2S.

Note that the HDMI codec requires an inverted frame clock and
a fixed I2S width. As there is no such option for I2S we use
TDM property of the simple-soundcard to do that.

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-a64.dtsi | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index dc238814013c..ba8a84e6e993 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -102,6 +102,25 @@ de: display-engine {
 		status = "disabled";
 	};
 
+	hdmi_sound: hdmi-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "sun50i-a64-hdmi";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,frame-inversion;
+		status = "disabled";
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s2>;
+			dai-tdm-slot-num = <2>;
+			dai-tdm-slot-width = <32>;
+		};
+	};
+
 	osc24M: osc24M_clk {
 		#clock-cells = <0>;
 		compatible = "fixed-clock";
@@ -846,6 +865,20 @@ i2s1: i2s@1c22400 {
 			status = "disabled";
 		};
 
+		i2s2: i2s@1c22800 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun50i-a64-i2s",
+				     "allwinner,sun8i-h3-i2s";
+			reg = <0x01c22800 0x400>;
+			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2S2>, <&ccu CLK_I2S2>;
+			clock-names = "apb", "mod";
+			resets = <&ccu RST_BUS_I2S2>;
+			dma-names = "rx", "tx";
+			dmas = <&dma 27>, <&dma 27>;
+			status = "disabled";
+		};
+
 		dai: dai@1c22c00 {
 			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-a64-codec-i2s";
@@ -1143,6 +1176,7 @@ deinterlace: deinterlace@1e00000 {
 		};
 
 		hdmi: hdmi@1ee0000 {
+			#sound-dai-cells = <0>;
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
 			reg = <0x01ee0000 0x10000>;
-- 
2.25.1

[PATCH v5 13/20] arm: sun8i: h3: Add HDMI audio to Orange Pi 2

[Clément Péron]

From: Marcus Cooper <codekipper@gmail.com>

Enable HDMI audio on the Orange Pi 2.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 597c425d08ec..64e8e2829f27 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -144,6 +144,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&r_ir_rx_pin>;
-- 
2.25.1

[PATCH v5 14/20] arm: sun8i: h3: Add HDMI audio to Beelink X2

[Clément Péron]

From: Marcus Cooper <codekipper@gmail.com>

Enable HDMI audio on the Beelink X2.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-beelink-x2.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
index 45a24441ff18..f9bec6935120 100644
--- a/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
@@ -142,6 +142,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
 &ir {
 	linux,rc-map-name = "rc-tanix-tx3mini";
 	pinctrl-names = "default";
-- 
2.25.1

[PATCH v5 15/20] arm64: dts: allwinner: a64: Add HDMI audio to Pine64

[Clément Péron]

From: Marcus Cooper <codekipper@gmail.com>

Enable HDMI audio on Pine64.

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-a64-pine64.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 329cf276561e..b54b02e59085 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -99,6 +99,10 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
 &i2c1 {
 	status = "okay";
 };
@@ -107,6 +111,10 @@ &i2c1_pins {
 	bias-pull-up;
 };
 
+&i2s2 {
+	status = "okay";
+};
+
 &mdio {
 	ext_rmii_phy1: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.25.1

[PATCH v5 16/20] arm64: dts: allwinner: Enable HDMI audio on Orange Pi PC 2

[Clément Péron]

From: Ondrej Jirman <megous@megous.com>

The board has HDMI output, enable audio on it.

Signed-off-by: Ondrej Jirman <megous@megous.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
index 7d7aad18f078..e6ed8774a17f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
@@ -144,6 +144,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&r_ir_rx_pin>;
-- 
2.25.1

[PATCH v5 17/20] ARM: dts: sun8i-h3: Enable HDMI audio on Orange Pi PC/One

[Clément Péron]

From: Ondrej Jirman <megous@megous.com>

The board has HDMI output, enable audio on it.

Signed-off-by: Ondrej Jirman <megous@megous.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts | 8 ++++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts  | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index 4759ba3f2986..a5b45655fcf6 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -146,6 +146,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_vcc3v3>;
 	bus-width = <4>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 5aff8ecc66cb..ebb12a6f3a9f 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -146,6 +146,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&r_ir_rx_pin>;
-- 
2.25.1

[PATCH v5 18/20] arm64: dts: sun50i-h6-orangepi-3: Enable HDMI audio

[Clément Péron]

From: Ondrej Jirman <megous@megous.com>

The board has HDMI output, enable audio on it.

Signed-off-by: Ondrej Jirman <megous@megous.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 15c9dd8c4479..193587e78031 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -128,6 +128,14 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2s1 {
+	status = "okay";
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_cldo1>;
 	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
-- 
2.25.1

[PATCH v5 19/20] arm64: defconfig: Enable Allwinner i2s driver

[Clément Péron]

Enable Allwinner I2S driver for arm64 defconfig.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 63003ec116ee..9a3c3bbe60e4 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -696,6 +696,7 @@ CONFIG_SND_SOC_ROCKCHIP_RT5645=m
 CONFIG_SND_SOC_RK3399_GRU_SOUND=m
 CONFIG_SND_SOC_SAMSUNG=y
 CONFIG_SND_SOC_RCAR=m
+CONFIG_SND_SUN4I_I2S=m
 CONFIG_SND_SUN4I_SPDIF=m
 CONFIG_SND_SOC_TEGRA=m
 CONFIG_SND_SOC_TEGRA210_AHUB=m
-- 
2.25.1

[PATCH v5 20/20] ASoC: sun4i-i2s: fix coding-style for callback definition

[Clément Péron]

Checkpatch script produces warning:
WARNING: function definition argument 'const struct sun4i_i2s *'
should also have an identifier name.

Let's fix this by adding identifier name to get_bclk_parent_rate()
and set_fmt() callback definition.

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

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9cd6cd1cd284..a6fd9bef30d1 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -180,13 +180,13 @@ struct sun4i_i2s_quirks {
 	const struct sun4i_i2s_clk_div	*mclk_dividers;
 	unsigned int			num_mclk_dividers;
 
-	unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
+	unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *i2s);
 	int	(*get_sr)(unsigned int width);
 	int	(*get_wss)(unsigned int width);
 	int	(*set_chan_cfg)(const struct sun4i_i2s *i2s,
 				unsigned int channels,	unsigned int slots,
 				unsigned int slot_width);
-	int	(*set_fmt)(const struct sun4i_i2s *, unsigned int);
+	int	(*set_fmt)(const struct sun4i_i2s *i2s, unsigned int fmt);
 };
 
 struct sun4i_i2s {
-- 
2.25.1

Re: [linux-sunxi] [PATCH v5 03/20] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> We are actually using a complex formula to just return a bunch of
> simple values. Also this formula is wrong for sun4i when calling
> get_wss() the function return 4 instead of 3.
>
> Replace this with a simpler switch case.
>
> Also drop the i2s params which is unused and return a simple int as
> returning an error code could be out of range for an s8 and there is
> no optim to return a s8 here.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")

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

Re: [linux-sunxi] [PATCH v5 01/20] ASoC: sun4i-i2s: Add support for H6 I2S

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> 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 | 224 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 224 insertions(+)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index f23ff29e7c1d..2baf6c276280 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;
>
>  /**
> @@ -474,6 +489,62 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>         return 0;
>  }
>
> +static int sun50i_h6_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);

Nit, since it supports up to 16 channels, you might want to map all 16 of them
now, instead of having to come back and fix it later.

Code wise, this patch is

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

I don't have a scope nor logic analyzer, so I wasn't able to participate in the
LRCK discussion.

Re: [linux-sunxi] [PATCH v5 02/20] ASoC: sun4i-i2s: Change set_chan_cfg() params

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> As slots and slot_width can be overwritter in case set_tdm() is
> called. Avoid to have this logic in set_chan_cfg().

It doesn't seem that set_tdm_slot() would get called concurrently
with hw_params(), at least not for the simple-card family. If so
then we'd have more problems like mismatched slots/slot-width
due to no serialization when interacting with those values.

> Instead pass the required values as params to set_chan_cfg().
>
> This will also avoid a bug when we will enable 20/24bits support,
> i2s->slot_width is not actually used in the lrck_period computation.
>
> Suggested-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 36 ++++++++++++++----------------------
>  1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 2baf6c276280..0633b9fba3d7 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -177,8 +177,9 @@ struct sun4i_i2s_quirks {
>         unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
>         s8      (*get_sr)(const struct sun4i_i2s *, int);
>         s8      (*get_wss)(const struct sun4i_i2s *, int);
> -       int     (*set_chan_cfg)(const struct sun4i_i2s *,
> -                               const struct snd_pcm_hw_params *);
> +       int     (*set_chan_cfg)(const struct sun4i_i2s *i2s,
> +                               unsigned int channels,  unsigned int slots,
> +                               unsigned int slot_width);
>         int     (*set_fmt)(const struct sun4i_i2s *, unsigned int);
>  };
>
> @@ -414,10 +415,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
>  }
>
>  static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> -                                 const struct snd_pcm_hw_params *params)
> +                                 unsigned int channels, unsigned int slots,
> +                                 unsigned int slot_width)
>  {
> -       unsigned int channels = params_channels(params);
> -
>         /* Map the channels for playback and capture */
>         regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
>         regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);
> @@ -434,15 +434,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>  }
>
>  static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> -                                 const struct snd_pcm_hw_params *params)
> +                                 unsigned int channels, unsigned int slots,
> +                                 unsigned int slot_width)
>  {
> -       unsigned int channels = params_channels(params);
> -       unsigned int slots = channels;
>         unsigned int lrck_period;
>
> -       if (i2s->slots)
> -               slots = i2s->slots;
> -

Based on the bug you highlighted in your expanded commit log, the simpler fix
would be to look at i2s->slot_width in addition to i2s->slots, and replace
params_physical_width(params) accordingly.

Also, I would put fixes (even preparatory ones) before patches that introduce
support for new features and hardware. That makes them easier to backport if
needed.


ChenYu


>         /* Map the channels for playback and capture */
>         regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
>         regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
> @@ -467,11 +463,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>         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;
> +               lrck_period = slot_width * slots;
>                 break;
>
>         case SND_SOC_DAIFMT_I2S:
> -               lrck_period = params_physical_width(params);
> +               lrck_period = slot_width;
>                 break;
>
>         default:

Re: [linux-sunxi] [PATCH v5 03/20] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 12:37 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > We are actually using a complex formula to just return a bunch of
> > simple values. Also this formula is wrong for sun4i when calling

BTW, it is entirely possible that the compiler optimizes your switch-case
back into the original complex formula you replaced. :)

> > get_wss() the function return 4 instead of 3.
> >
> > Replace this with a simpler switch case.
> >
> > Also drop the i2s params which is unused and return a simple int as
> > returning an error code could be out of range for an s8 and there is
> > no optim to return a s8 here.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
>
> Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Re: [linux-sunxi] [PATCH v5 04/20] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> 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>
> Acked-by: Rob Herring <robh@kernel.org>

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

Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> From: Jernej Skrabec <jernej.skrabec@siol.net>
>
> Add the I2S node used by the HDMI and a simple-soundcard to
> link audio between HDMI and I2S.
>
> Note that the HDMI codec requires an inverted frame clock and
> a fixed I2S width. As there is no such option for I2S we use
> TDM property of the simple-soundcard to do that.
>
> 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 | 33 ++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 28c77d6872f6..a8853ee7885a 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -67,6 +67,25 @@ de: display-engine {
>                 status = "disabled";
>         };
>
> +       hdmi_sound: hdmi-sound {
> +               compatible = "simple-audio-card";
> +               simple-audio-card,format = "i2s";
> +               simple-audio-card,name = "sun50i-h6-hdmi";
> +               simple-audio-card,mclk-fs = <128>;
> +               simple-audio-card,frame-inversion;
> +               status = "disabled";
> +
> +               simple-audio-card,codec {
> +                       sound-dai = <&hdmi>;
> +               };
> +
> +               simple-audio-card,cpu {
> +                       sound-dai = <&i2s1>;
> +                       dai-tdm-slot-num = <2>;

Doesn't this end up limiting the number of audio channels HDMI can carry?
AFAICT the TDM properties are all optional, so just leave it out.

Same goes for the other two patches.

> +                       dai-tdm-slot-width = <32>;
> +               };
> +       };
> +
>         osc24M: osc24M_clk {
>                 #clock-cells = <0>;
>                 compatible = "fixed-clock";
> @@ -609,6 +628,19 @@ mdio: mdio {
>                         };
>                 };
>
> +               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";
> +                       status = "disabled";
> +               };
> +
>                 spdif: spdif@5093000 {
>                         #sound-dai-cells = <0>;
>                         compatible = "allwinner,sun50i-h6-spdif";
> @@ -739,6 +771,7 @@ ohci3: usb@5311400 {
>                 };
>
>                 hdmi: hdmi@6000000 {
> +                       #sound-dai-cells = <0>;
>                         compatible = "allwinner,sun50i-h6-dw-hdmi";
>                         reg = <0x06000000 0x10000>;
>                         reg-io-width = <1>;

The rest of the patch looks OK.

Re: [linux-sunxi] [PATCH v5 10/20] arm64: dts: allwinner: h6: Enable HDMI sound for Beelink GS1

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> Now that HDMI sound node is available in the SoC dtsi.
> Enable it for this board.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

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

Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > From: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > Add the I2S node used by the HDMI and a simple-soundcard to
> > link audio between HDMI and I2S.
> >
> > Note that the HDMI codec requires an inverted frame clock and
> > a fixed I2S width. As there is no such option for I2S we use
> > TDM property of the simple-soundcard to do that.
> >
> > 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 | 33 ++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > index 28c77d6872f6..a8853ee7885a 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -67,6 +67,25 @@ de: display-engine {
> >                 status = "disabled";
> >         };
> >
> > +       hdmi_sound: hdmi-sound {
> > +               compatible = "simple-audio-card";
> > +               simple-audio-card,format = "i2s";
> > +               simple-audio-card,name = "sun50i-h6-hdmi";
> > +               simple-audio-card,mclk-fs = <128>;
> > +               simple-audio-card,frame-inversion;
> > +               status = "disabled";
> > +
> > +               simple-audio-card,codec {
> > +                       sound-dai = <&hdmi>;
> > +               };
> > +
> > +               simple-audio-card,cpu {
> > +                       sound-dai = <&i2s1>;
> > +                       dai-tdm-slot-num = <2>;
>
> Doesn't this end up limiting the number of audio channels HDMI can carry?
> AFAICT the TDM properties are all optional, so just leave it out.
>
> Same goes for the other two patches.
>
> > +                       dai-tdm-slot-width = <32>;
> > +               };
> > +       };
> > +
> >         osc24M: osc24M_clk {
> >                 #clock-cells = <0>;
> >                 compatible = "fixed-clock";
> > @@ -609,6 +628,19 @@ mdio: mdio {
> >                         };
> >                 };
> >
> > +               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";

Sorry, missed this one.

Given that usage for this interface is transmit only, and there is no
RX DRQ number assigned to it, you should drop the RX DMA number and name.

> > +                       status = "disabled";
> > +               };
> > +
> >                 spdif: spdif@5093000 {
> >                         #sound-dai-cells = <0>;
> >                         compatible = "allwinner,sun50i-h6-spdif";
> > @@ -739,6 +771,7 @@ ohci3: usb@5311400 {
> >                 };
> >
> >                 hdmi: hdmi@6000000 {
> > +                       #sound-dai-cells = <0>;
> >                         compatible = "allwinner,sun50i-h6-dw-hdmi";
> >                         reg = <0x06000000 0x10000>;
> >                         reg-io-width = <1>;
>
> The rest of the patch looks OK.

Re: [linux-sunxi] [PATCH v5 07/20] ASoC: sun4i-i2s: Fix sun8i volatile regs

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> The FIFO TX reg is volatile and sun8i i2s register
> mapping is different from sun4i.
>
> Even if in this case it's doesn't create an issue,
> Avoid setting some regs that are undefined in sun8i.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> Acked-by: Maxime Ripard <mripard@kernel.org>

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

Re: [linux-sunxi] [PATCH v5 08/20] ASoC: sun4i-i2s: Fix setting of FIFO modes

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> From: Samuel Holland <samuel@sholland.org>
>
> Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
> regmap is cache-only are ignored. To work around this, move the
> configuration to a callback that runs while the ASoC core has a
> runtime PM reference to the device.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

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

Re: [linux-sunxi] [PATCH v5 06/20] ASoc: sun4i-i2s: Add 20 and 24 bit support

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> 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>
> Acked-by: Maxime Ripard <mripard@kernel.org>

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

Re: [linux-sunxi] [PATCH v5 05/20] ASoC: sun4i-i2s: Set sign extend sample

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> 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 consistency and avoid relying on
> default.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

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

Re: [linux-sunxi] [PATCH v5 00/20] Add Allwinner H3/H5/H6/A64 HDMI audio

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi,
>
> This is exactly the same as v4 but with more details in some commit log
> and also device-tree soundcard and DAI node have been merged.
>
> Regards,
> Clement
>
> Change since v4;
> - add more comment on get_wss() and set_channel_cfg() patch
> - merge soundcard and DAI HDMI patches
>
> Change since v3:
> - add Samuel Holland patch to reconfigure FIFO_TX_REG when suspend is enabled
> - readd inversion to H6 LRCK sun50i_h6_i2s_set_soc_fmt()
> - Fix get_wss() for sun4i
> - Add a commit to fix checkpatch warning
>
> Change since v2:
> - rebase on next-20200918
> - drop revert LRCK polarity patch
> - readd simple-audio-card,frame-inversion in dts
> - Add patch for changing set_chan_cfg params
>
> Change since v1:
> - rebase on next-20200828
> - add revert LRCK polarity
> - remove all simple-audio-card,frame-inversion in dts
> - add Ondrej patches for Orange Pi board
> - Add arm64 defconfig patch
>
> Clément Péron (6):
>   ASoC: sun4i-i2s: Change set_chan_cfg() params
>   ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
>   ASoC: sun4i-i2s: Fix sun8i volatile regs
>   arm64: dts: allwinner: h6: Enable HDMI sound for Beelink GS1
>   arm64: defconfig: Enable Allwinner i2s driver
>   ASoC: sun4i-i2s: fix coding-style for callback definition
>
> Jernej Skrabec (3):
>   ASoC: sun4i-i2s: Add support for H6 I2S
>   dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
>   arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI
>
> Marcus Cooper (7):
>   ASoC: sun4i-i2s: Set sign extend sample
>   ASoc: sun4i-i2s: Add 20 and 24 bit support
>   arm: dts: sunxi: h3/h5: Add DAI node and soundcard for HDMI
>   arm64: dts: allwinner: a64: Add DAI node and soundcard for HDMI
>   arm: sun8i: h3: Add HDMI audio to Orange Pi 2
>   arm: sun8i: h3: Add HDMI audio to Beelink X2
>   arm64: dts: allwinner: a64: Add HDMI audio to Pine64
>
> Ondrej Jirman (3):
>   arm64: dts: allwinner: Enable HDMI audio on Orange Pi PC 2
>   ARM: dts: sun8i-h3: Enable HDMI audio on Orange Pi PC/One
>   arm64: dts: sun50i-h6-orangepi-3: Enable HDMI audio

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

for all the board DTS file changes.

Re: [linux-sunxi] [PATCH v5 01/20] ASoC: sun4i-i2s: Add support for H6 I2S

[Clément Péron]

Hi Chen-Yu,

On Mon, 28 Sep 2020 at 06:40, Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> 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 | 224 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 224 insertions(+)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index f23ff29e7c1d..2baf6c276280 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;
> >
> >  /**
> > @@ -474,6 +489,62 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> >         return 0;
> >  }
> >
> > +static int sun50i_h6_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);
>
> Nit, since it supports up to 16 channels, you might want to map all 16 of them
> now, instead of having to come back and fix it later.

Thanks for the review. Do you mean there is missing MAP0 for RX/TX ?

+ regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0xFEDCBA98);
  regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
+ regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0xFEDCBA98);
  regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);

Regards,
Clement

>
> Code wise, this patch is
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>
> I don't have a scope nor logic analyzer, so I wasn't able to participate in the
> LRCK discussion.

Re: [linux-sunxi] [PATCH v5 01/20] ASoC: sun4i-i2s: Add support for H6 I2S

[Chen-Yu Tsai]

On Mon, Sep 28, 2020 at 10:37 PM Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi Chen-Yu,
>
> On Mon, 28 Sep 2020 at 06:40, Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> 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 | 224 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 224 insertions(+)
> > >
> > > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > > index f23ff29e7c1d..2baf6c276280 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;
> > >
> > >  /**
> > > @@ -474,6 +489,62 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > >         return 0;
> > >  }
> > >
> > > +static int sun50i_h6_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);
> >
> > Nit, since it supports up to 16 channels, you might want to map all 16 of them
> > now, instead of having to come back and fix it later.
>
> Thanks for the review. Do you mean there is missing MAP0 for RX/TX ?
>
> + regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0xFEDCBA98);
>   regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
> + regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0xFEDCBA98);
>   regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210);

Correct.

ChenYu

> Regards,
> Clement
>
> >
> > Code wise, this patch is
> >
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >
> > I don't have a scope nor logic analyzer, so I wasn't able to participate in the
> > LRCK discussion.
>
> --
> 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@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/CAJiuCcevtzX_%2B02r54q6tH0%2BbOF%3DBM%3DknnaxN%2BG3QW035F8gZQ%40mail.gmail.com.

Re: [linux-sunxi] [PATCH v5 02/20] ASoC: sun4i-i2s: Change set_chan_cfg() params

[Clément Péron]

Hi Chen-Yu,

On Mon, 28 Sep 2020 at 07:00, Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > As slots and slot_width can be overwritter in case set_tdm() is
> > called. Avoid to have this logic in set_chan_cfg().
>
> It doesn't seem that set_tdm_slot() would get called concurrently
> with hw_params(), at least not for the simple-card family. If so
> then we'd have more problems like mismatched slots/slot-width
> due to no serialization when interacting with those values.

Sorry maybe the commit log is not clear.
I was not talking about a concurrent effect but more if the slot_width is setted
then we don't properly use it later.

>
> > Instead pass the required values as params to set_chan_cfg().
> >
> > This will also avoid a bug when we will enable 20/24bits support,
> > i2s->slot_width is not actually used in the lrck_period computation.
> >
> > Suggested-by: Samuel Holland <samuel@sholland.org>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  sound/soc/sunxi/sun4i-i2s.c | 36 ++++++++++++++----------------------
> >  1 file changed, 14 insertions(+), 22 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 2baf6c276280..0633b9fba3d7 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -177,8 +177,9 @@ struct sun4i_i2s_quirks {
> >         unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
> >         s8      (*get_sr)(const struct sun4i_i2s *, int);
> >         s8      (*get_wss)(const struct sun4i_i2s *, int);
> > -       int     (*set_chan_cfg)(const struct sun4i_i2s *,
> > -                               const struct snd_pcm_hw_params *);
> > +       int     (*set_chan_cfg)(const struct sun4i_i2s *i2s,
> > +                               unsigned int channels,  unsigned int slots,
> > +                               unsigned int slot_width);
> >         int     (*set_fmt)(const struct sun4i_i2s *, unsigned int);
> >  };
> >
> > @@ -414,10 +415,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
> >  }
> >
> >  static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > -                                 const struct snd_pcm_hw_params *params)
> > +                                 unsigned int channels, unsigned int slots,
> > +                                 unsigned int slot_width)
> >  {
> > -       unsigned int channels = params_channels(params);
> > -
> >         /* Map the channels for playback and capture */
> >         regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
> >         regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x00003210);
> > @@ -434,15 +434,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> >  }
> >
> >  static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > -                                 const struct snd_pcm_hw_params *params)
> > +                                 unsigned int channels, unsigned int slots,
> > +                                 unsigned int slot_width)
> >  {
> > -       unsigned int channels = params_channels(params);
> > -       unsigned int slots = channels;
> >         unsigned int lrck_period;
> >
> > -       if (i2s->slots)
> > -               slots = i2s->slots;
> > -
>
> Based on the bug you highlighted in your expanded commit log, the simpler fix
> would be to look at i2s->slot_width in addition to i2s->slots, and replace
> params_physical_width(params) accordingly.

That's what I did in the first version but was pointed out by Samuel
that the same logic
was done several times. As I can avoid it by passing the correct
params it's simpler.

>
> Also, I would put fixes (even preparatory ones) before patches that introduce
> support for new features and hardware. That makes them easier to backport if
> needed.

I will wait for Maxime to comment on this. Regarding the doc fixes tag
should only be used
to fix a previous commit. This will make the fixes commit a bit more
complicated for stable kernel team.

Thanks for your review :)

Regards
Clement


>
>
> ChenYu
>
>
> >         /* Map the channels for playback and capture */
> >         regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
> >         regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
> > @@ -467,11 +463,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> >         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;
> > +               lrck_period = slot_width * slots;
> >                 break;
> >
> >         case SND_SOC_DAIFMT_I2S:
> > -               lrck_period = params_physical_width(params);
> > +               lrck_period = slot_width;
> >                 break;
> >
> >         default:

Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Clément Péron]

Hi Chen-Yu,

On Mon, 28 Sep 2020 at 07:42, Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> > >
> > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > >
> > > Add the I2S node used by the HDMI and a simple-soundcard to
> > > link audio between HDMI and I2S.
> > >
> > > Note that the HDMI codec requires an inverted frame clock and
> > > a fixed I2S width. As there is no such option for I2S we use
> > > TDM property of the simple-soundcard to do that.
> > >
> > > 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 | 33 ++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > index 28c77d6872f6..a8853ee7885a 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > @@ -67,6 +67,25 @@ de: display-engine {
> > >                 status = "disabled";
> > >         };
> > >
> > > +       hdmi_sound: hdmi-sound {
> > > +               compatible = "simple-audio-card";
> > > +               simple-audio-card,format = "i2s";
> > > +               simple-audio-card,name = "sun50i-h6-hdmi";
> > > +               simple-audio-card,mclk-fs = <128>;
> > > +               simple-audio-card,frame-inversion;
> > > +               status = "disabled";
> > > +
> > > +               simple-audio-card,codec {
> > > +                       sound-dai = <&hdmi>;
> > > +               };
> > > +
> > > +               simple-audio-card,cpu {
> > > +                       sound-dai = <&i2s1>;
> > > +                       dai-tdm-slot-num = <2>;
> >
> > Doesn't this end up limiting the number of audio channels HDMI can carry?
> > AFAICT the TDM properties are all optional, so just leave it out.
> >
> > Same goes for the other two patches.
> >
> > > +                       dai-tdm-slot-width = <32>;
> > > +               };
> > > +       };
> > > +
> > >         osc24M: osc24M_clk {
> > >                 #clock-cells = <0>;
> > >                 compatible = "fixed-clock";
> > > @@ -609,6 +628,19 @@ mdio: mdio {
> > >                         };
> > >                 };
> > >
> > > +               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";
>
> Sorry, missed this one.
>
> Given that usage for this interface is transmit only, and there is no
> RX DRQ number assigned to it, you should drop the RX DMA number and name.

Indeed if there is no DRQ number assigned we shouldn't have it in the
device-tree

but Samuel told me that the `make dtbs_check` reports:

i2s@1c22800: dma-names:0: 'rx' was expected
i2s@1c22800: dma-names: ['tx'] is too short
i2s@1c22800: dmas: [[28, 27]] is too short

Should I fix the YAML so?

Regards,
Clement

>
> > > +                       status = "disabled";
> > > +               };
> > > +
> > >                 spdif: spdif@5093000 {
> > >                         #sound-dai-cells = <0>;
> > >                         compatible = "allwinner,sun50i-h6-spdif";
> > > @@ -739,6 +771,7 @@ ohci3: usb@5311400 {
> > >                 };
> > >
> > >                 hdmi: hdmi@6000000 {
> > > +                       #sound-dai-cells = <0>;
> > >                         compatible = "allwinner,sun50i-h6-dw-hdmi";
> > >                         reg = <0x06000000 0x10000>;
> > >                         reg-io-width = <1>;
> >
> > The rest of the patch looks OK.

Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Maxime Ripard]

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

On Fri, Oct 02, 2020 at 06:01:21PM +0200, Clément Péron wrote:
> Hi Chen-Yu,
> 
> On Mon, 28 Sep 2020 at 07:42, Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> > > >
> > > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > > >
> > > > Add the I2S node used by the HDMI and a simple-soundcard to
> > > > link audio between HDMI and I2S.
> > > >
> > > > Note that the HDMI codec requires an inverted frame clock and
> > > > a fixed I2S width. As there is no such option for I2S we use
> > > > TDM property of the simple-soundcard to do that.
> > > >
> > > > 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 | 33 ++++++++++++++++++++
> > > >  1 file changed, 33 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > index 28c77d6872f6..a8853ee7885a 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > @@ -67,6 +67,25 @@ de: display-engine {
> > > >                 status = "disabled";
> > > >         };
> > > >
> > > > +       hdmi_sound: hdmi-sound {
> > > > +               compatible = "simple-audio-card";
> > > > +               simple-audio-card,format = "i2s";
> > > > +               simple-audio-card,name = "sun50i-h6-hdmi";
> > > > +               simple-audio-card,mclk-fs = <128>;
> > > > +               simple-audio-card,frame-inversion;
> > > > +               status = "disabled";
> > > > +
> > > > +               simple-audio-card,codec {
> > > > +                       sound-dai = <&hdmi>;
> > > > +               };
> > > > +
> > > > +               simple-audio-card,cpu {
> > > > +                       sound-dai = <&i2s1>;
> > > > +                       dai-tdm-slot-num = <2>;
> > >
> > > Doesn't this end up limiting the number of audio channels HDMI can carry?
> > > AFAICT the TDM properties are all optional, so just leave it out.
> > >
> > > Same goes for the other two patches.
> > >
> > > > +                       dai-tdm-slot-width = <32>;
> > > > +               };
> > > > +       };
> > > > +
> > > >         osc24M: osc24M_clk {
> > > >                 #clock-cells = <0>;
> > > >                 compatible = "fixed-clock";
> > > > @@ -609,6 +628,19 @@ mdio: mdio {
> > > >                         };
> > > >                 };
> > > >
> > > > +               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";
> >
> > Sorry, missed this one.
> >
> > Given that usage for this interface is transmit only, and there is no
> > RX DRQ number assigned to it, you should drop the RX DMA number and name.
> 
> Indeed if there is no DRQ number assigned we shouldn't have it in the
> device-tree
> 
> but Samuel told me that the `make dtbs_check` reports:
> 
> i2s@1c22800: dma-names:0: 'rx' was expected
> i2s@1c22800: dma-names: ['tx'] is too short
> i2s@1c22800: dmas: [[28, 27]] is too short
> 
> Should I fix the YAML so?

Yep :)

Maxime

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

Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

[Clément Péron]

Hi,

On Fri, 2 Oct 2020 at 18:24, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Fri, Oct 02, 2020 at 06:01:21PM +0200, Clément Péron wrote:
> > Hi Chen-Yu,
> >
> > On Mon, 28 Sep 2020 at 07:42, Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > > >
> > > > On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@gmail.com> wrote:
> > > > >
> > > > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > >
> > > > > Add the I2S node used by the HDMI and a simple-soundcard to
> > > > > link audio between HDMI and I2S.
> > > > >
> > > > > Note that the HDMI codec requires an inverted frame clock and
> > > > > a fixed I2S width. As there is no such option for I2S we use
> > > > > TDM property of the simple-soundcard to do that.
> > > > >
> > > > > 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 | 33 ++++++++++++++++++++
> > > > >  1 file changed, 33 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > > index 28c77d6872f6..a8853ee7885a 100644
> > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > > @@ -67,6 +67,25 @@ de: display-engine {
> > > > >                 status = "disabled";
> > > > >         };
> > > > >
> > > > > +       hdmi_sound: hdmi-sound {
> > > > > +               compatible = "simple-audio-card";
> > > > > +               simple-audio-card,format = "i2s";
> > > > > +               simple-audio-card,name = "sun50i-h6-hdmi";
> > > > > +               simple-audio-card,mclk-fs = <128>;
> > > > > +               simple-audio-card,frame-inversion;
> > > > > +               status = "disabled";
> > > > > +
> > > > > +               simple-audio-card,codec {
> > > > > +                       sound-dai = <&hdmi>;
> > > > > +               };
> > > > > +
> > > > > +               simple-audio-card,cpu {
> > > > > +                       sound-dai = <&i2s1>;
> > > > > +                       dai-tdm-slot-num = <2>;
> > > >
> > > > Doesn't this end up limiting the number of audio channels HDMI can carry?
> > > > AFAICT the TDM properties are all optional, so just leave it out.
> > > >
> > > > Same goes for the other two patches.
> > > >
> > > > > +                       dai-tdm-slot-width = <32>;
> > > > > +               };
> > > > > +       };
> > > > > +
> > > > >         osc24M: osc24M_clk {
> > > > >                 #clock-cells = <0>;
> > > > >                 compatible = "fixed-clock";
> > > > > @@ -609,6 +628,19 @@ mdio: mdio {
> > > > >                         };
> > > > >                 };
> > > > >
> > > > > +               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";
> > >
> > > Sorry, missed this one.
> > >
> > > Given that usage for this interface is transmt only, and there is no
> > > RX DRQ number assigned to it, you should drop the RX DMA number and name.
> >
> > Indeed if there is no DRQ number assigned we shouldn't have it in the
> > device-tree

After reviewing the H6 User Manual it seems to be assigned :
page 269 3.10.3.3 DRQ TYPE
port 4 => I2S/PCM1-RX /  I2S/PCM1-TX
Same for A64
But indeed it's not mapped for H5.

I will fix the yaml for H3/H5 but not for A64/H6.

Regards,
Clement


> >
> > but Samuel told me that the `make dtbs_check` reports:
> >
> > i2s@1c22800: dma-names:0: 'rx' was expected
> > i2s@1c22800: dma-names: ['tx'] is too short
> > i2s@1c22800: dmas: [[28, 27]] is too short
> >
> > Should I fix the YAML so?
>
> Yep :)
>
> Maxime