Re: [PATCH 5/5] acpi_video: Don't handle ACPI brightness notifications by default

[Aaron Lu <aaron.lu@intel.com>]

On 03/08/2013 03:39 AM, Seth Forshee wrote:
> Windows 8 requires all backlight interfaces to report 101 brightness
> values, and as a result we're starting to see machines with that many
> brightness levels in _BCL. For machines which send these notifications
> when the brightness up/down keys are pressed this means a lot of key
> presses to get any kind of noticeable change in brightness.
> 
> For a while now we've had the ability to disable in-kernel handling of
> notifications via the video.brightness_switch_enabled parameter. Change
> this to default to off, and let userspace choose more reasonable
> increments for changing the brightness.

I just found one more reason for this param to default 0.

We are going to separate the backlight interface control and event
notification functionalities of the ACPI video module, it is highly
possible a lot of systems will use a combination of the event
notification handler and intel_backlight interface. So it doesn't make
sense to let video module to do any adjustment on its own if user space
has chosen a different interface to use. Actually, it can cause problems
as in ASUS's case:
https://bugzilla.kernel.org/show_bug.cgi?id=52951

The problem there is, on hotkey brightness up, the video module will
adjust the brightness level first and since its _BQC is broken, it gets
a wrong number(too low or too high or whatever) and then does the _BCM
call. The _BCM method works. Then user space picks the intel_backlight
to do the adjustment, but since the _BCM already sets a wrong value, the
user space's adjustment is affected too. The result is, user has only
two visible levels, very low and very high.

This only occurs on -rc3, since we removed the
acpi_video_verify_backlight_support from acpi_video_switch_brightness
function. So either we make this param default to 0, or we make a new
function to avoid brightness switch in video module for Win8 systems.

I prefer to set this param to 0 by default. What do you guys think?

Thanks,
Aaron

> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  drivers/acpi/video.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 6a19bf7..431b22e 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -69,7 +69,7 @@ MODULE_AUTHOR("Bruno Ducrot");
>  MODULE_DESCRIPTION("ACPI Video Driver");
>  MODULE_LICENSE("GPL");
>  
> -static bool brightness_switch_enabled = 1;
> +static bool brightness_switch_enabled = 0;
>  module_param(brightness_switch_enabled, bool, 0644);
>  
>  /*
>