Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"

[Takashi Iwai <tiwai@suse.de>]

On Tue, 13 Feb 2018 08:34:40 +0100,
Saarinen, Jani wrote:
> 
> HI, 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> > Takashi Iwai
> > Sent: tiistai 13. helmikuuta 2018 7.11
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> > <abhijeet.kumar@intel.com>
> > 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.kumar@intel.com>
> > > Cc: Takashi Iwai <tiwai@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:
> 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 (;;) {
 		state = snd_hdac_codec_read(codec, nid, 0,
 				AC_VERB_GET_POWER_STATE, 0);
-		if (state & AC_PWRST_ERROR) {
-			msleep(20);
+		if (state & AC_PWRST_ERROR)
 			break;
-		}
 		actual_state = (state >> 4) & 0x0f;
 		if (actual_state == power_state)
 			break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx