[alsa-devel] [PATCH 0/5] HD-audio widget power-save fixes
Hi,
this is a series of small fixes for the recently added HD-audio widget power-save feature. Especially the last patch should fix the Realtek codec breakage.
The patches are found in topic/hda branch of sound git tree, too.
Takashi
===
Takashi Iwai (5): ALSA: hda/generic - Check power state cap at updating the widget power ALSA: hda/generic - Fix wrong initial power state for fixed pins ALSA: hda/generic - Make snd_hda_gen_path_power_filter() always applicable ALSA: hda/generic - Don't override power_filter when power_save_node is set ALSA: hda/realtek - Fix the regression by widget power-saving
sound/pci/hda/hda_generic.c | 23 +++++++++++++++++++---- sound/pci/hda/patch_realtek.c | 5 +++-- 2 files changed, 22 insertions(+), 6 deletions(-)
The new widget power-saving tries to apply the power change no matter whether the node has a power cap or not. It's bad (although most of codecs chip just ignore it). Check the capability properly beforehand.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_generic.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 1f2ca7be1468..afc6b1b0898c 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -832,6 +832,8 @@ static hda_nid_t path_power_update(struct hda_codec *codec,
for (i = 0; i < path->depth; i++) { nid = path->path[i]; + if (!(get_wcaps(codec, nid) & AC_WCAP_POWER)) + continue; if (nid == codec->core.afg) continue; if (!allow_powerdown || is_active_nid_for_any(codec, nid))
When the widget power-saving is enabled, the first automute hook invocation checks through the whole pins and it also tries to synchronize the power state. However, this results in a wrong state because it calls unconditionally snd_hda_jack_detect_state(). This patch adds a check of jack detectability before the actual jack detection call.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_generic.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index afc6b1b0898c..46b559832d2c 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3959,6 +3959,14 @@ static hda_nid_t set_path_power(struct hda_codec *codec, hda_nid_t nid, return changed; }
+/* check the jack status for power control */ +static bool detect_pin_state(struct hda_codec *codec, hda_nid_t pin) +{ + if (!is_jack_detectable(codec, pin)) + return true; + return snd_hda_jack_detect_state(codec, pin) != HDA_JACK_NOT_PRESENT; +} + /* power up/down the paths of the given pin according to the jack state; * power = 0/1 : only power up/down if it matches with the jack state, * < 0 : force power up/down to follow the jack sate @@ -3973,7 +3981,8 @@ static hda_nid_t set_pin_power_jack(struct hda_codec *codec, hda_nid_t pin, if (!codec->power_save_node) return 0;
- on = snd_hda_jack_detect_state(codec, pin) != HDA_JACK_NOT_PRESENT; + on = detect_pin_state(codec, pin); + if (power >= 0 && on != power) return 0; return set_path_power(codec, pin, on, -1); @@ -4225,8 +4234,7 @@ static void do_automute(struct hda_codec *codec, int num_pins, hda_nid_t *pins, if (codec->power_save_node) { bool on = !mute; if (on) - on = snd_hda_jack_detect_state(codec, nid) - != HDA_JACK_NOT_PRESENT; + on = detect_pin_state(codec, nid); set_path_power(codec, nid, on, -1); } }
When the widget power-saving is enabled, the first automute hook invocation checks through the whole pins and it also tries to synchronize the power state. However, this results in a wrong state because it calls unconditionally snd_hda_jack_detect_state(). This patch adds a check of jack detectability before the actual jack detection call.
What happen when the computer has no headphone jack ?
No mic jack and internal mic
Those thin mini itx motherboard use firmware to disable mixer and remove all input pins
# Prevent the AA-loopback from being enabled which can pull in white noise (ACI#380) [hint] mixer_nid = 0
Will the driver still power down the audio mixer and audio inputs ?
Seem driver still create capture device when there is no input pins
At Fri, 10 Apr 2015 16:51:58 +0800, Raymond Yau wrote:
When the widget power-saving is enabled, the first automute hook invocation checks through the whole pins and it also tries to synchronize the power state. However, this results in a wrong state because it calls unconditionally snd_hda_jack_detect_state(). This patch adds a check of jack detectability before the actual jack detection call.
What happen when the computer has no headphone jack ?
Nothing wrong happens. The logic of widget power saving is to power down unused paths. It includes the paths with pins that aren't plugged and paths with the DAC/ADCs that aren't enabled (no stream running). Fixed pins are handled as always plugged.
Takashi
No mic jack and internal mic
Those thin mini itx motherboard use firmware to disable mixer and remove all input pins
# Prevent the AA-loopback from being enabled which can pull in white noise (ACI#380) [hint] mixer_nid = 0
Will the driver still power down the audio mixer and audio inputs ?
Seem driver still create capture device when there is no input pins
Add the check of power_save_node flag at the beginning of the function so that it skips the rest if the flag isn't set. In this way, we can call this function safely no matter whether the widget power-saving is really used or not.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_generic.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 46b559832d2c..f0475a19fad7 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -4703,6 +4703,10 @@ unsigned int snd_hda_gen_path_power_filter(struct hda_codec *codec, hda_nid_t nid, unsigned int power_state) { + struct hda_gen_spec *spec = codec->spec; + + if (!spec->power_down_unused && !codec->power_save_node) + return power_state; if (power_state != AC_PWRST_D0 || nid == codec->core.afg) return power_state; if (get_wcaps_type(get_wcaps(codec, nid)) >= AC_WID_POWER)
Currently the generic parser sets codec->power_filter when power_save_node flag is set. But this overrides the existing filter that has been already set by the codec driver, thus it looses some features. Instead, set the default power_filter only when it's not set yet.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_generic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index f0475a19fad7..3d2597b7037b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -4918,7 +4918,8 @@ int snd_hda_gen_parse_auto_config(struct hda_codec *codec, parse_digital(codec);
if (spec->power_down_unused || codec->power_save_node) - codec->power_filter = snd_hda_gen_path_power_filter; + if (!codec->power_filter) + codec->power_filter = snd_hda_gen_path_power_filter;
if (!spec->no_analog && spec->beep_nid) { err = snd_hda_attach_beep_device(codec, spec->beep_nid);
While enabling the widget power-saving for ALC269 & co, the important setup was forgotten -- stream_pm ops. Without this setup, the paths for PCM won't be powered up at all.
Also, the power_filter callbacks used in ALC269 & co need to chain to the default snd_hda_gen_path_power_filter().
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 7b5c93e0e78c..acccedf8a4be 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -3223,7 +3223,7 @@ static unsigned int led_power_filter(struct hda_codec *codec, snd_hda_set_pin_ctl(codec, nid, snd_hda_codec_get_pin_target(codec, nid));
- return AC_PWRST_D0; + return snd_hda_gen_path_power_filter(codec, nid, power_state); }
static void alc269_fixup_hp_mute_led(struct hda_codec *codec, @@ -4186,7 +4186,7 @@ static unsigned int alc_power_filter_xps13(struct hda_codec *codec, if (spec->gen.hp_jack_present) if (nid == codec->core.afg || nid == 0x02 || nid == 0x15) return AC_PWRST_D0; - return power_state; + return snd_hda_gen_path_power_filter(codec, nid, power_state); }
static void alc_fixup_dell_xps13(struct hda_codec *codec, @@ -5673,6 +5673,7 @@ static int patch_alc269(struct hda_codec *codec) set_beep_amp(spec, 0x0b, 0x04, HDA_INPUT);
codec->patch_ops = alc_patch_ops; + codec->patch_ops.stream_pm = snd_hda_gen_stream_pm, #ifdef CONFIG_PM codec->patch_ops.suspend = alc269_suspend; codec->patch_ops.resume = alc269_resume;
participants (2)
-
Raymond Yau -
Takashi Iwai