On 2/13/2018 1:44 PM, Takashi Iwai wrote:
On Tue, 13 Feb 2018 08:34:40 +0100,
Saarinen, Jani wrote:
HI,
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
Takashi Iwai
Sent: tiistai 13. helmikuuta 2018 7.11
To: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kumar, Abhijeet
<abhijeet.kumar@xxxxxxxxx>
Subject: Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core
codec functions to sync power state"
On Mon, 12 Feb 2018 18:29:53 +0100,
Chris Wilson wrote:
This reverts commit 3b5b899ca67db07a4c4825911072221f99e157e2.
Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to
sync power state")
Cc: Abhijeet Kumar <abhijeet.kumar@xxxxxxxxx>
Cc: Takashi Iwai <tiwai@xxxxxxx>
Did the patch break anything?
I don't understand it without any real context...
Yes. See resutls from link what is fixed by reverting:
https://patchwork.freedesktop.org/series/38097/
Test pm_rpm:
Subgroup basic-pci-d3-state:
fail -> PASS (fi-hsw-4770)
fail -> PASS (fi-bdw-5557u)
Subgroup basic-rte:
fail -> PASS (fi-hsw-4770)
fail -> PASS (fi-bdw-5557u)
Could you investigate why does the revert fix?
Two functions are almost identical. The difference is that the new
one has a counter and quit at 500 iteration, and it has a msleep(200)
at the error exit. But both shouldn't matter for the normal
operation...
FWIW, below is the patch to make the new function identical with the
original function. Does it change the behavior?
thanks,
Takashi
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -1079,15 +1079,13 @@ unsigned int snd_hdac_sync_power_state(struct hdac_device *codec,
hda_nid_t nid, unsigned int power_state)
{
unsigned long end_time = jiffies + msecs_to_jiffies(500);
- unsigned int state, actual_state, count;
+ unsigned int state, actual_state;
- for (count = 0; count < 500; count++) {
+ for (;;) {
I believe changing it back to finite should fix the regression.
state = snd_hdac_codec_read(codec, nid, 0,
AC_VERB_GET_POWER_STATE, 0);
- if (state & AC_PWRST_ERROR) {
- msleep(20);
When device raises the error state bit, we might endup sending two verbs
in back to
back manner.I would still recommend to have a 20ms sleep in-order to let
the device
clean up it's internal state.
+ if (state & AC_PWRST_ERROR)
break;
- }
actual_state = (state >> 4) & 0x0f;
if (actual_state == power_state)
break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
