[PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic
On 06/11/2015 05:19 PM, Hans de Goede wrote: > Hi, > > On 11-06-15 11:00, Aaron Lu wrote: >> On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote: >>> Currently we have 2 kernel commandline options + dmi-quirks in 3 places all >>> interacting (in interesting ways) to select which which backlight interface >>> to use. On the commandline we've acpi_backlight=[video|vendor] and >>> video.use_native_backlight=[0|1]. DMI quirks we have in >>> acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c . >>> >>> This commit is the first step to cleaning this up, replacing the 2 cmdline >>> options with just acpi_video.backlight=[video|vendor|native|none], and >>> adding a new API to video_detect.c to reflect this. >> >> I like backlight=[firmware|platform|raw|none] better, but that would >> require to put the selection/quirk logic to backlight core. What do you >> think? > > I think you are asking 3 separate questions: > > 1) Should the backlight selection logic maybe be moved to the backlight > class code ? > > My answer to that one is: no it should not as it is x86/acpi specific. > > 2) What would be a good kernel cmdline option name for this? > > Given the answer to one I think it is good to keep acpi in the name > space of the cmdline option. Also note that moving the code to the > backlight class code does not magically give us a backlight= cmdline > option. None module-name prefixed cmdline options are only available > to code which is always builtin, and the backlight class code can be > build as a module, so putting the cmdline option in the backlight > class will give us: backlight.backlight= > > So all in all I think acpi_video.backlight= is actually pretty ok. > > 3) What would be a good kernel cmdline option values for this? > > I agree that the old vendor/video/native value labels are not > necessarily the best labels for this, but I would like to keep > them as those are what people are used to. Ideally I would like > to have just kept acpi_backlight=[video|vendor|native|none] > but that is not possible without making all of the acpi_video > code always builtin. OK, I don't have other comments, thanks for doing this. Regards, Aaron > > Regards, > > Hans > >>> Follow up commits will also move other related code, like unregistering the >>> acpi_video backlight interface if it was registered before other drivers >>> which take priority over it are loaded, to video_detect.c where this >>> logic really belongs. >>> >>> Signed-off-by: Hans de Goede >>> --- >>> drivers/acpi/video_detect.c | 172 >>> +--- >>> include/acpi/video.h| 17 + >>> 2 files changed, 100 insertions(+), 89 deletions(-) >>> >>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c >>> index 0bc8b98..01c8c46 100644 >>> --- a/drivers/acpi/video_detect.c >>> +++ b/drivers/acpi/video_detect.c >>> @@ -1,4 +1,6 @@ >>> /* >>> + * Copyright (C) 2015 Red Hat Inc. >>> + * Hans de Goede >>>* Copyright (C) 2008 SuSE Linux Products GmbH >>>* Thomas Renninger >>>* >>> @@ -9,28 +11,24 @@ >>>* acpi_get_pci_dev() can be called to identify ACPI graphics >>>* devices for which a real graphics card is plugged in >>>* >>> - * Now acpi_video_get_capabilities() can be called to check which >>> - * capabilities the graphics cards plugged in support. The check for >>> general >>> - * video capabilities will be triggered by the first caller of >>> - * acpi_video_get_capabilities(NULL); which will happen when the first >>> - * backlight switching supporting driver calls: >>> - * acpi_video_backlight_support(); >>> - * >>>* Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix >>> B) >>>* are available, video.ko should be used to handle the device. >>>* >>>* Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop, >>>* sony_acpi,... can take care about backlight brightness. >>>* >>> - * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a >>> module (m) >>> - * this file will not be compiled, acpi_video_get_capabilities() and >>> - * acpi_video_backlight_support() will always return 0 and vendor specific >>> - * drivers always can handle b
[PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic
On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote:
> Currently we have 2 kernel commandline options + dmi-quirks in 3 places all
> interacting (in interesting ways) to select which which backlight interface
> to use. On the commandline we've acpi_backlight=[video|vendor] and
> video.use_native_backlight=[0|1]. DMI quirks we have in
> acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
>
> This commit is the first step to cleaning this up, replacing the 2 cmdline
> options with just acpi_video.backlight=[video|vendor|native|none], and
> adding a new API to video_detect.c to reflect this.
I like backlight=[firmware|platform|raw|none] better, but that would
require to put the selection/quirk logic to backlight core. What do you
think?
Regards,
Aaron
>
> Follow up commits will also move other related code, like unregistering the
> acpi_video backlight interface if it was registered before other drivers
> which take priority over it are loaded, to video_detect.c where this
> logic really belongs.
>
> Signed-off-by: Hans de Goede
> ---
> drivers/acpi/video_detect.c | 172
> +---
> include/acpi/video.h| 17 +
> 2 files changed, 100 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 0bc8b98..01c8c46 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -1,4 +1,6 @@
> /*
> + * Copyright (C) 2015 Red Hat Inc.
> + * Hans de Goede
> * Copyright (C) 2008 SuSE Linux Products GmbH
> * Thomas Renninger
> *
> @@ -9,28 +11,24 @@
> * acpi_get_pci_dev() can be called to identify ACPI graphics
> * devices for which a real graphics card is plugged in
> *
> - * Now acpi_video_get_capabilities() can be called to check which
> - * capabilities the graphics cards plugged in support. The check for general
> - * video capabilities will be triggered by the first caller of
> - * acpi_video_get_capabilities(NULL); which will happen when the first
> - * backlight switching supporting driver calls:
> - * acpi_video_backlight_support();
> - *
> * Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
> * are available, video.ko should be used to handle the device.
> *
> * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
> * sony_acpi,... can take care about backlight brightness.
> *
> - * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module
> (m)
> - * this file will not be compiled, acpi_video_get_capabilities() and
> - * acpi_video_backlight_support() will always return 0 and vendor specific
> - * drivers always can handle backlight.
> + * Backlight drivers can use acpi_video_get_backlight_type() to determine
> + * which driver should handle the backlight.
> *
> + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module
> (m)
> + * this file will not be compiled and acpi_video_get_backlight_type() will
> + * always return acpi_backlight_vendor.
> */
>
> +#include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -38,20 +36,24 @@
> ACPI_MODULE_NAME("video");
> #define _COMPONENT ACPI_VIDEO_COMPONENT
>
> -static long acpi_video_support;
> -static bool acpi_video_caps_checked;
> +static enum acpi_backlight_type acpi_backlight_cmdline =
> acpi_backlight_undef;
> +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
>
> static char acpi_backlight_str[16];
> module_param_string(backlight, acpi_backlight_str, 16, 0444);
> MODULE_PARM_DESC(backlight,
> - "Select which backlight interface to use [vendor|video]");
> + "Select which backlight interface to use [vendor|video|native]");
>
> static void acpi_video_parse_cmdline(void)
> {
> if (!strcmp("vendor", acpi_backlight_str))
> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
> + acpi_backlight_cmdline = acpi_backlight_vendor;
> if (!strcmp("video", acpi_backlight_str))
> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO;
> + acpi_backlight_cmdline = acpi_backlight_video;
> + if (!strcmp("native", acpi_backlight_str))
> + acpi_backlight_cmdline = acpi_backlight_native;
> + if (!strcmp("none", acpi_backlight_str))
> + acpi_backlight_cmdline = acpi_backlight_none;
> }
>
> static acpi_status
> @@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void
> **rv)
> * buggy */
> static int video_detect_force_vendor(const struct dmi_system_id *d)
> {
> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> + acpi_backlight_dmi = acpi_backlight_vendor;
> return 0;
> }
>
> @@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = {
> };
>
> /*
> - * Returns the video capabilities of a specific
[PATCH v2] acpi-video: Add a parameter to not register the backlight sysfs interface
On Tue, Jun 09, 2015 at 11:54:45PM +0200, Hans de Goede wrote: > Hi, > > On 06/09/2015 11:10 AM, Aaron Lu wrote: > >On Tue, Jun 09, 2015 at 10:32:25AM +0200, Hans de Goede wrote: > >>On some systems acpi-video backlight is broken in the sense that it cannot > >>control the brightness of the backlight, but it must still be called on > >>resume to power-up the backlight after resume. > > > >All the video module does on resume is a backlight set operation, it > >can't control backlight but can turn on the screen on resume? Hmm... > > > >I'll ask Sylvain to attach acpidump, let's see if there is anything > >special there. > > Ok, lets see what comes out of that. Note in the mean time Sylvain has > attached his acpidump. Thanks. According to the discussion in the bugzilla place, it doesn't seem we have any other way to handle this at the moment. Acked-by: Aaron Lu Best wishes, Aaron
[PATCH v2] acpi-video: Add a parameter to not register the backlight sysfs interface
On Tue, Jun 09, 2015 at 10:32:25AM +0200, Hans de Goede wrote:
> On some systems acpi-video backlight is broken in the sense that it cannot
> control the brightness of the backlight, but it must still be called on
> resume to power-up the backlight after resume.
All the video module does on resume is a backlight set operation, it
can't control backlight but can turn on the screen on resume? Hmm...
I'll ask Sylvain to attach acpidump, let's see if there is anything
special there.
Regards,
Aaron
>
> This commit allows these systems to work by going through all the usual
> backlight control moves, while not registering a sysfs backlight
> interface.
>
> This commit also adds a quirk enabling this parameter on Toshiba Portege
> R830 systems which are known to be affected by this.
>
> I wish there was a better way to deal with this, but we've been unable to
> find one.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=21012
> Buglink: https://bugs.freedesktop.org/show_bug.cgi?id=82634
> Reported-and-tested-by: Sylvain Pasche
> Signed-off-by: Hans de Goede
> ---
> Changes in v2:
> -Simplify check in acpi_video_switch_brightness()
> -If backlight registration fails set device->backlight to NULL, rather
> then leaving the PTR_ERR in there and trying to deref this later
> (this fixes a pre-existing bug)
> ---
> drivers/acpi/video.c | 43 +++
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 518f0e1..3bc4c68 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -92,6 +92,9 @@ static int use_native_backlight_param =
> NATIVE_BACKLIGHT_NOT_SET;
> module_param_named(use_native_backlight, use_native_backlight_param, int,
> 0444);
> static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET;
>
> +static int disable_backlight_sysfs_if = -1;
> +module_param(disable_backlight_sysfs_if, int, 0444);
> +
> static int register_count;
> static struct mutex video_list_lock;
> static struct list_head video_bus_head;
> @@ -431,6 +434,14 @@ static int __init video_enable_native_backlight(const
> struct dmi_system_id *d)
> return 0;
> }
>
> +static int __init video_disable_backlight_sysfs_if(
> + const struct dmi_system_id *d)
> +{
> + if (disable_backlight_sysfs_if == -1)
> + disable_backlight_sysfs_if = 1;
> + return 0;
> +}
> +
> static struct dmi_system_id video_dmi_table[] __initdata = {
> /*
>* Broken _BQC workaround
> http://bugzilla.kernel.org/show_bug.cgi?id=13121
> @@ -592,6 +603,23 @@ static struct dmi_system_id video_dmi_table[] __initdata
> = {
> DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro12,1"),
> },
> },
> +
> + /*
> + * Some machines have a broken acpi-video interface for brightness
> + * control, but still need an acpi_video_device_lcd_set_level() call
> + * on resume to turn the backlight power on. We Enable backlight
> + * control on these systems, but do not register a backlight sysfs
> + * as brightness control does not work.
> + */
> + {
> + /* https://bugs.freedesktop.org/show_bug.cgi?id=82634 */
> + .callback = video_disable_backlight_sysfs_if,
> + .ident = "Toshiba Portege R830",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R830"),
> + },
> + },
> {}
> };
>
> @@ -1391,7 +1419,7 @@ acpi_video_switch_brightness(struct work_struct *work)
> int result = -EINVAL;
>
> /* no warning message if acpi_backlight=vendor or a quirk is used */
> - if (!acpi_video_verify_backlight_support())
> + if (!device->backlight)
> return;
>
> if (!device->brightness)
> @@ -1666,8 +1694,9 @@ static int acpi_video_resume(struct notifier_block *nb,
>
> for (i = 0; i < video->attached_count; i++) {
> video_device = video->attached_array[i].bind_info;
> - if (video_device && video_device->backlight)
> - acpi_video_set_brightness(video_device->backlight);
> + if (video_device && video_device->brightness)
> + acpi_video_device_lcd_set_level(video_device,
> + video_device->brightness->curr);
> }
>
> return NOTIFY_OK;
> @@ -1716,6 +1745,10 @@ static void acpi_video_dev_register_backlight(struct
> acpi_video_device *device)
> result = acpi_video_init_brightness(device);
> if (result)
> return;
> +
> + if (disable_backlight_sysfs_if > 0)
> + return;
> +
> name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> if (!name)
> return;
> @@ -1738,8 +1771,10 @@ static void acpi_video_dev_register_backlight(struct
> acpi_video_device *device)
> &acpi_backlight
[PATCH 0/3] Use acpi_video_unregister_backlight instead of acpi_video_unregister in platfrom backlight drivers
On Tue, Jun 02, 2015 at 10:33:50PM +0800, Aaron Lu wrote: > On Tue, Jun 02, 2015 at 12:14:28PM +0200, Hans de Goede wrote: > > > > If this makes sense (*) then the cooling device stuff should maybe moved > > from the acpi/video.c code to the backlight core code. > > It's just a specific driver that provides thermal capability, no need to > move to the core code. I didn't get this when replying and now I think it's doable. Regards, Aaron
[PATCH 0/3] Use acpi_video_unregister_backlight instead of acpi_video_unregister in platfrom backlight drivers
On Tue, Jun 02, 2015 at 12:14:28PM +0200, Hans de Goede wrote: > Hi, > > On 02-06-15 11:59, Jani Nikula wrote: > >On Mon, 01 Jun 2015, Hans de Goede wrote: > >>Hi All, > >> > >>I'm working on cleaning up the currently somewhat convoluted logic to > >>select which backlight interfaces to register on x86 systems, see: > >>http://lists.freedesktop.org/archives/dri-devel/2014-December/074687.html > >> > >>For a rought outline (details will change in the actual patch-set). > >> > >>These 3 patches are a preparation for that work, as the behavior of the > >>current code is not always consistent (it changes depending on module > >>loading order in some cases). These 3 patches remove this inconsistency > >>which in some cases may result in a behavior change. > >> > >>This way the cleanup can have a consistent base to build upon, and I can > >>ensure that the cleanup itself does not cause any functional changes. > > > >Thanks for doing this. > > > >Slightly unrelated, but you'll end up calling: > > > >acpi_video_unregister_backlight > > acpi_video_bus_unregister_backlight > > acpi_video_dev_unregister_backlight > > thermal_cooling_device_unregister > > > >Uhm, err, what? What business does *that* have in the backlight > >unregister call chain?! > > I think the idea is that having the backlight on can cause the main board > to heat up as there is often a regulator / pwm for the backlight on the > main board. If that is turned off it may help to cool down the main board, > so the backlight is a cooling device in the sense that when it is turned > off it stops generating heat. I think so. > > If this makes sense (*) then the cooling device stuff should maybe moved > from the acpi/video.c code to the backlight core code. It's just a specific driver that provides thermal capability, no need to move to the core code. > > Or does the acpi-video code use some info from the BIOS to determine > whether or not to register a cooling device ? As long as we create the ACPI video backlight interface, we will create the thermal cooling device. If we somehow knows that the ACPI methods to adjust the backlight level doesn't work(a good hint is that the acpi_video_unregister_backlight is called), we should make that cooling device disappear. Regards, Aaron > > Regards, > > Hans > > > > > *) This is tricky, e.g. going low brightness may cause the main board > to heat up more then full brightness depending on how things are hooked > up. > > > > Should that be untangled from the mess too? > > > >BR, > >Jani. > > > > > >> > >>I hope to post a v1 of the actual cleanup patch-set in 1-2 weeks. > >> > >>Regards, > >> > >>Hans > >>___ > >>dri-devel mailing list > >>dri-devel at lists.freedesktop.org > >>http://lists.freedesktop.org/mailman/listinfo/dri-devel > >
[PATCH 0/3] Use acpi_video_unregister_backlight instead of acpi_video_unregister in platfrom backlight drivers
On Tue, Jun 02, 2015 at 12:59:10PM +0300, Jani Nikula wrote: > On Mon, 01 Jun 2015, Hans de Goede wrote: > > Hi All, > > > > I'm working on cleaning up the currently somewhat convoluted logic to > > select which backlight interfaces to register on x86 systems, see: > > http://lists.freedesktop.org/archives/dri-devel/2014-December/074687.html > > > > For a rought outline (details will change in the actual patch-set). > > > > These 3 patches are a preparation for that work, as the behavior of the > > current code is not always consistent (it changes depending on module > > loading order in some cases). These 3 patches remove this inconsistency > > which in some cases may result in a behavior change. > > > > This way the cleanup can have a consistent base to build upon, and I can > > ensure that the cleanup itself does not cause any functional changes. > > Thanks for doing this. > > Slightly unrelated, but you'll end up calling: > > acpi_video_unregister_backlight > acpi_video_bus_unregister_backlight > acpi_video_dev_unregister_backlight > thermal_cooling_device_unregister > > Uhm, err, what? What business does *that* have in the backlight > unregister call chain?! Should that be untangled from the mess too? I think it should be OK. We currently will create a thermal cooling device for the ACPI backlight device, its intention is that we can achieve some thermal purpose by increasing or decreasing the backlight level(using the same set of methods as the ACPI backlight one, though I'm not sure if it is being used by anyone). When we are going to unregister the ACPI video backlight interface, it most likely means that the interface doesn't work so it doesn't make much sense to keep that thermal interface and hence the thermal_cooling_device_unregister ends up being called here. Regards, Aaron > > BR, > Jani. > > > > > > I hope to post a v1 of the actual cleanup patch-set in 1-2 weeks. > > > > Regards, > > > > Hans > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center
[PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request
Some of the Thinkpads' firmware will issue a backlight change request
through i915 operation region unconditionally on AC plug/unplug, the
backlight level used is arbitrary and thus should be ignored. This is
handled by commit 0b9f7d93ca61 (ACPI / i915: ignore firmware requests
for backlight change). Then there is a Dell laptop whose vendor backlight
interface also makes use of operation region to change backlight level
and with the above commit, that interface no long works. The condition
used to ignore the backlight change request from firmware is thus
changed to: if the vendor backlight interface is not in use and the ACPI
backlight interface is broken, we ignore the requests; oterwise, we keep
processing them.
Reference: https://lkml.org/lkml/2014/9/23/854
Reported-and-tested-by: Pali Roh?r
Cc: # v3.16 and later
Signed-off-by: Aaron Lu
---
drivers/gpu/drm/i915/intel_opregion.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index ca52ad2ae7d1..d8de1d5140a7 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -396,6 +396,16 @@ int intel_opregion_notify_adapter(struct drm_device *dev,
pci_power_t state)
return -EINVAL;
}
+/*
+ * If the vendor backlight interface is not in use and ACPI backlight interface
+ * is broken, do not bother processing backlight change requests from firmware.
+ */
+static bool should_ignore_backlight_request(void)
+{
+ return acpi_video_backlight_support() &&
+ !acpi_video_verify_backlight_support();
+}
+
static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -404,11 +414,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
- /*
-* If the acpi_video interface is not supposed to be used, don't
-* bother processing backlight level change requests from firmware.
-*/
- if (!acpi_video_verify_backlight_support()) {
+ if (should_ignore_backlight_request()) {
DRM_DEBUG_KMS("opregion backlight request ignored\n");
return 0;
}
--
1.9.3
ACPI/i915: Cannot configure display brightness on Dell Latitude E6440
On 09/26/2014 03:58 AM, Rafael J. Wysocki wrote:
> On Thursday, September 25, 2014 11:15:35 AM Aaron Lu wrote:
>> Hi Hans,
>>
>> Thanks for following up and explaining the situation to Pali.
>>
>> On 09/25/2014 02:21 AM, Pali Roh?r wrote:
>>> On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
>>>> Ok, so the dell-laptop interface is just an obsolete wrapper
>>>> around the i915 opregion code, which shows that the right
>>>> interface to use is the i915 one, which we do if you don't
>>>> specify any kernel commandline parameters, case closed.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>
>>> Nope, its not closed.
>>>
>>> Still i915 interface has problem with setting backlight. It
>>> exports lot of levels which turning display off. Which breaking
>>> exiting applications for configuring display brightness. This is
>>> still big regression as black screen is not want people want to
>>> see.
>>>
>>> Driver dell-laptop has exported only few - not thousands level
>>> (which is insane) and only usefull levels (not lot of levels
>>> which turn display off).
>>>
>>> So for this reason using i915 backlight interface is not possible
>>> and also Dell (for E6440) set kernel param acpi_backlight=vendor
>>> to use dell_laptop module for controlling brightness.
>>>
>>> On my laptop E6440 is better for using dell-laptop and not acpi
>>> or i915.
>>
>> Hi Pali,
>>
>> Please test this patch:
>>
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
>> b/drivers/gpu/drm/i915/intel_opregion.c
>> index ca52ad2ae7d1..15534345bd57 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -396,6 +396,24 @@ int intel_opregion_notify_adapter(struct drm_device
>> *dev, pci_power_t state)
>> return -EINVAL;
>> }
>>
>> +/*
>> + * Some of the Thinkpads' firmware will issue a backlight change operation
>> + * region request unconditionally on AC plug/unplug, this is undesirable and
>> + * should be ignored. Then there is a Dell laptop whose vendor backlight
>> + * interface also makes use of operation region request to change backlight
>> + * level and we have to keep it work. The rule used here is: if the vendor
>> + * backlight interface is not in use and the ACPI backlight interface is
>> + * broken, we ignore the requests; oterwise, we keep processing them.
>> + */
>> +static bool should_ignore_backlight_request(void)
>> +{
>> +if (acpi_video_backlight_support() &&
>> +!acpi_video_verify_backlight_support())
>> +return true;
>> +
>> +return false;
>> +}
>
> Well, what about
>
> return acpi_video_backlight_support() &&
> !acpi_video_verify_backlight_support();
>
> ?
Yes that's better.
Will send out a patch with this change, thanks for the suggestion.
-Aaron
>
>> +
>> static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -404,11 +422,7 @@ static u32 asle_set_backlight(struct drm_device *dev,
>> u32 bclp)
>>
>> DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
>>
>> -/*
>> - * If the acpi_video interface is not supposed to be used, don't
>> - * bother processing backlight level change requests from firmware.
>> - */
>> -if (!acpi_video_verify_backlight_support()) {
>> +if (should_ignore_backlight_request()) {
>> DRM_DEBUG_KMS("opregion backlight request ignored\n");
>> return 0;
>> }
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
ACPI/i915: Cannot configure display brightness on Dell Latitude E6440
Hi Hans,
Thanks for following up and explaining the situation to Pali.
On 09/25/2014 02:21 AM, Pali Roh?r wrote:
> On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
>> Ok, so the dell-laptop interface is just an obsolete wrapper
>> around the i915 opregion code, which shows that the right
>> interface to use is the i915 one, which we do if you don't
>> specify any kernel commandline parameters, case closed.
>>
>> Regards,
>>
>> Hans
>
> Nope, its not closed.
>
> Still i915 interface has problem with setting backlight. It
> exports lot of levels which turning display off. Which breaking
> exiting applications for configuring display brightness. This is
> still big regression as black screen is not want people want to
> see.
>
> Driver dell-laptop has exported only few - not thousands level
> (which is insane) and only usefull levels (not lot of levels
> which turn display off).
>
> So for this reason using i915 backlight interface is not possible
> and also Dell (for E6440) set kernel param acpi_backlight=vendor
> to use dell_laptop module for controlling brightness.
>
> On my laptop E6440 is better for using dell-laptop and not acpi
> or i915.
Hi Pali,
Please test this patch:
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index ca52ad2ae7d1..15534345bd57 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -396,6 +396,24 @@ int intel_opregion_notify_adapter(struct drm_device *dev,
pci_power_t state)
return -EINVAL;
}
+/*
+ * Some of the Thinkpads' firmware will issue a backlight change operation
+ * region request unconditionally on AC plug/unplug, this is undesirable and
+ * should be ignored. Then there is a Dell laptop whose vendor backlight
+ * interface also makes use of operation region request to change backlight
+ * level and we have to keep it work. The rule used here is: if the vendor
+ * backlight interface is not in use and the ACPI backlight interface is
+ * broken, we ignore the requests; oterwise, we keep processing them.
+ */
+static bool should_ignore_backlight_request(void)
+{
+ if (acpi_video_backlight_support() &&
+ !acpi_video_verify_backlight_support())
+ return true;
+
+ return false;
+}
+
static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -404,11 +422,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
- /*
-* If the acpi_video interface is not supposed to be used, don't
-* bother processing backlight level change requests from firmware.
-*/
- if (!acpi_video_verify_backlight_support()) {
+ if (should_ignore_backlight_request()) {
DRM_DEBUG_KMS("opregion backlight request ignored\n");
return 0;
}
[PATCH v3] drm/i915: restore backlight precision when converting from opregion
On 05/04/2014 03:22 PM, Chris Wilson wrote: > 32b * 32b = 32b > > n = (u64)level * freq; to avoid overflow as you claim. Updated patch to fix this problem is here, thanks! >From a0f41a92d949c814c203672ff7efe219a90ca6df Mon Sep 17 00:00:00 2001 From: Aaron Lu Date: Mon, 28 Apr 2014 11:02:52 +0800 Subject: [PATCH] drm/i915: restore backlight precision when converting from ACPI When we set backlight on behalf of ACPI opregion, we will convert the backlight value in the 0-255 range defined in opregion to the actual hardware level. Commit 22505b82a2 (drm/i915: avoid brightness overflow when doing scale) is meant to fix the overflow problem when doing the conversion, but it also caused a problem that the converted hardware level doesn't quite represent the intended value: say user wants maximum backlight level(255 in opregion's range), then we will calculate the actual hardware level to be: level = freq / max * level, where freq is the hardware's max backlight level(937 on an user's box), and max and level are all 255. The converted value should be 937 but the above calculation will yield 765. To fix this issue, just use 64 bits to do the calculation to keep the precision and avoid overflow at the same time. Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=72491 Reported-by: Nico Schottelius Signed-off-by: Aaron Lu --- drivers/gpu/drm/i915/intel_panel.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 0eead16aeda7..cb8cfb7e0974 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -492,6 +492,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, enum pipe pipe = intel_get_pipe_from_connector(connector); u32 freq; unsigned long flags; + u64 n; if (!panel->backlight.present || pipe == INVALID_PIPE) return; @@ -502,10 +503,9 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, /* scale to hardware max, but be careful to not overflow */ freq = panel->backlight.max; - if (freq < max) - level = level * freq / max; - else - level = freq / max * level; + n = (u64)level * freq; + do_div(n, max); + level = n; panel->backlight.level = level; if (panel->backlight.device) -- 1.9.0
[Intel-gfx] [PATCH v2] drm/i915: restore backlight precision when converting from opregion
On 05/04/2014 06:41 PM, Chris Wilson wrote: > On Sun, May 04, 2014 at 03:31:01PM +0800, Aaron Lu wrote: >> On 05/04/2014 03:22 PM, Chris Wilson wrote: >>> Also this still has the same rounding error as before. >> >> I didn't get this, care to explain? > > The calculation you use, truncates, rather than say round to nearest, > would is the same discrepancy in your changelog. This patch is meant to fix the problem that when user specify the max backlight level in acpi_video interface, we will get the hardware max level too in i915. With this patch, the hardware level we will get is: level * freq / max = 255 * 937 / 255 = 937, which is correct. For other values, the truncates is OK: 254 * 937 / 255 = 933 253 * 937 / 255 = 929 ... ...
[Intel-gfx] [PATCH v2] drm/i915: restore backlight precision when converting from opregion
On 05/04/2014 03:22 PM, Chris Wilson wrote: > On Sun, May 04, 2014 at 03:16:05PM +0800, Aaron Lu wrote: >> On 04/28/2014 09:41 PM, Daniel Vetter wrote: >>> 64bit divisions won't compile on 32bit. You need one of the DO_DIV macros, >>> or whatever they're called again. I pain, I know ;-) >> >> Thanks for the correction, here is an updated patch :-) >> >> From: Aaron Lu >> Date: Mon, 28 Apr 2014 11:02:52 +0800 >> Subject: [PATCH v2] drm/i915: restore backlight precision when converting >> from >> ACPI >> >> When we set backlight on behalf of ACPI opregion, we will convert the >> backlight value in the 0-255 range defined in opregion to the actual >> hardware level. Commit 22505b82a2 (drm/i915: avoid brightness overflow >> when doing scale) is meant to fix the overflow problem when doing the >> conversion, but it also caused a problem that the converted hardware >> level doesn't quite represent the intended value: say user wants maximum >> backlight level(255 in opregion's range), then we will calculate the >> actual hardware level to be: level = freq / max * level, where freq is >> the hardware's max backlight level(937 on an user's box), and max and >> level are all 255. The converted value should be 937 but the above >> calculation will yield 765. >> >> To fix this issue, just use 64 bits to do the calculation to keep the >> precision and avoid overflow at the same time. >> >> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=72491 >> Reported-by: Nico Schottelius >> Signed-off-by: Aaron Lu >> --- >> v2: use do_div as reminded by Daniel. >> >> drivers/gpu/drm/i915/intel_panel.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c >> b/drivers/gpu/drm/i915/intel_panel.c >> index a953b081ee38..8725917a3d0d 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -492,6 +492,7 @@ void intel_panel_set_backlight(struct intel_connector >> *connector, u32 level, >> enum pipe pipe = intel_get_pipe_from_connector(connector); >> u32 freq; >> unsigned long flags; >> +u64 n; >> >> if (!panel->backlight.present || pipe == INVALID_PIPE) >> return; >> @@ -502,10 +503,9 @@ void intel_panel_set_backlight(struct intel_connector >> *connector, u32 level, >> >> /* scale to hardware max, but be careful to not overflow */ >> freq = panel->backlight.max; >> -if (freq < max) >> -level = level * freq / max; >> -else >> -level = freq / max * level; >> +n = level * freq; > > 32b * 32b = 32b > > n = (u64)level * freq; to avoid overflow as you claim. Ah...yes, my fault, thanks. > > Also this still has the same rounding error as before. I didn't get this, care to explain?
[PATCH v2] drm/i915: restore backlight precision when converting from opregion
On 04/28/2014 09:41 PM, Daniel Vetter wrote: > 64bit divisions won't compile on 32bit. You need one of the DO_DIV macros, > or whatever they're called again. I pain, I know ;-) Thanks for the correction, here is an updated patch :-) From: Aaron Lu Date: Mon, 28 Apr 2014 11:02:52 +0800 Subject: [PATCH v2] drm/i915: restore backlight precision when converting from ACPI When we set backlight on behalf of ACPI opregion, we will convert the backlight value in the 0-255 range defined in opregion to the actual hardware level. Commit 22505b82a2 (drm/i915: avoid brightness overflow when doing scale) is meant to fix the overflow problem when doing the conversion, but it also caused a problem that the converted hardware level doesn't quite represent the intended value: say user wants maximum backlight level(255 in opregion's range), then we will calculate the actual hardware level to be: level = freq / max * level, where freq is the hardware's max backlight level(937 on an user's box), and max and level are all 255. The converted value should be 937 but the above calculation will yield 765. To fix this issue, just use 64 bits to do the calculation to keep the precision and avoid overflow at the same time. Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=72491 Reported-by: Nico Schottelius Signed-off-by: Aaron Lu --- v2: use do_div as reminded by Daniel. drivers/gpu/drm/i915/intel_panel.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a953b081ee38..8725917a3d0d 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -492,6 +492,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, enum pipe pipe = intel_get_pipe_from_connector(connector); u32 freq; unsigned long flags; + u64 n; if (!panel->backlight.present || pipe == INVALID_PIPE) return; @@ -502,10 +503,9 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, /* scale to hardware max, but be careful to not overflow */ freq = panel->backlight.max; - if (freq < max) - level = level * freq / max; - else - level = freq / max * level; + n = level * freq; + do_div(n, max); + level = n; panel->backlight.level = level; if (panel->backlight.device) -- 1.9.0
[PATCH] drm/i915: restore backlight precision when converting from opregion
When we set backlight on behalf of ACPI opregion, we will convert the backlight value in the 0-255 range defined in opregion to the actual hardware level. Commit 22505b82a2 (drm/i915: avoid brightness overflow when doing scale) is meant to fix the overflow problem when doing the conversion, but it also caused a problem that the converted hardware level doesn't quite represent the intended value: say user wants maximum backlight level(255 in opregion's range), then we will calculate the actual hardware level to be: level = freq / max * level, where freq is the hardware's max backlight level(937 on an user's box), and max and level are all 255. The converted value should be 937 but the above calculation will yield 765. To fix this issue, just use 64 bits to do the calculation to keep the precision and avoid overflow at the same time. Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=72491 Reported-and-tested-by: Nico Schottelius Signed-off-by: Aaron Lu --- drivers/gpu/drm/i915/intel_panel.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a953b081ee38..bdd2f24b7a6b 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -502,10 +502,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, /* scale to hardware max, but be careful to not overflow */ freq = panel->backlight.max; - if (freq < max) - level = level * freq / max; - else - level = freq / max * level; + level = (u64)level * freq / max; panel->backlight.level = level; if (panel->backlight.device) -- 1.9.0
[PATCH] [ACPI] Default to using the native backlight control on Windows 8 systems
On 04/11/2014 06:58 PM, Jani Nikula wrote: > On Fri, 11 Apr 2014, Aaron Lu wrote: >> On 04/11/2014 04:13 AM, Matthew Garrett wrote: >>> The list of machines in the "Use native backlight" table is getting longer >>> and longer, which is a solid indication that we're doing something wrong. >>> Disable the ACPI backlight interface if the system claims to be Windows 8 >>> or later. >> >> And has a native backlight control interface. > > This. One thing we were doing wrong in i915 was unconditionally > registering a native backlight interface even when it was not > functional. We can't really know whether that is the case except by > trusting the VBT, and although we've been let down in the past, trusting > the VBT on this one is probably a better approximation of the reality > than assuming it always works. Thus we've queued [1] and [2] for 3.15 > which should help. > > [1] > http://cgit.freedesktop.org/drm-intel/commit/?id=39fbc9c8f6765959b55e0b127dd5c57df5a47d67 > [2] > http://cgit.freedesktop.org/drm-intel/commit/?id=c675949ec58ca50d5a3ae3c757892f1560f6e896 That's pretty cool :-) Regards, Aaron
[PATCH] [ACPI] Default to using the native backlight control on Windows 8 systems
On 04/11/2014 04:13 AM, Matthew Garrett wrote: > The list of machines in the "Use native backlight" table is getting longer > and longer, which is a solid indication that we're doing something wrong. > Disable the ACPI backlight interface if the system claims to be Windows 8 > or later. And has a native backlight control interface. > > Signed-off-by: Matthew Garrett > --- > > Let's at least get this into -next for 3.16 and figure out whether the > drivers actually work now. I agree.
[PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On 01/21/2014 08:11 PM, Matthew Garrett wrote: > On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: >> On 01/21/2014 11:17 AM, Matthew Garrett wrote: >>> We know that Windows 8 graphics drivers don't use the ACPI interface, >>> and that systems change their behaviour as a result, in some cases with >>> absolutely no way for the ACPI interface could possibly work. I haven't >>> seen any cases where that's obviously true for any non-Windows 8 >> >> Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor >> GPU's interface, I just meant for one specific win7 laptop I could re-use >> the existing code to make the GPU's interface as the only one left. And to >> achieve this, the Win8 OSI check in acpi_video_verify_backlight_support >> has to be gone. > > We could do that, but why do we think that's the correct fix? The plan > is to remove the native list entirely and do this for all Windows 8 What do you mean by native list, systems that are Win8 based but don't work with the GPU's backlight interface? If so, it doesn't seem we have such a native list now and if we don't make use_native_backlight=1 by default, we couldn't generate such a list to start with... BTW, it doesn't seem everyone agrees with this plan. May I ask, Rafael and Daniel, what's your opinion please? We need to agree with something to move things forward.
[PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On 01/21/2014 11:17 AM, Matthew Garrett wrote: > On Tue, 2014-01-21 at 10:24 +0800, Aaron Lu wrote: >> On 01/20/2014 09:34 PM, Matthew Garrett wrote: >>> On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: >>> >>>> 1 remove the win8 OSI check, I've seen win7 laptops that also needs to >>>> have only the GPU interface left and checking win8 doesn't make much >>>> sense now; >>> >>> Are we sure that those aren't simply some other bug? >> >> Well, the firmware on that laptop makes use of EC to do backlight >> control and the fact that the firmware interface doesn't work while the >> GPU's work seems to indicate that the backlight control circuit is not >> routed to EC. I think this is the same case as Win8 laptops. > > We know that Windows 8 graphics drivers don't use the ACPI interface, > and that systems change their behaviour as a result, in some cases with > absolutely no way for the ACPI interface could possibly work. I haven't > seen any cases where that's obviously true for any non-Windows 8 Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor GPU's interface, I just meant for one specific win7 laptop I could re-use the existing code to make the GPU's interface as the only one left. And to achieve this, the Win8 OSI check in acpi_video_verify_backlight_support has to be gone. BTW, I actually think use_native_backlight param should mean "the native backlight control interface will be the only one available on the system", it doesn't need to go side by side with Win8 OSI check. > systems. EC interfaces that don't work are often due to Linux leaving > the hardware in a state other than the one expected by the firmware. We Good to know this, thanks. > shouldn't assume that it's the same issue until we've investigated > further. OK, but I honestly don't have any idea how to proceed, in case you have some time, the bug is: https://bugzilla.kernel.org/show_bug.cgi?id=66501
[PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On 01/20/2014 09:34 PM, Matthew Garrett wrote: > On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: > >> 1 remove the win8 OSI check, I've seen win7 laptops that also needs to >> have only the GPU interface left and checking win8 doesn't make much >> sense now; > > Are we sure that those aren't simply some other bug? Well, the firmware on that laptop makes use of EC to do backlight control and the fact that the firmware interface doesn't work while the GPU's work seems to indicate that the backlight control circuit is not routed to EC. I think this is the same case as Win8 laptops.
[PATCH v3] ACPI / video: Add systems that should favor native backlight interface
Hi Igor, On 01/18/2014 09:54 PM, Igor Gnatenko wrote: > From: Aaron Lu > > 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 Thanks for updating this patch. In the meantime, I plan to make some small modifications to this patch in next revision: 1 remove the win8 OSI check, I've seen win7 laptops that also needs to have only the GPU interface left and checking win8 doesn't make much sense now; 2 add more entries for laptops that appeared in bugzilla recently.
I915 did not work whit modset=1
On 01/08/2014 05:09 PM, Jani Nikula wrote: > On Mon, 30 Dec 2013, Jani Nikula wrote: >> On Mon, 23 Dec 2013, der-b wrote: >>> Hello, >>> >>> i got a new Notebook with an Intel Core i7-3689Y whit a Intel HD >>> Graphics 4000. I got a blank screen while booting but I could use ssh. >>> This happened each time, when I enabled CONFIG_I915_KMS in the kernel >>> config (modeset=1). Setting the option "invert_brightness=1" solved the >>> problem. >> >> Hi Bernd - >> >> The i915.invert_brightness parameter has only ever been needed on some >> gen4 Acer laptops; I suspect this is something else. >> >> Please drop that parameter, and try one of the v3.13-rc kernels or >> video.use_bios_initial_backlight=0 module parameter. > > For completeness, the reporter replied privately that > video.use_bios_initial_backlight=0 works. Is it possible for the reporter to test a v3.13-rc kernel? That param is removed in v3.13 after I did some clean up so it would be great to know if v3.13 kernel still works for him. Thanks, Aaron > > Jani. > > >> >> If those don't help, please file a bug on Product=DRI, >> Component=DRM/Intel as instructed in [1]. >> >> BR, >> Jani. >> >> >> [1] https://01.org/linuxgraphics/documentation/how-report-bugs >> >>> >>> I attached the "lspci -s 00:002 -nn -v" output. >>> I use the gentoo kernel 3.10.17. >>> >>> I hope this helps you a little bit. >>> >>> Kind regards, >>> >>> Bernd Lehmann >>> >>> >>> 00:02.0 VGA compatible controller [0300]: Intel Corporation 3rd Gen Core >>> processor Graphics Controller [8086:0166] (rev 09) (prog-if 00 [VGA >>> controller]) >>> Subsystem: Lenovo Device [17aa:3977] >>> Flags: bus master, fast devsel, latency 0, IRQ 42 >>> Memory at e000 (64-bit, non-prefetchable) [size=4M] >>> Memory at d000 (64-bit, prefetchable) [size=256M] >>> I/O ports at 2000 [size=64] >>> Expansion ROM at [disabled] >>> Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- >>> Capabilities: [d0] Power Management version 2 >>> Capabilities: [a4] PCI Advanced Features >>> Kernel driver in use: i915 >>> Kernel modules: i915 >>> >>> ___ >>> dri-devel mailing list >>> dri-devel at lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> Jani Nikula, Intel Open Source Technology Center >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH v2 rebased] ACPI / video: Add systems that should favor native backlight interface
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
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
---
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;
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,46 @@ 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",
+
[PATCH v2] 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. 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&m=138448583006432&w=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_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
@@ -456,6 +471,46 @@ 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",
+.ma
[Intel-gfx] [Acer Aspire V5-573G] Black screen on boot due to low brightness setting
On 11/14/2013 02:24 PM, Jani Nikula wrote: > > Hi Mario - > > On Wed, 13 Nov 2013, Mario Kleinsasser gmail.com> wrote: >> In the last three weeks I've narrowed down the bug described in the >> subject through Ubuntu launchpad. My new Laptop (Acer Aspire V5-573G) >> with new Haswell CPU shows a black screen on reboot because the >> brightness is on zero level. I verified this behaviour with the latest >> mainline kernel from >> http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.12-trusty/ > > Thanks for the report. I'll have to ask you to try some kernels, and > logs running them, etc. that IMHO are better suited attached to a bug > report than cluttered on the mailing list. Please file a bug on > DRM/Intel at [1]. Reference your mail. There it will also pop up in our > bug team's searches, and has a better chance of not falling under the > radar. :) > >> I have done the bisect process and tracked the bug down to the >> following commit. >> >> For explanation: >> good commit means: The laptop starts with display brightness on full >> power, but low(er) resolution and the Fn+ keys are not working >> bad commit means: The laptop starts with black display (0 brightness), >> Fn+ keys are working and you can level up the brightness trough the >> Fn+ keys. The resolution is the native LCD hardware display resolution >> >> Bisect output: >> >> 70b12bb415463c1bd146b67c5fbf6784fd046ad9 is the first bad commit >> commit 70b12bb415463c1bd146b67c5fbf6784fd046ad9 >> Author: Paulo Zanoni >> Date: Tue Nov 20 13:32:30 2012 -0200 >> >> drm/i915: promote Haswell to full support >> >> Since it should be working a little bit better now. >> >> Signed-off-by: Paulo Zanoni >> Signed-off-by: Daniel Vetter >> >> :04 04 f93a7761de2157a8be61f20ca9a5499264bb5c55 >> 1fed098470278830f06b0bca1706f61ba231d38d M drivers >> >> For the full bug report and the additional hardware information please >> see https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1239183 >> >> I've also searched the LKML and maybe this bug could be in conjunction >> with http://marc.info/?t=13783778615&r=1&w=2 > > You only start using our driver for Haswell from the first "bad" commit, > and then we actually serve the ACPI opregion backlight requests. This > explains the backlight Fn keys in your good/bad commit explanations. If > this is a known bug in ACPI, it also explains the zero backlight. It's actually a bug in firmware and we try to work around it in ACPI video module :-) >From the description it indeed seems to be the firmware initial zero brightness level bug to me. Thanks, Aaron > > There's two branches I'd like you to try: > > 1) linux-next branch of [2]. This should contain a fix for some ACPI >initial backlight issues. (CC Aaron for this.) > > 2) backlight-rework branch of [3]. What the branch name says. > > Please add drm.debug=0xe module parameter and attach full dmesg to the > bug. > > > Thanks, > Jani. > > > [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI > [2] http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git > [3] git://gitorious.org/jani/drm.git > > >
[PATCH v5 0/4] Fix Win8 backlight issue
On 10/28/2013 04:09 PM, Jani Nikula wrote:
> On Mon, 28 Oct 2013, Aaron Lu wrote:
>> +static int __init video_set_use_native_backlight(const struct dmi_system_id
>> *d)
>> +{
>> +use_native_backlight = true;
>> +return 0;
>> +}
>
> Hi Aaron, it might be beneficial to make use_native_backlight support
> three values: force true, force false, and use platform default based on
> DMI.
Makes sense. I modified the patch a little bit so that if user has
specified the cmdline option use_native_backlight=0/1, it will always
take effect no matter if the system is in DMI table or not.
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. 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 and Dell Inspiron 7520
are added here.
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 register
backlight interface for it.
Thinkpad T430s:
Reported-by: Theodore Tso
Reported-and-tested-by: Peter Weber
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko
Lenovo Yoga 13:
Reported-by: Lennart Poettering
Reported-and-tested-by: Kevin Smith
Dell Inspiron 7520:
Reported-by: Rinat Ibragimov
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Signed-off-by: Aaron Lu
---
drivers/acpi/video.c | 57 +++-
1 file changed, 52 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 38c3a28..41bd4b4 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_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
@@ -512,6 +527,38 @@ static struct dmi_system_id video_dmi_table[] __initdata =
{
DMI_MATCH(DMI_PRODUCT_NAME, "HP 250 G1 Notebook PC"),
},
},
+ {
+.callback = video_set_use_native_backlight,
+.ident = "ThinkPad T430s",
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = "ThinkPad X230",
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = "Lenovo Yoga 13",
+.matches = {
+DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga
[PATCH v5 0/4] Fix Win8 backlight issue
On 10/25/2013 02:35 PM, Igor Gnatenko wrote:
> Aaron, add this notebook to list. I've CC'ed owner.
> And I've tested this patch on my TP X230 (add as Reported-and-Tested me
> please)
> + {
> + .callback = video_set_use_native_backlight,
> + .ident = "Dell Inspiron 7520",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),
> + },
> + },
Thanks Igor, updated patch follows:
From: Aaron Lu
Subject: [PATCH] ACPI / video: Add systems that should favour native backlight
interface
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 and Dell Inspiron 7520
are added here.
Thinkpad T430s:
Reported-by: Theodore Tso
Reported-and-tested-by: Peter Weber
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko
Lenovo Yoga 13:
Reported-by: Lennart Poettering
Reported-and-tested-by: Kevin Smith
Dell Inspiron 7520:
Reported-by: Renat Ibragimov
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Signed-off-by: Aaron Lu
---
drivers/acpi/video.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index d020df5..0a1b030 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(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 = true;
+ return 0;
+}
+
static struct dmi_system_id video_dmi_table[] __initdata = {
/*
* Broken _BQC workaround
http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -504,6 +510,38 @@ static struct dmi_system_id video_dmi_table[] __initdata =
{
DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"),
},
},
+ {
+.callback = video_set_use_native_backlight,
+.ident = "ThinkPad T430s",
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = "ThinkPad X230",
+.matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = "Lenovo Yoga 13",
+.matches = {
+DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
+ },
+ },
+ {
+.callback = video_set_use_native_backlight,
+.ident = "Dell Inspiron 7520",
+.matches = {
+DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),
+ },
+ },
{}
};
--
1.8.4.39.ga0d3f10
[PATCH v5 0/4] Fix Win8 backlight issue
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. 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 clear, the ThinkPad T430s/X230 and Lenovo Yoga 13 are added here. Reported-by: Theodore Tso Reported-and-tested-by: Peter Weber Reported-by: Lennart Poettering Reported-by: Igor Gnatenko Signed-off-by: Aaron Lu --- drivers/acpi/video.c | 30 ++ 1 file changed, 30 insertions(+) dif
[PATCH v5 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL
The tpacpi_acpi_handle_locate function makes use of acpi_get_devices to
locate handle for ACPI video by HID, the problem is, ACPI video node
doesn't really have HID defined(i.e. no _HID control method is defined
for video device), so.. that function would fail. This can be solved by
enhancing the callback function for acpi_get_devices, where we can use
acpi_device_hid function to check if the ACPI node corresponds to a
video controller.
In addition to that, the _BCL control method only exists under a video
output device node, not a video controller device node. So to evaluate
_BCL, we need the handle of a video output device node, which is child
of the located video controller node from tpacpi_acpi_handle_locate.
The two fix are necessary for some Thinkpad models to emit notification
on backlight hotkey press as a result of evaluation of _BCL.
Signed-off-by: Aaron Lu
Tested-by: Igor Gnatenko
Acked-by: Henrique de Moraes Holschuh
---
drivers/platform/x86/thinkpad_acpi.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 03ca6c1..170f278 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -700,6 +700,14 @@ static void __init drv_acpi_handle_init(const char *name,
static acpi_status __init tpacpi_acpi_handle_locate_callback(acpi_handle
handle,
u32 level, void *context, void **return_value)
{
+ struct acpi_device *dev;
+ if (!strcmp(context, "video")) {
+ if (acpi_bus_get_device(handle, &dev))
+ return AE_OK;
+ if (strcmp(ACPI_VIDEO_HID, acpi_device_hid(dev)))
+ return AE_OK;
+ }
+
*(acpi_handle *)return_value = handle;
return AE_CTRL_TERMINATE;
@@ -712,10 +720,10 @@ static void __init tpacpi_acpi_handle_locate(const char
*name,
acpi_status status;
acpi_handle device_found;
- BUG_ON(!name || !hid || !handle);
+ BUG_ON(!name || !handle);
vdbg_printk(TPACPI_DBG_INIT,
"trying to locate ACPI handle for %s, using HID %s\n",
- name, hid);
+ name, hid ? hid : "NULL");
memset(&device_found, 0, sizeof(device_found));
status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
@@ -6090,19 +6098,28 @@ static int __init tpacpi_query_bcl_levels(acpi_handle
handle)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
+ struct acpi_device *device, *child;
int rc;
- if (ACPI_SUCCESS(acpi_evaluate_object(handle, "_BCL", NULL, &buffer))) {
+ if (acpi_bus_get_device(handle, &device))
+ return 0;
+
+ rc = 0;
+ list_for_each_entry(child, &device->children, node) {
+ acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
+ NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ continue;
+
obj = (union acpi_object *)buffer.pointer;
if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
pr_err("Unknown _BCL data, please report this to %s\n",
- TPACPI_MAIL);
+ TPACPI_MAIL);
rc = 0;
} else {
rc = obj->package.count;
}
- } else {
- return 0;
+ break;
}
kfree(buffer.pointer);
@@ -6118,7 +6135,7 @@ static unsigned int __init
tpacpi_check_std_acpi_brightness_support(void)
acpi_handle video_device;
int bcl_levels = 0;
- tpacpi_acpi_handle_locate("video", ACPI_VIDEO_HID, &video_device);
+ tpacpi_acpi_handle_locate("video", NULL, &video_device);
if (video_device)
bcl_levels = tpacpi_query_bcl_levels(video_device);
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
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 does not need to register its own.
Since there are systems that don't work well with this approach, a
parameter for video module named use_native_backlight is introduced and
has the value of false by default. For users who have a broken ACPI
video backlight interface, video.use_native_backlight=1 is needed in
kernel cmdline.
Signed-off-by: Aaron Lu
---
drivers/acpi/internal.h | 4 +---
drivers/acpi/video.c| 25 -
drivers/acpi/video_detect.c | 4 ++--
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..e9304dc 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -169,9 +169,7 @@ int acpi_create_platform_device(struct acpi_device *adev,
Video
-- */
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-bool acpi_video_backlight_quirks(void);
-#else
-static inline bool acpi_video_backlight_quirks(void) { return false; }
+bool acpi_osi_is_win8(void);
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 3bd1eaa..d020df5 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -88,6 +88,13 @@ module_param(allow_duplicates, bool, 0644);
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.
+ */
+static bool use_native_backlight = false;
+module_param(use_native_backlight, bool, 0644);
+
static int register_count;
static struct mutex video_list_lock;
static struct list_head video_bus_head;
@@ -232,6 +239,14 @@ 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_verify_backlight_support(void)
+{
+ if (acpi_osi_is_win8() && use_native_backlight &&
+ backlight_device_registered(BACKLIGHT_RAW))
+ return false;
+ return acpi_video_backlight_support();
+}
+
/* backlight device sysfs support */
static int acpi_video_get_brightness(struct backlight_device *bd)
{
@@ -1256,8 +1271,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device->brightness)
@@ -1386,13 +1401,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 1 : 0);
+ acpi_osi_is_win8() ? 1 : 0);
}
static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 0 : 1);
+ acpi_osi_is_win8() ? 0 : 1);
}
static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
@@ -1558,7 +1573,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void
*context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 940edbf..b639934 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -233,11 +233,11 @@ static void acpi_video_caps_check(void)
acpi_video_get_capabilities(NULL);
}
-bool acpi_video_backlight_quirks(void)
+bool acpi_osi_is_win8(void)
{
return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
}
-E
[PATCH v5 2/4] ACPI / video: seperate backlight control and event interface
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
---
drivers/acpi/video.c | 434 +--
1 file changed, 245 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..3bd1eaa 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC"))
device->cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device->dev->handle, &acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = &pdev->dev;
- pci_dev_put(pdev);
- }
-
- memset(&props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device->brightness->count - 3;
- device->backlight = backlight_device_register(name,
- parent,
- device,
-
&acpi_backlight_ops,
- &props);
- kfree(name);
- if (IS_ERR(device->backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device->backlight->props.brightness =
- acpi_video_get_brightness(device->backlight);
-
- device->cooling_dev = thermal_cooling_device_register("LCD",
- device->dev, &video_cooling_ops);
- if (IS_ERR(device->cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device->cooling_dev = NULL;
- return;
- }
-
- dev_info(&device->dev->dev, "registered as cooling_device%d\n",
-device->cooling_dev->id);
- result = sysfs_create_link(&device->dev->dev.kobj,
- &device->cooling_dev->device.kobj,
- "thermal_cooling");
-
[PATCH v5 1/4] backlight: introduce backlight_device_registered
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
---
drivers/video/backlight/backlight.c | 31 +++
include/linux/backlight.h | 4
2 files changed, 35 insertions(+)
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 94a403a..5d0 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
#include
#endif
+static struct list_head backlight_dev_list;
+static struct mutex backlight_dev_list_mutex;
+
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
[BACKLIGHT_PLATFORM] = "platform",
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const
char *name,
mutex_unlock(&pmac_backlight_mutex);
#endif
+ mutex_lock(&backlight_dev_list_mutex);
+ list_add(&new_bd->entry, &backlight_dev_list);
+ mutex_unlock(&backlight_dev_list_mutex);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
+bool backlight_device_registered(enum backlight_type type)
+{
+ bool found = false;
+ struct backlight_device *bd;
+
+ mutex_lock(&backlight_dev_list_mutex);
+ list_for_each_entry(bd, &backlight_dev_list, entry) {
+ if (bd->props.type == type) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&backlight_dev_list_mutex);
+
+ return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
/**
* backlight_device_unregister - unregisters a backlight device object.
* @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device
*bd)
if (!bd)
return;
+ mutex_lock(&backlight_dev_list_mutex);
+ list_del(&bd->entry);
+ mutex_unlock(&backlight_dev_list_mutex);
+
#ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(&pmac_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
backlight_class->dev_groups = bl_device_groups;
backlight_class->pm = &backlight_class_dev_pm_ops;
+ INIT_LIST_HEAD(&backlight_dev_list);
+ mutex_init(&backlight_dev_list_mutex);
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
+ /* list entry of all registered backlight devices */
+ struct list_head entry;
+
struct device dev;
};
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device
*dev,
struct backlight_device *bd);
extern void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
#define to_backlight_device(obj) container_of(obj, struct backlight_device,
dev)
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/4] Fix Win8 backlight issue
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(-) -- 1.8.4.12.g2ea3df6 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
On 10/10/2013 08:59 PM, Rafael J. Wysocki wrote:
> On Thursday, October 10, 2013 09:02:55 AM Aaron Lu wrote:
>> On 10/10/2013 08:29 AM, Rafael J. Wysocki wrote:
>>> On Tuesday, October 08, 2013 02:40:00 PM 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.
>>>>
>>>> Signed-off-by: Aaron Lu
>>>> Tested-by: Igor Gnatenko
>>>> Tested-by: Yves-Alexis Perez
>>>> Tested-by: Mika Westerberg
>>>> ---
>>>> drivers/acpi/internal.h | 5 ++---
>>>> drivers/acpi/video.c| 10 +-
>>>> drivers/acpi/video_detect.c | 14 --
>>>> 3 files changed, 19 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>>>> index 20f4233..453ae8d 100644
>>>> --- a/drivers/acpi/internal.h
>>>> +++ b/drivers/acpi/internal.h
>>>> @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device
>>>> *adev,
>>>>Video
>>>>
>>>> --
>>>> */
>>>> #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
>>>> -bool acpi_video_backlight_quirks(void);
>>>> -#else
>>>> -static inline bool acpi_video_backlight_quirks(void) { return false; }
>>>> +bool acpi_osi_is_win8(void);
>>>> +bool acpi_video_verify_backlight_support(void);
>>>> #endif
>>>>
>>>> #endif /* _ACPI_INTERNAL_H_ */
>>>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>>>> index 3bd1eaa..343db59 100644
>>>> --- a/drivers/acpi/video.c
>>>> +++ b/drivers/acpi/video.c
>>>> @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct
>>>> acpi_video_device *device, int event)
>>>>unsigned long long level_current, level_next;
>>>>int result = -EINVAL;
>>>>
>>>> - /* no warning message if acpi_backlight=vendor is used */
>>>> - if (!acpi_video_backlight_support())
>>>> + /* no warning message if acpi_backlight=vendor or a quirk is used */
>>>> + if (!acpi_video_verify_backlight_support())
>>>>return 0;
>>>>
>>>>if (!device->brightness)
>>>> @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus
>>>> *video,
>>>> static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>>>> {
>>>>return acpi_video_bus_DOS(video, 0,
>>>> -acpi_video_backlight_quirks() ? 1 : 0);
>>>> +acpi_osi_is_win8() ? 1 : 0);
>>>> }
>>>>
>>>> static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>>>> {
>>>>return acpi_video_bus_DOS(video, 0,
>>>> -acpi_video_backlight_quirks() ? 0 : 1);
>>>> +acpi_osi_is_win8() ? 0 : 1);
>>>> }
>>>>
>>>> static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>>>> @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level,
>>>> void *context,
>>>>
>>>> static void acpi_video_dev_register_backlight(struct acpi_video_device
>>>> *device)
>>>> {
>>>> - if (acpi_video_backlight_support()) {
>>>> + if (acpi_video_verify_backlight_support()) {
>>>>struct backlight_properties props;
>>>>struct pci_dev *pdev;
>>>>
Re: [PATCH v4 1/4] backlight: introduce backlight_device_registered
On 10/10/2013 12:29 PM, Jani Nikula wrote:
> On Thu, 10 Oct 2013, Aaron Lu wrote:
>> On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
>>> On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
>>>> +bool backlight_device_registered(enum backlight_type type)
>>>> +{
>>>> + bool found = false;
>>>> + struct backlight_device *bd;
>>>> +
>>>> + mutex_lock(&bd_list_mutex);
>>>> + list_for_each_entry(bd, &bd_list_head, entry) {
>>>> + if (bd->props.type == type) {
>>>> + found = true;
>>>> + break;
>>>> + }
>>>> + }
>>>
>>> Isn't it useful to be able to register more than one backlight device of the
>>> same type sometimes?
>>
>> I think so for some kind of computers. OTOH, the above function should
>> be enough for the problem we are solving here, if someday we need to
>> differentiate, we can enhance the code then.
>
> Since both Baytrail and Haswell already have two backlight PWMs, this
> may be needed sooner than you think. But we shouldn't let that block
Do we need to differentiate which backlight PWM is registered to decide
if ACPI video backlight interface should be skipped? My understanding is
no.
Thanks,
Aaron
> fixing the more urgent issue we have now. So I'm fine with this. It
> doesn't prevent one from registering more than one device of the same
> type anyway.
>
> BR,
> Jani.
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
On 10/10/2013 08:29 AM, Rafael J. Wysocki wrote:
> On Tuesday, October 08, 2013 02:40:00 PM 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.
>>
>> Signed-off-by: Aaron Lu
>> Tested-by: Igor Gnatenko
>> Tested-by: Yves-Alexis Perez
>> Tested-by: Mika Westerberg
>> ---
>> drivers/acpi/internal.h | 5 ++---
>> drivers/acpi/video.c| 10 +-
>> drivers/acpi/video_detect.c | 14 --
>> 3 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index 20f4233..453ae8d 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
>> Video
>>
>> -- */
>> #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
>> -bool acpi_video_backlight_quirks(void);
>> -#else
>> -static inline bool acpi_video_backlight_quirks(void) { return false; }
>> +bool acpi_osi_is_win8(void);
>> +bool acpi_video_verify_backlight_support(void);
>> #endif
>>
>> #endif /* _ACPI_INTERNAL_H_ */
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index 3bd1eaa..343db59 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
>> *device, int event)
>> unsigned long long level_current, level_next;
>> int result = -EINVAL;
>>
>> -/* no warning message if acpi_backlight=vendor is used */
>> -if (!acpi_video_backlight_support())
>> +/* no warning message if acpi_backlight=vendor or a quirk is used */
>> +if (!acpi_video_verify_backlight_support())
>> return 0;
>>
>> if (!device->brightness)
>> @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus
>> *video,
>> static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>> {
>> return acpi_video_bus_DOS(video, 0,
>> - acpi_video_backlight_quirks() ? 1 : 0);
>> + acpi_osi_is_win8() ? 1 : 0);
>> }
>>
>> static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>> {
>> return acpi_video_bus_DOS(video, 0,
>> - acpi_video_backlight_quirks() ? 0 : 1);
>> + acpi_osi_is_win8() ? 0 : 1);
>> }
>>
>> static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>> @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level,
>> void *context,
>>
>> static void acpi_video_dev_register_backlight(struct acpi_video_device
>> *device)
>> {
>> -if (acpi_video_backlight_support()) {
>> +if (acpi_video_verify_backlight_support()) {
>> struct backlight_properties props;
>> struct pci_dev *pdev;
>> acpi_handle acpi_parent;
>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>> index 940edbf..23d7d26 100644
>> --- a/drivers/acpi/video_detect.c
>> +++ b/drivers/acpi/video_detect.c
>> @@ -37,6 +37,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "internal.h"
>>
>> @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
>> acpi_video_get_capabilities(NULL);
>> }
>>
>> -bool acpi_video_backlight_quirks(void)
>> +bool acpi_osi_is_win8(void)
>> {
>> return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
>> }
>> -EXPORT_SYMBOL(acpi_video_backlight_quirks);
>> +EXPORT_SYM
Re: [PATCH v4 1/4] backlight: introduce backlight_device_registered
On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
> On Tuesday, October 08, 2013 02:39:58 PM 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
>> Tested-by: Igor Gnatenko
>> Tested-by: Yves-Alexis Perez
>> Tested-by: Mika Westerberg
>> ---
>> drivers/video/backlight/backlight.c | 31 +++
>> include/linux/backlight.h | 4
>> 2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c
>> b/drivers/video/backlight/backlight.c
>> index 94a403a..bf2d71d 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -21,6 +21,9 @@
>> #include
>> #endif
>>
>> +static struct list_head bd_list_head;
>> +static struct mutex bd_list_mutex;
>
> I'd prefer these two things to be called backlight_dev_list and
> backlight_dev_list_mutex, respectively.
OK.
>
>> +
>> static const char *const backlight_types[] = {
>> [BACKLIGHT_RAW] = "raw",
>> [BACKLIGHT_PLATFORM] = "platform",
>> @@ -349,10 +352,32 @@ struct backlight_device
>> *backlight_device_register(const char *name,
>> mutex_unlock(&pmac_backlight_mutex);
>> #endif
>>
>> +mutex_lock(&bd_list_mutex);
>> +list_add(&new_bd->entry, &bd_list_head);
>> +mutex_unlock(&bd_list_mutex);
>> +
>> return new_bd;
>> }
>> EXPORT_SYMBOL(backlight_device_register);
>>
>> +bool backlight_device_registered(enum backlight_type type)
>> +{
>> +bool found = false;
>> +struct backlight_device *bd;
>> +
>> +mutex_lock(&bd_list_mutex);
>> +list_for_each_entry(bd, &bd_list_head, entry) {
>> +if (bd->props.type == type) {
>> +found = true;
>> +break;
>> +}
>> +}
>
> Isn't it useful to be able to register more than one backlight device of the
> same type sometimes?
I think so for some kind of computers. OTOH, the above function should
be enough for the problem we are solving here, if someday we need to
differentiate, we can enhance the code then.
>
>> +mutex_unlock(&bd_list_mutex);
>> +
>> +return found;
>> +}
>> +EXPORT_SYMBOL(backlight_device_registered);
>> +
>> /**
>> * backlight_device_unregister - unregisters a backlight device object.
>> * @bd: the backlight device object to be unregistered and freed.
>> @@ -364,6 +389,10 @@ void backlight_device_unregister(struct
>> backlight_device *bd)
>> if (!bd)
>> return;
>>
>> +mutex_lock(&bd_list_mutex);
>> +list_del(&bd->entry);
>> +mutex_unlock(&bd_list_mutex);
>> +
>> #ifdef CONFIG_PMAC_BACKLIGHT
>> mutex_lock(&pmac_backlight_mutex);
>> if (pmac_backlight == bd)
>> @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
>>
>> backlight_class->dev_groups = bl_device_groups;
>> backlight_class->pm = &backlight_class_dev_pm_ops;
>> +INIT_LIST_HEAD(&bd_list_head);
>> +mutex_init(&bd_list_mutex);
>> return 0;
>> }
>>
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 53b7794..5f9cd96 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -100,6 +100,9 @@ struct backlight_device {
>> /* The framebuffer notifier block */
>> struct notifier_block fb_notif;
>>
>> +/* list entry of all registered backlight devices */
>> +struct list_head entry;
>> +
>> struct device dev;
>> };
>>
>> @@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct
>> device *dev,
>> struct backlight_device *bd);
>> extern void backlight_force_update(struct backlight_device *bd,
>> enum backlight_update_reason reason);
>> +extern bool backlight_device_registered(enum backlight_type type);
>>
>> #define to_backlight_device(obj) container_of(obj, struct backlight_device,
>> dev)
>
> Thanks!
Thanks for the review,
-Aaron
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL
The tpacpi_acpi_handle_locate function makes use of acpi_get_devices to
locate handle for ACPI video by HID, the problem is, ACPI video node
doesn't really have HID defined(i.e. no _HID control method is defined
for video device), so.. that function would fail. This can be solved by
enhancing the callback function for acpi_get_devices, where we can use
acpi_device_hid function to check if the ACPI node corresponds to a
video controller.
In addition to that, the _BCL control method only exists under a video
output device node, not a video controller device node. So to evaluate
_BCL, we need the handle of a video output device node, which is child
of the located video controller node from tpacpi_acpi_handle_locate.
The two fix are necessary for some Thinkpad models to emit notification
on backlight hotkey press as a result of evaluation of _BCL.
Signed-off-by: Aaron Lu
Tested-by: Igor Gnatenko
Acked-by: Henrique de Moraes Holschuh
---
drivers/platform/x86/thinkpad_acpi.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 03ca6c1..170f278 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -700,6 +700,14 @@ static void __init drv_acpi_handle_init(const char *name,
static acpi_status __init tpacpi_acpi_handle_locate_callback(acpi_handle
handle,
u32 level, void *context, void **return_value)
{
+ struct acpi_device *dev;
+ if (!strcmp(context, "video")) {
+ if (acpi_bus_get_device(handle, &dev))
+ return AE_OK;
+ if (strcmp(ACPI_VIDEO_HID, acpi_device_hid(dev)))
+ return AE_OK;
+ }
+
*(acpi_handle *)return_value = handle;
return AE_CTRL_TERMINATE;
@@ -712,10 +720,10 @@ static void __init tpacpi_acpi_handle_locate(const char
*name,
acpi_status status;
acpi_handle device_found;
- BUG_ON(!name || !hid || !handle);
+ BUG_ON(!name || !handle);
vdbg_printk(TPACPI_DBG_INIT,
"trying to locate ACPI handle for %s, using HID %s\n",
- name, hid);
+ name, hid ? hid : "NULL");
memset(&device_found, 0, sizeof(device_found));
status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
@@ -6090,19 +6098,28 @@ static int __init tpacpi_query_bcl_levels(acpi_handle
handle)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
+ struct acpi_device *device, *child;
int rc;
- if (ACPI_SUCCESS(acpi_evaluate_object(handle, "_BCL", NULL, &buffer))) {
+ if (acpi_bus_get_device(handle, &device))
+ return 0;
+
+ rc = 0;
+ list_for_each_entry(child, &device->children, node) {
+ acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
+ NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ continue;
+
obj = (union acpi_object *)buffer.pointer;
if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
pr_err("Unknown _BCL data, please report this to %s\n",
- TPACPI_MAIL);
+ TPACPI_MAIL);
rc = 0;
} else {
rc = obj->package.count;
}
- } else {
- return 0;
+ break;
}
kfree(buffer.pointer);
@@ -6118,7 +6135,7 @@ static unsigned int __init
tpacpi_check_std_acpi_brightness_support(void)
acpi_handle video_device;
int bcl_levels = 0;
- tpacpi_acpi_handle_locate("video", ACPI_VIDEO_HID, &video_device);
+ tpacpi_acpi_handle_locate("video", NULL, &video_device);
if (video_device)
bcl_levels = tpacpi_query_bcl_levels(video_device);
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
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.
Signed-off-by: Aaron Lu
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
Tested-by: Mika Westerberg
---
drivers/acpi/internal.h | 5 ++---
drivers/acpi/video.c| 10 +-
drivers/acpi/video_detect.c | 14 --
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..453ae8d 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
Video
-- */
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-bool acpi_video_backlight_quirks(void);
-#else
-static inline bool acpi_video_backlight_quirks(void) { return false; }
+bool acpi_osi_is_win8(void);
+bool acpi_video_verify_backlight_support(void);
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 3bd1eaa..343db59 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device->brightness)
@@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 1 : 0);
+ acpi_osi_is_win8() ? 1 : 0);
}
static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 0 : 1);
+ acpi_osi_is_win8() ? 0 : 1);
}
static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
@@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void
*context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 940edbf..23d7d26 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
acpi_video_get_capabilities(NULL);
}
-bool acpi_video_backlight_quirks(void)
+bool acpi_osi_is_win8(void)
{
return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
}
-EXPORT_SYMBOL(acpi_video_backlight_quirks);
+EXPORT_SYMBOL(acpi_osi_is_win8);
/* Promote the vendor interface instead of the generic video module.
* This function allow DMI blacklists to be implemented by externals
@@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
}
EXPORT_SYMBOL(acpi_video_backlight_support);
+bool acpi_video_verify_backlight_support(void)
+{
+ if (!(acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) &&
+ acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
+ return false;
+ return 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.
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/4] ACPI / video: seperate backlight control and event interface
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
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
Tested-by: Mika Westerberg
---
drivers/acpi/video.c | 434 +--
1 file changed, 245 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..3bd1eaa 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC"))
device->cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device->dev->handle, &acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = &pdev->dev;
- pci_dev_put(pdev);
- }
-
- memset(&props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device->brightness->count - 3;
- device->backlight = backlight_device_register(name,
- parent,
- device,
-
&acpi_backlight_ops,
- &props);
- kfree(name);
- if (IS_ERR(device->backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device->backlight->props.brightness =
- acpi_video_get_brightness(device->backlight);
-
- device->cooling_dev = thermal_cooling_device_register("LCD",
- device->dev, &video_cooling_ops);
- if (IS_ERR(device->cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device->cooling_dev = NULL;
- return;
- }
-
- dev_info(&device->dev->dev, "registered as cooling_device%d\n",
-device->cooling_dev->id);
- result = sysfs_create_link(&device->dev->dev.kobj,
- &device->cooling_dev->devi
[PATCH v4 1/4] backlight: introduce backlight_device_registered
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
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
Tested-by: Mika Westerberg
---
drivers/video/backlight/backlight.c | 31 +++
include/linux/backlight.h | 4
2 files changed, 35 insertions(+)
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 94a403a..bf2d71d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
#include
#endif
+static struct list_head bd_list_head;
+static struct mutex bd_list_mutex;
+
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
[BACKLIGHT_PLATFORM] = "platform",
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const
char *name,
mutex_unlock(&pmac_backlight_mutex);
#endif
+ mutex_lock(&bd_list_mutex);
+ list_add(&new_bd->entry, &bd_list_head);
+ mutex_unlock(&bd_list_mutex);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
+bool backlight_device_registered(enum backlight_type type)
+{
+ bool found = false;
+ struct backlight_device *bd;
+
+ mutex_lock(&bd_list_mutex);
+ list_for_each_entry(bd, &bd_list_head, entry) {
+ if (bd->props.type == type) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&bd_list_mutex);
+
+ return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
/**
* backlight_device_unregister - unregisters a backlight device object.
* @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device
*bd)
if (!bd)
return;
+ mutex_lock(&bd_list_mutex);
+ list_del(&bd->entry);
+ mutex_unlock(&bd_list_mutex);
+
#ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(&pmac_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
backlight_class->dev_groups = bl_device_groups;
backlight_class->pm = &backlight_class_dev_pm_ops;
+ INIT_LIST_HEAD(&bd_list_head);
+ mutex_init(&bd_list_mutex);
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
+ /* list entry of all registered backlight devices */
+ struct list_head entry;
+
struct device dev;
};
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device
*dev,
struct backlight_device *bd);
extern void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
#define to_backlight_device(obj) container_of(obj, struct backlight_device,
dev)
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/4] Fix Win8 backlight issue
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 | 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/linux/backlight.h| 4 + 6 files changed, 322 insertions(+), 205 deletions(-) -- 1.8.4.12.g2ea3df6 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
On Wed, Sep 25, 2013 at 07:53:13PM +0200, Rafael J. Wysocki wrote: > On Tuesday, September 24, 2013 05:47:31 PM 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. > > I think the idea is to use the aggressive default for now and we can switch > the > default back to the current behavior before the merge window in case there are > too many problems with it? Yes I think so. Thanks, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL
On Wed, Sep 25, 2013 at 04:58:39PM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 24 Sep 2013, Aaron Lu wrote: > > locate handle for ACPI video by HID, the problem is, ACPI video node > > doesn't really have HID defined(i.e. no _HID control method is defined > > ACPI video is supposed to attach a virtual HID node (ACPI_VIDEO_HID) to ACPI > video devices so as to keep the non-trivial video device detection logic in > just one place instead of reinventing the wheel in every driver (which is > always a recipe for disaster). > > When did that break? I checked the git log for the commit to use tpacpi_acpi_handle_locate to locate video controller's ACPI handle, it's: commit 122f26726b5e16174bf8a707df14be1d93c49d62 Author: Henrique de Moraes Holschuh Date: Mon Aug 9 23:48:18 2010 -0300 thinkpad-acpi: find ACPI video device by synthetic HID So I checked out that commit and found that it shouldn't work either, since it has the same problem of the current code. The ACPI video controller device is given an id of ACPI_VIDEO_HID, but it's only known to Linux ACPI, not ACPICA, so the function provided by ACPICA acpi_get_devices will not work in this case, as that function will really check the control method of _HID under the handle, which does not exist no matter if Linux ACPI has added an id to its device structure or not. OTOH, the function provided by Linux ACPI acpi_device_hid will see the added id. In a word, the add of the HID will not affect the ASL namespace layout of the device node(thus, no _HID control method will be added to the device node). It seems that this problem has been found previously by: commit ff413195e830541afeae469fc866ecd0319abd7e Author: Alex Hung Date: Tue Apr 24 16:40:52 2012 +0800 thinkpad-acpi: fix issuing duplicated key events for brightness up/down The tp_features.bright_acpimode will not be set correctly for brightness control because ACPI_VIDEO_HID will not be located in ACPI. As a result, a duplicated key event will always be sent. acpi_video_backlight_support() is sufficient to detect standard ACPI brightness control. Signed-off-by: Alex Hung Signed-off-by: Matthew Garrett Thanks, Aaron ___ 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 Wed, Sep 25, 2013 at 10:29:37AM +0200, Jörg Otte wrote: > Backlight can't be modified with this patch set - neither with function > keys nor > with the gui. It is a step backward to v3.11-rc1 Thanks for the test. Please check /sys/class/backlight, is there only one link file intel_backlight? If so, please cd inside and try modify the brightness file using echo with some values in the range of 0 - max_brightness, does the backlight level change when you echo a new value? I guess it doesn't, or you wouldn't notice problem. If indeed so, perhaps file a bug at http://bugzilla.kernel.org, Drivers/Video(DRI-Intel)? I suppose Jani and Daniel can help fix your problem. Thanks, Aaron > > Video driver: i915 > FUJITSU LIFEBOOK AH532/FJNBB1C, BIOS Version 1.09 05/22/2012 > > acpi_backlight=video works. > > Jörg > > > 2013/9/24 Igor Gnatenko > > > 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 v3 2/4] ACPI / video: seperate backlight control and event interface
On 09/24/2013 05:47 PM, 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
> Tested-by: Igor Gnatenko
> Tested-by: Yves-Alexis Perez
> ---
> drivers/acpi/video.c | 434
> +--
> include/acpi/video.h | 2 +
> 2 files changed, 247 insertions(+), 189 deletions(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index aebcf63..3bd1eaa 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
> module_param(use_bios_initial_backlight, bool, 0644);
>
> static int register_count;
> +static struct mutex video_list_lock;
> +static struct list_head video_bus_head;
> static int acpi_video_bus_add(struct acpi_device *device);
> static int acpi_video_bus_remove(struct acpi_device *device);
> static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
> @@ -157,6 +159,7 @@ struct acpi_video_bus {
> struct acpi_video_bus_flags flags;
> struct list_head video_device_list;
> struct mutex device_list_lock; /* protects video_device_list */
> + struct list_head entry;
> struct input_dev *input;
> char phys[32]; /* for input device */
> struct notifier_block pm_nb;
> @@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
> acpi_video_device *device)
>
> if (acpi_has_method(device->dev->handle, "_DDC"))
> device->cap._DDC = 1;
> -
> - if (acpi_video_backlight_support()) {
> - struct backlight_properties props;
> - struct pci_dev *pdev;
> - acpi_handle acpi_parent;
> - struct device *parent = NULL;
> - int result;
> - static int count;
> - char *name;
> -
> - result = acpi_video_init_brightness(device);
> - if (result)
> - return;
> - name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> - if (!name)
> - return;
> - count++;
> -
> - acpi_get_parent(device->dev->handle, &acpi_parent);
> -
> - pdev = acpi_get_pci_dev(acpi_parent);
> - if (pdev) {
> - parent = &pdev->dev;
> - pci_dev_put(pdev);
> - }
> -
> - memset(&props, 0, sizeof(struct backlight_properties));
> - props.type = BACKLIGHT_FIRMWARE;
> - props.max_brightness = device->brightness->count - 3;
> - device->backlight = backlight_device_register(name,
> - parent,
> - device,
> -
> &acpi_backlight_ops,
> - &props);
> - kfree(name);
> - if (IS_ERR(device->backlight))
> - return;
> -
> - /*
> - * Save current brightness level in case we have to restore it
> - * before acpi_video_device_lcd_set_level() is called next time.
> - */
> - device->backlight->props.brightness =
> - acpi_video_get_brightness(device->backlight);
> -
> - device->cooling_dev = thermal_cooling_device_register("LCD",
> - device->dev, &video_cooling_ops);
> - if (IS_ERR(device->cooling_dev)) {
> - /*
> - * Set cooling_dev to NULL so we don't crash trying to
> - * free it.
> - * Also, why the hell we are returning early and
> - * not attempt to register video output if cooling
> - * device registration failed?
> -
[PATCH v3 4/4] thinkpad-acpi: fix handle locate for video and query of _BCL
The tpacpi_acpi_handle_locate function makes use of acpi_get_devices to
locate handle for ACPI video by HID, the problem is, ACPI video node
doesn't really have HID defined(i.e. no _HID control method is defined
for video device), so.. that function would fail. This can be solved by
enhancing the callback function for acpi_get_devices, where we can use
acpi_device_hid function to check if the ACPI node corresponds to a
video controller.
In addition to that, the _BCL control method only exists under a video
output device node, not a video controller device node. So to evaluate
_BCL, we need the handle of a video output device node, which is child
of the located video controller node from tpacpi_acpi_handle_locate.
The two fix are necessary for some Thinkpad models to emit notification
on backlight hotkey press as a result of evaluation of _BCL.
Signed-off-by: Aaron Lu
Tested-by: Igor Gnatenko
---
drivers/platform/x86/thinkpad_acpi.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 03ca6c1..170f278 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -700,6 +700,14 @@ static void __init drv_acpi_handle_init(const char *name,
static acpi_status __init tpacpi_acpi_handle_locate_callback(acpi_handle
handle,
u32 level, void *context, void **return_value)
{
+ struct acpi_device *dev;
+ if (!strcmp(context, "video")) {
+ if (acpi_bus_get_device(handle, &dev))
+ return AE_OK;
+ if (strcmp(ACPI_VIDEO_HID, acpi_device_hid(dev)))
+ return AE_OK;
+ }
+
*(acpi_handle *)return_value = handle;
return AE_CTRL_TERMINATE;
@@ -712,10 +720,10 @@ static void __init tpacpi_acpi_handle_locate(const char
*name,
acpi_status status;
acpi_handle device_found;
- BUG_ON(!name || !hid || !handle);
+ BUG_ON(!name || !handle);
vdbg_printk(TPACPI_DBG_INIT,
"trying to locate ACPI handle for %s, using HID %s\n",
- name, hid);
+ name, hid ? hid : "NULL");
memset(&device_found, 0, sizeof(device_found));
status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
@@ -6090,19 +6098,28 @@ static int __init tpacpi_query_bcl_levels(acpi_handle
handle)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
+ struct acpi_device *device, *child;
int rc;
- if (ACPI_SUCCESS(acpi_evaluate_object(handle, "_BCL", NULL, &buffer))) {
+ if (acpi_bus_get_device(handle, &device))
+ return 0;
+
+ rc = 0;
+ list_for_each_entry(child, &device->children, node) {
+ acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
+ NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ continue;
+
obj = (union acpi_object *)buffer.pointer;
if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
pr_err("Unknown _BCL data, please report this to %s\n",
- TPACPI_MAIL);
+ TPACPI_MAIL);
rc = 0;
} else {
rc = obj->package.count;
}
- } else {
- return 0;
+ break;
}
kfree(buffer.pointer);
@@ -6118,7 +6135,7 @@ static unsigned int __init
tpacpi_check_std_acpi_brightness_support(void)
acpi_handle video_device;
int bcl_levels = 0;
- tpacpi_acpi_handle_locate("video", ACPI_VIDEO_HID, &video_device);
+ tpacpi_acpi_handle_locate("video", NULL, &video_device);
if (video_device)
bcl_levels = tpacpi_query_bcl_levels(video_device);
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
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.
Signed-off-by: Aaron Lu
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
---
drivers/acpi/internal.h | 5 ++---
drivers/acpi/video.c| 10 +-
drivers/acpi/video_detect.c | 14 --
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..453ae8d 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
Video
-- */
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-bool acpi_video_backlight_quirks(void);
-#else
-static inline bool acpi_video_backlight_quirks(void) { return false; }
+bool acpi_osi_is_win8(void);
+bool acpi_video_verify_backlight_support(void);
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 3bd1eaa..343db59 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device->brightness)
@@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 1 : 0);
+ acpi_osi_is_win8() ? 1 : 0);
}
static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 0 : 1);
+ acpi_osi_is_win8() ? 0 : 1);
}
static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
@@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void
*context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 940edbf..23d7d26 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
acpi_video_get_capabilities(NULL);
}
-bool acpi_video_backlight_quirks(void)
+bool acpi_osi_is_win8(void)
{
return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
}
-EXPORT_SYMBOL(acpi_video_backlight_quirks);
+EXPORT_SYMBOL(acpi_osi_is_win8);
/* Promote the vendor interface instead of the generic video module.
* This function allow DMI blacklists to be implemented by externals
@@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
}
EXPORT_SYMBOL(acpi_video_backlight_support);
+bool acpi_video_verify_backlight_support(void)
+{
+ if (!(acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) &&
+ acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
+ return false;
+ return 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.
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] ACPI / video: seperate backlight control and event interface
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
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
---
drivers/acpi/video.c | 434 +--
include/acpi/video.h | 2 +
2 files changed, 247 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..3bd1eaa 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC"))
device->cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device->dev->handle, &acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = &pdev->dev;
- pci_dev_put(pdev);
- }
-
- memset(&props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device->brightness->count - 3;
- device->backlight = backlight_device_register(name,
- parent,
- device,
-
&acpi_backlight_ops,
- &props);
- kfree(name);
- if (IS_ERR(device->backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device->backlight->props.brightness =
- acpi_video_get_brightness(device->backlight);
-
- device->cooling_dev = thermal_cooling_device_register("LCD",
- device->dev, &video_cooling_ops);
- if (IS_ERR(device->cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device->cooling_dev = NULL;
- return;
- }
-
- dev_info(&device->dev->dev, "registered as cooling_device%d\n",
-device->cooling_dev->id);
- result = sysfs_create_link(&device->dev->dev.kobj,
- &device->cooling_dev-&
[PATCH v3 1/4] backlight: introduce backlight_device_registered
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
Tested-by: Igor Gnatenko
Tested-by: Yves-Alexis Perez
---
drivers/video/backlight/backlight.c | 31 +++
include/linux/backlight.h | 4
2 files changed, 35 insertions(+)
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 94a403a..bf2d71d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
#include
#endif
+static struct list_head bd_list_head;
+static struct mutex bd_list_mutex;
+
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
[BACKLIGHT_PLATFORM] = "platform",
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const
char *name,
mutex_unlock(&pmac_backlight_mutex);
#endif
+ mutex_lock(&bd_list_mutex);
+ list_add(&new_bd->entry, &bd_list_head);
+ mutex_unlock(&bd_list_mutex);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
+bool backlight_device_registered(enum backlight_type type)
+{
+ bool found = false;
+ struct backlight_device *bd;
+
+ mutex_lock(&bd_list_mutex);
+ list_for_each_entry(bd, &bd_list_head, entry) {
+ if (bd->props.type == type) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&bd_list_mutex);
+
+ return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
/**
* backlight_device_unregister - unregisters a backlight device object.
* @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device
*bd)
if (!bd)
return;
+ mutex_lock(&bd_list_mutex);
+ list_del(&bd->entry);
+ mutex_unlock(&bd_list_mutex);
+
#ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(&pmac_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
backlight_class->dev_groups = bl_device_groups;
backlight_class->pm = &backlight_class_dev_pm_ops;
+ INIT_LIST_HEAD(&bd_list_head);
+ mutex_init(&bd_list_mutex);
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
+ /* list entry of all registered backlight devices */
+ struct list_head entry;
+
struct device dev;
};
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device
*dev,
struct backlight_device *bd);
extern void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
#define to_backlight_device(obj) container_of(obj, struct backlight_device,
dev)
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] Fix Win8 backlight issue
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(-) -- 1.8.4.12.g2ea3df6 ___ 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 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 ___ 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 09/20/2013 04:36 PM, Jani Nikula wrote:
> On Tue, 17 Sep 2013, 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
>> ---
>> drivers/acpi/internal.h | 5 ++---
>> drivers/acpi/video.c| 27 +--
>> drivers/acpi/video_detect.c | 14 --
>> 3 files changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index 20f4233..453ae8d 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
>> Video
>>
>> -- */
>> #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
>> -bool acpi_video_backlight_quirks(void);
>> -#else
>> -static inline bool acpi_video_backlight_quirks(void) { return false; }
>> +bool acpi_osi_is_win8(void);
>> +bool acpi_video_verify_backlight_support(void);
>> #endif
>>
>> #endif /* _ACPI_INTERNAL_H_ */
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index 5ad5a71..343db59 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
>> *device, int event)
>> unsigned long long level_current, level_next;
>> int result = -EINVAL;
>>
>> -/* no warning message if acpi_backlight=vendor is used */
>> -if (!acpi_video_backlight_support())
>> +/* no warning message if acpi_backlight=vendor or a quirk is used */
>> +if (!acpi_video_verify_backlight_support())
>> return 0;
>>
>> if (!device->brightness)
>> @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus
>> *video,
>> static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>> {
>> return acpi_video_bus_DOS(video, 0,
>> - acpi_video_backlight_quirks() ? 1 : 0);
>> + acpi_osi_is_win8() ? 1 : 0);
>> }
>>
>> static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>> {
>> return acpi_video_bus_DOS(video, 0,
>> - acpi_video_backlight_quirks() ? 0 : 1);
>> + acpi_osi_is_win8() ? 0 : 1);
>> }
>>
>> static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>> @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level,
>> void *context,
>>
>> static void acpi_video_dev_register_backlight(struct acpi_video_device
>> *device)
>> {
>> -if (acpi_video_backlight_support()) {
>> +if (acpi_video_verify_backlight_support()) {
>> struct backlight_properties props;
>> struct pci_dev *pdev;
>> acpi_handle acpi_parent;
>> @@ -1677,23 +1677,6 @@ static int acpi_video_bus_unregister_backlight(struct
>> acpi_video_bus *video)
>> return error;
>> }
>>
>> -int acpi_video_unregister_backlight(void)
>> -{
>> -struct acpi_video_bus *video;
>> -int error = 0;
>> -
>> -mutex_lock(&video_list_lock);
>> -list_for_each_entry(video, &video_bus_head, entry) {
>> -error = acpi_video_bus_unregister_backlight(video);
>> -if (error)
>> -break;
>> -}
>> -mutex_unlock(&video_list_lock);
>> -
>> -return error;
>> -}
>> -EXPORT_SYMBOL(acpi_video_unregist
Re: [PATCH v2 0/3] Fix Win8 backlight issue
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 :-) -Aaron ___ 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 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] backlight: introduce backlight_device_registered
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
---
drivers/video/backlight/backlight.c | 31 +++
include/linux/backlight.h | 4
2 files changed, 35 insertions(+)
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 94a403a..bf2d71d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
#include
#endif
+static struct list_head bd_list_head;
+static struct mutex bd_list_mutex;
+
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
[BACKLIGHT_PLATFORM] = "platform",
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const
char *name,
mutex_unlock(&pmac_backlight_mutex);
#endif
+ mutex_lock(&bd_list_mutex);
+ list_add(&new_bd->entry, &bd_list_head);
+ mutex_unlock(&bd_list_mutex);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
+bool backlight_device_registered(enum backlight_type type)
+{
+ bool found = false;
+ struct backlight_device *bd;
+
+ mutex_lock(&bd_list_mutex);
+ list_for_each_entry(bd, &bd_list_head, entry) {
+ if (bd->props.type == type) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&bd_list_mutex);
+
+ return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
/**
* backlight_device_unregister - unregisters a backlight device object.
* @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device
*bd)
if (!bd)
return;
+ mutex_lock(&bd_list_mutex);
+ list_del(&bd->entry);
+ mutex_unlock(&bd_list_mutex);
+
#ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(&pmac_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
backlight_class->dev_groups = bl_device_groups;
backlight_class->pm = &backlight_class_dev_pm_ops;
+ INIT_LIST_HEAD(&bd_list_head);
+ mutex_init(&bd_list_mutex);
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
+ /* list entry of all registered backlight devices */
+ struct list_head entry;
+
struct device dev;
};
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device
*dev,
struct backlight_device *bd);
extern void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
#define to_backlight_device(obj) container_of(obj, struct backlight_device,
dev)
--
1.8.4.12.g2ea3df6
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] Fix Win8 backlight issue
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(-) -- 1.8.4.12.g2ea3df6 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] ACPI / video: seperate backlight control and event interface
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
---
drivers/acpi/video.c | 451 ++-
include/acpi/video.h | 2 +
2 files changed, 264 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..5ad5a71 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC"))
device->cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device->dev->handle, &acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = &pdev->dev;
- pci_dev_put(pdev);
- }
-
- memset(&props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device->brightness->count - 3;
- device->backlight = backlight_device_register(name,
- parent,
- device,
-
&acpi_backlight_ops,
- &props);
- kfree(name);
- if (IS_ERR(device->backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device->backlight->props.brightness =
- acpi_video_get_brightness(device->backlight);
-
- device->cooling_dev = thermal_cooling_device_register("LCD",
- device->dev, &video_cooling_ops);
- if (IS_ERR(device->cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device->cooling_dev = NULL;
- return;
- }
-
- dev_info(&device->dev->dev, "registered as cooling_device%d\n",
-device->cooling_dev->id);
- result = sysfs_create_link(&device->dev->dev.kobj,
- &device->cooling_dev->device.kobj,
- "
[PATCH v2 3/3] ACPI / video: Do not register backlight if win8 and native interface exists
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
---
drivers/acpi/internal.h | 5 ++---
drivers/acpi/video.c| 27 +--
drivers/acpi/video_detect.c | 14 --
3 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..453ae8d 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
Video
-- */
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-bool acpi_video_backlight_quirks(void);
-#else
-static inline bool acpi_video_backlight_quirks(void) { return false; }
+bool acpi_osi_is_win8(void);
+bool acpi_video_verify_backlight_support(void);
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5ad5a71..343db59 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device->brightness)
@@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 1 : 0);
+ acpi_osi_is_win8() ? 1 : 0);
}
static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
{
return acpi_video_bus_DOS(video, 0,
- acpi_video_backlight_quirks() ? 0 : 1);
+ acpi_osi_is_win8() ? 0 : 1);
}
static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
@@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void
*context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1677,23 +1677,6 @@ static int acpi_video_bus_unregister_backlight(struct
acpi_video_bus *video)
return error;
}
-int acpi_video_unregister_backlight(void)
-{
- struct acpi_video_bus *video;
- int error = 0;
-
- mutex_lock(&video_list_lock);
- list_for_each_entry(video, &video_bus_head, entry) {
- error = acpi_video_bus_unregister_backlight(video);
- if (error)
- break;
- }
- mutex_unlock(&video_list_lock);
-
- return error;
-}
-EXPORT_SYMBOL(acpi_video_unregister_backlight);
-
static void acpi_video_dev_add_notify_handler(struct acpi_video_device *device)
{
acpi_status status;
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 940edbf..23d7d26 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
acpi_video_get_capabilities(NULL);
}
-bool acpi_video_backlight_quirks(void)
+bool acpi_osi_is_win8(void)
{
return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
}
-EXPORT_SYMBOL(acpi_video_backlight_quirks);
+EXPORT_SYMBOL(acpi_osi_is_win8);
/* Promote the vendor interface instead of the generic video module.
* This function allow DMI blacklists to be implemented by externals
@@ -283,6 +284,15 @@ int acpi_video_backlight_support(void
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, Sep 11, 2013 at 11:45:19AM +0300, Jani Nikula wrote: > On Wed, 11 Sep 2013, Aaron Lu wrote: > > It is possible the i915 driver decides not to register a backlight > > interface for the graphics card for some reason(memory allocation failed > > or it knows the native control does not work on this card or whatever), > > so I would prefer let i915 tell ACPI video that it has registered a > > native backlight control interface as Jani has said. > > > > Then together with the video.use_native_backlight, we can register or > > not register ACPI video backlight interface accordingly. Or rather, we > > can simply not register ACPI video backlight interface for Win8 systems > > as long as i915 indicates that it has native backlight control(if the > > native control is broken, i915 should fix it or blacklist it so that > > i915 will not indicate it has native backlight control and ACPI video > > will continue to register its own). > > > > How does this sound? > > Sounds good to me. > > Before plunging forward, have you observed any difference between the > boot modes? We have reports [1] that the backlight behaviour is Not yet from ACPI's point of view. > different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the > acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole > story. This check in patch 2/2 is a policy: for Win8 system, we think the native backlight control has a better chance of working than the ACPI video's, so I think the check is enough in ACPI video. > > Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code > paths, what guarantees do we have of UEFI+CSM or legacy boots working? I suppose the 'tested BIOS code paths' means the pure UEFI boot mode? I don't know what guatantees do we have since I don't know what happened underneath after the backlight register is set in i915 driver, you or other i915 driver people should know more than I do :-) BTW, after the backlight register is set in i915, is it that some find of firmware code will run in response to the setting of the register(e.g. the BLC_PWM_CTL/BLC_PWM_CPU_CTL/PCI_LBPC reg)? Thanks, Aaron ___ 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, Sep 10, 2013 at 09:23:04PM +0200, Rafael J. Wysocki wrote: > On Tuesday, September 10, 2013 04:53:40 PM Jani Nikula wrote: > > On Mon, 09 Sep 2013, "Rafael J. Wysocki" wrote: > > > On Monday, September 09, 2013 05:21:18 PM Daniel Vetter wrote: > > >> On Mon, Sep 09, 2013 at 02:16:12PM +0200, Rafael J. Wysocki wrote: > > >> > Hi, > > >> > > > >> > On Monday, September 09, 2013 11:32:10 AM Daniel Vetter wrote: > > >> > > Hi Aaaron, > > >> > > > > >> > > Have we grown any clue meanwhile about which Intel boxes need this > > >> > > and for > > >> > > which we still need to keep the acpi backlight around? > > >> > > > >> > First of all, there is a bunch of boxes where ACPI backlight works > > >> > incorrectly > > >> > because of the Win8 compatibility issue. [In short, if we say we are > > >> > compatible > > >> > with Win8, the backlight AML goes into a special code path that is > > >> > broken on > > >> > those machines. Presumably Win8 uses native backlight control on them > > >> > and that > > >> > AML code path is never executed there.] The collection of machines > > >> > with this > > >> > problem appears to be kind of random (various models from various > > >> > vendors), but > > >> > I *think* they are machines that originally shipped with Win7 with a > > >> > Win8 > > >> > "upgrade" option (which in practice requires the BIOS to be updated > > >> > anyway). > > >> > > > >> > Because of that, last time we tried to switch all of the systems using > > >> > i915 > > >> > and telling the BIOS that they are compatible with Win8 over to native > > >> > backlight > > >> > control, but that didn't work for two reasons. The first reason is > > >> > that some > > >> > user space doesn't know how to use intel_backlight and needs to be > > >> > taught about > > >> > that (so some systems are just not ready for that switch). The second > > >> > and more > > >> > fundamental reason is that the native backlight control simply doesn't > > >> > work on > > >> > some machines and we don't seem to have any idea why and how to debug > > >> > this > > >> > particular problem (mostly because we don't have enough information > > >> > and we > > >> > don't know what to ask for). > > >> > > > >> > > I've grown _very_ reluctant to just adding tons of quirks to our > > >> > > driver for > > >> > > the backlight. > > >> > > > > >> > > Almost all the quirks we have added recently (or that have been > > >> > > proposed > > >> > > to be added) are for the backlight. Imo that indicates we get > > >> > > something > > >> > > fundamentally wrong ... > > >> > > > >> > This thing isn't really a quirk. It rather is an option for the users > > >> > of > > >> > the systems where ACPI backlight doesn't work to switch over to the > > >> > native > > >> > backlight control using a command line switch. This way they can at > > >> > least > > >> > *see* if the native backlight control works for them and (hopefully) > > >> > report > > >> > problems if that's not the case. This gives us a chance to get more > > >> > information about what the problem is and possibly to make some > > >> > progress > > >> > without making changes for everyone, reverting those changes when they > > >> > don't > > >> > work etc. > > >> > > > >> > An alternative for them is to pass acpi_osi="!Windows 2012" which will > > >> > probably > > >> > make the ACPI backlight work for them again, but this rather is a step > > >> > back, > > >> > because we can't possibly learn anything new from that. > > >> > > >> If Win8 is as broken as we are I'm ok with the module option. It just > > >> sounded to me like right now we don't know of a way to make all machines > > >> somewhat happy, combined with the other pile of random backlight issues > > >> the assumption that we do something (maybe something a bit racy) that > > >> windows doesn't do isn't too far-fetched. So I'm not wary of the machines > > >> where the aml is busted for acpi_os=win8, but for the others where this > > >> broke stuff. > > >> > > >> Or do I miss something here? > > > > > > The ACPI video driver doesn't do anything new now. It only does things > > > that > > > did work before we started to tell BIOSes we're compatible with Win8. The > > > reason why they don't work on some machines now is not related to whether > > > or > > > not Win8 is broken, but to what is there in the ACPI tables on those > > > machines. > > > That *is* pure garbage, but Win8 users don't see that (presumably, because > > > Win8 does't execute the AML in question). We don't see that either with > > > acpi_osi="!Windows 2012" (because then we don't execute that AML either). > > > > > > Whether or not Win8 is broken doesn't matter here. What matters is that > > > we > > > have to deal with broken AML somehow. > > > > > > One way may be to tell everyone affected by this to pass > > > acpi_osi="!Windows 2012" in the kernel command line or possibly create a > > > blacklist of those
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, Sep 09, 2013 at 11:32:10AM +0200, Daniel Vetter wrote: > Hi Aaaron, > > Have we grown any clue meanwhile about which Intel boxes need this and for > which we still need to keep the acpi backlight around? I've grown _very_ Sorry, no general rule has been found. As Rafael has said, it appears to be random... And in addition to the "shipped with Win7 with a Win8 upgrade option" case, I also find a Sony laptop that has Win8 pre-installed and a broken ACPI video backlight interface. Its ACPI video backlight control method is simply a stub, so even the acpi_osi="!Windows 2012" will not provide a working backlight for this system. -Aaron > reluctant to just adding tons of quirks to our driver for the backlight. > Almost all the quirks we have added recently (or that have been proposed > to be added) are for the backlight. Imo that indicates we get something > fundamentally wrong ... > > Cheers, Daniel > > On Mon, Sep 09, 2013 at 04:42:20PM +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 > > --- > > 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(-) > > > > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > > index 20f4233..a53832e 100644 > > --- a/drivers/acpi/internal.h > > +++ b/drivers/acpi/internal.h > > @@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device > > *adev, > > > > -- > > */ > > #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) > > bool acpi_video_backlight_quirks(void); > > +bool acpi_video_verify_backlight_support(void); > > #else >
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/10/2013 01:22 PM, Igor Gnatenko wrote:
> 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).
That patch is reverted as it cause problem for other system:
https://bugs.freedesktop.org/show_bug.cgi?id=68355
OTOH, the thinkpad-acpi module already has a call to _BCL except that
the tpacpi_acpi_handle_locate failed to locate video controller's handle:
https://bugzilla.kernel.org/show_bug.cgi?id=51231#c121
I'll see if I can figure out why.
Thanks,
Aaron
___
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 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
>
___
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 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] ACPI / video / i915: Remove 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(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
---
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(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..a53832e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device *adev,
-- */
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
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_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5ad5a71..cc709a7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device->brightness)
@@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void
*context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1933,14 +1933,22 @@ static int __init intel_opregion_present(void)
return opregion;
}
-int acpi_video_register(void)
+int __acpi_video_register(bool backlight_quirks)
{
- int result = 0;
+ bool no_backlight;
+ int result;
+
+ no_backlight = backlight_quirks ? acpi_video_backlight_quirks() : false;
+
if (register_count) {
/*
-* if the function of acpi_video_register is already called,
-* don't register
[PATCH 0/2] Rework ACPI video driver
The ACPI video module provides two functionalities: backlight control and backlight hotkey event delivery. It is possible the backlight control interface is broken while the system still needs its event delivery, so it's worth to seperate the two interfaces clearly. This patchset has two patches, the first is to sepeate the two interfaces in the ACPI video module and the second one solves some Win8 backlight control problems by removing ACPI video's backlight interface while still keeping its event delivery functionality. Due to some systems whose firmware claims win8 compatible have problems with i915's backlight control interface, a module param is introduced to give user a chance to select if they want to remove ACPI video's backlight control interface. The param is set to false by default. Aaron Lu (2): ACPI / video: seperate backlight control and event interface ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8 drivers/acpi/internal.h | 2 + drivers/acpi/video.c| 473 +++- 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| 11 +- include/linux/acpi.h| 1 + 8 files changed, 310 insertions(+), 200 deletions(-) -- 1.8.4.12.g2ea3df6 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] ACPI / video: seperate backlight control and event interface
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
---
drivers/acpi/video.c | 451 ++-
include/acpi/video.h | 2 +
2 files changed, 264 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aebcf63..5ad5a71 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
static int register_count;
+static struct mutex video_list_lock;
+static struct list_head video_bus_head;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
@@ -157,6 +159,7 @@ struct acpi_video_bus {
struct acpi_video_bus_flags flags;
struct list_head video_device_list;
struct mutex device_list_lock; /* protects video_device_list */
+ struct list_head entry;
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
@@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC"))
device->cap._DDC = 1;
-
- if (acpi_video_backlight_support()) {
- struct backlight_properties props;
- struct pci_dev *pdev;
- acpi_handle acpi_parent;
- struct device *parent = NULL;
- int result;
- static int count;
- char *name;
-
- result = acpi_video_init_brightness(device);
- if (result)
- return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
- if (!name)
- return;
- count++;
-
- acpi_get_parent(device->dev->handle, &acpi_parent);
-
- pdev = acpi_get_pci_dev(acpi_parent);
- if (pdev) {
- parent = &pdev->dev;
- pci_dev_put(pdev);
- }
-
- memset(&props, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_FIRMWARE;
- props.max_brightness = device->brightness->count - 3;
- device->backlight = backlight_device_register(name,
- parent,
- device,
-
&acpi_backlight_ops,
- &props);
- kfree(name);
- if (IS_ERR(device->backlight))
- return;
-
- /*
-* Save current brightness level in case we have to restore it
-* before acpi_video_device_lcd_set_level() is called next time.
-*/
- device->backlight->props.brightness =
- acpi_video_get_brightness(device->backlight);
-
- device->cooling_dev = thermal_cooling_device_register("LCD",
- device->dev, &video_cooling_ops);
- if (IS_ERR(device->cooling_dev)) {
- /*
-* Set cooling_dev to NULL so we don't crash trying to
-* free it.
-* Also, why the hell we are returning early and
-* not attempt to register video output if cooling
-* device registration failed?
-* -- dtor
-*/
- device->cooling_dev = NULL;
-
Backlight control only in the kernel?
On 08/07/2013 03:44 PM, Borislav Petkov wrote: > On Sun, Jun 09, 2013 at 07:01:36PM -0400, 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. > > Maybe tangential, so Aaron and I were wondering on > https://bugzilla.kernel.org/show_bug.cgi?id=60680 whether it would make > sense to handle the backlight control strictly in the kernel, without > going to userspace and back? I think this would require the kernel has the knowledge of which backlight interface this system is using or should be using, or it wouldn't know which interface should receive and process the event... -Aaron > > Background is that on my x230, I needed to connect the > Fn-Fx backlight hotkey presses to a script to write to > /sys/class/backlight/intel_backlight/brightness because Fluxbox doesn't > do that (and maybe doesn't have to). > > So, without presuming any ACPI or backlight knowledge, can we make the > backlight control work only in the kernel by connecting the hotkey > presses to some backlight controlling interface which backlight-capable > devices implement so that it works regardless of userspace environment? > Even if the machine is not running X? > > Hmmm. > > Thanks. >
Re: Backlight control only in the kernel?
On 08/07/2013 03:44 PM, Borislav Petkov wrote: > On Sun, Jun 09, 2013 at 07:01:36PM -0400, 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. > > Maybe tangential, so Aaron and I were wondering on > https://bugzilla.kernel.org/show_bug.cgi?id=60680 whether it would make > sense to handle the backlight control strictly in the kernel, without > going to userspace and back? I think this would require the kernel has the knowledge of which backlight interface this system is using or should be using, or it wouldn't know which interface should receive and process the event... -Aaron > > Background is that on my x230, I needed to connect the > Fn-Fx backlight hotkey presses to a script to write to > /sys/class/backlight/intel_backlight/brightness because Fluxbox doesn't > do that (and maybe doesn't have to). > > So, without presuming any ACPI or backlight knowledge, can we make the > backlight control work only in the kernel by connecting the hotkey > presses to some backlight controlling interface which backlight-capable > devices implement so that it works regardless of userspace environment? > Even if the machine is not running X? > > Hmmm. > > Thanks. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
i915 backlight
On 08/02/2013 02:25 PM, Josep Lladonosa wrote: > Hello, > > I am using a Lenovo Edge E530 and, with kernel 3.11.0-rc3, I had to > change to this parameter to the kernel boot: > > > GRUB_CMDLINE_LINUX="acpi_osi=\"!Windows 2012\"" What if you remove the above from kernel command line, and add video.brightness_switch_enabled=0 to kernel command line, then set the following in xorg.conf: $ cat /etc/X11/xorg.conf Section "Device" Option "Backlight" "intel_backlight" Identifier "Card0" Driver "intel" BusID "PCI:0:2:0" EndSection Does everything work? If not, please test if manually change brightness level through sysfs works: # cd /sys/calss/backlight/intel_backlight # echo xxx > brightness And also test if hotkey event is sent out or not by running acpi_listen and then press the hotkey. Thanks, Aaron > > > instead of previous > > GRUB_CMDLINE_LINUX="acpi_osi=Linux acpi_backlight=vendor" > > to be able to change brightness. In some kernel versions before, it > worked, but with 15 levels, but in graphical system brightness bar was > not moving. > > > Now I have, though, 8 possible values for brightness and brightness > bar shows its correct position. > > Josep > > On 2 August 2013 08:00, Aaron Lu wrote: >> On 08/01/2013 04:07 PM, Borislav Petkov wrote: >>> On Wed, Jul 31, 2013 at 11:16:52PM +0200, Rafael J. Wysocki wrote: >>>> Does reverting efaa14c help? >>> >>> Nope. >>> >>> But see my other reply to Aaron. >> >> Assume you have specified to use intel_backlight in xorg.conf, does >> booting with video.brightness_switch_enabled=0 help? >> >> Thanks, >> Aaron >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > > >
i915 backlight
On 08/01/2013 04:07 PM, Borislav Petkov wrote: > On Wed, Jul 31, 2013 at 11:16:52PM +0200, Rafael J. Wysocki wrote: >> Does reverting efaa14c help? > > Nope. > > But see my other reply to Aaron. Assume you have specified to use intel_backlight in xorg.conf, does booting with video.brightness_switch_enabled=0 help? Thanks, Aaron
i915 backlight
On 08/01/2013 05:07 PM, Aaron Lu wrote: > On 08/01/2013 04:12 PM, Borislav Petkov wrote: >> On Thu, Aug 01, 2013 at 09:13:35AM +0800, Aaron Lu wrote: >>> Can you please run acpi_listen and then press the Fn-Fx key, see if the >>> events are correctly sent out? >> >> Like this? >> >> # acpi_listen >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> ^C > > Yes, so the event is correctly sent out. > >> >>> From the bug page: >>> https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 >>> I got the impression that both the acpi_video interface and the vendor >>> interface thinkpad_screen are broken. So adding this cmdline here works >>> suggests that either thinkpad_screen works or thinkpad vendor driver >>> doesn't get loaded or doesn't create that interface for some reason. >>> >>> Alternatively, if the intel_backlight interface works(highly possible), >>> you can use xorg.conf to specify the that backlight interface for X. >>> >>> Section "Device" >>> Option "Backlight" "intel_backlight" >>> Identifier "Card0" >>> Driver "intel" >>> BusID "PCI:0:2:0" >>> EndSection >> >> Yeah, that didn't work *but* manually writing to both: >> >> /sys/class/backlight/acpi_video0/brightness >> >> and >> >> /sys/class/backlight/intel_backlight/brightness >> >> works. > > Err...we have the event sent out on hotkey press and the interface also > works, but still, using hotkey to adjust brightness level is broken... > > I just found an old acer laptop that has similar issue(or even worse: on > X starts, an almost black screen is shown and hotkey adjust doesn't > work), I'll look into this. Hi Jani & Daniel, It turned out there is an integer overflow problem, and the below patch fixed this problem on Acer Aspire 4732Z and thinkpad R61i. From: Aaron Lu Subject: [PATCH] drm/i915: avoid brightness overflow when doing scale Some card's max brightness level is pretty large, e.g. on Acer Aspire 4732Z, the max level is 989910. If user space set a large enough level then the current scale done in intel_panel_set_backlight will cause an integer overflow and the scaled level will be mistakenly small, leaving user with an almost black screen. This patch fixes this problem. Signed-off-by: Aaron Lu --- Since the only external user of intel_panel_set_backlight is operation region code where the max will be a constant of 255, this patch fixes the problem by comparing freq and max and then do things accordingly instead of converting to 64 bits. drivers/gpu/drm/i915/intel_panel.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 67e2c1f..7c674f0 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -498,7 +498,10 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max) } /* scale to hardware */ - level = level * freq / max; + if (freq < max) + level = level * freq / max; + else + level = freq / max * level; dev_priv->backlight.level = level; if (dev_priv->backlight.device) -- 1.8.3.1 Hi Boris, Since the sysfs interface works on your system, I think your problem should be different. Can you please file a bug for this? I can provide you with a debug patch and then see what happened. Please attach acpidump when filing the bug. https://bugzilla.kernel.org, ACPI/Power-Video. Thanks, Aaron > >> >> The ranges are different, though: >> >> intel_backlight/actual_brightness:1000 >> intel_backlight/bl_power:0 >> intel_backlight/brightness:1000 >> intel_backlight/max_brightness:4437 >> intel_backlight/type:raw >> >> acpi_video0/actual_brightness:41 >> acpi_video0/bl_power:0 >> acpi_video0/brightness:41 >> acpi_video0/max_brightness:100 >> acpi_video0/type:firmware > > Yes, different interface has different brightness ranges and a value in > one range may turn out to be the same actual brightness level of another > value in another range. > >> >> I guess I need to write me a dirty script for now ... :-) > > :-) > >> >> Thanks guys. >> > Thanks, > Aaron > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Re: i915 backlight
On 08/02/2013 02:25 PM, Josep Lladonosa wrote: > Hello, > > I am using a Lenovo Edge E530 and, with kernel 3.11.0-rc3, I had to > change to this parameter to the kernel boot: > > > GRUB_CMDLINE_LINUX="acpi_osi=\"!Windows 2012\"" What if you remove the above from kernel command line, and add video.brightness_switch_enabled=0 to kernel command line, then set the following in xorg.conf: $ cat /etc/X11/xorg.conf Section "Device" Option "Backlight" "intel_backlight" Identifier "Card0" Driver "intel" BusID "PCI:0:2:0" EndSection Does everything work? If not, please test if manually change brightness level through sysfs works: # cd /sys/calss/backlight/intel_backlight # echo xxx > brightness And also test if hotkey event is sent out or not by running acpi_listen and then press the hotkey. Thanks, Aaron > > > instead of previous > > GRUB_CMDLINE_LINUX="acpi_osi=Linux acpi_backlight=vendor" > > to be able to change brightness. In some kernel versions before, it > worked, but with 15 levels, but in graphical system brightness bar was > not moving. > > > Now I have, though, 8 possible values for brightness and brightness > bar shows its correct position. > > Josep > > On 2 August 2013 08:00, Aaron Lu wrote: >> On 08/01/2013 04:07 PM, Borislav Petkov wrote: >>> On Wed, Jul 31, 2013 at 11:16:52PM +0200, Rafael J. Wysocki wrote: >>>> Does reverting efaa14c help? >>> >>> Nope. >>> >>> But see my other reply to Aaron. >> >> Assume you have specified to use intel_backlight in xorg.conf, does >> booting with video.brightness_switch_enabled=0 help? >> >> Thanks, >> Aaron >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: i915 backlight
On 08/01/2013 04:07 PM, Borislav Petkov wrote: > On Wed, Jul 31, 2013 at 11:16:52PM +0200, Rafael J. Wysocki wrote: >> Does reverting efaa14c help? > > Nope. > > But see my other reply to Aaron. Assume you have specified to use intel_backlight in xorg.conf, does booting with video.brightness_switch_enabled=0 help? Thanks, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: i915 backlight
On 08/01/2013 05:07 PM, Aaron Lu wrote: > On 08/01/2013 04:12 PM, Borislav Petkov wrote: >> On Thu, Aug 01, 2013 at 09:13:35AM +0800, Aaron Lu wrote: >>> Can you please run acpi_listen and then press the Fn-Fx key, see if the >>> events are correctly sent out? >> >> Like this? >> >> # acpi_listen >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> video/brightnessup BRTUP 0086 >> video/brightnessdown BRTDN 0087 >> ^C > > Yes, so the event is correctly sent out. > >> >>> From the bug page: >>> https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 >>> I got the impression that both the acpi_video interface and the vendor >>> interface thinkpad_screen are broken. So adding this cmdline here works >>> suggests that either thinkpad_screen works or thinkpad vendor driver >>> doesn't get loaded or doesn't create that interface for some reason. >>> >>> Alternatively, if the intel_backlight interface works(highly possible), >>> you can use xorg.conf to specify the that backlight interface for X. >>> >>> Section "Device" >>> Option "Backlight" "intel_backlight" >>> Identifier "Card0" >>> Driver "intel" >>> BusID "PCI:0:2:0" >>> EndSection >> >> Yeah, that didn't work *but* manually writing to both: >> >> /sys/class/backlight/acpi_video0/brightness >> >> and >> >> /sys/class/backlight/intel_backlight/brightness >> >> works. > > Err...we have the event sent out on hotkey press and the interface also > works, but still, using hotkey to adjust brightness level is broken... > > I just found an old acer laptop that has similar issue(or even worse: on > X starts, an almost black screen is shown and hotkey adjust doesn't > work), I'll look into this. Hi Jani & Daniel, It turned out there is an integer overflow problem, and the below patch fixed this problem on Acer Aspire 4732Z and thinkpad R61i. From: Aaron Lu Subject: [PATCH] drm/i915: avoid brightness overflow when doing scale Some card's max brightness level is pretty large, e.g. on Acer Aspire 4732Z, the max level is 989910. If user space set a large enough level then the current scale done in intel_panel_set_backlight will cause an integer overflow and the scaled level will be mistakenly small, leaving user with an almost black screen. This patch fixes this problem. Signed-off-by: Aaron Lu --- Since the only external user of intel_panel_set_backlight is operation region code where the max will be a constant of 255, this patch fixes the problem by comparing freq and max and then do things accordingly instead of converting to 64 bits. drivers/gpu/drm/i915/intel_panel.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 67e2c1f..7c674f0 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -498,7 +498,10 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max) } /* scale to hardware */ - level = level * freq / max; + if (freq < max) + level = level * freq / max; + else + level = freq / max * level; dev_priv->backlight.level = level; if (dev_priv->backlight.device) -- 1.8.3.1 Hi Boris, Since the sysfs interface works on your system, I think your problem should be different. Can you please file a bug for this? I can provide you with a debug patch and then see what happened. Please attach acpidump when filing the bug. https://bugzilla.kernel.org, ACPI/Power-Video. Thanks, Aaron > >> >> The ranges are different, though: >> >> intel_backlight/actual_brightness:1000 >> intel_backlight/bl_power:0 >> intel_backlight/brightness:1000 >> intel_backlight/max_brightness:4437 >> intel_backlight/type:raw >> >> acpi_video0/actual_brightness:41 >> acpi_video0/bl_power:0 >> acpi_video0/brightness:41 >> acpi_video0/max_brightness:100 >> acpi_video0/type:firmware > > Yes, different interface has different brightness ranges and a value in > one range may turn out to be the same actual brightness level of another > value in another range. > >> >> I guess I need to write me a dirty script for now ... :-) > > :-) > >> >> Thanks guys. >> > Thanks, > Aaron > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: i915 backlight
On 08/01/2013 04:12 PM, Borislav Petkov wrote: > On Thu, Aug 01, 2013 at 09:13:35AM +0800, Aaron Lu wrote: >> Can you please run acpi_listen and then press the Fn-Fx key, see if the >> events are correctly sent out? > > Like this? > > # acpi_listen > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > ^C Yes, so the event is correctly sent out. > >> From the bug page: >> https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 >> I got the impression that both the acpi_video interface and the vendor >> interface thinkpad_screen are broken. So adding this cmdline here works >> suggests that either thinkpad_screen works or thinkpad vendor driver >> doesn't get loaded or doesn't create that interface for some reason. >> >> Alternatively, if the intel_backlight interface works(highly possible), >> you can use xorg.conf to specify the that backlight interface for X. >> >> Section "Device" >> Option "Backlight" "intel_backlight" >> Identifier "Card0" >> Driver "intel" >> BusID "PCI:0:2:0" >> EndSection > > Yeah, that didn't work *but* manually writing to both: > > /sys/class/backlight/acpi_video0/brightness > > and > > /sys/class/backlight/intel_backlight/brightness > > works. Err...we have the event sent out on hotkey press and the interface also works, but still, using hotkey to adjust brightness level is broken... I just found an old acer laptop that has similar issue(or even worse: on X starts, an almost black screen is shown and hotkey adjust doesn't work), I'll look into this. > > The ranges are different, though: > > intel_backlight/actual_brightness:1000 > intel_backlight/bl_power:0 > intel_backlight/brightness:1000 > intel_backlight/max_brightness:4437 > intel_backlight/type:raw > > acpi_video0/actual_brightness:41 > acpi_video0/bl_power:0 > acpi_video0/brightness:41 > acpi_video0/max_brightness:100 > acpi_video0/type:firmware Yes, different interface has different brightness ranges and a value in one range may turn out to be the same actual brightness level of another value in another range. > > I guess I need to write me a dirty script for now ... :-) :-) > > Thanks guys. > Thanks, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: i915 backlight
On 08/01/2013 12:36 AM, Borislav Petkov wrote: > On Wed, Jul 31, 2013 at 06:22:52PM +0200, Borislav Petkov wrote: >> Dudes, >> >> has anyone already reported this (happens on Linus of today + >> tip/master): > > Oh, one more thing: I can't control the backlight anymore on this x230 > with the Fn-Fx keys and this is most probably related to that recent > backlight revert story. If there is a new approach I can test, please > let me know. Can you please run acpi_listen and then press the Fn-Fx key, see if the events are correctly sent out? > > Btw, it worked before on this machine with "acpi_backlight=vendor" on > the cmdline. >From the bug page: https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 I got the impression that both the acpi_video interface and the vendor interface thinkpad_screen are broken. So adding this cmdline here works suggests that either thinkpad_screen works or thinkpad vendor driver doesn't get loaded or doesn't create that interface for some reason. Alternatively, if the intel_backlight interface works(highly possible), you can use xorg.conf to specify the that backlight interface for X. Section "Device" Option "Backlight" "intel_backlight" Identifier "Card0" Driver "intel" BusID "PCI:0:2:0" EndSection Thanks, Aaron ___ 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 07/31/2013 08:01 AM, 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? Sounds good to me. -Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
i915 backlight
On 08/01/2013 04:12 PM, Borislav Petkov wrote: > On Thu, Aug 01, 2013 at 09:13:35AM +0800, Aaron Lu wrote: >> Can you please run acpi_listen and then press the Fn-Fx key, see if the >> events are correctly sent out? > > Like this? > > # acpi_listen > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > video/brightnessup BRTUP 0086 > video/brightnessdown BRTDN 0087 > ^C Yes, so the event is correctly sent out. > >> From the bug page: >> https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 >> I got the impression that both the acpi_video interface and the vendor >> interface thinkpad_screen are broken. So adding this cmdline here works >> suggests that either thinkpad_screen works or thinkpad vendor driver >> doesn't get loaded or doesn't create that interface for some reason. >> >> Alternatively, if the intel_backlight interface works(highly possible), >> you can use xorg.conf to specify the that backlight interface for X. >> >> Section "Device" >> Option "Backlight" "intel_backlight" >> Identifier "Card0" >> Driver "intel" >> BusID "PCI:0:2:0" >> EndSection > > Yeah, that didn't work *but* manually writing to both: > > /sys/class/backlight/acpi_video0/brightness > > and > > /sys/class/backlight/intel_backlight/brightness > > works. Err...we have the event sent out on hotkey press and the interface also works, but still, using hotkey to adjust brightness level is broken... I just found an old acer laptop that has similar issue(or even worse: on X starts, an almost black screen is shown and hotkey adjust doesn't work), I'll look into this. > > The ranges are different, though: > > intel_backlight/actual_brightness:1000 > intel_backlight/bl_power:0 > intel_backlight/brightness:1000 > intel_backlight/max_brightness:4437 > intel_backlight/type:raw > > acpi_video0/actual_brightness:41 > acpi_video0/bl_power:0 > acpi_video0/brightness:41 > acpi_video0/max_brightness:100 > acpi_video0/type:firmware Yes, different interface has different brightness ranges and a value in one range may turn out to be the same actual brightness level of another value in another range. > > I guess I need to write me a dirty script for now ... :-) :-) > > Thanks guys. > Thanks, Aaron
i915 backlight
On 08/01/2013 12:36 AM, Borislav Petkov wrote: > On Wed, Jul 31, 2013 at 06:22:52PM +0200, Borislav Petkov wrote: >> Dudes, >> >> has anyone already reported this (happens on Linus of today + >> tip/master): > > Oh, one more thing: I can't control the backlight anymore on this x230 > with the Fn-Fx keys and this is most probably related to that recent > backlight revert story. If there is a new approach I can test, please > let me know. Can you please run acpi_listen and then press the Fn-Fx key, see if the events are correctly sent out? > > Btw, it worked before on this machine with "acpi_backlight=vendor" on > the cmdline. >From the bug page: https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80 I got the impression that both the acpi_video interface and the vendor interface thinkpad_screen are broken. So adding this cmdline here works suggests that either thinkpad_screen works or thinkpad vendor driver doesn't get loaded or doesn't create that interface for some reason. Alternatively, if the intel_backlight interface works(highly possible), you can use xorg.conf to specify the that backlight interface for X. Section "Device" Option "Backlight" "intel_backlight" Identifier "Card0" Driver "intel" BusID "PCI:0:2:0" EndSection Thanks, Aaron
[Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems
On 07/31/2013 08:01 AM, 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? Sounds good to me. -Aaron
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/15/2013 07:42 PM, Rafael J. Wysocki wrote: > On Monday, July 15, 2013 10:36:15 AM Aaron Lu wrote: >> On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki >>> >>> 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 >> >> Reviewed-by: Aaron Lu >> >> BTW, I also tested on a Toshiba laptop Z830 where its AML code >> claims support of win8, the result is as expected: ACPI video >> interface is removed, i915 Xorg driver picks intel_backlight. >> >> Thanks for the fix. > > Cool, thanks for testing this! > > Can you please also ask bug reporters in the BZ entires related to this to > test > it too? Sure. To be clear, I actually tested the patch in your linux-next branch, which turned out to be a little different in that you have fixed the problem Igor raised here. I'll ask reporters to test on a stable tree with the following two patches on top: https://patchwork.kernel.org/patch/2812951/ (expose OSI version) https://patchwork.kernel.org/patch/2827793/ (your updated patch) or your linux-next branch, whichever they like. -Aaron > > Rafael > > >>> --- >>> 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_backl
Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/15/2013 07:42 PM, Rafael J. Wysocki wrote: > On Monday, July 15, 2013 10:36:15 AM Aaron Lu wrote: >> On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki >>> >>> 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 >> >> Reviewed-by: Aaron Lu >> >> BTW, I also tested on a Toshiba laptop Z830 where its AML code >> claims support of win8, the result is as expected: ACPI video >> interface is removed, i915 Xorg driver picks intel_backlight. >> >> Thanks for the fix. > > Cool, thanks for testing this! > > Can you please also ask bug reporters in the BZ entires related to this to > test > it too? Sure. To be clear, I actually tested the patch in your linux-next branch, which turned out to be a little different in that you have fixed the problem Igor raised here. I'll ask reporters to test on a stable tree with the following two patches on top: https://patchwork.kernel.org/patch/2812951/ (expose OSI version) https://patchwork.kernel.org/patch/2827793/ (your updated patch) or your linux-next branch, whichever they like. -Aaron > > Rafael > > >>> --- >>> 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_backl
[Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> 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
Reviewed-by: Aaron Lu
BTW, I also tested on a Toshiba laptop Z830 where its AML code
claims support of win8, the result is as expected: ACPI video
interface is removed, i915 Xorg driver picks intel_backlight.
Thanks for the fix.
-Aaron
> ---
> 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, &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;
> +
Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> 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
Reviewed-by: Aaron Lu
BTW, I also tested on a Toshiba laptop Z830 where its AML code
claims support of win8, the result is as expected: ACPI video
interface is removed, i915 Xorg driver picks intel_backlight.
Thanks for the fix.
-Aaron
> ---
> 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, &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;
> +
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 07/07/2013 09:19 PM, Rafael J. Wysocki wrote:
> OK, the patch is appended. Please have a look and tell me what you think.
>
> Thanks,
> Rafael
>
>
> ---
> From: Rafael J. Wysocki
> Subject: ACPI / video / i915: Remove 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 the ACPI one is not
> registered in advance.
The condition thinkpad_acpi checks to decide if it wants to create
backlight control interface is acpi_video_backlight_support function,
not that if ACPI video driver has registered previously. I'm sorry if my
previous words gave you such a conclusion.
> Therefore we need to keep the ACPI backlight interface registered
> until the i915 driver is loaded which then will unregister it if
> the firmware has called _OSI for Windows 8.
>
> 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.
>
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/video.c| 61
>
> drivers/acpi/video_detect.c | 11 +++
> drivers/gpu/drm/i915/i915_dma.c |2 -
> include/acpi/video.h| 11 ++-
> include/linux/acpi.h|6 +++
> 5 files changed, 84 insertions(+), 7 deletions(-)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -1854,6 +1854,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);
> + }
> + if (dev->cooling_dev) {
> + sysfs_remove_link(&dev->dev->dev.kobj,
> + "thermal_cooling");
> + sysfs_remove_link(&dev->cooling_dev->device.kobj,
> + "device");
> + thermal_cooling_device_unregister(dev->cooling_dev);
> + dev->cooling_dev = NULL;
> + }
> + }
> + mutex_unlock(&video->device_list_lock);
> + acpi_video_bus_start_devices(video);
> + return AE_OK;
> +}
> +
> static int __init is_i740(struct pci_dev *dev)
> {
> if (dev->device == 0x00D1)
> @@ -1885,14 +1925,25 @@ static int __init intel_opregion_present
> return opregion;
> }
>
> -int acpi_video_register(void)
> +int __acpi_vid
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 07/07/2013 09:19 PM, Rafael J. Wysocki wrote:
> OK, the patch is appended. Please have a look and tell me what you think.
>
> Thanks,
> Rafael
>
>
> ---
> From: Rafael J. Wysocki
> Subject: ACPI / video / i915: Remove 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 the ACPI one is not
> registered in advance.
The condition thinkpad_acpi checks to decide if it wants to create
backlight control interface is acpi_video_backlight_support function,
not that if ACPI video driver has registered previously. I'm sorry if my
previous words gave you such a conclusion.
> Therefore we need to keep the ACPI backlight interface registered
> until the i915 driver is loaded which then will unregister it if
> the firmware has called _OSI for Windows 8.
>
> 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.
>
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/video.c| 61
>
> drivers/acpi/video_detect.c | 11 +++
> drivers/gpu/drm/i915/i915_dma.c |2 -
> include/acpi/video.h| 11 ++-
> include/linux/acpi.h|6 +++
> 5 files changed, 84 insertions(+), 7 deletions(-)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -1854,6 +1854,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);
> + }
> + if (dev->cooling_dev) {
> + sysfs_remove_link(&dev->dev->dev.kobj,
> + "thermal_cooling");
> + sysfs_remove_link(&dev->cooling_dev->device.kobj,
> + "device");
> + thermal_cooling_device_unregister(dev->cooling_dev);
> + dev->cooling_dev = NULL;
> + }
> + }
> + mutex_unlock(&video->device_list_lock);
> + acpi_video_bus_start_devices(video);
> + return AE_OK;
> +}
> +
> static int __init is_i740(struct pci_dev *dev)
> {
> if (dev->device == 0x00D1)
> @@ -1885,14 +1925,25 @@ static int __init intel_opregion_present
> return opregion;
> }
>
> -int acpi_video_register(void)
> +int __acpi_vid
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
> On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
>> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
>>> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> 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 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.
>
> Signed-off-by: Matthew Garrett
> ---
> drivers/gpu/drm/i915/i915_dma.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev,
> unsigned long flags)
> /* Must be done after probing outputs */
> intel_opregion_init(dev);
> acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
> }
>
> if (IS_GEN5(dev))
>
Well, this causes build failures to happen when the ACPI video driver is
modular and the graphics driver is not.
I'm not sure how to resolve that, so suggestions are welcome.
>>>
>>> Actually, that happened with the radeon patch.
>>>
>>> That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
>>> example.
>>>
>>> What about making acpi_video_register() do the quirk instead? We could add
>>> an
>>> argument to it indicating whether or not quirks should be applied.
>>
>> Actually, I wonder what about the appended patch (on top of the Aaron's
>> https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this
>> series.
>
> Or even something as simple as this one.
>
> ---
> drivers/acpi/video_detect.c |3 +++
> 1 file changed, 3 insertions(+)
>
> Index: linux-pm/drivers/acpi/video_detect.c
> ===
> --- linux-pm.orig/drivers/acpi/video_detect.c
> +++ linux-pm/drivers/acpi/video_detect.c
> @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
>*/
>
> dmi_check_system(video_detect_dmi_table);
> +
> + if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
> + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
Then vendor driver(thinkpad_acpi) will step in and create a backlight
interface for the system, which, unfortunately, is also broken for those
win8 thinkpads.
So we will need to do something in thinkpad_acpi to also not create
backlight interface for these systems too.
This actually doesn't feel bad to me, since the modules are blacklisting
their own interfaces. The downside is of course, two quirk code exist.
BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
have this problem since it made the platform driver think the ACPI video
driver will control the backlight and then use the newly added API to
remove ACPI video created backlight interface.
Thanks,
Aaron
> } else {
> status = acpi_bus_get_device(graphics_handle, &tmp_dev);
> if (ACPI_FAILURE(status)) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
> On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
>> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
>>> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> 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 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.
>
> Signed-off-by: Matthew Garrett
> ---
> drivers/gpu/drm/i915/i915_dma.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev,
> unsigned long flags)
> /* Must be done after probing outputs */
> intel_opregion_init(dev);
> acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
> }
>
> if (IS_GEN5(dev))
>
Well, this causes build failures to happen when the ACPI video driver is
modular and the graphics driver is not.
I'm not sure how to resolve that, so suggestions are welcome.
>>>
>>> Actually, that happened with the radeon patch.
>>>
>>> That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
>>> example.
>>>
>>> What about making acpi_video_register() do the quirk instead? We could add
>>> an
>>> argument to it indicating whether or not quirks should be applied.
>>
>> Actually, I wonder what about the appended patch (on top of the Aaron's
>> https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this
>> series.
>
> Or even something as simple as this one.
>
> ---
> drivers/acpi/video_detect.c |3 +++
> 1 file changed, 3 insertions(+)
>
> Index: linux-pm/drivers/acpi/video_detect.c
> ===
> --- linux-pm.orig/drivers/acpi/video_detect.c
> +++ linux-pm/drivers/acpi/video_detect.c
> @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
>*/
>
> dmi_check_system(video_detect_dmi_table);
> +
> + if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
> + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
Then vendor driver(thinkpad_acpi) will step in and create a backlight
interface for the system, which, unfortunately, is also broken for those
win8 thinkpads.
So we will need to do something in thinkpad_acpi to also not create
backlight interface for these systems too.
This actually doesn't feel bad to me, since the modules are blacklisting
their own interfaces. The downside is of course, two quirk code exist.
BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
have this problem since it made the platform driver think the ACPI video
driver will control the backlight and then use the newly added API to
remove ACPI video created backlight interface.
Thanks,
Aaron
> } else {
> status = acpi_bus_get_device(graphics_handle, &tmp_dev);
> if (ACPI_FAILURE(status)) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 12:19 PM, Matthew Garrett wrote: > On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote: >> On 06/15/2013 09:38 AM, Matthew Garrett wrote: >>> Well, Windows 8 will only use the ACPI backlight interface if the GPU >>> driver decides to, right? So the logic for deciding whether to remove >>> the ACPI backlight control or not should be left up to the GPU. There's >> >> I don't know this. From the document I googled, Microsoft suggests under >> win8, backlight should be controlled by the graphics driver for smooth >> brightness level change, instead of ACPI or other methods. So it is >> possible that OEM will not test the ACPI interface well and thus the >> interface is likely broken. I don't see why GPU driver has any better >> knowledge on which systems the firmware interface is broken or not. > > The vendor will presumably have tested that backlight control works - if > the GPU driver uses the ACPI interface and backlight control is broken, > then the vendor would fix it. I don't think GPU driver uses ACPI interface, that's why for some systems, ACPI interface is broken while the GPU's is not. > >>> no harm in refusing to expose a working method if there's another >>> working method, but there is harm in exposing a broken one and expecting >>> userspace to know the difference. >> >> BTW, the proposed solution is not meant to solve win8 problems alone, it >> should make solving other problems easy and make individual backlight >> control interface provider module independent with each other such as >> the platform drivers will not need to check if ACPI video driver will >> control backlight or not and can always create backlight interface(its >> default priority is lower that ACPI video driver's so will not be taken >> by user space by default, showing the same behavior of the current code). > > Sure, but it still requires the replacement of existing userspace. We > need to fix the problem with existing userspace, too. Yes. The problem is two way. First, some of the backlight interface privoder module provides a broken interface; Two, the userspace picked a broken interface while there are usable ones. For example, in the win8 thinkpad case, the ACPI video driver provides broken interface and the GPU driver provides usable interface, but userspace choose ACPI video's interface. To make things complicated, if we make the broken interface disappear in ACPI video module, then the platform driver thinkpad_acpi will start to provide another broken interface. I have to say, solve it in the GPU code is a clever way, it's just a little weird to me for a broken interface to be blacklisted, we have to do it in another module, not the broken module itself. > >> The current acpi_backlight=video/vendor kernel command line is pretty >> misleading, for laptops that do not have vendor backlight interface, >> adding acpi_backlight=vendor actually makes the system using the GPU's >> interface. Some laptops are using this switch to work around problems in >> ACPI video driver and users think they are using vendor interface. >> That's why I think we need a new command line as the >> backlight.force_interface=raw/firmware/platform. > > No, I think we need to fix the bugs that currently require users to pass > options. For ACPI video driver, since it relies on firmware to do the right thing, if the functionality is broken, then it is, there is hardly anything we can do...(not always, we can quirk in some cases, but there are cases we can do nothing). In this case, we can: 1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will not create backlight interface and userspace will not pick it; 2 Add that model in DMI table in video_detect.c, eliminating the need for that command line; 3 Add that model in DMI table in another module, let that module unregister backlight interface provided by ACPI video as is done in acer-wmi, asus-wmi, and i915 in this case; 3 Use the backlight section in xorg.conf to make X uses another backlight interface. This may not work for distros that use gsd-backlight-helper, which always prefers firmware. Which one do you prefer? > >> Instead of letting individual driver to make decisions on which >> backlight interface this system should use(either in platform driver as >> we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), >> I think we need a better and clear way to handle such things. For >> example, suppose an acer laptop: vendor does not support backlight, ACPI >> video's backlight interface is broken and GPU's works, to make it work, >> user will need to select acer-wmi modu
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 09:38 AM, Matthew Garrett wrote: > On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote: >> It's not easy to decide if they work or not sometimes, e.g. I came >> across a system that claims win8 in ACPI table and has an Intel GPU, >> while its ACPI video interface also works. With this patch, the working >> ACPI video interface is removed, while with the priority based solution, >> the GPU's interface priority gets higher, but the ACPI video interface >> still stays. > > Well, Windows 8 will only use the ACPI backlight interface if the GPU > driver decides to, right? So the logic for deciding whether to remove > the ACPI backlight control or not should be left up to the GPU. There's I don't know this. From the document I googled, Microsoft suggests under win8, backlight should be controlled by the graphics driver for smooth brightness level change, instead of ACPI or other methods. So it is possible that OEM will not test the ACPI interface well and thus the interface is likely broken. I don't see why GPU driver has any better knowledge on which systems the firmware interface is broken or not. > no harm in refusing to expose a working method if there's another > working method, but there is harm in exposing a broken one and expecting > userspace to know the difference. BTW, the proposed solution is not meant to solve win8 problems alone, it should make solving other problems easy and make individual backlight control interface provider module independent with each other such as the platform drivers will not need to check if ACPI video driver will control backlight or not and can always create backlight interface(its default priority is lower that ACPI video driver's so will not be taken by user space by default, showing the same behavior of the current code). The current acpi_backlight=video/vendor kernel command line is pretty misleading, for laptops that do not have vendor backlight interface, adding acpi_backlight=vendor actually makes the system using the GPU's interface. Some laptops are using this switch to work around problems in ACPI video driver and users think they are using vendor interface. That's why I think we need a new command line as the backlight.force_interface=raw/firmware/platform. Instead of letting individual driver to make decisions on which backlight interface this system should use(either in platform driver as we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), I think we need a better and clear way to handle such things. For example, suppose an acer laptop: vendor does not support backlight, ACPI video's backlight interface is broken and GPU's works, to make it work, user will need to select acer-wmi module while this module does not have anything to do with the functionality, but simply because it serves as the backlight manager for acer laptops. The above information and idea is formed while solving bugs reported in kernel bugzilla video component, the proposed solutin may not be good enough, but I hope we can find a better way to handle backlight problems. Thanks, Aaron
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 01:29 AM, Matthew Garrett wrote: > On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote: > >> What about a priority based solution? We can introduce a new field named >> priority to backlight_device and instead of calling another module's >> function like the unregister one here(which cause unnecessary module >> dependency), we only need to boost priority for its own interface. This >> field will be exported to sysfs, so user can change it during runtime >> too. And we can also introduce a new kernel command line as >> backlight.force_interface=raw/firmware/platform, to overcome the limited >> functionality provided by acpi_backlight=video/vendor, which does not >> involve GPU's interface. > > How would that work with existing userspace? User space tool will need to be updated to use this as stated in the gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel, for others we can add I think if the priority based solution is deemed useful. > >> And we can place the quirk code in backlight layer instead of individual >> backlight functionality provider module. Suppose we have a backlight >> manager there, for all win8 systems, we can boost the raw type's >> priority on its registration, so no need to add code in >> intel/amd/etc./'s GPU driver code. > > But we'd need to add code to every piece of userspace that currently > uses the backlight, right? Yes that's the case. > >> With priority based solution, all backlight control interfaces stay, >> the priority field is an indication given by kernel to user space. > > We shouldn't export interfaces if we don't expect them to work. It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Thanks, Aaron
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 12:19 PM, Matthew Garrett wrote: > On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote: >> On 06/15/2013 09:38 AM, Matthew Garrett wrote: >>> Well, Windows 8 will only use the ACPI backlight interface if the GPU >>> driver decides to, right? So the logic for deciding whether to remove >>> the ACPI backlight control or not should be left up to the GPU. There's >> >> I don't know this. From the document I googled, Microsoft suggests under >> win8, backlight should be controlled by the graphics driver for smooth >> brightness level change, instead of ACPI or other methods. So it is >> possible that OEM will not test the ACPI interface well and thus the >> interface is likely broken. I don't see why GPU driver has any better >> knowledge on which systems the firmware interface is broken or not. > > The vendor will presumably have tested that backlight control works - if > the GPU driver uses the ACPI interface and backlight control is broken, > then the vendor would fix it. I don't think GPU driver uses ACPI interface, that's why for some systems, ACPI interface is broken while the GPU's is not. > >>> no harm in refusing to expose a working method if there's another >>> working method, but there is harm in exposing a broken one and expecting >>> userspace to know the difference. >> >> BTW, the proposed solution is not meant to solve win8 problems alone, it >> should make solving other problems easy and make individual backlight >> control interface provider module independent with each other such as >> the platform drivers will not need to check if ACPI video driver will >> control backlight or not and can always create backlight interface(its >> default priority is lower that ACPI video driver's so will not be taken >> by user space by default, showing the same behavior of the current code). > > Sure, but it still requires the replacement of existing userspace. We > need to fix the problem with existing userspace, too. Yes. The problem is two way. First, some of the backlight interface privoder module provides a broken interface; Two, the userspace picked a broken interface while there are usable ones. For example, in the win8 thinkpad case, the ACPI video driver provides broken interface and the GPU driver provides usable interface, but userspace choose ACPI video's interface. To make things complicated, if we make the broken interface disappear in ACPI video module, then the platform driver thinkpad_acpi will start to provide another broken interface. I have to say, solve it in the GPU code is a clever way, it's just a little weird to me for a broken interface to be blacklisted, we have to do it in another module, not the broken module itself. > >> The current acpi_backlight=video/vendor kernel command line is pretty >> misleading, for laptops that do not have vendor backlight interface, >> adding acpi_backlight=vendor actually makes the system using the GPU's >> interface. Some laptops are using this switch to work around problems in >> ACPI video driver and users think they are using vendor interface. >> That's why I think we need a new command line as the >> backlight.force_interface=raw/firmware/platform. > > No, I think we need to fix the bugs that currently require users to pass > options. For ACPI video driver, since it relies on firmware to do the right thing, if the functionality is broken, then it is, there is hardly anything we can do...(not always, we can quirk in some cases, but there are cases we can do nothing). In this case, we can: 1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will not create backlight interface and userspace will not pick it; 2 Add that model in DMI table in video_detect.c, eliminating the need for that command line; 3 Add that model in DMI table in another module, let that module unregister backlight interface provided by ACPI video as is done in acer-wmi, asus-wmi, and i915 in this case; 3 Use the backlight section in xorg.conf to make X uses another backlight interface. This may not work for distros that use gsd-backlight-helper, which always prefers firmware. Which one do you prefer? > >> Instead of letting individual driver to make decisions on which >> backlight interface this system should use(either in platform driver as >> we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), >> I think we need a better and clear way to handle such things. For >> example, suppose an acer laptop: vendor does not support backlight, ACPI >> video's backlight interface is broken and GPU's works, to make it work, >> user will need to select acer-wmi modu
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 09:38 AM, Matthew Garrett wrote: > On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote: >> It's not easy to decide if they work or not sometimes, e.g. I came >> across a system that claims win8 in ACPI table and has an Intel GPU, >> while its ACPI video interface also works. With this patch, the working >> ACPI video interface is removed, while with the priority based solution, >> the GPU's interface priority gets higher, but the ACPI video interface >> still stays. > > Well, Windows 8 will only use the ACPI backlight interface if the GPU > driver decides to, right? So the logic for deciding whether to remove > the ACPI backlight control or not should be left up to the GPU. There's I don't know this. From the document I googled, Microsoft suggests under win8, backlight should be controlled by the graphics driver for smooth brightness level change, instead of ACPI or other methods. So it is possible that OEM will not test the ACPI interface well and thus the interface is likely broken. I don't see why GPU driver has any better knowledge on which systems the firmware interface is broken or not. > no harm in refusing to expose a working method if there's another > working method, but there is harm in exposing a broken one and expecting > userspace to know the difference. BTW, the proposed solution is not meant to solve win8 problems alone, it should make solving other problems easy and make individual backlight control interface provider module independent with each other such as the platform drivers will not need to check if ACPI video driver will control backlight or not and can always create backlight interface(its default priority is lower that ACPI video driver's so will not be taken by user space by default, showing the same behavior of the current code). The current acpi_backlight=video/vendor kernel command line is pretty misleading, for laptops that do not have vendor backlight interface, adding acpi_backlight=vendor actually makes the system using the GPU's interface. Some laptops are using this switch to work around problems in ACPI video driver and users think they are using vendor interface. That's why I think we need a new command line as the backlight.force_interface=raw/firmware/platform. Instead of letting individual driver to make decisions on which backlight interface this system should use(either in platform driver as we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), I think we need a better and clear way to handle such things. For example, suppose an acer laptop: vendor does not support backlight, ACPI video's backlight interface is broken and GPU's works, to make it work, user will need to select acer-wmi module while this module does not have anything to do with the functionality, but simply because it serves as the backlight manager for acer laptops. The above information and idea is formed while solving bugs reported in kernel bugzilla video component, the proposed solutin may not be good enough, but I hope we can find a better way to handle backlight problems. Thanks, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/15/2013 01:29 AM, Matthew Garrett wrote: > On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote: > >> What about a priority based solution? We can introduce a new field named >> priority to backlight_device and instead of calling another module's >> function like the unregister one here(which cause unnecessary module >> dependency), we only need to boost priority for its own interface. This >> field will be exported to sysfs, so user can change it during runtime >> too. And we can also introduce a new kernel command line as >> backlight.force_interface=raw/firmware/platform, to overcome the limited >> functionality provided by acpi_backlight=video/vendor, which does not >> involve GPU's interface. > > How would that work with existing userspace? User space tool will need to be updated to use this as stated in the gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel, for others we can add I think if the priority based solution is deemed useful. > >> And we can place the quirk code in backlight layer instead of individual >> backlight functionality provider module. Suppose we have a backlight >> manager there, for all win8 systems, we can boost the raw type's >> priority on its registration, so no need to add code in >> intel/amd/etc./'s GPU driver code. > > But we'd need to add code to every piece of userspace that currently > uses the backlight, right? Yes that's the case. > >> With priority based solution, all backlight control interfaces stay, >> the priority field is an indication given by kernel to user space. > > We shouldn't export interfaces if we don't expect them to work. It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Thanks, Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/10/2013 07:01 AM, Matthew Garrett wrote: > 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 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. > > Signed-off-by: Matthew Garrett > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 3b315ba..23b6292 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned > long flags) > /* Must be done after probing outputs */ > intel_opregion_init(dev); > acpi_video_register(); > + /* Don't use ACPI backlight functions on Windows 8 platforms */ > + if (acpi_osi_version() >= ACPI_OSI_WIN_8) > + acpi_video_backlight_unregister(); What about the platform driver created backlight interface? Oh right, thinkpad_acpi will not create backlight interface for them since they are not in DMI table, so acpi_video_backlight_support() will return true and thinkspad_acpi thinks ACPI video driver is controlling backlight so it will not create backlight interface for them. Then we will need to remember not to add any of those systems into the DMI table of video_detect.c, or thinkpad_acpi module will create backlight interface for them and according to reporter, that interface does not work either. What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. For a complete description on backlight control background and the proposed solution, please take a look at: https://gist.github.com/aaronlu/5779828 It would be good to know your opinions, thanks. -Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On 06/10/2013 07:01 AM, Matthew Garrett wrote: > 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 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. > > Signed-off-by: Matthew Garrett > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 3b315ba..23b6292 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned > long flags) > /* Must be done after probing outputs */ > intel_opregion_init(dev); > acpi_video_register(); > + /* Don't use ACPI backlight functions on Windows 8 platforms */ > + if (acpi_osi_version() >= ACPI_OSI_WIN_8) > + acpi_video_backlight_unregister(); What about the platform driver created backlight interface? Oh right, thinkpad_acpi will not create backlight interface for them since they are not in DMI table, so acpi_video_backlight_support() will return true and thinkspad_acpi thinks ACPI video driver is controlling backlight so it will not create backlight interface for them. Then we will need to remember not to add any of those systems into the DMI table of video_detect.c, or thinkpad_acpi module will create backlight interface for them and according to reporter, that interface does not work either. What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. For a complete description on backlight control background and the proposed solution, please take a look at: https://gist.github.com/aaronlu/5779828 It would be good to know your opinions, thanks. -Aaron
New prototype computer problem with S3 suspend
On 06/07/2013 02:11 AM, Boszormenyi Zoltan wrote: > Hi, > > we are working on an Intel Atom-based embedded PC and I have to > make suspend-to-RAM work but I can't seem to succeed. > > The symptom is that quite often, the machine resumes immediately > after pm-suspend. Sometimes more than 20 times out of 50 attempts. Can you please file a bug about this? https://bugzilla.kernel.org > > I have tried 3.7.10, 3.9.4, 3.10-rc[234] and the linux-next branch from the > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git repository. > The attached dmesg is from today's linux-pm/linux-next plus the > latest drm-fixes patchset posted by Dave Airlie. > > I have tried disabling wakeup devices via /proc/acpi/wakeup and via sysfs > files. > (/sys/devices/.../wakeup) >From the dmesg, the following three devices are still armed with wakeup capability and might be the cause: i8042 kbd 00:03: System wakeup enabled by ACPI PM: suspend of devices complete after 578.883 msecs PM: late suspend of devices complete after 0.279 msecs pcieport :00:1c.1: System wakeup enabled by ACPI ehci-pci :00:1d.7: System wakeup enabled by ACPI PM: noirq suspend of devices complete after 31.946 msecs Anyway, please file a bug there, thanks. -Aaron
Re: New prototype computer problem with S3 suspend
On 06/07/2013 02:11 AM, Boszormenyi Zoltan wrote: > Hi, > > we are working on an Intel Atom-based embedded PC and I have to > make suspend-to-RAM work but I can't seem to succeed. > > The symptom is that quite often, the machine resumes immediately > after pm-suspend. Sometimes more than 20 times out of 50 attempts. Can you please file a bug about this? https://bugzilla.kernel.org > > I have tried 3.7.10, 3.9.4, 3.10-rc[234] and the linux-next branch from the > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git repository. > The attached dmesg is from today's linux-pm/linux-next plus the > latest drm-fixes patchset posted by Dave Airlie. > > I have tried disabling wakeup devices via /proc/acpi/wakeup and via sysfs > files. > (/sys/devices/.../wakeup) >From the dmesg, the following three devices are still armed with wakeup capability and might be the cause: i8042 kbd 00:03: System wakeup enabled by ACPI PM: suspend of devices complete after 578.883 msecs PM: late suspend of devices complete after 0.279 msecs pcieport :00:1c.1: System wakeup enabled by ACPI ehci-pci :00:1d.7: System wakeup enabled by ACPI PM: noirq suspend of devices complete after 31.946 msecs Anyway, please file a bug there, thanks. -Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
