On Mon, Mar 01, 2021 at 08:21:56PM +0100, Hans de Goede wrote:
This feels icky, it seems like if userspace needs to stitch together a stereo mute control that doesn't already exist in the hardware
But it does exist in the hardware the digital mixer bits around DAC1 have some more functionality then those around DAc2, including a mixer directly behind the DAC1 volume-control which allows digital loopback.
Given that there's other inputs to the mixer (what with it being a mixer and all) I'm not convinced about that?
This is a nice hw optimization, but annoying to deal with in software, esp. since userspace generally expects a "Foo Playback Volume", "Foo Playback Switch" pair. By for the easiest way to deal with this is to undo the hw optimization in software and add the expected "Foo Playback Switch"
Eh, some userspace might have that expectation but it doesn't really map onto general hardware designs.
from existing mono controls then UCM ought to have support for figuring that out anyway or if we *must* bodge this in the kernel there should be some generic way of doing it rather than open coding in drivers.
This is highly codec specific. So far this has not really been an issue because so far on asoc based systems regular Linux userspace has always been using software volume-control. But now that we are starting to use hardware volume-control it really is desirable to also have a hardware mute switch available.
There's a lot of things that would be desirable but aren't really realistic... there's a bunch of hardware that this model just plain doesn't map onto. I mentioned the wm5012 based systems in the other thread - that's a fairly clear example where a singular DAC mute control just doesn't correspond to the hardware design at all, it's got any to any routing throughout the device with DACs at the outputs.
It also makes the whole mute LED thing feel a lot messier even for existing systems than you seemed to be suggesting in the other thread. This device has two I2S interfaces, two DACs (only one of which seems to be affected by this control), and it appears that there's bypass path from the ADC to DAC1 which won't be muted by the newly added mute switch here so this reliable mute control won't be entirely reliable. There look to also be some analogue bypass paths, I didn't fully check.
I don't believe that it is necessary to take bypass / loopback paths into account for the playback mute LED. These are normally always off and they don't involve the normal playback paths. So even if they are on any audio played from within the OS is still muted.
For me I would say that if the mute LED is on and I can hear audio coming out of the system then there is a bug, probably also if I can manage to record audio possibly depending on labelling. This all very clearly seems to be pointing towards this being a policy decision which probably belongs in userspace.
One could equally argue that a software defined mute control should be muting all the analogue outputs, it'd certainly seem more robust.
The mute switches in the analog output paths are part of the DAPM graph, which means that they will get turned off automatically to save power when the audio device is not playing audio (is not kept
So there's not actually any mute switches on the outputs for this device then and it only has power controls? In general device will often have separate power and mute controls, especially with older VMID based devices but that's often carried through to ground referenced stuff. This is yet another example of how devices may not conform to random policy decisions userspace might want them to have.
I honestly don't understand your objections against the current set of patches for dealing with the mute-led trigger. Your main worry seems to be that this is not flexible enough, but it currently is all handled inside the kernel. So if more complex cases come up then we can easily adjust the code to deal with this, since there is no userspace API part to worry about. And if these more complex cases do require more userspace involvement then we can worry about that then (and not now) when we actually have a concrete example of what such a more complex setup could look like and thus also have something to actually design any userspace api for this around.
All I've seen of this is the ASoC bits of this, including this series and it's all fitting patterns that look like forcing policy decisions into the kernel in ways that are hard to review - look at this patch as an example of this, there's stuff like the handling of bypass paths which you're dismissing. You say "when we actually have concrete examples of what such a more complex setup could look like" but this very patch seems to be for a device that causes issues, and I keep pointing at the wm5102 and similar devices which just break this model.
You have a clear model of what you want to do on your systems and are trying to implement that but that model looks to have policy in it which a resonable system integrator could want to decide differently even when running on the same hardware. If it is all handled inside the kernel then it's a lot more painful to do anything about that than when it's handled in userspace.