On 2023-05-15 5:33 PM, Takashi Iwai wrote:
On Mon, 15 May 2023 16:49:48 +0200, Amadeusz Sławiński wrote:
...
I think there are two problems:
- After probe codec is powered down
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/soun...), even though according to power management it is still running
I guess it's in the auto-suspend state, so it's still not suspended but the device itself is active, while the usage count is 0.
That's fine, and I thought my second patch handling it. That is, if the usage count is 0 and the device is not suspended, it should return -EAGAIN and make the caller retry with the full power up. The code path is with CALL_RUN_FUNC() macro in hdac_regmap.c, and with -EAGAIN return value, it tries snd_hdac_power_up_pm() and call the function again.
- When stream is started before first suspend, resume function
doesn't run and it is a function which syncs cached registers. By resume function I mean https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/soun... which calls snd_hda_regmap_sync() or through in case of the platform I test it on codec->patch_ops.resume(codec) -> alc269_resume, which also calls snd_hda_regmap_sync().
It's also expected, per se. Since it's been not suspended, it assumes that the value got already written, and no resume is needed.
After reading this conversation few times I came to conclusion that codec device should be kept up as long as its runtime_status=0 (RPM_ACTIVE), regardless if usage_count is 0 or not. Basically, in autosuspend case usage_count and runtime_status for the device are both 0 so, if we are not ignoring the former by calling pm_runtime_get_if_in_use() then we end up caching the writes during the autosuspend period - period when the device is no longer used but there is still some time left before it's suspended.
--- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -611,7 +611,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_power_up_pm); int snd_hdac_keep_power_up(struct hdac_device *codec) { if (!atomic_inc_not_zero(&codec->in_pm)) { - int ret = pm_runtime_get_if_in_use(&codec->dev); + int ret = pm_runtime_get_if_active(&codec->dev, true); if (!ret) return -1; if (ret < 0)
Results for the above look good.