[PATCH 0/5] ALSA: control - add generic LED trigger code

[Takashi Iwai]

On Fri, 12 Feb 2021 11:32:38 +0100,
Jaroslav Kysela wrote:
> 
> Dne 12. 02. 21 v 10:23 Takashi Iwai napsal(a):
> > On Thu, 11 Feb 2021 18:53:20 +0100,
> > Jaroslav Kysela wrote:
> >>
> >> Dne 11. 02. 21 v 18:15 Takashi Iwai napsal(a):
> >>
> >>>> Jaroslav Kysela (5):
> >>>>   ALSA: control - introduce snd_ctl_notify_one() helper
> >>>>   ALSA: control - add layer registration routines
> >>>>   ALSA: control - add generic LED trigger module as the new control
> >>>>     layer
> >>>>   ALSA: HDA - remove the custom implementation for the audio LED trigger
> >>>>   ALSA: control - add sysfs support to the LED trigger module
> >>
> >>> One thing I still miss from the picture is how to deal with the case
> >>> like AMD ACP.  It has no mixer control to bundle with the LED trigger.
> >>> Your idea is to make a (dummy) user element and tie the LED trigger
> >>> with it?
> >>
> >> Yes, the user-space code which guarantee the silence stream should create an
> >> user space control with the appropriate LED group access bits. The alsa-lib's
> >> softvol PCM plugin can do this silencing for example.
> > 
> > What control would it create?  In the case of softvol, it's a volume
> > control that really changes the volume.  For the mute LED, it's a
> > control turn on/off the mute?  If so, I wonder what makes better than
> > creating it from the kernel driver.  (Of course, we can list up like
> > "flexibility", etc, but it has a flip side of "complexity" and
> > "fragility"...)
> 
> The current code handles both switch / volume for the marked control (assuming
> that the minimal value - usually zero - is full mute). And actually, there are
> snd_pcm_areas_silence() calls in the softvol plugin, so we know that the PCM
> stream is not passed to the application at this point.
> 
> Condition:
> 
>       if (info.type == SNDRV_CTL_ELEM_TYPE_BOOLEAN ||
>           info.type == SNDRV_CTL_ELEM_TYPE_INTEGER)
>           ... value.value.integer.value[i] != info.value.integer.min
> 
> The softvol plugin may be extended to add the mute switch control, of course.

Well, my question was what kind of mixer control will be added at all,
although the chip does neither volume nor mute function.  Would we add
a fake volume/switch like softvol, or would we add rather a control
that is directly tied with the LED state?

> >>> Another slight concern is the possible regression: by moving the
> >>> mute-LED mode enum stuff into the sysfs, user will get
> >>> incompatibilities after the kernel update.  And it's not that trivial
> >>> to change the sysfs entry as default for each user.
> >>> It needs some detailed documentation or some temporary workaround
> >>> (e.g. keep providing the controls for now but warns if the value is
> >>> changed from the default value via the controls).
> >>
> >> I don't think that we have a user space application which is using those
> >> controls (Pulseaudio or so..) in an abstract way. I think that it's really
> >> minor issue. We should probably concentrate for the main designed purpose
> >> (notify about the mute / silent state) and handle those add-on features as an
> >> experimental stuff.
> > 
> > I'm sure that there are users of the reverse mic-mute LED ("follow
> > capture" mode); the feature was added because of the explicit request
> > from my colleague, and this mode works no matter whether ALSA native
> > or PA is used.
> I see. It looks like a corner case. The proposed sysfs based code is also user
> space independent. The issue with the HDA code is that it's card based, but
> the system wide LEDs should not be tied to one sound card. We are seeing that
> the hardware designers became very creative :-)

Heh, the flexiblity of the existing LED subsystem is already allowing
to be creative, although I haven't seen much exotic usage in the
reality (maybe the exception is the blinking at panic; IIRC, there is
Morse code conversion, too? :)


Takashi