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

[Takashi Iwai <tiwai@suse.de>]

At Sat, 24 Mar 2012 16:44:43 +0100,
Jens Taprogge wrote:
> 
> 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?

Well, in the style above, you can't know which device is the mute
stuff bound with.  What if you have a USB device in addition, for
example?

I'm not sure whether such a hard-binding in the kernel side is
inevitably necessary.  And, if it is, then we shouldn't expose the
control to user-space.  Or, just let user-space, such as PulseAudio,
working on it.

(Cc'ed David, who mentioned about micmute stuff in other threads.)


thanks,

Takashi