[PATCH 3/5] ASoC: rt5640: Add emulated 'DAC1 Playback Switch' control

[Mark Brown]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20210301/b8343836/attachment-0001.sig>