Suspend and Resume speed

Mark Brown broonie at
Wed Jun 11 19:26:17 CEST 2008

On Wed, Jun 11, 2008 at 04:31:46PM +0100, Andy Green wrote:

> It "works", audio is OK after resume, but it does not have any locking
> to stop races with other stuff wanting to touch codec registers while it
> is completing.  Do you have any ideas how to have it block other access
> to codec registers until it completes without making a locking Hell or
> lots of invasiveness?  It's fine if audio related userspace is going to
> block briefly until it is fully resumed, instead of whole of userspace
> is frozen until soc-audio completes resuming.

All you should need to do to make this work is use the snd_power*() API
which ought to be a matter of a few lines of code above what you've got
already.  If the core calls snd_power_change_state() to move into
SNDRV_CTL_POWER_D3hot as it starts to suspend and then goes back into
SNDRV_CTL_POWER_D0 once resume is complete then the ALSA core will
ensure that user space is blocked until the resume is complete.

Could you give that a spin?  If it works and survives further testing
it'd be good to push your revised patch out to ALSA for merge in 2.6.27
since it should be a noticable win for most users.

It also occurs to me that you will be able to save a little time overall
by making the driver only write back registers that aren't in the
default state on resume rather than blindly writing them all back as the
code currently does.  This is probably worth doing regardless of
anything else, though the saving will depend on the exact configuration
at suspend time.  Unfortunately this won't be too big a proportion of
the time consumed - a lot of it will come from the power up sequencing
writing registers multiple times to minimising pops.

Other than that, a couple of minor nits:

>  	struct delayed_work delayed_work;
> +	struct work_struct deferred_resume_work;
> +	struct platform_device *pdev;

The SoC device is always a platform device so you should be able to use
to_platform_device() on the dev that's in the structure already.

> +/* powers up audio subsystem after a suspend */
> +static int soc_resume(struct platform_device *pdev)
> +{
> +	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
> +	struct snd_soc_machine *machine = socdev->machine;
> +	int i;
> +
> +	socdev->pdev = pdev;
> +
> +	if (machine->resume_pre)
> +		machine->resume_pre(pdev);
> +
> +	for (i = 0; i < machine->num_links; i++) {
> +		struct snd_soc_cpu_dai *cpu_dai = machine->dai_link[i].cpu_dai;
> +		if (cpu_dai->resume && cpu_dai->type == SND_SOC_DAI_AC97)
> +			cpu_dai->resume(pdev, cpu_dai);
> +	}

We may as well punt everything to the workqueue - I can't see any need
to do this part of the resume immediately if we're going to punt the
rest anyway?

More information about the openmoko-kernel mailing list