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

[Takashi Iwai]

At Tue, 26 Nov 2013 16:23:40 -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 disabled, driver suspend time can be long especially when there
> are more than one codec on the bus.
> 
> To reduce driver suspend time, this patch creates a work queue for the bus, and
> suspends the codecs in parallel if there are multiple codecs on the 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..21c241c 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -104,6 +104,7 @@ static inline void hda_call_pm_notify(struct hda_bus *bus, bool power_up)
>  	if (bus->ops.pm_notify)
>  		bus->ops.pm_notify(bus, power_up);
>  }
> +static void hda_pm_work(struct work_struct *work);
>  #else
>  #define codec_in_pm(codec)	0
>  static inline void hda_keep_power_on(struct hda_codec *codec) {}
> @@ -826,11 +827,18 @@ static int snd_hda_bus_free(struct hda_bus *bus)
>  		kfree(bus->unsol);
>  	list_for_each_entry_safe(codec, n, &bus->codec_list, list) {
>  		snd_hda_codec_free(codec);
> +		bus->num_codecs--;
>  	}
>  	if (bus->ops.private_free)
>  		bus->ops.private_free(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;
>  }
> @@ -875,6 +883,7 @@ int snd_hda_bus_new(struct snd_card *card,
>  		.dev_register = snd_hda_bus_dev_register,
>  		.dev_free = snd_hda_bus_dev_free,
>  	};
> +	char tmp[32];
>  
>  	if (snd_BUG_ON(!temp))
>  		return -EINVAL;
> @@ -911,6 +920,16 @@ int snd_hda_bus_new(struct snd_card *card,
>  		return -ENOMEM;
>  	}
>  
> +#ifdef CONFIG_PM
> +	sprintf(tmp, "hda-pm-wq-card%d", card->number);

IMO, "hda-pm-wq-%d" should suffice.

> +	bus->pm_wq = create_workqueue(tmp);
> +	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);
> @@ -1428,6 +1447,7 @@ 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->pm_work, hda_pm_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.
> @@ -1445,6 +1465,9 @@ int snd_hda_codec_new(struct hda_bus *bus,
>  	}
>  
>  	list_add_tail(&codec->list, &bus->codec_list);
> +	bus->num_codecs++;
> +	workqueue_set_max_active(bus->pm_wq, bus->num_codecs);
> +
>  	bus->caddr_tbl[codec_addr] = codec;
>  
>  	codec->vendor_id = snd_hda_param_read(codec, AC_NODE_ROOT,
> @@ -5032,6 +5055,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
>  
>  /*
> @@ -5598,9 +5629,17 @@ int snd_hda_suspend(struct hda_bus *bus)
>  
>  	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)
> +				queue_work(bus->pm_wq, &codec->pm_work);
> +			else
> +				hda_call_codec_suspend(codec, false);
> +			}

A wrong indent level.


Takashi

>  	}
> +
> +	if (bus->num_codecs > 1)
> +		flush_workqueue(bus->pm_wq);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_HDA(snd_hda_suspend);
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 7aa9870..82807ef 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -673,6 +673,7 @@ struct hda_bus {
>  
>  	/* codec linked list */
>  	struct list_head codec_list;
> +	unsigned int num_codecs;
>  	/* link caddr -> codec */
>  	struct hda_codec *caddr_tbl[HDA_MAX_CODEC_ADDRESS + 1];
>  
> @@ -683,6 +684,9 @@ 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);
> @@ -916,6 +920,7 @@ struct hda_codec {
>  	unsigned long power_off_acct;
>  	unsigned long power_jiffies;
>  	spinlock_t power_lock;
> +	struct work_struct pm_work; /* task to parallel multi-codec PM */
>  #endif
>  
>  	/* filter the requested power state per nid */
> -- 
> 1.8.1.2
>