[alsa-devel] [PATCH] ALSA: hda - Fix runtime PM accounting

[Takashi Iwai]

The flag codec->d3_clk_stop_ok isn't always set when the codec is
powered off.  If it's not set, the controller driver doesn't change
the runtime PM count.  This causes an unblanced accounting when
multiple codecs are present.  Usually, the first codec is powered down
with CLK_STOP_OK=0, then the next codec with CLK_STOP_OK=1.  At this
moment, the first codec is changed also to CLK_STOP_OK=1, but this
isn't informed to the controller.  Thus it still thinks as if one
codec is active.

For fixing this issue, move the check of CLK_STOP_OK bit at the
runtime suspend time, and simple up/down by the codec power save in
azx_power_notifier().  For CLK_STOP_OK bit checks of all codecs, a new
helper function is introduced.

With this change, d3_clk_stop_ok field is no longer referred, thus
removed.  Also, d3_clk_stop bit field is moved to a place where more
efficiently packed.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---

I faced this issue while I'm checking the pm issue.
Mengdong, could you verify whether this change is OK?

 sound/pci/hda/hda_codec.c | 30 ++++++++++++++++++++----------
 sound/pci/hda/hda_codec.h |  6 +++---
 sound/pci/hda/hda_intel.c |  4 ++--
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 4a2f35c..b7ac1ff 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3546,10 +3546,6 @@ static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg,
 	int count;
 	unsigned int state;
 
-#ifdef CONFIG_SND_HDA_POWER_SAVE
-	codec->d3_stop_clk_ok = 0;
-#endif
-
 	if (codec->patch_ops.set_power_state) {
 		codec->patch_ops.set_power_state(codec, fg, power_state);
 		return;
@@ -3572,12 +3568,6 @@ static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg,
 		if (!(state & AC_PWRST_ERROR))
 			break;
 	}
-
-#ifdef CONFIG_SND_HDA_POWER_SAVE
-	if ((power_state == AC_PWRST_D3)
-		&& codec->d3_stop_clk && (state & AC_PWRST_CLK_STOP_OK))
-		codec->d3_stop_clk_ok = 1;
-#endif
 }
 
 #ifdef CONFIG_SND_HDA_HWDEP
@@ -4532,6 +4522,26 @@ void snd_hda_power_save(struct hda_codec *codec, int delta, bool d3wait)
 }
 EXPORT_SYMBOL_HDA(snd_hda_power_save);
 
+/* check whether all codecs are ready to stop clock */
+bool snd_hda_bus_ready_for_d3(struct hda_bus *bus)
+{
+	struct hda_codec *codec;
+
+	list_for_each_entry(codec, &bus->codec_list, list) {
+		unsigned int state;
+		int fg = codec->afg ? codec->afg : codec->mfg;
+
+		if (!codec->d3_stop_clk)
+			return false;
+		state = snd_hda_codec_read(codec, fg, 0,
+					   AC_VERB_GET_POWER_STATE, 0);
+		if (!(state & AC_PWRST_CLK_STOP_OK))
+			return false;
+	}
+	return true;
+}
+EXPORT_SYMBOL_HDA(snd_hda_bus_ready_for_d3);
+
 /**
  * snd_hda_check_amp_list_power - Check the amp list and update the power
  * @codec: HD-audio codec
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 13c834f..abd85e5 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -868,6 +868,7 @@ struct hda_codec {
 	unsigned int pcm_format_first:1; /* PCM format must be set first */
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	unsigned int power_on :1;	/* current (global) power-state */
+	unsigned int d3_stop_clk:1;	/* support D3 operation without BCLK */
 	int power_transition;	/* power-state in transition */
 	int power_count;	/* current (global) power refcount */
 	struct delayed_work power_work; /* delayed task for powerdown */
@@ -875,9 +876,6 @@ struct hda_codec {
 	unsigned long power_off_acct;
 	unsigned long power_jiffies;
 	spinlock_t power_lock;
-
-	unsigned int d3_stop_clk:1;	/* support D3 operation without BCLK */
-	unsigned int d3_stop_clk_ok:1; /* BCLK can stop */
 #endif
 
 	/* codec-specific additional proc output */
@@ -1068,9 +1066,11 @@ const char *snd_hda_get_jack_location(u32 cfg);
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 void snd_hda_power_save(struct hda_codec *codec, int delta, bool d3wait);
 void snd_hda_update_power_acct(struct hda_codec *codec);
+bool snd_hda_bus_ready_for_d3(struct hda_bus *bus);
 #else
 static inline void snd_hda_power_save(struct hda_codec *codec, int delta,
 				      bool d3wait) {}
+static inline bool snd_hda_bus_ready_for_d3(struct hda_bus *bus) { return false; }
 #endif
 
 /**
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 1c9c779..9b02174 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2410,7 +2410,7 @@ static void azx_power_notify(struct hda_bus *bus, struct hda_codec *codec)
 {
 	struct azx *chip = bus->private_data;
 
-	if (bus->power_keep_link_on || !codec->d3_stop_clk_ok)
+	if (bus->power_keep_link_on)
 		return;
 
 	if (codec->power_on)
@@ -2529,7 +2529,7 @@ static int azx_runtime_suspend(struct device *dev)
 	struct azx *chip = card->private_data;
 
 #ifdef CONFIG_SND_HDA_POWER_SAVE
-	if (!power_save_controller)
+	if (!power_save_controller || !snd_hda_bus_ready_for_d3(chip->bus))
 		return -EAGAIN;
 #endif
 
-- 
1.7.11.4