Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751461AbdFZRy6 (ORCPT ); Mon, 26 Jun 2017 13:54:58 -0400 Received: from mail-it0-f66.google.com ([209.85.214.66]:33256 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751381AbdFZRyv (ORCPT ); Mon, 26 Jun 2017 13:54:51 -0400 MIME-Version: 1.0 X-Originating-IP: [2a02:168:5640:0:960b:2678:e223:c1c6] In-Reply-To: References: <1498057734-14622-1-git-send-email-daniel.vetter@ffwll.ch> <149805863799.15021.15278760577807511996@mail.alporthouse.com> <20170626161630.u4ubdzgxr37o63ip@phenom.ffwll.local> From: Daniel Vetter Date: Mon, 26 Jun 2017 19:54:49 +0200 X-Google-Sender-Auth: pQN-8iTjcGwRDSoJWhIg86qwYqc Message-ID: Subject: Re: [PATCH] BUG-REPORT: snd-hda: hacked-together EPROBE_DEFER support To: Takashi Iwai Cc: Chris Wilson , Intel Graphics Development , Guneshwor Singh , Hardik T Shah , Libin Yang , "Subhransu S. Prusty" , Vinod Koul , Julia Lawall , Jaroslav Kysela , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4169 Lines: 95 On Mon, Jun 26, 2017 at 7:47 PM, Takashi Iwai wrote: > On Mon, 26 Jun 2017 18:16:30 +0200, > Daniel Vetter wrote: >> >> On Wed, Jun 21, 2017 at 05:30:10PM +0200, Takashi Iwai wrote: >> > On Wed, 21 Jun 2017 17:23:57 +0200, >> > Chris Wilson wrote: >> > > >> > > Quoting Daniel Vetter (2017-06-21 16:08:54) >> > > > So back when the i915 power well support landed in >> > > > >> > > > commit 99a2008d0b32d72dfc2a54e7be1eb698dd2e3bd6 >> > > > Author: Wang Xingchao >> > > > Date: Thu May 30 22:07:10 2013 +0800 >> > > > >> > > > ALSA: hda - Add power-welll support for haswell HDA >> > > > >> > > > the logic to handle the cross-module depencies was hand-rolled using a >> > > > async work item, and that just doesn't work. >> > > > >> > > > The correct way to handle cross-module deps is either: >> > > > - request_module + failing when the other module isn't there >> > > > >> > > > OR >> > > > >> > > > - failing the module load with EPROBE_DEFER. >> > > > >> > > > You can't mix them, if you do then the entire load path just >> > > > busy-spins blowing through cpu cycles forever with no way to stop >> > > > this. >> > > > >> > > > snd-hda-intel does mix it, because the hda codec drivers are loaded >> > > > using request_module, but the i915 depency is handled using >> > > > PROBE_DEFER (or well, should be, but I haven't found any code at all). >> > > > This is a major pain when trying to debug i915 load failures. >> > > > >> > > > This patch here is a horrible hackish attempt at somewhat correctly >> > > > wriing EPROBE_DEFER through. Stuff that's missing: >> > > > - Check all the other places where load errors are conveniently >> > > > dropped on the floor. >> > > > - Also fix up the firmware_cb path. >> > > > - Drop the debug noise I've left in to make it clear this isn't >> > > > anything for merging. >> > > >> > > This tames "hdaudio hdaudioC0D0: Unable to bind the codec" which was >> > > continuously spewing previously, and now the system is usable again. >> > >> > Could you give a failing scenario? I'm not opposing to the suggested >> > solution, we need to fix the mess in anyway, but I just would like to >> > know how to trigger the problem easily. >> >> Disable i915 loading e.g. with i915.modeset=0. Watch how snd-hda* >> collective blow through 100% of the cpu time spewing into dmesg (and make >> the system completely unuseable for kernel work because you can't find >> your own debug printk anymore). > > Ah, that's the case we discussed in the past. We know that it's > problematic for component binding, but we're ignoring this scenario > because it's supposed to be no real use-case but only for some > temporary workarounds. > > We had some bigger-hammer patchset, but it didn't justify for the > further development of the reasoning above. > >> This is on a snb, where we don't even need the cross-module stuff ... But >> I think it goes sideways in other cases too, if you simply build but don't >> load i915. So every time an i915 breaks module load things become real >> painful. > > Even on SNB, we still need i915 for the HDMI/DP ELD notification. The > hardware inquiry over HD-audio verb was so unstable, so we rather take > a path directly inquiring to the gfx driver. Ah right, forgot about that. >> Unfortunately the patch is a bit too big for our fixup branch in drm-tip, >> so plan B would be to stop building snd-hda (which will make the intel >> audio team unhappy, but mea culpa if they don't fix this mess). > > OK, let me think and take a look for older patchset, too. Yeah would be great if we can somehow address this, preferrably using EPROBE_DEFER or something else that's standard. At least the component stuff really doesn't work without wiring EPROBE_DEFER through. And if that patch series requires some soaking I think I could easily add it to our drm-tip CI branch for testing (and making our developers lifes easier), we already pull in your -next/-fixes trees anyway. Pulling in another topic branch would be simple. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch