[alsa-devel] [PATCH V2] ALSA: hda - power setting error check
codec may reject power state transition requests(reporting PS-ERROR set), in that case we re-issue a power state setting and check error bit again.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com --- sound/pci/hda/hda_codec.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index d0ca370..a93c7ca 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3526,6 +3526,7 @@ static bool snd_hda_codec_get_supported_ps(struct hda_codec *codec, hda_nid_t fg static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, unsigned int power_state) { + int state, count = 0; if (codec->patch_ops.set_power_state) { codec->patch_ops.set_power_state(codec, fg, power_state); return; @@ -3537,9 +3538,19 @@ static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, bool epss = snd_hda_codec_get_supported_ps(codec, fg, AC_PWRST_EPSS); msleep(epss? 10:100); } - 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); + + /* repeat power states setting at most 10 times*/ + while (count++ > 10) { + 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); + state = snd_hda_codec_read(codec, fg, 0, + AC_VERB_GET_POWER_STATE, 0); + if (!(state & AC_PWRST_ERROR)) + break; + } + if (count > 10) + snd_printk(KERN_WARNING "Power states transition reject!\n"); }
/* modem wake on ring: transition from D2 to D0 in less than 2ms. For modems,
Xingchao,
On Thu, Jun 07, 2012 at 06:21:30PM +0800, Wang Xingchao wrote:
codec may reject power state transition requests(reporting PS-ERROR set), in that case we re-issue a power state setting and check error bit again.
Is this patch based on real errors observed? And tested to fix the issue?
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index d0ca370..a93c7ca 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3526,6 +3526,7 @@ static bool snd_hda_codec_get_supported_ps(struct hda_codec *codec, hda_nid_t fg static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, unsigned int power_state) {
- int state, count = 0;
nit pick: break that into two lines?
state should be unsigned int.
if (codec->patch_ops.set_power_state) { codec->patch_ops.set_power_state(codec, fg, power_state); return; @@ -3537,9 +3538,19 @@ static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, bool epss = snd_hda_codec_get_supported_ps(codec, fg, AC_PWRST_EPSS); msleep(epss? 10:100); }
- 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);
- /* repeat power states setting at most 10 times*/
- while (count++ > 10) {
< 10?
for(;;) looks slightly better, by putting the initialization together with test and increase.
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);state = snd_hda_codec_read(codec, fg, 0,AC_VERB_GET_POWER_STATE, 0);if (!(state & AC_PWRST_ERROR))break;
Do you have any clue the condition of success? For example, is it time based (the codec takes some time to respond) or retry count based?
- }
- if (count > 10)
snd_printk(KERN_WARNING "Power states transition reject!\n");
Power state transition rejected
Hmm, if some codec always rejects it, the user may get lots of these warnings..
Thanks, Fengguang
Hi fengguang,
2012/6/7 Fengguang Wu fengguang.wu@intel.com:
Xingchao,
On Thu, Jun 07, 2012 at 06:21:30PM +0800, Wang Xingchao wrote:
codec may reject power state transition requests(reporting PS-ERROR set), in that case we re-issue a power state setting and check error bit again.
Is this patch based on real errors observed? And tested to fix the issue?
Not exactly, i tested the patch on my netbook with codec ALC269 and didnot meet regression. It's not based on real error but from the HDA spec description about PowerStates setting part. The spec said PS-ERROR bit indicates the power states transition rejection so we'd better check the status when try to set codec power state.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index d0ca370..a93c7ca 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3526,6 +3526,7 @@ static bool snd_hda_codec_get_supported_ps(struct hda_codec *codec, hda_nid_t fg static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, unsigned int power_state) {
- int state, count = 0;
nit pick: break that into two lines?
state should be unsigned int.
if (codec->patch_ops.set_power_state) { codec->patch_ops.set_power_state(codec, fg, power_state); return; @@ -3537,9 +3538,19 @@ static void hda_set_power_state(struct hda_codec *codec, hda_nid_t fg, bool epss = snd_hda_codec_get_supported_ps(codec, fg, AC_PWRST_EPSS); msleep(epss? 10:100); }
- 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);
- /* repeat power states setting at most 10 times*/
- while (count++ > 10) {
< 10?
for(;;) looks slightly better, by putting the initialization together with test and increase.
thanks your review above, i will fix these errors.
- 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);
- state = snd_hda_codec_read(codec, fg, 0,
- AC_VERB_GET_POWER_STATE, 0);
- if (!(state & AC_PWRST_ERROR))
- break;
Do you have any clue the condition of success? For example, is it time based (the codec takes some time to respond) or retry count based?
Good question. snd_hda_codec_read() will send the command and get the response, but power setting verb id (705h) acturally has response "0" all the time. As the spec said, if the node remain in its previous power states, it sets PS-ERROR bit immediately. If the node doesnot set PS-Error bit after it received the Set Power State Verb, that means the transition is successful. That's why i didnot add some time delay to wait for the response here.
- }
- if (count > 10)
- snd_printk(KERN_WARNING "Power states transition reject!\n");
Power state transition rejected
Hmm, if some codec always rejects it, the user may get lots of these warnings..
Will remove the warning message here.
--thanks xingchao
participants (3)
-
Fengguang Wu -
Wang Xingchao -
Wang Xingchao