Hi,
On 18-12-15 08:10, Michał Kępień wrote:
To clear things up, here is the current state of affairs:
* acpi_backlight=native solves the flickering issue, but doesn't help
with key event "lagging" after WMI is enabled,
* using the patch provided by Hans (or a similar one), the "lagged"
events can be filtered, leaving key event generation to dell-wmi,
* dell-wmi won't generate events unless acpi_backlight=vendor, which
in turn breaks brightness control due to faulty ASLE requests.
To break out of this loop, I suggest that:
* acpi_backlight should default to "native" for Vostro V131, based on
a DMI check,
Ack.
* brightness key event generation performed by the ACPI video driver
should always be suppressed on Vostro V131, because it's faulty due
to firmware bugs (and correct events will be reported anyway by
either i8042 or WMI),
s/should always be suppressed/should be suppressed by default/ otherwise
ack. We can easily do this the same way we currently deal with the
disable_backlight_sysfs_if option in acpi_video.c
* yet another quirk should be added to dell-wmi, so that brightness
key events are generated not only when acpi_backlight=vendor, but
also when acpi_backlight=native.
Nack, the real problem here is that checking if acpi_backlight!=vendor
is the wrong way to check if key event generation should be suppressed.
Well, I'm sure you know better, given that you wrote the code that this
new patch series fixes :)
This actually reminds me that I've the following item on my
todo list for a while but I've not gotten around to it yet:
-Add an acpi_video_handles_key_presses() helper, and use this where
appropriate (dell-wmi and others).
The mean reason for that item on my todo list was to make code doing
brigthness key-event suppression more readable. But we can also use
it for this case, if we check the new video.report_key_events parameter
in the acpi_video_handles_key_presses() helper, and switch dell-wmi
over to this helper, things will just work without needing yet another
quirk in dell-wmi :)
I've written a patch-set implementing this (attached), this obsoletes
my previous patch. As before, please test this with:
acpi_backlight=native video.report_key_events=1
On the kernel cmdline, we can add a patch adding dmi quirks to make
those the default later.
I tested the patch series you submitted and it seems to work fine for
me, given that proper kernel parameters are provided.
For that patch I'm going to need the dmi strings for your laptop,
can you please do:
for i in /sys/class/dmi/id/*_*; do echo $i; cat $i; done
And then include the output in your next mail ? Feel free to leave
out the serial numbers, asset tags and uuids, those are potentially
privacy sensitive and I don't need them.
/sys/class/dmi/id/bios_date
06/12/2014
/sys/class/dmi/id/bios_vendor
Dell Inc.
/sys/class/dmi/id/bios_version
A04
/sys/class/dmi/id/board_name
0X3GJK
/sys/class/dmi/id/board_vendor
Dell Inc.
/sys/class/dmi/id/board_version
A04
/sys/class/dmi/id/chassis_type
8
/sys/class/dmi/id/chassis_vendor
Dell Inc.
/sys/class/dmi/id/chassis_version
Not Specified
/sys/class/dmi/id/product_name
Vostro V131
/sys/class/dmi/id/product_version
Not Specified
/sys/class/dmi/id/sys_vendor
Dell Inc.
Thanks for both the testing and the dmi info, attached is a patch
which applies on top of the other 4 which should automatically do the
right thing, can you please test this patch with an "empty" kernel
cmdline, once that bit works I'll post the entire set upstream for
merging.
Regards,
Hans
>From 3225a9c43a02682e3cae5c02ca6d38e923256de1 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Fri, 18 Dec 2015 11:36:16 +0100
Subject: [PATCH 5/5] acpi-video: Add quirks for the Dell Vostro V131
The Dell Vostro V131 has an especially broken acpi-video implementation.
The backlight control bits work, but when the brightness is changed via
the acpi-video interface the backlight flickers annoyingly before settling
at the new brightness, switching to using the native interface fixes the
flickering so add a quirk for this (the vendor interface has the same
problem).
Brightness keypresses reported through the acpi-video-bus are also broken,
they get reported one event delayed, so if you press the brightness-up
hotkey on the keyboard nothing happens, then if you press brightness-down,
the previous brightness-up event gets reported. Since the keypresses are
also reported via wmi (if active) and via atkbd (when wmi is not active)
add a quirk to simply filter out the delayed (broken) events.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/acpi/acpi_video.c | 25 +++++++++++++++++++++++++
drivers/acpi/video_detect.c | 8 ++++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 2971154..80b13d4 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -419,6 +419,13 @@ static int video_enable_only_lcd(const struct dmi_system_id *d)
return 0;
}
+static int video_set_report_key_events(const struct dmi_system_id *id)
+{
+ if (report_key_events == -1)
+ report_key_events = (uintptr_t)id->driver_data;
+ return 0;
+}
+
static struct dmi_system_id video_dmi_table[] = {
/*
* Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -507,6 +514,24 @@ static struct dmi_system_id video_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "ESPRIMO Mobile M9410"),
},
},
+ /*
+ * Some machines report wrong key events on the acpi-bus, suppress
+ * key event reporting on these. Note this is only intended to work
+ * around events which are plain wrong. In some cases we get double
+ * events, in this case acpi-video is considered the canonical source
+ * and the events from the other source should be filtered. E.g.
+ * by calling acpi_video_handles_brightness_key_presses() from the
+ * vendor acpi/wmi driver or by using /lib/udev/hwdb.d/60-keyboard.hwdb
+ */
+ {
+ .callback = video_set_report_key_events,
+ .driver_data = (void *)((uintptr_t)REPORT_OUTPUT_KEY_EVENTS),
+ .ident = "Dell Vostro V131",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V131"),
+ },
+ },
{}
};
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index daaf1c4..8fe2682 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -279,6 +279,14 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro12,1"),
},
},
+ {
+ .callback = video_detect_force_native,
+ .ident = "Dell Vostro V131",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V131"),
+ },
+ },
{ },
};
--
2.5.0
