Re: HDA, power saving and recording

[Takashi Iwai <tiwai@suse.de>]

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;