[PATCH 0/2] ASoC: STA32x: filter coefficients, bug fix

[PATCH 0/2] ASoC: STA32x: filter coefficients, bug fix

[Johannes Stezenbach]

Hi,

here are two patches for the ST STA32x ASoC codec driver.
The first one adds SNDRV_CTL_ELEM_TYPE_BYTES controls
for access to the biquad filter coefficients.  I haven't found
any other driver using SNDRV_CTL_ELEM_TYPE_BYTES in this way,
but it works nicely.  The coefficients can be set with
amixer (although amixer has a quirk in that it prints
the values in hex but can read them only as decimal).

# amixer cget name='Ch1 - Biquad 1'
numid=43,iface=MIXER,name='Ch1 - Biquad 1'
  ; type=BYTES,access=rw------,values=15
  : values=0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00

# amixer cset name='Ch1 - Biquad 1' 12,34,0,0,0,0,0,0,0,0,0,0,0,255,78
numid=43,iface=MIXER,name='Ch1 - Biquad 1'
  ; type=BYTES,access=rw------,values=15
  : values=0x0c,0x22,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0x4e

Due to the way the internal coeffcient RAM is accessed, the
corresponding I2C registers are volatile, thus I added the
.volatile_register function in this patch.

The second patch preserves reset values as required by the
chip documentation, it is mainly for future compatibility
and was added during hardware testing to rule out issues from
this side, although it did not make a difference in my tests.
However, the chip spec insists it is important.


Thanks
Johannes

[PATCH 1/2] ASoC: STA32x: Add mixer controls for biquad coefficients

[Johannes Stezenbach]

The STA32x has a number of preset EQ settings, but also
allows full user control of the biquad filter coeffcients
(when "Automode EQ" is set to "User").
Each biquad has five signed, 24bit, fixed-point coefficients
representing the range -1...1.  The five biquad coefficients
can be uploaded in one atomic operation into on-chip
coefficient RAM.
There are also a few prescale, postscale and mixing
coefficients, in the same numeric format and range
(a negative coefficient inverts phase).

These coefficients are made available as SNDRV_CTL_ELEM_TYPE_BYTES
mixer controls.

Signed-off-by: Johannes Stezenbach <js@sig21.net>
---
 sound/soc/codecs/sta32x.c |  124 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 124 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c
index 486628a..9bf944c 100644
--- a/sound/soc/codecs/sta32x.c
+++ b/sound/soc/codecs/sta32x.c
@@ -177,6 +177,95 @@ static const struct soc_enum sta32x_limiter1_release_rate_enum =
 static const struct soc_enum sta32x_limiter2_release_rate_enum =
 	SOC_ENUM_SINGLE(STA32X_L2AR, STA32X_LxR_SHIFT,
 			16, sta32x_limiter_release_rate);
+
+/* byte array controls for setting biquad, mixer, scaling coefficients;
+ * for biquads all five coefficients need to be set in one go,
+ * mixer and pre/postscale coefs can be set individually;
+ * each coef is 24bit, the bytes are ordered in the same way
+ * as given in the STA32x data sheet (big endian; b1, b2, a1, a2, b0)
+ */
+
+static int sta32x_coefficient_info(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_info *uinfo)
+{
+	int numcoef = kcontrol->private_value >> 16;
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+	uinfo->count = 3 * numcoef;
+	return 0;
+}
+
+static int sta32x_coefficient_get(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	int numcoef = kcontrol->private_value >> 16;
+	int index = kcontrol->private_value & 0xffff;
+	unsigned int cfud;
+	int i;
+
+	/* preserve reserved bits in STA32X_CFUD */
+	cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0;
+	/* chip documentation does not say if the bits are self clearing,
+	 * so do it explicitly */
+	snd_soc_write(codec, STA32X_CFUD, cfud);
+
+	snd_soc_write(codec, STA32X_CFADDR2, index);
+	if (numcoef == 1)
+		snd_soc_write(codec, STA32X_CFUD, cfud | 0x04);
+	else if (numcoef == 5)
+		snd_soc_write(codec, STA32X_CFUD, cfud | 0x08);
+	else
+		return -EINVAL;
+	for (i = 0; i < 3 * numcoef; i++)
+		ucontrol->value.bytes.data[i] =
+			snd_soc_read(codec, STA32X_B1CF1 + i);
+
+	return 0;
+}
+
+static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	int numcoef = kcontrol->private_value >> 16;
+	int index = kcontrol->private_value & 0xffff;
+	unsigned int cfud;
+	int i;
+
+	/* preserve reserved bits in STA32X_CFUD */
+	cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0;
+	/* chip documentation does not say if the bits are self clearing,
+	 * so do it explicitly */
+	snd_soc_write(codec, STA32X_CFUD, cfud);
+
+	snd_soc_write(codec, STA32X_CFADDR2, index);
+	for (i = 0; i < 3 * numcoef; i++)
+		snd_soc_write(codec, STA32X_B1CF1 + i,
+			      ucontrol->value.bytes.data[i]);
+	if (numcoef == 1)
+		snd_soc_write(codec, STA32X_CFUD, cfud | 0x01);
+	else if (numcoef == 5)
+		snd_soc_write(codec, STA32X_CFUD, cfud | 0x02);
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+#define SINGLE_COEF(xname, index) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = sta32x_coefficient_info, \
+	.get = sta32x_coefficient_get,\
+	.put = sta32x_coefficient_put, \
+	.private_value = index | (1 << 16) }
+
+#define BIQUAD_COEFS(xname, index) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = sta32x_coefficient_info, \
+	.get = sta32x_coefficient_get,\
+	.put = sta32x_coefficient_put, \
+	.private_value = index | (5 << 16) }
+
 static const struct snd_kcontrol_new sta32x_snd_controls[] = {
 SOC_SINGLE_TLV("Master Volume", STA32X_MVOL, 0, 0xff, 1, mvol_tlv),
 SOC_SINGLE("Master Switch", STA32X_MMUTE, 0, 1, 1),
@@ -232,6 +321,29 @@ SOC_SINGLE_TLV("Limiter1 Release Threshold (DRC Mode)", STA32X_L1ATRT, STA32X_Lx
 	       16, 0, sta32x_limiter_drc_release_tlv),
 SOC_SINGLE_TLV("Limiter2 Release Threshold (DRC Mode)", STA32X_L2ATRT, STA32X_LxR_SHIFT,
 	       16, 0, sta32x_limiter_drc_release_tlv),
+
+BIQUAD_COEFS("Ch1 - Biquad 1", 0),
+BIQUAD_COEFS("Ch1 - Biquad 2", 5),
+BIQUAD_COEFS("Ch1 - Biquad 3", 10),
+BIQUAD_COEFS("Ch1 - Biquad 4", 15),
+BIQUAD_COEFS("Ch2 - Biquad 1", 20),
+BIQUAD_COEFS("Ch2 - Biquad 2", 25),
+BIQUAD_COEFS("Ch2 - Biquad 3", 30),
+BIQUAD_COEFS("Ch2 - Biquad 4", 35),
+BIQUAD_COEFS("High-pass", 40),
+BIQUAD_COEFS("Low-pass", 45),
+SINGLE_COEF("Ch1 - Prescale", 50),
+SINGLE_COEF("Ch2 - Prescale", 51),
+SINGLE_COEF("Ch1 - Postscale", 52),
+SINGLE_COEF("Ch2 - Postscale", 53),
+SINGLE_COEF("Ch3 - Postscale", 54),
+SINGLE_COEF("Thermal warning - Postscale", 55),
+SINGLE_COEF("Ch1 - Mix 1", 56),
+SINGLE_COEF("Ch1 - Mix 2", 57),
+SINGLE_COEF("Ch2 - Mix 1", 58),
+SINGLE_COEF("Ch2 - Mix 2", 59),
+SINGLE_COEF("Ch3 - Mix 1", 60),
+SINGLE_COEF("Ch3 - Mix 2", 61),
 };
 
 static const struct snd_soc_dapm_widget sta32x_dapm_widgets[] = {
@@ -686,6 +798,17 @@ static int sta32x_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+static int sta32x_reg_is_volatile(struct snd_soc_codec *codec,
+				  unsigned int reg)
+{
+	switch (reg) {
+	case STA32X_CONFA ... STA32X_L2ATRT:
+	case STA32X_MPCC1 ... STA32X_FDRC2:
+		return 0;
+	}
+	return 1;
+}
+
 static const struct snd_soc_codec_driver sta32x_codec = {
 	.probe =		sta32x_probe,
 	.remove =		sta32x_remove,
@@ -693,6 +816,7 @@ static const struct snd_soc_codec_driver sta32x_codec = {
 	.resume =		sta32x_resume,
 	.reg_cache_size =	STA32X_REGISTER_COUNT,
 	.reg_word_size =	sizeof(u8),
+	.volatile_register =	sta32x_reg_is_volatile,
 	.set_bias_level =	sta32x_set_bias_level,
 	.controls =		sta32x_snd_controls,
 	.num_controls =		ARRAY_SIZE(sta32x_snd_controls),
-- 
1.7.5.4

[PATCH 2/2] ASoC: STA32x: Preserve reserved register bits

[Johannes Stezenbach]

Chip documentation explicitly requires that the reset values
of reserved register bits are left untouched.  It is possible
there are differences between STA326 and STA328 or future
chip revisions in these bits, and clobbering them might
cause malfunction.

Signed-off-by: Johannes Stezenbach <js@sig21.net>
---
 sound/soc/codecs/sta32x.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c
index 9bf944c..409d89d 100644
--- a/sound/soc/codecs/sta32x.c
+++ b/sound/soc/codecs/sta32x.c
@@ -756,6 +756,22 @@ static int sta32x_probe(struct snd_soc_codec *codec)
 	for (i = 0; i < STA32X_REGISTER_COUNT; i++)
 		snd_soc_cache_write(codec, i, sta32x_regs[i]);
 
+	/* preserve reset values of reserved register bits */
+	snd_soc_cache_write(codec, STA32X_CONFC,
+			    codec->hw_read(codec, STA32X_CONFC));
+	snd_soc_cache_write(codec, STA32X_CONFE,
+			    codec->hw_read(codec, STA32X_CONFE));
+	snd_soc_cache_write(codec, STA32X_CONFF,
+			    codec->hw_read(codec, STA32X_CONFF));
+	snd_soc_cache_write(codec, STA32X_MMUTE,
+			    codec->hw_read(codec, STA32X_MMUTE));
+	snd_soc_cache_write(codec, STA32X_AUTO1,
+			    codec->hw_read(codec, STA32X_AUTO1));
+	snd_soc_cache_write(codec, STA32X_AUTO3,
+			    codec->hw_read(codec, STA32X_AUTO3));
+	snd_soc_cache_write(codec, STA32X_C3CFG,
+			    codec->hw_read(codec, STA32X_C3CFG));
+
 	/* FIXME enable thermal warning adjustment and recovery  */
 	snd_soc_update_bits(codec, STA32X_CONFA,
 			    STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, 0);
-- 
1.7.5.4

Re: [PATCH 0/2] ASoC: STA32x: filter coefficients, bug fix

[Mark Brown]

On Mon, Jul 11, 2011 at 05:01:22PM +0200, Johannes Stezenbach wrote:

> here are two patches for the ST STA32x ASoC codec driver.
> The first one adds SNDRV_CTL_ELEM_TYPE_BYTES controls
> for access to the biquad filter coefficients.  I haven't found
> any other driver using SNDRV_CTL_ELEM_TYPE_BYTES in this way,
> but it works nicely.  The coefficients can be set with

Applied, thanks.  _BYTES is actually the plan for general coefficient
work in the future so should be fine.