[PATCH] ACPI video: Add video_switch_key option

[PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

Some HP laptops send ACPI video events per lid open/close, and the
ACPI video driver translates to SWITCHVIDEOMODE input key events.
This behavior confuses the user-space daemon such as
gnome-settings-daemon since it changes the video mode per this key,
which results in a different video mode at each time you open/close
the laptop lid.  Fn-Fx keys are handled differently on such hardware,
thus this key generation is simply superfluous.

This patch adds a module parameter "video_switch_key" to either
assign a different key code for this ACPI event or disable by setting
zero there.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/acpi/video.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5cd0228..a61162f 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -80,6 +80,8 @@ module_param(brightness_switch_enabled, bool, 0644);
  */
 static int allow_duplicates;
 module_param(allow_duplicates, bool, 0644);
+static int video_switch_key = KEY_SWITCHVIDEOMODE;
+module_param(video_switch_key, int, 0644);
 
 static int register_count = 0;
 static int acpi_video_bus_add(struct acpi_device *device);
@@ -1511,7 +1513,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
 					 * most likely via hotkey. */
 		acpi_bus_generate_proc_event(device, event, 0);
-		keycode = KEY_SWITCHVIDEOMODE;
+		keycode = video_switch_key;
 		break;
 
 	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
@@ -1519,12 +1521,12 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 		acpi_video_device_enumerate(video);
 		acpi_video_device_rebind(video);
 		acpi_bus_generate_proc_event(device, event, 0);
-		keycode = KEY_SWITCHVIDEOMODE;
+		keycode = video_switch_key;
 		break;
 
 	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
 		acpi_bus_generate_proc_event(device, event, 0);
-		keycode = KEY_SWITCHVIDEOMODE;
+		keycode = video_switch_key;
 		break;
 	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
 		acpi_bus_generate_proc_event(device, event, 0);
@@ -1736,7 +1738,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
 	input->id.product = 0x06;
 	input->dev.parent = &device->dev;
 	input->evbit[0] = BIT(EV_KEY);
-	set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
+	if (video_switch_key && video_switch_key < KEY_CNT)
+		set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
 	set_bit(KEY_VIDEO_NEXT, input->keybit);
 	set_bit(KEY_VIDEO_PREV, input->keybit);
 	set_bit(KEY_BRIGHTNESS_CYCLE, input->keybit);
-- 
1.7.3.1

Re: [PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

Hi,

At Wed, 27 Oct 2010 12:39:17 +0200,
Takashi Iwai wrote:
> 
> Some HP laptops send ACPI video events per lid open/close, and the
> ACPI video driver translates to SWITCHVIDEOMODE input key events.
> This behavior confuses the user-space daemon such as
> gnome-settings-daemon since it changes the video mode per this key,
> which results in a different video mode at each time you open/close
> the laptop lid.  Fn-Fx keys are handled differently on such hardware,
> thus this key generation is simply superfluous.
> 
> This patch adds a module parameter "video_switch_key" to either
> assign a different key code for this ACPI event or disable by setting
> zero there.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Any comments on this patch?


thanks,

Takashi


> ---
>  drivers/acpi/video.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 5cd0228..a61162f 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -80,6 +80,8 @@ module_param(brightness_switch_enabled, bool, 0644);
>   */
>  static int allow_duplicates;
>  module_param(allow_duplicates, bool, 0644);
> +static int video_switch_key = KEY_SWITCHVIDEOMODE;
> +module_param(video_switch_key, int, 0644);
>  
>  static int register_count = 0;
>  static int acpi_video_bus_add(struct acpi_device *device);
> @@ -1511,7 +1513,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
>  					 * most likely via hotkey. */
>  		acpi_bus_generate_proc_event(device, event, 0);
> -		keycode = KEY_SWITCHVIDEOMODE;
> +		keycode = video_switch_key;
>  		break;
>  
>  	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
> @@ -1519,12 +1521,12 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>  		acpi_video_device_enumerate(video);
>  		acpi_video_device_rebind(video);
>  		acpi_bus_generate_proc_event(device, event, 0);
> -		keycode = KEY_SWITCHVIDEOMODE;
> +		keycode = video_switch_key;
>  		break;
>  
>  	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
>  		acpi_bus_generate_proc_event(device, event, 0);
> -		keycode = KEY_SWITCHVIDEOMODE;
> +		keycode = video_switch_key;
>  		break;
>  	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
>  		acpi_bus_generate_proc_event(device, event, 0);
> @@ -1736,7 +1738,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
>  	input->id.product = 0x06;
>  	input->dev.parent = &device->dev;
>  	input->evbit[0] = BIT(EV_KEY);
> -	set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
> +	if (video_switch_key && video_switch_key < KEY_CNT)
> +		set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
>  	set_bit(KEY_VIDEO_NEXT, input->keybit);
>  	set_bit(KEY_VIDEO_PREV, input->keybit);
>  	set_bit(KEY_BRIGHTNESS_CYCLE, input->keybit);
> -- 
> 1.7.3.1
> 

Re: [PATCH] ACPI video: Add video_switch_key option

[Matthew Garrett]

On Wed, Nov 24, 2010 at 08:05:35AM +0100, Takashi Iwai wrote:
> Hi,
> 
> At Wed, 27 Oct 2010 12:39:17 +0200,
> Takashi Iwai wrote:
> > Some HP laptops send ACPI video events per lid open/close, and the
> > ACPI video driver translates to SWITCHVIDEOMODE input key events.
> > This behavior confuses the user-space daemon such as
> > gnome-settings-daemon since it changes the video mode per this key,
> > which results in a different video mode at each time you open/close
> > the laptop lid.  Fn-Fx keys are handled differently on such hardware,
> > thus this key generation is simply superfluous.
> > 
> > This patch adds a module parameter "video_switch_key" to either
> > assign a different key code for this ACPI event or disable by setting
> > zero there.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> Any comments on this patch?

I'm not entirely clear on why this is wanted. Lid state changes seem to 
be a reasonable proxy for "Reprobe outputs" - what's userspace actually 
doing that causes problems?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

At Wed, 24 Nov 2010 14:26:18 +0000,
Matthew Garrett wrote:
> 
> On Wed, Nov 24, 2010 at 08:05:35AM +0100, Takashi Iwai wrote:
> > Hi,
> > 
> > At Wed, 27 Oct 2010 12:39:17 +0200,
> > Takashi Iwai wrote:
> > > Some HP laptops send ACPI video events per lid open/close, and the
> > > ACPI video driver translates to SWITCHVIDEOMODE input key events.
> > > This behavior confuses the user-space daemon such as
> > > gnome-settings-daemon since it changes the video mode per this key,
> > > which results in a different video mode at each time you open/close
> > > the laptop lid.  Fn-Fx keys are handled differently on such hardware,
> > > thus this key generation is simply superfluous.
> > > 
> > > This patch adds a module parameter "video_switch_key" to either
> > > assign a different key code for this ACPI event or disable by setting
> > > zero there.
> > > 
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > 
> > Any comments on this patch?
> 
> I'm not entirely clear on why this is wanted. Lid state changes seem to 
> be a reasonable proxy for "Reprobe outputs" - what's userspace actually 
> doing that causes problems?

Typically GNOME changes to the next monitor mode (e.g. xinearama,
clone, laptop-only, external-only) by each XF86VideoSwitch key event.
If you connect your laptop to a monitor, and sets to clone mode,
close the lid, and reopen.  Then you'll find the video mode is now
external-monitor only.  It's because two XF86VideoSwitch key events
are issued during close/open the lid.


thanks,

Takashi

Re: [PATCH] ACPI video: Add video_switch_key option

[Matthew Garrett]

On Wed, Nov 24, 2010 at 03:31:11PM +0100, Takashi Iwai wrote:

> Typically GNOME changes to the next monitor mode (e.g. xinearama,
> clone, laptop-only, external-only) by each XF86VideoSwitch key event.
> If you connect your laptop to a monitor, and sets to clone mode,
> close the lid, and reopen.  Then you'll find the video mode is now
> external-monitor only.  It's because two XF86VideoSwitch key events
> are issued during close/open the lid.

That's not what that keystroke is expected to do. Userspace needs to 
either interpret it as "Reprobe devices" or pay attention to the 
configuration that the firmware requested.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

At Wed, 24 Nov 2010 14:35:17 +0000,
Matthew Garrett wrote:
> 
> On Wed, Nov 24, 2010 at 03:31:11PM +0100, Takashi Iwai wrote:
> 
> > Typically GNOME changes to the next monitor mode (e.g. xinearama,
> > clone, laptop-only, external-only) by each XF86VideoSwitch key event.
> > If you connect your laptop to a monitor, and sets to clone mode,
> > close the lid, and reopen.  Then you'll find the video mode is now
> > external-monitor only.  It's because two XF86VideoSwitch key events
> > are issued during close/open the lid.
> 
> That's not what that keystroke is expected to do. Userspace needs to 
> either interpret it as "Reprobe devices" or pay attention to the 
> configuration that the firmware requested.

Unfortunately, this is the standard behavior on most desktops :-<
Fn-F4 (or whatever) media key produces this exact key code, and the
desktops (including Windows) expect this behavior.


thanks,

Takashi

Re: [PATCH] ACPI video: Add video_switch_key option

[Matthew Garrett]

On Wed, Nov 24, 2010 at 03:38:05PM +0100, Takashi Iwai wrote:

> Unfortunately, this is the standard behavior on most desktops :-<
> Fn-F4 (or whatever) media key produces this exact key code, and the
> desktops (including Windows) expect this behavior.

Hm. Ok, let's make sure that we're doing this right. There's several 
different notifications which can generate the same keycode 
(ACPI_VIDEO_NOTIFY_SWITCH, ACPI_VIDEO_NOTIFY_PROBE and 
ACPI_VIDEO_NOTIFY_CYCLE) - which of them's actually being triggered 
here?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

At Wed, 24 Nov 2010 14:42:39 +0000,
Matthew Garrett wrote:
> 
> On Wed, Nov 24, 2010 at 03:38:05PM +0100, Takashi Iwai wrote:
> 
> > Unfortunately, this is the standard behavior on most desktops :-<
> > Fn-F4 (or whatever) media key produces this exact key code, and the
> > desktops (including Windows) expect this behavior.
> 
> Hm. Ok, let's make sure that we're doing this right. There's several 
> different notifications which can generate the same keycode 
> (ACPI_VIDEO_NOTIFY_SWITCH, ACPI_VIDEO_NOTIFY_PROBE and 
> ACPI_VIDEO_NOTIFY_CYCLE) - which of them's actually being triggered 
> here?

It's ACPI_VIDEO_NOTIFY_SWITCH.

But, note that I'm checking only HP laptops.  There might be other
laptops behaving in a different manner.


thanks,

Takashi

Re: [PATCH] ACPI video: Add video_switch_key option

[Matthew Garrett]

Ok. Can you give me an example of a model with this behaviour?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI video: Add video_switch_key option

[Takashi Iwai]

At Wed, 24 Nov 2010 14:54:08 +0000,
Matthew Garrett wrote:
> 
> Ok. Can you give me an example of a model with this behaviour?

HP ProBook 6xxx, HP ProBook 4xxx, HP EliteBook, HP Compaq 65xx, HP
Compaq 67xx, and lots of other HP laptops sold in this year.  (I have
access to two dozen of machines here, but unfortunately official names
are not always known.)


thanks,

Takashi

Re: [PATCH] ACPI video: Add video_switch_key option

[Matthew Garrett]

On Wed, Nov 24, 2010 at 04:04:30PM +0100, Takashi Iwai wrote:
> At Wed, 24 Nov 2010 14:54:08 +0000,
> Matthew Garrett wrote:
> > 
> > Ok. Can you give me an example of a model with this behaviour?
> 
> HP ProBook 6xxx, HP ProBook 4xxx, HP EliteBook, HP Compaq 65xx, HP
> Compaq 67xx, and lots of other HP laptops sold in this year.  (I have
> access to two dozen of machines here, but unfortunately official names
> are not always known.)

Can you check what _DOS is set to? I'm having a little trouble 
reproducing this here on an HP 2530p.

-- 
Matthew Garrett | mjg59@srcf.ucam.org