[alsa-devel] [PATCH] ASoC: twl6040 - Add method to query optimum PDM_DL1 gain
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__ */
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?
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
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.
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
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.
participants (2)
-
Liam Girdwood -
Mark Brown