From patchwork Wed Jun 28 14:25:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 9814411 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C01F660383 for ; Wed, 28 Jun 2017 14:33:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C5A9927FAD for ; Wed, 28 Jun 2017 14:33:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BA55C28517; Wed, 28 Jun 2017 14:33:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9CFD27FAD for ; Wed, 28 Jun 2017 14:33:12 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id C5AA6267176; Wed, 28 Jun 2017 16:25:42 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 90CF7267179; Wed, 28 Jun 2017 16:25:41 +0200 (CEST) Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 25056266BC2 for ; Wed, 28 Jun 2017 16:25:39 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id E5B13AC60 for ; Wed, 28 Jun 2017 14:25:38 +0000 (UTC) Date: Wed, 28 Jun 2017 16:25:38 +0200 Message-ID: From: Takashi Iwai To: alsa-devel@alsa-project.org In-Reply-To: <20170628125854.13022-4-tiwai@suse.de> References: <20170628125854.13022-1-tiwai@suse.de> <20170628125854.13022-4-tiwai@suse.de> 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/25.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - Bind with i915 component before codec binding X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 28 Jun 2017 14:58:53 +0200, Takashi Iwai wrote: > > We used a on-demand i915 component binding for IvyBridge and > SandyBridge HDMI codecs, but it has a potential problem of the nested > module loading. For avoiding that situation, assure the i915 binding > happening at the controller driver level for PCH controller devices, > where the initialization is performed in a detached work, instead of > calling from the codec driver probe. > > Signed-off-by: Takashi Iwai > --- > sound/pci/hda/hda_intel.c | 4 ++-- > sound/pci/hda/patch_hdmi.c | 17 +++++------------ > 2 files changed, 7 insertions(+), 14 deletions(-) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index 01eb1dc7b5b3..2db3203090ed 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -294,11 +294,11 @@ enum { > > /* PCH up to IVB; no runtime PM */ > #define AZX_DCAPS_INTEL_PCH_NOPM \ > - (AZX_DCAPS_INTEL_PCH_BASE) > + (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_I915_POWERWELL) > > /* PCH for HSW/BDW; with runtime PM */ > #define AZX_DCAPS_INTEL_PCH \ > - (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME) > + (AZX_DCAPS_INTEL_PCH_NOPM | AZX_DCAPS_PM_RUNTIME) When I read back the code again, this doesn't look like a good solution. The bit flag is referred as the capability for the display power toggle, and old PCH doesn't need / have it. The revised patch is below. Takashi -- 8< -- From: Takashi Iwai Subject: [PATCH v2 3/4] ALSA: hda - Bind with i915 component before codec binding We used a on-demand i915 component binding for IvyBridge and SandyBridge HDMI codecs, but it has a potential problem of the nested module loading. For avoiding that situation, assure the i915 binding happening at the controller driver level for PCH controller devices, where the initialization is performed in a detached work, instead of calling from the codec driver probe. Signed-off-by: Takashi Iwai --- v1->v2: Check AZX_DRIVER_PCH instead of hacking AZX_DCAPS_I915_POWERWELL bit flag sound/pci/hda/hda_intel.c | 35 +++++++++++++++++++++-------------- sound/pci/hda/patch_hdmi.c | 17 +++++------------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 01eb1dc7b5b3..433a2df9edad 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1384,8 +1384,10 @@ static int azx_free(struct azx *chip) if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { if (hda->need_i915_power) snd_hdac_display_power(bus, false); - snd_hdac_i915_exit(bus); } + if (chip->driver_type == AZX_DRIVER_PCH || + (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)) + snd_hdac_i915_exit(bus); kfree(hda); return 0; @@ -2201,16 +2203,9 @@ static int azx_probe_continue(struct azx *chip) hda->probe_continued = 1; - /* Request display power well for the HDA controller or codec. For - * Haswell/Broadwell, both the display HDA controller and codec need - * this power. For other platforms, like Baytrail/Braswell, only the - * display codec needs the power and it can be released after probe. - */ - if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { - /* HSW/BDW controllers need this power */ - if (CONTROLLER_IN_GPU(pci)) - hda->need_i915_power = 1; - + /* bind with i915 if needed */ + if (chip->driver_type == AZX_DRIVER_PCH || + (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)) { err = snd_hdac_i915_init(bus); if (err < 0) { /* if the controller is bound only with HDMI/DP @@ -2222,9 +2217,22 @@ static int azx_probe_continue(struct azx *chip) dev_err(chip->card->dev, "HSW/BDW HD-audio HDMI/DP requires binding with gfx driver\n"); goto out_free; - } else - goto skip_i915; + } else { + /* don't bother any longer */ + chip->driver_caps &= ~AZX_DCAPS_I915_POWERWELL; + } } + } + + /* Request display power well for the HDA controller or codec. For + * Haswell/Broadwell, both the display HDA controller and codec need + * this power. For other platforms, like Baytrail/Braswell, only the + * display codec needs the power and it can be released after probe. + */ + if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) { + /* HSW/BDW controllers need this power */ + if (CONTROLLER_IN_GPU(pci)) + hda->need_i915_power = 1; err = snd_hdac_display_power(bus, true); if (err < 0) { @@ -2234,7 +2242,6 @@ static int azx_probe_continue(struct azx *chip) } } - skip_i915: err = azx_first_init(chip); if (err < 0) goto out_free; diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 90e4ff87445e..feed8e8de2af 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -174,7 +174,6 @@ struct hdmi_spec { /* 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? */ struct hdac_chmap chmap; hda_nid_t vendor_nid; @@ -2234,8 +2233,6 @@ static void generic_spec_free(struct hda_codec *codec) struct hdmi_spec *spec = codec->spec; if (spec) { - if (spec->i915_bound) - snd_hdac_i915_exit(&codec->bus->core); hdmi_array_free(spec); kfree(spec); codec->spec = NULL; @@ -2607,21 +2604,17 @@ static int patch_i915_cpt_hdmi(struct hda_codec *codec) struct hdmi_spec *spec; int err; - /* no i915 component should have been bound before this */ - if (WARN_ON(codec->bus->core.audio_component)) - return -EBUSY; + /* requires i915 binding */ + if (!codec->bus->core.audio_component) { + codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n"); + return -ENODEV; + } err = alloc_generic_hdmi(codec); if (err < 0) return err; spec = codec->spec; - /* Try to bind with i915 now */ - err = snd_hdac_i915_init(&codec->bus->core); - if (err < 0) - goto error; - spec->i915_bound = true; - err = hdmi_parse_codec(codec); if (err < 0) goto error;