From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: audio(4): postpone volume changes to the end of DVACT_WAKEUP
To: Alexandre Ratchov <alex@caoua.org>
Cc: tech@openbsd.org
Date: Wed, 14 Aug 2024 00:17:27 +0200

> Date: Tue, 13 Aug 2024 15:11:26 +0200
> From: Alexandre Ratchov <alex@caoua.org>
> 
> During DVACT_WAKEUP, acpithinkpad(4) restores the speaker mute control
> using the wskbd interface with a value obtained from the acpi
> controller. As the audio(4) driver saves and restores the full mixer
> state, the mute control is overwritten with the value saved during
> suspend, which is not acpithinkpad(4)'s intent.
> 
> As both saved and acpithinkpad states are the same most (all?) of the
> time the problems stays unnoticed, but let's fix it.
> 
> So, postpone changes with the wskbd interface until the end of the
> mixer's DVACT_WAKEUP section of audio(4).

That is a good idea.  Not sure how you spotted this, but I was playing
around with having azalia(4) do its suspend/resume in
DVACT_QUIESCE/DVACT_WAKEUP and this fixes the

azalia0: CORB is not running

message I saw.

> OK?

ok kettenis@

> Index: audio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/audio.c,v
> diff -u -p -r1.207 audio.c
> --- audio.c	7 Jun 2024 08:48:10 -0000	1.207
> +++ audio.c	13 Aug 2024 12:48:16 -0000
> @@ -1323,6 +1323,12 @@ audio_activate(struct device *self, int 
>  			audio_stop_do(sc);
>  
>  		/*
> +		 * prevent keyboard keys from touching the controls
> +		 */
> +		if (task_del(systq, &sc->wskbd_task))
> +			device_unref(&sc->dev);
> +
> +		/*
>  		 * save mixer state
>  		 */
>  		for (i = 0; i != sc->mix_nent; i++)
> @@ -1349,6 +1355,13 @@ audio_activate(struct device *self, int 
>  		sc->quiesce = 0;
>  		wakeup(&sc->quiesce);
>  
> +		/*
> +		 * run the wskbd task (changes can be scheduled during sleep)
> +		 */
> +		device_ref(&sc->dev);
> +		if (!task_add(systq, &sc->wskbd_task))
> +			device_unref(&sc->dev);
> +
>  		if (sc->mode != 0) {
>  			if (audio_setpar(sc) != 0)
>  				break;
> @@ -2440,7 +2453,7 @@ wskbd_set_mixermute(long mute, long out)
>  		return ENODEV;
>  	vol = out ? &sc->spkr : &sc->mic;
>  	vol->mute_pending = mute ? WSKBD_MUTE_ENABLE : WSKBD_MUTE_DISABLE;
> -	if (!task_add(systq, &sc->wskbd_task))
> +	if (sc->quiesce || !task_add(systq, &sc->wskbd_task))
>  		device_unref(&sc->dev);
>  	return 0;
>  }
> @@ -2494,7 +2507,7 @@ wskbd_set_mixervolume_unit(int unit, lon
>  		vol->mute_pending ^= WSKBD_MUTE_TOGGLE;
>  	else
>  		vol->val_pending += dir;
> -	if (!task_add(systq, &sc->wskbd_task))
> +	if (sc->quiesce || !task_add(systq, &sc->wskbd_task))
>  		device_unref(&sc->dev);
>  	return 0;
>  }
> 
>