Re: HDA, power saving and recording

[Takashi Iwai <tiwai@suse.de>]

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?


thanks,

Takashi

--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -610,8 +610,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_power_up_pm);
  */
 int snd_hdac_keep_power_up(struct hdac_device *codec)
 {
+	int ret;
+
 	if (!atomic_inc_not_zero(&codec->in_pm)) {
-		int ret = pm_runtime_get_if_in_use(&codec->dev);
+		if (pm_runtime_active(&codec->dev))
+			return pm_runtime_get_sync(&codec->dev);
+		ret = pm_runtime_get_if_in_use(&codec->dev);
 		if (!ret)
 			return -1;
 		if (ret < 0)