| Message ID | 1390053249-18919-1-git-send-email-i.gnatenko.brain@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Igor, On 01/18/2014 09:54 PM, Igor Gnatenko wrote: > From: Aaron Lu <aaron.lu@intel.com> > > Some system's ACPI video backlight control interface is broken and the > native backlight control interface should be used by default. This patch > sets the use_native_backlight parameter to true for those systems so > that video backlight control interface will not be created. To be > specific, the ThinkPad T430s/X230/X1 Carbon, Lenovo Yoga 13, Dell > Inspiron 7520, Acer Aspire 5733Z and HP ProBook 4340s are added here, > if they appear in some other DMI table before, they are removed from there. > > Note that the user specified kernel cmdline option will always have the > highest priority, i.e. if use_native_backlight=0 is specified and the > system is in the DMI table, the video module will not skip registering > backlight interface for it. > > Thinkpad T430s: > Reported-by: Theodore Tso <tytso@mit.edu> > Reported-and-tested-by: Peter Weber <bugs@ttyhoney.com> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Thinkpad X230: > Reported-and-tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > ThinkPad X1 Carbon: > Reported-and-tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> > Lenovo Yoga 13: > Reported-by: Lennart Poettering <lennart@poettering.net> > Reported-and-tested-by: Kevin Smith <thirdwiggin@gmail.com> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 > Dell Inspiron 7520: > Reported-by: Rinat Ibragimov <ibragimovrinat@mail.ru> > Acer Aspire 5733Z: > Reported-by: <sov.info@mail.ru> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941 > HP ProBook 4340s: > Reported-and-tested-by: Vladimir Sherenkov <a_12300@mail.ru> > Reference: http://redmine.russianfedora.pro/issues/1258 > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > Signed-off-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> Thanks for updating this patch. In the meantime, I plan to make some small modifications to this patch in next revision: 1 remove the win8 OSI check, I've seen win7 laptops that also needs to have only the GPU interface left and checking win8 doesn't make much sense now; 2 add more entries for laptops that appeared in bugzilla recently.
On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: > 1 remove the win8 OSI check, I've seen win7 laptops that also needs to > have only the GPU interface left and checking win8 doesn't make much > sense now; Are we sure that those aren't simply some other bug?
On 01/20/2014 09:34 PM, Matthew Garrett wrote: > On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: > >> 1 remove the win8 OSI check, I've seen win7 laptops that also needs to >> have only the GPU interface left and checking win8 doesn't make much >> sense now; > > Are we sure that those aren't simply some other bug? Well, the firmware on that laptop makes use of EC to do backlight control and the fact that the firmware interface doesn't work while the GPU's work seems to indicate that the backlight control circuit is not routed to EC. I think this is the same case as Win8 laptops.
On Tue, 2014-01-21 at 10:24 +0800, Aaron Lu wrote: > On 01/20/2014 09:34 PM, Matthew Garrett wrote: > > On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: > > > >> 1 remove the win8 OSI check, I've seen win7 laptops that also needs to > >> have only the GPU interface left and checking win8 doesn't make much > >> sense now; > > > > Are we sure that those aren't simply some other bug? > > Well, the firmware on that laptop makes use of EC to do backlight > control and the fact that the firmware interface doesn't work while the > GPU's work seems to indicate that the backlight control circuit is not > routed to EC. I think this is the same case as Win8 laptops. We know that Windows 8 graphics drivers don't use the ACPI interface, and that systems change their behaviour as a result, in some cases with absolutely no way for the ACPI interface could possibly work. I haven't seen any cases where that's obviously true for any non-Windows 8 systems. EC interfaces that don't work are often due to Linux leaving the hardware in a state other than the one expected by the firmware. We shouldn't assume that it's the same issue until we've investigated further.
On 01/21/2014 11:17 AM, Matthew Garrett wrote: > On Tue, 2014-01-21 at 10:24 +0800, Aaron Lu wrote: >> On 01/20/2014 09:34 PM, Matthew Garrett wrote: >>> On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: >>> >>>> 1 remove the win8 OSI check, I've seen win7 laptops that also needs to >>>> have only the GPU interface left and checking win8 doesn't make much >>>> sense now; >>> >>> Are we sure that those aren't simply some other bug? >> >> Well, the firmware on that laptop makes use of EC to do backlight >> control and the fact that the firmware interface doesn't work while the >> GPU's work seems to indicate that the backlight control circuit is not >> routed to EC. I think this is the same case as Win8 laptops. > > We know that Windows 8 graphics drivers don't use the ACPI interface, > and that systems change their behaviour as a result, in some cases with > absolutely no way for the ACPI interface could possibly work. I haven't > seen any cases where that's obviously true for any non-Windows 8 Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor GPU's interface, I just meant for one specific win7 laptop I could re-use the existing code to make the GPU's interface as the only one left. And to achieve this, the Win8 OSI check in acpi_video_verify_backlight_support has to be gone. BTW, I actually think use_native_backlight param should mean "the native backlight control interface will be the only one available on the system", it doesn't need to go side by side with Win8 OSI check. > systems. EC interfaces that don't work are often due to Linux leaving > the hardware in a state other than the one expected by the firmware. We Good to know this, thanks. > shouldn't assume that it's the same issue until we've investigated > further. OK, but I honestly don't have any idea how to proceed, in case you have some time, the bug is: https://bugzilla.kernel.org/show_bug.cgi?id=66501
On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: > On 01/21/2014 11:17 AM, Matthew Garrett wrote: > > We know that Windows 8 graphics drivers don't use the ACPI interface, > > and that systems change their behaviour as a result, in some cases with > > absolutely no way for the ACPI interface could possibly work. I haven't > > seen any cases where that's obviously true for any non-Windows 8 > > Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor > GPU's interface, I just meant for one specific win7 laptop I could re-use > the existing code to make the GPU's interface as the only one left. And to > achieve this, the Win8 OSI check in acpi_video_verify_backlight_support > has to be gone. We could do that, but why do we think that's the correct fix? The plan is to remove the native list entirely and do this for all Windows 8 systems, so the Win8 OSI check is the right thing to do.
On 01/21/2014 08:11 PM, Matthew Garrett wrote: > On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: >> On 01/21/2014 11:17 AM, Matthew Garrett wrote: >>> We know that Windows 8 graphics drivers don't use the ACPI interface, >>> and that systems change their behaviour as a result, in some cases with >>> absolutely no way for the ACPI interface could possibly work. I haven't >>> seen any cases where that's obviously true for any non-Windows 8 >> >> Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor >> GPU's interface, I just meant for one specific win7 laptop I could re-use >> the existing code to make the GPU's interface as the only one left. And to >> achieve this, the Win8 OSI check in acpi_video_verify_backlight_support >> has to be gone. > > We could do that, but why do we think that's the correct fix? The plan > is to remove the native list entirely and do this for all Windows 8 What do you mean by native list, systems that are Win8 based but don't work with the GPU's backlight interface? If so, it doesn't seem we have such a native list now and if we don't make use_native_backlight=1 by default, we couldn't generate such a list to start with... BTW, it doesn't seem everyone agrees with this plan. May I ask, Rafael and Daniel, what's your opinion please? We need to agree with something to move things forward.
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7..2b6a76b 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8, - .ident = "Dell Inspiron 15R SE", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), - DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"), - }, - }, - { - .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 995e91b..2d2fb84 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -82,11 +82,12 @@ static bool allow_duplicates; module_param(allow_duplicates, bool, 0644); /* - * For Windows 8 systems: if set ture and the GPU driver has - * registered a backlight interface, skip registering ACPI video's. + * For Windows 8 systems: used to decide if video module + * should skip registering backlight interface of its own. */ -static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false; static int register_count; static struct mutex video_list_lock; @@ -232,9 +233,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event); +static bool acpi_video_use_native_backlight(void) +{ + if (use_native_backlight_param != -1) + return use_native_backlight_param; + else + return use_native_backlight_dmi; +} + static bool acpi_video_verify_backlight_support(void) { - if (acpi_osi_is_win8() && use_native_backlight && + if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support(); @@ -399,6 +408,12 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d) return 0; } +static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight_dmi = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -443,6 +458,62 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X1 Carbon", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Acer Aspire 5733Z", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "HP ProBook 4340s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"), + DMI_MATCH(DMI_PRODUCT_VERSION, "HP ProBook 4340s"), + }, + }, {} }; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd..b639934 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, }, - { - .callback = video_detect_force_vendor, - .ident = "Lenovo Yoga 13", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), - }, - }, { }, };