[PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[Alex Hung]

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

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 7b82868..dc22a4c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6486,12 +6486,86 @@ enum tpacpi_volume_capabilities {
 	TPACPI_VOL_CAP_MAX
 };
 
+enum  {
+	TPACPI_AML_MUTE_GET_FUNC = 0x01,
+	TPACPI_AML_MUTE_SET_FUNC = 0x02,
+	TPACPI_AML_MUTE_SUPPORT_FUNC = 0x04,
+	TPACPI_AML_MUTE_READ_MASK = 0x01,
+	TPACPI_AML_MUTE_ERROR_STATE_MASK = 0x80000000,
+};
+
 static enum tpacpi_volume_access_mode volume_mode =
 	TPACPI_VOL_MODE_MAX;
 
 static enum tpacpi_volume_capabilities volume_capabilities;
 static int volume_control_allowed;
 
+
+static bool volume_bios_support(int func)
+{
+	acpi_handle temp;
+
+	if ((func | TPACPI_AML_MUTE_GET_FUNC) &&
+	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "GSMS", &temp)))
+		return false;
+
+	if ((func | TPACPI_AML_MUTE_SET_FUNC) &&
+	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SSMS", &temp)))
+		return false;
+
+	if ((func | TPACPI_AML_MUTE_SUPPORT_FUNC) &&
+	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SHDA", &temp)))
+		return false;
+
+	return true;
+}
+
+static int hotkey_get_mute_state(int *state)
+{
+	if (!acpi_evalf(hkey_handle, state, "GSMS", "dd"))
+		return -EIO;
+
+	if (*state & TPACPI_AML_MUTE_ERROR_STATE_MASK)
+		pr_warning("getting mute state failed.\n");
+
+	*state &= TPACPI_AML_MUTE_READ_MASK;
+	pr_info("get mute state = %s.\n", *state ? "muted" : "unmuted");
+
+	return 0;
+}
+
+static int hotkey_set_mute_state(int state)
+{
+	int output;
+
+	if (!acpi_evalf(hkey_handle, &output, "SSMS", "dd", state))
+		return -EIO;
+
+	if (output & TPACPI_AML_MUTE_ERROR_STATE_MASK) {
+		pr_warning("setting mute state failed.\n");
+		return -EIO;
+	}
+	pr_info("set to mute led state to %s.\n", state ? "on" : "off");
+
+	return 0;
+}
+
+static int hotkey_set_mute_support(int support)
+{
+	int output;
+
+	if (!acpi_evalf(hkey_handle, &output, "SHDA", "dd", support))
+		return -EIO;
+
+	if (output & TPACPI_AML_MUTE_ERROR_STATE_MASK) {
+		pr_warning("setting mute support failed.\n");
+		return -EIO;
+	}
+	pr_info("%s mute led support.\n", support ? "disable" : "enable");
+
+	return 0;
+}
+
 /*
  * Used to syncronize writers to TP_EC_AUDIO and
  * TP_NVRAM_ADDR_MIXER, as we need to do read-modify-write
@@ -6982,6 +7056,7 @@ static int __init volume_init(struct ibm_init_struct *iibm)
 static int volume_read(struct seq_file *m)
 {
 	u8 status;
+	int mute;
 
 	if (volume_get_status(&status) < 0) {
 		seq_printf(m, "level:\t\tunreadable\n");
@@ -6991,8 +7066,12 @@ static int volume_read(struct seq_file *m)
 		else
 			seq_printf(m, "level:\t\t%d\n",
 					status & TP_EC_AUDIO_LVL_MSK);
-
-		seq_printf(m, "mute:\t\t%s\n",
+		if (volume_bios_support(TPACPI_AML_MUTE_GET_FUNC) &&
+		    !hotkey_get_mute_state(&mute))
+			seq_printf(m, "mute:\t\t%s\n",
+				mute ? "muted" : "unmuted");
+		else
+			seq_printf(m, "mute:\t\t%s\n",
 				onoff(status, TP_EC_AUDIO_MUTESW));
 
 		if (volume_control_allowed) {
@@ -7005,7 +7084,8 @@ static int volume_read(struct seq_file *m)
 					       " (<level> is 0-%d)\n",
 					       TP_EC_VOLUME_MAX);
 			}
-		}
+		} else if (volume_bios_support(TPACPI_AML_MUTE_SET_FUNC))
+			seq_printf(m, "commands:\tunmute, mute\n");
 	}
 
 	return 0;
@@ -7019,6 +7099,21 @@ static int volume_write(char *buf)
 	char *cmd;
 	int rc;
 
+	if (volume_bios_support(
+		TPACPI_AML_MUTE_SET_FUNC | TPACPI_AML_MUTE_SUPPORT_FUNC)) {
+		while ((cmd = next_cmd(&buf))) {
+			if (strlencmp(cmd, "mute") == 0)
+				hotkey_set_mute_state(1);
+			else if (strlencmp(cmd, "unmute") == 0)
+				hotkey_set_mute_state(0);
+			else if (strlencmp(cmd, "disable") == 0)
+				hotkey_set_mute_support(1);
+			else if (strlencmp(cmd, "enable") == 0)
+				hotkey_set_mute_support(0);
+		}
+		return -EINVAL;
+	}
+
 	/*
 	 * We do allow volume control at driver startup, so that the
 	 * user can set initial state through the volume=... parameter hack.
@@ -7061,12 +7156,14 @@ static int volume_write(char *buf)
 				continue;
 			}
 		}
-		if (strlencmp(cmd, "mute") == 0)
-			new_mute = TP_EC_AUDIO_MUTESW_MSK;
-		else if (strlencmp(cmd, "unmute") == 0)
-			new_mute = 0;
-		else
-			return -EINVAL;
+		if (!volume_bios_support(TPACPI_AML_MUTE_SET_FUNC)) {
+			if (strlencmp(cmd, "mute") == 0)
+				new_mute = TP_EC_AUDIO_MUTESW_MSK;
+			else if (strlencmp(cmd, "unmute") == 0)
+				new_mute = 0;
+			else
+				return -EINVAL;
+		}
 	}
 
 	if (tp_features.mixer_no_level_control) {
-- 
1.7.5.4

Re: [PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[Alex Hung]

Hi,

I am modifying thinkpad_acpi so it can support the mute led, and I am 
looking for suggestions and feedbacks.

For newer BIOS, it includes three aml methods that can be used to get, 
set and enable the mute led.

In *volume_read* function, I modify it to call BIOS GSMS method when it 
is presented in AML. This is more straight-forward.

However, I had debated with myself whether it should be included inside 
the if-else statement. The reason is that the new function is not 
necessarily dependent on volume_get_status, but including it in if-else 
statement works perfectly and it has minimal impacts on the original 
behaviors and therefore I decide to put my patch there.

I implemented as it is now to demonstrate what I am intending to do; 
however, deciding where to add changes to *volume_write* is much more 
difficult to decide for the following reasons:

1. The new mute functions (calling to AML's SSMS and SHDA) are 
dependents on BIOS and they can be called when SSMS and SHDA are supported.
2. If the new functions are included in the original cmd handler (the 
while-loop), they will be blocked by the first if-statement *if 
(!volume_control_allowed && tpacpi_lifecycle != TPACPI_LIFE_INIT)*.
3. However, the changes now will changes the original behaviors (i.e. 
"up", "down" and so on) if BIOS supports new SSMS and SHDA methods.

Any feedbacks and suggestions are most appreciated.

Best Regards,
Alex Hung

On 04/11/2012 06:15 PM, Alex Hung wrote:
> Signed-off-by: Alex Hung<alex.hung@canonical.com>
> ---
>   drivers/platform/x86/thinkpad_acpi.c |  115 +++++++++++++++++++++++++++++++---
>   1 files changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 7b82868..dc22a4c 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6486,12 +6486,86 @@ enum tpacpi_volume_capabilities {
>   	TPACPI_VOL_CAP_MAX
>   };
>
> +enum  {
> +	TPACPI_AML_MUTE_GET_FUNC = 0x01,
> +	TPACPI_AML_MUTE_SET_FUNC = 0x02,
> +	TPACPI_AML_MUTE_SUPPORT_FUNC = 0x04,
> +	TPACPI_AML_MUTE_READ_MASK = 0x01,
> +	TPACPI_AML_MUTE_ERROR_STATE_MASK = 0x80000000,
> +};
> +
>   static enum tpacpi_volume_access_mode volume_mode =
>   	TPACPI_VOL_MODE_MAX;
>
>   static enum tpacpi_volume_capabilities volume_capabilities;
>   static int volume_control_allowed;
>
> +
> +static bool volume_bios_support(int func)
> +{
> +	acpi_handle temp;
> +
> +	if ((func | TPACPI_AML_MUTE_GET_FUNC)&&
> +	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "GSMS",&temp)))
> +		return false;
> +
> +	if ((func | TPACPI_AML_MUTE_SET_FUNC)&&
> +	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SSMS",&temp)))
> +		return false;
> +
> +	if ((func | TPACPI_AML_MUTE_SUPPORT_FUNC)&&
> +	    !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SHDA",&temp)))
> +		return false;
> +
> +	return true;
> +}
> +
> +static int hotkey_get_mute_state(int *state)
> +{
> +	if (!acpi_evalf(hkey_handle, state, "GSMS", "dd"))
> +		return -EIO;
> +
> +	if (*state&  TPACPI_AML_MUTE_ERROR_STATE_MASK)
> +		pr_warning("getting mute state failed.\n");
> +
> +	*state&= TPACPI_AML_MUTE_READ_MASK;
> +	pr_info("get mute state = %s.\n", *state ? "muted" : "unmuted");
> +
> +	return 0;
> +}
> +
> +static int hotkey_set_mute_state(int state)
> +{
> +	int output;
> +
> +	if (!acpi_evalf(hkey_handle,&output, "SSMS", "dd", state))
> +		return -EIO;
> +
> +	if (output&  TPACPI_AML_MUTE_ERROR_STATE_MASK) {
> +		pr_warning("setting mute state failed.\n");
> +		return -EIO;
> +	}
> +	pr_info("set to mute led state to %s.\n", state ? "on" : "off");
> +
> +	return 0;
> +}
> +
> +static int hotkey_set_mute_support(int support)
> +{
> +	int output;
> +
> +	if (!acpi_evalf(hkey_handle,&output, "SHDA", "dd", support))
> +		return -EIO;
> +
> +	if (output&  TPACPI_AML_MUTE_ERROR_STATE_MASK) {
> +		pr_warning("setting mute support failed.\n");
> +		return -EIO;
> +	}
> +	pr_info("%s mute led support.\n", support ? "disable" : "enable");
> +
> +	return 0;
> +}
> +
>   /*
>    * Used to syncronize writers to TP_EC_AUDIO and
>    * TP_NVRAM_ADDR_MIXER, as we need to do read-modify-write
> @@ -6982,6 +7056,7 @@ static int __init volume_init(struct ibm_init_struct *iibm)
>   static int volume_read(struct seq_file *m)
>   {
>   	u8 status;
> +	int mute;
>
>   	if (volume_get_status(&status)<  0) {
>   		seq_printf(m, "level:\t\tunreadable\n");
> @@ -6991,8 +7066,12 @@ static int volume_read(struct seq_file *m)
>   		else
>   			seq_printf(m, "level:\t\t%d\n",
>   					status&  TP_EC_AUDIO_LVL_MSK);
> -
> -		seq_printf(m, "mute:\t\t%s\n",
> +		if (volume_bios_support(TPACPI_AML_MUTE_GET_FUNC)&&
> +		    !hotkey_get_mute_state(&mute))
> +			seq_printf(m, "mute:\t\t%s\n",
> +				mute ? "muted" : "unmuted");
> +		else
> +			seq_printf(m, "mute:\t\t%s\n",
>   				onoff(status, TP_EC_AUDIO_MUTESW));
>
>   		if (volume_control_allowed) {
> @@ -7005,7 +7084,8 @@ static int volume_read(struct seq_file *m)
>   					       " (<level>  is 0-%d)\n",
>   					       TP_EC_VOLUME_MAX);
>   			}
> -		}
> +		} else if (volume_bios_support(TPACPI_AML_MUTE_SET_FUNC))
> +			seq_printf(m, "commands:\tunmute, mute\n");
>   	}
>
>   	return 0;
> @@ -7019,6 +7099,21 @@ static int volume_write(char *buf)
>   	char *cmd;
>   	int rc;
>
> +	if (volume_bios_support(
> +		TPACPI_AML_MUTE_SET_FUNC | TPACPI_AML_MUTE_SUPPORT_FUNC)) {
> +		while ((cmd = next_cmd(&buf))) {
> +			if (strlencmp(cmd, "mute") == 0)
> +				hotkey_set_mute_state(1);
> +			else if (strlencmp(cmd, "unmute") == 0)
> +				hotkey_set_mute_state(0);
> +			else if (strlencmp(cmd, "disable") == 0)
> +				hotkey_set_mute_support(1);
> +			else if (strlencmp(cmd, "enable") == 0)
> +				hotkey_set_mute_support(0);
> +		}
> +		return -EINVAL;
> +	}
> +
>   	/*
>   	 * We do allow volume control at driver startup, so that the
>   	 * user can set initial state through the volume=... parameter hack.
> @@ -7061,12 +7156,14 @@ static int volume_write(char *buf)
>   				continue;
>   			}
>   		}
> -		if (strlencmp(cmd, "mute") == 0)
> -			new_mute = TP_EC_AUDIO_MUTESW_MSK;
> -		else if (strlencmp(cmd, "unmute") == 0)
> -			new_mute = 0;
> -		else
> -			return -EINVAL;
> +		if (!volume_bios_support(TPACPI_AML_MUTE_SET_FUNC)) {
> +			if (strlencmp(cmd, "mute") == 0)
> +				new_mute = TP_EC_AUDIO_MUTESW_MSK;
> +			else if (strlencmp(cmd, "unmute") == 0)
> +				new_mute = 0;
> +			else
> +				return -EINVAL;
> +		}
>   	}
>
>   	if (tp_features.mixer_no_level_control) {

Re: [PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[Henrique de Moraes Holschuh]

On Wed, 11 Apr 2012, Alex Hung wrote:
> I am modifying thinkpad_acpi so it can support the mute led, and I
> am looking for suggestions and feedbacks.
> 
> For newer BIOS, it includes three aml methods that can be used to
> get, set and enable the mute led.

Aha!  THANK YOU.

I will look at the patches this week.  Please send me (directly) a full
dmidecode and ACPI table dump.

-- 
  "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

Re: [PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[Alex Hung]

Hi,

I saw a number of discussion on new thinkpad led and caused me to 
re-think about my previous patch.

The previous patches add interfaces to control hardware mute via BIOS 
functions; however there are nobody to control it and the problem of 
out-of-sync between hardware mute and software mute will still exist.

Will it be a better idea to disable hardware mute? The hardware mute 
will still send a scancode to mute the volue when thinkpad-acpi is 
loaded, though the led will still not work. More specifically, a patch 
that only calls with hoteky_set_mute_support(1) that works as below:


+static int hotkey_set_mute_support(int support)
+{
+	int output;
+
+	if (!acpi_evalf(hkey_handle, &output, "SHDA", "dd", support))
+		return -EIO;
+
+	if (output & TPACPI_AML_MUTE_ERROR_STATE_MASK) {
+		pr_warning("setting mute support failed.\n");
+		return -EIO;
+	}
+	pr_info("%s mute led support.\n", support ? "disable" : "enable");
+
+	return 0;
+}

This can solve the problems that unmute from desktop will not unmute the 
hardware mute until someone including myself works out a more 
comprehensive solution.

Best Regards,
Alex Hung


On 04/11/2012 07:42 PM, Henrique de Moraes Holschuh wrote:
> On Wed, 11 Apr 2012, Alex Hung wrote:
>> I am modifying thinkpad_acpi so it can support the mute led, and I
>> am looking for suggestions and feedbacks.
>>
>> For newer BIOS, it includes three aml methods that can be used to
>> get, set and enable the mute led.
>
> Aha!  THANK YOU.
>
> I will look at the patches this week.  Please send me (directly) a full
> dmidecode and ACPI table dump.
>



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH] thinkpad_acpi: added BIOS mute interfaces for volume

[Henrique de Moraes Holschuh]

On Sun, 08 Jul 2012, Alex Hung wrote:
> Will it be a better idea to disable hardware mute? The hardware mute
> will still send a scancode to mute the volue when thinkpad-acpi is
> loaded, though the led will still not work. More specifically, a
> patch that only calls with hoteky_set_mute_support(1) that works as
> below:

I don't mind adding that as a module paramenter and a sysfs control, but
make it a multi-value integer as follows:

-1:  auto   (right now that means don't touch it.  We can change its meaning
             after some testing)
0 : disable hw mute
1 : enable  hw mute

All other values must return -EINVAL (reserved, to be implemented later).

On thinkpads where no SHDA functionality is supported, do not publish the
sysfs node.  I can test in the T43 if it is EC-supported but not published
in the DSDT, please send me the relevant acpidump of a thinkpad that
implements SHDA, please.

Relevant documentation patch for Documentation/laptops/thinkpad-acpi.txt is
required for an Ack, of course :-)

> This can solve the problems that unmute from desktop will not unmute
> the hardware mute until someone including myself works out a more
> comprehensive solution.

Agreed.

-- 
  "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

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/