[alsa-devel] New snd-hda warning spew
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() [ 18.809869] Modules linked in: ax88179_178a usbnet mii intel_powerclamp coretemp crct10dif_pclmul i915 snd_hda_codec_hdmi snd_hda_codec_generic crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm mei_me mei lpc_ich sdhci_pci sdhci mmc_core e1000e ptp pps_core [ 18.809903] CPU: 0 PID: 39 Comm: kworker/0:1 Not tainted 4.5.0-gfxbench+ #1 [ 18.809905] Hardware name: Hewlett-Packard HP EliteBook 8440p/172A, BIOS 68CCU Ver. F.24 09/13/2013 [ 18.809912] Workqueue: events azx_probe_work [snd_hda_intel] [ 18.809915] 0000000000000000 ffff8800b646b9e8 ffffffff813fef15 0000000000000000 [ 18.809920] ffffffffa00f277c ffff8800b646ba20 ffffffff81078a21 ffff8800b2436000 [ 18.809924] ffff8800b24755e8 ffff8800b24755f0 0000000000000100 ffff8800aed08330 [ 18.809929] Call Trace: [ 18.809936] [<ffffffff813fef15>] dump_stack+0x67/0x92 [ 18.809942] [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0 [ 18.809945] [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20 [ 18.809953] [<ffffffffa00f12d5>] pin2port+0x25/0x30 [snd_hda_core] [ 18.809960] [<ffffffffa00f1378>] snd_hdac_acomp_get_eld+0x38/0x70 [snd_hda_core] [ 18.809966] [<ffffffffa017da52>] hdmi_present_sense+0x92/0x370 [snd_hda_codec_hdmi] [ 18.809971] [<ffffffffa017e0ff>] generic_hdmi_build_controls+0x11f/0x200 [snd_hda_codec_hdmi] [ 18.809985] [<ffffffffa01105b1>] snd_hda_codec_build_controls+0x191/0x1d0 [snd_hda_codec] [ 18.809993] [<ffffffffa0110881>] ? snd_hda_codec_build_pcms+0xe1/0x1a0 [snd_hda_codec] [ 18.809999] [<ffffffffa010b442>] hda_codec_driver_probe+0x82/0x100 [snd_hda_codec] [ 18.810005] [<ffffffff8153d729>] driver_probe_device+0x229/0x450 [ 18.810008] [<ffffffff8153da7d>] __device_attach_driver+0x6d/0x80 [ 18.810011] [<ffffffff8153da10>] ? driver_allows_async_probing+0x30/0x30 [ 18.810015] [<ffffffff8153b4b8>] bus_for_each_drv+0x58/0x90 [ 18.810018] [<ffffffff8153d3c8>] __device_attach+0xb8/0x130 [ 18.810022] [<ffffffff8153dc0e>] device_initial_probe+0xe/0x10 [ 18.810025] [<ffffffff8153c76e>] bus_probe_device+0x9e/0xb0 [ 18.810028] [<ffffffff8153a5a1>] device_add+0x421/0x5f0 [ 18.810035] [<ffffffff817c2599>] ? mutex_unlock+0x9/0x10 [ 18.810041] [<ffffffffa00eba7f>] snd_hdac_device_register+0xf/0x40 [snd_hda_core] [ 18.810048] [<ffffffffa010b1f6>] snd_hda_codec_configure+0x36/0x180 [snd_hda_codec] [ 18.810057] [<ffffffffa01150c3>] azx_codec_configure+0x23/0x40 [snd_hda_codec] [ 18.810062] [<ffffffffa0134707>] azx_probe_work+0x567/0x870 [snd_hda_intel] [ 18.810067] [<ffffffff81093c8b>] process_one_work+0x1cb/0x680 [ 18.810070] [<ffffffff81093c06>] ? process_one_work+0x146/0x680 [ 18.810073] [<ffffffff81094189>] worker_thread+0x49/0x490 [ 18.810076] [<ffffffff81094140>] ? process_one_work+0x680/0x680 [ 18.810081] [<ffffffff8109a87a>] kthread+0xea/0x100 [ 18.810085] [<ffffffff817c5407>] ? _raw_spin_unlock_irq+0x27/0x50 [ 18.810090] [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0 [ 18.810093] [<ffffffff817c60bf>] ret_from_fork+0x3f/0x70 [ 18.810097] [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0 [ 18.810277] ---[ end trace 2d4983bcfed2cd4b ]---
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]() [ 124.808551] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp i915 crct10dif_pclmul snd_hda_intel crc32_pclmul snd_hda_codec ghash_clmulni_intel snd_hwdep snd_hda_core snd_pcm mei_me mei i2c_hid sdhci_pci e1000e ptp sdhci pps_core mmc_core [ 124.808603] CPU: 3 PID: 173 Comm: kworker/3:2 Tainted: G W 4.5.0-gfxbench+ #1 [ 124.808608] Hardware name: /NUC6i5SYB, BIOS SYSKLi35.86A.0028.2015.1112.1822 11/12/2015 [ 124.808616] Workqueue: pm pm_runtime_work [ 124.808639] 0000000000000000 ffff88026423fbb8 ffffffff813fef15 0000000000000000 [ 124.808644] ffffffffa00f877c ffff88026423fbf0 ffffffff81078a21 0000000000000000 [ 124.808648] ffff880085a11148 ffff880085586ab8 ffff8800861b8000 0000000000000000 [ 124.808652] Call Trace: [ 124.808656] [<ffffffff813fef15>] dump_stack+0x67/0x92 [ 124.808660] [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0 [ 124.808662] [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20 [ 124.808668] [<ffffffffa00f77e1>] snd_hdac_display_power+0xf1/0x110 [snd_hda_core] [ 124.808673] [<ffffffffa015039d>] azx_intel_link_power+0xd/0x10 [snd_hda_intel] [ 124.808680] [<ffffffffa011e32a>] azx_link_power+0x1a/0x30 [snd_hda_codec] [ 124.808684] [<ffffffffa00f21f9>] snd_hdac_link_power+0x29/0x40 [snd_hda_core] [ 124.808690] [<ffffffffa01192a6>] hda_codec_runtime_suspend+0x76/0xa0 [snd_hda_codec] [ 124.808697] [<ffffffffa0119230>] ? hda_codec_runtime_resume+0x50/0x50 [snd_hda_codec] [ 124.808701] [<ffffffff8154719d>] __rpm_callback+0x2d/0x70 [ 124.808704] [<ffffffff815471ff>] rpm_callback+0x1f/0x80 [ 124.808710] [<ffffffffa0119230>] ? hda_codec_runtime_resume+0x50/0x50 [snd_hda_codec] [ 124.808713] [<ffffffff81547764>] rpm_suspend+0x134/0x7f0 [ 124.808716] [<ffffffff810c80a9>] ? __lock_is_held+0x49/0x70 [ 124.808719] [<ffffffff81548129>] rpm_idle+0x209/0x530 [ 124.808722] [<ffffffff815494f9>] pm_runtime_work+0xa9/0xc0 [ 124.808725] [<ffffffff81093c8b>] process_one_work+0x1cb/0x680 [ 124.808728] [<ffffffff81093c06>] ? process_one_work+0x146/0x680 [ 124.808731] [<ffffffff81094189>] worker_thread+0x49/0x490 [ 124.808733] [<ffffffff81094140>] ? process_one_work+0x680/0x680 [ 124.808735] [<ffffffff81094140>] ? process_one_work+0x680/0x680 [ 124.808738] [<ffffffff8109a87a>] kthread+0xea/0x100 [ 124.808742] [<ffffffff817c53d7>] ? _raw_spin_unlock_irq+0x27/0x50 [ 124.808745] [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0 [ 124.808749] [<ffffffff817c607f>] ret_from_fork+0x3f/0x70 [ 124.808752] [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0 [ 124.808755] ---[ end trace 2b97e417eb08d213 ]---
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Limit i915 HDMI binding only for HSW and later MIME-Version: 1.0
It turned out that the pre-HSW Intel chips are incompatible with the naive assumption we had -- the fixed mapping between the port and the HD-audio widget. This may result in the bad access, as captured by the recent patch to add a WARN_ON() for the port mapping check.
As a quick workaround, disable the i915 audio component binding for all pre-Haswell models.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3fc259154c0b..cde9746cda8e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2243,9 +2243,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
- /* Try to bind with i915 for any Intel codecs (if not done yet) */ + /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if (!codec_has_acomp(codec) && - (codec->core.vendor_id >> 16) == 0x8086) + (codec->core.vendor_id >> 16) == 0x8086 && + is_haswell_plus(codec)) if (!snd_hdac_i915_init(&codec->bus->core)) spec->i915_bound = true;
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK. Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
[ 1939.476507] 3 locks held by kworker/2:2/1016: [ 1939.476509] #0: ("events"){.+.+.+}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476519] #1: ((&bus->unsol_work)){+.+...}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476525] #2: (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476532] stack backtrace: [ 1939.476537] CPU: 2 PID: 1016 Comm: kworker/2:2 Not tainted 4.5.0-vga+ #13 [ 1939.476539] Hardware name: Dell Inc. Latitude E5410/03VXMC, BIOS A15 07/11/2013 [ 1939.476547] Workqueue: events process_unsol_events [snd_hda_core] [ 1939.476550] 0000000000000000 ffff8800d63a3960 ffffffff812f2265 ffffffff82370e80 [ 1939.476554] ffffffff82370e80 ffff8800d63a3a28 ffffffff810c1dc5 00000000d63a3990 [ 1939.476559] ffff88000001a6f8 ffff8800d6087400 00009b11dc5f82fc ffff88000001adf8 [ 1939.476564] Call Trace: [ 1939.476569] [<ffffffff812f2265>] dump_stack+0x67/0x92 [ 1939.476573] [<ffffffff810c1dc5>] __lock_acquire+0x1c25/0x1c80 [ 1939.476578] [<ffffffff8154435e>] ? mutex_unlock+0xe/0x10 [ 1939.476586] [<ffffffffa01a2fa6>] ? codec_exec_verb+0xa6/0xf0 [snd_hda_codec] [ 1939.476590] [<ffffffff810c26f6>] lock_acquire+0xb6/0x210 [ 1939.476594] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476598] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476601] [<ffffffff81542bf4>] mutex_lock_nested+0x54/0x3b0 [ 1939.476605] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476608] [<ffffffff810bfe3f>] ? trace_hardirqs_on_caller+0x12f/0x1c0 [ 1939.476611] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476618] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476627] [<ffffffff810dc4e6>] ? rcu_read_lock_sched_held+0x86/0x90 [ 1939.476631] [<ffffffff813cef46>] ? regcache_sync+0x356/0x400 [ 1939.476638] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi] [ 1939.476649] [<ffffffffa01a55ea>] hda_call_codec_resume+0xba/0x110 [snd_hda_codec] [ 1939.476656] [<ffffffffa01a5675>] hda_codec_runtime_resume+0x35/0x50 [snd_hda_codec] [ 1939.476660] [<ffffffff813bc992>] __rpm_callback+0x32/0x70 [ 1939.476667] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476670] [<ffffffff813bc9f4>] rpm_callback+0x24/0x80 [ 1939.476677] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476681] [<ffffffff813be125>] rpm_resume+0x455/0x820 [ 1939.476684] [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi] [ 1939.476705] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476711] [<ffffffffa01a2077>] hda_codec_unsol_event+0x17/0x20 [snd_hda_codec] [ 1939.476716] [<ffffffffa017d178>] process_unsol_events+0x68/0x80 [snd_hda_core] [ 1939.476719] [<ffffffff81093d48>] process_one_work+0x1d8/0x6b0 [ 1939.476722] [<ffffffff81093cc4>] ? process_one_work+0x154/0x6b0 [ 1939.476725] [<ffffffff81094a4e>] worker_thread+0x4e/0x480 [ 1939.476728] [<ffffffff81094a00>] ? cancel_delayed_work_sync+0x20/0x20 [ 1939.476732] [<ffffffff8109a664>] kthread+0xe4/0x100 [ 1939.476736] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200 [ 1939.476740] [<ffffffff81546cbf>] ret_from_fork+0x3f/0x70 [ 1939.476743] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200
[2] [<ffffffff813bde90>] rpm_resume+0x1c0/0x820 [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [<ffffffffa017db83>] snd_hdac_power_up+0x13/0x20 [snd_hda_core] [<ffffffffa01abf18>] azx_pcm_open+0x1f8/0x470 [snd_hda_codec] [<ffffffffa00fe17d>] snd_pcm_open_substream+0x4d/0xa0 [snd_pcm] [<ffffffffa00fe27f>] snd_pcm_open+0xaf/0x220 [snd_pcm] [<ffffffffa00fe4a4>] snd_pcm_playback_open+0x44/0x70 [snd_pcm] [<ffffffffa009e453>] snd_open+0xb3/0x180 [snd] [<ffffffff811b90ff>] chrdev_open+0x9f/0x1c0 [<ffffffff811b1e4f>] do_dentry_open+0x1cf/0x2f0 [<ffffffff811b31db>] vfs_open+0x5b/0x60 [<ffffffff811c39a3>] path_openat+0x1d3/0x1530 [<ffffffff811c5dae>] do_filp_open+0x7e/0xe0 [<ffffffff811b3516>] do_sys_open+0x116/0x1f0 [<ffffffff811b360e>] SyS_open+0x1e/0x20 [<ffffffff81546957>] entry_SYSCALL_64_fastpath+0x12/0x6f [<ffffffffffffffff>] 0xffffffffffffffff
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Limit i915 HDMI binding only for HSW and later MIME-Version: 1.0
It turned out that the pre-HSW Intel chips are incompatible with the naive assumption we had -- the fixed mapping between the port and the HD-audio widget. This may result in the bad access, as captured by the recent patch to add a WARN_ON() for the port mapping check.
As a quick workaround, disable the i915 audio component binding for all pre-Haswell models.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3fc259154c0b..cde9746cda8e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2243,9 +2243,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
- /* Try to bind with i915 for any Intel codecs (if not done yet) */
- /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if (!codec_has_acomp(codec) &&
(codec->core.vendor_id >> 16) == 0x8086)
(codec->core.vendor_id >> 16) == 0x8086 &&is_haswell_plus(codec))-- 2.7.3
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK. Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
Gah, this must be the side-effect of the recent MST works. Good to catch it before merging to Linus tree.
Libin, could you take a look at this quickly?
thanks,
Takashi
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
[ 1939.476507] 3 locks held by kworker/2:2/1016: [ 1939.476509] #0: ("events"){.+.+.+}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476519] #1: ((&bus->unsol_work)){+.+...}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476525] #2: (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476532] stack backtrace: [ 1939.476537] CPU: 2 PID: 1016 Comm: kworker/2:2 Not tainted 4.5.0-vga+ #13 [ 1939.476539] Hardware name: Dell Inc. Latitude E5410/03VXMC, BIOS A15 07/11/2013 [ 1939.476547] Workqueue: events process_unsol_events [snd_hda_core] [ 1939.476550] 0000000000000000 ffff8800d63a3960 ffffffff812f2265 ffffffff82370e80 [ 1939.476554] ffffffff82370e80 ffff8800d63a3a28 ffffffff810c1dc5 00000000d63a3990 [ 1939.476559] ffff88000001a6f8 ffff8800d6087400 00009b11dc5f82fc ffff88000001adf8 [ 1939.476564] Call Trace: [ 1939.476569] [<ffffffff812f2265>] dump_stack+0x67/0x92 [ 1939.476573] [<ffffffff810c1dc5>] __lock_acquire+0x1c25/0x1c80 [ 1939.476578] [<ffffffff8154435e>] ? mutex_unlock+0xe/0x10 [ 1939.476586] [<ffffffffa01a2fa6>] ? codec_exec_verb+0xa6/0xf0 [snd_hda_codec] [ 1939.476590] [<ffffffff810c26f6>] lock_acquire+0xb6/0x210 [ 1939.476594] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476598] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476601] [<ffffffff81542bf4>] mutex_lock_nested+0x54/0x3b0 [ 1939.476605] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476608] [<ffffffff810bfe3f>] ? trace_hardirqs_on_caller+0x12f/0x1c0 [ 1939.476611] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476618] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476627] [<ffffffff810dc4e6>] ? rcu_read_lock_sched_held+0x86/0x90 [ 1939.476631] [<ffffffff813cef46>] ? regcache_sync+0x356/0x400 [ 1939.476638] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi] [ 1939.476649] [<ffffffffa01a55ea>] hda_call_codec_resume+0xba/0x110 [snd_hda_codec] [ 1939.476656] [<ffffffffa01a5675>] hda_codec_runtime_resume+0x35/0x50 [snd_hda_codec] [ 1939.476660] [<ffffffff813bc992>] __rpm_callback+0x32/0x70 [ 1939.476667] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476670] [<ffffffff813bc9f4>] rpm_callback+0x24/0x80 [ 1939.476677] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476681] [<ffffffff813be125>] rpm_resume+0x455/0x820 [ 1939.476684] [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi] [ 1939.476705] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476711] [<ffffffffa01a2077>] hda_codec_unsol_event+0x17/0x20 [snd_hda_codec] [ 1939.476716] [<ffffffffa017d178>] process_unsol_events+0x68/0x80 [snd_hda_core] [ 1939.476719] [<ffffffff81093d48>] process_one_work+0x1d8/0x6b0 [ 1939.476722] [<ffffffff81093cc4>] ? process_one_work+0x154/0x6b0 [ 1939.476725] [<ffffffff81094a4e>] worker_thread+0x4e/0x480 [ 1939.476728] [<ffffffff81094a00>] ? cancel_delayed_work_sync+0x20/0x20 [ 1939.476732] [<ffffffff8109a664>] kthread+0xe4/0x100 [ 1939.476736] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200 [ 1939.476740] [<ffffffff81546cbf>] ret_from_fork+0x3f/0x70 [ 1939.476743] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200
[2] [<ffffffff813bde90>] rpm_resume+0x1c0/0x820 [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [<ffffffffa017db83>] snd_hdac_power_up+0x13/0x20 [snd_hda_core] [<ffffffffa01abf18>] azx_pcm_open+0x1f8/0x470 [snd_hda_codec] [<ffffffffa00fe17d>] snd_pcm_open_substream+0x4d/0xa0 [snd_pcm] [<ffffffffa00fe27f>] snd_pcm_open+0xaf/0x220 [snd_pcm] [<ffffffffa00fe4a4>] snd_pcm_playback_open+0x44/0x70 [snd_pcm] [<ffffffffa009e453>] snd_open+0xb3/0x180 [snd] [<ffffffff811b90ff>] chrdev_open+0x9f/0x1c0 [<ffffffff811b1e4f>] do_dentry_open+0x1cf/0x2f0 [<ffffffff811b31db>] vfs_open+0x5b/0x60 [<ffffffff811c39a3>] path_openat+0x1d3/0x1530 [<ffffffff811c5dae>] do_filp_open+0x7e/0xe0 [<ffffffff811b3516>] do_sys_open+0x116/0x1f0 [<ffffffff811b360e>] SyS_open+0x1e/0x20 [<ffffffff81546957>] entry_SYSCALL_64_fastpath+0x12/0x6f [<ffffffffffffffff>] 0xffffffffffffffff
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Limit i915 HDMI binding only for HSW and later MIME-Version: 1.0
It turned out that the pre-HSW Intel chips are incompatible with the naive assumption we had -- the fixed mapping between the port and the HD-audio widget. This may result in the bad access, as captured by the recent patch to add a WARN_ON() for the port mapping check.
As a quick workaround, disable the i915 audio component binding for all pre-Haswell models.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3fc259154c0b..cde9746cda8e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2243,9 +2243,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
- /* Try to bind with i915 for any Intel codecs (if not done yet) */
- /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if (!codec_has_acomp(codec) &&
(codec->core.vendor_id >> 16) == 0x8086)
(codec->core.vendor_id >> 16) == 0x8086 &&is_haswell_plus(codec))-- 2.7.3
-- Ville Syrjälä Intel OTC
Hi Takashi,
On 03/16/2016 10:07 PM, Takashi Iwai wrote:
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK. Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
Gah, this must be the side-effect of the recent MST works. Good to catch it before merging to Linus tree.
Libin, could you take a look at this quickly?
Sure. I will work on it.
Regards, Libin
thanks,
Takashi
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
[ 1939.476507] 3 locks held by kworker/2:2/1016: [ 1939.476509] #0: ("events"){.+.+.+}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476519] #1: ((&bus->unsol_work)){+.+...}, at: [<ffffffff81093cc4>] process_one_work+0x154/0x6b0 [ 1939.476525] #2: (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476532] stack backtrace: [ 1939.476537] CPU: 2 PID: 1016 Comm: kworker/2:2 Not tainted 4.5.0-vga+ #13 [ 1939.476539] Hardware name: Dell Inc. Latitude E5410/03VXMC, BIOS A15 07/11/2013 [ 1939.476547] Workqueue: events process_unsol_events [snd_hda_core] [ 1939.476550] 0000000000000000 ffff8800d63a3960 ffffffff812f2265 ffffffff82370e80 [ 1939.476554] ffffffff82370e80 ffff8800d63a3a28 ffffffff810c1dc5 00000000d63a3990 [ 1939.476559] ffff88000001a6f8 ffff8800d6087400 00009b11dc5f82fc ffff88000001adf8 [ 1939.476564] Call Trace: [ 1939.476569] [<ffffffff812f2265>] dump_stack+0x67/0x92 [ 1939.476573] [<ffffffff810c1dc5>] __lock_acquire+0x1c25/0x1c80 [ 1939.476578] [<ffffffff8154435e>] ? mutex_unlock+0xe/0x10 [ 1939.476586] [<ffffffffa01a2fa6>] ? codec_exec_verb+0xa6/0xf0 [snd_hda_codec] [ 1939.476590] [<ffffffff810c26f6>] lock_acquire+0xb6/0x210 [ 1939.476594] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476598] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476601] [<ffffffff81542bf4>] mutex_lock_nested+0x54/0x3b0 [ 1939.476605] [<ffffffffa020b868>] ? hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476608] [<ffffffff810bfe3f>] ? trace_hardirqs_on_caller+0x12f/0x1c0 [ 1939.476611] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476618] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476627] [<ffffffff810dc4e6>] ? rcu_read_lock_sched_held+0x86/0x90 [ 1939.476631] [<ffffffff813cef46>] ? regcache_sync+0x356/0x400 [ 1939.476638] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi] [ 1939.476649] [<ffffffffa01a55ea>] hda_call_codec_resume+0xba/0x110 [snd_hda_codec] [ 1939.476656] [<ffffffffa01a5675>] hda_codec_runtime_resume+0x35/0x50 [snd_hda_codec] [ 1939.476660] [<ffffffff813bc992>] __rpm_callback+0x32/0x70 [ 1939.476667] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476670] [<ffffffff813bc9f4>] rpm_callback+0x24/0x80 [ 1939.476677] [<ffffffffa01a5640>] ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec] [ 1939.476681] [<ffffffff813be125>] rpm_resume+0x455/0x820 [ 1939.476684] [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi] [ 1939.476705] [<ffffffff810bd409>] ? __lock_is_held+0x49/0x70 [ 1939.476711] [<ffffffffa01a2077>] hda_codec_unsol_event+0x17/0x20 [snd_hda_codec] [ 1939.476716] [<ffffffffa017d178>] process_unsol_events+0x68/0x80 [snd_hda_core] [ 1939.476719] [<ffffffff81093d48>] process_one_work+0x1d8/0x6b0 [ 1939.476722] [<ffffffff81093cc4>] ? process_one_work+0x154/0x6b0 [ 1939.476725] [<ffffffff81094a4e>] worker_thread+0x4e/0x480 [ 1939.476728] [<ffffffff81094a00>] ? cancel_delayed_work_sync+0x20/0x20 [ 1939.476732] [<ffffffff8109a664>] kthread+0xe4/0x100 [ 1939.476736] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200 [ 1939.476740] [<ffffffff81546cbf>] ret_from_fork+0x3f/0x70 [ 1939.476743] [<ffffffff8109a580>] ? kthread_create_on_node+0x200/0x200
[2] [<ffffffff813bde90>] rpm_resume+0x1c0/0x820 [<ffffffff813be530>] __pm_runtime_resume+0x40/0x60 [<ffffffffa017db83>] snd_hdac_power_up+0x13/0x20 [snd_hda_core] [<ffffffffa01abf18>] azx_pcm_open+0x1f8/0x470 [snd_hda_codec] [<ffffffffa00fe17d>] snd_pcm_open_substream+0x4d/0xa0 [snd_pcm] [<ffffffffa00fe27f>] snd_pcm_open+0xaf/0x220 [snd_pcm] [<ffffffffa00fe4a4>] snd_pcm_playback_open+0x44/0x70 [snd_pcm] [<ffffffffa009e453>] snd_open+0xb3/0x180 [snd] [<ffffffff811b90ff>] chrdev_open+0x9f/0x1c0 [<ffffffff811b1e4f>] do_dentry_open+0x1cf/0x2f0 [<ffffffff811b31db>] vfs_open+0x5b/0x60 [<ffffffff811c39a3>] path_openat+0x1d3/0x1530 [<ffffffff811c5dae>] do_filp_open+0x7e/0xe0 [<ffffffff811b3516>] do_sys_open+0x116/0x1f0 [<ffffffff811b360e>] SyS_open+0x1e/0x20 [<ffffffff81546957>] entry_SYSCALL_64_fastpath+0x12/0x6f [<ffffffffffffffff>] 0xffffffffffffffff
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Limit i915 HDMI binding only for HSW and later MIME-Version: 1.0
It turned out that the pre-HSW Intel chips are incompatible with the naive assumption we had -- the fixed mapping between the port and the HD-audio widget. This may result in the bad access, as captured by the recent patch to add a WARN_ON() for the port mapping check.
As a quick workaround, disable the i915 audio component binding for all pre-Haswell models.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3fc259154c0b..cde9746cda8e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2243,9 +2243,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
- /* Try to bind with i915 for any Intel codecs (if not done yet) */
- /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if (!codec_has_acomp(codec) &&
(codec->core.vendor_id >> 16) == 0x8086)
(codec->core.vendor_id >> 16) == 0x8086 &&is_haswell_plus(codec))-- 2.7.3
-- Ville Syrjälä Intel OTC
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
Unfortunately, no this is a real deadlock. Let's see below: hdmi_present_sense() gets called twice because the function issues a verb that does self-resume and it invokes hdmi_present_sense() again in runtime resume.
[ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
....
[ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi]
....
[ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi]
This wasn't seen until now because the code path using i915 audio notifier doesn't need to power up the codec. Now we switched to the old method for old chips, and the bug is revealed. It's good to have caught it now, because basically this hits all non-Intel chips.
Takashi
On Thu, 17 Mar 2016 14:38:42 +0100, Takashi Iwai wrote:
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
Unfortunately, no this is a real deadlock. Let's see below: hdmi_present_sense() gets called twice because the function issues a verb that does self-resume and it invokes hdmi_present_sense() again in runtime resume.
[ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
....
[ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi]
....
[ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi]
This wasn't seen until now because the code path using i915 audio notifier doesn't need to power up the codec. Now we switched to the old method for old chips, and the bug is revealed. It's good to have caught it now, because basically this hits all non-Intel chips.
... and the fix patch is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix mutex deadlock at HDMI/DP hotplug
The recent change in HD-audio HDMI/DP codec driver for allowing the dynamic PCM binding introduced a new spec->pcm_mutex. One of the protected area by this mutex is hdmi_present_sense(). As reported by Intel CI tests, unfortunately, the new mutex causes a deadlock when the hotplug/unplug is triggered during the codec is in runtime suspend. The buggy code path is like the following:
hdmi_unsol_event() -> ... -> hdmi_present_sense() ==> ** here taking pcm_mutex -> hdmi_present_sense_via_verbs() -> snd_hda_power_up_pm() -> ... (runtime resume calls) -> generic_hdmi_resume() -> hdmi_present_sense() ==> ** here taking pcm_mutex again!
As we can see here, the problem is that the mutex is taken before snd_hda_power_up_pm() call that triggers the runtime resume. That is, the obvious solution is to move the power up/down call outside the mutex; it is exactly what this patch provides.
The patch also clarifies why this bug wasn't caught beforehand. We used to have the i915 audio component for hotplug for all Intel chips, and in that code path, there is no power up required but the information is taken directly from the graphics side. However, we recently switched back to the old method for some old Intel chips due to regressions, and now the deadlock issue is surfaced.
Fixes: a76056f2e57e ('ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug') Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index cde9746cda8e..49ee4e55dd16 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1405,7 +1405,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, bool ret; bool do_repoll = false;
- snd_hda_power_up_pm(codec); present = snd_hda_pin_sense(codec, pin_nid);
mutex_lock(&per_pin->lock); @@ -1444,7 +1443,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, jack->block_report = !ret;
mutex_unlock(&per_pin->lock); - snd_hda_power_down_pm(codec); return ret; }
@@ -1522,6 +1520,10 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) struct hdmi_spec *spec = codec->spec; int ret;
+ /* no temporary power up/down needed for component notifier */ + if (!codec_has_acomp(codec)) + snd_hda_power_up_pm(codec); + mutex_lock(&spec->pcm_lock); if (codec_has_acomp(codec)) { sync_eld_via_acomp(codec, per_pin); @@ -1531,6 +1533,9 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) } mutex_unlock(&spec->pcm_lock);
+ if (!codec_has_acomp(codec)) + snd_hda_power_down_pm(codec); + return ret; }
On Thu, Mar 17, 2016 at 03:12:22PM +0100, Takashi Iwai wrote:
On Thu, 17 Mar 2016 14:38:42 +0100, Takashi Iwai wrote:
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
Unfortunately, no this is a real deadlock. Let's see below: hdmi_present_sense() gets called twice because the function issues a verb that does self-resume and it invokes hdmi_present_sense() again in runtime resume.
[ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
....
[ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi]
....
[ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi]
This wasn't seen until now because the code path using i915 audio notifier doesn't need to power up the codec. Now we switched to the old method for old chips, and the bug is revealed. It's good to have caught it now, because basically this hits all non-Intel chips.
... and the fix patch is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix mutex deadlock at HDMI/DP hotplug
The recent change in HD-audio HDMI/DP codec driver for allowing the dynamic PCM binding introduced a new spec->pcm_mutex. One of the protected area by this mutex is hdmi_present_sense(). As reported by Intel CI tests, unfortunately, the new mutex causes a deadlock when the hotplug/unplug is triggered during the codec is in runtime suspend. The buggy code path is like the following:
hdmi_unsol_event() -> ... -> hdmi_present_sense() ==> ** here taking pcm_mutex -> hdmi_present_sense_via_verbs() -> snd_hda_power_up_pm() -> ... (runtime resume calls) -> generic_hdmi_resume() -> hdmi_present_sense() ==> ** here taking pcm_mutex again!
As we can see here, the problem is that the mutex is taken before snd_hda_power_up_pm() call that triggers the runtime resume. That is, the obvious solution is to move the power up/down call outside the mutex; it is exactly what this patch provides.
The patch also clarifies why this bug wasn't caught beforehand. We used to have the i915 audio component for hotplug for all Intel chips, and in that code path, there is no power up required but the information is taken directly from the graphics side. However, we recently switched back to the old method for some old Intel chips due to regressions, and now the deadlock issue is surfaced.
Fixes: a76056f2e57e ('ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug') Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Takashi Iwai tiwai@suse.de
Yep, my ILK seems happy with this. No deadlocks, and HDMI audio works.
Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
sound/pci/hda/patch_hdmi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index cde9746cda8e..49ee4e55dd16 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1405,7 +1405,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, bool ret; bool do_repoll = false;
snd_hda_power_up_pm(codec); present = snd_hda_pin_sense(codec, pin_nid);
mutex_lock(&per_pin->lock);
@@ -1444,7 +1443,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, jack->block_report = !ret;
mutex_unlock(&per_pin->lock);
- snd_hda_power_down_pm(codec); return ret;
}
@@ -1522,6 +1520,10 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) struct hdmi_spec *spec = codec->spec; int ret;
- /* no temporary power up/down needed for component notifier */
- if (!codec_has_acomp(codec))
snd_hda_power_up_pm(codec);- mutex_lock(&spec->pcm_lock); if (codec_has_acomp(codec)) { sync_eld_via_acomp(codec, per_pin);
@@ -1531,6 +1533,9 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) } mutex_unlock(&spec->pcm_lock);
- if (!codec_has_acomp(codec))
snd_hda_power_down_pm(codec);- return ret;
}
-- 2.7.3
On Thu, 17 Mar 2016 15:25:10 +0100, Ville Syrjälä wrote:
On Thu, Mar 17, 2016 at 03:12:22PM +0100, Takashi Iwai wrote:
On Thu, 17 Mar 2016 14:38:42 +0100, Takashi Iwai wrote:
On Wed, 16 Mar 2016 15:04:20 +0100, Ville Syrjälä wrote:
But now I got a lockdep spew when I enabled the HDMI video output [1]
And sure enough mplayer got stuck in the kernel when I tried to use the HDMI audio device [2]
[1] [ 1939.476458] ============================================= [ 1939.476460] [ INFO: possible recursive locking detected ] [ 1939.476463] 4.5.0-vga+ #13 Not tainted [ 1939.476464] --------------------------------------------- [ 1939.476466] kworker/2:2/1016 is trying to acquire lock: [ 1939.476469] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476480] but task is already holding lock: [ 1939.476482] (&spec->pcm_lock){+.+...}, at: [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi] [ 1939.476489] other info that might help us debug this: [ 1939.476491] Possible unsafe locking scenario:
[ 1939.476493] CPU0 [ 1939.476495] ---- [ 1939.476496] lock(&spec->pcm_lock); [ 1939.476499] lock(&spec->pcm_lock); [ 1939.476502] *** DEADLOCK ***
[ 1939.476504] May be due to missing lock nesting notation
Unfortunately, no this is a real deadlock. Let's see below: hdmi_present_sense() gets called twice because the function issues a verb that does self-resume and it invokes hdmi_present_sense() again in runtime resume.
[ 1939.476622] [<ffffffffa020b868>] hdmi_present_sense+0x38/0x300 [snd_hda_codec_hdmi]
....
[ 1939.476642] [<ffffffffa020bd6d>] generic_hdmi_resume+0x4d/0x60 [snd_hda_codec_hdmi]
....
[ 1939.476690] [<ffffffffa017de62>] snd_hdac_power_up_pm+0x52/0x60 [snd_hda_core] [ 1939.476694] [<ffffffffa020b9c3>] hdmi_present_sense+0x193/0x300 [snd_hda_codec_hdmi] [ 1939.476699] [<ffffffffa020bba0>] check_presence_and_report+0x70/0x90 [snd_hda_codec_hdmi] [ 1939.476703] [<ffffffffa020bcba>] hdmi_unsol_event+0x9a/0xb0 [snd_hda_codec_hdmi]
This wasn't seen until now because the code path using i915 audio notifier doesn't need to power up the codec. Now we switched to the old method for old chips, and the bug is revealed. It's good to have caught it now, because basically this hits all non-Intel chips.
... and the fix patch is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix mutex deadlock at HDMI/DP hotplug
The recent change in HD-audio HDMI/DP codec driver for allowing the dynamic PCM binding introduced a new spec->pcm_mutex. One of the protected area by this mutex is hdmi_present_sense(). As reported by Intel CI tests, unfortunately, the new mutex causes a deadlock when the hotplug/unplug is triggered during the codec is in runtime suspend. The buggy code path is like the following:
hdmi_unsol_event() -> ... -> hdmi_present_sense() ==> ** here taking pcm_mutex -> hdmi_present_sense_via_verbs() -> snd_hda_power_up_pm() -> ... (runtime resume calls) -> generic_hdmi_resume() -> hdmi_present_sense() ==> ** here taking pcm_mutex again!
As we can see here, the problem is that the mutex is taken before snd_hda_power_up_pm() call that triggers the runtime resume. That is, the obvious solution is to move the power up/down call outside the mutex; it is exactly what this patch provides.
The patch also clarifies why this bug wasn't caught beforehand. We used to have the i915 audio component for hotplug for all Intel chips, and in that code path, there is no power up required but the information is taken directly from the graphics side. However, we recently switched back to the old method for some old Intel chips due to regressions, and now the deadlock issue is surfaced.
Fixes: a76056f2e57e ('ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug') Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Takashi Iwai tiwai@suse.de
Yep, my ILK seems happy with this. No deadlocks, and HDMI audio works.
Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
Great, now queued with your tested-by tag. Thanks for quick testing!
Takashi
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
[ 14.978872] ------------[ cut here ]------------ [ 14.978890] WARNING: CPU: 1 PID: 288 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() [ 14.978894] Modules linked in: snd_hda_codec_hdmi(+) snd_hda_codec_realtek snd_hda_codec_generic i915 snd_hda_intel snd_hda_codec intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hwdep snd_hda_core snd_pcm lpc_ich i2c_hid i2c_designware_platform i2c_designware_core r8169 mii sdhci_acpi sdhci mmc_core [ 14.978934] CPU: 1 PID: 288 Comm: modprobe Not tainted 4.5.0-gfxbench+ #1 [ 14.978938] Hardware name: \xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff \xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff\xffffffff/DN2820FYK, BIOS FYBYT10H.86A.0038.2014.0717.1455 07/17/2014 [ 14.978941] 0000000000000000 ffff8800732d3a48 ffffffff813fef15 0000000000000000 [ 14.978948] ffffffffa014077c ffff8800732d3a80 ffffffff81078a21 ffff880073201b90 [ 14.978954] ffff880073006878 ffff880073006880 0000000000000100 ffff880073006878 [ 14.978961] Call Trace: [ 14.978969] [<ffffffff813fef15>] dump_stack+0x67/0x92 [ 14.978976] [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0 [ 14.978980] [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20 [ 14.978988] [<ffffffffa013f2d5>] pin2port+0x25/0x30 [snd_hda_core] [ 14.978997] [<ffffffffa013f378>] snd_hdac_acomp_get_eld+0x38/0x70 [snd_hda_core] [ 14.979009] [<ffffffffa0050a69>] hdmi_present_sense+0xa9/0x3a0 [snd_hda_codec_hdmi] [ 14.979016] [<ffffffffa005112f>] generic_hdmi_build_controls+0x11f/0x200 [snd_hda_codec_hdmi] [ 14.979029] [<ffffffffa018e5b1>] snd_hda_codec_build_controls+0x191/0x1d0 [snd_hda_codec] [ 14.979039] [<ffffffffa018e881>] ? snd_hda_codec_build_pcms+0xe1/0x1a0 [snd_hda_codec] [ 14.979049] [<ffffffffa0189442>] hda_codec_driver_probe+0x82/0x100 [snd_hda_codec] [ 14.979056] [<ffffffff8153d9b9>] driver_probe_device+0x229/0x450 [ 14.979061] [<ffffffff8153dc63>] __driver_attach+0x83/0x90 [ 14.979066] [<ffffffff8153dbe0>] ? driver_probe_device+0x450/0x450 [ 14.979070] [<ffffffff8153b691>] bus_for_each_dev+0x61/0xa0 [ 14.979074] [<ffffffff8153d2a9>] driver_attach+0x19/0x20 [ 14.979078] [<ffffffff8153cd8f>] bus_add_driver+0x1ef/0x290 [ 14.979083] [<ffffffffa0059000>] ? 0xffffffffa0059000 [ 14.979087] [<ffffffff8153e98b>] driver_register+0x5b/0xe0 [ 14.979096] [<ffffffffa01890d5>] __hda_codec_driver_register+0x55/0x60 [snd_hda_codec] [ 14.979103] [<ffffffffa005901e>] hdmi_driver_init+0x1e/0x20 [snd_hda_codec_hdmi] [ 14.979110] [<ffffffff810003de>] do_one_initcall+0xae/0x1d0 [ 14.979115] [<ffffffff810e1091>] ? rcu_read_lock_sched_held+0x81/0x90 [ 14.979121] [<ffffffff811b8863>] ? kmem_cache_alloc_trace+0x293/0x300 [ 14.979126] [<ffffffff8115b3dc>] ? do_init_module+0x22/0x1c6 [ 14.979131] [<ffffffff8115b415>] do_init_module+0x5b/0x1c6 [ 14.979136] [<ffffffff81107d3a>] load_module+0x1c0a/0x24b0 [ 14.979142] [<ffffffff81105410>] ? symbol_put_addr+0x60/0x60 [ 14.979147] [<ffffffff81105706>] ? copy_module_from_fd.isra.63+0xe6/0x140 [ 14.979152] [<ffffffff811087ce>] SyS_finit_module+0x7e/0xa0 [ 14.979160] [<ffffffff817c5f5b>] entry_SYSCALL_64_fastpath+0x16/0x73 [ 14.979271] ---[ end trace 639ed871547f2d0f ]---
<snip>
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Limit i915 HDMI binding only for HSW and later MIME-Version: 1.0
It turned out that the pre-HSW Intel chips are incompatible with the naive assumption we had -- the fixed mapping between the port and the HD-audio widget. This may result in the bad access, as captured by the recent patch to add a WARN_ON() for the port mapping check.
As a quick workaround, disable the i915 audio component binding for all pre-Haswell models.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3fc259154c0b..cde9746cda8e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2243,9 +2243,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
- /* Try to bind with i915 for any Intel codecs (if not done yet) */
- /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if (!codec_has_acomp(codec) &&
(codec->core.vendor_id >> 16) == 0x8086)
(codec->core.vendor_id >> 16) == 0x8086 &&is_haswell_plus(codec))-- 2.7.3
-- Ville Syrjälä Intel OTC
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Thanks for reporting!
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Really restrict i915 notifier to HSW+
The commit [b62232d429fa: ALSA: hda - Limit i915 HDMI binding only for HSW and later] tried to limit the usage of i915 audio notifier to the recent Intel models and switch to the old method on pre-Haswell models. However, it assumed that the i915 component binding hasn't been done on such models, and the assumption was wrong: namely, Baytrail had already the i915 component binding due to powerwell control. Thus, the workaround wasn't applied to Baytrail.
For fixing this properly, this patch introduces a new flag indicating the usage of audio notifier and codec_has_acomp() refers to this flag instead of checking the existence of audio component.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 49ee4e55dd16..8cdb804aa9cf 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -152,6 +152,7 @@ struct hdmi_spec { struct hda_pcm_stream pcm_playback;
/* i915/powerwell (Haswell+/Valleyview+) specific */ + bool use_acomp_notifier; /* use i915 eld_notify callback for hotplug */ struct i915_audio_component_audio_ops i915_audio_ops; bool i915_bound; /* was i915 bound in this driver? */
@@ -159,8 +160,11 @@ struct hdmi_spec { };
#ifdef CONFIG_SND_HDA_I915 -#define codec_has_acomp(codec) \ - ((codec)->bus->core.audio_component != NULL) +static inline bool codec_has_acomp(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + return spec->use_acomp_notifier; +} #else #define codec_has_acomp(codec) false #endif @@ -2248,12 +2252,18 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
+#ifdef CONFIG_SND_HDA_I915 /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ - if (!codec_has_acomp(codec) && - (codec->core.vendor_id >> 16) == 0x8086 && - is_haswell_plus(codec)) - if (!snd_hdac_i915_init(&codec->bus->core)) - spec->i915_bound = true; + if ((codec->core.vendor_id >> 16) == 0x8086 && + is_haswell_plus(codec)) { + if (!codec->bus->core.audio_component) + if (!snd_hdac_i915_init(&codec->bus->core)) + spec->i915_bound = true; + /* use i915 audio component notifier for hotplug */ + if (codec->bus->core.audio_component) + spec->use_acomp_notifier = true; + } +#endif
if (is_haswell_plus(codec)) { intel_haswell_enable_all_pins(codec, true);
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
Thanks for reporting!
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Really restrict i915 notifier to HSW+
The commit [b62232d429fa: ALSA: hda - Limit i915 HDMI binding only for HSW and later] tried to limit the usage of i915 audio notifier to the recent Intel models and switch to the old method on pre-Haswell models. However, it assumed that the i915 component binding hasn't been done on such models, and the assumption was wrong: namely, Baytrail had already the i915 component binding due to powerwell control. Thus, the workaround wasn't applied to Baytrail.
For fixing this properly, this patch introduces a new flag indicating the usage of audio notifier and codec_has_acomp() refers to this flag instead of checking the existence of audio component.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 49ee4e55dd16..8cdb804aa9cf 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -152,6 +152,7 @@ struct hdmi_spec { struct hda_pcm_stream pcm_playback;
/* i915/powerwell (Haswell+/Valleyview+) specific */
- bool use_acomp_notifier; /* use i915 eld_notify callback for hotplug */ struct i915_audio_component_audio_ops i915_audio_ops; bool i915_bound; /* was i915 bound in this driver? */
@@ -159,8 +160,11 @@ struct hdmi_spec { };
#ifdef CONFIG_SND_HDA_I915 -#define codec_has_acomp(codec) \
- ((codec)->bus->core.audio_component != NULL)
+static inline bool codec_has_acomp(struct hda_codec *codec) +{
- struct hdmi_spec *spec = codec->spec;
- return spec->use_acomp_notifier;
+} #else #define codec_has_acomp(codec) false #endif @@ -2248,12 +2252,18 @@ static int patch_generic_hdmi(struct hda_codec *codec) codec->spec = spec; hdmi_array_init(spec, 4);
+#ifdef CONFIG_SND_HDA_I915 /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */
- if (!codec_has_acomp(codec) &&
(codec->core.vendor_id >> 16) == 0x8086 &&is_haswell_plus(codec))if (!snd_hdac_i915_init(&codec->bus->core))spec->i915_bound = true;
- if ((codec->core.vendor_id >> 16) == 0x8086 &&
is_haswell_plus(codec)) {if (!codec->bus->core.audio_component)if (!snd_hdac_i915_init(&codec->bus->core))spec->i915_bound = true;/* use i915 audio component notifier for hotplug */if (codec->bus->core.audio_component)spec->use_acomp_notifier = true;- }
+#endif
if (is_haswell_plus(codec)) { intel_haswell_enable_all_pins(codec, true); -- 2.7.4
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
We have a few new WARN spews from snd-hda causing some grief in i915 CI.
This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: [ 18.809850] ------------[ cut here ]------------ [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */ - snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate); + if (codec_has_acomp(codec)) + snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); mutex_lock(&per_pin->lock);
On Fri, 18 Mar 2016 19:51:43 +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote: > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > [ 18.809850] ------------[ cut here ]------------ > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
BTW, while writing this patch, I noticed that i915/intel_audio.c doesn't have the check for Broxton.
Don't we need a patch like below? (Or maybe checking like gen >= 9 is better?)
Takashi
--- diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 31f6d212fb1b..8105691ff299 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -599,7 +599,7 @@ static void i915_audio_component_codec_wake_override(struct device *dev, struct drm_i915_private *dev_priv = dev_to_i915(dev); u32 tmp;
- if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv)) + if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROXTON(dev_priv)) return;
/* @@ -651,6 +651,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
/* HSW, BDW, SKL, KBL need this fix */ if (!IS_SKYLAKE(dev_priv) && + !IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROADWELL(dev_priv) && !IS_HASWELL(dev_priv))
On Fri, Mar 18, 2016 at 07:56:36PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 19:51:43 +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > On Tue, 15 Mar 2016 17:02:07 +0100, > Ville Syrjälä wrote: > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > [ 18.809850] ------------[ cut here ]------------ > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > This is bad. Basically we had a naive assumption of the fixed mapping > between the port number and the HD-audio widget, but it doesn't apply > properly to pre-HSW models. > > The patch attached below disables the audio binding for pre-HSW > models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
BTW, while writing this patch, I noticed that i915/intel_audio.c doesn't have the check for Broxton.
Don't we need a patch like below?
I have no clue.
(Or maybe checking like gen >= 9 is better?)
If BXT needs this stuff, a gen check would seem nicer.
Takashi
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 31f6d212fb1b..8105691ff299 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -599,7 +599,7 @@ static void i915_audio_component_codec_wake_override(struct device *dev, struct drm_i915_private *dev_priv = dev_to_i915(dev); u32 tmp;
- if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv))
if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROXTON(dev_priv)) return;
/*
@@ -651,6 +651,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
/* HSW, BDW, SKL, KBL need this fix */ if (!IS_SKYLAKE(dev_priv) &&
!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROADWELL(dev_priv) && !IS_HASWELL(dev_priv))
On Fri, 18 Mar 2016 20:07:53 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 07:56:36PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 19:51:43 +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote: > On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > > On Tue, 15 Mar 2016 17:02:07 +0100, > > Ville Syrjälä wrote: > > > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > > [ 18.809850] ------------[ cut here ]------------ > > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > > > This is bad. Basically we had a naive assumption of the fixed mapping > > between the port number and the HD-audio widget, but it doesn't apply > > properly to pre-HSW models. > > > > The patch attached below disables the audio binding for pre-HSW > > models. I'm going to queue to for-linus branch. > > That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
BTW, while writing this patch, I noticed that i915/intel_audio.c doesn't have the check for Broxton.
Don't we need a patch like below?
I have no clue.
(Or maybe checking like gen >= 9 is better?)
If BXT needs this stuff, a gen check would seem nicer.
OK, let's pitch to intel-gfx. Can anyone check whether the fix for BXT is needed?
thanks,
Takashi
Takashi
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 31f6d212fb1b..8105691ff299 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -599,7 +599,7 @@ static void i915_audio_component_codec_wake_override(struct device *dev, struct drm_i915_private *dev_priv = dev_to_i915(dev); u32 tmp;
- if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv))
if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROXTON(dev_priv)) return;
/*
@@ -651,6 +651,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
/* HSW, BDW, SKL, KBL need this fix */ if (!IS_SKYLAKE(dev_priv) &&
!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) && !IS_BROADWELL(dev_priv) && !IS_HASWELL(dev_priv))-- Ville Syrjälä Intel OTC
On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote: > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > [ 18.809850] ------------[ cut here ]------------ > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
This is bad. Basically we had a naive assumption of the fixed mapping between the port number and the HD-audio widget, but it doesn't apply properly to pre-HSW models.
The patch attached below disables the audio binding for pre-HSW models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */
- snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
- if (codec_has_acomp(codec))
snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
Yeah seems like it should do it.
Though I'm not entirely enthusiastic about maintaining two code paths for audio stuff. So longer term I'd love to be able rip out the hardware based ELD passing from i915 and just use the audio component even for the oldest platforms.
non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); mutex_lock(&per_pin->lock); -- 2.7.4
On Fri, 18 Mar 2016 20:10:58 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > On Tue, 15 Mar 2016 17:02:07 +0100, > Ville Syrjälä wrote: > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > [ 18.809850] ------------[ cut here ]------------ > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > This is bad. Basically we had a naive assumption of the fixed mapping > between the port number and the HD-audio widget, but it doesn't apply > properly to pre-HSW models. > > The patch attached below disables the audio binding for pre-HSW > models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */
- snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
- if (codec_has_acomp(codec))
snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);Yeah seems like it should do it.
Though I'm not entirely enthusiastic about maintaining two code paths for audio stuff. So longer term I'd love to be able rip out the hardware based ELD passing from i915 and just use the audio component even for the oldest platforms.
Yes, that was my "grand plan", but it failed miserably because of the uncertain mapping between the digital port and audio widget node. Once when this mapping is clarified, we can reenable the direct notification. I hope it can be done for 4.7.
Takashi
On Fri, Mar 18, 2016 at 08:18:10PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 20:10:58 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote: > On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > > On Tue, 15 Mar 2016 17:02:07 +0100, > > Ville Syrjälä wrote: > > > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > > [ 18.809850] ------------[ cut here ]------------ > > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > > > This is bad. Basically we had a naive assumption of the fixed mapping > > between the port number and the HD-audio widget, but it doesn't apply > > properly to pre-HSW models. > > > > The patch attached below disables the audio binding for pre-HSW > > models. I'm going to queue to for-linus branch. > > That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */
- snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
- if (codec_has_acomp(codec))
snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);Yeah seems like it should do it.
Though I'm not entirely enthusiastic about maintaining two code paths for audio stuff. So longer term I'd love to be able rip out the hardware based ELD passing from i915 and just use the audio component even for the oldest platforms.
Yes, that was my "grand plan", but it failed miserably because of the uncertain mapping between the digital port and audio widget node. Once when this mapping is clarified, we can reenable the direct notification. I hope it can be done for 4.7.
Cool. I'll keep my fingers crossed :)
On Fri, Mar 18, 2016 at 09:10:58PM +0200, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > On Tue, 15 Mar 2016 17:02:07 +0100, > Ville Syrjälä wrote: > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > [ 18.809850] ------------[ cut here ]------------ > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > This is bad. Basically we had a naive assumption of the fixed mapping > between the port number and the HD-audio widget, but it doesn't apply > properly to pre-HSW models. > > The patch attached below disables the audio binding for pre-HSW > models. I'm going to queue to for-linus branch.
That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */
- snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
- if (codec_has_acomp(codec))
snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);Yeah seems like it should do it.
Yep, didn't see any more backtraces in the logs with this one. Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
Though I'm not entirely enthusiastic about maintaining two code paths for audio stuff. So longer term I'd love to be able rip out the hardware based ELD passing from i915 and just use the audio component even for the oldest platforms.
non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); mutex_lock(&per_pin->lock); -- 2.7.4
-- Ville Syrjälä Intel OTC
On Fri, 18 Mar 2016 20:27:54 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 09:10:58PM +0200, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 18:49:19 +0100, Ville Syrjälä wrote:
On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
On Fri, 18 Mar 2016 14:54:59 +0100, Ville Syrjälä wrote:
On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote: > On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote: > > On Tue, 15 Mar 2016 17:02:07 +0100, > > Ville Syrjälä wrote: > > > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI. > > > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load: > > > [ 18.809850] ------------[ cut here ]------------ > > > [ 18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]() > > > > This is bad. Basically we had a naive assumption of the fixed mapping > > between the port number and the HD-audio widget, but it doesn't apply > > properly to pre-HSW models. > > > > The patch attached below disables the audio binding for pre-HSW > > models. I'm going to queue to for-linus branch. > > That seems to eliminate the warn on my ILK.
Apparently it was less effective on BYT. We still get this:
Ouch, I forgot that Baytrail had already i915 component binding in the controller side. I assumed too naively that all old models have no binding.
Below is the additional fix patch.
Still getting blasted at least via snd_hdac_sync_audio_rate()
That code path is a slightly different. This is a kind of false positive, but now the function checks the validity of the passed argument more strictly, and starts grumbling.
The fix is attached below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
snd_hdac_sync_audio_rate() call is mandatory only for HSW and later models, but we call the function unconditionally blindly assuming that the function doesn't do anything harmful. But since recently, the function checks the validity of the passed pin NID, and eventually spews the warning if an unexpected pin is passed. This is seen on old chips like Baytrail.
The fix is to limit the call of this function again only for the chips with the proper binding. This can be identified by the same flag as the eld notifier.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index e7d9453ecd10..56d3575ee6cc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */ /* Todo: add DP1.2 MST audio support later */
- snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
- if (codec_has_acomp(codec))
snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);Yeah seems like it should do it.
Yep, didn't see any more backtraces in the logs with this one. Tested-by: Ville Syrjälä ville.syrjala@linux.intel.com
Great, now queued to for-linus branch. Thanks!
Takashi
On Tue, 15 Mar 2016 18:22:56 +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
So, we got a full log in bugzilla, and this looks like the on-demand binding leading to the unbalanced refcount.
Could you try the patch below? This essentially reverts to the 4.4 state, so a "regression" should be covered, at least. It's applied on top of sound.git tree for-linus branch. (But it should be easily applicable to older trees, too.)
We need a proper fix later, but now more urgently the CI test regression has to be papered over.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Workaround for unbalanced i915 power refcount by concurrent probe
The recent addition of on-demand i915 audio component binding in the codec driver seems leading to the unbalanced i915 power refcount, according to Intel CI tests. Typically, it gets a kernel WARNING like: WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]() Call Trace: [<ffffffff813fef15>] dump_stack+0x67/0x92 [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0 [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20 [<ffffffffa00f77e1>] snd_hdac_display_power+0xf1/0x110 [snd_hda_core] [<ffffffffa015039d>] azx_intel_link_power+0xd/0x10 [snd_hda_intel] [<ffffffffa011e32a>] azx_link_power+0x1a/0x30 [snd_hda_codec] [<ffffffffa00f21f9>] snd_hdac_link_power+0x29/0x40 [snd_hda_core] [<ffffffffa01192a6>] hda_codec_runtime_suspend+0x76/0xa0 [snd_hda_codec] .....
The scenario is like below: - HD-audio driver and i915 driver are probed concurrently at the (almost) same time; HDA bus tries to bind with i915, but it fails because i915 initialization is still being processed. - Later on, HD-audio probes the HDMI codec, where it again tries to bind with i915. At this time, it succeeds. - At finishing the probe of HDA, it decreases the refcount as if it were already bound at the bus probe, since the component is bound now. This triggers a kernel WARNING due to the unbalance.
As a workaround, in this patch, we just disable the on-demand i915 component binding in the codec driver. This essentially reverts back to the state of 4.4 kernel.
We know that this is no real solution, but it's a minimalistic simple change that can be applied to 4.5.x kernel as stable.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94566 Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 56d3575ee6cc..7ae614d27954 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2258,9 +2258,15 @@ static int patch_generic_hdmi(struct hda_codec *codec) /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if ((codec->core.vendor_id >> 16) == 0x8086 && is_haswell_plus(codec)) { +#if 0 + /* on-demand binding leads to an unbalanced refcount when + * both i915 and hda drivers are probed concurrently; + * disabled temporarily for now + */ if (!codec->bus->core.audio_component) if (!snd_hdac_i915_init(&codec->bus->core)) spec->i915_bound = true; +#endif /* use i915 audio component notifier for hotplug */ if (codec->bus->core.audio_component) spec->use_acomp_notifier = true;
On la, 2016-03-19 at 11:18 +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 18:22:56 +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
So, we got a full log in bugzilla, and this looks like the on-demand binding leading to the unbalanced refcount.
Could you try the patch below? This essentially reverts to the 4.4 state, so a "regression" should be covered, at least. It's applied on top of sound.git tree for-linus branch. (But it should be easily applicable to older trees, too.)
We need a proper fix later, but now more urgently the CI test regression has to be papered over.
Looking more into this, it seems the root cause for this failure is that request_module returned pre-maturely (and with 0 return value) when i915 probing was still not finished. I tracked this issue down to the following kmod bug, fixed meanwhile upstream: http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=fd44a98ae2e...
Updating to the latest kmod fixed the issue. Things will of course work only in the general case where both the i915 and the sound driver are built as modules, to support other configurations we'd need to move the audio probing to happen at component bind time.
--Imre
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Workaround for unbalanced i915 power refcount by concurrent probe
The recent addition of on-demand i915 audio component binding in the codec driver seems leading to the unbalanced i915 power refcount, according to Intel CI tests. Typically, it gets a kernel WARNING like: WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]() Call Trace: [<ffffffff813fef15>] dump_stack+0x67/0x92 [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0 [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20 [<ffffffffa00f77e1>] snd_hdac_display_power+0xf1/0x110 [snd_hda_core] [<ffffffffa015039d>] azx_intel_link_power+0xd/0x10 [snd_hda_intel] [<ffffffffa011e32a>] azx_link_power+0x1a/0x30 [snd_hda_codec] [<ffffffffa00f21f9>] snd_hdac_link_power+0x29/0x40 [snd_hda_core] [<ffffffffa01192a6>] hda_codec_runtime_suspend+0x76/0xa0 [snd_hda_codec] .....
The scenario is like below:
- HD-audio driver and i915 driver are probed concurrently at the
(almost) same time; HDA bus tries to bind with i915, but it fails because i915 initialization is still being processed.
- Later on, HD-audio probes the HDMI codec, where it again tries to
bind with i915. At this time, it succeeds.
- At finishing the probe of HDA, it decreases the refcount as if it
were already bound at the bus probe, since the component is bound now. This triggers a kernel WARNING due to the unbalance.
As a workaround, in this patch, we just disable the on-demand i915 component binding in the codec driver. This essentially reverts back to the state of 4.4 kernel.
We know that this is no real solution, but it's a minimalistic simple change that can be applied to 4.5.x kernel as stable.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94566 Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # v4.5 Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 56d3575ee6cc..7ae614d27954 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2258,9 +2258,15 @@ static int patch_generic_hdmi(struct hda_codec *codec) /* Try to bind with i915 for Intel HSW+ codecs (if not done yet) */ if ((codec->core.vendor_id >> 16) == 0x8086 && is_haswell_plus(codec)) { +#if 0
/* on-demand binding leads to an unbalanced refcountwhen
* both i915 and hda drivers are probedconcurrently;
* disabled temporarily for now*/if (!codec->bus->core.audio_component) if (!snd_hdac_i915_init(&codec->bus->core)) spec->i915_bound = true; +#endif /* use i915 audio component notifier for hotplug */ if (codec->bus->core.audio_component) spec->use_acomp_notifier = true;
On Mon, 21 Mar 2016 13:34:35 +0100, Imre Deak wrote:
On la, 2016-03-19 at 11:18 +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 18:22:56 +0100, Takashi Iwai wrote:
On Tue, 15 Mar 2016 17:02:07 +0100, Ville Syrjälä wrote:
This other one was seen at least on on SKL: [ 124.808525] ------------[ cut here ]------------ [ 124.808545] WARNING: CPU: 3 PID: 173 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
This is a different one, and it implies that the unbalanced power refcount. Might be related with the recent fix for the recursive regmap deadlock. I'll try later with a SKL machine here, too.
Didn't you see this before the recent tree, right? Some good/bad commits would be really helpful...
So, we got a full log in bugzilla, and this looks like the on-demand binding leading to the unbalanced refcount.
Could you try the patch below? This essentially reverts to the 4.4 state, so a "regression" should be covered, at least. It's applied on top of sound.git tree for-linus branch. (But it should be easily applicable to older trees, too.)
We need a proper fix later, but now more urgently the CI test regression has to be papered over.
Looking more into this, it seems the root cause for this failure is that request_module returned pre-maturely (and with 0 return value) when i915 probing was still not finished. I tracked this issue down to the following kmod bug, fixed meanwhile upstream: http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=fd44a98ae2e...
Updating to the latest kmod fixed the issue. Things will of course work only in the general case where both the i915 and the sound driver are built as modules, to support other configurations we'd need to move the audio probing to happen at component bind time.
Ah, good to know that the culprit is not only us! :)
I queued my temporary fix in anyway since the on-demand binding isn't used any longer for 4.5/4.6. It'll be re-added once when we take back the support of eld notifier for old chips (which I'm currently working on). And then we'll be heading to the more stable component binding with your patches to add component stub.
thanks,
Takashi
participants (4)
-
Imre Deak -
Libin Yang -
Takashi Iwai -
Ville Syrjälä