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. As the LibreELEC project is progressing extremely well then there
has been some activity getting surround sound working and this is included.
The functionality included with the new patch set has been extended to
cover more sample resolutions, multi-lane data output for HDMI audio
and some bug fixes that have been discovered along the way.
I can see more usage of the tdm property since I last attempted to push
these patches and the examples currently in mainline sort of the opposite
to what I'm trying to achieve. When we first started looking at the i2s
driver, the codecs that we were using allowed for the frame width to be
determined based on the sampling resolution but in most use cases it
seems that a fixed width is required(my highest priority should be to get
HDMI audio support in). We're using the tdm property to override the old
way to calculate the frame width. What I've seen in what has already been
mainlined is that the i2s driver has a frame width that is fixed to 32
bits and this can be overridden using the tdm property.
I still need to investigate the FIFO syncing issues which i've not had a
chance to change or address the concerns that broonie and wens brought up.
This change has been moved to the top of the patch stack.
BR,
CK
---
v4 changes compared to v3 are:
- Moved patches around so that the more controversial of patches are
at the top of the stack.
- Added more details to commit messages.
- Fixed 20bit audio PCM format to use 4 bytes.
- Reduced number of flags used to indicate a new SoC.
v3 changes compared to v2 are:
- added back slave mode changes
- added back the use of tdm properties
- changes to regmap and caching
- removed loopback functionality
- fixes to the channel offset mask
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 (9):
ASoC: sun4i-i2s: Fix sun8i tx channel offset mask
ASoC: sun4i-i2s: Add offset to RX channel select
ASoC: sun4i-i2s: Add regmap field to sign extend sample
ASoC: sun4i-i2s: Reduce quirks for sun8i-h3
ASoC: sun4i-i2s: Add set_tdm_slot functionality
ASoC: sun4i-i2s: Add multi-lane functionality
ASoC: sun4i-i2s: Add multichannel functionality
ASoc: sun4i-i2s: Add 20, 24 and 32 bit support
ASoC: sun4i-i2s: Adjust regmap settings
sound/soc/sunxi/sun4i-i2s.c | 242 ++++++++++++++++++++++++------------
1 file changed, 164 insertions(+), 78 deletions(-)
--
2.21.0
From: Marcus Cooper <[email protected]>
Whilst testing the capture functionality of the i2s on the newer
SoCs it was noticed that the recording was somewhat distorted.
This was due to the offset not being set correctly on the receiver
side.
Signed-off-by: Marcus Cooper <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 90bd3963d8ae..fd7c37596f21 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -456,6 +456,10 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
SUN8I_I2S_TX_CHAN_OFFSET_MASK,
SUN8I_I2S_TX_CHAN_OFFSET(offset));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+ SUN8I_I2S_TX_CHAN_OFFSET(offset));
}
regmap_field_write(i2s->field_fmt_mode, val);
--
2.21.0
From: Marcus Cooper <[email protected]>
Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
to reflect this.
Signed-off-by: Marcus Cooper <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 351b8021173b..92828a84902d 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -595,9 +595,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
{
/* Flush RX FIFO */
+ regcache_cache_bypass(i2s->regmap, true);
regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
+ regcache_cache_bypass(i2s->regmap, false);
/* Clear RX counter */
regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
@@ -616,9 +618,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
{
/* Flush TX FIFO */
+ regcache_cache_bypass(i2s->regmap, true);
regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
+ regcache_cache_bypass(i2s->regmap, false);
/* Clear TX counter */
regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
@@ -771,13 +775,7 @@ static const struct snd_soc_component_driver sun4i_i2s_component = {
static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
{
- switch (reg) {
- case SUN4I_I2S_FIFO_TX_REG:
- return false;
-
- default:
- return true;
- }
+ return true;
}
static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
@@ -796,6 +794,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_TX_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
case SUN4I_I2S_INT_STA_REG:
case SUN4I_I2S_RX_CNT_REG:
case SUN4I_I2S_TX_CNT_REG:
@@ -806,23 +806,12 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
}
}
-static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
-{
- switch (reg) {
- case SUN8I_I2S_FIFO_TX_REG:
- return false;
-
- default:
- return true;
- }
-}
-
static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
if (reg == SUN8I_I2S_INT_STA_REG)
return true;
if (reg == SUN8I_I2S_FIFO_TX_REG)
- return false;
+ return true;
return sun4i_i2s_volatile_reg(dev, reg);
}
@@ -877,7 +866,7 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
.reg_defaults = sun8i_i2s_reg_defaults,
.num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
.writeable_reg = sun4i_i2s_wr_reg,
- .readable_reg = sun8i_i2s_rd_reg,
+ .readable_reg = sun4i_i2s_rd_reg,
.volatile_reg = sun8i_i2s_volatile_reg,
};
--
2.21.0
From: Marcus Cooper <[email protected]>
The i2s block can be used to pass PCM data over multiple channels
and is sometimes used for the audio side of an HDMI connection.
Signed-off-by: Marcus Cooper <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 121 +++++++++++++++++++-----------------
1 file changed, 64 insertions(+), 57 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 75217fb52bfa..3549a87ed9e9 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -189,6 +189,7 @@ struct sun4i_i2s {
unsigned int tdm_slots;
unsigned int slot_width;
+ unsigned int offset;
};
struct sun4i_i2s_clk_div {
@@ -358,56 +359,71 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
int lines;
channels = params_channels(params);
- 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 (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ 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;
+ }
- if (i2s->variant->is_h3_i2s_based) {
- regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
- SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
- SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
- regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
- SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
- SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
- }
+ lines = (channels + 1) / 2;
- /* Map the channels for playback and capture */
- regmap_field_write(i2s->field_rxchanmap, 0x00003210);
- regmap_field_write(i2s->field_txchanmap, 0x10);
- if (i2s->variant->is_h3_i2s_based) {
- 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);
+ /* 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->is_h3_i2s_based)
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+ SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK,
+ SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels));
+
+ regmap_field_write(i2s->field_txchanmap, 0x10);
+ /* Configure the channels */
+ regmap_field_write(i2s->field_txchansel, SUN4I_I2S_CHAN_SEL(2));
+
+ if (i2s->variant->is_h3_i2s_based) {
+ u32 chan_sel = SUN8I_I2S_TX_CHAN_OFFSET(i2s->offset) | 0x1;
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
+ chan_sel | 0x30);
+
+ if (channels > 2) {
+ regmap_write(i2s->regmap,
+ SUN8I_I2S_TX_CHAN_MAP_REG+4, 0x32);
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG+4,
+ chan_sel | 0x30);
+ }
+ if (channels > 4) {
+ regmap_write(i2s->regmap,
+ SUN8I_I2S_TX_CHAN_MAP_REG+8, 0x54);
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG+8,
+ chan_sel | 0x30);
+ }
+ if (channels > 6) {
+ regmap_write(i2s->regmap,
+ SUN8I_I2S_TX_CHAN_MAP_REG+12, 0x76);
+ regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG+12,
+ chan_sel | 0x30);
+ }
+ }
+ } else {
+ /* Map the channels for capture */
+ regmap_field_write(i2s->field_rxchanmap, 0x00003210);
+ regmap_field_write(i2s->field_rxchansel,
+ SUN4I_I2S_CHAN_SEL(params_channels(params)));
+
+ if (i2s->variant->is_h3_i2s_based) {
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG,
+ SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK,
+ SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+ SUN8I_I2S_TX_CHAN_OFFSET(i2s->offset));
+ }
}
- /* Configure the channels */
- regmap_field_write(i2s->field_txchansel,
- SUN4I_I2S_CHAN_SEL(params_channels(params)));
-
- regmap_field_write(i2s->field_rxchansel,
- SUN4I_I2S_CHAN_SEL(params_channels(params)));
-
- if (i2s->variant->is_h3_i2s_based)
- regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
- SUN8I_I2S_TX_CHAN_EN_MASK,
- SUN8I_I2S_TX_CHAN_EN(channels));
-
switch (params_physical_width(params)) {
case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -445,7 +461,6 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
u32 val;
- u32 offset = 0;
u32 bclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
u32 lrclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
@@ -453,7 +468,7 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_I2S:
val = SUN4I_I2S_FMT0_FMT_I2S;
- offset = 1;
+ i2s->offset = 1;
break;
case SND_SOC_DAIFMT_LEFT_J:
val = SUN4I_I2S_FMT0_FMT_LEFT_J;
@@ -474,16 +489,8 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
* i2s shares the same setting with the LJ format. Increment
* val so that the bit to value to write is correct.
*/
- if (offset > 0)
+ if (i2s->offset > 0)
val++;
- /* blck offset determines whether i2s or LJ */
- regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
- SUN8I_I2S_TX_CHAN_OFFSET_MASK,
- SUN8I_I2S_TX_CHAN_OFFSET(offset));
-
- regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
- SUN8I_I2S_TX_CHAN_OFFSET_MASK,
- SUN8I_I2S_TX_CHAN_OFFSET(offset));
}
regmap_field_write(i2s->field_fmt_mode, val);
--
2.21.0
On Mon, Jun 03, 2019 at 07:47:28PM +0200, [email protected] wrote:
> From: Marcus Cooper <[email protected]>
>
> Whilst testing the capture functionality of the i2s on the newer
> SoCs it was noticed that the recording was somewhat distorted.
> This was due to the offset not being set correctly on the receiver
> side.
>
> Signed-off-by: Marcus Cooper <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, Jun 4, 2019 at 3:37 PM Maxime Ripard <[email protected]> wrote:
>
> On Mon, Jun 03, 2019 at 07:47:28PM +0200, [email protected] wrote:
> > From: Marcus Cooper <[email protected]>
> >
> > Whilst testing the capture functionality of the i2s on the newer
> > SoCs it was noticed that the recording was somewhat distorted.
> > This was due to the offset not being set correctly on the receiver
> > side.
> >
> > Signed-off-by: Marcus Cooper <[email protected]>
>
> Acked-by: Maxime Ripard <[email protected]>
Would be nice to have
Fixes: 7d2993811a1e ("ASoC: sun4i-i2s: Add support for H3")
But otherwise,
Acked-by: Chen-Yu Tsai <[email protected]>
On Mon, Jun 03, 2019 at 07:47:35PM +0200, [email protected] wrote:
> From: Marcus Cooper <[email protected]>
>
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
>
> Signed-off-by: Marcus Cooper <[email protected]>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 29 +++++++++--------------------
> 1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 351b8021173b..92828a84902d 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -595,9 +595,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
> {
> /* Flush RX FIFO */
> + regcache_cache_bypass(i2s->regmap, true);
> regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
> SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
> + regcache_cache_bypass(i2s->regmap, false);
Your commit log should say why you need to do this in the first place.
> @@ -771,13 +775,7 @@ static const struct snd_soc_component_driver sun4i_i2s_component = {
>
> static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
> {
> - switch (reg) {
> - case SUN4I_I2S_FIFO_TX_REG:
> - return false;
> -
> - default:
> - return true;
> - }
> + return true;
That doesn't seem related?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
The patch
ASoC: sun4i-i2s: Add offset to RX channel select
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From f9927000cb35f250051f0f1878db12ee2626eea1 Mon Sep 17 00:00:00 2001
From: Marcus Cooper <[email protected]>
Date: Mon, 3 Jun 2019 19:47:28 +0200
Subject: [PATCH] ASoC: sun4i-i2s: Add offset to RX channel select
Whilst testing the capture functionality of the i2s on the newer
SoCs it was noticed that the recording was somewhat distorted.
This was due to the offset not being set correctly on the receiver
side.
Signed-off-by: Marcus Cooper <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Acked-by: Chen-Yu Tsai <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 8162e107e50b..bc128e2a6096 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -460,6 +460,10 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
SUN8I_I2S_TX_CHAN_OFFSET_MASK,
SUN8I_I2S_TX_CHAN_OFFSET(offset));
+
+ regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
+ SUN8I_I2S_TX_CHAN_OFFSET_MASK,
+ SUN8I_I2S_TX_CHAN_OFFSET(offset));
}
regmap_field_write(i2s->field_fmt_mode, val);
--
2.20.1