| Message ID | 1375610369-16225-1-git-send-email-tim.gardner@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Aug 04, 2013 at 03:59:27AM -0600, Tim Gardner wrote: > From: Takashi Iwai <tiwai@suse.de> > > BugLink: http://bugs.launchpad.net/bugs/1183125 > > Add a hook to struct hda_codec for filtering the target power state of > each widget when powering up/down. The current hackish EAPD check is > implemented as the default hook pointer, too. > > This allows codec drivers to implement own power filter. In the > upcoming changes, the generic parser will have the better power filter > based on the active paths. > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > (cherry picked from commit 9419ab6b72325e20789a61004cf68dc9e909a009) > > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> This patch looks like it should have no functional change, and I don't think either of the subsequent patches depend on it. It looks safe enough, but it does contain a fair amount of churn. Are you sure this one is needed? Seth
On 08/05/2013 09:03 AM, Seth Forshee wrote: > On Sun, Aug 04, 2013 at 03:59:27AM -0600, Tim Gardner wrote: >> From: Takashi Iwai <tiwai@suse.de> >> >> BugLink: http://bugs.launchpad.net/bugs/1183125 >> >> Add a hook to struct hda_codec for filtering the target power state of >> each widget when powering up/down. The current hackish EAPD check is >> implemented as the default hook pointer, too. >> >> This allows codec drivers to implement own power filter. In the >> upcoming changes, the generic parser will have the better power filter >> based on the active paths. >> >> Signed-off-by: Takashi Iwai <tiwai@suse.de> >> (cherry picked from commit 9419ab6b72325e20789a61004cf68dc9e909a009) >> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > This patch looks like it should have no functional change, and I don't > think either of the subsequent patches depend on it. It looks safe > enough, but it does contain a fair amount of churn. Are you sure this > one is needed? > > Seth > I was basing the first commit on the info in https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1183125/comments/5 - so perhaps the simplest fix is just patch 2 & 3. rtg
So the 3rd commit (17df3f55652f7ea8fb1197b5c32e227b3da9f215) is written against a version of snd_hda_codec_set_power_to_all() that takes 3 arguments, whereas the current version of snd_hda_codec_set_power_to_all() in raring takes 4 args. The 1st commit (9419ab6b72325e20789a61004cf68dc9e909a009) is what changed the snd_hda_codec_set_power_to_all() function signature. So although you can cleanly cherry-pick just the 2nd and 3rd, you'll get a build failure. ----- Original Message ----- From: "Tim Gardner" <tim.gardner@canonical.com> To: "Seth Forshee" <seth.forshee@canonical.com> Cc: "Takashi Iwai" <tiwai@suse.de>, kernel-team@lists.ubuntu.com Sent: Monday, August 5, 2013 2:37:56 AM Subject: Re: [PATCH 1/3 Raring SRU] ALSA: hda - Add power state filtering On 08/05/2013 09:03 AM, Seth Forshee wrote: > On Sun, Aug 04, 2013 at 03:59:27AM -0600, Tim Gardner wrote: >> From: Takashi Iwai <tiwai@suse.de> >> >> BugLink: http://bugs.launchpad.net/bugs/1183125 >> >> Add a hook to struct hda_codec for filtering the target power state of >> each widget when powering up/down. The current hackish EAPD check is >> implemented as the default hook pointer, too. >> >> This allows codec drivers to implement own power filter. In the >> upcoming changes, the generic parser will have the better power filter >> based on the active paths. >> >> Signed-off-by: Takashi Iwai <tiwai@suse.de> >> (cherry picked from commit 9419ab6b72325e20789a61004cf68dc9e909a009) >> >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > This patch looks like it should have no functional change, and I don't > think either of the subsequent patches depend on it. It looks safe > enough, but it does contain a fair amount of churn. Are you sure this > one is needed? > > Seth > I was basing the first commit on the info in https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1183125/comments/5 - so perhaps the simplest fix is just patch 2 & 3. rtg
On 08/05/2013 10:29 PM, Jason DeRose wrote: > So the 3rd commit (17df3f55652f7ea8fb1197b5c32e227b3da9f215) is written against > a version of snd_hda_codec_set_power_to_all() that takes 3 arguments, whereas > the current version of snd_hda_codec_set_power_to_all() in raring takes 4 args. > > The 1st commit (9419ab6b72325e20789a61004cf68dc9e909a009) is what changed the > snd_hda_codec_set_power_to_all() function signature. So although you can > cleanly cherry-pick just the 2nd and 3rd, you'll get a build failure. > Good point.
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index e330933..303c54b 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1206,6 +1206,8 @@ static bool snd_hda_codec_get_supported_ps(struct hda_codec *codec, static unsigned int hda_set_power_state(struct hda_codec *codec, unsigned int power_state); +static unsigned int default_power_filter(struct hda_codec *codec, hda_nid_t nid, + unsigned int power_state); /** * snd_hda_codec_new - create a HDA codec @@ -1324,6 +1326,7 @@ int snd_hda_codec_new(struct hda_bus *bus, #endif codec->epss = snd_hda_codec_get_supported_ps(codec, fg, AC_PWRST_EPSS); + codec->power_filter = default_power_filter; /* power-up all before initialization */ hda_set_power_state(codec, AC_PWRST_D0); @@ -3521,29 +3524,23 @@ EXPORT_SYMBOL_HDA(snd_hda_sequence_write_cache); #endif /* CONFIG_PM */ void snd_hda_codec_set_power_to_all(struct hda_codec *codec, hda_nid_t fg, - unsigned int power_state, - bool eapd_workaround) + unsigned int power_state) { hda_nid_t nid = codec->start_nid; int i; for (i = 0; i < codec->num_nodes; i++, nid++) { unsigned int wcaps = get_wcaps(codec, nid); + unsigned int state = power_state; if (!(wcaps & AC_WCAP_POWER)) continue; - /* don't power down the widget if it controls eapd and - * EAPD_BTLENABLE is set. - */ - if (eapd_workaround && power_state == AC_PWRST_D3 && - get_wcaps_type(wcaps) == AC_WID_PIN && - (snd_hda_query_pin_caps(codec, nid) & AC_PINCAP_EAPD)) { - int eapd = snd_hda_codec_read(codec, nid, 0, - AC_VERB_GET_EAPD_BTLENABLE, 0); - if (eapd & 0x02) + if (codec->power_filter) { + state = codec->power_filter(codec, nid, power_state); + if (state != power_state && power_state == AC_PWRST_D3) continue; } snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE, - power_state); + state); } } EXPORT_SYMBOL_HDA(snd_hda_codec_set_power_to_all); @@ -3590,6 +3587,21 @@ static unsigned int hda_sync_power_state(struct hda_codec *codec, return state; } +/* don't power down the widget if it controls eapd and EAPD_BTLENABLE is set */ +static unsigned int default_power_filter(struct hda_codec *codec, hda_nid_t nid, + unsigned int power_state) +{ + if (power_state == AC_PWRST_D3 && + get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN && + (snd_hda_query_pin_caps(codec, nid) & AC_PINCAP_EAPD)) { + int eapd = snd_hda_codec_read(codec, nid, 0, + AC_VERB_GET_EAPD_BTLENABLE, 0); + if (eapd & 0x02) + return AC_PWRST_D0; + } + return power_state; +} + /* * set power state of the codec, and return the power state */ @@ -3615,8 +3627,7 @@ static unsigned int hda_set_power_state(struct hda_codec *codec, snd_hda_codec_read(codec, fg, 0, AC_VERB_SET_POWER_STATE, power_state); - snd_hda_codec_set_power_to_all(codec, fg, power_state, - true); + snd_hda_codec_set_power_to_all(codec, fg, power_state); } state = hda_sync_power_state(codec, fg, power_state); if (!(state & AC_PWRST_ERROR)) diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index af6259c..fa95d41 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -882,6 +882,10 @@ struct hda_codec { spinlock_t power_lock; #endif + /* filter the requested power state per nid */ + unsigned int (*power_filter)(struct hda_codec *codec, hda_nid_t nid, + unsigned int power_state); + /* codec-specific additional proc output */ void (*proc_widget_hook)(struct snd_info_buffer *buffer, struct hda_codec *codec, hda_nid_t nid); @@ -1038,8 +1042,7 @@ extern const struct snd_pcm_chmap_elem snd_pcm_2_1_chmaps[]; void snd_hda_get_codec_name(struct hda_codec *codec, char *name, int namelen); void snd_hda_bus_reboot_notify(struct hda_bus *bus); void snd_hda_codec_set_power_to_all(struct hda_codec *codec, hda_nid_t fg, - unsigned int power_state, - bool eapd_workaround); + unsigned int power_state); int snd_hda_lock_devices(struct hda_bus *bus); void snd_hda_unlock_devices(struct hda_bus *bus); diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index 09fae16..0eea727 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -435,7 +435,7 @@ static void conexant_set_power(struct hda_codec *codec, hda_nid_t fg, /* partial workaround for "azx_get_response timeout" */ if (power_state == AC_PWRST_D0) msleep(10); - snd_hda_codec_set_power_to_all(codec, fg, power_state, true); + snd_hda_codec_set_power_to_all(codec, fg, power_state); } static int conexant_init(struct hda_codec *codec) diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index a86547c..4b6cffe 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -5110,7 +5110,7 @@ static void stac92xx_set_power_state(struct hda_codec *codec, hda_nid_t fg, } snd_hda_codec_read(codec, fg, 0, AC_VERB_SET_POWER_STATE, afg_power_state); - snd_hda_codec_set_power_to_all(codec, fg, power_state, true); + snd_hda_codec_set_power_to_all(codec, fg, power_state); } #else #define stac92xx_suspend NULL