-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, August 29, 2012 10:59 PM
The patch below is the revised version. I think it's a good cleanup at the same time. Basically it makes pm_notify callback simply turning up/down the controller power without checking extra flags there. The conditions are checked in the caller side instead.
The pm_notify is called at codec creation and deletion times (but without checking extra flags), thus the refcounts can be managed more easily.
Hi Takashi,
Usage of the new pm_notify() looks nice. The v3 patch works well on my platform.
However, maybe we can add some check before notifying the codec power-down in snd_hda_codec_free().
static void snd_hda_codec_free(struct hda_codec *codec) { ... restore_init_pincfgs(codec); #ifdef CONFIG_SND_HDA_POWER_SAVE cancel_delayed_work(&codec->power_work); flush_workqueue(codec->bus->workq); #endif ... if (codec->patch_ops.free) codec->patch_ops.free(codec); module_put(codec->owner); hda_call_pm_notify(codec->bus, false); ... }
restore_init_pincfgs() will resume the codec to D0, and since the power_save timeout is at least 1second, so it's almost always safe to call hda_call_pm_notify(codec->bus, false) here. But an extremely low possibility is: the codec will be suspended again and call hda_call_pm_notify(,false) in hda_power_work() before the work is cancelled and queue is flushed. And the flag 'codec->d3_stop_clk_ok' also cannot tell us whether a pm_notify(, false) has already been called or not, because codec->patch_ops.free() may call hda_set_power_state (D3) unexpectedly.
So, I think that each codec still needs a flag 'power_count' and let the pm_notify modify the count:
static void azx_power_notify(struct hda_bus *bus, struct hda_codec *codec, bool power_up) { struct azx *chip = bus->private_data;
if (power_up) { pm_runtime_get_sync(&chip->pci->dev); codec->power_count ++; } else { pm_runtime_put_sync(&chip->pci->dev); codec->power_count --; } }
And static void snd_hda_codec_free(struct hda_codec *codec) { ... +#ifdef CONFIG_SND_HDA_POWER_SAVE + If(codec-> power_count) hda_call_pm_notify(codec->bus, false); +#endif ... }
Thanks Mengdong