> Now I introduced codec->epss flag that is initialized once in
> snd_hda_codec_new(), and let the codec driver overrides this flag.
> One bonus is that you can avoid the unnecessary codec parameter read at
> each power change.
Yes, this is better. And the patch works well for me - [PATCH 1/2] ALSA: hda - Avoid unnecessary parameter read for EPSS.
> In anyway, there are a few issues:
> - The refcounting gets broken after module unload. It's because we
> leave the codec's pm_runtime usages after removal.
Thanks for pointing out this! But I think the following codec can introduce risk of race among multiple codecs to modify ' chip->power_count',
because only one codec's power on/off is serialized.
static void azx_power_notify(struct hda_bus *bus, struct hda_codec *codec)
{
...
if (codec->power_on) {
pm_runtime_get_sync(&chip->pci->dev);
chip->power_count++;
} else {
pm_runtime_put_sync(&chip->pci->dev);
chip->power_count--;
}
}
How about calling pm_runtime_put_noidle() when freeing a codec in D0?
Maybe we can add a bus notify function like this:
Static void azx_codec_free_notify(struct hda_codec *codec)
{
If(codec->power_on)
pm_runtime_put_noidle(&codec->bus->pci->dev);
}
And snd_hda_codec_free() can trigger this notify.
> - The D3-stop-clock check isn't done properly for the codecs providing
> own set_power_state ops.
I cannot test the branch 'if (codec->patch_ops.set_power_state)' in hda_set_power_state() for lack of such a codec.
But the logic seems right and the other branch works well for me :-)
> - Ditto for the repeated power-set sequence, the error check, etc.
In hda_sync_power_state(),
if ((state & 0xff) == power_state) ... should be if ((state & 0xf0)>> 4 == power_state), because PS-Act is bits 7:4
break;
And would you please explain why changing to D3 need not wait? One of my codec does have a delay on change from D0 to D3.
Thanks
Mengdong