Re: Why doesn't mixer control (values) have some kind of locking mechanism? (mutex?)

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

[Adding Mark, Takashi and Jaroslav in CC: to make sure they see this thread]

On 8/5/20 12:31 PM, Tom Yan wrote:
> Hi all,
> 
> I just wonder if it's a "no one cares" or a "no one was aware of it"
> issue (or maybe both?).

none of the above, see below

> When you change (integer) values (e.g. volume) of a mixer control, it
> usually (if not always) involves calling two functions/methods of a
> snd_kcontrol_new, which are get and put, in order to do relative
> volume adjustments. (Apparently it is often done relatively even if we
> have absolute values, for reasons.)
> 
> While these two "actions" can be and probably are mostly "atomic"
> (with the help of mutex) in the kernel drivers *respectively*, they
> are not and cannot be atomic as a whole.
> 
> This won't really be an issue when the actions (either for one or
> multiple channels) are done "synchronously" in *one* program run (e.g.
> amixer -c STX set Master 1+). However, if such a program run is issued
> multiple times "asynchronously" (e.g. binding it to some
> XF86Audio{Raise,Lower}Volume scroll wheel), volume adjustment becomes
> a total mess / failure.
> 
> If it isn't obvious enough. it could happen like the following:
> get1(100 100)
> set1(101 100)
> get2(101 100)
> set2(102 100)
> ...
> 
> Or worse:
> get1(100 100)
> get2(100 100)
> set1(101 100)
> set2(100 101)
> ...
> 
> Not only that it may/will not finish the first set of adjustments for
> all channels before the second, get() from the second set could happen
> before set() from the first, reverting the effect of the earlier
> one(s).
> 
> Certainly one can use something like `flock` with amixer to make sure
> the atomicity of each issue/run, but not only that it looks silly and
> primitive, we don't always manipulate the mixer control with an
> "executable". For example, this weird issue in pulseaudio is probably
> related: https://bugs.freedesktop.org/show_bug.cgi?id=92717
> 
> So I wonder, is there a particular reason that mixer control doesn't
> possess some form of lock, which allows any form of userspace
> manipulation to lock it until what should be / is considered atomic is
> finished?

In the past on the Intel side we had a need for a control 'commit' 
operation, where a set of control values where passed all the way to DSP 
firmware, but were applied at the same time to avoid transients and 
inconsistent states. This was typically required when changing the 
orientation or changing the routing. We solved the problem by having a 
dedicated 'commit' control which gated all others, but there was no 
framework support for this.

These 'commit' operations are quite common in a number of 
specifications, e.g. OpenSL ES, and required when you update filter 
coefficients through independent transactions.

SoundWire 1.2 added support for dual-ranked registers which contains the 
'Current' and 'Next' value, and a commit mechanism to flip the two in 
synchronized ways. This will likely trickle in other hardware standards, 
and indeed I wonder if we should update the control/mixer framework - 
and possibly regmap as well?

It'd be a rather large plumbing exercise but it's worth it IMHO.
-Pierre