[RFC PATCH] report acpi video hot key event through input device

[RFC PATCH] report acpi video hot key event through input device

[Luming Yu]

[-- Attachment #1: Type: text/plain, Size: 5980 bytes --]

report acpi video hot key event through input device

Signed-off-by: Luming Yu <Luming.yu@intel.com>
--
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 00d25b3..03d84db 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -31,6 +31,7 @@
 #include <linux/list.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/input.h>

 #include <linux/backlight.h>
 #include <asm/uaccess.h>
@@ -131,6 +132,8 @@ struct acpi_video_bus {
 	struct semaphore sem;
 	struct list_head video_device_list;
 	struct proc_dir_entry *dir;
+	struct input_dev *input;
+	char phys[32];			/* for input device */
 };

 struct acpi_video_device_flags {
@@ -1714,6 +1717,8 @@ static void acpi_video_bus_notify(acpi_h
 {
 	struct acpi_video_bus *video = data;
 	struct acpi_device *device = NULL;
+	struct input_dev *input;
+	int keycode;

 	printk("video bus notify\n");

@@ -1721,11 +1726,13 @@ static void acpi_video_bus_notify(acpi_h
 		return;

 	device = video->device;
+	input = video->input;

 	switch (event) {
 	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
 					 * most likely via hotkey. */
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;

 	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
@@ -1734,21 +1741,37 @@ static void acpi_video_bus_notify(acpi_h
 		acpi_video_device_rebind(video);
 		acpi_video_switch_output(video, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;

 	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
+		acpi_video_switch_output(video, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
+		acpi_video_switch_output(video, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_VIDEO_NEXT;
+		break;
 	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output
hotkey pressed. */
 		acpi_video_switch_output(video, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_VIDEO_PREV;
 		break;

 	default:
+		keycode = KEY_UNKNOWN;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}

+	input_report_key(input, keycode, 1);
+	input_sync(input);
+	input_report_key(input, keycode, 0);
+	input_sync(input);
+
 	return;
 }

@@ -1756,30 +1779,60 @@ static void acpi_video_device_notify(acp
 {
 	struct acpi_video_device *video_device = data;
 	struct acpi_device *device = NULL;
+	struct acpi_video_bus *bus;
+	struct input_dev *input;
+	int keycode;

 	if (!video_device)
 		return;

 	device = video_device->dev;
+	bus = video_device->video;
+	input = bus->input;

 	switch (event) {
 	case ACPI_VIDEO_NOTIFY_SWITCH:	/* change in status (cycle output device) */
 	case ACPI_VIDEO_NOTIFY_PROBE:	/* change in status (output device status) */
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;
 	case ACPI_VIDEO_NOTIFY_CYCLE_BRIGHTNESS:	/* Cycle brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS:	/* Increase brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_UP;
+		break;
 	case ACPI_VIDEO_NOTIFY_DEC_BRIGHTNESS:	/* Decrease brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_DOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_ZERO_BRIGHTNESS:	/* zero brightnesss */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_ZERO;
+		break;
 	case ACPI_VIDEO_NOTIFY_DISPLAY_OFF:	/* display device off */
 		acpi_video_switch_brightness(video_device, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_OFF;
 		break;
 	default:
+		keycode = KEY_UNKNOWN;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}
+
+	input_report_key(input, keycode, 1);
+	input_sync(input);
+	input_report_key(input, keycode, 0);
+	input_sync(input);
+
 	return;
 }

@@ -1788,7 +1841,7 @@ static int acpi_video_bus_add(struct acp
 	int result = 0;
 	acpi_status status = 0;
 	struct acpi_video_bus *video = NULL;
-
+	struct input_dev *input;

 	if (!device)
 		return -EINVAL;
@@ -1830,7 +1883,18 @@ static int acpi_video_bus_add(struct acp
 		result = -ENODEV;
 		goto end;
 	}
-
+	
+	video->input = input = input_allocate_device();
+
+	snprintf(video->phys, sizeof(video->phys),
+		 "%s/video/input0", acpi_device_bid(video->device));
+
+	input->name = acpi_device_name(video->device);
+	input->phys = video->phys;
+	input->id.bustype = BUS_HOST;
+	input->id.product = 0x06;
+	input->evbit[0] = BIT(EV_KEY);
+	input_register_device(input);
 	printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s  rom: %s  post: %s)\n",
 	       ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device),
 	       video->flags.multihead ? "yes" : "no",
@@ -1863,7 +1927,7 @@ static int acpi_video_bus_remove(struct

 	acpi_video_bus_put_devices(video);
 	acpi_video_bus_remove_fs(device);
-
+	input_unregister_device(video->input);
 	kfree(video->attached_array);
 	kfree(video);

diff --git a/include/linux/input.h b/include/linux/input.h
index be2bf3a..2ccdb50 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -544,6 +544,13 @@ struct input_absinfo {
 #define KEY_BRL_DOT7		0x1f7
 #define KEY_BRL_DOT8		0x1f8

+#define KEY_VIDEO_NEXT		0x1f9
+#define KEY_VIDEO_PREV		0x1fa
+#define KEY_BRIGHTNESS_UP	0x1fb
+#define KEY_BRIGHTNESS_DOWN	0x1fc
+#define KEY_BRIGHTNESS_ZERO	0x1fd
+#define KEY_BRIGHTNESS_OFF	0x1fe
+
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING	KEY_MUTE
 #define KEY_MAX			0x1ff

[-- Attachment #2: ev.patch --]
[-- Type: text/x-patch, Size: 5896 bytes --]

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 00d25b3..03d84db 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -31,6 +31,7 @@
 #include <linux/list.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/input.h>
 
 #include <linux/backlight.h>
 #include <asm/uaccess.h>
@@ -131,6 +132,8 @@ struct acpi_video_bus {
 	struct semaphore sem;
 	struct list_head video_device_list;
 	struct proc_dir_entry *dir;
+	struct input_dev *input;
+	char phys[32];			/* for input device */
 };
 
 struct acpi_video_device_flags {
@@ -1714,6 +1717,8 @@ static void acpi_video_bus_notify(acpi_h
 {
 	struct acpi_video_bus *video = data;
 	struct acpi_device *device = NULL;
+	struct input_dev *input;
+	int keycode; 
 
 	printk("video bus notify\n");
 
@@ -1721,11 +1726,13 @@ static void acpi_video_bus_notify(acpi_h
 		return;
 
 	device = video->device;
+	input = video->input;
 
 	switch (event) {
 	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
 					 * most likely via hotkey. */
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;
 
 	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
@@ -1734,21 +1741,37 @@ static void acpi_video_bus_notify(acpi_h
 		acpi_video_device_rebind(video);
 		acpi_video_switch_output(video, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;
 
 	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
+		acpi_video_switch_output(video, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
+		acpi_video_switch_output(video, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_VIDEO_NEXT;
+		break;
 	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output hotkey pressed. */
 		acpi_video_switch_output(video, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_VIDEO_PREV;
 		break;
 
 	default:
+		keycode = KEY_UNKNOWN;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}
 
+	input_report_key(input, keycode, 1);
+	input_sync(input);
+	input_report_key(input, keycode, 0);
+	input_sync(input);
+
 	return;
 }
 
@@ -1756,30 +1779,60 @@ static void acpi_video_device_notify(acp
 {
 	struct acpi_video_device *video_device = data;
 	struct acpi_device *device = NULL;
+	struct acpi_video_bus *bus;
+	struct input_dev *input;
+	int keycode;
 
 	if (!video_device)
 		return;
 
 	device = video_device->dev;
+	bus = video_device->video;
+	input = bus->input;
 
 	switch (event) {
 	case ACPI_VIDEO_NOTIFY_SWITCH:	/* change in status (cycle output device) */
 	case ACPI_VIDEO_NOTIFY_PROBE:	/* change in status (output device status) */
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
 		break;
 	case ACPI_VIDEO_NOTIFY_CYCLE_BRIGHTNESS:	/* Cycle brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_UNKNOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS:	/* Increase brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_UP;
+		break;
 	case ACPI_VIDEO_NOTIFY_DEC_BRIGHTNESS:	/* Decrease brightness */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_DOWN;
+		break;
 	case ACPI_VIDEO_NOTIFY_ZERO_BRIGHTNESS:	/* zero brightnesss */
+		acpi_video_switch_brightness(video_device, event);
+		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_ZERO;
+		break;
 	case ACPI_VIDEO_NOTIFY_DISPLAY_OFF:	/* display device off */
 		acpi_video_switch_brightness(video_device, event);
 		acpi_bus_generate_event(device, event, 0);
+		keycode = KEY_BRIGHTNESS_OFF;
 		break;
 	default:
+		keycode = KEY_UNKNOWN;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}
+
+	input_report_key(input, keycode, 1);
+	input_sync(input);
+	input_report_key(input, keycode, 0);
+	input_sync(input);
+
 	return;
 }
 
@@ -1788,7 +1841,7 @@ static int acpi_video_bus_add(struct acp
 	int result = 0;
 	acpi_status status = 0;
 	struct acpi_video_bus *video = NULL;
-
+	struct input_dev *input;
 
 	if (!device)
 		return -EINVAL;
@@ -1830,7 +1883,18 @@ static int acpi_video_bus_add(struct acp
 		result = -ENODEV;
 		goto end;
 	}
-
+	
+	video->input = input = input_allocate_device();
+
+	snprintf(video->phys, sizeof(video->phys),
+		 "%s/video/input0", acpi_device_bid(video->device));
+
+	input->name = acpi_device_name(video->device);
+	input->phys = video->phys;
+	input->id.bustype = BUS_HOST;
+	input->id.product = 0x06;
+	input->evbit[0] = BIT(EV_KEY);
+	input_register_device(input);
 	printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s  rom: %s  post: %s)\n",
 	       ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device),
 	       video->flags.multihead ? "yes" : "no",
@@ -1863,7 +1927,7 @@ static int acpi_video_bus_remove(struct
 
 	acpi_video_bus_put_devices(video);
 	acpi_video_bus_remove_fs(device);
-
+	input_unregister_device(video->input);
 	kfree(video->attached_array);
 	kfree(video);
 
diff --git a/include/linux/input.h b/include/linux/input.h
index be2bf3a..2ccdb50 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -544,6 +544,13 @@ struct input_absinfo {
 #define KEY_BRL_DOT7		0x1f7
 #define KEY_BRL_DOT8		0x1f8
 
+#define KEY_VIDEO_NEXT		0x1f9
+#define KEY_VIDEO_PREV		0x1fa
+#define KEY_BRIGHTNESS_UP	0x1fb
+#define KEY_BRIGHTNESS_DOWN	0x1fc
+#define KEY_BRIGHTNESS_ZERO	0x1fd
+#define KEY_BRIGHTNESS_OFF	0x1fe
+
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING	KEY_MUTE
 #define KEY_MAX			0x1ff

Re: [RFC PATCH] report acpi video hot key event through input device

[Matthew Garrett]

On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
>  	switch (event) {
>  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
>  					 * most likely via hotkey. */
>  		acpi_bus_generate_event(device, event, 0);
> +		keycode = KEY_UNKNOWN;

KEY_SWITCHVIDEOMODE ?

> 
>  	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
> @@ -1734,21 +1741,37 @@ static void acpi_video_bus_notify(acpi_h
>  		acpi_video_device_rebind(video);
>  		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_event(device, event, 0);
> +		keycode = KEY_UNKNOWN;
>  		break;

Here too.

>  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* change in status (cycle output device) */
>  	case ACPI_VIDEO_NOTIFY_PROBE:	/* change in status (output device status) */
>  		acpi_bus_generate_event(device, event, 0);
> +		keycode = KEY_UNKNOWN;
>  		break;

And these? I'm not sure in this case, though.

> +#define KEY_VIDEO_NEXT		0x1f9
> +#define KEY_VIDEO_PREV		0x1fa
> +#define KEY_BRIGHTNESS_UP	0x1fb
> +#define KEY_BRIGHTNESS_DOWN	0x1fc

What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?

> +#define KEY_BRIGHTNESS_ZERO	0x1fd
> +#define KEY_BRIGHTNESS_OFF	0x1fe

You almost certainly want to go via linux-input if you're adding new 
keycodes.

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

Re: [RFC PATCH] report acpi video hot key event through input device

[Richard Hughes]

On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> >       switch (event) {
> >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
> >                                        * most likely via hotkey. */
> >               acpi_bus_generate_event(device, event, 0);
> > +             keycode = KEY_UNKNOWN;
>
> KEY_SWITCHVIDEOMODE ?

Yes, please use this. This is what other ACPI drivers are using.

> What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?

Agree, this matches other drivers like thinkpad_acpi and sony-laptop.

> > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > +#define KEY_BRIGHTNESS_OFF   0x1fe
>
> You almost certainly want to go via linux-input if you're adding new
> keycodes.

Yup - do we have to distinguish between zero and off? Or is off
"switch panel backlight off" and zero is "switch to lowest brightness
level"?

Thanks for doing this, this is another part or the puzzle to make keys
just work in userspace and is very appreciated.

Richard.

Re: [RFC PATCH] report acpi video hot key event through input device

[Dmitry Torokhov]

Hi,

On 7/30/07, Richard Hughes <hughsient@gmail.com> wrote:
> On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > >       switch (event) {
> > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
> > >                                        * most likely via hotkey. */
> > >               acpi_bus_generate_event(device, event, 0);
> > > +             keycode = KEY_UNKNOWN;
> >
> > KEY_SWITCHVIDEOMODE ?
>
> Yes, please use this. This is what other ACPI drivers are using.
>
> > What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?
>
> Agree, this matches other drivers like thinkpad_acpi and sony-laptop.
>
> > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> >
> > You almost certainly want to go via linux-input if you're adding new
> > keycodes.
>
> Yup - do we have to distinguish between zero and off? Or is off
> "switch panel backlight off" and zero is "switch to lowest brightness
> level"?
>
> Thanks for doing this, this is another part or the puzzle to make keys
> just work in userspace and is very appreciated.
>

I have some concerns over this patch. Some of the events reported are
not the regular key presses that something needs to process and
perform an action. Consider ACPI_VIDEO_NOTIFY_SWITCH is "Used to
notify OSPM whenever the state of one of the output devices attached
to the VGA controller has been switched or toggled." As far as I
understand the event (whatever it was) was already processed by
firmware and we getting a notification event of system change. So I am
not sure that this event should be sent as a key event.

The brightness events on the other hand seem to be perfectly suited
for an input device (unless are are also already processed by the
firmware and action has already been taken).

-- 
Dmitry

Re: [RFC PATCH] report acpi video hot key event through input device

[Henrique de Moraes Holschuh]

On Mon, 30 Jul 2007, Dmitry Torokhov wrote:
> I have some concerns over this patch. Some of the events reported are
> not the regular key presses that something needs to process and
> perform an action. Consider ACPI_VIDEO_NOTIFY_SWITCH is "Used to
> notify OSPM whenever the state of one of the output devices attached
> to the VGA controller has been switched or toggled." As far as I
> understand the event (whatever it was) was already processed by
> firmware and we getting a notification event of system change. So I am
> not sure that this event should be sent as a key event.

This is the whole "passive and active input events" crap I went over with
thinkpad-acpi not too long ago.

Can we please get a formal documentation in input of how it should be done,
and declare anything that doesn't follow it "buggy"?

You know my position on it: a new event type, or not producing such events
at all.  But I will follow whatever rule the input layer sets.

> The brightness events on the other hand seem to be perfectly suited
> for an input device (unless are are also already processed by the
> firmware and action has already been taken).

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

Re: [RFC PATCH] report acpi video hot key event through input device

[Dmitry Torokhov]

On 7/31/07, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> On Mon, 30 Jul 2007, Dmitry Torokhov wrote:
> > I have some concerns over this patch. Some of the events reported are
> > not the regular key presses that something needs to process and
> > perform an action. Consider ACPI_VIDEO_NOTIFY_SWITCH is "Used to
> > notify OSPM whenever the state of one of the output devices attached
> > to the VGA controller has been switched or toggled." As far as I
> > understand the event (whatever it was) was already processed by
> > firmware and we getting a notification event of system change. So I am
> > not sure that this event should be sent as a key event.
>
> This is the whole "passive and active input events" crap I went over with
> thinkpad-acpi not too long ago.
>
> Can we please get a formal documentation in input of how it should be done,
> and declare anything that doesn't follow it "buggy"?
>
> You know my position on it: a new event type, or not producing such events
> at all.  But I will follow whatever rule the input layer sets.
>

My position is that generic event reporting (such as something has
already switched video output from LCD to CRT, network link lost,
battery low, brightness has already been changed) are outside of input
layer domain (it would be silly to attach struct input_dev to network
cards, wouldn't it?).

-- 
Dmitry

Re: [RFC PATCH] report acpi video hot key event through input device

[Henrique de Moraes Holschuh]

On Wed, 01 Aug 2007, Dmitry Torokhov wrote:
> On 7/31/07, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> > On Mon, 30 Jul 2007, Dmitry Torokhov wrote:
> > > I have some concerns over this patch. Some of the events reported are
> > > not the regular key presses that something needs to process and
> > > perform an action. Consider ACPI_VIDEO_NOTIFY_SWITCH is "Used to
> > > notify OSPM whenever the state of one of the output devices attached
> > > to the VGA controller has been switched or toggled." As far as I
> > > understand the event (whatever it was) was already processed by
> > > firmware and we getting a notification event of system change. So I am
> > > not sure that this event should be sent as a key event.
> >
> > This is the whole "passive and active input events" crap I went over with
> > thinkpad-acpi not too long ago.
> >
> > Can we please get a formal documentation in input of how it should be done,
> > and declare anything that doesn't follow it "buggy"?
> >
> > You know my position on it: a new event type, or not producing such events
> > at all.  But I will follow whatever rule the input layer sets.
> 
> My position is that generic event reporting (such as something has
> already switched video output from LCD to CRT, network link lost,
> battery low, brightness has already been changed) are outside of input
> layer domain (it would be silly to attach struct input_dev to network
> cards, wouldn't it?).

Can we get that documented *in-tree*? It is an issue that will keep coming
back.

Now, for the people who want these notifications (to implement status
tracking, OSDs, etc), we can setup another way to send them.  Which one?  I
know thinkpad-acpi has a ton of such notifications that it can issue, and
that userspace would like to get them for OSDs, and also to tell the user to
not go around undocking without pressing the undock lever AND waiting for
the all-clear signal, etc.

Matthew, Richard: would generic netlink sockets work for HAL ?  Is that a
way to make at least part of the messages "generic" (maybe reusing the input
layer defines?) instead of driver-specific?  Or should we continue using the
ACPI event mechanism?

-- 
  "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: [RFC PATCH] report acpi video hot key event through input device

[Matthew Garrett]

On Wed, Aug 01, 2007 at 09:52:47AM -0400, Dmitry Torokhov wrote:

> My position is that generic event reporting (such as something has
> already switched video output from LCD to CRT, network link lost,
> battery low, brightness has already been changed) are outside of input
> layer domain (it would be silly to attach struct input_dev to network
> cards, wouldn't it?).

I think that's an understandable position, but it's not one which 
matches reality in some cases (sadly). Most Dells generate an i8042 
scancode when you hit the brightness keys, even though they'll also 
change the brightness. They'll do the same for the wireless killswitch.

Personally, I think it's logical for physical keys to generate events 
though the input layer.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH] report acpi video hot key event through input device

[Zhang Rui]

On Mon, 2007-07-30 at 01:50 +0100, Matthew Garrett wrote:
> On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> >  	switch (event) {
> >  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
> >  					 * most likely via hotkey. */
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> 
> KEY_SWITCHVIDEOMODE ?
> 
> > 
> >  	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
> > @@ -1734,21 +1741,37 @@ static void acpi_video_bus_notify(acpi_h
> >  		acpi_video_device_rebind(video);
> >  		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> >  		break;
> 
> Here too.
> 
> >  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* change in status (cycle output device) */
> >  	case ACPI_VIDEO_NOTIFY_PROBE:	/* change in status (output device status) */
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> >  		break;
> 
> And these? I'm not sure in this case, though.

ACPI_VIDEO_NOTIFY_SWITCH and ACPI_VIDEO_NOTIFY_PROBE
are only valid for Display devices.
Should output devices handle this kind of events?
IMO, we should remove this piece of code.

Thanks,
Rui

Re: [RFC PATCH] report acpi video hot key event through input device

[Zhang Rui]

On Mon, 2007-07-30 at 12:47 +0100, Richard Hughes wrote:
> On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > >       switch (event) {
> > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
> > >                                        * most likely via hotkey. */
> > >               acpi_bus_generate_event(device, event, 0);
> > > +             keycode = KEY_UNKNOWN;
> >
> > KEY_SWITCHVIDEOMODE ?
> 
> Yes, please use this. This is what other ACPI drivers are using.
> 
> > What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?
> 
> Agree, this matches other drivers like thinkpad_acpi and sony-laptop.
> 
> > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> >
> > You almost certainly want to go via linux-input if you're adding new
> > keycodes.
> 
> Yup - do we have to distinguish between zero and off? Or is off
> "switch panel backlight off" and zero is "switch to lowest brightness
> level"?
> 
Hmm, I think they are slightly different.
IMO, "zero" equals _BCM(0), it means to turn off the backlight, but we
may still see the screen if there is a ambient light.
"off" means turn off the output device, i.e. set to D1/D2/D3 state. 

Thanks,
Rui

Re: [RFC PATCH] report acpi video hot key event through input device

[Zhang Rui]

On Mon, 2007-07-30 at 01:50 +0100, Matthew Garrett wrote:
> On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> >  	switch (event) {
> >  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* User requested a switch,
> >  					 * most likely via hotkey. */
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> 
> KEY_SWITCHVIDEOMODE ?
> 
> > 
> >  	case ACPI_VIDEO_NOTIFY_PROBE:	/* User plugged in or removed a video
> > @@ -1734,21 +1741,37 @@ static void acpi_video_bus_notify(acpi_h
> >  		acpi_video_device_rebind(video);
> >  		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> >  		break;
> 
> Here too.
> 
> >  	case ACPI_VIDEO_NOTIFY_SWITCH:	/* change in status (cycle output device) */
> >  	case ACPI_VIDEO_NOTIFY_PROBE:	/* change in status (output device status) */
> >  		acpi_bus_generate_event(device, event, 0);
> > +		keycode = KEY_UNKNOWN;
> >  		break;
> 
> And these? I'm not sure in this case, though.
> 
ACPI_VIDEO_NOTIFY_SWITCH and ACPI_VIDEO_NOTIFY_PROBE
are only valid for Display devices.
Should output devices handle this kind of events?

Thanks,
Rui

Re: [RFC PATCH] report acpi video hot key event through input device

[Zhang Rui]

On Mon, 2007-07-30 at 12:47 +0100, Richard Hughes wrote:
> On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > >       switch (event) {
> > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
> > >                                        * most likely via hotkey. */
> > >               acpi_bus_generate_event(device, event, 0);
> > > +             keycode = KEY_UNKNOWN;
> >
> > KEY_SWITCHVIDEOMODE ?
> 
> Yes, please use this. This is what other ACPI drivers are using.
> 
> > What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?
> 
> Agree, this matches other drivers like thinkpad_acpi and sony-laptop.
> 
> > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> >
> > You almost certainly want to go via linux-input if you're adding new
> > keycodes.
> 
> Yup - do we have to distinguish between zero and off? Or is off
> "switch panel backlight off" and zero is "switch to lowest brightness
> level"?
> 
Hmm, I think they are slightly different.
"zero" equals _BCM(0)
"off" means turn off the output device, i.e. set to D1/D2/D3 state. 

Thanks,
Rui

Re: [RFC PATCH] report acpi video hot key event through inputdevice

[Zhang Rui]

On Tue, 2007-08-14 at 19:58 +0800, Richard Hughes wrote:
> 
> On Wed, 2007-08-08 at 14:37 +0800, Zhang Rui wrote:
> > On Mon, 2007-07-30 at 12:47 +0100, Richard Hughes wrote:
> > > On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > > > >       switch (event) {
> > > > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a
> switch,
> > > > >                                        * most likely via
> hotkey. */
> > > > >               acpi_bus_generate_event(device, event, 0);
> > > > > +             keycode = KEY_UNKNOWN;
> > > >
> > > > KEY_SWITCHVIDEOMODE ?
> > >
> > > Yes, please use this. This is what other ACPI drivers are using.
> > >
> > > > What's wrong with the existing KEY_BRIGHTNESSDOWN and
> KEY_BRIGHTNESSUP?
> > >
> > > Agree, this matches other drivers like thinkpad_acpi and
> sony-laptop.
> > >
> > > > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> > > >
> > > > You almost certainly want to go via linux-input if you're adding
> new
> > > > keycodes.
> > >
> > > Yup - do we have to distinguish between zero and off? Or is off
> > > "switch panel backlight off" and zero is "switch to lowest
> brightness
> > > level"?
> > >
> > Hmm, I think they are slightly different.
> > IMO, "zero" equals _BCM(0), it means to turn off the backlight, but
> we
> > may still see the screen if there is a ambient light.
> > "off" means turn off the output device, i.e. set to D1/D2/D3 state.
> 
> Sure, so they are different states, but to a user they are both
> effectively the same "screen is off". Is there any use case to
I'm not sure of this.
But in some cases, I do can still see the screen although it's very dark
after I "echo 0 >/sys/class/backlight/acpi_video0/brightness".
IMO, this is because the ambient light is on.

Thanks,
Rui
>  removing the backlight without powering down the display?

Re: [RFC PATCH] report acpi video hot key event through input device

[Richard Hughes]

On Wed, 2007-08-08 at 14:37 +0800, Zhang Rui wrote:
> On Mon, 2007-07-30 at 12:47 +0100, Richard Hughes wrote:
> > On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > > >       switch (event) {
> > > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
> > > >                                        * most likely via hotkey. */
> > > >               acpi_bus_generate_event(device, event, 0);
> > > > +             keycode = KEY_UNKNOWN;
> > >
> > > KEY_SWITCHVIDEOMODE ?
> > 
> > Yes, please use this. This is what other ACPI drivers are using.
> > 
> > > What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?
> > 
> > Agree, this matches other drivers like thinkpad_acpi and sony-laptop.
> > 
> > > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> > >
> > > You almost certainly want to go via linux-input if you're adding new
> > > keycodes.
> > 
> > Yup - do we have to distinguish between zero and off? Or is off
> > "switch panel backlight off" and zero is "switch to lowest brightness
> > level"?
> > 
> Hmm, I think they are slightly different.
> IMO, "zero" equals _BCM(0), it means to turn off the backlight, but we
> may still see the screen if there is a ambient light.
> "off" means turn off the output device, i.e. set to D1/D2/D3 state. 

Sure, so they are different states, but to a user they are both
effectively the same "screen is off". Is there any use case to removing
the backlight without powering down the display?

Richard.

Re: [RFC PATCH] report acpi video hot key event through inputdevice

[Zhang Rui]

On Tue, 2007-08-14 at 18:43 +0800, Zhang Rui wrote:
> On Tue, 2007-08-14 at 19:58 +0800, Richard Hughes wrote:
> > 
> > On Wed, 2007-08-08 at 14:37 +0800, Zhang Rui wrote:
> > > On Mon, 2007-07-30 at 12:47 +0100, Richard Hughes wrote:
> > > > On 30/07/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > > > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > > > > >       switch (event) {
> > > > > >       case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a
> > switch,
> > > > > >                                        * most likely via
> > hotkey. */
> > > > > >               acpi_bus_generate_event(device, event, 0);
> > > > > > +             keycode = KEY_UNKNOWN;
> > > > >
> > > > > KEY_SWITCHVIDEOMODE ?
> > > >
> > > > Yes, please use this. This is what other ACPI drivers are using.
> > > >
> > > > > What's wrong with the existing KEY_BRIGHTNESSDOWN and
> > KEY_BRIGHTNESSUP?
> > > >
> > > > Agree, this matches other drivers like thinkpad_acpi and
> > sony-laptop.
> > > >
> > > > > > +#define KEY_BRIGHTNESS_ZERO  0x1fd
> > > > > > +#define KEY_BRIGHTNESS_OFF   0x1fe
> > > > >
> > > > > You almost certainly want to go via linux-input if you're adding
> > new
> > > > > keycodes.
> > > >
> > > > Yup - do we have to distinguish between zero and off? Or is off
> > > > "switch panel backlight off" and zero is "switch to lowest
> > brightness
> > > > level"?
> > > >
> > > Hmm, I think they are slightly different.
> > > IMO, "zero" equals _BCM(0), it means to turn off the backlight, but
> > we
> > > may still see the screen if there is a ambient light.
> > > "off" means turn off the output device, i.e. set to D1/D2/D3 state.
> > 
> > Sure, so they are different states, but to a user they are both
> > effectively the same "screen is off". Is there any use case to
> I'm not sure of this.
> But in some cases, I do can still see the screen although it's very dark
> after I "echo 0 >/sys/class/backlight/acpi_video0/brightness".
> IMO, this is because the ambient light is on.
> 
Oops, I misunderstood what you mean. ;-p
> >  Is there any use case to removing the backlight without powering down the display?
I don't know.
Maybe "brightness zero" and "display device off" mean the same to a
user, but they are quite different to others, like the Graphics driver.
It still has to work when "brightness zero" because it's still
displaying, but it can throttle down/shutdown some parts when the
display device is off.
It's reasonable to send them as different input events.

Thanks,
Rui