[PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Liam Girdwood]

The DL1 PDM interface adds a little gain depending on the output device.
Add a method to retrieve the gain value for machine driver usage.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 sound/soc/codecs/twl6040.c |   23 +++++++++++++++++++++++
 sound/soc/codecs/twl6040.h |    1 +
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index 4ae6b82..3b00ee7 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -33,6 +33,7 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/soc-dapm.h>
 #include <sound/initval.h>
 #include <sound/tlv.h>
 
@@ -1042,6 +1043,28 @@ int twl6040_get_hs_step_size(struct snd_soc_codec *codec)
 }
 EXPORT_SYMBOL_GPL(twl6040_get_hs_step_size);
 
+int twl6040_get_dl1_gain(struct snd_soc_codec *codec)
+{
+	struct snd_soc_dapm_context *dapm = &codec->dapm;
+
+	if (snd_soc_dapm_get_pin_status(dapm, "EP"))
+		return -1; /* -1dB */
+
+	if (snd_soc_dapm_get_pin_status(dapm, "HSOR") ||
+		snd_soc_dapm_get_pin_status(dapm, "HSOL")) {
+
+		u8 val = snd_soc_read(codec, TWL6040_REG_HSLCTL);
+		if (val & TWL6040_HSDACMODE)
+			/* HSDACL in LP mode */
+			return -8; /* -8dB */
+		else
+			/* HSDACL in HP mode */
+			return -1; /* -1dB */
+	}
+	return 0; /* 0dB */
+}
+EXPORT_SYMBOL_GPL(twl6040_get_dl1_gain);
+
 static const struct snd_kcontrol_new twl6040_snd_controls[] = {
 	/* Capture gains */
 	SOC_DOUBLE_TLV("Capture Preamplifier Volume",
diff --git a/sound/soc/codecs/twl6040.h b/sound/soc/codecs/twl6040.h
index 0028699..b580c9d 100644
--- a/sound/soc/codecs/twl6040.h
+++ b/sound/soc/codecs/twl6040.h
@@ -39,5 +39,6 @@ void twl6040_hs_jack_detect(struct snd_soc_codec *codec,
 int twl6040_get_clk_id(struct snd_soc_codec *codec);
 int twl6040_get_trim_value(struct snd_soc_codec *codec, enum twl6040_trim trim);
 int twl6040_get_hs_step_size(struct snd_soc_codec *codec);
+int twl6040_get_dl1_gain(struct snd_soc_codec *codec);
 
 #endif /* End of __TWL6040_H__ */
-- 
1.7.5.4

Re: [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Mark Brown]

On Mon, Jan 09, 2012 at 12:11:22PM +0000, Liam Girdwood wrote:
> The DL1 PDM interface adds a little gain depending on the output device.
> Add a method to retrieve the gain value for machine driver usage.

I'm having a hard time understanding how the machine driver would use
this information...

> +int twl6040_get_dl1_gain(struct snd_soc_codec *codec)
> +{
> +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> +
> +	if (snd_soc_dapm_get_pin_status(dapm, "EP"))
> +		return -1; /* -1dB */

Shouldn't this be being done by reading the CODEC register settings to
know which output DL1 is connected to?

Re: [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Liam Girdwood]

On Tue, 2012-01-10 at 00:44 +0000, Mark Brown wrote:
> On Mon, Jan 09, 2012 at 12:11:22PM +0000, Liam Girdwood wrote:
> > The DL1 PDM interface adds a little gain depending on the output device.
> > Add a method to retrieve the gain value for machine driver usage.
> 
> I'm having a hard time understanding how the machine driver would use
> this information...
> 

It's intended that the machine driver would use this information to
configure the ABE to compensate any attenuation so that the output
volume level was constant.

> > +int twl6040_get_dl1_gain(struct snd_soc_codec *codec)
> > +{
> > +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> > +
> > +	if (snd_soc_dapm_get_pin_status(dapm, "EP"))
> > +		return -1; /* -1dB */
> 
> Shouldn't this be being done by reading the CODEC register settings to
> know which output DL1 is connected to?

The DL1 path output devices can all operate at the same time and may or
may not be connected by the machine drivers.

Liam

Re: [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Mark Brown]

On Tue, Jan 10, 2012 at 11:03:09AM +0000, Liam Girdwood wrote:
> On Tue, 2012-01-10 at 00:44 +0000, Mark Brown wrote:
> > On Mon, Jan 09, 2012 at 12:11:22PM +0000, Liam Girdwood wrote:

> > > The DL1 PDM interface adds a little gain depending on the output device.
> > > Add a method to retrieve the gain value for machine driver usage.

> > I'm having a hard time understanding how the machine driver would use
> > this information...

> It's intended that the machine driver would use this information to
> configure the ABE to compensate any attenuation so that the output
> volume level was constant.

So if you turn on some of the outputs in the CODEC the CODEC will apply
a gain to the signal coming in on DL1 which affects other CODEC outputs
and this needs to be worked around in the ABE so the machine driver is
doing that?

> > > +int twl6040_get_dl1_gain(struct snd_soc_codec *codec)
> > > +{
> > > +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> > > +
> > > +	if (snd_soc_dapm_get_pin_status(dapm, "EP"))
> > > +		return -1; /* -1dB */

> > Shouldn't this be being done by reading the CODEC register settings to
> > know which output DL1 is connected to?

> The DL1 path output devices can all operate at the same time and may or
> may not be connected by the machine drivers.

But surely the paths from DL1 to the various outputs within the CODEC
are controlled by registers in the CODEC?  I don't understand why the
CODEC needs to query DAPM here.

Re: [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Liam Girdwood]

On Tue, 2012-01-10 at 19:29 +0000, Mark Brown wrote:
> On Tue, Jan 10, 2012 at 11:03:09AM +0000, Liam Girdwood wrote:
> > On Tue, 2012-01-10 at 00:44 +0000, Mark Brown wrote:
> > > On Mon, Jan 09, 2012 at 12:11:22PM +0000, Liam Girdwood wrote:
> 
> > > > The DL1 PDM interface adds a little gain depending on the output device.
> > > > Add a method to retrieve the gain value for machine driver usage.
> 
> > > I'm having a hard time understanding how the machine driver would use
> > > this information...
> 
> > It's intended that the machine driver would use this information to
> > configure the ABE to compensate any attenuation so that the output
> > volume level was constant.
> 
> So if you turn on some of the outputs in the CODEC the CODEC will apply
> a gain to the signal coming in on DL1 which affects other CODEC outputs
> and this needs to be worked around in the ABE so the machine driver is
> doing that?

Yes, but the Codec component (widget) must also be enabled for the
attenuation to happen.

> 
> > > > +int twl6040_get_dl1_gain(struct snd_soc_codec *codec)
> > > > +{
> > > > +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> > > > +
> > > > +	if (snd_soc_dapm_get_pin_status(dapm, "EP"))
> > > > +		return -1; /* -1dB */
> 
> > > Shouldn't this be being done by reading the CODEC register settings to
> > > know which output DL1 is connected to?
> 
> > The DL1 path output devices can all operate at the same time and may or
> > may not be connected by the machine drivers.
> 
> But surely the paths from DL1 to the various outputs within the CODEC
> are controlled by registers in the CODEC?  I don't understand why the
> CODEC needs to query DAPM here.

The machine driver may not have the Earpiece, Handsfree or Headset
connected (even though the codec path is set to use them). The codec has
no way of knowing this except to check the widget DAPM status for the
outputs. Now we can check the DAPM status of each widget by reading the
codec register, but I think that defeats DAPMs codec widget power
control and status APIs.

I've applied this since I'm doing pull request this morning, you are in
Asia and this is a trivial patch. We can rework if required later.

Liam

Re: [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain

[Mark Brown]

On Wed, Jan 11, 2012 at 10:32:13AM +0000, Liam Girdwood wrote:
> On Tue, 2012-01-10 at 19:29 +0000, Mark Brown wrote:

> > So if you turn on some of the outputs in the CODEC the CODEC will apply
> > a gain to the signal coming in on DL1 which affects other CODEC outputs
> > and this needs to be worked around in the ABE so the machine driver is
> > doing that?

> Yes, but the Codec component (widget) must also be enabled for the
> attenuation to happen.

And (since it wasn't entirely clear to me) this is the DL1 interface the
CODEC has not the similarly named interface that the ABE has.

> > > > Shouldn't this be being done by reading the CODEC register settings to
> > > > know which output DL1 is connected to?

> > > The DL1 path output devices can all operate at the same time and may or
> > > may not be connected by the machine drivers.

> > But surely the paths from DL1 to the various outputs within the CODEC
> > are controlled by registers in the CODEC?  I don't understand why the
> > CODEC needs to query DAPM here.

> The machine driver may not have the Earpiece, Handsfree or Headset
> connected (even though the codec path is set to use them). The codec has
> no way of knowing this except to check the widget DAPM status for the
> outputs. Now we can check the DAPM status of each widget by reading the

Yeah, I was expecting the code to just read the power bits that DAPM
sets to see which outputs were enabled - having to ask DAPM looked like
a lot of work and made me wonder if there was something else going on.

> codec register, but I think that defeats DAPMs codec widget power
> control and status APIs.

It makes me a bit nervous when CODEC drivers need to go back and ask
DAPM what they've done with themselves, that way can lie polling and
reentrancy issues if the driver ends up querying DAPM from
within DAPM.

> I've applied this since I'm doing pull request this morning, you are in
> Asia and this is a trivial patch. We can rework if required later.

There was no rush here, there's no users at the minute and we're into
the merge window so shouldn't really be adding non-bugfix code anyway.