Hi, On 3/1/21 8:21 PM, Hans de Goede wrote: > 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. I think we might be conversion on a solution for this in the other thread (see the email which I am about to send but have not sent yet), so lets continue this discussion there. Regards, Hans
