[PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

Newer Thinkpad models comes with a mute hotkey with a mute led. It also comes
with ACPI AML functions to control hardware mute + led. The patches add sysfs
nodes so it is possible to enable, disable and control from user-space.


Alex Hung (2):
  thinkpad-acpi: add sysfs for getting and setting hardware mute
  thinkpad-acpi: add sysfs for enabling and disabling hardware mute

 drivers/platform/x86/thinkpad_acpi.c | 77 ++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

-- 
1.8.1.2

[PATCH 1/2] thinkpad-acpi: add sysfs for getting and setting hardware mute

[Alex Hung]

Newer thinkpad models have a mute hotkey with a led to indicate
current mute states. Thinkpad BIOS provides ACPI interfaces for
getting and changing the hardware mute along with the led.

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 50 ++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 54d31c0..3b648c0 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2929,6 +2929,55 @@ static void hotkey_wakeup_hotunplug_complete_notify_change(void)
 		     "wakeup_hotunplug_complete");
 }
 
+/* sysfs hotkey_mute_state --------------------------------------------- */
+enum {
+	TPACPI_AML_MUTE_READ_MASK = 0x01,
+	TPACPI_AML_MUTE_ERROR_STATE_MASK = 0x80000000,
+};
+
+static ssize_t hotkey_mute_state_show(struct device *dev,
+			   struct device_attribute *attr,
+			   char *buf)
+{
+	int state;
+
+	if (!acpi_evalf(hkey_handle, &state, "GSMS", "dd"))
+		return -EIO;
+
+	if (state & TPACPI_AML_MUTE_ERROR_STATE_MASK)
+		pr_warn("getting mute state failed.\n");
+
+	state &= TPACPI_AML_MUTE_READ_MASK;
+
+	return snprintf(buf, PAGE_SIZE, "%x\n", state);
+}
+
+static ssize_t hotkey_mute_state_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned long state;
+	int output;
+
+	if (parse_strtoul(buf, 25, &state))
+		return -EINVAL;
+
+	if (state != 1 && state != 0)
+		return -EINVAL;
+
+	if (!acpi_evalf(hkey_handle, &output, "SSMS", "dd", state))
+		return -EIO;
+
+	if (output & TPACPI_AML_MUTE_ERROR_STATE_MASK)
+		return -EIO;
+
+	return count;
+}
+
+static struct device_attribute dev_attr_hotkey_mute_state =
+	__ATTR(hotkey_mute_state, S_IWUSR | S_IRUGO,
+	hotkey_mute_state_show, hotkey_mute_state_store);
+
 /* --------------------------------------------------------------------- */
 
 static struct attribute *hotkey_attributes[] __initdata = {
@@ -2945,6 +2994,7 @@ static struct attribute *hotkey_attributes[] __initdata = {
 	&dev_attr_hotkey_source_mask.attr,
 	&dev_attr_hotkey_poll_freq.attr,
 #endif
+	&dev_attr_hotkey_mute_state.attr,
 };
 
 /*
-- 
1.8.1.2

[PATCH 2/2] thinkpad-acpi: add sysfs for enabling and disabling hardware mute

[Alex Hung]

Thinkpad BIOS provides an ACPI interface to disable hardware mute.
When enabled (as default), Thinkpad issues a scancode and toggles
hardware mute + led. Once disabled, Thinkpad will only issues the
scancode.

When hardware mute is enabled, hardware mute can be out-of-sync
with user-space if a user changes mute state by user application.
This patch gives a chance to disable hardware mute.

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 3b648c0..a901fac 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2978,6 +2978,32 @@ static struct device_attribute dev_attr_hotkey_mute_state =
 	__ATTR(hotkey_mute_state, S_IWUSR | S_IRUGO,
 	hotkey_mute_state_show, hotkey_mute_state_store);
 
+/* sysfs hotkey_mute_enable -------------------------------------------- */
+static ssize_t hotkey_mute_enable_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned long state;
+	int output;
+
+	if (parse_strtoul(buf, 25, &state))
+		return -EINVAL;
+
+	if (state != 1 && state != 0)
+		return -EINVAL;
+
+	if (!acpi_evalf(hkey_handle, &output, "SHDA", "dd", state))
+		return -EIO;
+
+	if (output & TPACPI_AML_MUTE_ERROR_STATE_MASK)
+		return -EIO;
+
+	return count;
+}
+
+static struct device_attribute dev_attr_hotkey_mute_enable =
+	__ATTR(hotkey_mute_enable, S_IWUSR, NULL, hotkey_mute_enable_store);
+
 /* --------------------------------------------------------------------- */
 
 static struct attribute *hotkey_attributes[] __initdata = {
@@ -2995,6 +3021,7 @@ static struct attribute *hotkey_attributes[] __initdata = {
 	&dev_attr_hotkey_poll_freq.attr,
 #endif
 	&dev_attr_hotkey_mute_state.attr,
+	&dev_attr_hotkey_mute_enable.attr,
 };
 
 /*
-- 
1.8.1.2

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 13:40 +0800, Alex Hung wrote:
> Newer Thinkpad models comes with a mute hotkey with a mute led. It also comes
> with ACPI AML functions to control hardware mute + led. The patches add sysfs
> nodes so it is possible to enable, disable and control from user-space.

What's expected to use this interface? If it's supposed to be some
system wide configuration at boot time, we should just add a config
option to the kernel and let it be overridden by module parameter.

-- 
Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

[-- Attachment #1.1: Type: text/plain, Size: 1092 bytes --]

Hi Matthew,

The scenario will be to have an user-application to listen to the mute key
event, compare to hardware mute against OS mute, and sync both mute. When
the application is loaded, it can use this interface to enable it; when it
is unloaded it can disable it.

A config option is also a good idea. I will develop another patch.

Cheers,
Alex Hung



On Wed, Aug 14, 2013 at 1:42 PM, Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org
> wrote:

> On Wed, 2013-08-14 at 13:40 +0800, Alex Hung wrote:
> > Newer Thinkpad models comes with a mute hotkey with a mute led. It also
> comes
> > with ACPI AML functions to control hardware mute + led. The patches add
> sysfs
> > nodes so it is possible to enable, disable and control from user-space.
>
> What's expected to use this interface? If it's supposed to be some
> system wide configuration at boot time, we should just add a config
> option to the kernel and let it be overridden by module parameter.
>
> --
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
>



-- 
Cheers,
Alex Hung

[-- Attachment #1.2: Type: text/html, Size: 2075 bytes --]

[-- Attachment #2: Type: text/plain, Size: 405 bytes --]

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 13:53 +0800, Alex Hung wrote:
> Hi Matthew,
> 
> 
> The scenario will be to have an user-application to listen to the mute
> key event, compare to hardware mute against OS mute, and sync both
> mute. When the application is loaded, it can use this interface to
> enable it; when it is unloaded it can disable it.

Is there any reason you'd want this rather than just having it generate
a scancode and be under userspace control?

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

[-- Attachment #1.1: Type: text/plain, Size: 1359 bytes --]

We are seeing bugs that mute led is out-of-sync with OS's mute.
Steps to duplicate:
1. Press mute hotkey - it generates scancode and turns on mute led
2. Unmute by desktop application.

Expected result: System is un-muted and led is turned off
Actual result: System, depending on models, can be muted or unmuted. The
led is always on.

One solution to the problem is to disable hardware mute. Mute hotkey will
only generate scancode but led will not be toggled.

The other solution will be to develop an application or improve existing
desktop daemons to handle the event as previously discussed, but it will
need to have the sysfs nodes for mute/led controls.


On Wed, Aug 14, 2013 at 1:54 PM, Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org
> wrote:

> On Wed, 2013-08-14 at 13:53 +0800, Alex Hung wrote:
> > Hi Matthew,
> >
> >
> > The scenario will be to have an user-application to listen to the mute
> > key event, compare to hardware mute against OS mute, and sync both
> > mute. When the application is loaded, it can use this interface to
> > enable it; when it is unloaded it can disable it.
>
> Is there any reason you'd want this rather than just having it generate
> a scancode and be under userspace control?
>
> --
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
>



-- 
Cheers,
Alex Hung

[-- Attachment #1.2: Type: text/html, Size: 2640 bytes --]

[-- Attachment #2: Type: text/plain, Size: 405 bytes --]

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 14:11 +0800, Alex Hung wrote:

> One solution to the problem is to disable hardware mute. Mute hotkey
> will only generate scancode but led will not be toggled.

So this works, but muting doesn't control the LED? It seems like this is
something that ought to be fixed in-kernel as well. Having an
application to make the LED work isn't a terribly generic solution.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

[-- Attachment #1.1: Type: text/plain, Size: 1619 bytes --]

On Wed, Aug 14, 2013 at 2:11 PM, Alex Hung <alex.hung-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:

> We are seeing bugs that mute led is out-of-sync with OS's mute.
> Steps to duplicate:
> 1. Press mute hotkey - it generates scancode and turns on mute led
> 2. Unmute by desktop application.
>
> Expected result: System is un-muted and led is turned off
> Actual result: System, depending on models, can be muted or unmuted. The
> led is always on.
>
> One solution to the problem is to disable hardware mute. Mute hotkey will
> only generate scancode but led will not be toggled.
>

The above can be addressed by a kernel parameter during boot time.



>
> The other solution will be to develop an application or improve existing
> desktop daemons to handle the event as previously discussed, but it will
> need to have the sysfs nodes for mute/led controls.
>
>
> On Wed, Aug 14, 2013 at 1:54 PM, Matthew Garrett <
> matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>
>> On Wed, 2013-08-14 at 13:53 +0800, Alex Hung wrote:
>> > Hi Matthew,
>> >
>> >
>> > The scenario will be to have an user-application to listen to the mute
>> > key event, compare to hardware mute against OS mute, and sync both
>> > mute. When the application is loaded, it can use this interface to
>> > enable it; when it is unloaded it can disable it.
>>
>> Is there any reason you'd want this rather than just having it generate
>> a scancode and be under userspace control?
>>
>> --
>> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
>>
>
>
>
> --
> Cheers,
> Alex Hung
>



-- 
Cheers,
Alex Hung

[-- Attachment #1.2: Type: text/html, Size: 3465 bytes --]

[-- Attachment #2: Type: text/plain, Size: 405 bytes --]

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

[-- Attachment #1.1: Type: text/plain, Size: 913 bytes --]

Hi Matthew,

The problem is that thinkpad-acpi does not aware of mute is changed by
either hotkey presses or in desktop.

I believe Lenovo has an application to handle it Windows in similar ways,
and the design makes controlling by user-applications more feasible.

Cheers,
Alex Hung


On Wed, Aug 14, 2013 at 2:14 PM, Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org
> wrote:

> On Wed, 2013-08-14 at 14:11 +0800, Alex Hung wrote:
>
> > One solution to the problem is to disable hardware mute. Mute hotkey
> > will only generate scancode but led will not be toggled.
>
> So this works, but muting doesn't control the LED? It seems like this is
> something that ought to be fixed in-kernel as well. Having an
> application to make the LED work isn't a terribly generic solution.
>
> --
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
>



-- 
Cheers,
Alex Hung

[-- Attachment #1.2: Type: text/html, Size: 1678 bytes --]

[-- Attachment #2: Type: text/plain, Size: 405 bytes --]

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 14:41 +0800, Alex Hung wrote:
> Hi Matthew,
> 
> The problem is that thinkpad-acpi does not aware of mute is changed by
> either hotkey presses or in desktop. 

Ok, how about this. Register the LED with the kernel LED subsystem. Add
a new LED trigger for volume-mute. Have the HDA driver call that trigger
whenever the mute state changes.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

[-- Attachment #1.1: Type: text/plain, Size: 672 bytes --]

Hi Mattew,

That sounds good. I will work on it.

Cheers,
Alex Hung


On Wed, Aug 14, 2013 at 3:00 PM, Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org
> wrote:

> On Wed, 2013-08-14 at 14:41 +0800, Alex Hung wrote:
> > Hi Matthew,
> >
> > The problem is that thinkpad-acpi does not aware of mute is changed by
> > either hotkey presses or in desktop.
>
> Ok, how about this. Register the LED with the kernel LED subsystem. Add
> a new LED trigger for volume-mute. Have the HDA driver call that trigger
> whenever the mute state changes.
>
> --
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
>



-- 
Cheers,
Alex Hung

[-- Attachment #1.2: Type: text/html, Size: 1431 bytes --]

[-- Attachment #2: Type: text/plain, Size: 405 bytes --]

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Alex Hung]

Added in David and Tim who also has some ideas to share.

On Wed, Aug 14, 2013 at 3:00 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Wed, 2013-08-14 at 14:41 +0800, Alex Hung wrote:
>> Hi Matthew,
>>
>> The problem is that thinkpad-acpi does not aware of mute is changed by
>> either hotkey presses or in desktop.
>
> Ok, how about this. Register the LED with the kernel LED subsystem. Add
> a new LED trigger for volume-mute. Have the HDA driver call that trigger
> whenever the mute state changes.
>
> --
> Matthew Garrett <matthew.garrett@nebula.com>



-- 
Cheers,
Alex Hung

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

(Added alsa-devel to CC)

On 08/14/2013 09:16 AM, Alex Hung wrote:
> Added in David and Tim who also has some ideas to share.
> 
> On Wed, Aug 14, 2013 at 3:00 PM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
>> On Wed, 2013-08-14 at 14:41 +0800, Alex Hung wrote:
>>> Hi Matthew,
>>>
>>> The problem is that thinkpad-acpi does not aware of mute is changed by
>>> either hotkey presses or in desktop.
>>
>> Ok, how about this. Register the LED with the kernel LED subsystem. Add
>> a new LED trigger for volume-mute. Have the HDA driver call that trigger
>> whenever the mute state changes.
>>
>> --
>> Matthew Garrett <matthew.garrett@nebula.com>
> 
> 
> 

So, to take a wider grip on this issue.

First, the hardware mute. Because this is directly connected to the
internal speaker (and headphones?), it needs to turn into a ALSA
kcontrol on the sound card that controls speaker and/or headphones. And
it needs to named accordingly, e g "Speaker Playback Switch" if it
controls the speaker only.
Exactly how this is going to work out given the arbitrary module load
order of snd-hda-intel and thinkpad-acpi, I'm not sure, but the way of
creating another sound card (which I've seen on some thinkpads) just
isn't working out for userspace.

Second, about the mute LEDs and mic-mute LEDs, we currently have several
interfaces to choose from:
 a) HP LEDs currently uses a ALSA kcontrol to control them. For Mute LED
there is also a limited automatic control of this LED from kernel space
(by default), so it follows the status of "Master Playback Switch" IIRC.
 b) thinkpad-acpi currently uses custom sysfs nodes to control the LEDs.
 c) and we also have the /sys/class/leds interface.
There might be even more solutions.

It's important we get a consistent interfaces towards userspace for the
LEDs. And we should also make sure we don't have a permissions problem:
if we want the mute/mic-mute LED to follow the status of the currently
selected sound card in PulseAudio, which IMO should be our goal, writes
cannot be root-only.
To have the HDA driver call into the LED subsystem is better than
nothing, but in the end we would want the LEDs to follow whatever sound
input/output the user currently uses, whether that is bluetooth, USB,
HDMI, or internal audio.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 09:43 +0200, David Henningsson wrote:

> First, the hardware mute. Because this is directly connected to the
> internal speaker (and headphones?), it needs to turn into a ALSA
> kcontrol on the sound card that controls speaker and/or headphones. And
> it needs to named accordingly, e g "Speaker Playback Switch" if it
> controls the speaker only.
> Exactly how this is going to work out given the arbitrary module load
> order of snd-hda-intel and thinkpad-acpi, I'm not sure, but the way of
> creating another sound card (which I've seen on some thinkpads) just
> isn't working out for userspace.

Sure. There was arguably a benefit in exposing the hardware mixer back
in the *40 and earlier machines, since volume was also under hardware
control back then. The mute support is somewhat vestigial, and I don't
think trying to tie into it is going to make things better.

> Second, about the mute LEDs and mic-mute LEDs, we currently have several
> interfaces to choose from:
>  a) HP LEDs currently uses a ALSA kcontrol to control them. For Mute LED
> there is also a limited automatic control of this LED from kernel space
> (by default), so it follows the status of "Master Playback Switch" IIRC.

HDA has support for mute LEDs that are tied to GPIO lines on the codec -
is this the extent of it?

>  b) thinkpad-acpi currently uses custom sysfs nodes to control the LEDs.

Right.

>  c) and we also have the /sys/class/leds interface.

Yeah.

> It's important we get a consistent interfaces towards userspace for the
> LEDs. And we should also make sure we don't have a permissions problem:
> if we want the mute/mic-mute LED to follow the status of the currently
> selected sound card in PulseAudio, which IMO should be our goal, writes
> cannot be root-only.

I'm not convinced that should be our goal - if the mic mute light is on,
I'd expect *all* mics to be muted, since the point of the light there is
something of a privacy guard. It's not quite as clear that I'd expect
the mute LED to bind to everything. However, given a single "currently
selected sound card", there's no obviously user-visible difference
between muting the current device and muting all devices.

Given the privacy concerns around the microphone LED, I think any
exposed LED would have to be either under kernel control. Having a
compromised component of a user session be able to record audio while
leaving the LED lit would be a problem.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

On 08/14/2013 09:51 AM, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 09:43 +0200, David Henningsson wrote:
> 
>> First, the hardware mute. Because this is directly connected to the
>> internal speaker (and headphones?), it needs to turn into a ALSA
>> kcontrol on the sound card that controls speaker and/or headphones. And
>> it needs to named accordingly, e g "Speaker Playback Switch" if it
>> controls the speaker only.
>> Exactly how this is going to work out given the arbitrary module load
>> order of snd-hda-intel and thinkpad-acpi, I'm not sure, but the way of
>> creating another sound card (which I've seen on some thinkpads) just
>> isn't working out for userspace.
> 
> Sure. There was arguably a benefit in exposing the hardware mixer back
> in the *40 and earlier machines, since volume was also under hardware
> control back then. The mute support is somewhat vestigial, and I don't
> think trying to tie into it is going to make things better.

You seem to know the history better than I do, but if there are
significant power savings or some faint noise removed (e g by turning an
amplifier off or so) by using the hardware mute, then it could still be
worth pursuing.

>> Second, about the mute LEDs and mic-mute LEDs, we currently have several
>> interfaces to choose from:
>>  a) HP LEDs currently uses a ALSA kcontrol to control them. For Mute LED
>> there is also a limited automatic control of this LED from kernel space
>> (by default), so it follows the status of "Master Playback Switch" IIRC.
> 
> HDA has support for mute LEDs that are tied to GPIO lines on the codec -
> is this the extent of it?

Yes, essentially. HDA has support for GPIOs, and HP on IDT codecs use
that to control LEDs. HP on Realtek codecs (not as common) use VREF
outputs on unused mic pins.

>>  b) thinkpad-acpi currently uses custom sysfs nodes to control the LEDs.
> 
> Right.
> 
>>  c) and we also have the /sys/class/leds interface.
> 
> Yeah.
> 
>> It's important we get a consistent interfaces towards userspace for the
>> LEDs. And we should also make sure we don't have a permissions problem:
>> if we want the mute/mic-mute LED to follow the status of the currently
>> selected sound card in PulseAudio, which IMO should be our goal, writes
>> cannot be root-only.
> 
> I'm not convinced that should be our goal - if the mic mute light is on,
> I'd expect *all* mics to be muted, since the point of the light there is
> something of a privacy guard. 

I see your point, but the question is what is more of a privacy guard
here. Do you expect USB mics and Bluetooth mics/headsets to be muted too?

If so, are you expecting some kind of kernel-wide block of recording,
including bluetooth, USB, and other sound cards? It does not sound like
a practical approach to me.

And if not, where do you draw the line and why?

> It's not quite as clear that I'd expect
> the mute LED to bind to everything. However, given a single "currently
> selected sound card", there's no obviously user-visible difference
> between muting the current device and muting all devices.
> 
> Given the privacy concerns around the microphone LED, I think any
> exposed LED would have to be either under kernel control. Having a
> compromised component of a user session be able to record audio while
> leaving the LED lit would be a problem.

To decide what the LEDs bind to is a design issue. I was thinking that
by giving control to userspace would give different
implementations/distros/etc the most freedom to do what design they wanted.

However, since, the hotkeys and the mutes are often very closely related
physically (e g the mute LED is physically located on the mute button),
I would expect them to follow each other. And the mute hotkey mutes the
current default PulseAudio output, so I'd expect the mic mute hotkey to
mute the current default PulseAudio input, and the LEDs to update
according to those mute states.

The privacy issue is interesting, but I don't see a practical way of
implementing things that would protect us against compromised userspaces.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 11:27 +0200, David Henningsson wrote:

> The privacy issue is interesting, but I don't see a practical way of
> implementing things that would protect us against compromised userspaces.

That's pretty easy - just tie the LED control to the HDA device in-kernel.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

On 08/14/2013 04:57 PM, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 11:27 +0200, David Henningsson wrote:
> 
>> The privacy issue is interesting, but I don't see a practical way of
>> implementing things that would protect us against compromised userspaces.
> 
> That's pretty easy - just tie the LED control to the HDA device in-kernel.
> 

Well, my point was that the compromised userspace could still record
from other possibly connected microphones (such as USB or bluetooth
headsets).

But I guess one compromise could be to refuse userspace turn the mic
mute LED on, if the internal mic is unmuted.
Userspace would still be able to turn the mic mute LED off, to indicate
that recording can happen from other sources. It will be slightly more
complex for userspace though.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 21:53 +0200, David Henningsson wrote:
> On 08/14/2013 04:57 PM, Matthew Garrett wrote:
> > On Wed, 2013-08-14 at 11:27 +0200, David Henningsson wrote:
> > 
> >> The privacy issue is interesting, but I don't see a practical way of
> >> implementing things that would protect us against compromised userspaces.
> > 
> > That's pretty easy - just tie the LED control to the HDA device in-kernel.
> > 
> 
> Well, my point was that the compromised userspace could still record
> from other possibly connected microphones (such as USB or bluetooth
> headsets).

Ok, true. It'd need to be at a higher level than HDA to make this
consistent. But in theory, this is something that could be tied to the
full set of input sources.

> But I guess one compromise could be to refuse userspace turn the mic
> mute LED on, if the internal mic is unmuted.
> Userspace would still be able to turn the mic mute LED off, to indicate
> that recording can happen from other sources. It will be slightly more
> complex for userspace though.

Like I said, I don't think there's any reason for this to be in
userspace. If it's only going to represent the internal device, that's
trivial. If it's going to represent the global state of audio devices,
that's more complicated but doable. The only policy aspect is "What if I
want it to track the state of the currently selected audio device",
which just doesn't seem like a useful thing for it to do.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

On 08/14/2013 10:05 PM, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 21:53 +0200, David Henningsson wrote:
>> On 08/14/2013 04:57 PM, Matthew Garrett wrote:
>>> On Wed, 2013-08-14 at 11:27 +0200, David Henningsson wrote:
>>>
>>>> The privacy issue is interesting, but I don't see a practical way of
>>>> implementing things that would protect us against compromised userspaces.
>>>
>>> That's pretty easy - just tie the LED control to the HDA device in-kernel.
>>>
>>
>> Well, my point was that the compromised userspace could still record
>> from other possibly connected microphones (such as USB or bluetooth
>> headsets).
> 
> Ok, true. It'd need to be at a higher level than HDA to make this
> consistent. But in theory, this is something that could be tied to the
> full set of input sources.

In theory perhaps, but in practice, I doubt it. USB audio perhaps,
because that's at least ALSA. Bluetooth uses its own stack (it's not
even ALSA), and I'm not sure how much of that stack is in the kernel.

And then we haven't considered other oddities such as FFADO, network
connected devices and what not...but perhaps we could leave those out?

>> But I guess one compromise could be to refuse userspace turn the mic
>> mute LED on, if the internal mic is unmuted.
>> Userspace would still be able to turn the mic mute LED off, to indicate
>> that recording can happen from other sources. It will be slightly more
>> complex for userspace though.
> 
> Like I said, I don't think there's any reason for this to be in
> userspace. If it's only going to represent the internal device, that's
> trivial. If it's going to represent the global state of audio devices,
> that's more complicated but doable. The only policy aspect is "What if I
> want it to track the state of the currently selected audio device",
> which just doesn't seem like a useful thing for it to do.

I say it's the *most* useful thing for it to do.

Imagine you're a non-technical user, who has never heard the words
"compromised userspace". You connect your headset where it fits (or
cordless), and then select the headset in sound settings (if it didn't
get selected for you when you plugged it in). You're on a VOIP call and
press the mic mute hotkey. Which mic did you expect to mute? The
selected one. On the mic mute hotkey button, there is also a LED. You
expect it to lit, because you muted the mic that you currently care
about, i e, the selected one.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 22:36 +0200, David Henningsson wrote:

> Imagine you're a non-technical user, who has never heard the words
> "compromised userspace". You connect your headset where it fits (or
> cordless), and then select the headset in sound settings (if it didn't
> get selected for you when you plugged it in). You're on a VOIP call and
> press the mic mute hotkey. Which mic did you expect to mute? The
> selected one. On the mic mute hotkey button, there is also a LED. You
> expect it to lit, because you muted the mic that you currently care
> about, i e, the selected one.

The user hit the mute key. Why would they expect *anything* to be
unmuted?

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

On 08/14/2013 10:38 PM, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 22:36 +0200, David Henningsson wrote:
> 
>> Imagine you're a non-technical user, who has never heard the words
>> "compromised userspace". You connect your headset where it fits (or
>> cordless), and then select the headset in sound settings (if it didn't
>> get selected for you when you plugged it in). You're on a VOIP call and
>> press the mic mute hotkey. Which mic did you expect to mute? The
>> selected one. On the mic mute hotkey button, there is also a LED. You
>> expect it to lit, because you muted the mic that you currently care
>> about, i e, the selected one.
> 
> The user hit the mute key. Why would they expect *anything* to be
> unmuted?
> 

Why should the userspace application, who just wants to lit a LED, have
to care about a lot of other sound cards and interfaces and mute them,
when the user does not care?

And what about multiseat setups? If a multiseat keyboard has a mic mute
LED, do you think another user's mic mute state should influence the LED
of your keyboard?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Matthew Garrett]

On Wed, 2013-08-14 at 22:59 +0200, David Henningsson wrote:
> On 08/14/2013 10:38 PM, Matthew Garrett wrote:
> > The user hit the mute key. Why would they expect *anything* to be
> > unmuted?
> > 
> 
> Why should the userspace application, who just wants to lit a LED, have
> to care about a lot of other sound cards and interfaces and mute them,
> when the user does not care?

Because there's no interface for the userspace application to light an
LED. Henrique has previously said that he doesn't want userspace to have
arbitrary control over it, and I agree with him.

> And what about multiseat setups? If a multiseat keyboard has a mic mute
> LED, do you think another user's mic mute state should influence the LED
> of your keyboard?

Once we see an external keyboard with a microphone mute LED, that's
certainly something we can worry about.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Henrique de Moraes Holschuh]

On Wed, 14 Aug 2013, Alex Hung wrote:
> That sounds good. I will work on it.

AND that sounds like what I've asked people to do the last time the "user
privacy" LEDs (MIC mute, sound mute) topic was raised, so I am also all for
it.

I don't know if we should allow anything but the alsa layer to toggle those
leds, though (at least without a module parameter).  It would be really nice
if, by default, the LED was permanently locked to the mixer hardware state
with no userspace override possible.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk

Re: [alsa-devel] [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[Arun Raghavan]

On Wed, 2013-08-14 at 07:51 +0000, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 09:43 +0200, David Henningsson wrote:
> 
> > First, the hardware mute. Because this is directly connected to the
> > internal speaker (and headphones?), it needs to turn into a ALSA
> > kcontrol on the sound card that controls speaker and/or headphones. And
> > it needs to named accordingly, e g "Speaker Playback Switch" if it
> > controls the speaker only.
> > Exactly how this is going to work out given the arbitrary module load
> > order of snd-hda-intel and thinkpad-acpi, I'm not sure, but the way of
> > creating another sound card (which I've seen on some thinkpads) just
> > isn't working out for userspace.
> 
> Sure. There was arguably a benefit in exposing the hardware mixer back
> in the *40 and earlier machines, since volume was also under hardware
> control back then. The mute support is somewhat vestigial, and I don't
> think trying to tie into it is going to make things better.
> 
> > Second, about the mute LEDs and mic-mute LEDs, we currently have several
> > interfaces to choose from:
> >  a) HP LEDs currently uses a ALSA kcontrol to control them. For Mute LED
> > there is also a limited automatic control of this LED from kernel space
> > (by default), so it follows the status of "Master Playback Switch" IIRC.
> 
> HDA has support for mute LEDs that are tied to GPIO lines on the codec -
> is this the extent of it?
> 
> >  b) thinkpad-acpi currently uses custom sysfs nodes to control the LEDs.
> 
> Right.
> 
> >  c) and we also have the /sys/class/leds interface.
> 
> Yeah.
> 
> > It's important we get a consistent interfaces towards userspace for the
> > LEDs. And we should also make sure we don't have a permissions problem:
> > if we want the mute/mic-mute LED to follow the status of the currently
> > selected sound card in PulseAudio, which IMO should be our goal, writes
> > cannot be root-only.
> 
> I'm not convinced that should be our goal - if the mic mute light is on,
> I'd expect *all* mics to be muted, since the point of the light there is
> something of a privacy guard. It's not quite as clear that I'd expect
> the mute LED to bind to everything. However, given a single "currently
> selected sound card", there's no obviously user-visible difference
> between muting the current device and muting all devices.
> 
> Given the privacy concerns around the microphone LED, I think any
> exposed LED would have to be either under kernel control. Having a
> compromised component of a user session be able to record audio while
> leaving the LED lit would be a problem.

The privacy concerns valid, but they're not insurmountable. You could
make userspace listen to the LED status and react as if the mute button
was pressed, making sure the two can never really go out of sync (just
an example off the top of my head).

Even if you don't do that, IMO it makes sense to expose the LEDs to
userspace. We're effectively talking about how the LEDs should behave as
an extension of the UI, and freezing the policy that governs that UI in
userspace is pretty inflexible.

-- Arun

Re: [PATCH 0/2] add sysfs for acpi interfaces of thinkpad hardware mute

[David Henningsson]

On 08/14/2013 11:15 PM, Matthew Garrett wrote:
> On Wed, 2013-08-14 at 22:59 +0200, David Henningsson wrote:
>> On 08/14/2013 10:38 PM, Matthew Garrett wrote:
>>> The user hit the mute key. Why would they expect *anything* to be
>>> unmuted?
>>>
>>
>> Why should the userspace application, who just wants to lit a LED, have
>> to care about a lot of other sound cards and interfaces and mute them,
>> when the user does not care?
> 
> Because there's no interface for the userspace application to light an
> LED. Henrique has previously said that he doesn't want userspace to have
> arbitrary control over it, and I agree with him.

Trying to figure out the mute status of all the inputs is not an option,
due to the extreme diversity of audio hardware.

Having the LED follow the mute status of the internal mic is doable, but
IMO, it severely limits the usability of the LED.

I still think the better solution would be to let userspace handle it,
but anyway, this question is not thinkpad-acpi specific. So maybe we
need to make it a /sys/class/led then for the time being - that would at
least be a uniform interface.

>> And what about multiseat setups? If a multiseat keyboard has a mic mute
>> LED, do you think another user's mic mute state should influence the LED
>> of your keyboard?
> 
> Once we see an external keyboard with a microphone mute LED, that's
> certainly something we can worry about.

Well, a quick google search came up with this one [1]. But never mind -
this argument was just to show that the "mute status of all inputs" idea
is bad, and that's not happening anyway.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[1]
http://www.bloomberg.com/professional/files/2012/10/bloomberg_keyboard_2_installation_guide.pdf