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 (;;) { 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
