2018-03-12 16:03:07

by Code Kipper

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

From: Marcus Cooper <[email protected]>

Hi All,

here is a patch series to add some improvements to the sun4i-i2s driver
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



2018-03-12 15:59:26

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
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


2018-03-12 15:59:45

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
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


2018-03-12 16:00:18

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
.../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


2018-03-12 16:00:52

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
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


2018-03-12 16:01:22

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
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


2018-03-12 16:01:27

by Code Kipper

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

From: Marcus Cooper <[email protected]>

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 <[email protected]>
---
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


2018-03-12 18:22:13

by Mark Brown

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

On Mon, Mar 12, 2018 at 04:57:48PM +0100, [email protected] 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.


Attachments:
(No filename) (625.00 B)
signature.asc (499.00 B)
Download all attachments

2018-03-12 18:36:39

by Mark Brown

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

On Mon, Mar 12, 2018 at 04:57:53PM +0100, [email protected] 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.


Attachments:
(No filename) (435.00 B)
signature.asc (499.00 B)
Download all attachments

2018-03-13 08:01:55

by Maxime Ripard

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

On Mon, Mar 12, 2018 at 04:57:51PM +0100, [email protected] wrote:
> From: Marcus Cooper <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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


Attachments:
(No filename) (1.47 kB)
signature.asc (849.00 B)
Download all attachments

2018-03-13 08:18:32

by Code Kipper

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

On 13 March 2018 at 09:00, Maxime Ripard <[email protected]> wrote:
> On Mon, Mar 12, 2018 at 04:57:51PM +0100, [email protected] wrote:
>> From: Marcus Cooper <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> .../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

2018-03-13 08:21:26

by Maxime Ripard

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

On Mon, Mar 12, 2018 at 04:57:50PM +0100, [email protected] wrote:
> From: Marcus Cooper <[email protected]>
>
> 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 <[email protected]>
> ---
> 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


Attachments:
(No filename) (5.55 kB)
signature.asc (849.00 B)
Download all attachments

2018-03-13 08:24:48

by Maxime Ripard

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

On Tue, Mar 13, 2018 at 09:15:49AM +0100, Code Kipper wrote:
> On 13 March 2018 at 09:00, Maxime Ripard <[email protected]> wrote:
> > On Mon, Mar 12, 2018 at 04:57:51PM +0100, [email protected] wrote:
> >> From: Marcus Cooper <[email protected]>
> >>
> >> 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 <[email protected]>
> >> ---
> >> .../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


Attachments:
(No filename) (2.43 kB)
signature.asc (849.00 B)
Download all attachments

2018-03-13 09:00:30

by Code Kipper

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

On 13 March 2018 at 09:23, Maxime Ripard <[email protected]> wrote:
> On Tue, Mar 13, 2018 at 09:15:49AM +0100, Code Kipper wrote:
>> On 13 March 2018 at 09:00, Maxime Ripard <[email protected]> wrote:
>> > On Mon, Mar 12, 2018 at 04:57:51PM +0100, [email protected] wrote:
>> >> From: Marcus Cooper <[email protected]>
>> >>
>> >> 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 <[email protected]>
>> >> ---
>> >> .../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

2018-03-13 12:01:34

by Julian Calaby

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

Hi Marcus,

On Tue, Mar 13, 2018 at 2:57 AM, <[email protected]> wrote:
> From: Marcus Cooper <[email protected]>
>
> 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 <[email protected]>
> ---
> 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: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

2018-03-13 14:05:45

by Maxime Ripard

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

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


Attachments:
(No filename) (954.00 B)
signature.asc (849.00 B)
Download all attachments

2018-03-13 14:10:13

by Christopher Obbard

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

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

2018-03-13 16:28:51

by Mark Brown

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

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.


Attachments:
(No filename) (835.00 B)
signature.asc (499.00 B)
Download all attachments