[PATCH libdrm] meson: do not use cairo/valgrind if disabled
-Dcairo-tests=false currently results into enabling cairo support if it was found. Same for valgrind. v2: * Use underscore-prefixed variables to not change type of variable * Use empty array for "fake" dependency instead of real empty object v3: * Fix typo Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> Signed-off-by: Igor Gnatenko <ignate...@redhat.com> --- meson.build | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index 166559e8..7f786a8c 100644 --- a/meson.build +++ b/meson.build @@ -32,8 +32,6 @@ pkg = import('pkgconfig') with_udev = get_option('udev') with_freedreno_kgsl = get_option('freedreno-kgsl') with_install_tests = get_option('install-test-programs') -with_cairo_tests = get_option('cairo-tests') -with_valgrind = get_option('valgrind') config = configuration_data() @@ -226,8 +224,22 @@ endforeach dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required : with_intel) dep_cunit = dependency('cunit', version : '>= 2.1', required : false) -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') +_cairo_tests = get_option('cairo-tests') +if _cairo_tests != 'false' + dep_cairo = dependency('cairo', required : _cairo_tests == 'true') + with_cairo_tests = dep_cairo.found() +else + dep_cairo = [] + with_cairo_tests = false +endif +_valgrind = get_option('valgrind') +if _valgrind != 'false' + dep_valgrind = dependency('valgrind', required : _valgrind == 'true') + with_valgrind = dep_valgrind.found() +else + dep_valgrind = [] + with_valgrind = false +endif with_man_pages = get_option('man-pages') prog_xslt = find_program('xsltproc', required : with_man_pages == 'true') @@ -259,8 +271,8 @@ foreach t : [ [with_radeon, 'RADEON'], [with_vc4, 'VC4'], [with_vmwgfx, 'VMWGFX'], - [dep_cairo.found(), 'CAIRO'], - [dep_valgrind.found(), 'VALGRIND'], + [with_cairo_tests, 'CAIRO'], + [with_valgrind, 'VALGRIND'], ] config.set10('HAVE_@0@'.format(t[1]), t[0]) endforeach -- 2.16.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson: do not use cairo/valgrind if disabled
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On Mon, 2018-02-19 at 12:15 +, Eric Engestrom wrote: > On Sunday, 2018-02-18 14:00:50 +0100, Igor Gnatenko wrote: > > -Dcairo-tests=false currently results into enabling cairo support if it > > was found. Same for valgrind. > > Indeed, this was wrong; thanks for the fix! > Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> > > Do you have commit access, or do you want me to push this for you? I don't have commit access ☹ v2 sent with all your comments. > > > > Signed-off-by: Igor Gnatenko <ignate...@redhat.com> > > --- > > meson.build | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index 166559e8..695f89b3 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -226,8 +226,20 @@ endforeach > > > > dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required : > > with_intel) > > dep_cunit = dependency('cunit', version : '>= 2.1', required : false) > > -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') > > -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') > > +if with_cairo_tests != 'false' > > + dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') > > + with_cairo_tests = dep_cairo.found() > > +else > > + dep_cairo = declare_dependency() > > Nit: `dep_cairo = []` is enough; I'll change that if I'm the one to push it. > > > + with_cairo_tests = false > > We try to avoid changing the type of a var; could you send a follow-up > patch to rename the get_option() var to `_cairo_tests`? > > (same obviously applies for the valgrind bits) > > > +endif > > +if with_valgrind != 'false' > > + dep_valgrind = dependency('valgrind', required : with_valgrind == > > 'true') > > + with_valgrind = dep_valgrind.found() > > +else > > + dep_valgrind = declare_dependency() > > + with_valgrind = false > > +endif > > > > with_man_pages = get_option('man-pages') > > prog_xslt = find_program('xsltproc', required : with_man_pages == 'true') > > @@ -259,8 +271,8 @@ foreach t : [ > > [with_radeon, 'RADEON'], > > [with_vc4, 'VC4'], > > [with_vmwgfx, 'VMWGFX'], > > - [dep_cairo.found(), 'CAIRO'], > > - [dep_valgrind.found(), 'VALGRIND'], > > + [with_cairo_tests, 'CAIRO'], > > + [with_valgrind, 'VALGRIND'], > > ] > >config.set10('HAVE_@0@'.format(t[1]), t[0]) > > endforeach > > -- > > 2.16.2 > > - -- - -Igor Gnatenko -BEGIN PGP SIGNATURE- iQIzBAEBCAAdFiEEhLFO09aHZVqO+CM6aVcUvRu8X0wFAlqKyFAACgkQaVcUvRu8 X0zP3xAAsa1mi9r8Oi+pdJrXy02Bm2mGSzp8IcrDGF3i0v9d9uP7o484fmYxxz0t RDu/4YpertGmR+aYiZiEeREnZ7n+GtaBb35O1leU/zz/TniEi+Qne6kQbUcUNE3z 3URS2VKmHPAivR5ctL5/DvwOaAP7sIojlDjsu3ydZafoPVw+FF/cnJaK6yItVH7T 7ZE48i434qBUJT6IsxUCy2jb7gQSCzs/G/gNxYjVxLO/h/rb52eIiPQ5XFd6Aqmt R5ptwgb4wpQERTEkvSmFTe+tsIwmSyD8o6jJRAK4U0mGV8g5+AB/7fmZmFA7nxQ4 J9Ycqd2JZ96XruBE9qSvu9gK50oVKcQyJaq2heTKkRSwq+HP82qfhtaHsC5hsrTG lg2+bzpyGStxXzt1bndYQ2u9hcPcbvDxb9mDP5wimacdmD/qmAg2LAv5OiZESgvd Zm8TWygb/bjJoLZOTdbGqdjFGmOCq3g9ZTqbjWfhv1mmc2ZENoo/fcyyCjUg4MD3 P4IP3ogAUk9H1MORhN6I5rw2ERDGaXy60z4dJwhAfHWmwsoKRbdpRmu9Y2vFQ2n9 /kysL8yG+DPsUHB6f5ZZh/r7dsJDWeIea8ZI2gEMrGOIFxSZUcypwmjxxgC5Yo2x EwRsGvw/GAKhQQX8ukgMMy878Qj7yRvf6PoMGlVqtjlmFRVXlgM= =q2az -END PGP SIGNATURE- ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] meson: do not use cairo/valgrind if disabled
-Dcairo-tests=false currently results into enabling cairo support if it was found. Same for valgrind. v2: * Use underscore-prefixed variables to not change type of variable * Use empty array for "fake" dependency instead of real empty object Signed-off-by: Igor Gnatenko <ignate...@redhat.com> --- meson.build | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index 166559e8..e4e21bae 100644 --- a/meson.build +++ b/meson.build @@ -32,8 +32,6 @@ pkg = import('pkgconfig') with_udev = get_option('udev') with_freedreno_kgsl = get_option('freedreno-kgsl') with_install_tests = get_option('install-test-programs') -with_cairo_tests = get_option('cairo-tests') -with_valgrind = get_option('valgrind') config = configuration_data() @@ -226,8 +224,22 @@ endforeach dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required : with_intel) dep_cunit = dependency('cunit', version : '>= 2.1', required : false) -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') +_cairo_tests = get_option('cairo-tests') +if _cairo_tests != 'false' + dep_cairo = dependency('cairo', required : _cairo_tests == 'true') + with_cairo_tests = dep_cairo.found() +else + dep_cairo = [] + with_cairo_tests = false +endif +_valgrind = get_option('valgrind') +if _valgrind != 'false' + dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') + with_valgrind = dep_valgrind.found() +else + dep_valgrind = [] + with_valgrind = false +endif with_man_pages = get_option('man-pages') prog_xslt = find_program('xsltproc', required : with_man_pages == 'true') @@ -259,8 +271,8 @@ foreach t : [ [with_radeon, 'RADEON'], [with_vc4, 'VC4'], [with_vmwgfx, 'VMWGFX'], - [dep_cairo.found(), 'CAIRO'], - [dep_valgrind.found(), 'VALGRIND'], + [with_cairo_tests, 'CAIRO'], + [with_valgrind, 'VALGRIND'], ] config.set10('HAVE_@0@'.format(t[1]), t[0]) endforeach -- 2.16.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] meson: do not use cairo/valgrind if disabled
-Dcairo-tests=false currently results into enabling cairo support if it was found. Same for valgrind. Signed-off-by: Igor Gnatenko <ignate...@redhat.com> --- meson.build | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index 166559e8..695f89b3 100644 --- a/meson.build +++ b/meson.build @@ -226,8 +226,20 @@ endforeach dep_pciaccess = dependency('pciaccess', version : '>= 0.10', required : with_intel) dep_cunit = dependency('cunit', version : '>= 2.1', required : false) -dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') -dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') +if with_cairo_tests != 'false' + dep_cairo = dependency('cairo', required : with_cairo_tests == 'true') + with_cairo_tests = dep_cairo.found() +else + dep_cairo = declare_dependency() + with_cairo_tests = false +endif +if with_valgrind != 'false' + dep_valgrind = dependency('valgrind', required : with_valgrind == 'true') + with_valgrind = dep_valgrind.found() +else + dep_valgrind = declare_dependency() + with_valgrind = false +endif with_man_pages = get_option('man-pages') prog_xslt = find_program('xsltproc', required : with_man_pages == 'true') @@ -259,8 +271,8 @@ foreach t : [ [with_radeon, 'RADEON'], [with_vc4, 'VC4'], [with_vmwgfx, 'VMWGFX'], - [dep_cairo.found(), 'CAIRO'], - [dep_valgrind.found(), 'VALGRIND'], + [with_cairo_tests, 'CAIRO'], + [with_valgrind, 'VALGRIND'], ] config.set10('HAVE_@0@'.format(t[1]), t[0]) endforeach -- 2.16.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [PATCH v4 1/3] Add meson build system
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On Thu, 2018-01-04 at 10:28 -0800, Dylan Baker wrote: > This patch adds a complete meson build system, including tests and > install. It has the necessary hooks to allow it be used as a subproject > for other meson based builds such as mesa. Builds fine on all architectures supported by Fedora, boots fine on my laptop (x86_64). All nitpicks are inline. > Signed-off-by: Dylan Baker <dylan.c.ba...@intel.com> Reviewed-and-tested-by: Igor Gnatenko <i.gnatenko.br...@gmail.com> > diff --git a/meson_options.txt b/meson_options.txt > new file mode 100644 > index 000..08c2ccd > --- /dev/null > +++ b/meson_options.txt > @@ -0,0 +1,143 @@ > [...] > +option( > + 'intel', > + type : 'combo', > + value : 'auto', > + choices : ['true', 'false', 'auto'], > + description : '''Enable support for Intel's KMS API.''', Any reason to use `'''` here and there? > [...] > +option( > + 'man-pages', > + type : 'combo', > + value : 'auto', > + choices : ['true', 'false', 'auto'], > + description : 'Enable manpage generation and install.', "installation". > [...] > +option( > + 'valgrind', > + type : 'combo', > + value : 'auto', > + choices : ['true', 'false', 'auto'], > + description : 'build libdrm with valgrind support', "Build". And fullstop at the end of sentence. - -- - -Igor Gnatenko -BEGIN PGP SIGNATURE- iQIzBAEBCAAdFiEEhLFO09aHZVqO+CM6aVcUvRu8X0wFAlpOoBcACgkQaVcUvRu8 X0yF9w//cgaMVkU4xTKegRJY4uuzTE3MQvMmaCoA8ivBaCWPuoX3ozlwsAgZZXaZ Vo83tZ0u80cjgoSG4I/JcNp3UhsxtGgqcrqqcof/SGn+YS43eFKPL57dowwQ5qk3 ccAUgHtAdQXuCJFaQFsTISSEj1X07RA04mIMe7QZFh7AHsKmv+ctaTUO7uJsXJzi aX7Z9rntTCnXvzZy7Y56XPCleXfi+yDzQPdDopZAEdLYT8hYUvebo6JGQUpg8iNd YuvZsbkrpyV1uMY/2feSJ3Ns4ZTAj3I4F41Xbb7CqZt/BX60EnkZJXog4RSbdlri cxMX7gPkrOXxNJbllmdN0nPdBP/atViRY7dDkE4Lv4YrmwL8oT4Mjfyb/TeINT2X 6NltSgc8+zSvQSkjWyKHzQ3ZQCQHIAheG+V2Cvnc1NIfX06AV9USRsSRzBMza+gW cWNT2g/M0jjmLTVEoLR8MSLXAB9gfsBdRDEnvqEsZCqDh1idW1Ttuk3m/h3+BT8i GMyCrswVgKLI7gBbdVFdLDarEIVtTJlYvPkGXxRyOzv1r5dM/MMmeay7P3WD+liE CLF9nRVrekQA7Mh4Y61RSyFAntzBokNKL+FrSzSuseNtgYAM3Es0JgY1ndsczvVX zUqULC0AEAEwmAIQmDlYFG+ut8nIvmk6aWHHlvLwUHgiDD+MEc4= =dAKV -END PGP SIGNATURE- ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [PATCH 1/3] Add meson build system
alse', choices : ['true', > 'false', 'auto']) > +option('vc4', type : 'combo', value : 'auto', choices : ['true', > 'false', 'auto']) > +option('etnaviv', type : 'combo', value : 'false', choices : ['true', > 'false', 'auto']) > +option('cairo_tests', type : 'combo', value : 'auto', choices : ['true', > 'false', 'auto']) > +option('man_pages', type : 'combo', value : 'auto', choices : ['true', > 'false', 'auto']) > +option('valgrind',type : 'combo', value : 'auto', choices : ['true', > 'false', 'auto']) > +option('freedreno-kgls',type : 'boolean', value : false) Probably using `_` would be better for consistency.. Also, option should be named kgsl, not kgls (note sl vs ls). > +option('install_test_programs', type : 'boolean', value : false) > +option('udev', type : 'boolean', value : false) Having description on all those options would be extremely useful [...] Still testing it, once will have more results -- will come back. - -- - -Igor Gnatenko -BEGIN PGP SIGNATURE- iQIzBAEBCAAdFiEEhLFO09aHZVqO+CM6aVcUvRu8X0wFAlpNZbwACgkQaVcUvRu8 X0xuQRAAulgnkHdNb2LCq6R+4A+IwEbyVD3uNIBv0eF0X/k/8w7IT7PcNwdS8VN3 XBrSLqFTxHTsghtMXSDXDW6LqA3Zv8VCkWZb3G/bek4w6iwwHmEVLGawiO1jVlHY uX7bQEf/bdrM/UnXY1PnBmzmfhIcu6LhAry+pPS0iYxJWgcv6XbFil5fYu+N9T1H vdlj8WJtZA9u4VhHdqFaf8JN8OwuBC2+87mNvuZMwKF1d4BD9r77WRuhDnyRWzJs NoxufXz9JPx2YRbg6V8V5V4fsCV5oFpSpkpnTqIasjWlyJXo2Dhatt6oCbtu+ip4 Vf5Nr7WzIbXhT8WAYnLPc8E2mtWK5HG8NhrLJnqY4kdyiJ+w7X0PXKYeBPXRN1NG zrF+h7Kd4LJvPwh0KMl1idGiGa5Mmr/vF/apIKrMBMvJ++E9zs+sPCNbSjnI3aWe 47LIqTkVxCLjwHIpQqDWZ+bUNanpwFaWtVWm9xvlPcpZ35pP2PqeyZ6abpqykP8b OSQK9fN0PpiZsfVT25K8SaGvFzUy37lFSs/3cvTFQ+rmKo7xYxj1qbd4xvml0CU8 4IcPcFKRZRMFIE1cecqg7lNxlgfcA7bdrOSbvjHpKNlEUpRZOtNDPjgkx8RWlA3J x1u8mcARzI5m837isa0eOatVgYXy+Eg86zWxiIy1jGliWaJTvN0= =Rikj -END PGP SIGNATURE- ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] ACPI / video: Add systems that should favor native backlight interface
From: Aaron Lu <aaron...@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 Reported-and-tested-by: Peter Weber Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 ThinkPad X1 Carbon: Reported-and-tested-by: Igor Gnatenko Lenovo Yoga 13: Reported-by: Lennart Poettering Reported-and-tested-by: Kevin Smith Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov Acer Aspire 5733Z: Reported-by: Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941 HP ProBook 4340s: Reported-and-tested-by: Vladimir Sherenkov Reference: http://redmine.russianfedora.pro/issues/1258 Signed-off-by: Aaron Lu Signed-off-by: Igor Gnatenko --- drivers/acpi/blacklist.c| 8 - drivers/acpi/video.c| 81 ++--- drivers/acpi/video_detect.c | 8 - 3 files changed, 76 insertions(+), 21 deletions(-) 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, "Thi
[PATCH v2 rebased] ACPI / video: Add systems that should favor native backlight interface
Hi, add some updates to the patch ;) On Thu, 2013-11-21 at 13:29 +0800, Aaron Lu wrote: > On 11/21/2013 04:56 AM, Igor Gnatenko wrote: > > Any news here? If no - I think we need re-send patch as new.. > > Since the v2 patch can't apply cleanly on top of pm's -next tree, I > think it's worth a re-send, so here it comes. > > --- > Subject: [PATCH] ACPI / video: Add systems that should favor native backlight > interface > From: Aaron Lu > Date: Thu, 21 Nov 2013 11:24:48 +0800 > > 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, Lenovo Yoga 13, Dell Inspiron 7520 > and Acer Aspire 5733Z 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 > Reported-and-tested-by: Peter Weber > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Thinkpad X230: > Reported-and-tested-by: Igor Gnatenko > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 ThinkPad X1 Carbon: Reported-and-tested-by: Igor Gnatenko > Lenovo Yoga 13: > Reported-by: Lennart Poettering > Reported-and-tested-by: Kevin Smith > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 > Dell Inspiron 7520: > Reported-by: Rinat Ibragimov > Acer Aspire 5733Z: > Reported-by: > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941 HP ProBook 4340s: Reported-and-tested-by: Vladimir Sherenkov Reference: http://redmine.russianfedora.pro/issues/1258 > > Signed-off-by: Aaron Lu > --- > drivers/acpi/blacklist.c| 8 -- > drivers/acpi/video.c| 65 > + > drivers/acpi/video_detect.c | 8 -- > 3 files changed, 60 insertions(+), 21 deletions(-) > > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c > index 078c4f7fe2dd..2b6a76b6d59a 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 995e91bcb97b..7dc6071a04b6 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; >
[PATCH v2 rebased] ACPI / video: Add systems that should favor native backlight interface
Hi, please add some updates ;) On Thu, 2013-11-21 at 13:29 +0800, Aaron Lu wrote: > On 11/21/2013 04:56 AM, Igor Gnatenko wrote: > > Any news here? If no - I think we need re-send patch as new.. > > Since the v2 patch can't apply cleanly on top of pm's -next tree, I > think it's worth a re-send, so here it comes. > > --- > Subject: [PATCH] ACPI / video: Add systems that should favor native backlight > interface > From: Aaron Lu > Date: Thu, 21 Nov 2013 11:24:48 +0800 > > 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, Lenovo Yoga 13, Dell Inspiron 7520 > and Acer Aspire 5733Z 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 > Reported-and-tested-by: Peter Weber > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Thinkpad X230: > Reported-and-tested-by: Igor Gnatenko > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 ThinkPad X1 Carbon: Reported-and-tested-by: Igor Gnatenko Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Lenovo Yoga 13: > Reported-by: Lennart Poettering > Reported-and-tested-by: Kevin Smith > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 > Dell Inspiron 7520: > Reported-by: Rinat Ibragimov > Acer Aspire 5733Z: > Reported-by: > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941 HP ProBook 4340s: Reported-and-tested-by: Vladimir Sherenkov Reference: http://redmine.russianfedora.pro/issues/1258 > > Signed-off-by: Aaron Lu > --- > drivers/acpi/blacklist.c| 8 -- > drivers/acpi/video.c| 65 > + > drivers/acpi/video_detect.c | 8 -- > 3 files changed, 60 insertions(+), 21 deletions(-) > > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c > index 078c4f7fe2dd..2b6a76b6d59a 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 995e91bcb97b..7dc6071a04b6 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_r
[PATCH v2] ACPI / video: Add systems that should favor native backlight interface
On Fri, 2013-11-15 at 14:07 +0800, Aaron Lu wrote: > 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, Lenovo Yoga 13, Dell Inspiron 7520 > and Acer Aspire 5733Z 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 > Reported-and-tested-by: Peter Weber > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Thinkpad X230: > Reported-and-tested-by: Igor Gnatenko > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > Lenovo Yoga 13: > Reported-by: Lennart Poettering > Reported-and-tested-by: Kevin Smith > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 > Dell Inspiron 7520: > Reported-by: Rinat Ibragimov > Acer Aspire 5733Z: > Reported-by: > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941 > > Signed-off-by: Aaron Lu > --- > v2: > Add Acer Aspire 5733Z, see bug #62941; > Remove Inspiron 7520 from acpi_osi_dmi_table and Yoga 13 from > video_detect_dmi_table. > > Based on top of Rafael's linux-next branch + the following patch: > [PATCH] ACPI / video: clean up DMI table for initial black screen problem > http://marc.info/?l=linux-acpi=138448583006432=2 > > drivers/acpi/blacklist.c| 8 -- > drivers/acpi/video.c| 65 > + > drivers/acpi/video_detect.c | 8 -- > 3 files changed, 60 insertions(+), 21 deletions(-) > > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c > index 078c4f7fe2dd..2b6a76b6d59a 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 4ccb89e5c4ad..49abe0348b03 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1; > module_param(use_bios_initial_backlight, 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; > @@ -239,9 +240,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(); > @@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(const struct > dmi_system_id *d) > return 0; > } > > +static int __init video_set_use_n
[PATCH v5 0/4] Fix Win8 backlight issue
On Thu, 2013-10-24 at 16:13 +0800, Aaron Lu wrote: > On 10/16/2013 07:33 AM, Rafael J. Wysocki wrote: > > On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote: > >> v5: > >> 1 Introduce video.use_native_backlight module parameter and set its > >> value to false by default as suggested by Rafael. For Win8 systems > >> which have broken ACPI video backlight control, the parameter can be > >> set to 1 in kernel cmdline to skip registering ACPI video's backlight > >> interface. Due to this change, the acpi_video_verify_backlight_support > >> is moved from video_detect.c to video.c - patch 3/4; > >> 2 Rename bd_list_head and bd_list_mutex in backlight.c to > >> backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael > >> - patch 1/4. > >> > >> v4: > >> Remove decleration and stub for acpi_video_unregister_backlight in > >> video.h of patch 2/4 since that function doesn't exist anymore in v3. > >> > >> v3: > >> 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; > >> 2 Remove unnecessary function acpi_video_unregister_backlight introduced > >> in patch 2/4 as pointed out by Jani Nikula. > >> > >> v2: > >> v1 has the subject of "Rework ACPI video driver" and is posted here: > >> http://lkml.org/lkml/2013/9/9/74 > >> Since the objective is really to fix Win8 backlight issues, I changed > >> the subject in this version, sorry about that. > >> > >> This patchset has four patches, the first introduced a new API named > >> backlight_device_registered in backlight layer that can be used for > >> backlight interface provider module to check if a specific type backlight > >> interface has been registered, see changelog for patch 1/4 for details. > >> Then patch 2/4 does the cleanup to sepeate the backlight control and > >> event delivery functionality in the ACPI video module and patch 3/4 > >> solves some Win8 backlight control problems by avoiding register ACPI > >> video's backlight interface if: > >> 1 Kernel cmdline option acpi_backlight=video is not given; > >> 2 This is a Win8 system; > >> 3 Native backlight control interface exists. > >> Patch 4/4 fixes some problems in thinkpad-acpi module. > >> > >> Technically, patch 2/4 is not required to fix the issue here. So if you > >> think it is not necessary, I can remove it from the series. > >> > >> Aaron Lu (4): > >> backlight: introduce backlight_device_registered > >> ACPI / video: seperate backlight control and event interface > >> ACPI / video: Do not register backlight if win8 and native interface > >> exists > >> thinkpad-acpi: fix handle locate for video and query of _BCL > >> > >> drivers/acpi/internal.h | 4 +- > >> drivers/acpi/video.c | 457 > >> --- > >> drivers/acpi/video_detect.c | 4 +- > >> drivers/platform/x86/thinkpad_acpi.c | 31 ++- > >> drivers/video/backlight/backlight.c | 31 +++ > >> include/linux/backlight.h| 4 + > >> 6 files changed, 326 insertions(+), 205 deletions(-) > > > > I've added this series to my queue for 3.13. > > > > Since the next step will be to introduce a list of systems that need > > video.use_native_backlight=1 *and* don't break in that configuration, I > > don't > > see much point adding another Kconfig option for the default. > > > > Hopefully, in the future we'll be able to fix the problems causing > > video.use_native_backlight=1 to fail of the systems where it fails and then > > we'll be able to make that the default behavior and drop the option > > altogether. > > I've prepared a patch(at the end of the mail) to set use_native_backlight > by default for some systems. There are 3 systems currently that I'm > kind of sure that should be added: > > The ThinkPad T430s and X230 is: > Reported-by: Theodore Tso > Reported-and-tested-by: Peter Weber > Reported-by: Igor Gnatenko > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 > > The Lenovo Yoga is: > Reported-by: Lennart Poettering > Reference: https://lkml.org/lkml/2013/10/13/178 > > From: Aaron Lu > Subject: [PATCH] ACPI / video: Add systems that should favor native backlight > interface > > Some system's ACPI video backlight control interface is broken and the > native backlight control interface should be used by default. T
Re: [PATCH v5 0/4] Fix Win8 backlight issue
On Fri, 2013-10-11 at 12:42 -0400, Josh Boyer wrote: On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote: v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true? If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles? josh I think more better to use this unregister as default and give option to disable it. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.4-301.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/4] Fix Win8 backlight issue
On Sat, 2013-10-12 at 00:45 +0200, Rafael J. Wysocki wrote: On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote: On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote: On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote: v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true? Well, we have a rule in the kernel not to introduce regressions for users even if they are minority. If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles? The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully). Of course, distro kernels may always change the default to true if they want. They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel. Well, adding a Kconfig option should be simple enough, but I'm not sure I understand the point. You'll still need to rebuild the kernel after changing that option. I think he meant that we should have Kconfig option alike CONFIG_I915_BACKLIGHT_UNREGISTER which have true or false. If it true - unregister acpi backlight as default. I think so. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.4-301.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/4] Fix Win8 backlight issue
On Tue, 2013-09-24 at 17:47 +0800, Aaron Lu wrote: v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister introduced in patch 2/3 as pointed out by Jani Nikula. v2: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL drivers/acpi/internal.h | 5 +- drivers/acpi/video.c | 442 --- drivers/acpi/video_detect.c | 14 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h | 2 + include/linux/backlight.h| 4 + 7 files changed, 324 insertions(+), 205 deletions(-) Excellent! I've tested this patch-set on ThinkPad X230 and backlight issue is exhausted. Thanks. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] Fix Win8 backlight issue
On Sun, 2013-09-22 at 17:10 +0800, Aaron Lu wrote: On 09/18/2013 08:36 PM, Igor Gnatenko wrote: On Wed, 2013-09-18 at 20:31 +0800, Aaron Lu wrote: On 09/18/2013 02:30 PM, Igor Gnatenko wrote: On Wed, 2013-09-18 at 09:03 +0800, Aaron Lu wrote: On 09/17/2013 09:34 PM, Igor Gnatenko wrote: Aaron, how about fix indicator on ThinkPads ? Can you please describe the problem in detail, is it that when you adjust brightness level through hotkey, there is no GUI indication? Thanks. -Aaron Yes. On my ThinkPad X230 I pressing backlight hotkeys. Actually brightnes changing, but have no indicator in GUI. Oh, that's still the problem of _BCL not getting executed once for Lenovo thinkpad laptops. I borrowed a Thinkpad X1 this afternoon and can reproduce this, I'll take a look at this issue. The thinkpad-acpi module already has a call to _BCL but somehow that doesn't happen. Since it's national holidays here, I'll check this issue when I got back to work on this Saturday. Thanks for the quick test :-) Thanks. No problem ;-) Here is a quick fix for thinkpad-acpi.c: https://github.com/aaronlu/linux acpi_video_win8 commit thinkpad-acpi: fix handle locate for video and query of _BCL. Note that it is a separate issue specifically for thinkpad so I'll submit that patch in another thread. Thanks, Aaron Excellent! I've tested 3 patches from this patchset + 1 latest patch from you branch and it is works fine. Regulating and indicating works OK. Thank you. I think you need to make new patch-set within 4 patches. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.12.0-0.rc1.git0.1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] Fix Win8 backlight issue
On Wed, 2013-09-18 at 20:31 +0800, Aaron Lu wrote: On 09/18/2013 02:30 PM, Igor Gnatenko wrote: On Wed, 2013-09-18 at 09:03 +0800, Aaron Lu wrote: On 09/17/2013 09:34 PM, Igor Gnatenko wrote: Aaron, how about fix indicator on ThinkPads ? Can you please describe the problem in detail, is it that when you adjust brightness level through hotkey, there is no GUI indication? Thanks. -Aaron Yes. On my ThinkPad X230 I pressing backlight hotkeys. Actually brightnes changing, but have no indicator in GUI. Oh, that's still the problem of _BCL not getting executed once for Lenovo thinkpad laptops. I borrowed a Thinkpad X1 this afternoon and can reproduce this, I'll take a look at this issue. The thinkpad-acpi module already has a call to _BCL but somehow that doesn't happen. Since it's national holidays here, I'll check this issue when I got back to work on this Saturday. Thanks for the quick test :-) Thanks. No problem ;-) -Aaron -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] Fix Win8 backlight issue
On Wed, 2013-09-18 at 09:03 +0800, Aaron Lu wrote: On 09/17/2013 09:34 PM, Igor Gnatenko wrote: On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Apply on top of v3.12-rc1. Aaron Lu (3): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists drivers/acpi/internal.h | 5 +- drivers/acpi/video.c| 442 drivers/acpi/video_detect.c | 14 +- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h| 2 + include/linux/backlight.h | 4 + 6 files changed, 300 insertions(+), 198 deletions(-) Aaron, how about fix indicator on ThinkPads ? Can you please describe the problem in detail, is it that when you adjust brightness level through hotkey, there is no GUI indication? Thanks. -Aaron Yes. On my ThinkPad X230 I pressing backlight hotkeys. Actually brightnes changing, but have no indicator in GUI. -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] ACPI / video: seperate backlight control and event interface
On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: The backlight control and event delivery functionality provided by ACPI video module is mixed together and registered all during video device enumeration time. As a result, the two functionality are also removed together on module unload time or by the acpi_video_unregister function. The two functionalities are actually independent and one may be useful while the other one may be broken, so it is desirable to seperate the two functionalities such that it is clear and easy to disable one functionality without affecting the other one. APIs to selectively remove backlight control interface and/or event delivery functionality can be easily added once needed. Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/video.c | 451 ++- include/acpi/video.h | 2 + 2 files changed, 264 insertions(+), 189 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.12.0-0.rc1.git0.1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] ACPI / video: Do not register backlight if win8 and native interface exists
On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows 8 doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. So for Win8 systems, if there is native backlight control interface registered by GPU driver, ACPI video will not register its own. For users who prefer to keep ACPI video's backlight interface, the existing kernel cmdline option acpi_backlight=video can be used. This patch is an evolution from previous work done by Matthew Garrett, Chun-Yi Lee, Seth Forshee and Rafael J. Wysocki. Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/internal.h | 5 ++--- drivers/acpi/video.c| 27 +-- drivers/acpi/video_detect.c | 14 -- 3 files changed, 19 insertions(+), 27 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.12.0-0.rc1.git0.1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] Fix Win8 backlight issue
On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: v1 has the subject of Rework ACPI video driver and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that. This patchset has three patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/3 for details. Then patch 2/3 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/3 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Technically, patch 2/3 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series. Apply on top of v3.12-rc1. Aaron Lu (3): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists drivers/acpi/internal.h | 5 +- drivers/acpi/video.c| 442 drivers/acpi/video_detect.c | 14 +- drivers/video/backlight/backlight.c | 31 +++ include/acpi/video.h| 2 + include/linux/backlight.h | 4 + 6 files changed, 300 insertions(+), 198 deletions(-) Aaron, how about fix indicator on ThinkPads ? -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.1-300.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] backlight: introduce backlight_device_registered
On Tue, 2013-09-17 at 17:23 +0800, Aaron Lu wrote: Introduce a new API for modules to query if a specific type of backlight device has been registered. This is useful for some backlight device provider module(e.g. ACPI video) to know if a native control interface(e.g. the interface created by i915) is available and then do things accordingly(e.g. avoid register its own on Win8 systems). Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h | 4 2 files changed, 35 insertions(+) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.12.0-0.rc1.git0.1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote: On 09/09/2013 07:44 PM, Igor Gnatenko wrote: On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f466980..75fba17 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (INTEL_INFO(dev)-num_pipes) { /* Must be done after probing outputs */ intel_opregion_init(dev); - acpi_video_register(); + __acpi_video_register(i915_take_over_backlight); } if (IS_GEN5(dev)) I can't compile: DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load': DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit declaration of function '__acpi_video_register' [-Werror=implicit-function-declaration] DEBUG:__acpi_video_register(i915_take_over_backlight); DEBUG:^ DEBUG: cc1: some warnings being treated as errors DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1 DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2 DEBUG: make[2]: *** [drivers/gpu/drm] Error 2 DEBUG: make[1]: *** [drivers/gpu] Error 2 DEBUG: make: *** [drivers] Error 2 The two patches are based on top of Rafael's linux-next tree. I just tried it again, no compile problem for me. I also tried on today Linus' master tree, as there are some updates from i915, two conflicts exist. I've just resolved them and will update it in next revision. If you want to try it now, please use: https://github.com/aaronlu/linux acpi_video_rework Thanks, Aaron Thanks. this patch fixes my problems w/ compilation. I've tested this two patches and after apply I have: $ tree /sys/class/backlight/ /sys/class/backlight/ |-- acpi_video0 - ../../devices/pci:00/:00:02.0/backlight/acpi_video0 `-- intel_backlight - ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight 2 directories, 0 files I think it's didn't unregistered.. I may forget. I need to apply one of patch from Matthew ? Some strings from logs: DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013 thinkpad_acpi: Standard ACPI backlight interface available, not loading native one -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 13:16 +0800, Aaron Lu wrote: On 09/10/2013 01:13 PM, Igor Gnatenko wrote: On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote: On 09/09/2013 07:44 PM, Igor Gnatenko wrote: On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f466980..75fba17 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (INTEL_INFO(dev)-num_pipes) { /* Must be done after probing outputs */ intel_opregion_init(dev); -acpi_video_register(); +__acpi_video_register(i915_take_over_backlight); } if (IS_GEN5(dev)) I can't compile: DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load': DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit declaration of function '__acpi_video_register' [-Werror=implicit-function-declaration] DEBUG:__acpi_video_register(i915_take_over_backlight); DEBUG:^ DEBUG: cc1: some warnings being treated as errors DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1 DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2 DEBUG: make[2]: *** [drivers/gpu/drm] Error 2 DEBUG: make[1]: *** [drivers/gpu] Error 2 DEBUG: make: *** [drivers] Error 2 The two patches are based on top of Rafael's linux-next tree. I just tried it again, no compile problem for me. I also tried on today Linus' master tree, as there are some updates from i915, two conflicts exist. I've just resolved them and will update it in next revision. If you want to try it now, please use: https://github.com/aaronlu/linux acpi_video_rework Thanks, Aaron Thanks. this patch fixes my problems w/ compilation. I've tested this two patches and after apply I have: $ tree /sys/class/backlight/ /sys/class/backlight/ |-- acpi_video0 - ../../devices/pci:00/:00:02.0/backlight/acpi_video0 `-- intel_backlight - ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight 2 directories, 0 files I think it's didn't unregistered.. I may forget. I need to apply one of patch from Matthew ? You need to specify i915.take_over_backlight=1 in kernel cmdline, that module option is set to false by default for now. Thanks for the test. -Aaron Some strings from logs: DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013 thinkpad_acpi: Standard ACPI backlight interface available, not loading native one Thanks for quick answer. Yes. This option do unregister. Thanks. but for this patch-set I also need [PATCH 2/3] ACPI / video: Always call acpi_video_init_brightness() on init from Matthew (for notifications in DE). -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] ACPI / video: seperate backlight control and event interface
On Mon, 2013-09-09 at 16:40 +0800, Aaron Lu wrote: The backlight control and event delivery functionality provided by ACPI video module is mixed together and registered all during video device enumeration time. As a result, the two functionality are also removed together on module unload time or by the acpi_video_unregister function. The two functionalities are actually independent and one may be useful while the other one may be broken, so it is desirable to seperate the two functionalities such that it is clear and easy to disable one functionality without affecting the other one. This patch does the seperation and as a result, a new video_bus_head list is introduced to store all registered video bus structure and a new function acpi_video_unregister_backlight is introduced to unregister backlight interfaces for all video devices belonging to stored video buses. Currently, there is no need to unregister ACPI video's event delivery functionality alone so the function acpi_video_remove_notify_handler is not introduced, it can be easily added when needed. Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/video.c | 451 ++- include/acpi/video.h | 2 + 2 files changed, 264 insertions(+), 189 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-3.fc20.x86_64 -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. There's a problem with that approach, however, because simply avoiding to register the ACPI backlight interface if the firmware calls _OSI for Windows 8 may not work in the following situations: (1) The ACPI backlight interface actually works on the given system and the i915 driver is not loaded (e.g. another graphics driver is used). (2) The ACPI backlight interface doesn't work on the given system, but there is a vendor platform driver that will register its own, equally broken, backlight interface if not prevented from doing so by the ACPI subsystem. Therefore we need to allow the ACPI backlight interface to be registered until the i915 driver is loaded which then will unregister it if the firmware has called _OSI for Windows 8 (or will register the ACPI video driver without backlight support if not already present). For this reason, introduce an alternative function for registering ACPI video, __acpi_video_register(bool), that if ture is passed, will check whether or not the ACPI video driver has already been registered and whether or not the backlight Windows 8 quirk has to be applied. If the quirk has to be applied, it will block the ACPI backlight support and either unregister the backlight interface if the ACPI video driver has already been registered, or register the ACPI video driver without the backlight interface otherwise. Make the i915 driver use __acpi_video_register() instead of acpi_video_register() in i915_driver_load(), and the param passed there is controlled by the i915 module level parameter i915_take_over_backlight, which is set to false by default. This change is evolved from earlier patches of Matthew Garrett, Chun-Yi Lee and Seth Forshee and is heavily based on two patches from Rafael: https://lkml.org/lkml/2013/7/17/720 https://lkml.org/lkml/2013/7/24/806 Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/internal.h | 2 ++ drivers/acpi/video.c| 24 drivers/acpi/video_detect.c | 15 ++- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 5 + drivers/gpu/drm/i915/i915_drv.h | 1 + include/acpi/video.h| 9 +++-- include/linux/acpi.h| 1 + 8 files changed, 47 insertions(+), 12 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-3.fc20.x86_64 -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f466980..75fba17 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (INTEL_INFO(dev)-num_pipes) { /* Must be done after probing outputs */ intel_opregion_init(dev); - acpi_video_register(); + __acpi_video_register(i915_take_over_backlight); } if (IS_GEN5(dev)) I can't compile: DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load': DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit declaration of function '__acpi_video_register' [-Werror=implicit-function-declaration] DEBUG:__acpi_video_register(i915_take_over_backlight); DEBUG:^ DEBUG: cc1: some warnings being treated as errors DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1 DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2 DEBUG: make[2]: *** [drivers/gpu/drm] Error 2 DEBUG: make[1]: *** [drivers/gpu] Error 2 DEBUG: make: *** [drivers] Error 2 -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-3.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Wed, 2013-07-31 at 02:01 +0200, Rafael J. Wysocki wrote: On Thursday, July 18, 2013 02:16:09 AM Rafael J. Wysocki wrote: On Sunday, June 09, 2013 07:01:36 PM Matthew Garrett wrote: Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. Well, after some more time spent on that, we now have a series of 3 patches (different from the $subject one) that we think may be used to address this issue. As far as I can say, it has been tested by multiple people whose systems have those problems and they generally saw improvement. It is not my ideal approach, but it seems to be the least intrusive and/or with the least amount of possible side effects that we can do right now as a general measure (alternatively, we could create a possibly long blacklist table of affected systems with different workarounds for them, but let's just say that is not overwhelmingly attractive). [1/3] Make ACPICA export things that we need for checking OSI(Win8). [2/3] Make acpi_video_device_find_cap() call acpi_video_init_brightness() even if it is not going to register the backlight interface (needed for Thinkpads). [3/3] Avoid using ACPI backlight if i915 is in use and the firmware believes we are Windows 8. Many thanks to everyone involved! So this didn't work, as we had to revert [3/3], but I think we should try to make some progress with this nevertheless. A way forward I'm seeing now could be to (1) Split the ACPI video driver so that it is possible to register the backlight control separately from the event interface. (2) Add a command line option to i915 to make it use the native backlight control (without registering the ACPI one) if set. Make unset the default initially. (3) Fix i915 backlight control issues for all systems known to have them (that may take a while) and flip the defailt for that option to set when we think we're ready. (4) If there still are problem reports, flip the default back to unset and repeat (3). If this converges, everyone will be using the native backlight control by default and the original problem will go away automatically. Thoughts? Rafael Good idea. I have 3-4 laptops with this problem. I can test, but I don't have devices, on which found regressions (in backlight) in previous patch. -- Igor Gnatenko Fedora release 19 (Schrödinger’s Cat) Linux 3.10.3-300.fc19.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Wed, 2013-07-31 at 02:01 +0200, Rafael J. Wysocki wrote: > On Thursday, July 18, 2013 02:16:09 AM Rafael J. Wysocki wrote: > > On Sunday, June 09, 2013 07:01:36 PM Matthew Garrett wrote: > > > Windows 8 introduced new policy for backlight control by pushing it out to > > > graphics drivers. This appears to have coincided with a range of vendors > > > adding Windows 8 checks to their backlight control code which trigger > > > either > > > awkward behaviour (Lenovo) or complete brokenness (some Dells). The > > > simplest > > > thing to do would be to just disable ACPI backlight control entirely if > > > the > > > firmware indicates Windows 8 support, but it's entirely possible that > > > individual graphics drivers might still make use of the ACPI > > > functionality in > > > preference to native control. > > > > > > The first two patches in this series are picked from other patchesets > > > aimed at > > > solving similar problems. The last simply unregisters ACPI backlight > > > control > > > on Windows 8 systems when using an Intel GPU. Similar code could be added > > > to > > > other drivers, but I'm reluctant to do so without further investigation as > > > to the behaviour of the vendor drivers under Windows. > > > > Well, after some more time spent on that, we now have a series of 3 patches > > (different from the $subject one) that we think may be used to address this > > issue. As far as I can say, it has been tested by multiple people whose > > systems have those problems and they generally saw improvement. > > > > It is not my ideal approach, but it seems to be the least intrusive and/or > > with the least amount of possible side effects that we can do right now > > as a general measure (alternatively, we could create a possibly long > > blacklist table of affected systems with different workarounds for them, > > but let's just say that is not overwhelmingly attractive). > > > > [1/3] Make ACPICA export things that we need for checking OSI(Win8). > > > > [2/3] Make acpi_video_device_find_cap() call acpi_video_init_brightness() > > even > > if it is not going to register the backlight interface (needed for > > Thinkpads). > > > > [3/3] Avoid using ACPI backlight if i915 is in use and the firmware believes > > we are Windows 8. > > > > Many thanks to everyone involved! > > So this didn't work, as we had to revert [3/3], but I think we should try to > make some progress with this nevertheless. A way forward I'm seeing now could > be to > (1) Split the ACPI video driver so that it is possible to register the > backlight control separately from the event interface. > (2) Add a command line option to i915 to make it use the native backlight > control (without registering the ACPI one) if set. Make unset the > default initially. > (3) Fix i915 backlight control issues for all systems known to have them > (that may take a while) and flip the defailt for that option to set when > we > think we're ready. > (4) If there still are problem reports, flip the default back to unset and > repeat (3). > > If this converges, everyone will be using the native backlight control by > default and the original problem will go away automatically. > > Thoughts? > > Rafael > > Good idea. I have 3-4 laptops with this problem. I can test, but I don't have devices, on which found regressions (in backlight) in previous patch. -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.10.3-300.fc19.x86_64
[PATCH 2/3] ACPI / video: Always call acpi_video_init_brightness() on init
From: Matthew Garrett <matthew.garr...@nebula.com> We have to call acpi_video_init_brightness() even if we're not going to initialise the backlight - Thinkpads seem to use this as the trigger for enabling ACPI notifications rather than handling it in firmware. [rjw: Drop the brightness object created by acpi_video_init_brightness() if we are not going to use it.] Signed-off-by: Matthew Garrett Signed-off-by: Rafael J. Wysocki Tested-by: Igor Gnatenko --- drivers/acpi/video.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) Index: linux-pm/drivers/acpi/video.c === --- linux-pm.orig/drivers/acpi/video.c +++ linux-pm/drivers/acpi/video.c @@ -884,6 +884,9 @@ static void acpi_video_device_find_cap(s if (acpi_has_method(device->dev->handle, "_DDC")) device->cap._DDC = 1; + if (acpi_video_init_brightness(device)) + return; + if (acpi_video_backlight_support()) { struct backlight_properties props; struct pci_dev *pdev; @@ -893,9 +896,6 @@ static void acpi_video_device_find_cap(s static int count = 0; char *name; - result = acpi_video_init_brightness(device); - if (result) - return; name = kasprintf(GFP_KERNEL, "acpi_video%d", count); if (!name) return; @@ -955,6 +955,11 @@ static void acpi_video_device_find_cap(s if (result) printk(KERN_ERR PREFIX "Create sysfs link\n"); + } else { + /* Remove the brightness object. */ + kfree(device->brightness->levels); + kfree(device->brightness); + device->brightness = NULL; } } -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.9.9-302.fc19.x86_64
[PATCH 1/3] ACPICA: expose OSI version
From: Aaron Lu <aaron...@intel.com> Expose acpi_gbl_osi_data so that code outside of ACPICA can check the value of the last successfull _OSI call. The definitions for OSI versions are moved to actypes.h so that other components can access them too. Based on a patch from Matthew Garrett which in turn was based on an earlier patch from Seth Forshee. [rjw: Changelog] Signed-off-by: Aaron Lu Signed-off-by: Rafael J. Wysocki Tested-by: Igor Gnatenko --- drivers/acpi/acpica/aclocal.h | 13 - include/acpi/acpixf.h | 1 + include/acpi/actypes.h| 15 +++ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index dfed265..d4a49016 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -931,19 +931,6 @@ struct acpi_bit_register_info { /* Structs and definitions for _OSI support and I/O port validation */ -#define ACPI_OSI_WIN_2000 0x01 -#define ACPI_OSI_WIN_XP 0x02 -#define ACPI_OSI_WIN_XP_SP1 0x03 -#define ACPI_OSI_WINSRV_20030x04 -#define ACPI_OSI_WIN_XP_SP2 0x05 -#define ACPI_OSI_WINSRV_2003_SP10x06 -#define ACPI_OSI_WIN_VISTA 0x07 -#define ACPI_OSI_WINSRV_20080x08 -#define ACPI_OSI_WIN_VISTA_SP1 0x09 -#define ACPI_OSI_WIN_VISTA_SP2 0x0A -#define ACPI_OSI_WIN_7 0x0B -#define ACPI_OSI_WIN_8 0x0C - #define ACPI_ALWAYS_ILLEGAL 0x00 struct acpi_interface_info { diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 1b09300..22d497e 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -62,6 +62,7 @@ extern u32 acpi_current_gpe_count; extern struct acpi_table_fadt acpi_gbl_FADT; extern u8 acpi_gbl_system_awake_and_running; extern u8 acpi_gbl_reduced_hardware; /* ACPI 5.0 */ +extern u8 acpi_gbl_osi_data; /* Runtime configuration of debug print levels */ diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index a64adcc..22b03c9 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -1144,4 +1144,19 @@ struct acpi_memory_list { #endif }; +/* Definitions for _OSI support */ + +#define ACPI_OSI_WIN_2000 0x01 +#define ACPI_OSI_WIN_XP 0x02 +#define ACPI_OSI_WIN_XP_SP1 0x03 +#define ACPI_OSI_WINSRV_20030x04 +#define ACPI_OSI_WIN_XP_SP2 0x05 +#define ACPI_OSI_WINSRV_2003_SP10x06 +#define ACPI_OSI_WIN_VISTA 0x07 +#define ACPI_OSI_WINSRV_20080x08 +#define ACPI_OSI_WIN_VISTA_SP1 0x09 +#define ACPI_OSI_WIN_VISTA_SP2 0x0A +#define ACPI_OSI_WIN_7 0x0B +#define ACPI_OSI_WIN_8 0x0C + #endif /* __ACTYPES_H__ */ -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.9.9-302.fc19.x86_64
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 13:38 +0200, Rafael J. Wysocki wrote: > On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote: > > On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote: > > > On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote: > > > > On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote: > > > > > Hmm. I found regression in user-space. In GNOME (maybe and other DEs) > > > > > we no longer see switch status of backlight. > > > > > > > > Yeah, I can duplicate that. Rafael, we have to call > > > > acpi_video_init_brightness() even if we're not going to initialise the > > > > backlight - Thinkpads seem to use this as the trigger for enabling ACPI > > > > notifications rather than handling it in firmware. This seems to do the > > > > job: > > > > > > Igor, does this additional patch from Matthew help? > > Yes. With this patch I have backlight switch indicator on my ThinkPad X230. > > OK, thanks for the confirmation. > > Can you please also check if applying the appended patch on top of the > Matthew's > one changes anything (ie. things still work)? Yes. I've tested and not found regressions in indicator or in switcher. Good work. > > Rafael > > > --- Tested-by: Igor Gnatenko > drivers/acpi/video.c |5 + > 1 file changed, 5 insertions(+) > > Index: linux-pm/drivers/acpi/video.c > === > --- linux-pm.orig/drivers/acpi/video.c > +++ linux-pm/drivers/acpi/video.c > @@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s > if (result) > printk(KERN_ERR PREFIX "Create sysfs link\n"); > > + } else { > + /* Remove the brightness object. */ > + kfree(device->brightness->levels); > + kfree(device->brightness); > + device->brightness = NULL; > } > } > > > -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote: > On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote: > > On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote: > > > Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we > > > no longer see switch status of backlight. > > > > Yeah, I can duplicate that. Rafael, we have to call > > acpi_video_init_brightness() even if we're not going to initialise the > > backlight - Thinkpads seem to use this as the trigger for enabling ACPI > > notifications rather than handling it in firmware. This seems to do the > > job: > > Igor, does this additional patch from Matthew help? Yes. With this patch I have backlight switch indicator on my ThinkPad X230. > > Rafael > > Tested-by: Igor Gnatenko > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > > index 01b1a25..71865f7 100644 > > --- a/drivers/acpi/video.c > > +++ b/drivers/acpi/video.c > > @@ -900,6 +900,9 @@ static void acpi_video_device_find_cap(struct > > acpi_video_device *device) > > device->cap._DDC = 1; > > } > > > > + if (acpi_video_init_brightness(device)) > > + return; > > + > > if (acpi_video_verify_backlight_support()) { > > struct backlight_properties props; > > struct pci_dev *pdev; > > @@ -909,9 +912,6 @@ static void acpi_video_device_find_cap(struct > > acpi_video_device *device) > > static int count = 0; > > char *name; > > > > - result = acpi_video_init_brightness(device); > > - if (result) > > - return; > > name = kasprintf(GFP_KERNEL, "acpi_video%d", count); > > if (!name) > > return; > > > > > > -- > > Matthew Garrett | mjg59 at srcf.ucam.org > > -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-07-16 at 01:53 +0200, Rafael J. Wysocki wrote: On Monday, July 15, 2013 05:06:09 PM Igor Gnatenko wrote: On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote: [...] I can't build it. Where did I go wrong? Probably nowhere, you tried to build the ACPI video driver as a module, that's all. And you need to apply https://patchwork.kernel.org/patch/2812951/ before. Please try the appended version (on top of https://patchwork.kernel.org/patch/2812951/). Thanks, Rafael Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- From: Rafael J. Wysocki rafael.j.wyso...@intel.com Subject: ACPI / video / i915: No ACPI backlight if firmware expects Windows 8 According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. There's a problem with that approach, however, because simply avoiding to register the ACPI backlight interface if the firmware calls _OSI for Windows 8 may not work in the following situations: (1) The ACPI backlight interface actually works on the given system and the i915 driver is not loaded (e.g. another graphics driver is used). (2) The ACPI backlight interface doesn't work on the given system, but there is a vendor platform driver that will register its own, equally broken, backlight interface if not prevented from doing so by the ACPI subsystem. Therefore we need to allow the ACPI backlight interface to be registered until the i915 driver is loaded which then will unregister it if the firmware has called _OSI for Windows 8 (or will register the ACPI video driver without backlight support if not already present). For this reason, introduce an alternative function for registering ACPI video, acpi_video_register_with_quirks(), that will check whether or not the ACPI video driver has already been registered and whether or not the backlight Windows 8 quirk has to be applied. If the quirk has to be applied, it will block the ACPI backlight support and either unregister the backlight interface if the ACPI video driver has already been registered, or register the ACPI video driver without the backlight interface otherwise. Make the i915 driver use acpi_video_register_with_quirks() instead of acpi_video_register() in i915_driver_load(). This change is based on earlier patches from Matthew Garrett, Chun-Yi Lee and Seth Forshee and Aaron Lu's comments. Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com --- drivers/acpi/internal.h | 11 ++ drivers/acpi/video.c| 65 drivers/acpi/video_detect.c | 21 drivers/gpu/drm/i915/i915_dma.c |2 - include/acpi/video.h| 11 ++ include/linux/acpi.h|1 6 files changed, 103 insertions(+), 8 deletions(-) Index: linux-pm/drivers/acpi/video.c === --- linux-pm.orig/drivers/acpi/video.c +++ linux-pm/drivers/acpi/video.c @@ -44,6 +44,8 @@ #include linux/suspend.h #include acpi/video.h +#include internal.h + #define PREFIX ACPI: #define ACPI_VIDEO_BUS_NAME Video Bus @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s device-cap._DDC = 1; } - if (acpi_video_backlight_support()) { + if (acpi_video_verify_backlight_support()) { struct backlight_properties props; struct pci_dev *pdev; acpi_handle acpi_parent; @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct return 0; } +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl, + void *context, void **rv) +{ + struct acpi_device *acpi_dev; + struct acpi_video_bus *video; + struct acpi_video_device *dev, *next; + + if (acpi_bus_get_device(handle, acpi_dev)) + return AE_OK; + + if (acpi_match_device_ids(acpi_dev, video_device_ids)) + return AE_OK; + + video = acpi_driver_data(acpi_dev); + if (!video) + return AE_OK; + + acpi_video_bus_stop_devices(video); + mutex_lock(video-device_list_lock); + list_for_each_entry_safe(dev, next, video-video_device_list, entry) { + if (dev-backlight) { + backlight_device_unregister(dev-backlight); + dev-backlight = NULL; + kfree(dev-brightness-levels); + kfree(dev-brightness
Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 13:38 +0200, Rafael J. Wysocki wrote: On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote: On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote: On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote: On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote: Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no longer see switch status of backlight. Yeah, I can duplicate that. Rafael, we have to call acpi_video_init_brightness() even if we're not going to initialise the backlight - Thinkpads seem to use this as the trigger for enabling ACPI notifications rather than handling it in firmware. This seems to do the job: Igor, does this additional patch from Matthew help? Yes. With this patch I have backlight switch indicator on my ThinkPad X230. OK, thanks for the confirmation. Can you please also check if applying the appended patch on top of the Matthew's one changes anything (ie. things still work)? Yes. I've tested and not found regressions in indicator or in switcher. Good work. Rafael --- Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com drivers/acpi/video.c |5 + 1 file changed, 5 insertions(+) Index: linux-pm/drivers/acpi/video.c === --- linux-pm.orig/drivers/acpi/video.c +++ linux-pm/drivers/acpi/video.c @@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s if (result) printk(KERN_ERR PREFIX Create sysfs link\n); + } else { + /* Remove the brightness object. */ + kfree(device-brightness-levels); + kfree(device-brightness); + device-brightness = NULL; } } -- Igor Gnatenko Fedora release 19 (Schrödinger’s Cat) Linux 3.11.0-0.rc0.git7.1.fc20.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] ACPICA: expose OSI version
From: Aaron Lu aaron...@intel.com Expose acpi_gbl_osi_data so that code outside of ACPICA can check the value of the last successfull _OSI call. The definitions for OSI versions are moved to actypes.h so that other components can access them too. Based on a patch from Matthew Garrett which in turn was based on an earlier patch from Seth Forshee. [rjw: Changelog] Signed-off-by: Aaron Lu aaron...@intel.com Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/acpica/aclocal.h | 13 - include/acpi/acpixf.h | 1 + include/acpi/actypes.h| 15 +++ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index dfed265..d4a49016 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -931,19 +931,6 @@ struct acpi_bit_register_info { /* Structs and definitions for _OSI support and I/O port validation */ -#define ACPI_OSI_WIN_2000 0x01 -#define ACPI_OSI_WIN_XP 0x02 -#define ACPI_OSI_WIN_XP_SP1 0x03 -#define ACPI_OSI_WINSRV_20030x04 -#define ACPI_OSI_WIN_XP_SP2 0x05 -#define ACPI_OSI_WINSRV_2003_SP10x06 -#define ACPI_OSI_WIN_VISTA 0x07 -#define ACPI_OSI_WINSRV_20080x08 -#define ACPI_OSI_WIN_VISTA_SP1 0x09 -#define ACPI_OSI_WIN_VISTA_SP2 0x0A -#define ACPI_OSI_WIN_7 0x0B -#define ACPI_OSI_WIN_8 0x0C - #define ACPI_ALWAYS_ILLEGAL 0x00 struct acpi_interface_info { diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 1b09300..22d497e 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -62,6 +62,7 @@ extern u32 acpi_current_gpe_count; extern struct acpi_table_fadt acpi_gbl_FADT; extern u8 acpi_gbl_system_awake_and_running; extern u8 acpi_gbl_reduced_hardware; /* ACPI 5.0 */ +extern u8 acpi_gbl_osi_data; /* Runtime configuration of debug print levels */ diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index a64adcc..22b03c9 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -1144,4 +1144,19 @@ struct acpi_memory_list { #endif }; +/* Definitions for _OSI support */ + +#define ACPI_OSI_WIN_2000 0x01 +#define ACPI_OSI_WIN_XP 0x02 +#define ACPI_OSI_WIN_XP_SP1 0x03 +#define ACPI_OSI_WINSRV_20030x04 +#define ACPI_OSI_WIN_XP_SP2 0x05 +#define ACPI_OSI_WINSRV_2003_SP10x06 +#define ACPI_OSI_WIN_VISTA 0x07 +#define ACPI_OSI_WINSRV_20080x08 +#define ACPI_OSI_WIN_VISTA_SP1 0x09 +#define ACPI_OSI_WIN_VISTA_SP2 0x0A +#define ACPI_OSI_WIN_7 0x0B +#define ACPI_OSI_WIN_8 0x0C + #endif /* __ACTYPES_H__ */ -- Igor Gnatenko Fedora release 19 (Schrödinger’s Cat) Linux 3.9.9-302.fc19.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] ACPI / video: Always call acpi_video_init_brightness() on init
From: Matthew Garrett matthew.garr...@nebula.com We have to call acpi_video_init_brightness() even if we're not going to initialise the backlight - Thinkpads seem to use this as the trigger for enabling ACPI notifications rather than handling it in firmware. [rjw: Drop the brightness object created by acpi_video_init_brightness() if we are not going to use it.] Signed-off-by: Matthew Garrett matthew.garr...@nebula.com Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/video.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) Index: linux-pm/drivers/acpi/video.c === --- linux-pm.orig/drivers/acpi/video.c +++ linux-pm/drivers/acpi/video.c @@ -884,6 +884,9 @@ static void acpi_video_device_find_cap(s if (acpi_has_method(device-dev-handle, _DDC)) device-cap._DDC = 1; + if (acpi_video_init_brightness(device)) + return; + if (acpi_video_backlight_support()) { struct backlight_properties props; struct pci_dev *pdev; @@ -893,9 +896,6 @@ static void acpi_video_device_find_cap(s static int count = 0; char *name; - result = acpi_video_init_brightness(device); - if (result) - return; name = kasprintf(GFP_KERNEL, acpi_video%d, count); if (!name) return; @@ -955,6 +955,11 @@ static void acpi_video_device_find_cap(s if (result) printk(KERN_ERR PREFIX Create sysfs link\n); + } else { + /* Remove the brightness object. */ + kfree(device-brightness-levels); + kfree(device-brightness); + device-brightness = NULL; } } -- Igor Gnatenko Fedora release 19 (Schrödinger’s Cat) Linux 3.9.9-302.fc19.x86_64 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no longer see switch status of backlight. -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-07-16 at 01:53 +0200, Rafael J. Wysocki wrote: > On Monday, July 15, 2013 05:06:09 PM Igor Gnatenko wrote: > > On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote: > > [...] > > > > > I can't build it. Where did I go wrong? > > Probably nowhere, you tried to build the ACPI video driver as a module, that's > all. And you need to apply https://patchwork.kernel.org/patch/2812951/ > before. > > Please try the appended version (on top of > https://patchwork.kernel.org/patch/2812951/). > > Thanks, > Rafael > Tested-by: Igor Gnatenko > > --- > From: Rafael J. Wysocki > Subject: ACPI / video / i915: No ACPI backlight if firmware expects Windows 8 > > According to Matthew Garrett, "Windows 8 leaves backlight control up > to individual graphics drivers rather than making ACPI calls itself. > There's plenty of evidence to suggest that the Intel driver for > Windows [8] doesn't use the ACPI interface, including the fact that > it's broken on a bunch of machines when the OS claims to support > Windows 8. The simplest thing to do appears to be to disable the > ACPI backlight interface on these systems". > > There's a problem with that approach, however, because simply > avoiding to register the ACPI backlight interface if the firmware > calls _OSI for Windows 8 may not work in the following situations: > (1) The ACPI backlight interface actually works on the given system > and the i915 driver is not loaded (e.g. another graphics driver > is used). > (2) The ACPI backlight interface doesn't work on the given system, > but there is a vendor platform driver that will register its > own, equally broken, backlight interface if not prevented from > doing so by the ACPI subsystem. > Therefore we need to allow the ACPI backlight interface to be > registered until the i915 driver is loaded which then will unregister > it if the firmware has called _OSI for Windows 8 (or will register > the ACPI video driver without backlight support if not already > present). > > For this reason, introduce an alternative function for registering > ACPI video, acpi_video_register_with_quirks(), that will check > whether or not the ACPI video driver has already been registered > and whether or not the backlight Windows 8 quirk has to be applied. > If the quirk has to be applied, it will block the ACPI backlight > support and either unregister the backlight interface if the ACPI > video driver has already been registered, or register the ACPI > video driver without the backlight interface otherwise. Make > the i915 driver use acpi_video_register_with_quirks() instead of > acpi_video_register() in i915_driver_load(). > > This change is based on earlier patches from Matthew Garrett, > Chun-Yi Lee and Seth Forshee and Aaron Lu's comments. > > Signed-off-by: Rafael J. Wysocki > --- > drivers/acpi/internal.h | 11 ++ > drivers/acpi/video.c| 65 > > drivers/acpi/video_detect.c | 21 > drivers/gpu/drm/i915/i915_dma.c |2 - > include/acpi/video.h| 11 ++ > include/linux/acpi.h|1 > 6 files changed, 103 insertions(+), 8 deletions(-) > > Index: linux-pm/drivers/acpi/video.c > === > --- linux-pm.orig/drivers/acpi/video.c > +++ linux-pm/drivers/acpi/video.c > @@ -44,6 +44,8 @@ > #include > #include > > +#include "internal.h" > + > #define PREFIX "ACPI: " > > #define ACPI_VIDEO_BUS_NAME "Video Bus" > @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s > device->cap._DDC = 1; > } > > - if (acpi_video_backlight_support()) { > + if (acpi_video_verify_backlight_support()) { > struct backlight_properties props; > struct pci_dev *pdev; > acpi_handle acpi_parent; > @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct > return 0; > } > > +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl, > + void *context, void **rv) > +{ > + struct acpi_device *acpi_dev; > + struct acpi_video_bus *video; > + struct acpi_video_device *dev, *next; > + > + if (acpi_bus_get_device(handle, _dev)) > + return AE_OK; > + > + if (acpi_match_device_ids(acpi_dev, video_device_ids)) > + return AE_OK; > + > + video = acpi_driver_data(acpi_dev); > + if (!video) > + return AE_OK; > + > + acp
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
cklist in video_detect.c > @@ -278,6 +291,14 @@ int acpi_video_backlight_support(void) > } > EXPORT_SYMBOL(acpi_video_backlight_support); > > +/* For the ACPI video driver's use only. */ > +bool acpi_video_verify_backlight_support(void) > +{ > + return (acpi_video_support & ACPI_VIDEO_SKIP_BACKLIGHT) ? > + false : acpi_video_backlight_support(); > +} > +EXPORT_SYMBOL(acpi_video_verify_backlight_support); > + > /* > * Use acpi_backlight=vendor/video to force that backlight switching > * is processed by vendor specific acpi drivers or video.ko driver. > Index: linux-pm/include/linux/acpi.h > === > --- linux-pm.orig/include/linux/acpi.h > +++ linux-pm/include/linux/acpi.h > @@ -191,6 +191,7 @@ extern bool wmi_has_guid(const char *gui > #define ACPI_VIDEO_BACKLIGHT_DMI_VIDEO 0x0200 > #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR 0x0400 > #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO0x0800 > +#define ACPI_VIDEO_SKIP_BACKLIGHT0x1000 > > #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) > > Index: linux-pm/drivers/acpi/internal.h > === > --- linux-pm.orig/drivers/acpi/internal.h > +++ linux-pm/drivers/acpi/internal.h > @@ -164,4 +164,15 @@ struct platform_device; > int acpi_create_platform_device(struct acpi_device *adev, > const struct acpi_device_id *id); > > +/*-- > + Video > + -- > */ > +#ifdef CONFIG_ACPI_VIDEO > +bool acpi_video_backlight_quirks(void); > +bool acpi_video_verify_backlight_support(void); > +#else > +static inline bool acpi_video_backlight_quirks(void) { return false; } > +static inline bool acpi_video_verify_backlight_support(void) { return false; > } > +#endif > + > #endif /* _ACPI_INTERNAL_H_ */ I can't build it. Where did I go wrong? drivers/acpi/video_detect.c:239:6: error: redefinition of 'acpi_video_backlight_quirks' bool acpi_video_backlight_quirks(void) ^ In file included from drivers/acpi/video_detect.c:41:0: drivers/acpi/internal.h:174:60: note: previous definition of 'acpi_video_backlight_quirks' was here static inline bool acpi_video_backlight_quirks(void) { return false; } ^ drivers/acpi/video_detect.c: In function 'acpi_video_backlight_quirks': drivers/acpi/video_detect.c:241:6: error: 'acpi_gbl_osi_data' undeclared (first use in this function) if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8) { ^ drivers/acpi/video_detect.c:241:6: note: each undeclared identifier is reported only once for each function it appears in drivers/acpi/video_detect.c:241:27: error: 'ACPI_OSI_WIN_8' undeclared (first use in this function) if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8) { ^ drivers/acpi/video_detect.c: At top level: drivers/acpi/video_detect.c:295:6: error: redefinition of 'acpi_video_verify_backlight_support' bool acpi_video_verify_backlight_support(void) ^ In file included from drivers/acpi/video_detect.c:41:0: drivers/acpi/internal.h:175:60: note: previous definition of 'acpi_video_verify_backlight_support' was here static inline bool acpi_video_verify_backlight_support(void) { return false; } ^ -- Igor Gnatenko Fedora release 19 (Schr?dinger?s Cat) Linux 3.9.9-302.fc19.x86_64