From: Marcus Cooper <[email protected]>
On the newer SoCs such as the H3 and A64 this is set by default
to transfer a 0 after each sample in each slot. However the A10
and A20 SoCs that this driver was developed on had a 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 SEXT
bits should be cleared so that they also pad at the LSB. Without
this the audio gets distorted.
Signed-off-by: Marcus Cooper <[email protected]>
---
sound/soc/sunxi/sun4i-i2s.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 34575a8aa9f6..056a299c03fb 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -135,6 +135,7 @@ struct sun4i_i2s;
* @field_clkdiv_mclk_en: regmap field to enable mclk output.
* @field_fmt_wss: regmap field to set word select size.
* @field_fmt_sr: regmap field to set sample resolution.
+ * @field_fmt_sext: regmap field to set the sign extension.
*/
struct sun4i_i2s_quirks {
bool has_reset;
@@ -145,6 +146,7 @@ struct sun4i_i2s_quirks {
struct reg_field field_clkdiv_mclk_en;
struct reg_field field_fmt_wss;
struct reg_field field_fmt_sr;
+ struct reg_field field_fmt_sext;
const struct sun4i_i2s_clk_div *bclk_dividers;
unsigned int num_bclk_dividers;
@@ -177,6 +179,7 @@ struct sun4i_i2s {
struct regmap_field *field_clkdiv_mclk_en;
struct regmap_field *field_fmt_wss;
struct regmap_field *field_fmt_sr;
+ struct regmap_field *field_fmt_sext;
const struct sun4i_i2s_quirks *variant;
};
@@ -354,6 +357,10 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
regmap_field_write(i2s->field_clkdiv_mclk_en, 1);
+
+ /* Set sign extension to pad out LSB with 0 */
+ regmap_field_write(i2s->field_fmt_sext, 0);
+
return 0;
}
@@ -1073,6 +1080,7 @@ static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
.mclk_dividers = sun4i_i2s_mclk_div,
.num_mclk_dividers = ARRAY_SIZE(sun4i_i2s_mclk_div),
.get_bclk_parent_rate = sun4i_i2s_get_bclk_parent_rate,
+ .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
.get_sr = sun4i_i2s_get_sr,
.get_wss = sun4i_i2s_get_wss,
.set_chan_cfg = sun4i_i2s_set_chan_cfg,
@@ -1091,6 +1099,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
.mclk_dividers = sun4i_i2s_mclk_div,
.num_mclk_dividers = ARRAY_SIZE(sun4i_i2s_mclk_div),
.get_bclk_parent_rate = sun4i_i2s_get_bclk_parent_rate,
+ .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
.get_sr = sun4i_i2s_get_sr,
.get_wss = sun4i_i2s_get_wss,
.set_chan_cfg = sun4i_i2s_set_chan_cfg,
@@ -1109,6 +1118,7 @@ static const struct sun4i_i2s_quirks sun8i_a83t_i2s_quirks = {
.mclk_dividers = sun8i_i2s_clk_div,
.num_mclk_dividers = ARRAY_SIZE(sun8i_i2s_clk_div),
.get_bclk_parent_rate = sun8i_i2s_get_bclk_parent_rate,
+ .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 4, 5),
.get_sr = sun8i_i2s_get_sr_wss,
.get_wss = sun8i_i2s_get_sr_wss,
.set_chan_cfg = sun8i_i2s_set_chan_cfg,
@@ -1127,6 +1137,7 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
.mclk_dividers = sun4i_i2s_mclk_div,
.num_mclk_dividers = ARRAY_SIZE(sun4i_i2s_mclk_div),
.get_bclk_parent_rate = sun4i_i2s_get_bclk_parent_rate,
+ .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
.get_sr = sun4i_i2s_get_sr,
.get_wss = sun4i_i2s_get_wss,
.set_chan_cfg = sun4i_i2s_set_chan_cfg,
@@ -1154,6 +1165,12 @@ static int sun4i_i2s_init_regmap_fields(struct device *dev,
if (IS_ERR(i2s->field_fmt_sr))
return PTR_ERR(i2s->field_fmt_sr);
+ 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);
+
return 0;
}
--
2.23.0
On Mon, Aug 26, 2019 at 08:07:33PM +0200, [email protected] wrote:
> From: Marcus Cooper <[email protected]>
>
> On the newer SoCs such as the H3 and A64 this is set by default
> to transfer a 0 after each sample in each slot. However the A10
> and A20 SoCs that this driver was developed on had a 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 SEXT
> bits should be cleared so that they also pad at the LSB. Without
> this the audio gets distorted.
>
> Signed-off-by: Marcus Cooper <[email protected]>
If anything, I'd like to have less regmap_fields rather than more of
them. This is pretty easy to add to one of the callbacks, especially
since the field itself has been completely reworked from one
generation to the other.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, 27 Aug 2019 at 11:34, Maxime Ripard <[email protected]> wrote:
>
> On Mon, Aug 26, 2019 at 08:07:33PM +0200, [email protected] wrote:
> > From: Marcus Cooper <[email protected]>
> >
> > On the newer SoCs such as the H3 and A64 this is set by default
> > to transfer a 0 after each sample in each slot. However the A10
> > and A20 SoCs that this driver was developed on had a 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 SEXT
> > bits should be cleared so that they also pad at the LSB. Without
> > this the audio gets distorted.
> >
> > Signed-off-by: Marcus Cooper <[email protected]>
>
> If anything, I'd like to have less regmap_fields rather than more of
> them. This is pretty easy to add to one of the callbacks, especially
> since the field itself has been completely reworked from one
> generation to the other.
>
ACK
That's fine....I've been doing that with the patches which follow this.
CK
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com