| Message ID | 1426668658-15911-1-git-send-email-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2015-03-18 09:50, Takashi Iwai wrote: > Hi, > > here is a patchset for supporting more aggressive PM for HD-audio. > This allows to change the power state of each widget more dynamically > with jack and stream states. It's activated only when the codec > driver (or via sysfs or f/w patch) sets codec->power_mgmt flag. Cool. Could you elaborate on how the power_mgmt flag interacts with the power_save module parameter (which, on Ubuntu, is set dynamically based on AC power or not)? I e, does it make sense to test this both with and without power_save enabled? Also I assume power_save_controller should not matter, right? > > In theory, this should work for the recent Realtek codecs, but > currently I have no machine for test. > > David, could you or your team check whether this works for ALC282 or > such? Just add like: > > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -5415,6 +5415,7 @@ static int patch_alc269(struct hda_codec *codec) > > spec = codec->spec; > spec->gen.shared_mic_vref_pin = 0x18; > + codec->power_mgmt = 1; > > snd_hda_pick_fixup(codec, alc269_fixup_models, > alc269_fixup_tbl, alc269_fixups); > > > The patchset is for for-next branch of sound git tree, but they might > be applicable to 4.0-rc (or even older), too. The current patches are > found in topic/hda-power branch. > > > thanks, > > Takashi > > === > > Takashi Iwai (4): > ALSA: hda - Simplify PCM setup overrides > ALSA: hda - Support advanced power state controls > ALSA: hda - Use the new power control for VIA codecs > ALSA: hda - Adjust power of beep widget and outputs > > sound/pci/hda/hda_beep.c | 29 +- > sound/pci/hda/hda_beep.h | 1 + > sound/pci/hda/hda_codec.c | 4 + > sound/pci/hda/hda_codec.h | 2 + > sound/pci/hda/hda_generic.c | 480 ++++++++++++++++++++++++------ > sound/pci/hda/hda_generic.h | 5 +- > sound/pci/hda/patch_realtek.c | 41 --- > sound/pci/hda/patch_sigmatel.c | 5 + > sound/pci/hda/patch_via.c | 662 +---------------------------------------- > 9 files changed, 427 insertions(+), 802 deletions(-) >
At Wed, 18 Mar 2015 20:34:34 +0100, David Henningsson wrote: > > > > On 2015-03-18 09:50, Takashi Iwai wrote: > > Hi, > > > > here is a patchset for supporting more aggressive PM for HD-audio. > > This allows to change the power state of each widget more dynamically > > with jack and stream states. It's activated only when the codec > > driver (or via sysfs or f/w patch) sets codec->power_mgmt flag. > > Cool. Could you elaborate on how the power_mgmt flag interacts with the > power_save module parameter (which, on Ubuntu, is set dynamically based > on AC power or not)? I e, does it make sense to test this both with and > without power_save enabled? > > Also I assume power_save_controller should not matter, right? The new stuff basically works independently from traditional power_save and power_save_controller options. But, you can think it's on top of power_save stuff, i.e. trying to power on/off of each widget in the codec while the codec itself is powered on by power_save. Hierarchy is something like: power_mgmt (managing D0/D3 of each codec widget) --> power_save (managing D0/D3 of FG node of the codec) --> power_save_controller (managing D0/D3 of HDA controller) Takashi
On 2015-03-18 09:50, Takashi Iwai wrote: > Hi, > > here is a patchset for supporting more aggressive PM for HD-audio. > This allows to change the power state of each widget more dynamically > with jack and stream states. It's activated only when the codec > driver (or via sysfs or f/w patch) sets codec->power_mgmt flag. > > In theory, this should work for the recent Realtek codecs, but > currently I have no machine for test. > > David, could you or your team check whether this works for ALC282 or > such? Just add like: > > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -5415,6 +5415,7 @@ static int patch_alc269(struct hda_codec *codec) > > spec = codec->spec; > spec->gen.shared_mic_vref_pin = 0x18; > + codec->power_mgmt = 1; > > snd_hda_pick_fixup(codec, alc269_fixup_models, > alc269_fixup_tbl, alc269_fixups); > > > The patchset is for for-next branch of sound git tree, but they might > be applicable to 4.0-rc (or even older), too. The current patches are > found in topic/hda-power branch. So I hoped to be able to look at this today, but it turns out the machine I was thinking of using for testing has an ALC262 codec, which hardly counts as "new". Hui, is this something you feel like taking on? Otherwise I'll try to talk to someone in Taipei. Anyhow, after reading through the code, I have a question about LEDs. It could be that if vref is used for controlling a LED, maybe that pin needs to stay in D0 for the LED to stay lit. Is this correctly handled? I couldn't find any code specific to that issue, but maybe I just missed it. Also, I find the power_mgmt name easy to confuse with the existing power_save parameter, perhaps power_save_pin, power_save_node or power_save_widget is better? > > > thanks, > > Takashi > > === > > Takashi Iwai (4): > ALSA: hda - Simplify PCM setup overrides > ALSA: hda - Support advanced power state controls > ALSA: hda - Use the new power control for VIA codecs > ALSA: hda - Adjust power of beep widget and outputs > > sound/pci/hda/hda_beep.c | 29 +- > sound/pci/hda/hda_beep.h | 1 + > sound/pci/hda/hda_codec.c | 4 + > sound/pci/hda/hda_codec.h | 2 + > sound/pci/hda/hda_generic.c | 480 ++++++++++++++++++++++++------ > sound/pci/hda/hda_generic.h | 5 +- > sound/pci/hda/patch_realtek.c | 41 --- > sound/pci/hda/patch_sigmatel.c | 5 + > sound/pci/hda/patch_via.c | 662 +---------------------------------------- > 9 files changed, 427 insertions(+), 802 deletions(-) >
At Fri, 20 Mar 2015 17:20:59 +0100, David Henningsson wrote: > > Anyhow, after reading through the code, I have a question about LEDs. It > could be that if vref is used for controlling a LED, maybe that pin > needs to stay in D0 for the LED to stay lit. Is this correctly handled? > I couldn't find any code specific to that issue, but maybe I just missed it. Good point. As far as I know, IDT codecs keep pinctl value no matter whether the widget power state is. So, it should be OK, judging only from the spec. But we need testing with the actual machine. Maybe it's really need a power up. The VREF pins have to be constantly powered on, the fix shouldn't be too difficult. > Also, I find the power_mgmt name easy to confuse with the existing > power_save parameter, perhaps power_save_pin, power_save_node or > power_save_widget is better? I don't mind renaming. power_save_pin isn't appropriate because it's not only about pins but all paths in general. power_save_node might sound generic enough. thanks, Takashi
On 03/21/2015 12:20 AM, David Henningsson wrote: > > On 2015-03-18 09:50, Takashi Iwai wrote: >> Hi, >> >> here is a patchset for supporting more aggressive PM for HD-audio. >> This allows to change the power state of each widget more dynamically >> with jack and stream states. It's activated only when the codec >> driver (or via sysfs or f/w patch) sets codec->power_mgmt flag. >> >> In theory, this should work for the recent Realtek codecs, but >> currently I have no machine for test. >> >> David, could you or your team check whether this works for ALC282 or >> such? Just add like: >> >> --- a/sound/pci/hda/patch_realtek.c >> +++ b/sound/pci/hda/patch_realtek.c >> @@ -5415,6 +5415,7 @@ static int patch_alc269(struct hda_codec *codec) >> >> spec = codec->spec; >> spec->gen.shared_mic_vref_pin = 0x18; >> + codec->power_mgmt = 1; >> >> snd_hda_pick_fixup(codec, alc269_fixup_models, >> alc269_fixup_tbl, alc269_fixups); >> >> >> The patchset is for for-next branch of sound git tree, but they might >> be applicable to 4.0-rc (or even older), too. The current patches are >> found in topic/hda-power branch. > > So I hoped to be able to look at this today, but it turns out the > machine I was thinking of using for testing has an ALC262 codec, which > hardly counts as "new". > > Hui, is this something you feel like taking on? Otherwise I'll try to > talk to someone in Taipei. > OK? I will look for the machine to do the test next week. Regards, Hui. > Anyhow, after reading through the code, I have a question about LEDs. > It could be that if vref is used for controlling a LED, maybe that pin > needs to stay in D0 for the LED to stay lit. Is this correctly > handled? I couldn't find any code specific to that issue, but maybe I > just missed it. > > Also, I find the power_mgmt name easy to confuse with the existing > power_save parameter, perhaps power_save_pin, power_save_node or > power_save_widget is better? > >> >> >> thanks, >> >> Takashi >> >> === >> >> Takashi Iwai (4): >> ALSA: hda - Simplify PCM setup overrides >> ALSA: hda - Support advanced power state controls >> ALSA: hda - Use the new power control for VIA codecs >> ALSA: hda - Adjust power of beep widget and outputs >> >> sound/pci/hda/hda_beep.c | 29 +- >> sound/pci/hda/hda_beep.h | 1 + >> sound/pci/hda/hda_codec.c | 4 + >> sound/pci/hda/hda_codec.h | 2 + >> sound/pci/hda/hda_generic.c | 480 ++++++++++++++++++++++++------ >> sound/pci/hda/hda_generic.h | 5 +- >> sound/pci/hda/patch_realtek.c | 41 --- >> sound/pci/hda/patch_sigmatel.c | 5 + >> sound/pci/hda/patch_via.c | 662 >> +---------------------------------------- >> 9 files changed, 427 insertions(+), 802 deletions(-) >> >
At Thu, 26 Mar 2015 20:24:43 +0800, Hui Wang wrote: > > [1 <text/plain; utf-8 (8bit)>] > On 03/21/2015 02:38 PM, Hui Wang wrote: > > On 03/21/2015 12:20 AM, David Henningsson wrote: > >> > >> On 2015-03-18 09:50, Takashi Iwai wrote: > >>> Hi, > >>> > >>> here is a patchset for supporting more aggressive PM for HD-audio. > >>> This allows to change the power state of each widget more dynamically > >>> with jack and stream states. It's activated only when the codec > >>> driver (or via sysfs or f/w patch) sets codec->power_mgmt flag. > >>> > >>> In theory, this should work for the recent Realtek codecs, but > >>> currently I have no machine for test. > >>> > >>> David, could you or your team check whether this works for ALC282 or > >>> such? Just add like: > >>> > >>> --- a/sound/pci/hda/patch_realtek.c > >>> +++ b/sound/pci/hda/patch_realtek.c > >>> @@ -5415,6 +5415,7 @@ static int patch_alc269(struct hda_codec *codec) > >>> > >>> spec = codec->spec; > >>> spec->gen.shared_mic_vref_pin = 0x18; > >>> + codec->power_mgmt = 1; > >>> > >>> snd_hda_pick_fixup(codec, alc269_fixup_models, > >>> alc269_fixup_tbl, alc269_fixups); > >>> > >>> > >>> The patchset is for for-next branch of sound git tree, but they might > >>> be applicable to 4.0-rc (or even older), too. The current patches are > >>> found in topic/hda-power branch. > >> > >> So I hoped to be able to look at this today, but it turns out the > >> machine I was thinking of using for testing has an ALC262 codec, > >> which hardly counts as "new". > >> > >> Hui, is this something you feel like taking on? Otherwise I'll try to > >> talk to someone in Taipei. > >> > > OK? I will look for the machine to do the test next week. > > > > Regards, > > Hui. > > > Sorry for late response, today is my first day in the office back from > vacation, I checked all machines in the Beijing office, none of them has > the ALC282 codec, I will continue to look for the machine from other office. > > And I did the test on the machines with the alc283, alc255, alc292 and > alc269, the testing result were same, there were no sound output from > internal speaker or headphone, and the internal mic or external mic > can't record any sound. The test steps as below: > > 1. power_save_node = 0 > checkout the hda-power branch, build the kernel based on this branch. > Install the kernel to the above machines and boot into the desktop > test internal speaker and internal mic, works very well, plug a headset, > test headphone and external mic, works very well. > run pm_suspend, wait 5 seconds, wakeup the system, redo the above test, > everything works very well. OK, this is expected. The patch shouldn't touch this case. > 2. power_save_node = 1 > enable the power_save_node as below: > @@ -5426,6 +5426,8 @@ static int patch_alc269(struct hda_codec *codec) > > alc_auto_parse_customize_define(codec); > > + codec->power_save_node = 1; > + > if (has_cdefine_beep(codec)) > spec->gen.beep_nid = 0x01; > rebuild the kernel, install the kernel to the above machines and boot > into the desktop > test internal speaker and internal mic, we can play sound to internal > speaker without any errors, but I can't hear any sound from the speaker; > I can use the internal mic to record without errors, but recorded file > did not include any sound pcm (maybe all 0x00 or 0xff) > I plug a headset into the headset jack, the detection works very well, > but I can't hear sound from headphone when play a sound, and I can't use > headset mic to record any sound as well. > > > And I attached 2 alsa-info.txt, one is the power_save_node=0, the other > is the power_save_node=1 Thanks. The alsa-info.sh outputs show no difference but the power state, so the widget attributes seem kept with the power state change, as it seems. Could you give alsa-info.sh output *during* playing with power_save_node=1? Takashi
--- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5415,6 +5415,7 @@ static int patch_alc269(struct hda_codec *codec) spec = codec->spec; spec->gen.shared_mic_vref_pin = 0x18; + codec->power_mgmt = 1; snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups);