Re: [PATCH] [thinkpad-acpi] Add T410s and T420s LED support

[Jens Taprogge <jens.taprogge@taprogge.org>]

On Fri, Mar 23, 2012 at 11:50:38PM -0300, Henrique de Moraes Holschuh wrote:
> On Sat, 24 Mar 2012, Jens Taprogge wrote:
> > I am not sure thinkpad_acpi needs to provide a mixer.  I think what is
> 
> It does, but not on all thinkpads.  AFAIK, even the newer ones that are not
> OEM junk with a thin layer of thinkpad IP added still have a FET mute-gating
> the speakers, controlled by the EC.

Sorry, I have been imprecise.  I am not questioning the need for the
output mixer.  The models I have looked at all feature the EC controlled
hardware muting.  What I was trying to say was that there does not
necessarily need to be a mixer for the mic to properly report the muting
state.

> 
> > important is that the user can rely on the LED to be only on if the mic
> > is muted.  The mute key can be handled by userspace as long as the led
> > always gives the correct information.
> 
> And as long as it is not in control of the firmware.  Otherwise, bad things
> happen.
Agreed. 

> > The modern laptops are all based on HDA audio.  In the BIOS the
> > different functions of the respective connections are stored.  So the
> > sound layer knows which connection is the internal mic.  (An after
> > analyzing the information provided on the Thinkpads seems to be correct
> > in this regard.)
> 
> Have you tested and made sure Lenovo did not mute-gate the MIC through the
> EC on the better thinkpad lines?

I have tested pressing the mute key as well as switching the LED on the
T410s and T420s.  The mic stays open.  

> 
> > So to make sure the LED presents the actual state of the muting it would
> > be sufficient for ALSA to expose the mixer state to thinkpad_acpi or
> > better call a callback on changes.  To make it applicable to a wider
> > range of devices one could come up with a design like this:
> > 
> >    #define SND_ENABLED_INTMIC      1 << 0
> >    #define SND_ENABLED_INT_SPEAKER 1 << 16
> >    #define SND_ENABLED_ALLINPUTS   0x0000ffff
> >    #define SND_ENABLED_ALLOUTPUTS  0xffff0000
> > 
> >    /* returns current state or -1 for error */
> >    int snd_register_mute_cb(void (*callback)(int muted, void *private), void *private);
> >    int snd_unregister_mute_cb(void (*callback)(int muted, void *private));
> > 
> > This would give any other module the ability to receive a notification
> > when a device is muted.  The internal mic is special since the user can
> > not unplug it.  So it gets a defined position in the array.  Apart from
> > that one could use the LED to display if any mic is open.
> 
> If you're going to do it that way, why not provide LED triggers?

I just did not know about them.  Is there a way to prevent userspace
from changing the trigger -> led association?

Takashi:  where would we need to add the LED triggers? In
core/control.c?  Would such a change be acceptable to ALSA?