[Intel-gfx] [PATCH v3 4/5] ALSA: hda: add component support

Imre Deak imre.deak at intel.com
Tue Dec 9 08:15:55 PST 2014


Register a component master to be used to interface with the i915
driver. This is meant to replace the current interface which is based on
module symbol lookups.

Note that currently we keep the existing behavior and pin the i915
module while the hda driver is loaded. Using the component interface
allows us to remove this dependency once support for dynamically
enabling / disabling the HDMI functionality is added to the driver.

v2:
- change roles between the hda and i915 components (Daniel)
v3:
- rename display_component to audio_component (Daniel)

Signed-off-by: Imre Deak <imre.deak at intel.com>
---
 include/drm/i915_powerwell.h |  37 ------------
 sound/pci/hda/hda_i915.c     | 136 +++++++++++++++++++++++++++++++------------
 sound/pci/hda/hda_intel.c    |   3 +-
 sound/pci/hda/hda_priv.h     |   4 ++
 4 files changed, 103 insertions(+), 77 deletions(-)
 delete mode 100644 include/drm/i915_powerwell.h

diff --git a/include/drm/i915_powerwell.h b/include/drm/i915_powerwell.h
deleted file mode 100644
index baa6f11..0000000
--- a/include/drm/i915_powerwell.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2013 Intel Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- *
- **************************************************************************/
-
-#ifndef _I915_POWERWELL_H_
-#define _I915_POWERWELL_H_
-
-/* For use by hda_i915 driver */
-extern int i915_request_power_well(void);
-extern int i915_release_power_well(void);
-extern int i915_get_cdclk_freq(void);
-
-#endif				/* _I915_POWERWELL_H_ */
diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
index 4e4b733..78894ec 100644
--- a/sound/pci/hda/hda_i915.c
+++ b/sound/pci/hda/hda_i915.c
@@ -18,8 +18,10 @@
 
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/component.h>
+#include <drm/i915_component.h>
 #include <sound/core.h>
-#include <drm/i915_powerwell.h>
 #include "hda_priv.h"
 #include "hda_i915.h"
 
@@ -31,32 +33,33 @@
 #define AZX_REG_EM4			0x100c
 #define AZX_REG_EM5			0x1010
 
-static int (*get_power)(void);
-static int (*put_power)(void);
-static int (*get_cdclk)(void);
-
 int hda_display_power(struct azx *chip, bool enable)
 {
-	if (!get_power || !put_power)
+	struct i915_audio_component *acomp = &chip->audio_component;
+
+	if (!acomp->ops)
 		return -ENODEV;
 
 	pr_debug("HDA display power %s \n",
 			enable ? "Enable" : "Disable");
 	if (enable)
-		return get_power();
+		acomp->ops->get_power(acomp->dev);
 	else
-		return put_power();
+		acomp->ops->put_power(acomp->dev);
+
+	return 0;
 }
 
 void haswell_set_bclk(struct azx *chip)
 {
 	int cdclk_freq;
 	unsigned int bclk_m, bclk_n;
+	struct i915_audio_component *acomp = &chip->audio_component;
 
-	if (!get_cdclk)
+	if (!acomp->ops)
 		return;
 
-	cdclk_freq = get_cdclk();
+	cdclk_freq = acomp->ops->get_cdclk_freq(acomp->dev);
 	switch (cdclk_freq) {
 	case 337500:
 		bclk_m = 16;
@@ -84,47 +87,104 @@ void haswell_set_bclk(struct azx *chip)
 	azx_writew(chip, EM5, bclk_n);
 }
 
+static int hda_component_master_bind(struct device *dev)
+{
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
+	struct i915_audio_component *acomp = &chip->audio_component;
+	int ret;
+
+	ret = component_bind_all(dev, acomp);
+	if (ret < 0)
+		return ret;
+
+	if (WARN_ON(!(acomp->dev && acomp->ops && acomp->ops->get_power &&
+		      acomp->ops->put_power && acomp->ops->get_cdclk_freq))) {
+		ret = -EINVAL;
+		goto out_unbind;
+	}
+
+	/*
+	 * Atm, we don't support dynamic unbinding initiated by the child
+	 * component, so pin its containing module until we unbind.
+	 */
+	if (!try_module_get(acomp->ops->owner)) {
+		ret = -ENODEV;
+		goto out_unbind;
+	}
+
+	return 0;
+
+out_unbind:
+	component_unbind_all(dev, acomp);
+
+	return ret;
+}
+
+static void hda_component_master_unbind(struct device *dev)
+{
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
+	struct i915_audio_component *acomp = &chip->audio_component;
+
+	module_put(acomp->ops->owner);
+	component_unbind_all(dev, acomp);
+	WARN_ON(acomp->ops || acomp->dev);
+}
+
+static const struct component_master_ops hda_component_master_ops = {
+	.bind = hda_component_master_bind,
+	.unbind = hda_component_master_unbind,
+};
+
+static int hda_component_master_match(struct device *dev, void *data)
+{
+	/* i915 is the only supported component */
+	return !strcmp(dev->driver->name, "i915");
+}
 
 int hda_i915_init(struct azx *chip)
 {
-	int err = 0;
+	struct component_match *match = NULL;
+	struct device *dev = &chip->pci->dev;
+	struct i915_audio_component *acomp = &chip->audio_component;
+	int ret;
 
-	get_power = symbol_request(i915_request_power_well);
-	if (!get_power) {
-		pr_warn("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;
-	}
-
-	get_cdclk = symbol_request(i915_get_cdclk_freq);
-	if (!get_cdclk)	/* may have abnormal BCLK and audio playback rate */
-		pr_warn("hda-i915: get_cdclk symbol get fail\n");
-
+	component_match_add(dev, &match, hda_component_master_match, chip);
+	ret = component_master_add_with_match(dev, &hda_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.
+	 */
+	ret = request_module("i915");
+	if (ret < 0)
+		goto out_master_del;
+
+	if (!acomp->ops) {
+		ret = -ENODEV;
+		goto out_master_del;
+	}
+
-	pr_debug("HDA driver get symbol successfully from i915 module\n");
+	pr_debug("hda-i915: bound to i915 component\n");
+
+	return 0;
+out_master_del:
+	component_master_del(dev, &hda_component_master_ops);
+out_err:
+	pr_warn("hda-i915: failed to add component master (%d)\n", ret);
 
-	return err;
+	return ret;
 }
 
 int hda_i915_exit(struct azx *chip)
 {
-	if (get_power) {
-		symbol_put(i915_request_power_well);
-		get_power = NULL;
-	}
-	if (put_power) {
-		symbol_put(i915_release_power_well);
-		put_power = NULL;
-	}
-	if (get_cdclk) {
-		symbol_put(i915_get_cdclk_freq);
-		get_cdclk = NULL;
-	}
+	struct device *dev = &chip->pci->dev;
+
+	component_master_del(dev, &hda_component_master_ops);
 
 	return 0;
 }
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index f3b5dcd..e15b30e 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1912,8 +1912,7 @@ static int azx_probe_continue(struct azx *chip)
 #ifdef CONFIG_SND_HDA_I915
 		err = hda_i915_init(chip);
 		if (err < 0) {
-			dev_err(chip->card->dev,
-				"Error request power-well from i915\n");
+			dev_err(chip->card->dev, "Error binding to i915\n");
 			goto out_free;
 		}
 		err = hda_display_power(chip, true);
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index aa484fd..e6ac4e3 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -18,6 +18,7 @@
 #include <linux/clocksource.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
+#include <drm/i915_component.h>
 
 /*
  * registers
@@ -291,6 +292,9 @@ struct azx {
 	struct pci_dev *pci;
 	int dev_index;
 
+	/* i915 component interface */
+	struct i915_audio_component audio_component;
+
 	/* chip type specific */
 	int driver_type;
 	unsigned int driver_caps;
-- 
1.8.4



More information about the Intel-gfx mailing list