Hi, On 3/1/21 7:55 PM, Mark Brown wrote: > On Fri, Feb 26, 2021 at 03:38:15PM +0100, Hans de Goede wrote: >> When using AIF1 the 'DAC1 Playback Volume' control will be used as the >> PlaybackMasterElem in UCM. >> >> We need a matching 'DAC1 Playback Switch' for 2 reasons: >> >> 1. To be able to truely fully mute the output (the softest volume setting >> is not fully muted). >> 2. For reliable output-mute LED control. >> >> The path from the IF1_DAC data input to the 'Stereo DAC MIXL' / >> 'Stereo DAC MIXR' digital mixer has a 'DAC MIXL' / 'DAC MIXR' digital >> mixer with IF1_DAC data as one of its inputs direclty after the >> 'DAC1 Playback Volume' control. >> >> This commit adds an emulated "DAC1 Playback Switch" control by: > > 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. The inputs to those mixer are all 4 (2 pairs of l/r) controlled by mute bits. The codec designer has left out the mute switches normally directly following the volume control since then there would be 2 mute switches in series, one as part of the volume-control/mute combo which is usually used and 1 directly behind that to mute/unmute the mixer input. 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" > 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. Also problematic here is that things like volume-controls and their accompanying mute switches (often bit 15 + 8 of the same register) are modules as "normal" mixer elements which are not seen as part of the DAPM graph, where as the mixer in this case is part of the DAPM graph. > 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. > 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 open by userspace). AFAIK this makes them unsuitable to be used as a source for the mute-led trigger, we want the mute LED to turn on when the volume control is set to mute, not when the last app closes the audio device. 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. Regards, Hans