[alsa-devel] [PATCH v2 0/6] ASoC: sun4i-i2s: Updates to the driver
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(-)
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);
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.
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 thevalue specified. Min 8, Max 32.
This sounds like something that would be useful for other I2S controllers.
Thanks,
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);
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,
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
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;
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 theoutputs enabled.
Isn't it something that is fixed for each generation of SoCs? Can't we attach it to the compatible?
Maxime
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 theoutputs 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
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 theoutputs 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
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 theoutputs 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/997c622ae10ef136bae0a35a0e9... and https://github.com/codekipper/linux-sunxi/commit/4a81676d30a0262c57ffc0827e4...
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
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)
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);
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 inputfifo.
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.
Hi Mark,
+- loopback: if this property is present then the dai is configured in
loopback mode where the output fifo is redirected to the inputfifo.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...
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 inputfifo.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
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 inputfifo.
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.
participants (6)
-
Chris Obbard -
Code Kipper -
codekipper＠gmail.com
-
Julian Calaby -
Mark Brown -
Maxime Ripard