[v4,3/9] ASoC: sun4i-i2s: Add regmap field to sign extend sample
diff mbox series

Message ID 20190603174735.21002-4-codekipper@gmail.com
State New
Headers show
Series
  • ASoC: sun4i-i2s: Updates to the driver
Related show

Commit Message

Code Kipper June 3, 2019, 5:47 p.m. UTC
From: Marcus Cooper <codekipper@gmail.com>

On the newer SoCs this is set by default to transfer a 0 after
each sample in each slot. However the platform that this driver
was developed on had the default setting where it padded the
audio gain with zeros. This isn't a problem whilst we have only
support for 16bit audio but with larger sample resolution rates
in the pipeline then it should be fixed to also pad. Without this
the audio gets distorted.

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

Comments

Maxime Ripard June 4, 2019, 7:43 a.m. UTC | #1
On Mon, Jun 03, 2019 at 07:47:29PM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> On the newer SoCs this is set by default to transfer a 0 after

Which SoCs?

> each sample in each slot. However the platform that this driver

Which platform?

> was developed on had the default setting where it padded the audio
> gain with zeros. This isn't a problem whilst we have only support
> for 16bit audio but with larger sample resolution rates in the
> pipeline then it should be fixed to also pad. Without this the audio
> gets distorted.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>

Once the commit log fixed,
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Chen-Yu Tsai June 4, 2019, 7:53 a.m. UTC | #2
On Tue, Jun 4, 2019 at 1:47 AM <codekipper@gmail.com> wrote:
>
> From: Marcus Cooper <codekipper@gmail.com>
>
> On the newer SoCs this is set by default to transfer a 0 after
> each sample in each slot. However the platform that this driver
> was developed on had the default setting where it padded the
> audio gain with zeros. This isn't a problem whilst we have only
> support for 16bit audio but with larger sample resolution rates
> in the pipeline then it should be fixed to also pad. Without this
> the audio gets distorted.

Curious, both the A10 and A20 manuals say the default value for this
field is 0, which means 0 padding.

sun4i_i2s_reg_defaults[] also has that field set to 0.

You're saying you are seeing the field set to 1?

ChenYu

> 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 fd7c37596f21..e2961d8f6e8c 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -134,6 +134,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.
> @@ -159,6 +160,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;
> @@ -183,6 +185,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;
> @@ -342,6 +345,9 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
>                                    SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
>                                    SUN8I_I2S_FMT0_LRCK_PERIOD(32));
>
> +       /* Set sign extension to pad out LSB with 0 */
> +       regmap_field_write(i2s->field_fmt_sext, 0);
> +
>         return 0;
>  }
>
> @@ -887,6 +893,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),
> @@ -904,6 +911,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),
> @@ -944,6 +952,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),
> @@ -1006,6 +1015,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.21.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190603174735.21002-4-codekipper%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
Code Kipper June 4, 2019, 11:46 a.m. UTC | #3
On Tue, 4 Jun 2019 at 09:53, Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Tue, Jun 4, 2019 at 1:47 AM <codekipper@gmail.com> wrote:
> >
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > On the newer SoCs this is set by default to transfer a 0 after
> > each sample in each slot. However the platform that this driver
> > was developed on had the default setting where it padded the
> > audio gain with zeros. This isn't a problem whilst we have only
> > support for 16bit audio but with larger sample resolution rates
> > in the pipeline then it should be fixed to also pad. Without this
> > the audio gets distorted.
>
> Curious, both the A10 and A20 manuals say the default value for this
> field is 0, which means 0 padding.
>
> sun4i_i2s_reg_defaults[] also has that field set to 0.
>
> You're saying you are seeing the field set to 1?

On the newer SoCs (H3 onwards) this setting defaults to 3 which is
"Transfer 0 after each sample in each slot" which resulted in distortion.
Setting SEXT to 0 "Zeros or audio gain padding at LSB" alligns the
setup with that of the earlier block and fixed the issue we were hearing.
It's really noticeable with HDMI audio.
BR,
CK
>
> ChenYu
>
> > 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 fd7c37596f21..e2961d8f6e8c 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -134,6 +134,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.
> > @@ -159,6 +160,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;
> > @@ -183,6 +185,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;
> > @@ -342,6 +345,9 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> >                                    SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> >                                    SUN8I_I2S_FMT0_LRCK_PERIOD(32));
> >
> > +       /* Set sign extension to pad out LSB with 0 */
> > +       regmap_field_write(i2s->field_fmt_sext, 0);
> > +
> >         return 0;
> >  }
> >
> > @@ -887,6 +893,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),
> > @@ -904,6 +911,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),
> > @@ -944,6 +952,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),
> > @@ -1006,6 +1015,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.21.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> > To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190603174735.21002-4-codekipper%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.

Patch
diff mbox series

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index fd7c37596f21..e2961d8f6e8c 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -134,6 +134,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.
@@ -159,6 +160,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;
@@ -183,6 +185,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;
@@ -342,6 +345,9 @@  static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
 				   SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
 				   SUN8I_I2S_FMT0_LRCK_PERIOD(32));
 
+	/* Set sign extension to pad out LSB with 0 */
+	regmap_field_write(i2s->field_fmt_sext, 0);
+
 	return 0;
 }
 
@@ -887,6 +893,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),
@@ -904,6 +911,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),
@@ -944,6 +952,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),
@@ -1006,6 +1015,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);