* [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
@ 2018-02-12 17:29 Chris Wilson
2018-02-12 18:28 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Chris Wilson @ 2018-02-12 17:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Takashi Iwai, Abhijeet Kumar
This reverts commit 3b5b899ca67db07a4c4825911072221f99e157e2.
Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state")
Cc: Abhijeet Kumar <abhijeet.kumar@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
---
sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
sound/pci/hda/hda_local.h | 6 +-----
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 5bc3a7468e17..e018ecbf78a8 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2702,6 +2702,32 @@ void snd_hda_codec_set_power_to_all(struct hda_codec *codec, hda_nid_t fg,
}
EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
+/*
+ * wait until the state is reached, returns the current state
+ */
+static unsigned int hda_sync_power_state(struct hda_codec *codec,
+ hda_nid_t fg,
+ unsigned int power_state)
+{
+ unsigned long end_time = jiffies + msecs_to_jiffies(500);
+ unsigned int state, actual_state;
+
+ for (;;) {
+ state = snd_hda_codec_read(codec, fg, 0,
+ AC_VERB_GET_POWER_STATE, 0);
+ if (state & AC_PWRST_ERROR)
+ break;
+ actual_state = (state >> 4) & 0x0f;
+ if (actual_state == power_state)
+ break;
+ if (time_after_eq(jiffies, end_time))
+ break;
+ /* wait until the codec reachs to the target state */
+ msleep(1);
+ }
+ return state;
+}
+
/**
* snd_hda_codec_eapd_power_filter - A power filter callback for EAPD
* @codec: the HDA codec
@@ -2764,7 +2790,7 @@ static unsigned int hda_set_power_state(struct hda_codec *codec,
state);
snd_hda_codec_set_power_to_all(codec, fg, power_state);
}
- state = snd_hda_sync_power_state(codec, fg, power_state);
+ state = hda_sync_power_state(codec, fg, power_state);
if (!(state & AC_PWRST_ERROR))
break;
}
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 321e78baa63c..5b5c324c99b9 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -622,11 +622,7 @@ snd_hda_check_power_state(struct hda_codec *codec, hda_nid_t nid,
{
return snd_hdac_check_power_state(&codec->core, nid, target_state);
}
-static inline bool snd_hda_sync_power_state(struct hda_codec *codec,
- hda_nid_t nid, unsigned int target_state)
-{
- return snd_hdac_sync_power_state(&codec->core, nid, target_state);
-}
+
unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec *codec,
hda_nid_t nid,
unsigned int power_state);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-12 17:29 [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state" Chris Wilson
@ 2018-02-12 18:28 ` Patchwork
2018-02-12 22:19 ` ✓ Fi.CI.IGT: " Patchwork
2018-02-13 5:11 ` [PATCH] " Takashi Iwai
2 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-02-12 18:28 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
URL : https://patchwork.freedesktop.org/series/38097/
State : success
== Summary ==
Series 38097v1 AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
https://patchwork.freedesktop.org/api/1.0/series/38097/revisions/1/mbox/
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
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)
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:417s
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:495s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:288s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:482s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:488s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:469s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:455s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:564s
fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:574s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:417s
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:515s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:391s
fi-ilk-650 total:288 pass:227 dwarn:0 dfail:0 fail:1 skip:60 time:409s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:451s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:421s
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:497s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:502s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:593s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:429s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:505s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:525s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:484s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:480s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:418s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:431s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:397s
83878f486e686dd291ef3566a45103962d7617ed drm-tip: 2018y-02m-12d-17h-43m-07s UTC integration manifest
26049ad1632f AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7984/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-12 17:29 [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state" Chris Wilson
2018-02-12 18:28 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-02-12 22:19 ` Patchwork
2018-02-13 5:11 ` [PATCH] " Takashi Iwai
2 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-02-12 22:19 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
URL : https://patchwork.freedesktop.org/series/38097/
State : success
== Summary ==
Test pm_rpm:
Subgroup gem-execbuf:
fail -> PASS (shard-hsw)
Subgroup debugfs-read:
fail -> PASS (shard-hsw)
Subgroup modeset-lpsp:
fail -> SKIP (shard-hsw)
Subgroup universal-planes-dpms:
fail -> PASS (shard-hsw)
Subgroup universal-planes:
fail -> PASS (shard-hsw)
Subgroup gem-pread:
fail -> PASS (shard-hsw)
Subgroup modeset-non-lpsp:
fail -> PASS (shard-hsw)
Subgroup drm-resources-equal:
fail -> PASS (shard-hsw)
Subgroup modeset-non-lpsp-stress:
fail -> PASS (shard-hsw)
Subgroup gem-idle:
fail -> PASS (shard-hsw)
Subgroup reg-read-ioctl:
fail -> PASS (shard-hsw)
Subgroup modeset-lpsp-stress:
fail -> SKIP (shard-hsw)
Subgroup legacy-planes-dpms:
fail -> PASS (shard-hsw)
Subgroup system-suspend:
fail -> PASS (shard-hsw) fdo#103375 +1
Subgroup dpms-non-lpsp:
fail -> PASS (shard-hsw)
Subgroup pm-caching:
fail -> PASS (shard-hsw)
Subgroup gem-execbuf-stress-extra-wait:
fail -> PASS (shard-hsw)
Subgroup fences:
fail -> PASS (shard-hsw)
Subgroup gem-execbuf-stress:
fail -> PASS (shard-hsw)
Subgroup dpms-mode-unset-non-lpsp:
fail -> PASS (shard-hsw)
Subgroup dpms-lpsp:
fail -> SKIP (shard-hsw)
Subgroup modeset-stress-extra-wait:
fail -> PASS (shard-hsw)
Subgroup cursor-dpms:
fail -> PASS (shard-hsw)
Subgroup pm-tiling:
fail -> PASS (shard-hsw)
Subgroup gem-mmap-cpu:
fail -> PASS (shard-hsw)
skip -> PASS (shard-apl)
Subgroup system-suspend-modeset:
fail -> PASS (shard-hsw)
skip -> PASS (shard-apl)
Subgroup basic-pci-d3-state:
fail -> PASS (shard-hsw)
Subgroup gem-evict-pwrite:
fail -> PASS (shard-hsw)
Subgroup basic-rte:
fail -> PASS (shard-hsw)
Subgroup debugfs-forcewake-user:
fail -> PASS (shard-hsw)
Subgroup dpms-mode-unset-lpsp:
fail -> SKIP (shard-hsw)
Subgroup gem-mmap-gtt:
fail -> PASS (shard-hsw)
Subgroup legacy-planes:
fail -> PASS (shard-hsw)
Subgroup sysfs-read:
fail -> PASS (shard-hsw)
Subgroup cursor:
fail -> PASS (shard-hsw)
Subgroup fences-dpms:
fail -> PASS (shard-hsw)
Test kms_vblank:
Subgroup pipe-b-ts-continuation-modeset-rpm:
fail -> PASS (shard-hsw)
Subgroup pipe-c-ts-continuation-dpms-rpm:
fail -> PASS (shard-hsw)
Subgroup pipe-c-ts-continuation-modeset-rpm:
fail -> PASS (shard-hsw)
Subgroup pipe-b-wait-busy:
skip -> PASS (shard-apl)
Subgroup pipe-a-ts-continuation-dpms-rpm:
fail -> PASS (shard-hsw)
Subgroup pipe-b-ts-continuation-dpms-rpm:
fail -> PASS (shard-hsw)
Subgroup pipe-a-ts-continuation-modeset-rpm:
fail -> PASS (shard-hsw)
Test kms_plane_scaling:
Subgroup pipe-a-scaler-with-rotation:
skip -> PASS (shard-apl)
Test kms_color:
Subgroup pipe-c-ctm-max:
skip -> PASS (shard-apl)
Subgroup pipe-c-degamma:
skip -> PASS (shard-apl)
Subgroup pipe-b-ctm-green-to-red:
skip -> PASS (shard-apl)
Test pm_lpsp:
Subgroup screens-disabled:
fail -> PASS (shard-hsw) fdo#104941
Test gem_eio:
Subgroup in-flight-external:
pass -> FAIL (shard-hsw) fdo#104676
Test perf_pmu:
Subgroup rc6-runtime-pm:
skip -> PASS (shard-hsw) fdo#105010 +1
Test kms_mmio_vs_cs_flip:
Subgroup setcrtc_vs_cs_flip:
skip -> PASS (shard-apl)
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-crc-legacy:
skip -> PASS (shard-apl)
Subgroup cursora-vs-flipa-legacy:
skip -> PASS (shard-apl)
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-offscren-pri-indfb-draw-render:
skip -> PASS (shard-apl) fdo#103167 +1
Test kms_flip:
Subgroup blocking-absolute-wf_vblank:
skip -> PASS (shard-apl)
Subgroup wf_vblank-ts-check-interruptible:
skip -> PASS (shard-apl) fdo#100368
Test kms_rmfb:
Subgroup rmfb-ioctl:
skip -> PASS (shard-apl)
Test kms_rotation_crc:
Subgroup exhaust-fences:
skip -> PASS (shard-apl)
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#104941 https://bugs.freedesktop.org/show_bug.cgi?id=104941
fdo#104676 https://bugs.freedesktop.org/show_bug.cgi?id=104676
fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
shard-apl total:3410 pass:1767 dwarn:1 dfail:0 fail:22 skip:1619 time:13703s
shard-hsw total:3427 pass:1757 dwarn:1 dfail:0 fail:12 skip:1656 time:14680s
shard-snb total:3427 pass:1349 dwarn:1 dfail:0 fail:10 skip:2067 time:7606s
Blacklisted hosts:
shard-kbl total:3427 pass:1908 dwarn:1 dfail:0 fail:22 skip:1496 time:11060s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7984/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-12 17:29 [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state" Chris Wilson
2018-02-12 18:28 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-02-12 22:19 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-02-13 5:11 ` Takashi Iwai
2018-02-13 5:18 ` Kumar, Abhijeet
2018-02-13 7:34 ` Saarinen, Jani
2 siblings, 2 replies; 15+ messages in thread
From: Takashi Iwai @ 2018-02-13 5:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Abhijeet Kumar
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@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
Did the patch break anything?
I don't understand it without any real context...
thanks,
Takashi
> ---
> sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
> sound/pci/hda/hda_local.h | 6 +-----
> 2 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 5bc3a7468e17..e018ecbf78a8 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -2702,6 +2702,32 @@ void snd_hda_codec_set_power_to_all(struct hda_codec *codec, hda_nid_t fg,
> }
> EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
>
> +/*
> + * wait until the state is reached, returns the current state
> + */
> +static unsigned int hda_sync_power_state(struct hda_codec *codec,
> + hda_nid_t fg,
> + unsigned int power_state)
> +{
> + unsigned long end_time = jiffies + msecs_to_jiffies(500);
> + unsigned int state, actual_state;
> +
> + for (;;) {
> + state = snd_hda_codec_read(codec, fg, 0,
> + AC_VERB_GET_POWER_STATE, 0);
> + if (state & AC_PWRST_ERROR)
> + break;
> + actual_state = (state >> 4) & 0x0f;
> + if (actual_state == power_state)
> + break;
> + if (time_after_eq(jiffies, end_time))
> + break;
> + /* wait until the codec reachs to the target state */
> + msleep(1);
> + }
> + return state;
> +}
> +
> /**
> * snd_hda_codec_eapd_power_filter - A power filter callback for EAPD
> * @codec: the HDA codec
> @@ -2764,7 +2790,7 @@ static unsigned int hda_set_power_state(struct hda_codec *codec,
> state);
> snd_hda_codec_set_power_to_all(codec, fg, power_state);
> }
> - state = snd_hda_sync_power_state(codec, fg, power_state);
> + state = hda_sync_power_state(codec, fg, power_state);
> if (!(state & AC_PWRST_ERROR))
> break;
> }
> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> index 321e78baa63c..5b5c324c99b9 100644
> --- a/sound/pci/hda/hda_local.h
> +++ b/sound/pci/hda/hda_local.h
> @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct hda_codec *codec, hda_nid_t nid,
> {
> return snd_hdac_check_power_state(&codec->core, nid, target_state);
> }
> -static inline bool snd_hda_sync_power_state(struct hda_codec *codec,
> - hda_nid_t nid, unsigned int target_state)
> -{
> - return snd_hdac_sync_power_state(&codec->core, nid, target_state);
> -}
> +
> unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec *codec,
> hda_nid_t nid,
> unsigned int power_state);
> --
> 2.16.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 5:11 ` [PATCH] " Takashi Iwai
@ 2018-02-13 5:18 ` Kumar, Abhijeet
2018-02-13 7:35 ` Saarinen, Jani
2018-02-13 7:34 ` Saarinen, Jani
1 sibling, 1 reply; 15+ messages in thread
From: Kumar, Abhijeet @ 2018-02-13 5:18 UTC (permalink / raw)
To: Takashi Iwai, Chris Wilson; +Cc: intel-gfx
On 2/13/2018 10:41 AM, Takashi Iwai wrote:
> 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@intel.com>
>> Cc: Takashi Iwai <tiwai@suse.de>
> Did the patch break anything?
Not sure, why it would break anything. As there was literally no
functional change.
Chris,
Can you help us in understanding the issue caused by this patch ?
Warm Regards,
Abhijeet
> I don't understand it without any real context...
>
>
> thanks,
>
> Takashi
>
>> ---
>> sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
>> sound/pci/hda/hda_local.h | 6 +-----
>> 2 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
>> index 5bc3a7468e17..e018ecbf78a8 100644
>> --- a/sound/pci/hda/hda_codec.c
>> +++ b/sound/pci/hda/hda_codec.c
>> @@ -2702,6 +2702,32 @@ void snd_hda_codec_set_power_to_all(struct hda_codec *codec, hda_nid_t fg,
>> }
>> EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
>>
>> +/*
>> + * wait until the state is reached, returns the current state
>> + */
>> +static unsigned int hda_sync_power_state(struct hda_codec *codec,
>> + hda_nid_t fg,
>> + unsigned int power_state)
>> +{
>> + unsigned long end_time = jiffies + msecs_to_jiffies(500);
>> + unsigned int state, actual_state;
>> +
>> + for (;;) {
>> + state = snd_hda_codec_read(codec, fg, 0,
>> + AC_VERB_GET_POWER_STATE, 0);
>> + if (state & AC_PWRST_ERROR)
>> + break;
>> + actual_state = (state >> 4) & 0x0f;
>> + if (actual_state == power_state)
>> + break;
>> + if (time_after_eq(jiffies, end_time))
>> + break;
>> + /* wait until the codec reachs to the target state */
>> + msleep(1);
>> + }
>> + return state;
>> +}
>> +
>> /**
>> * snd_hda_codec_eapd_power_filter - A power filter callback for EAPD
>> * @codec: the HDA codec
>> @@ -2764,7 +2790,7 @@ static unsigned int hda_set_power_state(struct hda_codec *codec,
>> state);
>> snd_hda_codec_set_power_to_all(codec, fg, power_state);
>> }
>> - state = snd_hda_sync_power_state(codec, fg, power_state);
>> + state = hda_sync_power_state(codec, fg, power_state);
>> if (!(state & AC_PWRST_ERROR))
>> break;
>> }
>> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
>> index 321e78baa63c..5b5c324c99b9 100644
>> --- a/sound/pci/hda/hda_local.h
>> +++ b/sound/pci/hda/hda_local.h
>> @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct hda_codec *codec, hda_nid_t nid,
>> {
>> return snd_hdac_check_power_state(&codec->core, nid, target_state);
>> }
>> -static inline bool snd_hda_sync_power_state(struct hda_codec *codec,
>> - hda_nid_t nid, unsigned int target_state)
>> -{
>> - return snd_hdac_sync_power_state(&codec->core, nid, target_state);
>> -}
>> +
>> unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec *codec,
>> hda_nid_t nid,
>> unsigned int power_state);
>> --
>> 2.16.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 5:11 ` [PATCH] " Takashi Iwai
2018-02-13 5:18 ` Kumar, Abhijeet
@ 2018-02-13 7:34 ` Saarinen, Jani
2018-02-13 8:14 ` Takashi Iwai
1 sibling, 1 reply; 15+ messages in thread
From: Saarinen, Jani @ 2018-02-13 7:34 UTC (permalink / raw)
To: Takashi Iwai, Chris Wilson; +Cc: intel-gfx, Kumar, Abhijeet
HI,
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Takashi Iwai
> Sent: tiistai 13. helmikuuta 2018 7.11
> To: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> <abhijeet.kumar@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.kumar@intel.com>
> > Cc: Takashi Iwai <tiwai@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)
See also:
https://intel-gfx-ci.01.org/tree/drm-tip/igt@pm_rpm@basic-pci-d3-state.html
https://intel-gfx-ci.01.org/tree/drm-tip/igt@pm_rpm@basic-rte.html
That build when breakage started had these commits: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_3755/commits_short.log
>
>
> thanks,
>
> Takashi
Br,
Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
>
> > ---
> > sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
> > sound/pci/hda/hda_local.h | 6 +-----
> > 2 files changed, 28 insertions(+), 6 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 5bc3a7468e17..e018ecbf78a8 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -2702,6 +2702,32 @@ void snd_hda_codec_set_power_to_all(struct
> > hda_codec *codec, hda_nid_t fg, }
> > EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
> >
> > +/*
> > + * wait until the state is reached, returns the current state */
> > +static unsigned int hda_sync_power_state(struct hda_codec *codec,
> > + hda_nid_t fg,
> > + unsigned int power_state)
> > +{
> > + unsigned long end_time = jiffies + msecs_to_jiffies(500);
> > + unsigned int state, actual_state;
> > +
> > + for (;;) {
> > + state = snd_hda_codec_read(codec, fg, 0,
> > + AC_VERB_GET_POWER_STATE, 0);
> > + if (state & AC_PWRST_ERROR)
> > + break;
> > + actual_state = (state >> 4) & 0x0f;
> > + if (actual_state == power_state)
> > + break;
> > + if (time_after_eq(jiffies, end_time))
> > + break;
> > + /* wait until the codec reachs to the target state */
> > + msleep(1);
> > + }
> > + return state;
> > +}
> > +
> > /**
> > * snd_hda_codec_eapd_power_filter - A power filter callback for EAPD
> > * @codec: the HDA codec
> > @@ -2764,7 +2790,7 @@ static unsigned int hda_set_power_state(struct
> hda_codec *codec,
> > state);
> > snd_hda_codec_set_power_to_all(codec, fg,
> power_state);
> > }
> > - state = snd_hda_sync_power_state(codec, fg, power_state);
> > + state = hda_sync_power_state(codec, fg, power_state);
> > if (!(state & AC_PWRST_ERROR))
> > break;
> > }
> > diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> > index 321e78baa63c..5b5c324c99b9 100644
> > --- a/sound/pci/hda/hda_local.h
> > +++ b/sound/pci/hda/hda_local.h
> > @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct hda_codec
> > *codec, hda_nid_t nid, {
> > return snd_hdac_check_power_state(&codec->core, nid, target_state);
> > } -static inline bool snd_hda_sync_power_state(struct hda_codec
> > *codec,
> > - hda_nid_t nid, unsigned int target_state)
> > -{
> > - return snd_hdac_sync_power_state(&codec->core, nid, target_state);
> > -}
> > +
> > unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec *codec,
> > hda_nid_t nid,
> > unsigned int power_state);
> > --
> > 2.16.1
> >
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 5:18 ` Kumar, Abhijeet
@ 2018-02-13 7:35 ` Saarinen, Jani
2018-02-13 7:40 ` Lofstedt, Marta
0 siblings, 1 reply; 15+ messages in thread
From: Saarinen, Jani @ 2018-02-13 7:35 UTC (permalink / raw)
To: Kumar, Abhijeet, Takashi Iwai, Chris Wilson; +Cc: intel-gfx
Hi,
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Kumar, Abhijeet
> Sent: tiistai 13. helmikuuta 2018 7.18
> To: Takashi Iwai <tiwai@suse.de>; Chris Wilson <chris@chris-wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core
> codec functions to sync power state"
>
>
>
> On 2/13/2018 10:41 AM, Takashi Iwai wrote:
> > 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@intel.com>
> >> Cc: Takashi Iwai <tiwai@suse.de>
> > Did the patch break anything?
> Not sure, why it would break anything. As there was literally no functional
> change.
You can also doing so by yourself just looking at resutls from revert, but I also sent mail to explain.
>
> Chris,
> Can you help us in understanding the issue caused by this patch ?
>
> Warm Regards,
> Abhijeet
> > I don't understand it without any real context...
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> ---
> >> sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
> >> sound/pci/hda/hda_local.h | 6 +-----
> >> 2 files changed, 28 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> >> index 5bc3a7468e17..e018ecbf78a8 100644
> >> --- a/sound/pci/hda/hda_codec.c
> >> +++ b/sound/pci/hda/hda_codec.c
> >> @@ -2702,6 +2702,32 @@ void snd_hda_codec_set_power_to_all(struct
> hda_codec *codec, hda_nid_t fg,
> >> }
> >> EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
> >>
> >> +/*
> >> + * wait until the state is reached, returns the current state */
> >> +static unsigned int hda_sync_power_state(struct hda_codec *codec,
> >> + hda_nid_t fg,
> >> + unsigned int power_state)
> >> +{
> >> + unsigned long end_time = jiffies + msecs_to_jiffies(500);
> >> + unsigned int state, actual_state;
> >> +
> >> + for (;;) {
> >> + state = snd_hda_codec_read(codec, fg, 0,
> >> + AC_VERB_GET_POWER_STATE, 0);
> >> + if (state & AC_PWRST_ERROR)
> >> + break;
> >> + actual_state = (state >> 4) & 0x0f;
> >> + if (actual_state == power_state)
> >> + break;
> >> + if (time_after_eq(jiffies, end_time))
> >> + break;
> >> + /* wait until the codec reachs to the target state */
> >> + msleep(1);
> >> + }
> >> + return state;
> >> +}
> >> +
> >> /**
> >> * snd_hda_codec_eapd_power_filter - A power filter callback for EAPD
> >> * @codec: the HDA codec
> >> @@ -2764,7 +2790,7 @@ static unsigned int hda_set_power_state(struct
> hda_codec *codec,
> >> state);
> >> snd_hda_codec_set_power_to_all(codec, fg,
> power_state);
> >> }
> >> - state = snd_hda_sync_power_state(codec, fg, power_state);
> >> + state = hda_sync_power_state(codec, fg, power_state);
> >> if (!(state & AC_PWRST_ERROR))
> >> break;
> >> }
> >> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> >> index 321e78baa63c..5b5c324c99b9 100644
> >> --- a/sound/pci/hda/hda_local.h
> >> +++ b/sound/pci/hda/hda_local.h
> >> @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct hda_codec
> *codec, hda_nid_t nid,
> >> {
> >> return snd_hdac_check_power_state(&codec->core, nid, target_state);
> >> }
> >> -static inline bool snd_hda_sync_power_state(struct hda_codec *codec,
> >> - hda_nid_t nid, unsigned int target_state)
> >> -{
> >> - return snd_hdac_sync_power_state(&codec->core, nid, target_state);
> >> -}
> >> +
> >> unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec *codec,
> >> hda_nid_t nid,
> >> unsigned int power_state);
> >> --
> >> 2.16.1
> >>
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 7:35 ` Saarinen, Jani
@ 2018-02-13 7:40 ` Lofstedt, Marta
2018-02-13 8:06 ` Kumar, Abhijeet
0 siblings, 1 reply; 15+ messages in thread
From: Lofstedt, Marta @ 2018-02-13 7:40 UTC (permalink / raw)
To: Saarinen, Jani, Kumar, Abhijeet, Takashi Iwai, Chris Wilson; +Cc: intel-gfx
See bug: https://bugs.freedesktop.org/show_bug.cgi?id=105069
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Saarinen, Jani
> Sent: Tuesday, February 13, 2018 9:35 AM
> To: Kumar, Abhijeet <abhijeet.kumar@intel.com>; Takashi Iwai
> <tiwai@suse.de>; Chris Wilson <chris@chris-wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of
> core codec functions to sync power state"
>
> Hi,
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > Behalf Of Kumar, Abhijeet
> > Sent: tiistai 13. helmikuuta 2018 7.18
> > To: Takashi Iwai <tiwai@suse.de>; Chris Wilson
> > <chris@chris-wilson.co.uk>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use
> > of core codec functions to sync power state"
> >
> >
> >
> > On 2/13/2018 10:41 AM, Takashi Iwai wrote:
> > > 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@intel.com>
> > >> Cc: Takashi Iwai <tiwai@suse.de>
> > > Did the patch break anything?
> > Not sure, why it would break anything. As there was literally no
> > functional change.
> You can also doing so by yourself just looking at resutls from revert, but I also
> sent mail to explain.
>
> >
> > Chris,
> > Can you help us in understanding the issue caused by this patch ?
> >
> > Warm Regards,
> > Abhijeet
> > > I don't understand it without any real context...
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > >> ---
> > >> sound/pci/hda/hda_codec.c | 28 +++++++++++++++++++++++++++-
> > >> sound/pci/hda/hda_local.h | 6 +-----
> > >> 2 files changed, 28 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > >> index 5bc3a7468e17..e018ecbf78a8 100644
> > >> --- a/sound/pci/hda/hda_codec.c
> > >> +++ b/sound/pci/hda/hda_codec.c
> > >> @@ -2702,6 +2702,32 @@ void
> snd_hda_codec_set_power_to_all(struct
> > hda_codec *codec, hda_nid_t fg,
> > >> }
> > >> EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_to_all);
> > >>
> > >> +/*
> > >> + * wait until the state is reached, returns the current state */
> > >> +static unsigned int hda_sync_power_state(struct hda_codec *codec,
> > >> +
> hda_nid_t fg,
> > >> +
> unsigned int power_state)
> > >> +{
> > >> + unsigned long end_time = jiffies + msecs_to_jiffies(500);
> > >> + unsigned int state, actual_state;
> > >> +
> > >> + for (;;) {
> > >> + state = snd_hda_codec_read(codec, fg, 0,
> > >> +
> AC_VERB_GET_POWER_STATE, 0);
> > >> + if (state & AC_PWRST_ERROR)
> > >> + break;
> > >> + actual_state = (state >> 4) & 0x0f;
> > >> + if (actual_state == power_state)
> > >> + break;
> > >> + if (time_after_eq(jiffies, end_time))
> > >> + break;
> > >> + /* wait until the codec reachs to the target state
> */
> > >> + msleep(1);
> > >> + }
> > >> + return state;
> > >> +}
> > >> +
> > >> /**
> > >> * snd_hda_codec_eapd_power_filter - A power filter callback for
> EAPD
> > >> * @codec: the HDA codec
> > >> @@ -2764,7 +2790,7 @@ static unsigned int
> > >> hda_set_power_state(struct
> > hda_codec *codec,
> > >>
> state);
> > >>
> snd_hda_codec_set_power_to_all(codec, fg,
> > power_state);
> > >> }
> > >> - state = snd_hda_sync_power_state(codec, fg,
> power_state);
> > >> + state = hda_sync_power_state(codec, fg,
> power_state);
> > >> if (!(state & AC_PWRST_ERROR))
> > >> break;
> > >> }
> > >> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> > >> index 321e78baa63c..5b5c324c99b9 100644
> > >> --- a/sound/pci/hda/hda_local.h
> > >> +++ b/sound/pci/hda/hda_local.h
> > >> @@ -622,11 +622,7 @@ snd_hda_check_power_state(struct
> hda_codec
> > *codec, hda_nid_t nid,
> > >> {
> > >> return snd_hdac_check_power_state(&codec->core, nid,
> target_state);
> > >> }
> > >> -static inline bool snd_hda_sync_power_state(struct hda_codec
> *codec,
> > >> - hda_nid_t nid, unsigned int
> target_state)
> > >> -{
> > >> - return snd_hdac_sync_power_state(&codec->core, nid,
> target_state);
> > >> -}
> > >> +
> > >> unsigned int snd_hda_codec_eapd_power_filter(struct hda_codec
> *codec,
> > >>
> hda_nid_t nid,
> > >>
> unsigned int power_state);
> > >> --
> > >> 2.16.1
> > >>
> >
> > _______________________________________________
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 7:40 ` Lofstedt, Marta
@ 2018-02-13 8:06 ` Kumar, Abhijeet
0 siblings, 0 replies; 15+ messages in thread
From: Kumar, Abhijeet @ 2018-02-13 8:06 UTC (permalink / raw)
To: Lofstedt, Marta, Saarinen, Jani, Takashi Iwai, Chris Wilson; +Cc: intel-gfx
Thanks for sharing the results Marta and Jani.
I'll fix it and send the updated patch soon.
Warm Regards,
Abhijeet
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 7:34 ` Saarinen, Jani
@ 2018-02-13 8:14 ` Takashi Iwai
2018-02-13 8:26 ` Saarinen, Jani
2018-02-13 8:27 ` Kumar, Abhijeet
0 siblings, 2 replies; 15+ messages in thread
From: Takashi Iwai @ 2018-02-13 8:14 UTC (permalink / raw)
To: Saarinen, Jani; +Cc: intel-gfx, Kumar, Abhijeet
On Tue, 13 Feb 2018 08:34:40 +0100,
Saarinen, Jani wrote:
>
> HI,
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> > Takashi Iwai
> > Sent: tiistai 13. helmikuuta 2018 7.11
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> > <abhijeet.kumar@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.kumar@intel.com>
> > > Cc: Takashi Iwai <tiwai@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?
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 8:14 ` Takashi Iwai
@ 2018-02-13 8:26 ` Saarinen, Jani
2018-02-13 8:30 ` Takashi Iwai
2018-02-13 8:27 ` Kumar, Abhijeet
1 sibling, 1 reply; 15+ messages in thread
From: Saarinen, Jani @ 2018-02-13 8:26 UTC (permalink / raw)
To: Takashi Iwai; +Cc: intel-gfx, Kumar, Abhijeet
Hi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: tiistai 13. helmikuuta 2018 10.14
> To: Saarinen, Jani <jani.saarinen@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org;
> Kumar, Abhijeet <abhijeet.kumar@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-bounces@lists.freedesktop.org] On
> > > Behalf Of Takashi Iwai
> > > Sent: tiistai 13. helmikuuta 2018 7.11
> > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> > > <abhijeet.kumar@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.kumar@intel.com>
> > > > Cc: Takashi Iwai <tiwai@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 8:14 ` Takashi Iwai
2018-02-13 8:26 ` Saarinen, Jani
@ 2018-02-13 8:27 ` Kumar, Abhijeet
1 sibling, 0 replies; 15+ messages in thread
From: Kumar, Abhijeet @ 2018-02-13 8:27 UTC (permalink / raw)
To: Takashi Iwai, Saarinen, Jani; +Cc: intel-gfx
On 2/13/2018 1:44 PM, Takashi Iwai wrote:
> On Tue, 13 Feb 2018 08:34:40 +0100,
> Saarinen, Jani wrote:
>> HI,
>>> -----Original Message-----
>>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>> Takashi Iwai
>>> Sent: tiistai 13. helmikuuta 2018 7.11
>>> To: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
>>> <abhijeet.kumar@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.kumar@intel.com>
>>>> Cc: Takashi Iwai <tiwai@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?
> 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 (;;) {
I believe changing it back to finite should fix the regression.
> state = snd_hdac_codec_read(codec, nid, 0,
> AC_VERB_GET_POWER_STATE, 0);
> - if (state & AC_PWRST_ERROR) {
> - msleep(20);
When device raises the error state bit, we might endup sending two verbs
in back to
back manner.I would still recommend to have a 20ms sleep in-order to let
the device
clean up it's internal state.
> + 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 8:26 ` Saarinen, Jani
@ 2018-02-13 8:30 ` Takashi Iwai
2018-02-13 8:32 ` Kumar, Abhijeet
2018-02-13 8:37 ` Chris Wilson
0 siblings, 2 replies; 15+ messages in thread
From: Takashi Iwai @ 2018-02-13 8:30 UTC (permalink / raw)
To: Saarinen, Jani; +Cc: intel-gfx, Kumar, Abhijeet
On Tue, 13 Feb 2018 09:26:32 +0100,
Saarinen, Jani wrote:
>
> Hi,
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: tiistai 13. helmikuuta 2018 10.14
> > To: Saarinen, Jani <jani.saarinen@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org;
> > Kumar, Abhijeet <abhijeet.kumar@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-bounces@lists.freedesktop.org] On
> > > > Behalf Of Takashi Iwai
> > > > Sent: tiistai 13. helmikuuta 2018 7.11
> > > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> > > > <abhijeet.kumar@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.kumar@intel.com>
> > > > > Cc: Takashi Iwai <tiwai@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.
Once when a proper bug report is sent to us upstream :)
> > 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.
I guess Abhijeet can manage it better. It's Intel stuff, after all.
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 8:30 ` Takashi Iwai
@ 2018-02-13 8:32 ` Kumar, Abhijeet
2018-02-13 8:37 ` Chris Wilson
1 sibling, 0 replies; 15+ messages in thread
From: Kumar, Abhijeet @ 2018-02-13 8:32 UTC (permalink / raw)
To: Takashi Iwai, Saarinen, Jani; +Cc: intel-gfx
On 2/13/2018 2:00 PM, Takashi Iwai wrote:
> On Tue, 13 Feb 2018 09:26:32 +0100,
> Saarinen, Jani wrote:
>> Hi,
>>> -----Original Message-----
>>> From: Takashi Iwai [mailto:tiwai@suse.de]
>>> Sent: tiistai 13. helmikuuta 2018 10.14
>>> To: Saarinen, Jani <jani.saarinen@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org;
>>> Kumar, Abhijeet <abhijeet.kumar@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-bounces@lists.freedesktop.org] On
>>>>> Behalf Of Takashi Iwai
>>>>> Sent: tiistai 13. helmikuuta 2018 7.11
>>>>> To: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
>>>>> <abhijeet.kumar@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.kumar@intel.com>
>>>>>> Cc: Takashi Iwai <tiwai@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.
> Once when a proper bug report is sent to us upstream :)
>
>>> 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.
> I guess Abhijeet can manage it better. It's Intel stuff, after all.
I would make those changes and share it to ml.
>
> Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state"
2018-02-13 8:30 ` Takashi Iwai
2018-02-13 8:32 ` Kumar, Abhijeet
@ 2018-02-13 8:37 ` Chris Wilson
1 sibling, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2018-02-13 8:37 UTC (permalink / raw)
To: Takashi Iwai, Saarinen, Jani; +Cc: intel-gfx, Kumar, Abhijeet
Quoting Takashi Iwai (2018-02-13 08:30:52)
> On Tue, 13 Feb 2018 09:26:32 +0100,
> Saarinen, Jani wrote:
> >
> > Hi,
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: tiistai 13. helmikuuta 2018 10.14
> > > To: Saarinen, Jani <jani.saarinen@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org;
> > > Kumar, Abhijeet <abhijeet.kumar@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-bounces@lists.freedesktop.org] On
> > > > > Behalf Of Takashi Iwai
> > > > > Sent: tiistai 13. helmikuuta 2018 7.11
> > > > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet
> > > > > <abhijeet.kumar@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.kumar@intel.com>
> > > > > > Cc: Takashi Iwai <tiwai@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.
>
> Once when a proper bug report is sent to us upstream :)
Now there's a challenge for the dim masters, make dim fixes provide the
right link. As it was it only gave the authors' email, and I thought the
heads up was appropriate.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-02-13 8:37 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 17:29 [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state" Chris Wilson
2018-02-12 18:28 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-02-12 22:19 ` ✓ Fi.CI.IGT: " Patchwork
2018-02-13 5:11 ` [PATCH] " Takashi Iwai
2018-02-13 5:18 ` Kumar, Abhijeet
2018-02-13 7:35 ` Saarinen, Jani
2018-02-13 7:40 ` Lofstedt, Marta
2018-02-13 8:06 ` Kumar, Abhijeet
2018-02-13 7:34 ` Saarinen, Jani
2018-02-13 8:14 ` Takashi Iwai
2018-02-13 8:26 ` Saarinen, Jani
2018-02-13 8:30 ` Takashi Iwai
2018-02-13 8:32 ` Kumar, Abhijeet
2018-02-13 8:37 ` Chris Wilson
2018-02-13 8:27 ` Kumar, Abhijeet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.