On Fri, 12 May 2023 13:23:49 +0200, Takashi Iwai wrote: > > On Thu, 11 May 2023 19:20:17 +0200, > Amadeusz Sławiński wrote: > > > > On 5/11/2023 5:58 PM, Takashi Iwai wrote: > > > On Thu, 11 May 2023 17:31:37 +0200, > > > Amadeusz Sławiński wrote: > > >> > > >> On 5/10/2023 2:21 PM, Takashi Iwai wrote: > > >>> On Tue, 09 May 2023 12:10:06 +0200, > > >>> Amadeusz Sławiński wrote: > > >> Then capture stream starts and seems to assume that > > >> registers were already set, so it doesn't write them to hw. > > > > > > ... it seems this didn't happen, and that's the inconsistency. > > > > > > So the further question is: > > > At the point just before you start recording, is the codec in runtime > > > suspended? Or it's running? > > > > > > If it's runtime-suspended, snd_hda_regmap_sync() must be called from > > > alc269_resume() via runtime-resume, and this must write out the > > > cached values. Then the bug can be along with that line. > > > > > > Or if it's running, it means that the previous check of > > > snd_hdac_keep_power_up() was bogus (or racy). > > > > > > > Well, it is in... let's call it semi powered state. When snd_hda_intel > > driver is loaded with power_save=X option it sets timeout to X seconds > > and problem only happens when I start the stream before those X > > seconds pass and it runs first runtime suspend. After it suspends it > > then uses standard pm_runtime_resume and works correctly. That's why > > the pm_runtime_force_suspend(&codec->core.dev); mentioned in first > > email in thread "fixes" the problem, as it forces it to be instantly > > suspended instead of waiting for timeout and then later normal > > resume-play/record-suspend flow can be followed. > > Hm, then maybe it's a bad idea to rely on the usage count there. > Even if the usage is 0, the device can be still active, and the update > can be missed. > > How about the patch like below? Scratch that, it returns a wrong value. A simpler version like below works instead? Takashi --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -611,10 +611,9 @@ 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); - if (!ret) + if (!pm_runtime_active(&codec->dev)) return -1; - if (ret < 0) + if (pm_runtime_get_sync(&codec->dev) < 0) return 0; } return 1;