-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, November 26, 2013 10:05 PM
+#ifdef CONFIG_PM
- bus->pm_wq = create_workqueue("hda-pm-workq");
Better to use a unique name, e.g. with a card number suffix.
Ok.
- if (!bus->pm_wq) {
snd_printk(KERN_ERR "cannot create PM workqueue\n");snd_hda_bus_free(bus);return -ENOMEM;- }
- workqueue_set_max_active(bus->pm_wq, HDA_MAX_NUM_CODECS);
As mentioned in another post, this can be set dynamically, e.g. at snd_hda_suspend() with bus->num_codecs.
Can I move this to snd_hda_codec_new() like this? list_add_tail(&codec->list, &bus->codec_list); +bus->num_codecs++; +workqueue_set_max_active(bus->pm_wq, bus->num_codecs);
The codec number does not change after initialization, and usually 1 or 2 codecs are connected to the bus.
err = snd_device_new(card, SNDRV_DEV_BUS, bus, &dev_ops); if (err < 0) { snd_hda_bus_free(bus); @@ -1434,6 +1451,8 @@ int snd_hda_codec_new(struct hda_bus *bus, */ hda_keep_power_on(codec); hda_call_pm_notify(bus, true);
- INIT_WORK(&codec->pm_work, hda_pm_work);
Better to put this before hda_keep_power_on() call.
Ok.
if (codec->bus->modelname) { @@ -1445,6 +1464,7 @@ int snd_hda_codec_new(struct hda_bus *bus, }
list_add_tail(&codec->list, &bus->codec_list);
bus->num_codecs++; bus->caddr_tbl[codec_addr] = codec;
codec->vendor_id = snd_hda_param_read(codec, AC_NODE_ROOT,
Also decrease bus->num_codecs in snd_hda_codec_free(), just to be sure.
Can I decrease bus->num_codecs in snd_hda_bus_free(), after the codec is freed? list_for_each_entry_safe(codec, n, &bus->codec_list, list) { snd_hda_codec_free(codec); + bus->num_codecs--; }
@@ -5032,6 +5052,14 @@ int snd_hda_check_amp_list_power(struct
hda_codec *codec,
return 0; } EXPORT_SYMBOL_HDA(snd_hda_check_amp_list_power);
+static void hda_pm_work(struct work_struct *work) {
- struct hda_codec *codec =
container_of(work, struct hda_codec, pm_work);- hda_call_codec_suspend(codec, false); }
#endif
/* @@ -5595,11 +5623,27 @@
EXPORT_SYMBOL_HDA(snd_hda_add_imux_item);
int snd_hda_suspend(struct hda_bus *bus) { struct hda_codec *codec;
unsigned int mask = 0;
list_for_each_entry(codec, &bus->codec_list, list) { cancel_delayed_work_sync(&codec->jackpoll_work);
if (hda_codec_is_power_on(codec))hda_call_codec_suspend(codec, false);
if (hda_codec_is_power_on(codec)) {if (bus->num_codecs == 1)hda_call_codec_suspend(codec, false);else {mask |= 1 << codec->addr;queue_work(bus->pm_wq, &codec->pm_work);}}- }
- if (!mask)
return 0;- list_for_each_entry(codec, &bus->codec_list, list) {
if (mask & (1 << codec->addr))flush_work(&codec->pm_work);- }
You can drop the mask bit things, and here can be replaced with just a single call like: if (bus->num_codecs > 1) flush_workqueue(bus->pm_wq);
Ok. This is much better.
Thanks again! Mengdong