diff mbox

ASoC: rt5645: Add the HWEQ for the customized speaker output

Message ID 1445410711-28828-1-git-send-email-oder_chiou@realtek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Oder Chiou Oct. 21, 2015, 6:58 a.m. UTC 
The patch adds the HWEQ function for the customized speaker output that is
in order to the customer's requirement. The customer can enable the HWEQ in
the platform data, and set the parameters of the HWEQ in the ALSA binary
control byte-by-byte.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
---
 include/sound/rt5645.h    |   2 +
 sound/soc/codecs/rt5645.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

Comments

Mark Brown Oct. 21, 2015, 10:35 a.m. UTC | #1 
On Wed, Oct 21, 2015 at 02:58:31PM +0800, Oder Chiou wrote:
> The patch adds the HWEQ function for the customized speaker output that is
> in order to the customer's requirement. The customer can enable the HWEQ in
> the platform data, and set the parameters of the HWEQ in the ALSA binary
> control byte-by-byte.

> +struct rt5645_eq_param_s {
> +	unsigned short reg;
> +	unsigned short val;
> +};

As I said in reply to your earlier version just now this seems like a
really strange way of configuring things - we'd need to understand why
this is a good format for doing the configuration.
Oder Chiou Oct. 21, 2015, 12:47 p.m. UTC | #2 
> > +struct rt5645_eq_param_s {
> > +	unsigned short reg;
> > +	unsigned short val;
> > +};
> 
> As I said in reply to your earlier version just now this seems like a
> really strange way of configuring things - we'd need to understand why
> this is a good format for doing the configuration.
> 
There are 56 registers(0x1a4 to 0x1cd and 0x1e5 to 0x1f8) for setting the
EQ parameters, and there is a register(0xb1) to control how many bands are
enabled. The EQ parameters are affected by which bands are enabled, and we
can set the EQ parameters which are required. The goal is to implement the
function as simple as possible. In case of the Google Celes, we can only set
5 registers. We have added the check function in the "rt5645_hweq_put", if
the settings are invalid, they are not copied to private data. In the enable
function, we also validate the registers, and only the 57 registers can be
set. Could you give us some hint?
Mark Brown Oct. 22, 2015, 1:08 p.m. UTC | #3 
On Wed, Oct 21, 2015 at 12:47:34PM +0000, Oder Chiou wrote:
> > > +struct rt5645_eq_param_s {
> > > +	unsigned short reg;
> > > +	unsigned short val;
> > > +};

> > As I said in reply to your earlier version just now this seems like a
> > really strange way of configuring things - we'd need to understand why
> > this is a good format for doing the configuration.

> There are 56 registers(0x1a4 to 0x1cd and 0x1e5 to 0x1f8) for setting the
> EQ parameters, and there is a register(0xb1) to control how many bands are
> enabled. The EQ parameters are affected by which bands are enabled, and we
> can set the EQ parameters which are required. The goal is to implement the

56 registers isn't so many in total but...

> function as simple as possible. In case of the Google Celes, we can only set
> 5 registers. We have added the check function in the "rt5645_hweq_put", if
> the settings are invalid, they are not copied to private data. In the enable
> function, we also validate the registers, and only the 57 registers can be
> set. Could you give us some hint?

It's a lot simpler from a kernel code point of view to just export the
full register coefficient block - but it's nice to see your tuning
software is smart enough to be able to output the smaller set of
register writes!  I think this is fine if you can put the above
explanation into the changelog and also change things to remove the need
to specify platform data and just always provide these controls, with
validation of the registers being written I think that is a good idea,
especially given the separate register for specifying how many bands are
enabled.
diff mbox

Patch

diff --git a/include/sound/rt5645.h b/include/sound/rt5645.h
index a5cf615..efa9f08 100644
--- a/include/sound/rt5645.h
+++ b/include/sound/rt5645.h
@@ -23,6 +23,8 @@  struct rt5645_platform_data {
 	unsigned int jd_mode;
 	/* Invert JD when jack insert */
 	bool jd_invert;
+	/* Speaker HWEQ */
+	bool hweq;
 };
 
 #endif
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c
index 4d54999..f26f08b 100644
--- a/sound/soc/codecs/rt5645.c
+++ b/sound/soc/codecs/rt5645.c
@@ -42,6 +42,8 @@ 
 
 #define RT5645_PR_BASE (RT5645_PR_RANGE_BASE + (0 * RT5645_PR_SPACING))
 
+#define RT5645_HWEQ_NUM 57
+
 static const struct regmap_range_cfg rt5645_ranges[] = {
 	{
 		.name = "PR",
@@ -224,6 +226,11 @@  static const struct reg_default rt5645_reg[] = {
 	{ 0xff, 0x6308 },
 };
 
+struct rt5645_eq_param_s {
+	unsigned short reg;
+	unsigned short val;
+};
+
 static const char *const rt5645_supply_names[] = {
 	"avdd",
 	"cpvdd",
@@ -240,6 +247,7 @@  struct rt5645_priv {
 	struct snd_soc_jack *btn_jack;
 	struct delayed_work jack_detect_work;
 	struct regulator_bulk_data supplies[ARRAY_SIZE(rt5645_supply_names)];
+	struct rt5645_eq_param_s *eq_param;
 
 	int codec_type;
 	int sysclk;
@@ -477,6 +485,80 @@  static const DECLARE_TLV_DB_RANGE(spk_clsd_tlv,
 	7, 7, TLV_DB_SCALE_ITEM(228, 0, 0)
 );
 
+static int rt5645_hweq_info(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+	uinfo->count = RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s);
+
+	return 0;
+}
+
+static int rt5645_hweq_get(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct rt5645_priv *rt5645 = snd_soc_component_get_drvdata(component);
+	struct rt5645_eq_param_s *eq_param =
+		(struct rt5645_eq_param_s *)ucontrol->value.bytes.data;
+
+	if (rt5645->pdata.hweq)
+		memcpy(eq_param, rt5645->eq_param,
+			RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s));
+
+	return 0;
+}
+
+static bool rt5645_validate_hweq(unsigned short reg)
+{
+	if ((reg >= 0x1a4 && reg <= 0x1cd) | (reg >= 0x1e5 && reg <= 0x1f8) |
+		(reg == RT5645_EQ_CTRL2))
+		return true;
+
+	return false;
+}
+
+static int rt5645_hweq_put(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct rt5645_priv *rt5645 = snd_soc_component_get_drvdata(component);
+	struct rt5645_eq_param_s *eq_param =
+		(struct rt5645_eq_param_s *)ucontrol->value.bytes.data;
+	int i;
+
+	if (rt5645->pdata.hweq) {
+		/* The final setting of the table should be RT5645_EQ_CTRL2 */
+		for (i = RT5645_HWEQ_NUM - 1; i >= 0; i--) {
+			if (eq_param[i].reg == 0)
+				continue;
+			else if (eq_param[i].reg != RT5645_EQ_CTRL2)
+				return 0;
+			else
+				break;
+		}
+
+		for (i = 0; i < RT5645_HWEQ_NUM; i++) {
+			if (!rt5645_validate_hweq(eq_param[i].reg) &&
+				eq_param[i].reg != 0)
+				return 0;
+			else if (eq_param[i].reg == 0)
+				break;
+		}
+
+		memcpy(rt5645->eq_param, eq_param,
+			RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s));
+	}
+
+	return 0;
+}
+
+#define RT5645_HWEQ(xname) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = rt5645_hweq_info, \
+	.get = rt5645_hweq_get,\
+	.put = rt5645_hweq_put }
+
 static const struct snd_kcontrol_new rt5645_snd_controls[] = {
 	/* Speaker Output Volume */
 	SOC_DOUBLE("Speaker Channel Switch", RT5645_SPK_VOL,
@@ -541,6 +623,7 @@  static const struct snd_kcontrol_new rt5645_snd_controls[] = {
 	/* I2S2 function select */
 	SOC_SINGLE("I2S2 Func Switch", RT5645_GPIO_CTRL1, RT5645_I2S2_SEL_SFT,
 		1, 1),
+	RT5645_HWEQ("Speaker HWEQ"),
 };
 
 /**
@@ -631,6 +714,22 @@  static int is_using_asrc(struct snd_soc_dapm_widget *source,
 
 }
 
+static int rt5645_enable_hweq(struct snd_soc_codec *codec)
+{
+	struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec);
+	int i;
+
+	for (i = 0; i < RT5645_HWEQ_NUM; i++) {
+		if (rt5645_validate_hweq(rt5645->eq_param[i].reg))
+			regmap_write(rt5645->regmap, rt5645->eq_param[i].reg,
+					rt5645->eq_param[i].val);
+		else
+			break;
+	}
+
+	return 0;
+}
+
 /**
  * rt5645_sel_asrc_clk_src - select ASRC clock source for a set of filters
  * @codec: SoC audio codec device.
@@ -1532,9 +1631,12 @@  static int rt5645_spk_event(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *kcontrol, int event)
 {
 	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec);
 
 	switch (event) {
 	case SND_SOC_DAPM_POST_PMU:
+		if (rt5645->pdata.hweq)
+			rt5645_enable_hweq(codec);
 		snd_soc_update_bits(codec, RT5645_PWR_DIG1,
 			RT5645_PWR_CLS_D | RT5645_PWR_CLS_D_R |
 			RT5645_PWR_CLS_D_L,
@@ -1543,6 +1645,8 @@  static int rt5645_spk_event(struct snd_soc_dapm_widget *w,
 		break;
 
 	case SND_SOC_DAPM_PRE_PMD:
+		if (rt5645->pdata.hweq)
+			snd_soc_write(codec, RT5645_EQ_CTRL2, 0);
 		snd_soc_update_bits(codec, RT5645_PWR_DIG1,
 			RT5645_PWR_CLS_D | RT5645_PWR_CLS_D_R |
 			RT5645_PWR_CLS_D_L, 0);
@@ -3060,6 +3164,11 @@  static int rt5645_probe(struct snd_soc_codec *codec)
 		snd_soc_dapm_sync(dapm);
 	}
 
+	if (rt5645->pdata.hweq)
+		rt5645->eq_param = devm_kzalloc(codec->dev,
+			RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s),
+			GFP_KERNEL);
+
 	return 0;
 }