diff mbox

[alsa-devel] HDMI codec, way forward?

Message ID s5h1tcoyxq1.wl-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai Oct. 21, 2015, 4:48 p.m. UTC 
On Wed, 21 Oct 2015 18:46:23 +0200,
Takashi Iwai wrote:
> 
> Below is a test patch I cooked quickly.  This is the third patch
> applied after other two more patches: a cleanup patch and a patch for
> deferred probe of HD-audio with component.

And this is the patch to defer the probe.

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH 2/3] ALSA: hda - Implement deferred probe for i915 audio
 component

For Intel controllers that mandate i915 binding, the probe is deferred
and done through the component master bind ops.  The controller driver
needs to pass its ops to continue the probe.  For the legacy HDA, we
just call the existing azx_probe_continue() there.

A possible drawback is that there might be no longer fallback working
when the i915 graphics is unavailable, e.g. boot with nomodeset
option, on SKL/BSW.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/hda_i915.h  |  7 ++++--
 sound/hda/hdac_i915.c     | 60 ++++++++++++++++++++++-------------------------
 sound/pci/hda/hda_intel.c | 45 +++++++++++++++++++----------------
 3 files changed, 58 insertions(+), 54 deletions(-)
diff mbox

Patch

diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index 7b19f1f8cc23..b42449af6cd2 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -4,13 +4,15 @@ 
 #ifndef __SOUND_HDA_I915_H
 #define __SOUND_HDA_I915_H
 
+#include <linux/component.h>
 #include <drm/i915_component.h>
 
 #ifdef CONFIG_SND_HDA_I915
 int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);
 int snd_hdac_display_power(struct hdac_bus *bus, bool enable);
 int snd_hdac_get_display_clk(struct hdac_bus *bus);
-int snd_hdac_i915_init(struct hdac_bus *bus);
+int snd_hdac_i915_init(struct hdac_bus *bus,
+		       const struct component_master_ops *codec_ops);
 int snd_hdac_i915_exit(struct hdac_bus *bus);
 int snd_hdac_i915_register_notifier(struct hdac_device *codec,
 				    const struct i915_audio_component_audio_ops *);
@@ -27,7 +29,8 @@  static inline int snd_hdac_get_display_clk(struct hdac_bus *bus)
 {
 	return 0;
 }
-static inline int snd_hdac_i915_init(struct hdac_bus *bus)
+static inline int snd_hdac_i915_init(struct hdac_bus *bus,
+				     const struct component_master_ops *codec_ops)
 {
 	return -ENODEV;
 }
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index e36ed18f23c4..f6fc16cfd02c 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -21,6 +21,11 @@ 
 #include <sound/hdaudio.h>
 #include <sound/hda_i915.h>
 
+struct hdac_gfx_component {
+	struct i915_audio_component acomp;
+	const struct component_master_ops *ops;
+};
+
 int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
 {
 	struct i915_audio_component *acomp = bus->audio_component;
@@ -85,6 +90,8 @@  static int hdac_component_master_bind(struct device *dev)
 	struct hdac_device *codec = dev_to_hdac_dev(dev);
 	struct hdac_bus *bus = codec->bus;
 	struct i915_audio_component *acomp = bus->audio_component;
+	struct hdac_gfx_component *hda_comp =
+		container_of(acomp, struct hdac_gfx_component, acomp);
 	int ret;
 
 	ret = component_bind_all(dev, acomp);
@@ -106,6 +113,14 @@  static int hdac_component_master_bind(struct device *dev)
 		goto out_unbind;
 	}
 
+	if (hda_comp->ops && hda_comp->ops->bind) {
+		ret = hda_comp->ops->bind(dev);
+		if (ret < 0) {
+			module_put(acomp->ops->owner);
+			goto out_unbind;
+		}
+	}
+
 	return 0;
 
 out_unbind:
@@ -119,7 +134,11 @@  static void hdac_component_master_unbind(struct device *dev)
 	struct hdac_device *codec = dev_to_hdac_dev(dev);
 	struct hdac_bus *bus = codec->bus;
 	struct i915_audio_component *acomp = bus->audio_component;
+	struct hdac_gfx_component *hda_comp =
+		container_of(acomp, struct hdac_gfx_component, acomp);
 
+	if (hda_comp->ops && hda_comp->ops->unbind)
+		hda_comp->ops->unbind(dev);
 	module_put(acomp->ops->owner);
 	component_unbind_all(dev, acomp);
 	WARN_ON(acomp->ops || acomp->dev);
@@ -150,45 +169,22 @@  int snd_hdac_i915_register_notifier(struct hdac_device *codec,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_register_notifier);
 
-int snd_hdac_i915_init(struct hdac_bus *bus)
+int snd_hdac_i915_init(struct hdac_bus *bus,
+		       const struct component_master_ops *codec_ops)
 {
 	struct component_match *match = NULL;
 	struct device *dev = bus->dev;
-	struct i915_audio_component *acomp;
-	int ret;
+	struct hdac_gfx_component *hda_comp;
 
-	acomp = kzalloc(sizeof(*acomp), GFP_KERNEL);
-	if (!acomp)
+	hda_comp = kzalloc(sizeof(*hda_comp), GFP_KERNEL);
+	if (!hda_comp)
 		return -ENOMEM;
-	bus->audio_component = acomp;
+	bus->audio_component = &hda_comp->acomp;
+	hda_comp->ops = codec_ops;
 
 	component_match_add(dev, &match, hdac_component_master_match, bus);
-	ret = component_master_add_with_match(dev, &hdac_component_master_ops,
-					      match);
-	if (ret < 0)
-		goto out_err;
-
-	/*
-	 * Atm, we don't support deferring the component binding, so make sure
-	 * i915 is loaded and that the binding successfully completes.
-	 */
-	request_module("i915");
-
-	if (!acomp->ops) {
-		ret = -ENODEV;
-		goto out_master_del;
-	}
-	dev_dbg(dev, "bound to i915 component master\n");
-
-	return 0;
-out_master_del:
-	component_master_del(dev, &hdac_component_master_ops);
-out_err:
-	kfree(acomp);
-	bus->audio_component = NULL;
-	dev_err(dev, "failed to add i915 component master (%d)\n", ret);
-
-	return ret;
+	return component_master_add_with_match(dev, &hdac_component_master_ops,
+					       match);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_init);
 
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index c38c68f57938..78deda018894 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1884,6 +1884,20 @@  static const struct hda_controller_ops pci_hda_ops = {
 	.link_power = azx_intel_link_power,
 };
 
+#ifdef CONFIG_SND_HDA_I915
+static int azx_i915_bind(struct device *dev)
+{
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
+
+	return azx_probe_continue(chip);
+}
+
+static const struct component_master_ops component_ops = {
+	.bind = azx_i915_bind,
+};
+#endif /* CONFIG_SND_HDA_I915 */
+
 static int azx_probe(struct pci_dev *pci,
 		     const struct pci_device_id *pci_id)
 {
@@ -1943,10 +1957,19 @@  static int azx_probe(struct pci_dev *pci,
 	}
 #endif /* CONFIG_SND_HDA_PATCH_LOADER */
 
-#ifndef CONFIG_SND_HDA_I915
-	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
+#ifdef CONFIG_SND_HDA_I915
+		/* HSW/BDW controllers need this power */
+		if (CONTROLLER_IN_GPU(pci))
+			hda->need_i915_power = 1;
+		err = snd_hdac_i915_init(azx_bus(chip), &component_ops);
+		if (err < 0)
+			goto out_free;
+		schedule_probe = false; /* continue via bind ops */
+#else
 		dev_err(card->dev, "Haswell must build in CONFIG_SND_HDA_I915\n");
 #endif
+	}
 
 	if (schedule_probe)
 		schedule_work(&hda->probe_work);
@@ -1983,23 +2006,6 @@  static int azx_probe_continue(struct azx *chip)
 	 * 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_i915_init(bus);
-		if (err < 0) {
-			/* if the controller is bound only with HDMI/DP
-			 * (for HSW and BDW), we need to abort the probe;
-			 * for other chips, still continue probing as other
-			 * codecs can be on the same link.
-			 */
-			if (CONTROLLER_IN_GPU(pci))
-				goto out_free;
-			else
-				goto skip_i915;
-		}
-
 		err = snd_hdac_display_power(bus, true);
 		if (err < 0) {
 			dev_err(chip->card->dev,
@@ -2008,7 +2014,6 @@  static int azx_probe_continue(struct azx *chip)
 		}
 	}
 
- skip_i915:
 	err = azx_first_init(chip);
 	if (err < 0)
 		goto out_free;