[alsa-devel] [PATCH 1/4] ASoC:rt5670:Add runtime PM support
From: Bard Liao bardliao@realtek.com
This patch adds runtime PM support on rt5670 codec. And will switch sysclk to internal clock during runtime suspend.
Signed-off-by: Bard Liao bardliao@realtek.com --- sound/soc/codecs/rt5670.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index da4b689..97359b3 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -14,6 +14,7 @@ #include <linux/init.h> #include <linux/delay.h> #include <linux/pm.h> +#include <linux/pm_runtime.h> #include <linux/i2c.h> #include <linux/platform_device.h> #include <linux/firmware.h> @@ -2666,6 +2667,13 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, /*Give sysclk a default value*/ rt5670->sysclk = 24576000;
+ /* The set initial power status to active, to be consist with ACPI power + * state D0. The codec will be suspended after probe and runtime status + * change will trigger ACPI D3 method for power saving. + */ + pm_runtime_set_active(&i2c->dev); + pm_runtime_enable(&i2c->dev); + ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5670, rt5670_dai, ARRAY_SIZE(rt5670_dai)); if (ret < 0) @@ -2673,20 +2681,61 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
return 0; err: + pm_runtime_disable(&i2c->dev); + return ret; }
static int rt5670_i2c_remove(struct i2c_client *i2c) { + pm_runtime_disable(&i2c->dev); snd_soc_unregister_codec(&i2c->dev);
return 0; }
+#ifdef CONFIG_PM_RUNTIME +static void rt5670_use_internal_clk(struct rt5670_priv *rt5670) +{ + struct snd_soc_codec *codec = rt5670->codec; + + if (!codec || + (0 == rt5670->sysclk && RT5670_SCLK_S_RCCLK == rt5670->sysclk_src)) + return; + + snd_soc_update_bits(codec, RT5670_GLB_CLK, + RT5670_SCLK_SRC_MASK, RT5670_SCLK_SRC_RCCLK); + + rt5670->sysclk = 0; + rt5670->sysclk_src = RT5670_SCLK_S_RCCLK; +} + +static int rt5670_runtime_suspend(struct device *dev) +{ + struct rt5670_priv *rt5670 = dev_get_drvdata(dev); + + rt5670_use_internal_clk(rt5670); + return 0; +} + +static int rt5670_runtime_resume(struct device *dev) +{ + return 0; +} + +static const struct dev_pm_ops rt5670_pm = { + SET_RUNTIME_PM_OPS(rt5670_runtime_suspend, rt5670_runtime_resume, NULL) +}; +#define RT5670_PM_OPS rt5670_pm +#else +#define RT5670_PM_OPS NULL +#endif /* CONFIG_PM_RUNTIME */ + static struct i2c_driver rt5670_i2c_driver = { .driver = { .name = "rt5670", .owner = THIS_MODULE, + .pm = &RT5670_PM_OPS, }, .probe = rt5670_i2c_probe, .remove = rt5670_i2c_remove,
From: Bard Liao bardliao@realtek.com
The patch corrects the incorrect default register values.
Signed-off-by: Bard Liao bardliao@realtek.com --- sound/soc/codecs/rt5670.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 97359b3..7592814 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -102,18 +102,18 @@ static const struct reg_default rt5670_reg[] = { { 0x4c, 0x5380 }, { 0x4f, 0x0073 }, { 0x52, 0x00d3 }, - { 0x53, 0xf0f0 }, + { 0x53, 0xf000 }, { 0x61, 0x0000 }, { 0x62, 0x0001 }, { 0x63, 0x00c3 }, { 0x64, 0x0000 }, - { 0x65, 0x0000 }, + { 0x65, 0x0001 }, { 0x66, 0x0000 }, { 0x6f, 0x8000 }, { 0x70, 0x8000 }, { 0x71, 0x8000 }, { 0x72, 0x8000 }, - { 0x73, 0x1110 }, + { 0x73, 0x7770 }, { 0x74, 0x0e00 }, { 0x75, 0x1505 }, { 0x76, 0x0015 }, @@ -127,21 +127,21 @@ static const struct reg_default rt5670_reg[] = { { 0x83, 0x0000 }, { 0x84, 0x0000 }, { 0x85, 0x0000 }, - { 0x86, 0x0008 }, + { 0x86, 0x0004 }, { 0x87, 0x0000 }, { 0x88, 0x0000 }, { 0x89, 0x0000 }, { 0x8a, 0x0000 }, { 0x8b, 0x0000 }, - { 0x8c, 0x0007 }, + { 0x8c, 0x0003 }, { 0x8d, 0x0000 }, { 0x8e, 0x0004 }, { 0x8f, 0x1100 }, { 0x90, 0x0646 }, { 0x91, 0x0c06 }, { 0x93, 0x0000 }, - { 0x94, 0x0000 }, - { 0x95, 0x0000 }, + { 0x94, 0x1270 }, + { 0x95, 0x1000 }, { 0x97, 0x0000 }, { 0x98, 0x0000 }, { 0x99, 0x0000 }, @@ -152,11 +152,11 @@ static const struct reg_default rt5670_reg[] = { { 0x9e, 0x0400 }, { 0xae, 0x7000 }, { 0xaf, 0x0000 }, - { 0xb0, 0x6000 }, + { 0xb0, 0x7000 }, { 0xb1, 0x0000 }, { 0xb2, 0x0000 }, { 0xb3, 0x001f }, - { 0xb4, 0x2206 }, + { 0xb4, 0x220c }, { 0xb5, 0x1f00 }, { 0xb6, 0x0000 }, { 0xb7, 0x0000 }, @@ -173,25 +173,25 @@ static const struct reg_default rt5670_reg[] = { { 0xcf, 0x1813 }, { 0xd0, 0x0690 }, { 0xd1, 0x1c17 }, - { 0xd3, 0xb320 }, + { 0xd3, 0xa220 }, { 0xd4, 0x0000 }, { 0xd6, 0x0400 }, { 0xd9, 0x0809 }, { 0xda, 0x0000 }, { 0xdb, 0x0001 }, { 0xdc, 0x0049 }, - { 0xdd, 0x0009 }, + { 0xdd, 0x0024 }, { 0xe6, 0x8000 }, { 0xe7, 0x0000 }, - { 0xec, 0xb300 }, + { 0xec, 0xa200 }, { 0xed, 0x0000 }, - { 0xee, 0xb300 }, + { 0xee, 0xa200 }, { 0xef, 0x0000 }, { 0xf8, 0x0000 }, { 0xf9, 0x0000 }, { 0xfa, 0x8010 }, { 0xfb, 0x0033 }, - { 0xfc, 0x0080 }, + { 0xfc, 0x0100 }, };
static bool rt5670_volatile_register(struct device *dev, unsigned int reg)
On Thu, Nov 06, 2014 at 12:23:52PM +0800, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
The patch corrects the incorrect default register values.
Applied, thanks - again, subject line formatting.
From: Bard Liao bardliao@realtek.com
rt5672 is very similar to rt5670. Therefore we use one codec driver to support both codecs. The difference between rt5670 and rt5672 is there is some difference in their dapm routing table.
Signed-off-by: Bard Liao bardliao@realtek.com --- sound/soc/codecs/rt5670.c | 71 ++++++++++++++++++++++++++++++++++++++--------- sound/soc/codecs/rt5670.h | 6 ++++ 2 files changed, 64 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 7592814..c3914e8 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -1635,29 +1635,39 @@ static const struct snd_soc_dapm_widget rt5670_dapm_widgets[] = { /* PDM */ SND_SOC_DAPM_SUPPLY("PDM1 Power", RT5670_PWR_DIG2, RT5670_PWR_PDM1_BIT, 0, NULL, 0), - SND_SOC_DAPM_SUPPLY("PDM2 Power", RT5670_PWR_DIG2, - RT5670_PWR_PDM2_BIT, 0, NULL, 0), - SND_SOC_DAPM_MUX("PDM1 L Mux", RT5670_PDM_OUT_CTRL, RT5670_M_PDM1_L_SFT, 1, &rt5670_pdm1_l_mux), SND_SOC_DAPM_MUX("PDM1 R Mux", RT5670_PDM_OUT_CTRL, RT5670_M_PDM1_R_SFT, 1, &rt5670_pdm1_r_mux), - SND_SOC_DAPM_MUX("PDM2 L Mux", RT5670_PDM_OUT_CTRL, - RT5670_M_PDM2_L_SFT, 1, &rt5670_pdm2_l_mux), - SND_SOC_DAPM_MUX("PDM2 R Mux", RT5670_PDM_OUT_CTRL, - RT5670_M_PDM2_R_SFT, 1, &rt5670_pdm2_r_mux),
/* Output Lines */ SND_SOC_DAPM_OUTPUT("HPOL"), SND_SOC_DAPM_OUTPUT("HPOR"), SND_SOC_DAPM_OUTPUT("LOUTL"), SND_SOC_DAPM_OUTPUT("LOUTR"), +}; + +static const struct snd_soc_dapm_widget rt5670_specific_dapm_widgets[] = { + SND_SOC_DAPM_SUPPLY("PDM2 Power", RT5670_PWR_DIG2, + RT5670_PWR_PDM2_BIT, 0, NULL, 0), + SND_SOC_DAPM_MUX("PDM2 L Mux", RT5670_PDM_OUT_CTRL, + RT5670_M_PDM2_L_SFT, 1, &rt5670_pdm2_l_mux), + SND_SOC_DAPM_MUX("PDM2 R Mux", RT5670_PDM_OUT_CTRL, + RT5670_M_PDM2_R_SFT, 1, &rt5670_pdm2_r_mux), SND_SOC_DAPM_OUTPUT("PDM1L"), SND_SOC_DAPM_OUTPUT("PDM1R"), SND_SOC_DAPM_OUTPUT("PDM2L"), SND_SOC_DAPM_OUTPUT("PDM2R"), };
+static const struct snd_soc_dapm_widget rt5672_specific_dapm_widgets[] = { + SND_SOC_DAPM_PGA("SPO Amp", SND_SOC_NOPM, 0, 0, NULL, 0), + SND_SOC_DAPM_OUTPUT("SPOLP"), + SND_SOC_DAPM_OUTPUT("SPOLN"), + SND_SOC_DAPM_OUTPUT("SPORP"), + SND_SOC_DAPM_OUTPUT("SPORN"), +}; + static const struct snd_soc_dapm_route rt5670_dapm_routes[] = { { "ADC Stereo1 Filter", NULL, "ADC STO1 ASRC", is_using_asrc }, { "ADC Stereo2 Filter", NULL, "ADC STO2 ASRC", is_using_asrc }, @@ -2010,12 +2020,6 @@ static const struct snd_soc_dapm_route rt5670_dapm_routes[] = { { "PDM1 R Mux", "Stereo DAC", "Stereo DAC MIXR" }, { "PDM1 R Mux", "Mono DAC", "Mono DAC MIXR" }, { "PDM1 R Mux", NULL, "PDM1 Power" }, - { "PDM2 L Mux", "Stereo DAC", "Stereo DAC MIXL" }, - { "PDM2 L Mux", "Mono DAC", "Mono DAC MIXL" }, - { "PDM2 L Mux", NULL, "PDM2 Power" }, - { "PDM2 R Mux", "Stereo DAC", "Stereo DAC MIXR" }, - { "PDM2 R Mux", "Mono DAC", "Mono DAC MIXR" }, - { "PDM2 R Mux", NULL, "PDM2 Power" },
{ "HP Amp", NULL, "HPO MIX" }, { "HP Amp", NULL, "Mic Det Power" }, @@ -2033,13 +2037,30 @@ static const struct snd_soc_dapm_route rt5670_dapm_routes[] = { { "LOUTR", NULL, "LOUT R Playback" }, { "LOUTL", NULL, "Improve HP Amp Drv" }, { "LOUTR", NULL, "Improve HP Amp Drv" }, +};
+static const struct snd_soc_dapm_route rt5670_specific_dapm_routes[] = { + { "PDM2 L Mux", "Stereo DAC", "Stereo DAC MIXL" }, + { "PDM2 L Mux", "Mono DAC", "Mono DAC MIXL" }, + { "PDM2 L Mux", NULL, "PDM2 Power" }, + { "PDM2 R Mux", "Stereo DAC", "Stereo DAC MIXR" }, + { "PDM2 R Mux", "Mono DAC", "Mono DAC MIXR" }, + { "PDM2 R Mux", NULL, "PDM2 Power" }, { "PDM1L", NULL, "PDM1 L Mux" }, { "PDM1R", NULL, "PDM1 R Mux" }, { "PDM2L", NULL, "PDM2 L Mux" }, { "PDM2R", NULL, "PDM2 R Mux" }, };
+static const struct snd_soc_dapm_route rt5672_specific_dapm_routes[] = { + { "SPO Amp", NULL, "PDM1 L Mux" }, + { "SPO Amp", NULL, "PDM1 R Mux" }, + { "SPOLP", NULL, "SPO Amp" }, + { "SPOLN", NULL, "SPO Amp" }, + { "SPORP", NULL, "SPO Amp" }, + { "SPORN", NULL, "SPO Amp" }, +}; + static int rt5670_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) { @@ -2371,6 +2392,29 @@ static int rt5670_probe(struct snd_soc_codec *codec) { struct rt5670_priv *rt5670 = snd_soc_codec_get_drvdata(codec);
+ switch (snd_soc_read(codec, RT5670_RESET) & RT5670_ID_MASK) { + case RT5670_ID_5670: + case RT5670_ID_5671: + snd_soc_dapm_new_controls(&codec->dapm, + rt5670_specific_dapm_widgets, + ARRAY_SIZE(rt5670_specific_dapm_widgets)); + snd_soc_dapm_add_routes(&codec->dapm, + rt5670_specific_dapm_routes, + ARRAY_SIZE(rt5670_specific_dapm_routes)); + break; + case RT5670_ID_5672: + snd_soc_dapm_new_controls(&codec->dapm, + rt5672_specific_dapm_widgets, + ARRAY_SIZE(rt5672_specific_dapm_widgets)); + snd_soc_dapm_add_routes(&codec->dapm, + rt5672_specific_dapm_routes, + ARRAY_SIZE(rt5672_specific_dapm_routes)); + break; + default: + dev_err(codec->dev, + "The driver is for RT5670 RT5671 or RT5672 only\n"); + return -ENODEV; + } rt5670->codec = codec; rt5670_dsp_probe(codec);
@@ -2493,6 +2537,7 @@ static const struct regmap_config rt5670_regmap = {
static const struct i2c_device_id rt5670_i2c_id[] = { { "rt5670", 0 }, + { "rt5672", 0 }, { } }; MODULE_DEVICE_TABLE(i2c, rt5670_i2c_id); diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index 70285c9..8b2ab69 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -228,6 +228,12 @@ #define RT5670_R_VOL_MASK (0x3f) #define RT5670_R_VOL_SFT 0
+/* SW Reset & Device ID (0x00) */ +#define RT5670_ID_MASK (0x3 << 1) +#define RT5670_ID_5670 (0x0 << 1) +#define RT5670_ID_5672 (0x1 << 1) +#define RT5670_ID_5671 (0x2 << 1) + /* Combo Jack Control 1 (0x0a) */ #define RT5670_CBJ_BST1_MASK (0xf << 12) #define RT5670_CBJ_BST1_SFT (12)
On Thu, Nov 06, 2014 at 12:23:53PM +0800, bardliao@realtek.com wrote:
- case RT5670_ID_5670:
- case RT5670_ID_5671:
This looks like it also adds rt5671 support but...
static const struct i2c_device_id rt5670_i2c_id[] = { { "rt5670", 0 },
- { "rt5672", 0 }, { }
};
...not here. Otherwise this is fine, though it would be better to warn if the user registered the device as one type but it turns out to be another when we read the ID register.
From: Bard Liao bardliao@realtek.com
PLL should be powered up once filter power is on. So, "PLL1" should be connected to filters instead of DACs.
Signed-off-by: Bard Liao bardliao@realtek.com --- sound/soc/codecs/rt5670.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index c3914e8..c4d4e3f 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -1927,6 +1927,10 @@ static const struct snd_soc_dapm_route rt5670_dapm_routes[] = { { "DAC1 MIXR", "DAC1 Switch", "DAC1 R Mux" }, { "DAC1 MIXR", NULL, "DAC Stereo1 Filter" },
+ { "DAC Stereo1 Filter", NULL, "PLL1", is_sys_clk_from_pll }, + { "DAC Mono Left Filter", NULL, "PLL1", is_sys_clk_from_pll }, + { "DAC Mono Right Filter", NULL, "PLL1", is_sys_clk_from_pll }, + { "DAC MIX", NULL, "DAC1 MIXL" }, { "DAC MIX", NULL, "DAC1 MIXR" },
@@ -1976,14 +1980,10 @@ static const struct snd_soc_dapm_route rt5670_dapm_routes[] = {
{ "DAC L1", NULL, "DAC L1 Power" }, { "DAC L1", NULL, "Stereo DAC MIXL" }, - { "DAC L1", NULL, "PLL1", is_sys_clk_from_pll }, { "DAC R1", NULL, "DAC R1 Power" }, { "DAC R1", NULL, "Stereo DAC MIXR" }, - { "DAC R1", NULL, "PLL1", is_sys_clk_from_pll }, { "DAC L2", NULL, "Mono DAC MIXL" }, - { "DAC L2", NULL, "PLL1", is_sys_clk_from_pll }, { "DAC R2", NULL, "Mono DAC MIXR" }, - { "DAC R2", NULL, "PLL1", is_sys_clk_from_pll },
{ "OUT MIXL", "BST1 Switch", "BST1" }, { "OUT MIXL", "INL Switch", "INL VOL" },
On Thu, Nov 06, 2014 at 12:23:54PM +0800, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
PLL should be powered up once filter power is on. So, "PLL1" should be connected to filters instead of DACs.
Applied, thanks.
On Thu, Nov 06, 2014 at 12:23:51PM +0800, bardliao@realtek.com wrote:
+#ifdef CONFIG_PM_RUNTIME +static void rt5670_use_internal_clk(struct rt5670_priv *rt5670) +{
- struct snd_soc_codec *codec = rt5670->codec;
- if (!codec ||
(0 == rt5670->sysclk && RT5670_SCLK_S_RCCLK == rt5670->sysclk_src))
Normal kernel coding style is to put the variable before the constant.
return;- snd_soc_update_bits(codec, RT5670_GLB_CLK,
RT5670_SCLK_SRC_MASK, RT5670_SCLK_SRC_RCCLK);
Instead of using snd_soc_ here use regmap and then you don't need to worry if the card is registered.
+static int rt5670_runtime_resume(struct device *dev) +{
- return 0;
+}
This should be undoing the change done on suspend otherwise we'll be stuck on the internal clock after resuming. Alternatively if something will undo that elsewhere then presumably we don't need to use runtime PM to select the internal clock anyway?
If this were an empty function it should be removed.
+#define RT5670_PM_OPS rt5670_pm +#else +#define RT5670_PM_OPS NULL +#endif /* CONFIG_PM_RUNTIME */
static struct i2c_driver rt5670_i2c_driver = { .driver = { .name = "rt5670", .owner = THIS_MODULE,
.pm = &RT5670_PM_OPS,
Usual idiom is to include the & in the #define for the PM active case.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Thursday, November 06, 2014 1:55 PM To: bardliao@realtek.com
+static int rt5670_runtime_resume(struct device *dev) {
- return 0;
+}
This should be undoing the change done on suspend otherwise we'll be stuck on the internal clock after resuming. Alternatively if something will undo that elsewhere then presumably we don't need to use runtime PM to select the internal clock anyway?
Is it okay to restore default sysclock setting is runtime_resume ops?
The codec sysclk source is MCLK (PLL is powered down) after boot by default. We can also apply this when runtime resuming.
On Intel CherryTrail/Braswell platform, the BIOS will always restore MCLK when the codec resumes from D3 to D0. So it's safe to use MCLK as source for codec sysclk in runtime_resume ops. And later hw_params ops can further configure sysclk source and PLL for active audio streaming.
Thanks Mengdong
On Fri, Nov 07, 2014 at 09:24:54AM +0000, Lin, Mengdong wrote:
+static int rt5670_runtime_resume(struct device *dev) {
- return 0;
+}
This should be undoing the change done on suspend otherwise we'll be stuck on the internal clock after resuming. Alternatively if something will undo that elsewhere then presumably we don't need to use runtime PM to select the internal clock anyway?
Is it okay to restore default sysclock setting is runtime_resume ops?
Yes, that's fine of course - I'm just assuming that this works already so presumably there is some restore code somewhere already that should be removed?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, November 07, 2014 5:44 PM
+static int rt5670_runtime_resume(struct device *dev) {
- return 0;
+}
This should be undoing the change done on suspend otherwise we'll be stuck on the internal clock after resuming. Alternatively if something will undo that elsewhere then presumably we don't need to use runtime PM to select the internal clock anyway?
Is it okay to restore default sysclock setting is runtime_resume ops?
Yes, that's fine of course - I'm just assuming that this works already so presumably there is some restore code somewhere already that should be removed?
Bard verified the patch on a Tegra platform and I did on a Braswell platform. On Braswell, the codec keeps using the internal clock after runtime resumed until machine driver select its sysclk source to PLL in hw_param ops. No other restore clk restore code.
Since other machine driver can have different behavior, so it seems better to restore default clk settings when runtime resumed, as after boot. And I can test this next Monday.
Thanks Mengdong
participants (3)
-
bardliao＠realtek.com -
Lin, Mengdong -
Mark Brown