All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wang Xingchao <xingchao.wang@linux.intel.com>
To: tiwai@suse.de, daniel.vetter@ffwll.ch
Cc: alsa-devel@alsa-project.org, intel-gfx@lists.freedesktop.org,
	Wang Xingchao <xingchao.wang@linux.intel.com>,
	xingchao.wang@intel.com, liam.r.girdwood@intel.com,
	david.henningsson@canonical.com
Subject: [PATCH 3/4 V7] ALSA: hda - Add power-welll support for haswell HDA
Date: Thu, 30 May 2013 22:07:10 +0800	[thread overview]
Message-ID: <1369922831-727-4-git-send-email-xingchao.wang@linux.intel.com> (raw)
In-Reply-To: <1369922831-727-1-git-send-email-xingchao.wang@linux.intel.com>

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.wang@linux.intel.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: David Henningsson <david.henningsson@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

  parent reply	other threads:[~2013-05-30 14:34 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-30 14:07 [PATCH 0/4 V7] Power-well API implementation for Haswell Wang Xingchao
2013-05-30 14:07 ` [PATCH 1/4 V7] ALSA: hda - Fix runtime PM check Wang Xingchao
2013-05-30 14:07 ` [PATCH 2/4] ALSA: hda - Move azx_first_init() into azx_probe_continue() Wang Xingchao
2013-05-30 14:07 ` Wang Xingchao [this message]
2013-05-30 14:07 ` [PATCH 4/4 V7] i915/drm: Add private api for power well usage Wang Xingchao
2013-06-06 15:34 ` [PATCH 0/4 V7] Power-well API implementation for Haswell Daniel Vetter
2013-07-03 20:00   ` Paulo Zanoni
2013-07-04  8:23     ` Wang xingchao
2013-07-04 13:24       ` Paulo Zanoni
2013-07-04 13:13         ` [Intel-gfx] " Wang xingchao
2013-07-05 21:19           ` Paulo Zanoni
2013-07-06  6:20             ` [Intel-gfx] " Takashi Iwai
2013-07-07 23:59               ` Wang xingchao
2013-07-08  8:07                 ` Takashi Iwai
2013-07-17  2:52               ` [Intel-gfx] " Wang, Xingchao
2013-07-17  7:34                 ` [alsa-devel] " Takashi Iwai
2013-07-17  8:03                   ` [Intel-gfx] " Wang, Xingchao
2013-07-17  8:18                     ` Takashi Iwai
2013-07-17  8:25                       ` Wang, Xingchao
2013-07-17 13:31                         ` [alsa-devel] " Paulo Zanoni
2013-07-17 14:00                           ` [Intel-gfx] " Takashi Iwai
2013-07-17 14:05                             ` David Henningsson
2013-07-17 14:21                               ` [alsa-devel] " Takashi Iwai
2013-07-17 23:17                                 ` [Intel-gfx] " Wang, Xingchao
2013-07-18  1:00                                 ` Wang, Xingchao
2013-07-18  6:44                                   ` Daniel Vetter
2013-07-18  9:23                                     ` [alsa-devel] " Wang, Xingchao
2013-07-18  9:34                                       ` Takashi Iwai
2013-07-24 10:31                                         ` [Intel-gfx] " Wang, Xingchao
2013-07-24 11:02                                           ` [alsa-devel] " Takashi Iwai
2013-07-24 11:33                                             ` [Intel-gfx] " Wang, Xingchao
2013-07-24 11:57                                               ` [alsa-devel] " David Henningsson
2013-07-24 12:13                                                 ` [Intel-gfx] " Takashi Iwai
2013-07-24 13:14                                                 ` Rafael J. Wysocki
2013-07-24 13:30                                                   ` Wang, Xingchao
2013-07-24 13:42                                                     ` Takashi Iwai
2013-07-24 13:46                                                       ` Wang, Xingchao
2013-07-24 14:00                                                       ` Wang, Xingchao
2013-07-25  6:50                                                         ` [alsa-devel] " Wang, Xingchao
2013-07-24 14:36                                                       ` [Intel-gfx] " Wang, Xingchao
2013-07-18  6:54                                   ` [alsa-devel] " Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1369922831-727-4-git-send-email-xingchao.wang@linux.intel.com \
    --to=xingchao.wang@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=david.henningsson@canonical.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=liam.r.girdwood@intel.com \
    --cc=tiwai@suse.de \
    --cc=xingchao.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.