Hi,
> -----Original Message-----
> From: Takashi Iwai [mailto:ti...@suse.de]
> Sent: tiistai 13. helmikuuta 2018 10.14
> To: Saarinen, Jani <jani.saari...@intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org;
> Kumar, Abhijeet <abhijeet.ku...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core
> codec functions to sync power state"
>
> On Tue, 13 Feb 2018 08:34:40 +0100,
> Saarinen, Jani wrote:
> >
> > HI,
> > > -----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
> > > <abhijeet.ku...@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.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:
> > 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?
Maybe you instead.
> 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?
>
Just send it for ml so that CI can test.
>
> thanks,
>
> Takashi
Br,
Jani
>
>
> --- 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
