Hi, > From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] > Subject: Re: [RFC PATCH v3 5/5] ACPI: button: Always notify kernel space > using _LID returning value > > 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-...@lists.freedesktop.org> > > Cc: <nouv...@lists.freedesktop.org> > > Cc: Benjamin Tissoires <benjamin.tissoi...@redhat.com> > > Cc: Peter Hutterer <peter.hutte...@who-t.net> > > Signed-off-by: Lv Zheng <lv.zh...@intel.com> > > --- > > 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.
OK, I'll drop i915 related changes. However I can see cleanup chances in button.c. I feel I should at least do minimal tunings in button driver to allow future improvements. Cheers, Lv > Cheers, > Benjamin
