[PATCH v2 0/6] ASoC: sun4i-i2s: Updates to the driver

[PATCH v2 0/6] ASoC: sun4i-i2s: Updates to the driver

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

Hi All,

here is a patch series to add some improvements to the sun4i-i2s driver
found whilst getting slave clocking and hdmi audio working on the newer
SoCs. The original attempt had some changes related to the slave work
but I've left them out for this release as I would still like to do some
more investigations.

The functionality included with the new patch set has been extended to
cover more sample resolutions, multi-lane data output for HDMI audio
and loopback so block testing can be made without an external codec.

I've also moved away from using tdm to set the slot width and now use
a dedicated property.

This has been tested on a Pine64 using the ES9023 audio POT board
(https://github.com/codekipper/linux-sunxi/commits/upstream)
and HDMI audio
(https://github.com/codekipper/linux-sunxi/commits/sunxi-wip-a64)

BR,
CK

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

---

Marcus Cooper (6):
  ASoC: sun4i-i2s: Add slot width override
  ASoC: sun4i-i2s: Add regmap field to sign extend sample
  ASoC: sun4i-i2s: Correct divider calculations
  ASoC: sun4i-i2s: Add multi-lane functionality
  ASoc: sun4i-i2s: Add 20, 24 and 32 bit support
  ASoC: sun4i-i2s: Add support for loopback

 .../devicetree/bindings/sound/sun4i-i2s.txt        |  11 ++
 sound/soc/sunxi/sun4i-i2s.c                        | 199 ++++++++++++++++-----
 2 files changed, 170 insertions(+), 40 deletions(-)

-- 
2.16.2

[PATCH v2 1/6] ASoC: sun4i-i2s: Add slot width override

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

Some codecs require a different amount of a bit clocks per frame
than what is calculated by using the sample width. Use a slot
width override property to provide this mechanism.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  5 +++++
 sound/soc/sunxi/sun4i-i2s.c                           | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index b9d50d6cdef3..48addef65b8f 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -28,6 +28,11 @@ Required properties for the following compatibles:
 	- "allwinner,sun8i-h3-i2s"
 - resets: phandle to the reset line for this codec
 
+Optional properties:
+- allwinner,slot-width-override:if this property is present then the dai is
+				configured to extend the slot width to the
+				value specified. Min 8, Max 32.
+
 Example:
 
 i2s0: i2s@1c22400 {
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index a4aa931ebfae..873054a6c3be 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -193,6 +193,8 @@ struct sun4i_i2s {
 	struct regmap_field	*field_rxchansel;
 
 	const struct sun4i_i2s_quirks	*variant;
+
+	unsigned int	slot_width;
 };
 
 struct sun4i_i2s_clk_div {
@@ -344,7 +346,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 	if (i2s->variant->has_fmt_set_lrck_period)
 		regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
 				   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
-				   SUN8I_I2S_FMT0_LRCK_PERIOD(32));
+				   SUN8I_I2S_FMT0_LRCK_PERIOD(word_size));
 
 	return 0;
 }
@@ -418,7 +420,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 			   sr + i2s->variant->fmt_offset);
 
 	return sun4i_i2s_set_clk_rate(dai, params_rate(params),
-				      params_width(params));
+				      i2s->slot_width ?
+				      i2s->slot_width : params_width(params));
 }
 
 static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
@@ -1029,7 +1032,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 	struct sun4i_i2s *i2s;
 	struct resource *res;
 	void __iomem *regs;
-	int irq, ret;
+	int irq, ret, val;
 
 	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
 	if (!i2s)
@@ -1096,6 +1099,12 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 	i2s->capture_dma_data.addr = res->start + SUN4I_I2S_FIFO_RX_REG;
 	i2s->capture_dma_data.maxburst = 8;
 
+	if (!of_property_read_u32(pdev->dev.of_node,
+				  "allwinner,slot-width-override", &val)) {
+		if (val >= 8 && val <= 32)
+			i2s->slot_width = val;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = sun4i_i2s_runtime_resume(&pdev->dev);
-- 
2.16.2

[PATCH v2 2/6] ASoC: sun4i-i2s: Add regmap field to sign extend sample

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

On the newer SoCs (H3, H5, A64 etc) this is set by default to
transfer a 0 after each sample in each slot whereas on the
earlier SoCs (A20, A31 etc) the default sign extension is to
pad the LSB.

Add the regmap field to configure this and set it so that it
pads the sample with 0s.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 873054a6c3be..396b11346361 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -138,6 +138,7 @@
  * @field_fmt_bclk: regmap field to set clk polarity.
  * @field_fmt_lrclk: regmap field to set frame polarity.
  * @field_fmt_mode: regmap field to set the operational mode.
+ * @field_fmt_sext: regmap field to set the sign extension.
  * @field_txchanmap: location of the tx channel mapping register.
  * @field_rxchanmap: location of the rx channel mapping register.
  * @field_txchansel: location of the tx channel select bit fields.
@@ -163,6 +164,7 @@ struct sun4i_i2s_quirks {
 	struct reg_field		field_fmt_bclk;
 	struct reg_field		field_fmt_lrclk;
 	struct reg_field		field_fmt_mode;
+	struct reg_field		field_fmt_sext;
 	struct reg_field		field_txchanmap;
 	struct reg_field		field_rxchanmap;
 	struct reg_field		field_txchansel;
@@ -187,6 +189,7 @@ struct sun4i_i2s {
 	struct regmap_field	*field_fmt_bclk;
 	struct regmap_field	*field_fmt_lrclk;
 	struct regmap_field	*field_fmt_mode;
+	struct regmap_field	*field_fmt_sext;
 	struct regmap_field	*field_txchanmap;
 	struct regmap_field	*field_rxchanmap;
 	struct regmap_field	*field_txchansel;
@@ -348,6 +351,9 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 				   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
 				   SUN8I_I2S_FMT0_LRCK_PERIOD(word_size));
 
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_field_write(i2s->field_fmt_sext, 0);
+
 	return 0;
 }
 
@@ -901,6 +907,7 @@ static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
 	.field_fmt_lrclk	= REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
 	.has_slave_select_bit	= true,
 	.field_fmt_mode		= REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
 	.field_txchanmap	= REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
 	.field_rxchanmap	= REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
 	.field_txchansel	= REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
@@ -918,6 +925,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
 	.field_fmt_lrclk	= REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
 	.has_slave_select_bit	= true,
 	.field_fmt_mode		= REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
 	.field_txchanmap	= REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
 	.field_rxchanmap	= REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
 	.field_txchansel	= REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
@@ -958,6 +966,7 @@ static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
 	.field_fmt_bclk		= REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
 	.field_fmt_lrclk	= REG_FIELD(SUN4I_I2S_FMT0_REG, 19, 19),
 	.field_fmt_mode		= REG_FIELD(SUN4I_I2S_CTRL_REG, 4, 5),
+	.field_fmt_sext		= REG_FIELD(SUN4I_I2S_FMT1_REG, 4, 5),
 	.field_txchanmap	= REG_FIELD(SUN8I_I2S_TX_CHAN_MAP_REG, 0, 31),
 	.field_rxchanmap	= REG_FIELD(SUN8I_I2S_RX_CHAN_MAP_REG, 0, 31),
 	.field_txchansel	= REG_FIELD(SUN8I_I2S_TX_CHAN_SEL_REG, 0, 2),
@@ -1003,6 +1012,12 @@ static int sun4i_i2s_init_regmap_fields(struct device *dev,
 	if (IS_ERR(i2s->field_fmt_mode))
 		return PTR_ERR(i2s->field_fmt_mode);
 
+	i2s->field_fmt_sext =
+			devm_regmap_field_alloc(dev, i2s->regmap,
+						i2s->variant->field_fmt_sext);
+	if (IS_ERR(i2s->field_fmt_sext))
+		return PTR_ERR(i2s->field_fmt_sext);
+
 	i2s->field_txchanmap =
 			devm_regmap_field_alloc(dev, i2s->regmap,
 						i2s->variant->field_txchanmap);
-- 
2.16.2

[PATCH v2 3/6] ASoC: sun4i-i2s: Correct divider calculations

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

The clock division circuitry is different on the H3 and later SoCs.
The division of bclk is now based on pll2.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 76 +++++++++++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 396b11346361..2bd0befa02a8 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -129,10 +129,10 @@
  * @has_chsel_offset: SoC uses offset for selecting dai operational mode.
  * @reg_offset_txdata: offset of the tx fifo.
  * @sun4i_i2s_regmap: regmap config to use.
- * @mclk_offset: Value by which mclkdiv needs to be adjusted.
- * @bclk_offset: Value by which bclkdiv needs to be adjusted.
  * @fmt_offset: Value by which wss and sr needs to be adjusted.
  * @field_clkdiv_mclk_en: regmap field to enable mclk output.
+ * @field_clkdiv_mclk: regmap field for mclkdiv.
+ * @field_clkdiv_bclk: regmap field for bclkdiv.
  * @field_fmt_wss: regmap field to set word select size.
  * @field_fmt_sr: regmap field to set sample resolution.
  * @field_fmt_bclk: regmap field to set clk polarity.
@@ -153,8 +153,6 @@ struct sun4i_i2s_quirks {
 	bool				has_chsel_offset;
 	unsigned int			reg_offset_txdata;	/* TX FIFO */
 	const struct regmap_config	*sun4i_i2s_regmap;
-	unsigned int			mclk_offset;
-	unsigned int			bclk_offset;
 	unsigned int			fmt_offset;
 
 	/* Register fields for i2s */
@@ -212,7 +210,25 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
 	{ .div = 8, .val = 3 },
 	{ .div = 12, .val = 4 },
 	{ .div = 16, .val = 5 },
-	/* TODO - extend divide ratio supported by newer SoCs */
+};
+
+static const struct sun4i_i2s_clk_div sun8i_i2s_clk_div[] = {
+	{ .div = 0, .val = 0 },
+	{ .div = 1, .val = 1 },
+	{ .div = 2, .val = 2 },
+	{ .div = 4, .val = 3 },
+	{ .div = 6, .val = 4 },
+	{ .div = 8, .val = 5 },
+	{ .div = 12, .val = 6 },
+	{ .div = 16, .val = 7 },
+	{ .div = 24, .val = 8 },
+	{ .div = 32, .val = 9 },
+	{ .div = 48, .val = 10 },
+	{ .div = 64, .val = 11 },
+	{ .div = 96, .val = 12 },
+	{ .div = 128, .val = 13 },
+	{ .div = 176, .val = 14 },
+	{ .div = 192, .val = 15 },
 };
 
 static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
@@ -224,21 +240,21 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
 	{ .div = 12, .val = 5 },
 	{ .div = 16, .val = 6 },
 	{ .div = 24, .val = 7 },
-	/* TODO - extend divide ratio supported by newer SoCs */
 };
 
 static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
 				  unsigned int oversample_rate,
-				  unsigned int word_size)
+				  unsigned int word_size,
+				  const struct sun4i_i2s_clk_div *bdiv,
+				  unsigned int size)
 {
 	int div = oversample_rate / word_size / 2;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(sun4i_i2s_bclk_div); i++) {
-		const struct sun4i_i2s_clk_div *bdiv = &sun4i_i2s_bclk_div[i];
-
+	for (i = 0; i < size; i++) {
 		if (bdiv->div == div)
 			return bdiv->val;
+		bdiv++;
 	}
 
 	return -EINVAL;
@@ -247,16 +263,17 @@ static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
 static int sun4i_i2s_get_mclk_div(struct sun4i_i2s *i2s,
 				  unsigned int oversample_rate,
 				  unsigned int module_rate,
-				  unsigned int sampling_rate)
+				  unsigned int sampling_rate,
+				  const struct sun4i_i2s_clk_div *mdiv,
+				  unsigned int size)
 {
 	int div = module_rate / sampling_rate / oversample_rate;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(sun4i_i2s_mclk_div); i++) {
-		const struct sun4i_i2s_clk_div *mdiv = &sun4i_i2s_mclk_div[i];
-
+	for (i = 0; i < size; i++) {
 		if (mdiv->div == div)
 			return mdiv->val;
+		mdiv++;
 	}
 
 	return -EINVAL;
@@ -321,24 +338,37 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 		return -EINVAL;
 	}
 
-	bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
-					  word_size);
+	if (i2s->variant->has_fmt_set_lrck_period)
+		bclk_div = sun4i_i2s_get_bclk_div(i2s, clk_rate / rate,
+						  word_size,
+						  sun8i_i2s_clk_div,
+						  ARRAY_SIZE(sun8i_i2s_clk_div));
+	else
+		bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
+						  word_size,
+						  sun4i_i2s_bclk_div,
+						  ARRAY_SIZE(sun4i_i2s_bclk_div));
 	if (bclk_div < 0) {
 		dev_err(dai->dev, "Unsupported BCLK divider: %d\n", bclk_div);
 		return -EINVAL;
 	}
 
-	mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
-					  clk_rate, rate);
+
+	if (i2s->variant->has_fmt_set_lrck_period)
+		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
+						  clk_rate, rate,
+						  sun8i_i2s_clk_div,
+						  ARRAY_SIZE(sun8i_i2s_clk_div));
+	else
+		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
+						  clk_rate, rate,
+						  sun4i_i2s_mclk_div,
+						  ARRAY_SIZE(sun4i_i2s_mclk_div));
 	if (mclk_div < 0) {
 		dev_err(dai->dev, "Unsupported MCLK divider: %d\n", mclk_div);
 		return -EINVAL;
 	}
 
-	/* Adjust the clock division values if needed */
-	bclk_div += i2s->variant->bclk_offset;
-	mclk_div += i2s->variant->mclk_offset;
-
 	regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG,
 		     SUN4I_I2S_CLK_DIV_BCLK(bclk_div) |
 		     SUN4I_I2S_CLK_DIV_MCLK(mclk_div));
@@ -953,8 +983,6 @@ static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
 	.has_reset		= true,
 	.reg_offset_txdata	= SUN8I_I2S_FIFO_TX_REG,
 	.sun4i_i2s_regmap	= &sun8i_i2s_regmap_config,
-	.mclk_offset		= 1,
-	.bclk_offset		= 2,
 	.fmt_offset		= 3,
 	.has_fmt_set_lrck_period = true,
 	.has_chcfg		= true,
-- 
2.16.2

[PATCH v2 4/6] ASoC: sun4i-i2s: Add multi-lane functionality

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

The i2s block supports multi-lane i2s output however this functionality
is only possible in earlier SoCs where the pins are exposed and for
the i2s block used for HDMI audio on the later SoCs.

To enable this functionality, an optional property has been added to
the bindings.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 .../devicetree/bindings/sound/sun4i-i2s.txt        |  3 ++
 sound/soc/sunxi/sun4i-i2s.c                        | 48 +++++++++++++++++-----
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index 48addef65b8f..3f966ac61b9e 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -33,6 +33,9 @@ Optional properties:
 				configured to extend the slot width to the
 				value specified. Min 8, Max 32.
 
+- allwinner,playback-channels:  if this property is present then the number
+				of available channels is extended and the
+				outputs enabled.
 Example:
 
 i2s0: i2s@1c22400 {
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 2bd0befa02a8..436480df1844 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -27,7 +27,7 @@
 
 #define SUN4I_I2S_CTRL_REG		0x00
 #define SUN4I_I2S_CTRL_SDO_EN_MASK		GENMASK(11, 8)
-#define SUN4I_I2S_CTRL_SDO_EN(sdo)			BIT(8 + (sdo))
+#define SUN4I_I2S_CTRL_SDO_EN(lines)		(((1 << lines) - 1) << 8)
 #define SUN4I_I2S_CTRL_MODE_MASK		BIT(5)
 #define SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
 #define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
@@ -394,14 +394,23 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
 	int sr, wss, channels;
 	u32 width;
+	int lines;
 
 	channels = params_channels(params);
-	if (channels != 2) {
+	if ((channels > dai->driver->playback.channels_max) ||
+		(channels < dai->driver->playback.channels_min)) {
 		dev_err(dai->dev, "Unsupported number of channels: %d\n",
 			channels);
 		return -EINVAL;
 	}
 
+	lines = (channels + 1) / 2;
+
+	/* Enable the required output lines */
+	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+			   SUN4I_I2S_CTRL_SDO_EN_MASK,
+			   SUN4I_I2S_CTRL_SDO_EN(lines));
+
 	if (i2s->variant->has_chcfg) {
 		regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
 				   SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
@@ -412,8 +421,19 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* Map the channels for playback and capture */
-	regmap_field_write(i2s->field_txchanmap, 0x76543210);
 	regmap_field_write(i2s->field_rxchanmap, 0x00003210);
+	regmap_field_write(i2s->field_txchanmap, 0x10);
+	if (i2s->variant->has_chsel_tx_chen) {
+		if (channels > 2)
+			regmap_write(i2s->regmap,
+				     SUN8I_I2S_TX_CHAN_MAP_REG+4, 0x32);
+		if (channels > 4)
+			regmap_write(i2s->regmap,
+				     SUN8I_I2S_TX_CHAN_MAP_REG+8, 0x54);
+		if (channels > 6)
+			regmap_write(i2s->regmap,
+				     SUN8I_I2S_TX_CHAN_MAP_REG+12, 0x76);
+	}
 
 	/* Configure the channels */
 	regmap_field_write(i2s->field_txchansel,
@@ -692,12 +712,6 @@ static int sun4i_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
 			   SUN4I_I2S_CTRL_GL_EN, SUN4I_I2S_CTRL_GL_EN);
 
-	/* Enable the first output line */
-	regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
-			   SUN4I_I2S_CTRL_SDO_EN_MASK,
-			   SUN4I_I2S_CTRL_SDO_EN(0));
-
-
 	return clk_prepare_enable(i2s->mod_clk);
 }
 
@@ -1073,6 +1087,7 @@ static int sun4i_i2s_init_regmap_fields(struct device *dev,
 static int sun4i_i2s_probe(struct platform_device *pdev)
 {
 	struct sun4i_i2s *i2s;
+	struct snd_soc_dai_driver *soc_dai;
 	struct resource *res;
 	void __iomem *regs;
 	int irq, ret, val;
@@ -1148,6 +1163,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 			i2s->slot_width = val;
 	}
 
+	soc_dai = devm_kmemdup(&pdev->dev, &sun4i_i2s_dai,
+			       sizeof(*soc_dai), GFP_KERNEL);
+	if (!soc_dai) {
+		ret = -ENOMEM;
+		goto err_pm_disable;
+	}
+
+	if (!of_property_read_u32(pdev->dev.of_node,
+				  "allwinner,playback-channels", &val)) {
+		if (val >= 2 && val <= 8)
+			soc_dai->playback.channels_max = val;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = sun4i_i2s_runtime_resume(&pdev->dev);
@@ -1157,7 +1185,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 
 	ret = devm_snd_soc_register_component(&pdev->dev,
 					      &sun4i_i2s_component,
-					      &sun4i_i2s_dai, 1);
+					      soc_dai, 1);
 	if (ret) {
 		dev_err(&pdev->dev, "Could not register DAI\n");
 		goto err_suspend;
-- 
2.16.2

[PATCH v2 5/6] ASoc: sun4i-i2s: Add 20, 24 and 32 bit support

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

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

Newer SoCs can also handle 32bit samples.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 436480df1844..7cf4dfe440de 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -451,6 +451,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 	case 16:
 		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		break;
+	case 20:
+	case 24:
+	case 32:
+		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
 	default:
 		dev_err(dai->dev, "Unsupported physical sample width: %d\n",
 			params_physical_width(params));
@@ -463,7 +468,18 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
 		sr = 0;
 		wss = 0;
 		break;
-
+	case 20:
+		sr = 1;
+		wss = 1;
+		break;
+	case 24:
+		sr = 2;
+		wss = 2;
+		break;
+	case 32:
+		sr = 4;
+		wss = 4;
+		break;
 	default:
 		dev_err(dai->dev, "Unsupported sample width: %d\n",
 			params_width(params));
@@ -766,6 +782,13 @@ 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_3LE | \
+			 SNDRV_PCM_FMTBIT_S24_LE)
+
+#define SUN8I_FORMATS	(SUN4I_FORMATS | \
+			 SNDRV_PCM_FMTBIT_S32_LE)
+
 static struct snd_soc_dai_driver sun4i_i2s_dai = {
 	.probe = sun4i_i2s_dai_probe,
 	.capture = {
@@ -773,14 +796,14 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {
 		.channels_min = 2,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_8000_192000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SUN4I_FORMATS,
 	},
 	.playback = {
 		.stream_name = "Playback",
 		.channels_min = 2,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_8000_192000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SUN4I_FORMATS,
 	},
 	.ops = &sun4i_i2s_dai_ops,
 	.symmetric_rates = 1,
@@ -1170,6 +1193,11 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 		goto err_pm_disable;
 	}
 
+	if (i2s->variant->has_fmt_set_lrck_period) {
+		soc_dai->playback.formats = SUN8I_FORMATS;
+		soc_dai->capture.formats = SUN8I_FORMATS;
+	}
+
 	if (!of_property_read_u32(pdev->dev.of_node,
 				  "allwinner,playback-channels", &val)) {
 		if (val >= 2 && val <= 8)
-- 
2.16.2

[PATCH v2 6/6] ASoC: sun4i-i2s: Add support for loopback

[codekipper]

From: Marcus Cooper <codekipper@gmail.com>

The DAI has a loopback register which can be set and therefore routes
the transmit fifo to receive fifo. This is useful for testing the block
without the need for any external hardware.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  3 +++
 sound/soc/sunxi/sun4i-i2s.c                           | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index 3f966ac61b9e..325e3d4571f1 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -32,6 +32,9 @@ Optional properties:
 - allwinner,slot-width-override:if this property is present then the dai is
 				configured to extend the slot width to the
 				value specified. Min 8, Max 32.
+- loopback:	if this property is present then the dai is configured in
+		loopback mode where the output fifo is redirected to the input
+		fifo.
 
 - allwinner,playback-channels:  if this property is present then the number
 				of available channels is extended and the
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 7cf4dfe440de..a3c33814d33e 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -31,6 +31,7 @@
 #define SUN4I_I2S_CTRL_MODE_MASK		BIT(5)
 #define SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
 #define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
+#define SUN4I_I2S_CTRL_LOOP			BIT(3)
 #define SUN4I_I2S_CTRL_TX_EN			BIT(2)
 #define SUN4I_I2S_CTRL_RX_EN			BIT(1)
 #define SUN4I_I2S_CTRL_GL_EN			BIT(0)
@@ -195,6 +196,8 @@ struct sun4i_i2s {
 
 	const struct sun4i_i2s_quirks	*variant;
 
+	bool loopback;
+
 	unsigned int	slot_width;
 };
 
@@ -639,6 +642,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
 			   SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN,
 			   SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN);
+
+	/* Debugging without codec */
+	if (i2s->loopback)
+		regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+				   SUN4I_I2S_CTRL_LOOP, SUN4I_I2S_CTRL_LOOP);
 }
 
 static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
@@ -1204,6 +1212,9 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 			soc_dai->playback.channels_max = val;
 	}
 
+	if (of_property_read_bool(pdev->dev.of_node, "loopback"))
+		i2s->loopback = true;
+
 	pm_runtime_enable(&pdev->dev);
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = sun4i_i2s_runtime_resume(&pdev->dev);
-- 
2.16.2

Re: [PATCH v2 1/6] ASoC: sun4i-i2s: Add slot width override

[Mark Brown]

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

On Mon, Mar 12, 2018 at 04:57:48PM +0100, codekipper@gmail.com wrote:

> Some codecs require a different amount of a bit clocks per frame
> than what is calculated by using the sample width. Use a slot
> width override property to provide this mechanism.

If this is a CODEC requirement we should really be working this out from
the CODEC rather than adding a DT property on the I2S controller, or at
the very least putting this in the machine drivers.  The generic drivers
already have support for mclk-fs and there's an operation set_bclk_ratio() 
for them to use though we don't have a DT binding for that yet.

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

Re: [PATCH v2 6/6] ASoC: sun4i-i2s: Add support for loopback

[Mark Brown]

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

On Mon, Mar 12, 2018 at 04:57:53PM +0100, codekipper@gmail.com wrote:

> +- loopback:	if this property is present then the dai is configured in
> +		loopback mode where the output fifo is redirected to the input
> +		fifo.

This really doesn't seem like something that ought to go into the DT
and hence ABI given that there's obviously no reason to use this in
production.  Just make it be a #define in the code or something.

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

Re: [PATCH v2 4/6] ASoC: sun4i-i2s: Add multi-lane functionality

[Maxime Ripard]

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

On Mon, Mar 12, 2018 at 04:57:51PM +0100, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> The i2s block supports multi-lane i2s output however this functionality
> is only possible in earlier SoCs where the pins are exposed and for
> the i2s block used for HDMI audio on the later SoCs.
> 
> To enable this functionality, an optional property has been added to
> the bindings.
> 
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  .../devicetree/bindings/sound/sun4i-i2s.txt        |  3 ++
>  sound/soc/sunxi/sun4i-i2s.c                        | 48 +++++++++++++++++-----
>  2 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index 48addef65b8f..3f966ac61b9e 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -33,6 +33,9 @@ Optional properties:
>  				configured to extend the slot width to the
>  				value specified. Min 8, Max 32.
>  
> +- allwinner,playback-channels:  if this property is present then the number
> +				of available channels is extended and the
> +				outputs enabled.

Isn't it something that is fixed for each generation of SoCs? Can't we
attach it to the compatible?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v2 4/6] ASoC: sun4i-i2s: Add multi-lane functionality

[Code Kipper]

On 13 March 2018 at 09:00, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> On Mon, Mar 12, 2018 at 04:57:51PM +0100, codekipper@gmail.com wrote:
>> From: Marcus Cooper <codekipper@gmail.com>
>>
>> The i2s block supports multi-lane i2s output however this functionality
>> is only possible in earlier SoCs where the pins are exposed and for
>> the i2s block used for HDMI audio on the later SoCs.
>>
>> To enable this functionality, an optional property has been added to
>> the bindings.
>>
>> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>> ---
>>  .../devicetree/bindings/sound/sun4i-i2s.txt        |  3 ++
>>  sound/soc/sunxi/sun4i-i2s.c                        | 48 +++++++++++++++++-----
>>  2 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> index 48addef65b8f..3f966ac61b9e 100644
>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> @@ -33,6 +33,9 @@ Optional properties:
>>                               configured to extend the slot width to the
>>                               value specified. Min 8, Max 32.
>>
>> +- allwinner,playback-channels:  if this property is present then the number
>> +                             of available channels is extended and the
>> +                             outputs enabled.
>
> Isn't it something that is fixed for each generation of SoCs? Can't we
> attach it to the compatible?

I'm not sure as the documentation is pretty poor. It looks like it is
supported by all of the variations that we've seen but only exposed as
pins on A10 and A20. I'm also not sure who would ever use it with
external devices.
The reason why I've added it is that it is required by HDMI for
supporting surround sound on the newer SoCs.
CK

>
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH v2 3/6] ASoC: sun4i-i2s: Correct divider calculations

[Maxime Ripard]

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

On Mon, Mar 12, 2018 at 04:57:50PM +0100, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> The clock division circuitry is different on the H3 and later SoCs.
> The division of bclk is now based on pll2.

You're doing too many things here.

> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 76 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 52 insertions(+), 24 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 396b11346361..2bd0befa02a8 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -129,10 +129,10 @@
>   * @has_chsel_offset: SoC uses offset for selecting dai operational mode.
>   * @reg_offset_txdata: offset of the tx fifo.
>   * @sun4i_i2s_regmap: regmap config to use.
> - * @mclk_offset: Value by which mclkdiv needs to be adjusted.
> - * @bclk_offset: Value by which bclkdiv needs to be adjusted.
>   * @fmt_offset: Value by which wss and sr needs to be adjusted.
>   * @field_clkdiv_mclk_en: regmap field to enable mclk output.
> + * @field_clkdiv_mclk: regmap field for mclkdiv.
> + * @field_clkdiv_bclk: regmap field for bclkdiv.
>   * @field_fmt_wss: regmap field to set word select size.
>   * @field_fmt_sr: regmap field to set sample resolution.
>   * @field_fmt_bclk: regmap field to set clk polarity.
> @@ -153,8 +153,6 @@ struct sun4i_i2s_quirks {
>  	bool				has_chsel_offset;
>  	unsigned int			reg_offset_txdata;	/* TX FIFO */
>  	const struct regmap_config	*sun4i_i2s_regmap;
> -	unsigned int			mclk_offset;
> -	unsigned int			bclk_offset;
>  	unsigned int			fmt_offset;
>  
>  	/* Register fields for i2s */
> @@ -212,7 +210,25 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
>  	{ .div = 8, .val = 3 },
>  	{ .div = 12, .val = 4 },
>  	{ .div = 16, .val = 5 },
> -	/* TODO - extend divide ratio supported by newer SoCs */
> +};
> +
> +static const struct sun4i_i2s_clk_div sun8i_i2s_clk_div[] = {
> +	{ .div = 0, .val = 0 },
> +	{ .div = 1, .val = 1 },
> +	{ .div = 2, .val = 2 },
> +	{ .div = 4, .val = 3 },
> +	{ .div = 6, .val = 4 },
> +	{ .div = 8, .val = 5 },
> +	{ .div = 12, .val = 6 },
> +	{ .div = 16, .val = 7 },
> +	{ .div = 24, .val = 8 },
> +	{ .div = 32, .val = 9 },
> +	{ .div = 48, .val = 10 },
> +	{ .div = 64, .val = 11 },
> +	{ .div = 96, .val = 12 },
> +	{ .div = 128, .val = 13 },
> +	{ .div = 176, .val = 14 },
> +	{ .div = 192, .val = 15 },
>  };
>  
>  static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
> @@ -224,21 +240,21 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
>  	{ .div = 12, .val = 5 },
>  	{ .div = 16, .val = 6 },
>  	{ .div = 24, .val = 7 },
> -	/* TODO - extend divide ratio supported by newer SoCs */
>  };
>  
>  static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
>  				  unsigned int oversample_rate,
> -				  unsigned int word_size)
> +				  unsigned int word_size,
> +				  const struct sun4i_i2s_clk_div *bdiv,
> +				  unsigned int size)
>  {
>  	int div = oversample_rate / word_size / 2;
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(sun4i_i2s_bclk_div); i++) {
> -		const struct sun4i_i2s_clk_div *bdiv = &sun4i_i2s_bclk_div[i];
> -
> +	for (i = 0; i < size; i++) {
>  		if (bdiv->div == div)
>  			return bdiv->val;
> +		bdiv++;
>  	}
>  
>  	return -EINVAL;
> @@ -247,16 +263,17 @@ static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
>  static int sun4i_i2s_get_mclk_div(struct sun4i_i2s *i2s,
>  				  unsigned int oversample_rate,
>  				  unsigned int module_rate,
> -				  unsigned int sampling_rate)
> +				  unsigned int sampling_rate,
> +				  const struct sun4i_i2s_clk_div *mdiv,
> +				  unsigned int size)
>  {
>  	int div = module_rate / sampling_rate / oversample_rate;
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(sun4i_i2s_mclk_div); i++) {
> -		const struct sun4i_i2s_clk_div *mdiv = &sun4i_i2s_mclk_div[i];
> -
> +	for (i = 0; i < size; i++) {
>  		if (mdiv->div == div)
>  			return mdiv->val;
> +		mdiv++;
>  	}
>  
>  	return -EINVAL;
> @@ -321,24 +338,37 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
>  		return -EINVAL;
>  	}
>  
> -	bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
> -					  word_size);
> +	if (i2s->variant->has_fmt_set_lrck_period)
> +		bclk_div = sun4i_i2s_get_bclk_div(i2s, clk_rate / rate,
> +						  word_size,
> +						  sun8i_i2s_clk_div,
> +						  ARRAY_SIZE(sun8i_i2s_clk_div));

For example, you mention nowhere why here you would need clk_rate /
rate, instead of the previous oversample_rate.

> +	else
> +		bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate,
> +						  word_size,
> +						  sun4i_i2s_bclk_div,
> +						  ARRAY_SIZE(sun4i_i2s_bclk_div));


>  	if (bclk_div < 0) {
>  		dev_err(dai->dev, "Unsupported BCLK divider: %d\n", bclk_div);
>  		return -EINVAL;
>  	}
>  
> -	mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
> -					  clk_rate, rate);
> +
> +	if (i2s->variant->has_fmt_set_lrck_period)
> +		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
> +						  clk_rate, rate,
> +						  sun8i_i2s_clk_div,
> +						  ARRAY_SIZE(sun8i_i2s_clk_div));
> +	else
> +		mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate,
> +						  clk_rate, rate,
> +						  sun4i_i2s_mclk_div,
> +						  ARRAY_SIZE(sun4i_i2s_mclk_div));

While here it's ok to use oversample_rate.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v2 4/6] ASoC: sun4i-i2s: Add multi-lane functionality

[Maxime Ripard]

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

On Tue, Mar 13, 2018 at 09:15:49AM +0100, Code Kipper wrote:
> On 13 March 2018 at 09:00, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Mon, Mar 12, 2018 at 04:57:51PM +0100, codekipper@gmail.com wrote:
> >> From: Marcus Cooper <codekipper@gmail.com>
> >>
> >> The i2s block supports multi-lane i2s output however this functionality
> >> is only possible in earlier SoCs where the pins are exposed and for
> >> the i2s block used for HDMI audio on the later SoCs.
> >>
> >> To enable this functionality, an optional property has been added to
> >> the bindings.
> >>
> >> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> >> ---
> >>  .../devicetree/bindings/sound/sun4i-i2s.txt        |  3 ++
> >>  sound/soc/sunxi/sun4i-i2s.c                        | 48 +++++++++++++++++-----
> >>  2 files changed, 41 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> >> index 48addef65b8f..3f966ac61b9e 100644
> >> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> >> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> >> @@ -33,6 +33,9 @@ Optional properties:
> >>                               configured to extend the slot width to the
> >>                               value specified. Min 8, Max 32.
> >>
> >> +- allwinner,playback-channels:  if this property is present then the number
> >> +                             of available channels is extended and the
> >> +                             outputs enabled.
> >
> > Isn't it something that is fixed for each generation of SoCs? Can't we
> > attach it to the compatible?
> 
> I'm not sure as the documentation is pretty poor. It looks like it is
> supported by all of the variations that we've seen but only exposed as
> pins on A10 and A20. I'm also not sure who would ever use it with
> external devices.

Well, you were saying that it was the case on the older SoCs in your
commit log, so this needs to be figured out.

> The reason why I've added it is that it is required by HDMI for
> supporting surround sound on the newer SoCs.

Why isn't that mentionned in your commit log?

Can you post the whole set of changes needed for HDMI audio? this
would make things much easier to see where you're going.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v2 4/6] ASoC: sun4i-i2s: Add multi-lane functionality

[Code Kipper]

On 13 March 2018 at 09:23, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> On Tue, Mar 13, 2018 at 09:15:49AM +0100, Code Kipper wrote:
>> On 13 March 2018 at 09:00, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>> > On Mon, Mar 12, 2018 at 04:57:51PM +0100, codekipper@gmail.com wrote:
>> >> From: Marcus Cooper <codekipper@gmail.com>
>> >>
>> >> The i2s block supports multi-lane i2s output however this functionality
>> >> is only possible in earlier SoCs where the pins are exposed and for
>> >> the i2s block used for HDMI audio on the later SoCs.
>> >>
>> >> To enable this functionality, an optional property has been added to
>> >> the bindings.
>> >>
>> >> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>> >> ---
>> >>  .../devicetree/bindings/sound/sun4i-i2s.txt        |  3 ++
>> >>  sound/soc/sunxi/sun4i-i2s.c                        | 48 +++++++++++++++++-----
>> >>  2 files changed, 41 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> >> index 48addef65b8f..3f966ac61b9e 100644
>> >> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> >> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> >> @@ -33,6 +33,9 @@ Optional properties:
>> >>                               configured to extend the slot width to the
>> >>                               value specified. Min 8, Max 32.
>> >>
>> >> +- allwinner,playback-channels:  if this property is present then the number
>> >> +                             of available channels is extended and the
>> >> +                             outputs enabled.
>> >
>> > Isn't it something that is fixed for each generation of SoCs? Can't we
>> > attach it to the compatible?
>>
>> I'm not sure as the documentation is pretty poor. It looks like it is
>> supported by all of the variations that we've seen but only exposed as
>> pins on A10 and A20. I'm also not sure who would ever use it with
>> external devices.
>
> Well, you were saying that it was the case on the older SoCs in your
> commit log, so this needs to be figured out.
>
>> The reason why I've added it is that it is required by HDMI for
>> supporting surround sound on the newer SoCs.
>
> Why isn't that mentionned in your commit log?
>
> Can you post the whole set of changes needed for HDMI audio? this
> would make things much easier to see where you're going.

Maybe we should drop this change for now as basic HDMI audio for two
channels doesn't require it. I've also not tested multi-channel audio
yet.

To get HDMI audio working the following patches are required for the dts
https://github.com/codekipper/linux-sunxi/commit/997c622ae10ef136bae0a35a0e9bf9c6dd27910a
and
https://github.com/codekipper/linux-sunxi/commit/4a81676d30a0262c57ffc0827e46c7934fb3b358

they don't need this patch but do need the slot-width-override patch.

CK
>
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [linux-sunxi] [PATCH v2 1/6] ASoC: sun4i-i2s: Add slot width override

[Julian Calaby]

Hi Marcus,

On Tue, Mar 13, 2018 at 2:57 AM,  <codekipper@gmail.com> wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> Some codecs require a different amount of a bit clocks per frame
> than what is calculated by using the sample width. Use a slot
> width override property to provide this mechanism.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  5 +++++
>  sound/soc/sunxi/sun4i-i2s.c                           | 15 ++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index b9d50d6cdef3..48addef65b8f 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -28,6 +28,11 @@ Required properties for the following compatibles:
>         - "allwinner,sun8i-h3-i2s"
>  - resets: phandle to the reset line for this codec
>
> +Optional properties:
> +- allwinner,slot-width-override:if this property is present then the dai is
> +                               configured to extend the slot width to the
> +                               value specified. Min 8, Max 32.
> +

This sounds like something that would be useful for other I2S controllers.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] Re: [PATCH v2 6/6] ASoC: sun4i-i2s: Add support for loopback

[Chris Obbard]

Hi Mark,

>
>> +- loopback:  if this property is present then the dai is configured in
>> +             loopback mode where the output fifo is redirected to the input
>> +             fifo.
>
> This really doesn't seem like something that ought to go into the DT
> and hence ABI given that there's obviously no reason to use this in
> production.  Just make it be a #define in the code or something.

It would be nice to have this as an optional devicetree property so
when testing JACK latency etc I don't have to manually re-compile a
kernel...

Re: [linux-sunxi] Re: [PATCH v2 6/6] ASoC: sun4i-i2s: Add support for loopback

[Maxime Ripard]

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

On Tue, Mar 13, 2018 at 01:39:44PM +0000, Chris Obbard wrote:
> >> +- loopback:  if this property is present then the dai is configured in
> >> +             loopback mode where the output fifo is redirected to the input
> >> +             fifo.
> >
> > This really doesn't seem like something that ought to go into the DT
> > and hence ABI given that there's obviously no reason to use this in
> > production.  Just make it be a #define in the code or something.
> 
> It would be nice to have this as an optional devicetree property so
> when testing JACK latency etc I don't have to manually re-compile a
> kernel...

Another one might also argue that having to recompile the DT and
reflash the whole firmware or EEPROM is overkill for just testing JACK
latency.

Can't we create a module parameter for it?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [alsa-devel] [linux-sunxi] Re: [PATCH v2 6/6] ASoC: sun4i-i2s: Add support for loopback

[Mark Brown]

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

On Tue, Mar 13, 2018 at 01:39:44PM +0000, Chris Obbard wrote:
> >> +- loopback:  if this property is present then the dai is configured in
> >> +             loopback mode where the output fifo is redirected to the input
> >> +             fifo.

> > This really doesn't seem like something that ought to go into the DT
> > and hence ABI given that there's obviously no reason to use this in
> > production.  Just make it be a #define in the code or something.

> It would be nice to have this as an optional devicetree property so
> when testing JACK latency etc I don't have to manually re-compile a
> kernel...

You'll have to rebuild your DT anyway, I'm not sure the kernel is that
much of a difference...  For your use case you'd want a regular runtime
ALSA control, not a DT property anyway.  That would be fine.

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