Re: [alsa-devel] [PATCH 5/5] ALSA/i915: Check power well API existense before calling

At Mon, 13 May 2013 15:37:28 +0800, Wang Xingchao wrote:

I915 module maybe loaded after snd_hda_intel, the power-well API doesnot exist in such case. This patch intended to avoid loading dependency between snd-hda-intel and i915 module.

Signed-off-by: Wang Xingchao xingchao.wang@linux.intel.com

Hm.... somehow the split of the patches hasn't been done in a logical way, so reviewing each small change is rather confusing. In the patchset, we need just two patches: one change for i915 and one for HD-audio. The development timeline is sometimes useful, but not really in this case.

thanks,

Takashi

---

drivers/gpu/drm/i915/i915_dma.c | 3 ++ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 12 +++++++ include/drm/i915_powerwell.h | 1 + sound/pci/hda/hda_i915.c | 69 ++++++++++++++++++++++++-------------- sound/pci/hda/hda_i915.h | 5 +++ sound/pci/hda/hda_intel.c | 8 +++-- 7 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a1648eb..307ca4b 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1671,6 +1671,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (IS_GEN5(dev)) intel_gpu_ips_init(dev_priv);

• if (IS_HASWELL(dev))

• intel_gpu_audio_init();
  

• return 0;

out_gem_unload: diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dfcf546..f159f12 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -758,6 +758,8 @@ extern void intel_update_fbc(struct drm_device *dev); /* IPS */ extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv); extern void intel_gpu_ips_teardown(void); +/* Display audio */ +extern void intel_gpu_audio_init(void);

extern bool intel_using_power_well(struct drm_device *dev); extern void intel_init_power_well(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 642c4b6..8c1df21 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -29,6 +29,7 @@ #include "i915_drv.h" #include "intel_drv.h" #include "../../../platform/x86/intel_ips.h" +#include "../../../../sound/pci/hda/hda_i915.h" #include <linux/module.h>

#define FORCEWAKE_ACK_TIMEOUT_MS 2 @@ -4393,6 +4394,17 @@ struct i915_power_well_user { struct list_head list; };

+void intel_gpu_audio_init(void) +{

• void (*link)(void);
• link = symbol_get(audio_link_to_i915_driver);
• if (link) {

• link();
  

• symbol_put(audio_link_to_i915_driver);
  

• }

+}

void i915_request_power_well(const char *name) { struct i915_power_well_user *user; diff --git a/include/drm/i915_powerwell.h b/include/drm/i915_powerwell.h index 87e0a2e..de03dc8 100644 --- a/include/drm/i915_powerwell.h +++ b/include/drm/i915_powerwell.h @@ -33,6 +33,7 @@ #ifndef _I915_POWERWELL_H_ #define _I915_POWERWELL_H_

+/* For use by hda_i915 driver */ extern void i915_request_power_well(const char *name); extern void i915_release_power_well(const char *name);

diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c index edf1a08..d11f255 100644 --- a/sound/pci/hda/hda_i915.c +++ b/sound/pci/hda/hda_i915.c @@ -22,54 +22,71 @@ #include <drm/i915_powerwell.h> #include "hda_i915.h"

-const char *name = "i915"; -static void (*get_power)(const char *name); -static void (*put_power)(const char *name); +char *module_name = "i915"; +void (*get_power)(const char *); +void (*put_power)(const char *); +static bool hsw_i915_load;

+void audio_link_to_i915_driver(void) +{

• /* it's time to try getting the symbols again.*/
• hsw_i915_load = true;

+} +EXPORT_SYMBOL_GPL(audio_link_to_i915_driver);

/* Power well has impact on Haswell controller and codecs */ void hda_display_power(bool enable) {

• snd_printk(KERN_INFO "HDA display power %s \n", enable ? "Enable" : "Disable");
• if (!get_power || !put_power)

• return;
  

• if (!get_power || !put_power) {

• if (hsw_i915_load) {
  

• 	get_power = i915_request_power_well;
  

• 	put_power = i915_release_power_well;
  

• } else
  

• 	return;
  

• }

• snd_printk(KERN_DEBUG "HDA display power %s \n",

• 	enable ? "Enable" : "Disable");
  

  if (enable) get_power("hda"); else put_power("hda");

} -EXPORT_SYMBOL(hda_display_power); +EXPORT_SYMBOL_GPL(hda_display_power);

-static int __init hda_i915_init(void) +/* In case i915 module loaded first, the APIs are there.

• • Otherwise wait until i915 module notify us. */

+int hda_i915_init(void) {

• struct module *i915;
• mutex_lock(&module_mutex);
• i915 = find_module(name);
• mutex_unlock(&module_mutex);

• struct module *i915;

• if (!i915)

• request_module_nowait(name);
  

• mutex_lock(&module_mutex);
• i915 = find_module(module_name);
• mutex_unlock(&module_mutex);

• if (!try_module_get(i915))

• return -EFAULT;
  

• if (!i915)

• request_module_nowait(module_name);
  
  

  get_power = symbol_get(i915_request_power_well);

• if (!get_power)

• goto out;
  

• put_power = symbol_get(i915_release_power_well);

• if (!put_power)

• goto out;
  
  

• module_put(i915);

• snd_printk(KERN_DEBUG "HDA driver get symbol successfully from i915 module\n");

+out: return 0; } +EXPORT_SYMBOL_GPL(hda_i915_init);

-#if 0 -static void __exit hda_i915_exit(void) +int hda_i915_exit(void) {

• drm_pci_exit(&driver, &i915_pci_driver);

• symbol_put(i915_request_power_well);
• symbol_put(i915_release_power_well);

} +EXPORT_SYMBOL_GPL(hda_i915_exit);

-module_init(hda_i915_init); -module_exit(hda_i915_exit); -#endif -module_init(hda_i915_init); -MODULE_DESCRIPTION("HDA power well"); +MODULE_DESCRIPTION("HDA power well For Haswell"); MODULE_LICENSE("GPL"); diff --git a/sound/pci/hda/hda_i915.h b/sound/pci/hda/hda_i915.h index a7e5324..b0516ab 100644 --- a/sound/pci/hda/hda_i915.h +++ b/sound/pci/hda/hda_i915.h @@ -3,8 +3,13 @@

#ifdef CONFIG_SND_HDA_I915 void hda_display_power(bool enable); +int hda_i915_init(void); +int hda_i915_exit(void); #else void hda_display_power(bool enable) {} +int hda_i915_init(void) {} +int hda_i915_exit(void) {} #endif

+void audio_link_to_i915_driver(void); #endif diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 8bb6075..431027d 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -3684,8 +3684,10 @@ static int azx_probe(struct pci_dev *pci, return -ENOENT; }

• if (IS_HSW(pci))

• if (IS_HSW(pci)) {

• hda_i915_init();
  

  hda_display_power(true);

• }

  err = snd_card_create(index[dev], id[dev], THIS_MODULE, 0, &card); if (err < 0) {

@@ -3822,8 +3824,10 @@ static void azx_remove(struct pci_dev *pci) if (card) snd_card_free(card); pci_set_drvdata(pci, NULL);

• if (IS_HSW(pci))

• if (IS_HSW(pci)) { hda_display_power(false);

• hda_i915_exit();
  

• }

}

/* PCI IDs */

1.7.9.5

---

Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel