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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Kumar, Abhijeet (2018-02-13 12:41:42)
> 
> 
> On 2/13/2018 3:54 PM, abhijeet.kumar@xxxxxxxxx wrote:
> 
>     From: Abhijeet Kumar <abhijeet.kumar@xxxxxxxxx>
> 
>     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@xxxxxxxxx>
>     ---
>     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.
 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7993/all.html
> 
> Are we sure that this is the only bad commit for regression ?

Absolutely.

> And by looking at the logs it seems like the reason for test failure is that
> after disabling all the screen
> 
> the device state has still not reached the suspended state. Thus timing out and
> asserting.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux