At Fri, 17 Aug 2012 14:41:17 +0000, Lin, Mengdong wrote:
... Basically the patch below should do almost equivalently. It doesn't free/reacquire irq, which I'm not quite sure whether mandatory for power-saving.
@@ -2402,11 +2402,14 @@ static void azx_power_notify(struct hda_bus *bus) break; } }
- if (power_on)
- if (power_on) {
pci_set_power_state(chip->pci, PCI_D0);
- else if (chip->running && power_save_controller &&
!bus->power_keep_link_on)
- } else if (chip->running && power_save_controller &&
!bus->power_keep_link_on) {pci_set_power_state(chip->pci, PCI_D3hot);- }
} #endif /* CONFIG_SND_HDA_POWER_SAVE */
Hi Takashi,
Thanks for your comments.
I think that adding runtime PM support can bring some benefits: (1) Make the HD-A devices become part of system runtime PM and can bring more power saving. When the HD-A controller is suspended via runtime PM API, the runtime PM subsystem will also try to suspend the parent device. (2) PCI subsystem has implemented a framework for runtime PM, supporting both PCI PM and ACPI PM. When the HD-A controller is to be suspended, it can choose the lowest power state the device can signal wake up from. This state can be D3cold if the platform can support via ACPI. This also means more power saving. (3) Provide a general sysfs interface to upper level policy manager.
Heh, I'm not against the runtime PM support at all.
But when I read your initial comment, the only required thing is just to set PCI state of the controller, which was obviously missing from the beginning (my bad). So it was my follow up.
Of course, the runtime PM support would bring more. This must be mentioned in the patch itself properly (but not too detailed to be boring to read).
Looking at your original patch, just a few points I'd like to clarify:
- Don't use snd_printd(). It's a debug print most of distros turn ON, thus it'd be annoying. If any, use snd_printdd().
- The IRQ reassignment is required for runtime PM?
- Why you call snd_pcm_suspend_all() in azx_runtime_suspend()? In which situation would a PCM stream be pending?
- Are snd_power_change_state() calls needed for runtime PM?
thanks,
Takashi