[alsa-devel] HD-audio runtime PM

[Takashi Iwai]

At Tue, 26 Nov 2013 09:56:00 +0100,
Takashi Iwai wrote:
> 
> At Tue, 26 Nov 2013 08:53:00 +0000,
> Lin, Mengdong wrote:
> > 
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > Sent: Tuesday, November 26, 2013 3:28 PM
> > > To: Lin, Mengdong
> > > Cc: Wang Xingchao; Alsa-devel at alsa-project.org; David Henningsson
> > > Subject: Re: [alsa-devel] HD-audio runtime PM
> > > 
> > > At Tue, 26 Nov 2013 06:56:12 +0000,
> > > Lin, Mengdong wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > > > Sent: Tuesday, November 26, 2013 2:14 PM
> > > > > To: Wang Xingchao
> > > >
> > > > > > I test WAKEEN feature on Haswell machines before, it could really
> > > > > > wakeup system from D3.
> > > > >
> > > > > But the runtime suspend doesn't power down to D3 by itself.
> > > > > Did you test really with D3?
> > > >
> > > > I will double check this on Haswell with sound git tree for-linus branch.
> > > > We've tried Android on Haswell-ULT last week, the display HD-A
> > > controller can enter D3 and HDMI cable plug-in can wake up the controller
> > > and codec. The code base is v3.9 with various driver patches.
> > > 
> > > Hrm, but as I mentioned, we have no D3 call in runtime suspend callback.
> > > How does it reach to D3 then?  Maybe that part is patched in your side?
> > > 
> > > > For non-Haswell platforms, hardware team suggest that unless all of the
> > > codecs on the HD-A link support EPSS and ClkStopOK, runtime PM of the
> > > controller should not be enabled on the controller. Otherwise,
> > > functionality will be lost and there are likely going to be audio artifacts.
> > > 
> > > Is EPSS mandatory?  We have already a check of ClkStopOK bit, but EPSS
> > > influences only on the wait time, so far.
> > 
> > It's a suggestion based on Windows experience. 
> > So maybe we can add this check at the moment. How do you think?
> 
> Yes, it should be mostly harmless, as the recent codecs support
> already EPSS, and we enabled the runtime PM only on new controllers.
> 
> We can simply move the code in patch_hdmi.c to hda_codec.c.

That is, a patch like below.  This was queued to test/hda branch of
sound-unstable git tree.


Takashi

-- >8 --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: hda - Disable runtime PM when EPSS is unavailable

According to Mengdong, we shouldn't enable runtime PM when a codec
doesn't support EPSS, based on the experiences on Windows.
We have already this check in HDMI codec drivers, but now apply it in
general in hda_codec.c.

Credit goes to Mengdong Lin <mengdong.lin at intel.com>

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/hda/hda_codec.c  |  7 +++++--
 sound/pci/hda/patch_hdmi.c | 11 -----------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 8f7295bdccbd..6deb8d1852cc 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1510,11 +1510,14 @@ int snd_hda_codec_new(struct hda_bus *bus,
 #ifdef CONFIG_PM
 	codec->d3_stop_clk = snd_hda_codec_get_supported_ps(codec, fg,
 					AC_PWRST_CLKSTOP);
-	if (!codec->d3_stop_clk)
-		bus->power_keep_link_on = 1;
 #endif
 	codec->epss = snd_hda_codec_get_supported_ps(codec, fg,
 					AC_PWRST_EPSS);
+#ifdef CONFIG_PM
+	if (!codec->d3_stop_clk || !codec->epss)
+		bus->power_keep_link_on = 1;
+#endif
+
 
 	/* power-up all before initialization */
 	hda_set_power_state(codec, AC_PWRST_D0);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 1773a640e72e..4098196b2300 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1692,17 +1692,6 @@ static int hdmi_parse_codec(struct hda_codec *codec)
 		}
 	}
 
-#ifdef CONFIG_PM
-	/*
-	 * G45/IbexPeak don't support EPSS: the unsolicited pin hot plug event
-	 * can be lost and presence sense verb will become inaccurate if the
-	 * HDA link is powered off at hot plug or hw initialization time.
-	 */
-	if (!(snd_hda_param_read(codec, codec->afg, AC_PAR_POWER_STATE) &
-	      AC_PWRST_EPSS))
-		codec->bus->power_keep_link_on = 1;
-#endif
-
 	return 0;
 }
 
-- 
1.8.4.3