Hi Daniel,
On Saturday 16 June 2007 20:03, Daniel Drake wrote:
> Many Dell laptops have the DSDT coded to power down the display when the lid
> is closed, and leave it off when it is opened.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=5155
>
> Based on ideas from Len Brown and Dmitry Torokhov, this patch creates an input
> handler in the video driver which monitors for lid input events. When a lid
> open event is detected, the video driver reactivates the LCD.
>
Appears to be working on my Inspiron 8100. Couple of comments:
> +
> +static int lid_connect(struct input_handler *handler, struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct acpi_video_bus *video = handler->private;
> + int r;
> +
> + if (dev->id.product != ACPI_BUTTON_TYPE_LID ||
> + strcmp(dev->name, ACPI_BUTTON_DEVICE_NAME_LID) != 0)
> + return -ENODEV;
> +
> + if (video->lid_handle != NULL) {
> + printk(KERN_ERR PREFIX "trying to bind to multiple lids?\n");
> + return -ENODEV;
If we do not support multiple LIDs -EBUSY would be a better error code.
> + }
> +
> + video->lid_handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> + if (!video->lid_handle)
> + return -ENOMEM;
> + video->lid_handle->dev = dev;
> + video->lid_handle->handler = handler;
> + video->lid_handle->private = video;
Need to setup handler->name, otherwise "cat /proc/bus/input/devices" looks
"interesting".
> +
> +static void lid_event(struct input_handle *handle, unsigned int type,
> + unsigned int code, int value)
> +{
> + struct acpi_video_device *dev, *tmp;
> + struct acpi_video_bus *video = handle->private;
> +
> + if (type != EV_SW || value != 0)
> + return;
> +
> + list_for_each_entry_safe(dev, tmp, &video->video_device_list, entry) {
Safe from what? I don't see anything altering list state in the body
of this loop. Where's list locking? Also, once input core locking is
in place handler_>event will be called under a spinlock with interrupts
off. Can acpi_video_device_set_state be used in this case?
Below is a small cleanup patch you may want to fold into yours.
But I guess more important question if this is a good solution overall.
My box already generates video bus events when I close and open the lid
so maybe video.c should just call acpi_video__device_set_state() from
acpi_video_bus_notify()?
--
Dmitry
Signed-off-by: Dmitry Torokhov <[EMAIL PROTECTED]>
---
drivers/acpi/video.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
Index: work/drivers/acpi/video.c
===================================================================
--- work.orig/drivers/acpi/video.c
+++ work/drivers/acpi/video.c
@@ -1737,6 +1737,7 @@ static int lid_connect(struct input_hand
const struct input_device_id *id)
{
struct acpi_video_bus *video = handler->private;
+ struct input_handle *lid_handle;
int r;
if (dev->id.product != ACPI_BUTTON_TYPE_LID ||
@@ -1748,18 +1749,22 @@ static int lid_connect(struct input_hand
return -ENODEV;
}
- video->lid_handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
- if (!video->lid_handle)
+ lid_handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+ if (!lid_handle)
return -ENOMEM;
- video->lid_handle->dev = dev;
- video->lid_handle->handler = handler;
- video->lid_handle->private = video;
- r = input_register_handle(video->lid_handle);
+ video->lid_handle = lid_handle;
+
+ lid_handle->dev = dev;
+ lid_handle->handler = handler;
+ lid_handle->name = "acpi-video";
+ lid_handle->private = video;
+
+ r = input_register_handle(lid_handle);
if (r)
goto err;
- r = input_open_device(video->lid_handle);
+ r = input_open_device(lid_handle);
if (r)
goto err_unregister;
@@ -1777,9 +1782,10 @@ err:
static void lid_disconnect(struct input_handle *handle)
{
struct acpi_video_bus *video = handle->private;
- input_unregister_handle(handle);
+
input_close_device(handle);
- kfree(video->lid_handle);
+ input_unregister_handle(handle);
+ kfree(handle);
video->lid_handle = NULL;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
