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.

Signed-off-by: Wang Xingchao <xingchao.w...@linux.intel.com>
Reviewed-by: Takashi Iwai <ti...@suse.de>
Reviewed-by: Liam Girdwood <liam.r.girdw...@intel.com>
Reviewed-by: David Henningsson <david.hennings...@canonical.com>
---
 sound/pci/hda/Kconfig     |   10 ++++++
 sound/pci/hda/Makefile    |    2 ++
 sound/pci/hda/hda_i915.c  |   75 +++++++++++++++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_i915.h  |   35 +++++++++++++++++++++
 sound/pci/hda/hda_intel.c |   60 ++++++++++++++++++++++++++++++++++--
 5 files changed, 179 insertions(+), 3 deletions(-)
 create mode 100644 sound/pci/hda/hda_i915.c
 create mode 100644 sound/pci/hda/hda_i915.h

diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 80a7d44..c5a872c 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -152,6 +152,16 @@ config SND_HDA_CODEC_HDMI
          snd-hda-codec-hdmi.
          This module is automatically loaded at probing.
 
+config SND_HDA_I915
+       bool "Build Display HD-audio controller/codec power well support for 
i915 cards"
+       depends on DRM_I915
+       help
+         Say Y here to include full HDMI and DisplayPort HD-audio 
controller/codec
+         power-well support for Intel Haswell graphics cards based on the i915 
driver.
+
+         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.
+
 config SND_HDA_CODEC_CIRRUS
        bool "Build Cirrus Logic codec support"
        default y
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index 24a2514..c091438 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -1,4 +1,6 @@
 snd-hda-intel-objs := hda_intel.o
+# for haswell power well
+snd-hda-intel-$(CONFIG_SND_HDA_I915) +=        hda_i915.o
 
 snd-hda-codec-y := hda_codec.o hda_jack.o hda_auto_parser.o
 snd-hda-codec-$(CONFIG_SND_HDA_GENERIC) += hda_generic.o
diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
new file mode 100644
index 0000000..76c13d5
--- /dev/null
+++ b/sound/pci/hda/hda_i915.c
@@ -0,0 +1,75 @@
+/*
+ *  hda_i915.c - routines for Haswell HDA controller power well support
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the Free
+ *  Software Foundation; either version 2 of the License, or (at your option)
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ *  or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ *  for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software Foundation,
+ *  Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <sound/core.h>
+#include <drm/i915_powerwell.h>
+#include "hda_i915.h"
+
+static void (*get_power)(void);
+static void (*put_power)(void);
+
+void hda_display_power(bool enable)
+{
+       if (!get_power || !put_power)
+               return;
+
+       snd_printdd("HDA display power %s \n",
+                       enable ? "Enable" : "Disable");
+       if (enable)
+               get_power();
+       else
+               put_power();
+}
+
+int hda_i915_init(void)
+{
+       int err = 0;
+
+       get_power = symbol_request(i915_request_power_well);
+       if (!get_power) {
+               snd_printk(KERN_WARNING "hda-i915: get_power symbol get 
fail\n");
+               return -ENODEV;
+       }
+
+       put_power = symbol_request(i915_release_power_well);
+       if (!put_power) {
+               symbol_put(i915_request_power_well);
+               get_power = NULL;
+               return -ENODEV;
+       }
+
+       snd_printd("HDA driver get symbol successfully from i915 module\n");
+
+       return err;
+}
+
+int hda_i915_exit(void)
+{
+       if (get_power) {
+               symbol_put(i915_request_power_well);
+               get_power = NULL;
+       }
+       if (put_power) {
+               symbol_put(i915_release_power_well);
+               put_power = NULL;
+       }
+
+       return 0;
+}
diff --git a/sound/pci/hda/hda_i915.h b/sound/pci/hda/hda_i915.h
new file mode 100644
index 0000000..5a63da2
--- /dev/null
+++ b/sound/pci/hda/hda_i915.h
@@ -0,0 +1,35 @@
+/*
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the Free
+ *  Software Foundation; either version 2 of the License, or (at your option)
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ *  more details.
+ *
+ *  You should have received a copy of the GNU General Public License along 
with
+ *  this program; if not, write to the Free Software Foundation, Inc., 59
+ *  Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+#ifndef __SOUND_HDA_I915_H
+#define __SOUND_HDA_I915_H
+
+#ifdef CONFIG_SND_HDA_I915
+void hda_display_power(bool enable);
+int hda_i915_init(void);
+int hda_i915_exit(void);
+#else
+static inline void hda_display_power(bool enable) {}
+static inline int hda_i915_init(void)
+{
+       return -ENODEV;
+}
+static inline int hda_i915_exit(void)
+{
+       return 0;
+}
+#endif
+
+#endif
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 49cc942..1dedcb1 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -62,6 +62,7 @@
 #include <linux/vga_switcheroo.h>
 #include <linux/firmware.h>
 #include "hda_codec.h"
+#include "hda_i915.h"
 
 
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
@@ -541,6 +542,10 @@ struct azx {
        /* for pending irqs */
        struct work_struct irq_pending_work;
 
+#ifdef CONFIG_SND_HDA_I915
+       struct work_struct probe_work;
+#endif
+
        /* reboot notifier (for mysterious hangup problem at power-down) */
        struct notifier_block reboot_notifier;
 
@@ -594,6 +599,7 @@ enum {
 #define AZX_DCAPS_4K_BDLE_BOUNDARY (1 << 23)   /* BDLE in 4k boundary */
 #define AZX_DCAPS_COUNT_LPIB_DELAY  (1 << 25)  /* Take LPIB as delay */
 #define AZX_DCAPS_PM_RUNTIME   (1 << 26)       /* runtime PM support */
+#define AZX_DCAPS_I915_POWERWELL (1 << 27)     /* HSW i915 power well support 
*/
 
 /* quirks for Intel PCH */
 #define AZX_DCAPS_INTEL_PCH_NOPM \
@@ -2869,6 +2875,8 @@ static int azx_suspend(struct device *dev)
        pci_disable_device(pci);
        pci_save_state(pci);
        pci_set_power_state(pci, PCI_D3hot);
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+               hda_display_power(false);
        return 0;
 }
 
@@ -2881,6 +2889,8 @@ static int azx_resume(struct device *dev)
        if (chip->disabled)
                return 0;
 
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+               hda_display_power(true);
        pci_set_power_state(pci, PCI_D0);
        pci_restore_state(pci);
        if (pci_enable_device(pci) < 0) {
@@ -2913,6 +2923,8 @@ static int azx_runtime_suspend(struct device *dev)
 
        azx_stop_chip(chip);
        azx_clear_irq_pending(chip);
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+               hda_display_power(false);
        return 0;
 }
 
@@ -2921,6 +2933,8 @@ static int azx_runtime_resume(struct device *dev)
        struct snd_card *card = dev_get_drvdata(dev);
        struct azx *chip = card->private_data;
 
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+               hda_display_power(true);
        azx_init_pci(chip);
        azx_init_chip(chip, 1);
        return 0;
@@ -3147,6 +3161,10 @@ static int azx_free(struct azx *chip)
        if (chip->fw)
                release_firmware(chip->fw);
 #endif
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
+               hda_display_power(false);
+               hda_i915_exit();
+       }
        kfree(chip);
 
        return 0;
@@ -3372,6 +3390,13 @@ static void azx_check_snoop_available(struct azx *chip)
        }
 }
 
+#ifdef CONFIG_SND_HDA_I915
+static void azx_probe_work(struct work_struct *work)
+{
+       azx_probe_continue(container_of(work, struct azx, probe_work));
+}
+#endif
+
 /*
  * constructor
  */
@@ -3447,7 +3472,13 @@ static int azx_create(struct snd_card *card, struct 
pci_dev *pci,
                return err;
        }
 
+#ifdef CONFIG_SND_HDA_I915
+       /* continue probing in work context as may trigger request module */
+       INIT_WORK(&chip->probe_work, azx_probe_work);
+#endif
+
        *rchip = chip;
+
        return 0;
 }
 
@@ -3718,6 +3749,16 @@ static int azx_probe(struct pci_dev *pci,
        }
 #endif /* CONFIG_SND_HDA_PATCH_LOADER */
 
+       /* continue probing in work context, avoid request_module deadlock */
+       if (probe_now && (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)) {
+#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
+       }
+
        if (probe_now) {
                err = azx_probe_continue(chip);
                if (err < 0)
@@ -3740,6 +3781,16 @@ static int azx_probe_continue(struct azx *chip)
        int dev = chip->dev_index;
        int err;
 
+       /* Request power well for Haswell HDA controller and codec */
+       if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
+               err = hda_i915_init();
+               if (err < 0) {
+                       snd_printk(KERN_ERR SFX "Error request power-well from 
i915\n");
+                       goto out_free;
+               }
+               hda_display_power(true);
+       }
+
        err = azx_first_init(chip);
        if (err < 0)
                goto out_free;
@@ -3834,11 +3885,14 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = {
          .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH },
        /* Haswell */
        { PCI_DEVICE(0x8086, 0x0a0c),
-         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH },
+         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH |
+         AZX_DCAPS_I915_POWERWELL },
        { PCI_DEVICE(0x8086, 0x0c0c),
-         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH },
+         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH |
+         AZX_DCAPS_I915_POWERWELL },
        { PCI_DEVICE(0x8086, 0x0d0c),
-         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH },
+         .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH |
+         AZX_DCAPS_I915_POWERWELL },
        /* 5 Series/3400 */
        { PCI_DEVICE(0x8086, 0x3b56),
          .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_INTEL_PCH_NOPM },
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to