[alsa-devel] [PATCH] ALSA: hda - do delayed suspend for HD-audio devices
Takashi Iwai
tiwai at suse.de
Thu Dec 5 12:19:42 CET 2013
At Thu, 05 Dec 2013 11:39:08 +0100,
Takashi Iwai wrote:
>
> At Thu, 05 Dec 2013 09:21:48 +0100,
> Takashi Iwai wrote:
> >
> > At Wed, 4 Dec 2013 20:04:25 -0500,
> > mengdong.lin at intel.com wrote:
> > >
> > > From: Mengdong Lin <mengdong.lin at intel.com>
> > >
> > > HD-Audio codecs tends to take a long time to supend and is time-consuming part
> > > in driver suspend. In this patch, codec suspending and the left operations are
> > > delayed to a worker thead, so that they can be done asynchronously in parallel
> > > with suspending of other devices. And azx_suspend_late() is defined as PM
> > > .suspend_late ops to wait for completion of the delayed suspending.
> > >
> > > Credit goes to Liam Girdwood, who suggested this workaround.
> > >
> > > Signed-off-by: Mengdong Lin <mengdong.lin at intel.com>
> >
> > Did you try to set power.async_suspend flag?
> > The PM core has already async operation, so we don't have to implement
> > in the driver side.
>
> Actually the async_suspend is always set for PCI devices, thus the
> suspend and resume are executed asynchronously.
>
> It makes me wonder whether your patch gave any real measurable
> improvement...
BTW, while looking back at the recent code additions, I noticed that
we completely forgot about the standard async infrastructure.
The simplification patch is attached below. This seems working on my
test machine, at least. Give it a try.
Takashi
-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: hda - Clean up async codec PM using standard async
infrastructure
This simplifies lots of codes indeed.
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
sound/pci/hda/hda_codec.c | 64 ++++++++++++++---------------------------------
sound/pci/hda/hda_codec.h | 6 -----
2 files changed, 19 insertions(+), 51 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 5b7622034eee..2e090c8d7144 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -26,6 +26,7 @@
#include <linux/pci.h>
#include <linux/mutex.h>
#include <linux/module.h>
+#include <linux/async.h>
#include <sound/core.h>
#include "hda_codec.h"
#include <sound/asoundef.h>
@@ -96,8 +97,6 @@ EXPORT_SYMBOL_HDA(snd_hda_delete_codec_preset);
#ifdef CONFIG_PM
#define codec_in_pm(codec) ((codec)->in_pm)
-static void hda_suspend_work(struct work_struct *work);
-static void hda_resume_work(struct work_struct *work);
static void hda_power_work(struct work_struct *work);
static void hda_keep_power_on(struct hda_codec *codec);
#define hda_codec_is_power_on(codec) ((codec)->power_on)
@@ -842,11 +841,6 @@ static int snd_hda_bus_free(struct hda_bus *bus)
if (bus->workq)
destroy_workqueue(bus->workq);
-#ifdef CONFIG_PM
- if (bus->pm_wq)
- destroy_workqueue(bus->pm_wq);
-#endif
-
kfree(bus);
return 0;
}
@@ -891,9 +885,6 @@ int snd_hda_bus_new(struct snd_card *card,
.dev_register = snd_hda_bus_dev_register,
.dev_free = snd_hda_bus_dev_free,
};
-#ifdef CONFIG_PM
- char wqname[16];
-#endif
if (snd_BUG_ON(!temp))
return -EINVAL;
@@ -930,16 +921,6 @@ int snd_hda_bus_new(struct snd_card *card,
return -ENOMEM;
}
-#ifdef CONFIG_PM
- sprintf(wqname, "hda-pm-wq-%d", card->number);
- bus->pm_wq = create_workqueue(wqname);
- if (!bus->pm_wq) {
- snd_printk(KERN_ERR "cannot create PM workqueue\n");
- snd_hda_bus_free(bus);
- return -ENOMEM;
- }
-#endif
-
err = snd_device_new(card, SNDRV_DEV_BUS, bus, &dev_ops);
if (err < 0) {
snd_hda_bus_free(bus);
@@ -1476,8 +1457,6 @@ int snd_hda_codec_new(struct hda_bus *bus,
#ifdef CONFIG_PM
spin_lock_init(&codec->power_lock);
INIT_DELAYED_WORK(&codec->power_work, hda_power_work);
- INIT_WORK(&codec->suspend_work, hda_suspend_work);
- INIT_WORK(&codec->resume_work, hda_resume_work);
/* snd_hda_codec_new() marks the codec as power-up, and leave it as is.
* the caller has to power down appropriatley after initialization
* phase.
@@ -1495,9 +1474,6 @@ int snd_hda_codec_new(struct hda_bus *bus,
list_add_tail(&codec->list, &bus->codec_list);
bus->num_codecs++;
-#ifdef CONFIG_PM
- workqueue_set_max_active(bus->pm_wq, bus->num_codecs);
-#endif
bus->caddr_tbl[codec_addr] = codec;
@@ -5120,22 +5096,6 @@ 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_suspend_work(struct work_struct *work)
-{
- struct hda_codec *codec =
- container_of(work, struct hda_codec, suspend_work);
-
- hda_call_codec_suspend(codec, false);
-}
-
-static void hda_resume_work(struct work_struct *work)
-{
- struct hda_codec *codec =
- container_of(work, struct hda_codec, resume_work);
-
- hda_call_codec_resume(codec);
-}
#endif
/*
@@ -5699,6 +5659,17 @@ EXPORT_SYMBOL_HDA(snd_hda_add_imux_item);
* power management
*/
+
+static void hda_async_suspend(void *data, async_cookie_t cookie)
+{
+ hda_call_codec_suspend(data, false);
+}
+
+static void hda_async_resume(void *data, async_cookie_t cookie)
+{
+ hda_call_codec_resume(data);
+}
+
/**
* snd_hda_suspend - suspend the codecs
* @bus: the HDA bus
@@ -5708,19 +5679,21 @@ EXPORT_SYMBOL_HDA(snd_hda_add_imux_item);
int snd_hda_suspend(struct hda_bus *bus)
{
struct hda_codec *codec;
+ ASYNC_DOMAIN_EXCLUSIVE(domain);
list_for_each_entry(codec, &bus->codec_list, list) {
cancel_delayed_work_sync(&codec->jackpoll_work);
if (hda_codec_is_power_on(codec)) {
if (bus->num_codecs > 1)
- queue_work(bus->pm_wq, &codec->suspend_work);
+ async_schedule_domain(hda_async_suspend, codec,
+ &domain);
else
hda_call_codec_suspend(codec, false);
}
}
if (bus->num_codecs > 1)
- flush_workqueue(bus->pm_wq);
+ async_synchronize_full_domain(&domain);
return 0;
}
@@ -5735,16 +5708,17 @@ EXPORT_SYMBOL_HDA(snd_hda_suspend);
int snd_hda_resume(struct hda_bus *bus)
{
struct hda_codec *codec;
+ ASYNC_DOMAIN_EXCLUSIVE(domain);
list_for_each_entry(codec, &bus->codec_list, list) {
if (bus->num_codecs > 1)
- queue_work(bus->pm_wq, &codec->resume_work);
+ async_schedule_domain(hda_async_resume, codec, &domain);
else
hda_call_codec_resume(codec);
}
if (bus->num_codecs > 1)
- flush_workqueue(bus->pm_wq);
+ async_synchronize_full_domain(&domain);
return 0;
}
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 3ab4834761a8..260b190adf74 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -684,9 +684,6 @@ struct hda_bus {
struct hda_bus_unsolicited *unsol;
char workq_name[16];
struct workqueue_struct *workq; /* common workqueue for codecs */
-#ifdef CONFIG_PM
- struct workqueue_struct *pm_wq; /* workqueue to parallel codec PM */
-#endif
/* assigned PCMs */
DECLARE_BITMAP(pcm_dev_bits, SNDRV_PCM_DEVICES);
@@ -921,9 +918,6 @@ struct hda_codec {
unsigned long power_off_acct;
unsigned long power_jiffies;
spinlock_t power_lock;
- /* tasks to parallel multi-codec suspend/resume */
- struct work_struct suspend_work;
- struct work_struct resume_work;
#endif
/* filter the requested power state per nid */
--
1.8.5
More information about the Alsa-devel
mailing list