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:
1. After probe codec is powered down
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/pci/hda/hda_codec.c#n833),
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.
2. 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/sound/pci/hda/hda_codec.c#n2899
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.
