[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1464778871.2432.12.camel@hadess.net>
Date: Wed, 01 Jun 2016 13:01:11 +0200
From: Bastien Nocera <hadess@...ess.net>
To: Lv Zheng <lv.zheng@...el.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <len.brown@...el.com>
Cc: Lv Zheng <zetalog@...il.com>, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org,
Benjamin Tissoires <benjamin.tissoires@...il.com>
Subject: Re: [PATCH v3 3/3] ACPI / button: Add quirks for initial lid state
notification
On Wed, 2016-06-01 at 18:10 +0800, Lv Zheng wrote:
> Linux userspace (systemd-logind) keeps on rechecking lid state when the
> lid state is closed. If it failed to update the lid state to open after
> boot/resume, the system suspending right after the boot/resume could be
> resulted.
> Graphics drivers also uses the lid notifications to implment
> MODESET_ON_LID_OPEN option.
"implement"
> Before the situation is improved from the userspace and from the graphics
> driver, users can simply configure ACPI button driver to send initial
> "open" lid state using button.lid_init_state=open to avoid such kind of
> issues. And our ultimate target should be making
> button.lid_init_state=ignore the default behavior. This patch implements
> the 2 options and keep the old behavior (button.lid_init_state=method).
I still don't think it's reasonable to expect any changes in user-space
unless you start documenting what the API to user-space actually is.
(I work on UPower, which also exports that information, and which gets
used in gnome-settings-daemon in a number of ways)
> Link 1: https://lkml.org/2016/3/7/460
> Link 2: https://github.com/systemd/systemd/issues/2087
> Signed-off-by: Lv Zheng <lv.zheng@...el.com>
> Cc: Bastien Nocera: <hadess@...ess.net>
> Cc: Benjamin Tissoires <benjamin.tissoires@...il.com>
> ---
> drivers/acpi/button.c | 61
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 6e291c1..148f4e5 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -53,6 +53,10 @@
> #define ACPI_BUTTON_DEVICE_NAME_LID "Lid Switch"
> #define ACPI_BUTTON_TYPE_LID 0x05
>
> +#define ACPI_BUTTON_LID_INIT_IGNORE 0x00
> +#define ACPI_BUTTON_LID_INIT_OPEN 0x01
> +#define ACPI_BUTTON_LID_INIT_METHOD 0x02
> +
> #define _COMPONENT ACPI_BUTTON_COMPONENT
> ACPI_MODULE_NAME("button");
>
> @@ -105,6 +109,7 @@ struct acpi_button {
>
> static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
> static struct acpi_device *lid_device;
> +static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
>
> /* ---------------------------------------------------------------
> -----------
> FS Interface (/proc)
> @@ -285,6 +290,21 @@ static int acpi_lid_update_state(struct
> acpi_device *device)
> return acpi_lid_notify_state(device, state);
> }
>
> +static void acpi_lid_initialize_state(struct acpi_device *device)
> +{
> + switch (lid_init_state) {
> + case ACPI_BUTTON_LID_INIT_OPEN:
> + (void)acpi_lid_notify_state(device, 1);
> + break;
> + case ACPI_BUTTON_LID_INIT_METHOD:
> + (void)acpi_lid_update_state(device);
> + break;
> + case ACPI_BUTTON_LID_INIT_IGNORE:
> + default:
> + break;
> + }
> +}
> +
> static void acpi_button_notify(struct acpi_device *device, u32
> event)
> {
> struct acpi_button *button = acpi_driver_data(device);
> @@ -341,6 +361,8 @@ static int acpi_button_resume(struct device *dev)
> struct acpi_button *button = acpi_driver_data(device);
>
> button->suspended = false;
> + if (button->type == ACPI_BUTTON_TYPE_LID)
> + acpi_lid_initialize_state(device);
> return 0;
> }
> #endif
> @@ -421,6 +443,7 @@ static int acpi_button_add(struct acpi_device
> *device)
> if (error)
> goto err_remove_fs;
> if (button->type == ACPI_BUTTON_TYPE_LID) {
> + acpi_lid_initialize_state(device);
> /*
> * This assumes there's only one lid device, or if
> there are
> * more we only care about the last one...
> @@ -450,4 +473,42 @@ static int acpi_button_remove(struct acpi_device
> *device)
> return 0;
> }
>
> +static int param_set_lid_init_state(const char *val, struct
> kernel_param *kp)
> +{
> + int result = 0;
> +
> + if (!strncmp(val, "open", sizeof("open") - 1)) {
> + lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
> + pr_info("Notify initial lid state as open\n");
> + } else if (!strncmp(val, "method", sizeof("method") - 1)) {
> + lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
> + pr_info("Notify initial lid state with _LID return
> value\n");
> + } else if (!strncmp(val, "ignore", sizeof("ignore") - 1)) {
> + lid_init_state = ACPI_BUTTON_LID_INIT_IGNORE;
> + pr_info("Do not notify initial lid state\n");
> + } else
> + result = -EINVAL;
> + return result;
> +}
> +
> +static int param_get_lid_init_state(char *buffer, struct
> kernel_param *kp)
> +{
> + switch (lid_init_state) {
> + case ACPI_BUTTON_LID_INIT_OPEN:
> + return sprintf(buffer, "open");
> + case ACPI_BUTTON_LID_INIT_METHOD:
> + return sprintf(buffer, "method");
> + case ACPI_BUTTON_LID_INIT_IGNORE:
> + return sprintf(buffer, "ignore");
> + default:
> + return sprintf(buffer, "invalid");
> + }
> + return 0;
> +}
> +
> +module_param_call(lid_init_state,
> + param_set_lid_init_state,
> param_get_lid_init_state,
> + NULL, 0644);
> +MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initial
> state");
> +
> module_acpi_driver(acpi_button_driver);
Powered by blists - more mailing lists