Re: [PATCH v2] ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

[Takashi Iwai <tiwai@suse.de>]

On Tue, 13 Feb 2018 13:47:25 +0100,
Chris Wilson wrote:
> 
> Quoting Kumar, Abhijeet (2018-02-13 12:41:42)
> > 
> > 
> > On 2/13/2018 3:54 PM, abhijeet.kumar@intel.com wrote:
> > 
> >     From: Abhijeet Kumar <abhijeet.kumar@intel.com>
> > 
> >     Finite loop and msleep was causing few igt@pm_rpm tests failure
> >     for HSW and BDW. Thus removing them.
> > 
> >     Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to
> >                     sync power state")
> >     References: https://bugs.freedesktop.org/show_bug.cgi?id=105069
> > 
> >     Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>
> >     ---
> >     Changes in v2:
> >     1. Removed msleep as well.
> >     2. Modified commit message.
> >      sound/hda/hdac_device.c | 8 +++-----
> >      1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> >     diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> >     index 7ba100bb1c3f..678ef8950d0c 100644
> >     --- 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;
> > 
> > The above changes is as good as revert. But we can still repro the issue.
> 
> What about the different between snd_hda_codec_read() and
> snd_hdac_codec_read() ?
> 
> It used to pass &codec->core and now it's just using codec.

It's identical.  "codec" in the earlier code is struct snd_hda_codec,
and it embeds struct hdac_device in codec->core field.  So
&codec->core points to hdac_device object to be passed to
snd_hdac_codec_read().


Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx