[PATCH 0/3] Add support for X51-R3 to alienware-wmi

[PATCH 0/3] Add support for X51-R3 to alienware-wmi

[Mario Limonciello]

An upcoming Alienware platform, X51-R3 will support some new features in the 
WMI BIOS control API as well as emit some scan codes when particular hardware 
is used in conjunction.

Mario Limonciello (3):
  Add support for X51-R3
  Add support for Alienware graphics amplifier.
  Capture cable events created by Alienware GFX Amplifier.

 drivers/platform/x86/Kconfig         |   1 +
 drivers/platform/x86/alienware-wmi.c | 179 ++++++++++++++++++++++++++++++-----
 2 files changed, 156 insertions(+), 24 deletions(-)

-- 
1.9.1

[PATCH 1/3] Add support for X51-R3

[Mario Limonciello]

Upcoming platform X51-R3 will support 4 LED zones.

Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
 drivers/platform/x86/alienware-wmi.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
index 1e1e594..69f9d72 100644
--- a/drivers/platform/x86/alienware-wmi.c
+++ b/drivers/platform/x86/alienware-wmi.c
@@ -69,11 +69,16 @@ static struct quirk_entry quirk_unknown = {
 	.hdmi_mux = 0,
 };
 
-static struct quirk_entry quirk_x51_family = {
+static struct quirk_entry quirk_x51_r1_r2 = {
 	.num_zones = 3,
 	.hdmi_mux = 0.
 };
 
+static struct quirk_entry quirk_x51_r3 = {
+	.num_zones = 4,
+	.hdmi_mux = 0,
+};
+
 static struct quirk_entry quirk_asm100 = {
 	.num_zones = 2,
 	.hdmi_mux = 1,
@@ -88,12 +93,12 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
 static const struct dmi_system_id alienware_quirks[] __initconst = {
 	{
 	 .callback = dmi_matched,
-	 .ident = "Alienware X51 R1",
+	 .ident = "Alienware X51 R3",
 	 .matches = {
 		     DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
-		     DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51"),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R3"),
 		     },
-	 .driver_data = &quirk_x51_family,
+	 .driver_data = &quirk_x51_r3,
 	 },
 	{
 	 .callback = dmi_matched,
@@ -102,7 +107,16 @@ static const struct dmi_system_id alienware_quirks[] __initconst = {
 		     DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
 		     DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R2"),
 		     },
-	 .driver_data = &quirk_x51_family,
+	 .driver_data = &quirk_x51_r1_r2,
+	 },
+	{
+	 .callback = dmi_matched,
+	 .ident = "Alienware X51 R1",
+	 .matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51"),
+		     },
+	 .driver_data = &quirk_x51_r1_r2,
 	 },
 	{
 		.callback = dmi_matched,
-- 
1.9.1

[PATCH 2/3] Add support for Alienware graphics amplifier.

[Mario Limonciello]

Interface is only supported on the X51-R3 currently.

Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
 drivers/platform/x86/alienware-wmi.c | 110 +++++++++++++++++++++++++++++------
 1 file changed, 91 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
index 69f9d72..5bd8faf 100644
--- a/drivers/platform/x86/alienware-wmi.c
+++ b/drivers/platform/x86/alienware-wmi.c
@@ -33,6 +33,7 @@
 #define WMAX_METHOD_BRIGHTNESS		0x3
 #define WMAX_METHOD_ZONE_CONTROL	0x4
 #define WMAX_METHOD_HDMI_CABLE		0x5
+#define WMAX_METHOD_AMPLIFIER_CABLE	0x6
 
 MODULE_AUTHOR("Mario Limonciello <mario_limonciello@dell.com>");
 MODULE_DESCRIPTION("Alienware special feature control");
@@ -60,6 +61,7 @@ enum WMAX_CONTROL_STATES {
 struct quirk_entry {
 	u8 num_zones;
 	u8 hdmi_mux;
+	u8 amplifier;
 };
 
 static struct quirk_entry *quirks;
@@ -67,21 +69,25 @@ static struct quirk_entry *quirks;
 static struct quirk_entry quirk_unknown = {
 	.num_zones = 2,
 	.hdmi_mux = 0,
+	.amplifier = 0,
 };
 
 static struct quirk_entry quirk_x51_r1_r2 = {
 	.num_zones = 3,
-	.hdmi_mux = 0.
+	.hdmi_mux = 0,
+	.amplifier = 0,
 };
 
 static struct quirk_entry quirk_x51_r3 = {
 	.num_zones = 4,
 	.hdmi_mux = 0,
+	.amplifier = 1,
 };
 
 static struct quirk_entry quirk_asm100 = {
 	.num_zones = 2,
 	.hdmi_mux = 1,
+	.amplifier = 0,
 };
 
 static int __init dmi_matched(const struct dmi_system_id *dmi)
@@ -147,7 +153,7 @@ struct wmax_brightness_args {
 	u32 percentage;
 };
 
-struct hdmi_args {
+struct wmax_basic_args {
 	u8 arg;
 };
 
@@ -232,16 +238,16 @@ static int alienware_update_led(struct platform_zone *zone)
 	char *guid;
 	struct acpi_buffer input;
 	struct legacy_led_args legacy_args;
-	struct wmax_led_args wmax_args;
+	struct wmax_led_args wmax_basic_args;
 	if (interface == WMAX) {
-		wmax_args.led_mask = 1 << zone->location;
-		wmax_args.colors = zone->colors;
-		wmax_args.state = lighting_control_state;
+		wmax_basic_args.led_mask = 1 << zone->location;
+		wmax_basic_args.colors = zone->colors;
+		wmax_basic_args.state = lighting_control_state;
 		guid = WMAX_CONTROL_GUID;
 		method_id = WMAX_METHOD_ZONE_CONTROL;
 
-		input.length = (acpi_size) sizeof(wmax_args);
-		input.pointer = &wmax_args;
+		input.length = (acpi_size) sizeof(wmax_basic_args);
+		input.pointer = &wmax_basic_args;
 	} else {
 		legacy_args.colors = zone->colors;
 		legacy_args.brightness = global_brightness;
@@ -449,11 +455,7 @@ static void alienware_zone_exit(struct platform_device *dev)
 	kfree(zone_attrs);
 }
 
-/*
-	The HDMI mux sysfs node indicates the status of the HDMI input mux.
-	It can toggle between standard system GPU output and HDMI input.
-*/
-static acpi_status alienware_hdmi_command(struct hdmi_args *in_args,
+static acpi_status alienware_wmax_command(struct wmax_basic_args *in_args,
 					  u32 command, int *out_data)
 {
 	acpi_status status;
@@ -481,16 +483,20 @@ static acpi_status alienware_hdmi_command(struct hdmi_args *in_args,
 
 }
 
+/*
+	The HDMI mux sysfs node indicates the status of the HDMI input mux.
+	It can toggle between standard system GPU output and HDMI input.
+*/
 static ssize_t show_hdmi_cable(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
 	acpi_status status;
 	u32 out_data;
-	struct hdmi_args in_args = {
+	struct wmax_basic_args in_args = {
 		.arg = 0,
 	};
 	status =
-	    alienware_hdmi_command(&in_args, WMAX_METHOD_HDMI_CABLE,
+	    alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_CABLE,
 				   (u32 *) &out_data);
 	if (ACPI_SUCCESS(status)) {
 		if (out_data == 0)
@@ -509,11 +515,11 @@ static ssize_t show_hdmi_source(struct device *dev,
 {
 	acpi_status status;
 	u32 out_data;
-	struct hdmi_args in_args = {
+	struct wmax_basic_args in_args = {
 		.arg = 0,
 	};
 	status =
-	    alienware_hdmi_command(&in_args, WMAX_METHOD_HDMI_STATUS,
+	    alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_STATUS,
 				   (u32 *) &out_data);
 
 	if (ACPI_SUCCESS(status)) {
@@ -533,7 +539,7 @@ static ssize_t toggle_hdmi_source(struct device *dev,
 				  const char *buf, size_t count)
 {
 	acpi_status status;
-	struct hdmi_args args;
+	struct wmax_basic_args args;
 	if (strcmp(buf, "gpu\n") == 0)
 		args.arg = 1;
 	else if (strcmp(buf, "input\n") == 0)
@@ -542,7 +548,7 @@ static ssize_t toggle_hdmi_source(struct device *dev,
 		args.arg = 3;
 	pr_debug("alienware-wmi: setting hdmi to %d : %s", args.arg, buf);
 
-	status = alienware_hdmi_command(&args, WMAX_METHOD_HDMI_SOURCE, NULL);
+	status = alienware_wmax_command(&args, WMAX_METHOD_HDMI_SOURCE, NULL);
 
 	if (ACPI_FAILURE(status))
 		pr_err("alienware-wmi: HDMI toggle failed: results: %u\n",
@@ -585,6 +591,65 @@ error_create_hdmi:
 	return ret;
 }
 
+/* Alienware GFX amplifier support
+ - Currently supports reading cable status
+ - Leaving expansion room to possibly support dock/undock events later
+*/
+static ssize_t show_amplifier_status(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	acpi_status status;
+	u32 out_data;
+	struct wmax_basic_args in_args = {
+		.arg = 0,
+	};
+	status =
+	    alienware_wmax_command(&in_args, WMAX_METHOD_AMPLIFIER_CABLE,
+				   (u32 *) &out_data);
+	if (ACPI_SUCCESS(status)) {
+		if (out_data == 0)
+			return scnprintf(buf, PAGE_SIZE,
+					 "[unconnected] connected unknown\n");
+		else if (out_data == 1)
+			return scnprintf(buf, PAGE_SIZE,
+					 "unconnected [connected] unknown\n");
+	}
+	pr_err("alienware-wmi: unknown amplifier cable status: %d\n", status);
+	return scnprintf(buf, PAGE_SIZE, "unconnected connected [unknown]\n");
+}
+
+static DEVICE_ATTR(status, S_IRUGO, show_amplifier_status, NULL);
+
+static struct attribute *amplifier_attrs[] = {
+	&dev_attr_status.attr,
+	NULL,
+};
+
+static struct attribute_group amplifier_attribute_group = {
+	.name = "amplifier",
+	.attrs = amplifier_attrs,
+};
+
+static void remove_amplifier(struct platform_device *dev)
+{
+	if (quirks->amplifier > 0)
+		sysfs_remove_group(&dev->dev.kobj, &amplifier_attribute_group);
+}
+
+static int create_amplifier(struct platform_device *dev)
+{
+	int ret;
+
+	ret = sysfs_create_group(&dev->dev.kobj, &amplifier_attribute_group);
+	if (ret)
+		goto error_create_amplifier;
+	return 0;
+
+error_create_amplifier:
+	remove_amplifier(dev);
+	return ret;
+}
+
 static int __init alienware_wmi_init(void)
 {
 	int ret;
@@ -620,6 +685,12 @@ static int __init alienware_wmi_init(void)
 			goto fail_prep_hdmi;
 	}
 
+	if (quirks->amplifier > 0) {
+		ret = create_amplifier(platform_device);
+		if (ret)
+			goto fail_prep_amplifier;
+	}
+
 	ret = alienware_zone_init(platform_device);
 	if (ret)
 		goto fail_prep_zones;
@@ -628,6 +699,7 @@ static int __init alienware_wmi_init(void)
 
 fail_prep_zones:
 	alienware_zone_exit(platform_device);
+fail_prep_amplifier:
 fail_prep_hdmi:
 	platform_device_del(platform_device);
 fail_platform_device2:
-- 
1.9.1

[PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Mario Limonciello]

These events are outputted via the keyboard controller.
Right now the only actionable event is to restart the system
if the cable is removed,  but the others are documented in
case they will be bubbled up to other parts of the kernel or
userspace later on.

Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
 drivers/platform/x86/Kconfig         |  1 +
 drivers/platform/x86/alienware-wmi.c | 45 ++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f9f205c..8a7bd7c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -60,6 +60,7 @@ config ALIENWARE_WMI
 	depends on LEDS_CLASS
 	depends on NEW_LEDS
 	depends on ACPI_WMI
+	depends on SERIO_I8042
 	---help---
 	 This is a driver for controlling Alienware BIOS driven
 	 features.  It exposes an interface for controlling the AlienFX
diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
index 5bd8faf..ffccbd9 100644
--- a/drivers/platform/x86/alienware-wmi.c
+++ b/drivers/platform/x86/alienware-wmi.c
@@ -23,6 +23,8 @@
 #include <linux/dmi.h>
 #include <linux/acpi.h>
 #include <linux/leds.h>
+#include <linux/i8042.h>
+#include <linux/reboot.h>
 
 #define LEGACY_CONTROL_GUID		"A90597CE-A997-11DA-B012-B622A1EF5492"
 #define LEGACY_POWER_CONTROL_GUID	"A80593CE-A997-11DA-B012-B622A1EF5492"
@@ -650,6 +652,44 @@ error_create_amplifier:
 	return ret;
 }
 
+static bool alienware_i8042_filter(unsigned char data, unsigned char str,
+			struct serio *port)
+{
+	static bool extended;
+
+	if (str & I8042_STR_AUXDATA)
+		return false;
+
+	if (unlikely(data == 0xe0)) {
+		extended = true;
+		return false;
+	} else if (unlikely(extended)) {
+		switch (data) {
+		/*Connect */
+		case 0x3F:
+			pr_debug("Received connect event\n");
+			break;
+		/* Disconnect by hotkey */
+		case 0x40:
+			pr_debug("Received disconnect hotkey event\n");
+			break;
+		/*disconnect by cable undock button */
+		case 0x41:
+			pr_debug("Received disconnect button event\n");
+			break;
+		/* surprise disconnect */
+		case 0x42:
+			pr_crit("Graphics amplifier removed, initiating reboot.\n");
+			emergency_restart();
+			break;
+		}
+		extended = false;
+	}
+
+	return false;
+}
+
+
 static int __init alienware_wmi_init(void)
 {
 	int ret;
@@ -689,6 +729,9 @@ static int __init alienware_wmi_init(void)
 		ret = create_amplifier(platform_device);
 		if (ret)
 			goto fail_prep_amplifier;
+		ret = i8042_install_filter(alienware_i8042_filter);
+		if (ret)
+			goto fail_prep_i8042;
 	}
 
 	ret = alienware_zone_init(platform_device);
@@ -699,6 +742,7 @@ static int __init alienware_wmi_init(void)
 
 fail_prep_zones:
 	alienware_zone_exit(platform_device);
+fail_prep_i8042:
 fail_prep_amplifier:
 fail_prep_hdmi:
 	platform_device_del(platform_device);
@@ -715,6 +759,7 @@ module_init(alienware_wmi_init);
 static void __exit alienware_wmi_exit(void)
 {
 	if (platform_device) {
+		i8042_remove_filter(alienware_i8042_filter);
 		alienware_zone_exit(platform_device);
 		remove_hdmi(platform_device);
 		platform_device_unregister(platform_device);
-- 
1.9.1

Re: [PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Mario Limonciello]

On 05/27/2015 02:37 PM, Limonciello, Mario wrote:
> These events are outputted via the keyboard controller.
> Right now the only actionable event is to restart the system
> if the cable is removed,  but the others are documented in
> case they will be bubbled up to other parts of the kernel or
> userspace later on.
>
> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> ---
>   drivers/platform/x86/Kconfig         |  1 +
>   drivers/platform/x86/alienware-wmi.c | 45 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 46 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f9f205c..8a7bd7c 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -60,6 +60,7 @@ config ALIENWARE_WMI
>   	depends on LEDS_CLASS
>   	depends on NEW_LEDS
>   	depends on ACPI_WMI
> +	depends on SERIO_I8042
>   	---help---
>   	 This is a driver for controlling Alienware BIOS driven
>   	 features.  It exposes an interface for controlling the AlienFX
> diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
> index 5bd8faf..ffccbd9 100644
> --- a/drivers/platform/x86/alienware-wmi.c
> +++ b/drivers/platform/x86/alienware-wmi.c
> @@ -23,6 +23,8 @@
>   #include <linux/dmi.h>
>   #include <linux/acpi.h>
>   #include <linux/leds.h>
> +#include <linux/i8042.h>
> +#include <linux/reboot.h>
>   
>   #define LEGACY_CONTROL_GUID		"A90597CE-A997-11DA-B012-B622A1EF5492"
>   #define LEGACY_POWER_CONTROL_GUID	"A80593CE-A997-11DA-B012-B622A1EF5492"
> @@ -650,6 +652,44 @@ error_create_amplifier:
>   	return ret;
>   }
>   
> +static bool alienware_i8042_filter(unsigned char data, unsigned char str,
> +			struct serio *port)
> +{
> +	static bool extended;
> +
> +	if (str & I8042_STR_AUXDATA)
> +		return false;
> +
> +	if (unlikely(data == 0xe0)) {
> +		extended = true;
> +		return false;
> +	} else if (unlikely(extended)) {
> +		switch (data) {
> +		/*Connect */
> +		case 0x3F:
> +			pr_debug("Received connect event\n");
> +			break;
> +		/* Disconnect by hotkey */
> +		case 0x40:
> +			pr_debug("Received disconnect hotkey event\n");
> +			break;
> +		/*disconnect by cable undock button */
> +		case 0x41:
> +			pr_debug("Received disconnect button event\n");
> +			break;
> +		/* surprise disconnect */
> +		case 0x42:
> +			pr_crit("Graphics amplifier removed, initiating reboot.\n");
> +			emergency_restart();

I had some discussion with someone in #systemd.  Just to clarify why the 
reboot is needed I wanted to mention it here.
X does continue to run and the kernel doesn't panic, but the GPU drivers 
gets stuck in a hung state.
Restarting X or plugging the cable back in doesn't actually fix it.
Since you just pulled your U/I from under yourself you don't really have 
a way to message what happened, or to fix this other than restarting the 
system.

I also later discovered that this should probably be:

     orderly_reboot();

I'll resubmit with that change after any other discussion around 
iterating with this.

> +			break;
> +		}
> +		extended = false;
> +	}
> +
> +	return false;
> +}
> +
> +
>   static int __init alienware_wmi_init(void)
>   {
>   	int ret;
> @@ -689,6 +729,9 @@ static int __init alienware_wmi_init(void)
>   		ret = create_amplifier(platform_device);
>   		if (ret)
>   			goto fail_prep_amplifier;
> +		ret = i8042_install_filter(alienware_i8042_filter);
> +		if (ret)
> +			goto fail_prep_i8042;
>   	}
>   
>   	ret = alienware_zone_init(platform_device);
> @@ -699,6 +742,7 @@ static int __init alienware_wmi_init(void)
>   
>   fail_prep_zones:
>   	alienware_zone_exit(platform_device);
> +fail_prep_i8042:
>   fail_prep_amplifier:
>   fail_prep_hdmi:
>   	platform_device_del(platform_device);
> @@ -715,6 +759,7 @@ module_init(alienware_wmi_init);
>   static void __exit alienware_wmi_exit(void)
>   {
>   	if (platform_device) {
> +		i8042_remove_filter(alienware_i8042_filter);
>   		alienware_zone_exit(platform_device);
>   		remove_hdmi(platform_device);
>   		platform_device_unregister(platform_device);

Re: [PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Greg KH]

On Wed, May 27, 2015 at 02:37:02PM -0500, Mario Limonciello wrote:
> These events are outputted via the keyboard controller.
> Right now the only actionable event is to restart the system
> if the cable is removed,

What?  No, that's not an acceptable "event" at all.  You don't reboot
for something as "simple" as this.

Please do the correct thing and properly handle the hotplug event, just
like all other platforms do.

> but the others are documented in
> case they will be bubbled up to other parts of the kernel or
> userspace later on.
> 
> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> ---
>  drivers/platform/x86/Kconfig         |  1 +
>  drivers/platform/x86/alienware-wmi.c | 45 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f9f205c..8a7bd7c 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -60,6 +60,7 @@ config ALIENWARE_WMI
>  	depends on LEDS_CLASS
>  	depends on NEW_LEDS
>  	depends on ACPI_WMI
> +	depends on SERIO_I8042
>  	---help---
>  	 This is a driver for controlling Alienware BIOS driven
>  	 features.  It exposes an interface for controlling the AlienFX
> diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
> index 5bd8faf..ffccbd9 100644
> --- a/drivers/platform/x86/alienware-wmi.c
> +++ b/drivers/platform/x86/alienware-wmi.c
> @@ -23,6 +23,8 @@
>  #include <linux/dmi.h>
>  #include <linux/acpi.h>
>  #include <linux/leds.h>
> +#include <linux/i8042.h>
> +#include <linux/reboot.h>
>  
>  #define LEGACY_CONTROL_GUID		"A90597CE-A997-11DA-B012-B622A1EF5492"
>  #define LEGACY_POWER_CONTROL_GUID	"A80593CE-A997-11DA-B012-B622A1EF5492"
> @@ -650,6 +652,44 @@ error_create_amplifier:
>  	return ret;
>  }
>  
> +static bool alienware_i8042_filter(unsigned char data, unsigned char str,
> +			struct serio *port)
> +{
> +	static bool extended;
> +
> +	if (str & I8042_STR_AUXDATA)
> +		return false;
> +
> +	if (unlikely(data == 0xe0)) {
> +		extended = true;
> +		return false;
> +	} else if (unlikely(extended)) {
> +		switch (data) {
> +		/*Connect */
> +		case 0x3F:
> +			pr_debug("Received connect event\n");
> +			break;
> +		/* Disconnect by hotkey */
> +		case 0x40:
> +			pr_debug("Received disconnect hotkey event\n");
> +			break;
> +		/*disconnect by cable undock button */
> +		case 0x41:
> +			pr_debug("Received disconnect button event\n");
> +			break;
> +		/* surprise disconnect */
> +		case 0x42:
> +			pr_crit("Graphics amplifier removed, initiating reboot.\n");
> +			emergency_restart();

Ick, no, do the correct GPU change.  Other platforms can do this, why
can't you?

Especially as you have access to the hardware specs, do the correct
thing.  We have supported pci hotplug for well over a decade, why would
we want to regress to this type of "solution"?

You will note, other operating systems don't do this, why do you think
that Linux should do this?

greg k-h

Re: [PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Greg KH]

On Wed, May 27, 2015 at 06:46:05PM -0500, Mario Limonciello wrote:
> 
> 
> On 05/27/2015 02:37 PM, Limonciello, Mario wrote:
> >These events are outputted via the keyboard controller.
> >Right now the only actionable event is to restart the system
> >if the cable is removed,  but the others are documented in
> >case they will be bubbled up to other parts of the kernel or
> >userspace later on.
> >
> >Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> >---
> >  drivers/platform/x86/Kconfig         |  1 +
> >  drivers/platform/x86/alienware-wmi.c | 45 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> >
> >diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> >index f9f205c..8a7bd7c 100644
> >--- a/drivers/platform/x86/Kconfig
> >+++ b/drivers/platform/x86/Kconfig
> >@@ -60,6 +60,7 @@ config ALIENWARE_WMI
> >  	depends on LEDS_CLASS
> >  	depends on NEW_LEDS
> >  	depends on ACPI_WMI
> >+	depends on SERIO_I8042
> >  	---help---
> >  	 This is a driver for controlling Alienware BIOS driven
> >  	 features.  It exposes an interface for controlling the AlienFX
> >diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
> >index 5bd8faf..ffccbd9 100644
> >--- a/drivers/platform/x86/alienware-wmi.c
> >+++ b/drivers/platform/x86/alienware-wmi.c
> >@@ -23,6 +23,8 @@
> >  #include <linux/dmi.h>
> >  #include <linux/acpi.h>
> >  #include <linux/leds.h>
> >+#include <linux/i8042.h>
> >+#include <linux/reboot.h>
> >  #define LEGACY_CONTROL_GUID		"A90597CE-A997-11DA-B012-B622A1EF5492"
> >  #define LEGACY_POWER_CONTROL_GUID	"A80593CE-A997-11DA-B012-B622A1EF5492"
> >@@ -650,6 +652,44 @@ error_create_amplifier:
> >  	return ret;
> >  }
> >+static bool alienware_i8042_filter(unsigned char data, unsigned char str,
> >+			struct serio *port)
> >+{
> >+	static bool extended;
> >+
> >+	if (str & I8042_STR_AUXDATA)
> >+		return false;
> >+
> >+	if (unlikely(data == 0xe0)) {
> >+		extended = true;
> >+		return false;
> >+	} else if (unlikely(extended)) {
> >+		switch (data) {
> >+		/*Connect */
> >+		case 0x3F:
> >+			pr_debug("Received connect event\n");
> >+			break;
> >+		/* Disconnect by hotkey */
> >+		case 0x40:
> >+			pr_debug("Received disconnect hotkey event\n");
> >+			break;
> >+		/*disconnect by cable undock button */
> >+		case 0x41:
> >+			pr_debug("Received disconnect button event\n");
> >+			break;
> >+		/* surprise disconnect */
> >+		case 0x42:
> >+			pr_crit("Graphics amplifier removed, initiating reboot.\n");
> >+			emergency_restart();
> 
> I had some discussion with someone in #systemd.  Just to clarify why the
> reboot is needed I wanted to mention it here.
> X does continue to run and the kernel doesn't panic, but the GPU drivers
> gets stuck in a hung state.

Then fix the GPU drivers.

> Restarting X or plugging the cable back in doesn't actually fix it.

Fix the GPU drivers.

> Since you just pulled your U/I from under yourself you don't really have a
> way to message what happened, or to fix this other than restarting the
> system.

Fix the GPU drivers.

> I also later discovered that this should probably be:
> 
>     orderly_reboot();

Nope, fix the GPU drivers.

> I'll resubmit with that change after any other discussion around iterating
> with this.

With a fix for the GPU drivers?  Great, that's the only acceptable
solution.

thanks,

greg k-h

Re: [PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Mario Limonciello]

On 05/28/2015 01:30 PM, Greg KH wrote:
> On Wed, May 27, 2015 at 06:46:05PM -0500, Mario Limonciello wrote:
> Then fix the GPU drivers.
>
> Fix the GPU drivers.
>
> Fix the GPU drivers.
>
> Nope, fix the GPU drivers.
>
> With a fix for the GPU drivers?  Great, that's the only acceptable
> solution.
>
> thanks,
>
> greg k-h
Thanks for the feedback, point taken that you don't want to see the 
system restart when the xGPU is removed from the system.  Can you please 
help to advise me the flow you would realistically like to see here?  
I'm looking for the best experience for the end user with what the 
situation is with the Xorg architecture.
Also, there's two factors I'd like to draw attention to that the Other 
OS can support.

1) It supports something called Heterogenous graphics.  That means that 
you can actually have a GPU from Intel, NVIDIA, and AMD all in the 
system at the same time with acceleration active.  If one drops off the 
bus, the OS moves the apps rendering to that GPU over (or kills them if 
they can't be moved).  Xorg supports having multiple GPU's via Xinerama, 
but HW acceleration is disabled.  No one on a gaming PC would actually 
use Xinerama.
So you can-not simultaneously use an NVIDIA, AMD and Intel GPU in a 
gaming PC on Linux's Xorg architecture.  Even if you got to the ideal 
scenario of using all 3 GPU's with open source drivers, this is still a 
problem for Linux w/ Xorg.


2) If the xGPU is the active GPU on the system and someone removes the 
cable what should happen?  Remember, you can only have one GPU active in 
Xorg.  There "might" be a second GPU in the system, but again in order 
to use you would have to restart Xorg.  If it turns out that GPU is not 
the same vendor as the GPU in your xGPU you'll also have to reconfigure 
Xorg and possibly install different drivers.

Re: [PATCH 3/3] Capture cable events created by Alienware GFX Amplifier.

[Greg KH]

On Thu, May 28, 2015 at 01:50:02PM -0500, Mario Limonciello wrote:
> 
> 
> On 05/28/2015 01:30 PM, Greg KH wrote:
> >On Wed, May 27, 2015 at 06:46:05PM -0500, Mario Limonciello wrote:
> >Then fix the GPU drivers.
> >
> >Fix the GPU drivers.
> >
> >Fix the GPU drivers.
> >
> >Nope, fix the GPU drivers.
> >
> >With a fix for the GPU drivers?  Great, that's the only acceptable
> >solution.
> >
> >thanks,
> >
> >greg k-h
> Thanks for the feedback, point taken that you don't want to see the system
> restart when the xGPU is removed from the system.  Can you please help to
> advise me the flow you would realistically like to see here?  I'm looking
> for the best experience for the end user with what the situation is with the
> Xorg architecture.

I suggest asking about xorg specifics to the xorg developers, they know
this much better than kernel developers do :)

> Also, there's two factors I'd like to draw attention to that the Other OS
> can support.
> 
> 1) It supports something called Heterogenous graphics.  That means that you
> can actually have a GPU from Intel, NVIDIA, and AMD all in the system at the
> same time with acceleration active.  If one drops off the bus, the OS moves
> the apps rendering to that GPU over (or kills them if they can't be moved).
> Xorg supports having multiple GPU's via Xinerama, but HW acceleration is
> disabled.  No one on a gaming PC would actually use Xinerama.
> So you can-not simultaneously use an NVIDIA, AMD and Intel GPU in a gaming
> PC on Linux's Xorg architecture.  Even if you got to the ideal scenario of
> using all 3 GPU's with open source drivers, this is still a problem for
> Linux w/ Xorg.

Take it up with the xorg developers, but I think this might already
work, as there are laptops that do allow switching between gpus pretty
easily.  Well, "easily" is relative, it depends on the specific laptop.

But in no case is the solution "reboot the box", never do that.

> 2) If the xGPU is the active GPU on the system and someone removes the cable
> what should happen?  Remember, you can only have one GPU active in Xorg.

Again, take this up with the xorg developers, this isn't a kernel
limitation from what I can tell.

best of luck,

greg k-h

Re: [PATCH 0/3] Add support for X51-R3 to alienware-wmi

[Mario Limonciello]

On 05/27/2015 02:36 PM, Limonciello, Mario wrote:
> An upcoming Alienware platform, X51-R3 will support some new features in the
> WMI BIOS control API as well as emit some scan codes when particular hardware
> is used in conjunction.
>
> Mario Limonciello (3):
>    Add support for X51-R3
>    Add support for Alienware graphics amplifier.
>    Capture cable events created by Alienware GFX Amplifier.
>
>   drivers/platform/x86/Kconfig         |   1 +
>   drivers/platform/x86/alienware-wmi.c | 179 ++++++++++++++++++++++++++++++-----
>   2 files changed, 156 insertions(+), 24 deletions(-)
>

Matthew,

Aside from the third patch which has unfavorable feedback from Greg, any 
thoughts on the first two?  I purposefully broke them up and would like 
to get the first two included even if some variation of the third ends 
up not landing.

Thanks,

Re: [PATCH 0/3] Add support for X51-R3 to alienware-wmi

[Mario Limonciello]

On 06/03/2015 06:17 PM, Mario Limonciello wrote:
>
> On 05/27/2015 02:36 PM, Limonciello, Mario wrote:
>> An upcoming Alienware platform, X51-R3 will support some new features in the
>> WMI BIOS control API as well as emit some scan codes when particular hardware
>> is used in conjunction.
>>
>> Mario Limonciello (3):
>>     Add support for X51-R3
>>     Add support for Alienware graphics amplifier.
>>     Capture cable events created by Alienware GFX Amplifier.
>>
>>    drivers/platform/x86/Kconfig         |   1 +
>>    drivers/platform/x86/alienware-wmi.c | 179 ++++++++++++++++++++++++++++++-----
>>    2 files changed, 156 insertions(+), 24 deletions(-)
>>
> Matthew,
>
> Aside from the third patch which has unfavorable feedback from Greg, any
> thoughts on the first two?  I purposefully broke them up and would like
> to get the first two included even if some variation of the third ends
> up not landing.
>
> Thanks,
>
Hi Matthew,

I wanted to check in once more on the first two patches.  If you've got 
them queued up for inclusion in a future kernel already please let me 
know.  If any changes are needed I'd like to get them taken care of.

Thanks,

Re: [PATCH 0/3] Add support for X51-R3 to alienware-wmi

[Mario Limonciello]

On 06/16/2015 12:25 AM, Mario Limonciello wrote:
>
> On 06/03/2015 06:17 PM, Mario Limonciello wrote:
>> On 05/27/2015 02:36 PM, Limonciello, Mario wrote:
>>> An upcoming Alienware platform, X51-R3 will support some new features in the
>>> WMI BIOS control API as well as emit some scan codes when particular hardware
>>> is used in conjunction.
>>>
>>> Mario Limonciello (3):
>>>      Add support for X51-R3
>>>      Add support for Alienware graphics amplifier.
>>>      Capture cable events created by Alienware GFX Amplifier.
>>>
>>>     drivers/platform/x86/Kconfig         |   1 +
>>>     drivers/platform/x86/alienware-wmi.c | 179 ++++++++++++++++++++++++++++++-----
>>>     2 files changed, 156 insertions(+), 24 deletions(-)
>>>
>> Matthew,
>>
>> Aside from the third patch which has unfavorable feedback from Greg, any
>> thoughts on the first two?  I purposefully broke them up and would like
>> to get the first two included even if some variation of the third ends
>> up not landing.
>>
>> Thanks,
>>
> Hi Matthew,
>
> I wanted to check in once more on the first two patches.  If you've got
> them queued up for inclusion in a future kernel already please let me
> know.  If any changes are needed I'd like to get them taken care of.
>
> Thanks,
Hello,

If I need to resubmit these first 2 patches, or you have some concerns 
with them, please let me know.  I haven't gotten any feedback on them in 
over a month.

Thanks,