Re: [RFC PATCH v3 5/5] ACPI: button: Always notify kernel space using _LID returning value

[Benjamin Tissoires]

Hi Lv,

On May 27 2017 or thereabouts, Lv Zheng wrote:
> Both nouveau and i915, the only 2 kernel space lid notification listeners,
> invoke acpi_lid_open() API to obtain _LID returning value instead of using
> the notified value.
> 
> So this patch moves this logic from listeners to lid driver, always notify
> kernel space listeners using _LID returning value.
> 
> This is a no-op cleanup, but facilitates administrators to configure to
> notify kernel drivers with faked lid init states via command line
> "button.lid_notify_init_state=Y".
> 
> Cc: <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>
> Cc: <nouveau@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> Cc: Peter Hutterer <peter.hutterer@xxxxxxxxx>
> Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx>
> ---
>  drivers/acpi/button.c             | 16 ++++++++++++++--
>  drivers/gpu/drm/i915/intel_lvds.c |  2 +-
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 4abf8ae..e047d34 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -119,6 +119,9 @@ static u8 lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
>  static unsigned long lid_report_interval __read_mostly = 500;
>  module_param(lid_report_interval, ulong, 0644);
>  MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
> +static bool lid_notify_init_state __read_mostly = false;
> +module_param(lid_notify_init_state, bool, 0644);
> +MODULE_PARM_DESC(lid_notify_init_state, "Notify init lid state to kernel drivers after boot/resume");
>  
>  /* --------------------------------------------------------------------------
>                                FS Interface (/proc)
> @@ -224,6 +227,15 @@ static void acpi_lid_notify_state(struct acpi_device *device,
>  	if (state)
>  		pm_wakeup_event(&device->dev, 0);
>  
> +	if (!lid_notify_init_state) {
> +		/*
> +		 * There are cases "state" is not a _LID return value, so
> +		 * correct it before notification.
> +		 */
> +		if (!bios_notify &&
> +		    lid_init_state != ACPI_BUTTON_LID_INIT_METHOD)
> +			state = acpi_lid_evaluate_state(device);
> +	}
>  	acpi_lid_notifier_call(device, state);
>  }
>  
> @@ -572,10 +584,10 @@ static int param_set_lid_init_state(const char *val, struct kernel_param *kp)
>  
>  	if (!strncmp(val, "open", sizeof("open") - 1)) {
>  		lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
> -		pr_info("Notify initial lid state as open\n");
> +		pr_info("Notify initial lid state to users space as open and kernel drivers with _LID return value\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");
> +		pr_info("Notify initial lid state to user/kernel space 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");
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 9ca4dc4..8ca9080 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -548,7 +548,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
>  	/* Don't force modeset on machines where it causes a GPU lockup */
>  	if (dmi_check_system(intel_no_modeset_on_lid))
>  		goto exit;
> -	if (!acpi_lid_open()) {
> +	if (!val) {
>  		/* do modeset on next lid open event */
>  		dev_priv->modeset_restore = MODESET_ON_LID_OPEN;
>  		goto exit;

This last hunk should really be in its own patch because the intel GPU
folks would need to apply the rest of the series for their CI suite, and
also because there is no reason for this change to be alongside any
other acpi/button.c change.

Cheers,
Benjamin

> -- 
> 2.7.4
>