On 2/13/2018 1:44 PM, Takashi Iwai wrote:
On Tue, 13 Feb 2018 08:34:40 +0100,
Saarinen, Jani wrote:
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Takashi Iwai
Sent: tiistai 13. helmikuuta 2018 7.11
To: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
Subject: Re: [Intel-gfx] [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.ku...@intel.com>
Cc: Takashi Iwai <ti...@suse.de>
Did the patch break anything?
I don't understand it without any real context...
Yes. See resutls from link what is fixed by reverting:
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

FWIW, below is the patch to make the new function identical with the
original function.  Does it change the behavior?



--- 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)
-               }
                actual_state = (state >> 4) & 0x0f;
                if (actual_state == power_state)

Intel-gfx mailing list

Reply via email to