Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757473Ab3G3GHS (ORCPT ); Tue, 30 Jul 2013 02:07:18 -0400 Received: from cantor2.suse.de ([195.135.220.15]:40138 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752230Ab3G3GHR (ORCPT ); Tue, 30 Jul 2013 02:07:17 -0400 Date: Tue, 30 Jul 2013 08:08:42 +0200 Message-ID: From: Takashi Iwai To: Dave Jones Cc: Linux Kernel Mailing List , xingchao.wang@linux.intel.com, liam.r.girdwood@intel.com, david.henningsson@canonical.com, daniel.vetter@ffwll.ch, Linus Torvalds Subject: Re: ALSA: hda - Add power-welll support for haswell HDA In-Reply-To: <20130729201208.GA21037@redhat.com> References: <20130709233626.3E8F4660AF6@gitolite.kernel.org> <20130729201208.GA21037@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2900 Lines: 69 At Mon, 29 Jul 2013 16:12:08 -0400, Dave Jones wrote: > > On Tue, Jul 09, 2013 at 11:36:26PM +0000, Linux Kernel wrote: > > Gitweb: http://git.kernel.org/linus/;a=commit;h=99a2008d0b32d72dfc2a54e7be1eb698dd2e3bd6 > > Commit: 99a2008d0b32d72dfc2a54e7be1eb698dd2e3bd6 > > Parent: 5c90680e42b08a1e4a6800ca02e75ad201f8037f > > Author: Wang Xingchao > > AuthorDate: Thu May 30 22:07:10 2013 +0800 > > Committer: Daniel Vetter > > CommitDate: Thu Jun 6 17:31:56 2013 +0200 > > > > ALSA: hda - Add power-welll support for haswell HDA > > > > For Intel Haswell chip, HDA controller and codec have > > power well dependency from GPU side. This patch added support > > to request/release power well in audio driver. Power save > > feature should be enabled to get runtime power saving. > > > > There's deadlock when request_module(i915) in azx_probe. > > It looks like: > > device_lock(audio pci device) -> azx_probe -> module_request > > (or symbol_request) -> modprobe (userspace) -> i915 init -> > > drm_pci_init -> pci_register_driver -> bus_add_driver -> driver_attach -> > > which in turn tries all locks on pci bus, and when it tries the one on the > > audio device, it will deadlock. > > > > This patch introduce a work to store remaining probe stuff, and let > > request_module run in safe work context. > > That deadlock sounds nasty, but.. > > > +#ifdef CONFIG_SND_HDA_I915 > > + probe_now = false; > > + schedule_work(&chip->probe_work); > > +#else > > + snd_printk(KERN_ERR SFX "Haswell must build in CONFIG_SND_HDA_I915\n"); > > +#endif > > runtime surprises like this are also a pain. There has to be a way in Kconfig > to express this (through addition of a new option). Well, sound/pci/hda/Kconfig gives a warning: Note that this option must be enabled for Intel Haswell C+ stepping machines, otherwise the GPU audio controller/codecs will not be initialized or damaged when exit from S3 mode. I guess "damaged" is a bit too strong word, though. So the driver still continues with just an error message. It can be changed to stop loading and return the error, too, if such behavior is preferred. > Though having to build this > in for distro kernels is also unfortunate. This means we'll also have to build-in > all the ALSA stuff even for users who never use it. We discussed about it, too, and that's the reason why the driver tries to bind dynamically with symbol_request(). The glue code is fairly small (found in hda_i915.c). thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/