[alsa-devel] [RFC PATCH] ALSA: hda - suspend codecs in parallel

[Takashi Iwai]

At Sun, 24 Nov 2013 22:33:41 -0500,
mengdong.lin at intel.com wrote:
> 
> From: Mengdong Lin <mengdong.lin at intel.com>
> 
> The time to suspend a single codec may be several hundreds of ms. When runtime
> power saving is not enabled, driver suspend time can be long especially there
> are more than one codec on the bus.
> 
> To reduce driver suspend time, this patch creates a work queue for each codec
> and will suspend the codecs in parallel, if there are multiple codecs on bus.
> 
> Signed-off-by: Mengdong Lin <mengdong.lin at intel.com>
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index cb0c776..c9092a1 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -97,6 +97,7 @@ EXPORT_SYMBOL_HDA(snd_hda_delete_codec_preset);
>  #ifdef CONFIG_PM
>  #define codec_in_pm(codec)	((codec)->in_pm)
>  static void hda_power_work(struct work_struct *work);
> +static void hda_pm_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)
>  static inline void hda_call_pm_notify(struct hda_bus *bus, bool power_up)
> @@ -1355,6 +1356,8 @@ static void snd_hda_codec_free(struct hda_codec *codec)
>  #ifdef CONFIG_PM
>  	if (!codec->pm_down_notified) /* cancel leftover refcounts */
>  		hda_call_pm_notify(codec->bus, false);
> +	if (codec->pm_wq)
> +		destroy_workqueue(codec->pm_wq);
>  #endif
>  	module_put(codec->owner);
>  	free_hda_cache(&codec->amp_cache);
> @@ -1434,6 +1437,13 @@ int snd_hda_codec_new(struct hda_bus *bus,
>  	 */
>  	hda_keep_power_on(codec);
>  	hda_call_pm_notify(bus, true);
> +
> +	codec->pm_wq = create_workqueue("hda_codec_wq");
> +	if (!codec->pm_wq) {
> +		snd_hda_codec_free(codec);
> +		return -ENOMEM;
> +	}
> +	INIT_WORK(&codec->pm_work, hda_pm_work);
>  #endif
>  
>  	if (codec->bus->modelname) {
> @@ -1445,6 +1455,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,
> @@ -5032,6 +5043,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 +5614,24 @@ 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 (codec->epss || bus->num_codecs == 1)
> +				hda_call_codec_suspend(codec, false);

What's the reason for codec->epss check exceptionally?

> +			else {
> +				mask |= 1 << codec->addr;
> +				queue_work(codec->pm_wq, &codec->pm_work);
> +			}
> +		}
> +	}
> +
> +	list_for_each_entry(codec, &bus->codec_list, list) {
> +		if (mask & (1 << codec->addr))
> +			flush_work(&codec->pm_work);
> +
>  	}

I thought that the recent workqueue is performed asynchronously as
default (unless an ordered workqueue is used), so it should be enough
to have a single workqueue in the bus, and flush it.


thanks,

Takashi