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

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

[abhijeet.kumar]

From: Abhijeet Kumar <abhijeet.kumar@intel.com>

Finite loop was causing few igt@pm_rpm tests failure for HSW and BDW.
Thus changing it back to infinite one.

References: https://bugs.freedesktop.org/show_bug.cgi?id=105069

Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>
---
 sound/hda/hdac_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index 7ba100bb1c3f..5a1dd570cea7 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -1079,9 +1079,9 @@ 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) {
-- 
1.9.1

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

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

[Takashi Iwai]

On Tue, 13 Feb 2018 10:09:48 +0100,
abhijeet.kumar@intel.com wrote:
> 
> From: Abhijeet Kumar <abhijeet.kumar@intel.com>
> 
> Finite loop was causing few igt@pm_rpm tests failure for HSW and BDW.
> Thus changing it back to infinite one.

This missing the explanation why the finite loop fails.
I can guess, but some more details please.


Takashi

> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=105069
> 
> Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>
> ---
>  sound/hda/hdac_device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> index 7ba100bb1c3f..5a1dd570cea7 100644
> --- a/sound/hda/hdac_device.c
> +++ b/sound/hda/hdac_device.c
> @@ -1079,9 +1079,9 @@ 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) {
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

[Patchwork]

== Series Details ==

Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
URL   : https://patchwork.freedesktop.org/series/38140/
State : success

== Summary ==

Series 38140v1 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
https://patchwork.freedesktop.org/api/1.0/series/38140/revisions/1/mbox/

fi-bdw-5557u     total:288  pass:265  dwarn:0   dfail:0   fail:2   skip:21  time:449s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:494s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:289s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:478s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:475s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:459s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:585s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:285s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:2   skip:27  time:413s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:414s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:416s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:590s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:431s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:417s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:429s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:528s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-glk-dsi       total:117  pass:104  dwarn:0   dfail:0   fail:0   skip:12 

013536408c7f1cd9049580cfc71865fcdeb86fa0 drm-tip: 2018y-02m-13d-07h-39m-27s UTC integration manifest
8af4811b4156 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7990/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

[Chris Wilson]

Quoting Patchwork (2018-02-13 09:42:41)
> == Series Details ==
> 
> Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
> URL   : https://patchwork.freedesktop.org/series/38140/
> State : success
> 
> == Summary ==
> 
> Series 38140v1 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
> https://patchwork.freedesktop.org/api/1.0/series/38140/revisions/1/mbox/
[snip]
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7990/issues.html

In case it's not clear, this did not fix the pm regression. Odd!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

[Kumar, Abhijeet]

On 2/13/2018 3:15 PM, Chris Wilson wrote:
> Quoting Patchwork (2018-02-13 09:42:41)
>> == Series Details ==
>>
>> Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
>> URL   : https://patchwork.freedesktop.org/series/38140/
>> State : success
>>
>> == Summary ==
>>
>> Series 38140v1 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
>> https://patchwork.freedesktop.org/api/1.0/series/38140/revisions/1/mbox/
> [snip]
>> == Logs ==
>>
>> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7990/issues.html
> In case it's not clear, this did not fix the pm regression. Odd!
> -Chris
Strange. I believe the theory i was expecting doesn't hold strong.
Nevertheless shared another patch which makes the new function look 
exactly similar to the original one.

Hoping to see diff in behavior.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

[Saarinen, Jani]

Hi, 
Still fails. 

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Patchwork
> Sent: tiistai 13. helmikuuta 2018 11.43
> To: Kumar, Abhijeet <abhijeet.kumar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] ✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from
> snd_hdac_sync_power_state()
> 
> == Series Details ==
> 
> Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
> URL   : https://patchwork.freedesktop.org/series/38140/
> State : success
> 
> == Summary ==
> 
> Series 38140v1 ALSA: hda: Remove finite loop from
> snd_hdac_sync_power_state()
> https://patchwork.freedesktop.org/api/1.0/series/38140/revisions/1/mbox/
> 
> fi-bdw-5557u     total:288  pass:265  dwarn:0   dfail:0   fail:2   skip:21  time:449s
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7990/all.html

> fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
> fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
> fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:494s
> fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:289s
> fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:478s
> fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
> fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:475s
> fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:459s
> fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
> fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:585s
> fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
> fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:285s
> fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
> fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:2   skip:27  time:413s
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7990/all.html

> fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:414s
> fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:463s
> fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:416s
> fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
> fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
> fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
> fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:590s
> fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:431s
> fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:513s
> fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
> fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
> fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:498s
> fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:417s
> fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:429s
> fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:528s
> fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:401s
> Blacklisted hosts:
> fi-glk-dsi       total:117  pass:104  dwarn:0   dfail:0   fail:0   skip:12
> 
> 013536408c7f1cd9049580cfc71865fcdeb86fa0 drm-tip: 2018y-02m-13d-07h-
> 39m-27s UTC integration manifest
> 8af4811b4156 ALSA: hda: Remove finite loop from
> snd_hdac_sync_power_state()
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-
> tip/Patchwork_7990/issues.html
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[abhijeet.kumar]

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;
-- 
1.9.1

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

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

[Kumar, Abhijeet]

[-- Attachment #1.1: Type: text/plain, Size: 3965 bytes --]



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.

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 ?



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.

<7>[357.854160] [IGT] pm_rpm: starting subtest basic-pci-d3-state

<7>[357.854231] [drm:drm_mode_setcrtc] [CRTC:37:pipe A]

<7>[357.854519] [drm:intel_atomic_check [i915]] New cdclk calculated to 
be logical 337500 kHz, actual 337500 kHz

<7>[357.854617] [drm:intel_atomic_check [i915]] New voltage level 
calculated to be logical 2, actual 2

<7>[357.868799] [drm:intel_disable_pipe [i915]] disabling pipe A

<7>[357.887493] [drm:__intel_fbc_disable [i915]] Disabling FBC on pipe A

<7>[357.887600] [drm:intel_disable_shared_dpll [i915]] disable WRPLL 1 
(active 1, on? 1) for crtc 37

<7>[357.887695] [drm:intel_disable_shared_dpll [i915]] disabling WRPLL 1

<7>[357.887796] [drm:intel_atomic_commit_tail [i915]] [ENCODER:58:DDI B]

<7>[357.887884] [drm:intel_atomic_commit_tail [i915]] [ENCODER:64:DDI C]

<7>[357.887972] [drm:intel_atomic_commit_tail [i915]] [ENCODER:66:DP-MST A]

<7>[357.888052] [drm:intel_atomic_commit_tail [i915]] [ENCODER:67:DP-MST B]

<7>[357.888130] [drm:intel_atomic_commit_tail [i915]] [ENCODER:68:DP-MST C]

<7>[357.888363] [drm:verify_connector_state.isra.76 [i915]] 
[CONNECTOR:70:HDMI-A-2]

<7>[357.888490] [drm:verify_single_dpll_state.isra.77 [i915]] WRPLL 1

<7>[357.888608] [drm:verify_single_dpll_state.isra.77 [i915]] WRPLL 2

<7>[357.888727] [drm:verify_single_dpll_state.isra.77 [i915]] SPLL

<7>[ 357.888838] [drm:verify_single_dpll_state.isra.77 [i915]] LCPLL 810

<7>[357.888940] [drm:verify_single_dpll_state.isra.77 [i915]] LCPLL 1350

<7>[357.889052] [drm:verify_single_dpll_state.isra.77 [i915]] LCPLL 2700

<7>[357.889347] [drm:intel_atomic_commit_tail [i915]] [CRTC:37:pipe A]

<7>[357.889583] [drm:drm_mode_setcrtc] [CRTC:47:pipe B]

<7>[357.890214] [drm:drm_mode_setcrtc] [CRTC:57:pipe C]

<6>[358.533287] e1000e: enp0s25 NIC Link is Up 1000 Mbps Full Duplex, 
Flow Control: None

<7>[368.413068] [IGT] pm_rpm: exiting, ret=99 <- 10 secs diff. that's 
the timeout for wait_for_suspended().

<6>[368.423030] ahci 0000:00:1f.2: port does not support device sleep

Any clue why ata port is reporting such error when Device is trying to 
sleep ?


Warm Regards,

Abhijeet






[-- Attachment #1.2: Type: text/html, Size: 21135 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

[Chris Wilson]

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

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

[Kumar, Abhijeet]

On 2/13/2018 6:17 PM, 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() ?
Both helper function boils down to same thing.
> It used to pass &codec->core and now it's just using codec.
Earlier codec was pointing to hda_codec struct
and now it's pointing to hdac_device (which is nothing but core) .
-Abhijeet
>   
>> 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.

If that's the case then can we not try increasing the timeout from igt 
test just for a trial run?
To see whether it actually reaches to the suspended state or not?
>
>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Takashi Iwai]

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

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

[Kumar, Abhijeet]

On 2/13/2018 6:17 PM, 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.
>   
>> 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.

One more thing to note in kernel log is that we don't see display 
power-well going off despite the fact we
had unset all crtcs. Does that mean (not all but some) screen are still 
enabled ?
> Correct.
> -Chris

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

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

[Jani Nikula]

On Tue, 13 Feb 2018, 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")

Side note, you're not supposed to word-wrap tags like these.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state() (rev2)

[Patchwork]

== Series Details ==

Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state() (rev2)
URL   : https://patchwork.freedesktop.org/series/38140/
State : success

== Summary ==

Series 38140v2 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()
https://patchwork.freedesktop.org/api/1.0/series/38140/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713 +1

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:265  dwarn:0   dfail:0   fail:2   skip:21  time:440s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:426s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:490s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:477s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:473s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:456s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:585s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:282s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:2   skip:27  time:416s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:411s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:459s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:419s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:498s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:591s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:526s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:414s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:471s

fb7ff325f92703d89572a55584a73f03fa296ebc drm-tip: 2018y-02m-13d-10h-09m-45s UTC integration manifest
c224b9c32f62 ALSA: hda: Remove finite loop from snd_hdac_sync_power_state()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7993/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for ALSA: hda: Remove finite loop from snd_hdac_sync_power_state() (rev2)

[Patchwork]

== Series Details ==

Series: ALSA: hda: Remove finite loop from snd_hdac_sync_power_state() (rev2)
URL   : https://patchwork.freedesktop.org/series/38140/
State : warning

== Summary ==

Test kms_vblank:
        Subgroup pipe-a-wait-forked:
                pass       -> SKIP       (shard-snb)
Test pm_rpm:
        Subgroup cursor:
                fail       -> PASS       (shard-hsw) fdo#105069 +5
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                pass       -> SKIP       (shard-snb)
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                skip       -> FAIL       (shard-snb) fdo#102365
Test perf:
        Subgroup enable-disable:
                pass       -> FAIL       (shard-apl) fdo#103715
Test kms_mmap_write_crc:
                skip       -> PASS       (shard-apl)
Test kms_flip:
        Subgroup 2x-plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-apl) fdo#99912

fdo#105069 https://bugs.freedesktop.org/show_bug.cgi?id=105069
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#103715 https://bugs.freedesktop.org/show_bug.cgi?id=103715
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-apl        total:3394 pass:1768 dwarn:1   dfail:0   fail:23  skip:1600 time:13696s
shard-hsw        total:3427 pass:1721 dwarn:1   dfail:0   fail:50  skip:1654 time:14676s
shard-snb        total:3427 pass:1346 dwarn:1   dfail:0   fail:12  skip:2068 time:7600s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7993/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx