ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 4353 bytes --]

Hello,

after big changes in acpi video/i915 code I cannot change display 
brightness on my Dell Latitude E6440 with kernel 3.17-rc6. With 
kernel 3.13 everything worked fine.

More information about this problem:

For configuring brightness on Dell laptops there are 4 ways:
1) via acpi video driver
2) via dell-laptop driver
3) via i915 drm driver
4) from userspace with special dell SMI call
   (e.g with program dellLcdBrightness from libsmbios package)

Methods 2) and 4) are same, both making special SMI call and Bios 
handing this request (just 2 is from kernel and 4 from userspace)

Method 1) via acpi video driver working, but is not perfect. 
Driver can be used to change brightness (but only some levels, 
probably this depends on acpi/DSDT tables), but cannot be used to 
retrieve current brightness (when BIOS/SMI change brightness acpi 
driver report old incorrect value). So I prefer dell-laptop 
driver instead acpi video.

Method 3) working even with 3.17-rc6 kernel but because that 
backlight device exported by i915 is marked as raw, desktop 
programs prefer to use other devices.

Moreover it looks like that methods 1) 2) and 4) just forward 
request to method 3). So in any cased brightness is changed by 
i915 drm driver.

I'm not sure (correct me if I'm wrong!) but I think that intel 
i915 drm driver accept changes (file intel_opregion.c) only if 
acpi function acpi_video_verify_backlight_support() returns true.

Function acpi_video_verify_backlight_support() returns true iff:
function acpi_video_backlight_support() returns true AND at least 
one of these functions returns false:
acpi_osi_is_win8()
acpi_video_use_native_backlight() 
backlight_device_registered(BACKLIGHT_RAW)

On my notebook acpi_osi_is_win8() returns true (as is win8 
compliant), backlight_device_registered(BACKLIGHT_RAW) returns 
true as I'm using intel i915 drm driver with raw backlight device 
and acpi_video_use_native_backlight() returns true/false 
depending on "video.use_native_backlight" kernel param. Default 
is true.

So if I want to have working acpi video driver with display 
brightness support I need to boot kernel with param: 
"video.use_native_backlight=0". I tested it with kernel 3.17-rc6 
and this param really enabled display brightness support via acpi 
video driver -- which is good.

Driver dell-laptop creating backligh device for brightness 
control only if acpi_video_backlight_support() returns false. 
There is complicated condition for it and when kernel is booted 
with "video.use_native_backlight=0" that function returns true.

So conclusion is: With current code in kernel 3.17-rc6 it is not 
possible to control brightness of display with native driver 
dell-laptop on Dell Latitude E6440 (and probably on others 
too)!!!

And Because laptop is win8 compliant and you create decision to 
use native driver (instead acpi one) it is not possible to 
control display brightness without tweeks in kernel cmdline.

As I wrote I would rather to use native dell-laptop driver for 
controlling brightness, but it is not possible.

So how to solve this problem?

Quick solution would be to set use_native_backlight false for 
some Dell laptops which means, that acpi video will be used and 
in this case intel i915 driver will *not* drop backlight change 
request.

Another solution could be to disable check in dell_laptop driver 
and add use_native_backlight=0 to hooks. But this create two 
backlight interfaces (which is not good), but only way (for now) 
how to make dell_laptop working again.

Better and maybe only one proper solution would be to teach intel 
drm i915 driver to not drop backlight change request for Dell 
laptops (or all??). (This allows to work both acpi video and 
dell_laptop drivers without any change and with *any* value in 
param use_native_backlight). I think that problematic code is in 
function asle_set_backlight() in file intel_opregion.c (but I'm 
not sure). My idea is that "drop" event function it is caused by 
this commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not 
sure).

At least something must be done as I think that I'm not only one 
who has Dell laptop and brightness configuration is really 
important...

Thanks.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/23/2014 10:06 PM, Pali Rohár wrote:
> Hello,
>
> after big changes in acpi video/i915 code I cannot change display
> brightness on my Dell Latitude E6440 with kernel 3.17-rc6. With
> kernel 3.13 everything worked fine.
>
> More information about this problem:
>
> For configuring brightness on Dell laptops there are 4 ways:
> 1) via acpi video driver
> 2) via dell-laptop driver
> 3) via i915 drm driver
> 4) from userspace with special dell SMI call
>     (e.g with program dellLcdBrightness from libsmbios package)
>
> Methods 2) and 4) are same, both making special SMI call and Bios
> handing this request (just 2 is from kernel and 4 from userspace)
>
> Method 1) via acpi video driver working, but is not perfect.
> Driver can be used to change brightness (but only some levels,
> probably this depends on acpi/DSDT tables), but cannot be used to
> retrieve current brightness (when BIOS/SMI change brightness acpi
> driver report old incorrect value). So I prefer dell-laptop
> driver instead acpi video.
>
> Method 3) working even with 3.17-rc6 kernel but because that
> backlight device exported by i915 is marked as raw, desktop
> programs prefer to use other devices.
>
> Moreover it looks like that methods 1) 2) and 4) just forward
> request to method 3). So in any cased brightness is changed by
> i915 drm driver.
>
> I'm not sure (correct me if I'm wrong!) but I think that intel
> i915 drm driver accept changes (file intel_opregion.c) only if
> acpi function acpi_video_verify_backlight_support() returns true.
>
> Function acpi_video_verify_backlight_support() returns true iff:
> function acpi_video_backlight_support() returns true AND at least
> one of these functions returns false:
> acpi_osi_is_win8()
> acpi_video_use_native_backlight()
> backlight_device_registered(BACKLIGHT_RAW)
>
> On my notebook acpi_osi_is_win8() returns true (as is win8
> compliant), backlight_device_registered(BACKLIGHT_RAW) returns
> true as I'm using intel i915 drm driver with raw backlight device
> and acpi_video_use_native_backlight() returns true/false
> depending on "video.use_native_backlight" kernel param. Default
> is true.
>
> So if I want to have working acpi video driver with display
> brightness support I need to boot kernel with param:
> "video.use_native_backlight=0". I tested it with kernel 3.17-rc6
> and this param really enabled display brightness support via acpi
> video driver -- which is good.
>
> Driver dell-laptop creating backligh device for brightness
> control only if acpi_video_backlight_support() returns false.
> There is complicated condition for it and when kernel is booted
> with "video.use_native_backlight=0" that function returns true.
>
> So conclusion is: With current code in kernel 3.17-rc6 it is not
> possible to control brightness of display with native driver
> dell-laptop on Dell Latitude E6440 (and probably on others
> too)!!!
>
> And Because laptop is win8 compliant and you create decision to
> use native driver (instead acpi one) it is not possible to
> control display brightness without tweeks in kernel cmdline.
>
> As I wrote I would rather to use native dell-laptop driver for
> controlling brightness, but it is not possible.
>
> So how to solve this problem?
>
> Quick solution would be to set use_native_backlight false for
> some Dell laptops which means, that acpi video will be used and
> in this case intel i915 driver will *not* drop backlight change
> request.
>
> Another solution could be to disable check in dell_laptop driver
> and add use_native_backlight=0 to hooks. But this create two
> backlight interfaces (which is not good), but only way (for now)
> how to make dell_laptop working again.
>
> Better and maybe only one proper solution would be to teach intel
> drm i915 driver to not drop backlight change request for Dell
> laptops (or all??). (This allows to work both acpi video and
> dell_laptop drivers without any change and with *any* value in
> param use_native_backlight). I think that problematic code is in
> function asle_set_backlight() in file intel_opregion.c (but I'm
> not sure). My idea is that "drop" event function it is caused by
> this commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> sure).
>
> At least something must be done as I think that I'm not only one
> who has Dell laptop and brightness configuration is really
> important...

I don't understand your problem, the kernel is selecting the
i915 backlight driver, making that the only one available to
userspace, so the one problem you state with the i915 driver, that
it is "raw" is not an issue, as userspace will pick it when it is
the only one.

Why would you want to use dell-laptop despite the i915 driver
working fine ?

Regards,

Hans

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 6084 bytes --]

On Tuesday 23 September 2014 22:31:31 you wrote:
> Hi,
> 
> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> > Hello,
> > 
> > after big changes in acpi video/i915 code I cannot change
> > display brightness on my Dell Latitude E6440 with kernel
> > 3.17-rc6. With kernel 3.13 everything worked fine.
> > 
> > More information about this problem:
> > 
> > For configuring brightness on Dell laptops there are 4 ways:
> > 1) via acpi video driver
> > 2) via dell-laptop driver
> > 3) via i915 drm driver
> > 4) from userspace with special dell SMI call
> > 
> >     (e.g with program dellLcdBrightness from libsmbios
> >     package)
> > 
> > Methods 2) and 4) are same, both making special SMI call and
> > Bios handing this request (just 2 is from kernel and 4 from
> > userspace)
> > 
> > Method 1) via acpi video driver working, but is not perfect.
> > Driver can be used to change brightness (but only some
> > levels, probably this depends on acpi/DSDT tables), but
> > cannot be used to retrieve current brightness (when
> > BIOS/SMI change brightness acpi driver report old incorrect
> > value). So I prefer dell-laptop driver instead acpi video.
> > 
> > Method 3) working even with 3.17-rc6 kernel but because that
> > backlight device exported by i915 is marked as raw, desktop
> > programs prefer to use other devices.
> > 
> > Moreover it looks like that methods 1) 2) and 4) just
> > forward request to method 3). So in any cased brightness is
> > changed by i915 drm driver.
> > 
> > I'm not sure (correct me if I'm wrong!) but I think that
> > intel i915 drm driver accept changes (file
> > intel_opregion.c) only if acpi function
> > acpi_video_verify_backlight_support() returns true.
> > 
> > Function acpi_video_verify_backlight_support() returns true
> > iff: function acpi_video_backlight_support() returns true
> > AND at least one of these functions returns false:
> > acpi_osi_is_win8()
> > acpi_video_use_native_backlight()
> > backlight_device_registered(BACKLIGHT_RAW)
> > 
> > On my notebook acpi_osi_is_win8() returns true (as is win8
> > compliant), backlight_device_registered(BACKLIGHT_RAW)
> > returns true as I'm using intel i915 drm driver with raw
> > backlight device and acpi_video_use_native_backlight()
> > returns true/false depending on
> > "video.use_native_backlight" kernel param. Default is true.
> > 
> > So if I want to have working acpi video driver with display
> > brightness support I need to boot kernel with param:
> > "video.use_native_backlight=0". I tested it with kernel
> > 3.17-rc6 and this param really enabled display brightness
> > support via acpi video driver -- which is good.
> > 
> > Driver dell-laptop creating backligh device for brightness
> > control only if acpi_video_backlight_support() returns
> > false. There is complicated condition for it and when
> > kernel is booted with "video.use_native_backlight=0" that
> > function returns true.
> > 
> > So conclusion is: With current code in kernel 3.17-rc6 it is
> > not possible to control brightness of display with native
> > driver dell-laptop on Dell Latitude E6440 (and probably on
> > others too)!!!
> > 
> > And Because laptop is win8 compliant and you create decision
> > to use native driver (instead acpi one) it is not possible
> > to control display brightness without tweeks in kernel
> > cmdline.
> > 
> > As I wrote I would rather to use native dell-laptop driver
> > for controlling brightness, but it is not possible.
> > 
> > So how to solve this problem?
> > 
> > Quick solution would be to set use_native_backlight false
> > for some Dell laptops which means, that acpi video will be
> > used and in this case intel i915 driver will *not* drop
> > backlight change request.
> > 
> > Another solution could be to disable check in dell_laptop
> > driver and add use_native_backlight=0 to hooks. But this
> > create two backlight interfaces (which is not good), but
> > only way (for now) how to make dell_laptop working again.
> > 
> > Better and maybe only one proper solution would be to teach
> > intel drm i915 driver to not drop backlight change request
> > for Dell laptops (or all??). (This allows to work both acpi
> > video and dell_laptop drivers without any change and with
> > *any* value in param use_native_backlight). I think that
> > problematic code is in function asle_set_backlight() in
> > file intel_opregion.c (but I'm not sure). My idea is that
> > "drop" event function it is caused by this commit
> > 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> > sure).
> > 
> > At least something must be done as I think that I'm not only
> > one who has Dell laptop and brightness configuration is
> > really important...
> 
> I don't understand your problem, the kernel is selecting the
> i915 backlight driver, making that the only one available to
> userspace, so the one problem you state with the i915 driver,
> that it is "raw" is not an issue, as userspace will pick it
> when it is the only one.
> 

It is not only one. Also dell-laptop (or acpi video) backlight 
interface is created (depends on use_native_backlight param). And 
userspace will pick dell-laptop (or acpi video) to use (which 
cannot change brightness).

> Why would you want to use dell-laptop despite the i915 driver
> working fine ?
> 

i915 working only if I compile kernel without acpi video dell-
laptop support (distributions compile kernel with these drivers) 
and i915 is not good for usage. First it provides more then 
thousand brightness levels and lot of (with low numbers) 
completely turn display off. And if userspace map these thousand 
levels into 5-10 levels (as nobody want to press brightness key 
up/down 1000x) two lowest levels cause display off. With acpi 
video and dell-laptop driver levels are mapped into small level 
space in perfect way. So this is reason I want to use dell-laptop 
for controlling brightness.

> Regards,
> 
> Hans

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/23/2014 10:44 PM, Pali Rohár wrote:
> On Tuesday 23 September 2014 22:31:31 you wrote:
>> Hi,
>>
>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>> Hello,
>>>
>>> after big changes in acpi video/i915 code I cannot change
>>> display brightness on my Dell Latitude E6440 with kernel
>>> 3.17-rc6. With kernel 3.13 everything worked fine.
>>>
>>> More information about this problem:
>>>
>>> For configuring brightness on Dell laptops there are 4 ways:
>>> 1) via acpi video driver
>>> 2) via dell-laptop driver
>>> 3) via i915 drm driver
>>> 4) from userspace with special dell SMI call
>>>
>>>     (e.g with program dellLcdBrightness from libsmbios
>>>     package)
>>>
>>> Methods 2) and 4) are same, both making special SMI call and
>>> Bios handing this request (just 2 is from kernel and 4 from
>>> userspace)
>>>
>>> Method 1) via acpi video driver working, but is not perfect.
>>> Driver can be used to change brightness (but only some
>>> levels, probably this depends on acpi/DSDT tables), but
>>> cannot be used to retrieve current brightness (when
>>> BIOS/SMI change brightness acpi driver report old incorrect
>>> value). So I prefer dell-laptop driver instead acpi video.
>>>
>>> Method 3) working even with 3.17-rc6 kernel but because that
>>> backlight device exported by i915 is marked as raw, desktop
>>> programs prefer to use other devices.
>>>
>>> Moreover it looks like that methods 1) 2) and 4) just
>>> forward request to method 3). So in any cased brightness is
>>> changed by i915 drm driver.
>>>
>>> I'm not sure (correct me if I'm wrong!) but I think that
>>> intel i915 drm driver accept changes (file
>>> intel_opregion.c) only if acpi function
>>> acpi_video_verify_backlight_support() returns true.
>>>
>>> Function acpi_video_verify_backlight_support() returns true
>>> iff: function acpi_video_backlight_support() returns true
>>> AND at least one of these functions returns false:
>>> acpi_osi_is_win8()
>>> acpi_video_use_native_backlight()
>>> backlight_device_registered(BACKLIGHT_RAW)
>>>
>>> On my notebook acpi_osi_is_win8() returns true (as is win8
>>> compliant), backlight_device_registered(BACKLIGHT_RAW)
>>> returns true as I'm using intel i915 drm driver with raw
>>> backlight device and acpi_video_use_native_backlight()
>>> returns true/false depending on
>>> "video.use_native_backlight" kernel param. Default is true.
>>>
>>> So if I want to have working acpi video driver with display
>>> brightness support I need to boot kernel with param:
>>> "video.use_native_backlight=0". I tested it with kernel
>>> 3.17-rc6 and this param really enabled display brightness
>>> support via acpi video driver -- which is good.
>>>
>>> Driver dell-laptop creating backligh device for brightness
>>> control only if acpi_video_backlight_support() returns
>>> false. There is complicated condition for it and when
>>> kernel is booted with "video.use_native_backlight=0" that
>>> function returns true.
>>>
>>> So conclusion is: With current code in kernel 3.17-rc6 it is
>>> not possible to control brightness of display with native
>>> driver dell-laptop on Dell Latitude E6440 (and probably on
>>> others too)!!!
>>>
>>> And Because laptop is win8 compliant and you create decision
>>> to use native driver (instead acpi one) it is not possible
>>> to control display brightness without tweeks in kernel
>>> cmdline.
>>>
>>> As I wrote I would rather to use native dell-laptop driver
>>> for controlling brightness, but it is not possible.
>>>
>>> So how to solve this problem?
>>>
>>> Quick solution would be to set use_native_backlight false
>>> for some Dell laptops which means, that acpi video will be
>>> used and in this case intel i915 driver will *not* drop
>>> backlight change request.
>>>
>>> Another solution could be to disable check in dell_laptop
>>> driver and add use_native_backlight=0 to hooks. But this
>>> create two backlight interfaces (which is not good), but
>>> only way (for now) how to make dell_laptop working again.
>>>
>>> Better and maybe only one proper solution would be to teach
>>> intel drm i915 driver to not drop backlight change request
>>> for Dell laptops (or all??). (This allows to work both acpi
>>> video and dell_laptop drivers without any change and with
>>> *any* value in param use_native_backlight). I think that
>>> problematic code is in function asle_set_backlight() in
>>> file intel_opregion.c (but I'm not sure). My idea is that
>>> "drop" event function it is caused by this commit
>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>> sure).
>>>
>>> At least something must be done as I think that I'm not only
>>> one who has Dell laptop and brightness configuration is
>>> really important...
>>
>> I don't understand your problem, the kernel is selecting the
>> i915 backlight driver, making that the only one available to
>> userspace, so the one problem you state with the i915 driver,
>> that it is "raw" is not an issue, as userspace will pick it
>> when it is the only one.
>>
> 
> It is not only one.

Are you sure, if you do not specify any commandline parameters, then
neither dell-laptop nor acpi-video should register a backlight interface.

dell-laptop.c has:

#ifdef CONFIG_ACPI
        /* In the event of an ACPI backlight being available, don't
         * register the platform controller.
         */
        if (acpi_video_backlight_support())
                return 0;
#endif

And acpi_video_backlight_support() will (normally) return true on
acpi-backlight capable systems independent of use_native_backlight.

And drivers/acpi/video.c has this:

        /* no warning message if acpi_backlight=vendor or a quirk is used */
        if (!acpi_video_verify_backlight_support())
                return;

...

bool acpi_video_verify_backlight_support(void)
{
        if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
            backlight_device_registered(BACKLIGHT_RAW))
                return false;
        return acpi_video_backlight_support();
}

So unlike the check in dell-laptop this one does depend on the
use_native_backlight setting.

I've just checked 3.17 on my E6430 and there this works as it should,
I only get the intel_backlight in /sys/class/backlight

> Also dell-laptop (or acpi video) backlight 
> interface is created (depends on use_native_backlight param). And 
> userspace will pick dell-laptop (or acpi video) to use (which 
> cannot change brightness).
> 
>> Why would you want to use dell-laptop despite the i915 driver
>> working fine ?
>>
> 
> i915 working only if I compile kernel without acpi video dell-
> laptop support (distributions compile kernel with these drivers) 
> and i915 is not good for usage. First it provides more then 
> thousand brightness levels and lot of (with low numbers) 
> completely turn display off. And if userspace map these thousand 
> levels into 5-10 levels (as nobody want to press brightness key 
> up/down 1000x) two lowest levels cause display off.

More and more laptops will only have working backlight control at all
when using i915, so userspace will need to learn to better deal with
backlight controls with these ranges. And low pwm levels turning
the backlight of is normal for raw interfaces, if userspace does
not want this, then they should not go so low.

I suggest that you file a bug against your desktop environment
that it is not using the backlight controls in an optimal manner,
from the kernel pov everything is working fine.

> With acpi 
> video and dell-laptop driver levels are mapped into small level 
> space in perfect way. So this is reason I want to use dell-laptop 
> for controlling brightness.

If you want to use dell-laptop, specify acpi_backlight=vendor on
the kernel commandline, that will give you dell_laptop +
intel_backlight as backlight interfaces, and userspace should prefer
dell_laptop. But IMHO it would be better to file a bug with your
desktop environment, and get that fixed to work properly with
intel_backlight (or with raw backlight interfaces in general).

Regards,

Hans



> 
>> Regards,
>>
>> Hans
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/23/2014 10:44 PM, Pali Rohár wrote:
> On Tuesday 23 September 2014 22:31:31 you wrote:
>> Hi,
>>
>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>> Hello,
>>>
>>> after big changes in acpi video/i915 code I cannot change
>>> display brightness on my Dell Latitude E6440 with kernel
>>> 3.17-rc6. With kernel 3.13 everything worked fine.
>>>
>>> More information about this problem:
>>>
>>> For configuring brightness on Dell laptops there are 4 ways:
>>> 1) via acpi video driver
>>> 2) via dell-laptop driver
>>> 3) via i915 drm driver
>>> 4) from userspace with special dell SMI call
>>>
>>>     (e.g with program dellLcdBrightness from libsmbios
>>>     package)
>>>
>>> Methods 2) and 4) are same, both making special SMI call and
>>> Bios handing this request (just 2 is from kernel and 4 from
>>> userspace)
>>>
>>> Method 1) via acpi video driver working, but is not perfect.
>>> Driver can be used to change brightness (but only some
>>> levels, probably this depends on acpi/DSDT tables), but
>>> cannot be used to retrieve current brightness (when
>>> BIOS/SMI change brightness acpi driver report old incorrect
>>> value). So I prefer dell-laptop driver instead acpi video.
>>>
>>> Method 3) working even with 3.17-rc6 kernel but because that
>>> backlight device exported by i915 is marked as raw, desktop
>>> programs prefer to use other devices.
>>>
>>> Moreover it looks like that methods 1) 2) and 4) just
>>> forward request to method 3). So in any cased brightness is
>>> changed by i915 drm driver.
>>>
>>> I'm not sure (correct me if I'm wrong!) but I think that
>>> intel i915 drm driver accept changes (file
>>> intel_opregion.c) only if acpi function
>>> acpi_video_verify_backlight_support() returns true.
>>>
>>> Function acpi_video_verify_backlight_support() returns true
>>> iff: function acpi_video_backlight_support() returns true
>>> AND at least one of these functions returns false:
>>> acpi_osi_is_win8()
>>> acpi_video_use_native_backlight()
>>> backlight_device_registered(BACKLIGHT_RAW)
>>>
>>> On my notebook acpi_osi_is_win8() returns true (as is win8
>>> compliant), backlight_device_registered(BACKLIGHT_RAW)
>>> returns true as I'm using intel i915 drm driver with raw
>>> backlight device and acpi_video_use_native_backlight()
>>> returns true/false depending on
>>> "video.use_native_backlight" kernel param. Default is true.
>>>
>>> So if I want to have working acpi video driver with display
>>> brightness support I need to boot kernel with param:
>>> "video.use_native_backlight=0". I tested it with kernel
>>> 3.17-rc6 and this param really enabled display brightness
>>> support via acpi video driver -- which is good.
>>>
>>> Driver dell-laptop creating backligh device for brightness
>>> control only if acpi_video_backlight_support() returns
>>> false. There is complicated condition for it and when
>>> kernel is booted with "video.use_native_backlight=0" that
>>> function returns true.
>>>
>>> So conclusion is: With current code in kernel 3.17-rc6 it is
>>> not possible to control brightness of display with native
>>> driver dell-laptop on Dell Latitude E6440 (and probably on
>>> others too)!!!
>>>
>>> And Because laptop is win8 compliant and you create decision
>>> to use native driver (instead acpi one) it is not possible
>>> to control display brightness without tweeks in kernel
>>> cmdline.
>>>
>>> As I wrote I would rather to use native dell-laptop driver
>>> for controlling brightness, but it is not possible.
>>>
>>> So how to solve this problem?
>>>
>>> Quick solution would be to set use_native_backlight false
>>> for some Dell laptops which means, that acpi video will be
>>> used and in this case intel i915 driver will *not* drop
>>> backlight change request.
>>>
>>> Another solution could be to disable check in dell_laptop
>>> driver and add use_native_backlight=0 to hooks. But this
>>> create two backlight interfaces (which is not good), but
>>> only way (for now) how to make dell_laptop working again.
>>>
>>> Better and maybe only one proper solution would be to teach
>>> intel drm i915 driver to not drop backlight change request
>>> for Dell laptops (or all??). (This allows to work both acpi
>>> video and dell_laptop drivers without any change and with
>>> *any* value in param use_native_backlight). I think that
>>> problematic code is in function asle_set_backlight() in
>>> file intel_opregion.c (but I'm not sure). My idea is that
>>> "drop" event function it is caused by this commit
>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>> sure).
>>>
>>> At least something must be done as I think that I'm not only
>>> one who has Dell laptop and brightness configuration is
>>> really important...
>>
>> I don't understand your problem, the kernel is selecting the
>> i915 backlight driver, making that the only one available to
>> userspace, so the one problem you state with the i915 driver,
>> that it is "raw" is not an issue, as userspace will pick it
>> when it is the only one.
>>
> 
> It is not only one.

Are you sure, if you do not specify any commandline parameters, then
neither dell-laptop nor acpi-video should register a backlight interface.

dell-laptop.c has:

#ifdef CONFIG_ACPI
        /* In the event of an ACPI backlight being available, don't
         * register the platform controller.
         */
        if (acpi_video_backlight_support())
                return 0;
#endif

And acpi_video_backlight_support() will (normally) return true on
acpi-backlight capable systems independent of use_native_backlight.

And drivers/acpi/video.c has this:

        /* no warning message if acpi_backlight=vendor or a quirk is used */
        if (!acpi_video_verify_backlight_support())
                return;

...

bool acpi_video_verify_backlight_support(void)
{
        if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
            backlight_device_registered(BACKLIGHT_RAW))
                return false;
        return acpi_video_backlight_support();
}

So unlike the check in dell-laptop this one does depend on the
use_native_backlight setting.

I've just checked 3.17 on my E6430 and there this works as it should,
I only get the intel_backlight in /sys/class/backlight

> Also dell-laptop (or acpi video) backlight 
> interface is created (depends on use_native_backlight param). And 
> userspace will pick dell-laptop (or acpi video) to use (which 
> cannot change brightness).
> 
>> Why would you want to use dell-laptop despite the i915 driver
>> working fine ?
>>
> 
> i915 working only if I compile kernel without acpi video dell-
> laptop support (distributions compile kernel with these drivers) 
> and i915 is not good for usage. First it provides more then 
> thousand brightness levels and lot of (with low numbers) 
> completely turn display off. And if userspace map these thousand 
> levels into 5-10 levels (as nobody want to press brightness key 
> up/down 1000x) two lowest levels cause display off.

More and more laptops will only have working backlight control at all
when using i915, so userspace will need to learn to better deal with
backlight controls with these ranges. And low pwm levels turning
the backlight of is normal for raw interfaces, if userspace does
not want this, then they should not go so low.

I suggest that you file a bug against your desktop environment
that it is not using the backlight controls in an optimal manner,
from the kernel pov everything is working fine.

> With acpi 
> video and dell-laptop driver levels are mapped into small level 
> space in perfect way. So this is reason I want to use dell-laptop 
> for controlling brightness.

If you want to use dell-laptop, specify acpi_backlight=vendor on
the kernel commandline, that will give you dell_laptop +
intel_backlight as backlight interfaces, and userspace should prefer
dell_laptop. But IMHO it would be better to file a bug with your
desktop environment, and get that fixed to work properly with
intel_backlight (or with raw backlight interfaces in general).

Regards,

Hans



> 
>> Regards,
>>
>> Hans
> 

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 9487 bytes --]

On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
> Hi,
> 
> On 09/23/2014 10:44 PM, Pali Rohár wrote:
> > On Tuesday 23 September 2014 22:31:31 you wrote:
> >> Hi,
> >> 
> >> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> >>> Hello,
> >>> 
> >>> after big changes in acpi video/i915 code I cannot change
> >>> display brightness on my Dell Latitude E6440 with kernel
> >>> 3.17-rc6. With kernel 3.13 everything worked fine.
> >>> 
> >>> More information about this problem:
> >>> 
> >>> For configuring brightness on Dell laptops there are 4
> >>> ways: 1) via acpi video driver
> >>> 2) via dell-laptop driver
> >>> 3) via i915 drm driver
> >>> 4) from userspace with special dell SMI call
> >>> 
> >>>     (e.g with program dellLcdBrightness from libsmbios
> >>>     package)
> >>> 
> >>> Methods 2) and 4) are same, both making special SMI call
> >>> and Bios handing this request (just 2 is from kernel and
> >>> 4 from userspace)
> >>> 
> >>> Method 1) via acpi video driver working, but is not
> >>> perfect. Driver can be used to change brightness (but
> >>> only some levels, probably this depends on acpi/DSDT
> >>> tables), but cannot be used to retrieve current
> >>> brightness (when BIOS/SMI change brightness acpi driver
> >>> report old incorrect value). So I prefer dell-laptop
> >>> driver instead acpi video.
> >>> 
> >>> Method 3) working even with 3.17-rc6 kernel but because
> >>> that backlight device exported by i915 is marked as raw,
> >>> desktop programs prefer to use other devices.
> >>> 
> >>> Moreover it looks like that methods 1) 2) and 4) just
> >>> forward request to method 3). So in any cased brightness
> >>> is changed by i915 drm driver.
> >>> 
> >>> I'm not sure (correct me if I'm wrong!) but I think that
> >>> intel i915 drm driver accept changes (file
> >>> intel_opregion.c) only if acpi function
> >>> acpi_video_verify_backlight_support() returns true.
> >>> 
> >>> Function acpi_video_verify_backlight_support() returns
> >>> true iff: function acpi_video_backlight_support() returns
> >>> true AND at least one of these functions returns false:
> >>> acpi_osi_is_win8()
> >>> acpi_video_use_native_backlight()
> >>> backlight_device_registered(BACKLIGHT_RAW)
> >>> 
> >>> On my notebook acpi_osi_is_win8() returns true (as is win8
> >>> compliant), backlight_device_registered(BACKLIGHT_RAW)
> >>> returns true as I'm using intel i915 drm driver with raw
> >>> backlight device and acpi_video_use_native_backlight()
> >>> returns true/false depending on
> >>> "video.use_native_backlight" kernel param. Default is
> >>> true.
> >>> 
> >>> So if I want to have working acpi video driver with
> >>> display brightness support I need to boot kernel with
> >>> param: "video.use_native_backlight=0". I tested it with
> >>> kernel 3.17-rc6 and this param really enabled display
> >>> brightness support via acpi video driver -- which is
> >>> good.
> >>> 
> >>> Driver dell-laptop creating backligh device for brightness
> >>> control only if acpi_video_backlight_support() returns
> >>> false. There is complicated condition for it and when
> >>> kernel is booted with "video.use_native_backlight=0" that
> >>> function returns true.
> >>> 
> >>> So conclusion is: With current code in kernel 3.17-rc6 it
> >>> is not possible to control brightness of display with
> >>> native driver dell-laptop on Dell Latitude E6440 (and
> >>> probably on others too)!!!
> >>> 
> >>> And Because laptop is win8 compliant and you create
> >>> decision to use native driver (instead acpi one) it is
> >>> not possible to control display brightness without tweeks
> >>> in kernel cmdline.
> >>> 
> >>> As I wrote I would rather to use native dell-laptop driver
> >>> for controlling brightness, but it is not possible.
> >>> 
> >>> So how to solve this problem?
> >>> 
> >>> Quick solution would be to set use_native_backlight false
> >>> for some Dell laptops which means, that acpi video will be
> >>> used and in this case intel i915 driver will *not* drop
> >>> backlight change request.
> >>> 
> >>> Another solution could be to disable check in dell_laptop
> >>> driver and add use_native_backlight=0 to hooks. But this
> >>> create two backlight interfaces (which is not good), but
> >>> only way (for now) how to make dell_laptop working again.
> >>> 
> >>> Better and maybe only one proper solution would be to
> >>> teach intel drm i915 driver to not drop backlight change
> >>> request for Dell laptops (or all??). (This allows to work
> >>> both acpi video and dell_laptop drivers without any
> >>> change and with *any* value in param
> >>> use_native_backlight). I think that problematic code is
> >>> in function asle_set_backlight() in file intel_opregion.c
> >>> (but I'm not sure). My idea is that "drop" event function
> >>> it is caused by this commit
> >>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> >>> sure).
> >>> 
> >>> At least something must be done as I think that I'm not
> >>> only one who has Dell laptop and brightness configuration
> >>> is really important...
> >> 
> >> I don't understand your problem, the kernel is selecting
> >> the i915 backlight driver, making that the only one
> >> available to userspace, so the one problem you state with
> >> the i915 driver, that it is "raw" is not an issue, as
> >> userspace will pick it when it is the only one.
> > 
> > It is not only one.
> 
> Are you sure, if you do not specify any commandline
> parameters, then neither dell-laptop nor acpi-video should
> register a backlight interface.
> 
> dell-laptop.c has:
> 
> #ifdef CONFIG_ACPI
>         /* In the event of an ACPI backlight being available,
> don't * register the platform controller.
>          */
>         if (acpi_video_backlight_support())
>                 return 0;
> #endif
> 
> And acpi_video_backlight_support() will (normally) return true
> on acpi-backlight capable systems independent of
> use_native_backlight.
> 
> And drivers/acpi/video.c has this:
> 
>         /* no warning message if acpi_backlight=vendor or a
> quirk is used */ if (!acpi_video_verify_backlight_support())
>                 return;
> 
> ...
> 
> bool acpi_video_verify_backlight_support(void)
> {
>         if (acpi_osi_is_win8() &&
> acpi_video_use_native_backlight() &&
> backlight_device_registered(BACKLIGHT_RAW)) return false;
>         return acpi_video_backlight_support();
> }
> 
> So unlike the check in dell-laptop this one does depend on the
> use_native_backlight setting.
> 

It depends (see my previous mail). Here is code:

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;
}

> I've just checked 3.17 on my E6430 and there this works as it
> should, I only get the intel_backlight in
> /sys/class/backlight
> 
> > Also dell-laptop (or acpi video) backlight
> > interface is created (depends on use_native_backlight
> > param). And userspace will pick dell-laptop (or acpi video)
> > to use (which cannot change brightness).
> > 
> >> Why would you want to use dell-laptop despite the i915
> >> driver working fine ?
> > 
> > i915 working only if I compile kernel without acpi video
> > dell- laptop support (distributions compile kernel with
> > these drivers) and i915 is not good for usage. First it
> > provides more then thousand brightness levels and lot of
> > (with low numbers) completely turn display off. And if
> > userspace map these thousand levels into 5-10 levels (as
> > nobody want to press brightness key up/down 1000x) two
> > lowest levels cause display off.
> 
> More and more laptops will only have working backlight control
> at all when using i915, so userspace will need to learn to
> better deal with backlight controls with these ranges. And
> low pwm levels turning the backlight of is normal for raw
> interfaces, if userspace does not want this, then they should
> not go so low.
> 

So then kernel should report which low levels turn backlight off 
so userspace will know which levels should avoid. But this is not 
implemented yet.

> I suggest that you file a bug against your desktop environment
> that it is not using the backlight controls in an optimal
> manner, from the kernel pov everything is working fine.
> 

Once I will know which levels should not DE use I can report bug.

> > With acpi
> > video and dell-laptop driver levels are mapped into small
> > level space in perfect way. So this is reason I want to use
> > dell-laptop for controlling brightness.
> 
> If you want to use dell-laptop, specify acpi_backlight=vendor
> on the kernel commandline, that will give you dell_laptop +
> intel_backlight as backlight interfaces, and userspace should
> prefer dell_laptop.

With acpi_backlight=vendor dell-laptop is not working, see my 
previous mail. In this case intel i915 drm driver ignore bios 
events for changing brightness. And userspace prefer to use 
dell_laptop which do nothing!

> But IMHO it would be better to file a bug
> with your desktop environment, and get that fixed to work
> properly with intel_backlight (or with raw backlight
> interfaces in general).
> 
> Regards,
> 
> Hans
> 
> >> Regards,
> >> 
> >> Hans


-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 10546 bytes --]

On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
> > Hi,
> > 
> > On 09/23/2014 10:44 PM, Pali Rohár wrote:
> > > On Tuesday 23 September 2014 22:31:31 you wrote:
> > >> Hi,
> > >> 
> > >> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> > >>> Hello,
> > >>> 
> > >>> after big changes in acpi video/i915 code I cannot
> > >>> change display brightness on my Dell Latitude E6440
> > >>> with kernel 3.17-rc6. With kernel 3.13 everything
> > >>> worked fine.
> > >>> 
> > >>> More information about this problem:
> > >>> 
> > >>> For configuring brightness on Dell laptops there are 4
> > >>> ways: 1) via acpi video driver
> > >>> 2) via dell-laptop driver
> > >>> 3) via i915 drm driver
> > >>> 4) from userspace with special dell SMI call
> > >>> 
> > >>>     (e.g with program dellLcdBrightness from libsmbios
> > >>>     package)
> > >>> 
> > >>> Methods 2) and 4) are same, both making special SMI call
> > >>> and Bios handing this request (just 2 is from kernel and
> > >>> 4 from userspace)
> > >>> 
> > >>> Method 1) via acpi video driver working, but is not
> > >>> perfect. Driver can be used to change brightness (but
> > >>> only some levels, probably this depends on acpi/DSDT
> > >>> tables), but cannot be used to retrieve current
> > >>> brightness (when BIOS/SMI change brightness acpi driver
> > >>> report old incorrect value). So I prefer dell-laptop
> > >>> driver instead acpi video.
> > >>> 
> > >>> Method 3) working even with 3.17-rc6 kernel but because
> > >>> that backlight device exported by i915 is marked as raw,
> > >>> desktop programs prefer to use other devices.
> > >>> 
> > >>> Moreover it looks like that methods 1) 2) and 4) just
> > >>> forward request to method 3). So in any cased brightness
> > >>> is changed by i915 drm driver.
> > >>> 
> > >>> I'm not sure (correct me if I'm wrong!) but I think that
> > >>> intel i915 drm driver accept changes (file
> > >>> intel_opregion.c) only if acpi function
> > >>> acpi_video_verify_backlight_support() returns true.
> > >>> 
> > >>> Function acpi_video_verify_backlight_support() returns
> > >>> true iff: function acpi_video_backlight_support()
> > >>> returns true AND at least one of these functions
> > >>> returns false: acpi_osi_is_win8()
> > >>> acpi_video_use_native_backlight()
> > >>> backlight_device_registered(BACKLIGHT_RAW)
> > >>> 
> > >>> On my notebook acpi_osi_is_win8() returns true (as is
> > >>> win8 compliant),
> > >>> backlight_device_registered(BACKLIGHT_RAW) returns true
> > >>> as I'm using intel i915 drm driver with raw backlight
> > >>> device and acpi_video_use_native_backlight() returns
> > >>> true/false depending on
> > >>> "video.use_native_backlight" kernel param. Default is
> > >>> true.
> > >>> 
> > >>> So if I want to have working acpi video driver with
> > >>> display brightness support I need to boot kernel with
> > >>> param: "video.use_native_backlight=0". I tested it with
> > >>> kernel 3.17-rc6 and this param really enabled display
> > >>> brightness support via acpi video driver -- which is
> > >>> good.
> > >>> 
> > >>> Driver dell-laptop creating backligh device for
> > >>> brightness control only if
> > >>> acpi_video_backlight_support() returns false. There is
> > >>> complicated condition for it and when kernel is booted
> > >>> with "video.use_native_backlight=0" that function
> > >>> returns true.
> > >>> 
> > >>> So conclusion is: With current code in kernel 3.17-rc6
> > >>> it is not possible to control brightness of display
> > >>> with native driver dell-laptop on Dell Latitude E6440
> > >>> (and probably on others too)!!!
> > >>> 
> > >>> And Because laptop is win8 compliant and you create
> > >>> decision to use native driver (instead acpi one) it is
> > >>> not possible to control display brightness without
> > >>> tweeks in kernel cmdline.
> > >>> 
> > >>> As I wrote I would rather to use native dell-laptop
> > >>> driver for controlling brightness, but it is not
> > >>> possible.
> > >>> 
> > >>> So how to solve this problem?
> > >>> 
> > >>> Quick solution would be to set use_native_backlight
> > >>> false for some Dell laptops which means, that acpi
> > >>> video will be used and in this case intel i915 driver
> > >>> will *not* drop backlight change request.
> > >>> 
> > >>> Another solution could be to disable check in
> > >>> dell_laptop driver and add use_native_backlight=0 to
> > >>> hooks. But this create two backlight interfaces (which
> > >>> is not good), but only way (for now) how to make
> > >>> dell_laptop working again.
> > >>> 
> > >>> Better and maybe only one proper solution would be to
> > >>> teach intel drm i915 driver to not drop backlight change
> > >>> request for Dell laptops (or all??). (This allows to
> > >>> work both acpi video and dell_laptop drivers without
> > >>> any change and with *any* value in param
> > >>> use_native_backlight). I think that problematic code is
> > >>> in function asle_set_backlight() in file
> > >>> intel_opregion.c (but I'm not sure). My idea is that
> > >>> "drop" event function it is caused by this commit
> > >>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> > >>> sure).
> > >>> 
> > >>> At least something must be done as I think that I'm not
> > >>> only one who has Dell laptop and brightness
> > >>> configuration is really important...
> > >> 
> > >> I don't understand your problem, the kernel is selecting
> > >> the i915 backlight driver, making that the only one
> > >> available to userspace, so the one problem you state with
> > >> the i915 driver, that it is "raw" is not an issue, as
> > >> userspace will pick it when it is the only one.
> > > 
> > > It is not only one.
> > 
> > Are you sure, if you do not specify any commandline
> > parameters, then neither dell-laptop nor acpi-video should
> > register a backlight interface.
> > 
> > dell-laptop.c has:
> > 
> > #ifdef CONFIG_ACPI
> > 
> >         /* In the event of an ACPI backlight being
> >         available,
> > 
> > don't * register the platform controller.
> > 
> >          */
> >         
> >         if (acpi_video_backlight_support())
> >         
> >                 return 0;
> > 
> > #endif
> > 
> > And acpi_video_backlight_support() will (normally) return
> > true on acpi-backlight capable systems independent of
> > use_native_backlight.
> > 
> > And drivers/acpi/video.c has this:
> >         /* no warning message if acpi_backlight=vendor or a
> > 
> > quirk is used */ if (!acpi_video_verify_backlight_support())
> > 
> >                 return;
> > 
> > ...
> > 
> > bool acpi_video_verify_backlight_support(void)
> > {
> > 
> >         if (acpi_osi_is_win8() &&
> > 
> > acpi_video_use_native_backlight() &&
> > backlight_device_registered(BACKLIGHT_RAW)) return false;
> > 
> >         return acpi_video_backlight_support();
> > 
> > }
> > 
> > So unlike the check in dell-laptop this one does depend on
> > the use_native_backlight setting.
> 
> It depends (see my previous mail). Here is code:
> 
> 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;
> }
> 
> > I've just checked 3.17 on my E6430 and there this works as
> > it should, I only get the intel_backlight in
> > /sys/class/backlight
> > 
> > > Also dell-laptop (or acpi video) backlight
> > > interface is created (depends on use_native_backlight
> > > param). And userspace will pick dell-laptop (or acpi
> > > video) to use (which cannot change brightness).
> > > 
> > >> Why would you want to use dell-laptop despite the i915
> > >> driver working fine ?
> > > 
> > > i915 working only if I compile kernel without acpi video
> > > dell- laptop support (distributions compile kernel with
> > > these drivers) and i915 is not good for usage. First it
> > > provides more then thousand brightness levels and lot of
> > > (with low numbers) completely turn display off. And if
> > > userspace map these thousand levels into 5-10 levels (as
> > > nobody want to press brightness key up/down 1000x) two
> > > lowest levels cause display off.
> > 
> > More and more laptops will only have working backlight
> > control at all when using i915, so userspace will need to
> > learn to better deal with backlight controls with these
> > ranges. And low pwm levels turning the backlight of is
> > normal for raw interfaces, if userspace does not want this,
> > then they should not go so low.
> 
> So then kernel should report which low levels turn backlight
> off so userspace will know which levels should avoid. But
> this is not implemented yet.
> 
> > I suggest that you file a bug against your desktop
> > environment that it is not using the backlight controls in
> > an optimal manner, from the kernel pov everything is
> > working fine.
> 
> Once I will know which levels should not DE use I can report
> bug.
> 
> > > With acpi
> > > video and dell-laptop driver levels are mapped into small
> > > level space in perfect way. So this is reason I want to
> > > use dell-laptop for controlling brightness.
> > 
> > If you want to use dell-laptop, specify
> > acpi_backlight=vendor on the kernel commandline, that will
> > give you dell_laptop + intel_backlight as backlight
> > interfaces, and userspace should prefer dell_laptop.
> 
> With acpi_backlight=vendor dell-laptop is not working, see my
> previous mail. In this case intel i915 drm driver ignore bios
> events for changing brightness. And userspace prefer to use
> dell_laptop which do nothing!
> 

Ok, that problematic commit is:

ACPI / i915: ignore firmware requests for backlight change
0b9f7d93ca6109048a4eb06332b666b6e29df4fe

When I reverted it acpi_backlight=vendor started working again 
(via dell_laptop). Without reverting that commit dell_laptop 
simply do nothing.

Tested and on my laptop Dell Latitude E6440 driver dell_laptop 
does not work with above commit.

> > But IMHO it would be better to file a bug
> > with your desktop environment, and get that fixed to work
> > properly with intel_backlight (or with raw backlight
> > interfaces in general).
> > 
> > Regards,
> > 
> > Hans
> > 
> > >> Regards,
> > >> 
> > >> Hans

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/24/2014 11:14 AM, Pali Rohár wrote:
> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
>> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
>>> Hi,
>>>
>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
>>>>> Hi,
>>>>>
>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>>>>> Hello,
>>>>>>
>>>>>> after big changes in acpi video/i915 code I cannot
>>>>>> change display brightness on my Dell Latitude E6440
>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
>>>>>> worked fine.
>>>>>>
>>>>>> More information about this problem:
>>>>>>
>>>>>> For configuring brightness on Dell laptops there are 4
>>>>>> ways: 1) via acpi video driver
>>>>>> 2) via dell-laptop driver
>>>>>> 3) via i915 drm driver
>>>>>> 4) from userspace with special dell SMI call
>>>>>>
>>>>>>     (e.g with program dellLcdBrightness from libsmbios
>>>>>>     package)
>>>>>>
>>>>>> Methods 2) and 4) are same, both making special SMI call
>>>>>> and Bios handing this request (just 2 is from kernel and
>>>>>> 4 from userspace)
>>>>>>
>>>>>> Method 1) via acpi video driver working, but is not
>>>>>> perfect. Driver can be used to change brightness (but
>>>>>> only some levels, probably this depends on acpi/DSDT
>>>>>> tables), but cannot be used to retrieve current
>>>>>> brightness (when BIOS/SMI change brightness acpi driver
>>>>>> report old incorrect value). So I prefer dell-laptop
>>>>>> driver instead acpi video.
>>>>>>
>>>>>> Method 3) working even with 3.17-rc6 kernel but because
>>>>>> that backlight device exported by i915 is marked as raw,
>>>>>> desktop programs prefer to use other devices.
>>>>>>
>>>>>> Moreover it looks like that methods 1) 2) and 4) just
>>>>>> forward request to method 3). So in any cased brightness
>>>>>> is changed by i915 drm driver.
>>>>>>
>>>>>> I'm not sure (correct me if I'm wrong!) but I think that
>>>>>> intel i915 drm driver accept changes (file
>>>>>> intel_opregion.c) only if acpi function
>>>>>> acpi_video_verify_backlight_support() returns true.
>>>>>>
>>>>>> Function acpi_video_verify_backlight_support() returns
>>>>>> true iff: function acpi_video_backlight_support()
>>>>>> returns true AND at least one of these functions
>>>>>> returns false: acpi_osi_is_win8()
>>>>>> acpi_video_use_native_backlight()
>>>>>> backlight_device_registered(BACKLIGHT_RAW)
>>>>>>
>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
>>>>>> win8 compliant),
>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns true
>>>>>> as I'm using intel i915 drm driver with raw backlight
>>>>>> device and acpi_video_use_native_backlight() returns
>>>>>> true/false depending on
>>>>>> "video.use_native_backlight" kernel param. Default is
>>>>>> true.
>>>>>>
>>>>>> So if I want to have working acpi video driver with
>>>>>> display brightness support I need to boot kernel with
>>>>>> param: "video.use_native_backlight=0". I tested it with
>>>>>> kernel 3.17-rc6 and this param really enabled display
>>>>>> brightness support via acpi video driver -- which is
>>>>>> good.
>>>>>>
>>>>>> Driver dell-laptop creating backligh device for
>>>>>> brightness control only if
>>>>>> acpi_video_backlight_support() returns false. There is
>>>>>> complicated condition for it and when kernel is booted
>>>>>> with "video.use_native_backlight=0" that function
>>>>>> returns true.
>>>>>>
>>>>>> So conclusion is: With current code in kernel 3.17-rc6
>>>>>> it is not possible to control brightness of display
>>>>>> with native driver dell-laptop on Dell Latitude E6440
>>>>>> (and probably on others too)!!!
>>>>>>
>>>>>> And Because laptop is win8 compliant and you create
>>>>>> decision to use native driver (instead acpi one) it is
>>>>>> not possible to control display brightness without
>>>>>> tweeks in kernel cmdline.
>>>>>>
>>>>>> As I wrote I would rather to use native dell-laptop
>>>>>> driver for controlling brightness, but it is not
>>>>>> possible.
>>>>>>
>>>>>> So how to solve this problem?
>>>>>>
>>>>>> Quick solution would be to set use_native_backlight
>>>>>> false for some Dell laptops which means, that acpi
>>>>>> video will be used and in this case intel i915 driver
>>>>>> will *not* drop backlight change request.
>>>>>>
>>>>>> Another solution could be to disable check in
>>>>>> dell_laptop driver and add use_native_backlight=0 to
>>>>>> hooks. But this create two backlight interfaces (which
>>>>>> is not good), but only way (for now) how to make
>>>>>> dell_laptop working again.
>>>>>>
>>>>>> Better and maybe only one proper solution would be to
>>>>>> teach intel drm i915 driver to not drop backlight change
>>>>>> request for Dell laptops (or all??). (This allows to
>>>>>> work both acpi video and dell_laptop drivers without
>>>>>> any change and with *any* value in param
>>>>>> use_native_backlight). I think that problematic code is
>>>>>> in function asle_set_backlight() in file
>>>>>> intel_opregion.c (but I'm not sure). My idea is that
>>>>>> "drop" event function it is caused by this commit
>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>>>>> sure).
>>>>>>
>>>>>> At least something must be done as I think that I'm not
>>>>>> only one who has Dell laptop and brightness
>>>>>> configuration is really important...
>>>>>
>>>>> I don't understand your problem, the kernel is selecting
>>>>> the i915 backlight driver, making that the only one
>>>>> available to userspace, so the one problem you state with
>>>>> the i915 driver, that it is "raw" is not an issue, as
>>>>> userspace will pick it when it is the only one.
>>>>
>>>> It is not only one.
>>>
>>> Are you sure, if you do not specify any commandline
>>> parameters, then neither dell-laptop nor acpi-video should
>>> register a backlight interface.
>>>
>>> dell-laptop.c has:
>>>
>>> #ifdef CONFIG_ACPI
>>>
>>>         /* In the event of an ACPI backlight being
>>>         available,
>>>
>>> don't * register the platform controller.
>>>
>>>          */
>>>         
>>>         if (acpi_video_backlight_support())
>>>         
>>>                 return 0;
>>>
>>> #endif
>>>
>>> And acpi_video_backlight_support() will (normally) return
>>> true on acpi-backlight capable systems independent of
>>> use_native_backlight.
>>>
>>> And drivers/acpi/video.c has this:
>>>         /* no warning message if acpi_backlight=vendor or a
>>>
>>> quirk is used */ if (!acpi_video_verify_backlight_support())
>>>
>>>                 return;
>>>
>>> ...
>>>
>>> bool acpi_video_verify_backlight_support(void)
>>> {
>>>
>>>         if (acpi_osi_is_win8() &&
>>>
>>> acpi_video_use_native_backlight() &&
>>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>>
>>>         return acpi_video_backlight_support();
>>>
>>> }
>>>
>>> So unlike the check in dell-laptop this one does depend on
>>> the use_native_backlight setting.
>>
>> It depends (see my previous mail). Here is code:
>>
>> 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;
>> }

Not sure what you're trying to say here, the default for
this is 1, so if you don't specify anything, then this:

bool acpi_video_verify_backlight_support(void)
{
        if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
            backlight_device_registered(BACKLIGHT_RAW))
                return false;
        return acpi_video_backlight_support();
}

Will return false, and you won't get an acpi_video0 backlight interface,
only the intel_backlight one, and everything should just work
(except for the turning off on low settings).

Have you tried not using any kernel commandline options? What happens?

>>
>>> I've just checked 3.17 on my E6430 and there this works as
>>> it should, I only get the intel_backlight in
>>> /sys/class/backlight
>>>
>>>> Also dell-laptop (or acpi video) backlight
>>>> interface is created (depends on use_native_backlight
>>>> param). And userspace will pick dell-laptop (or acpi
>>>> video) to use (which cannot change brightness).
>>>>
>>>>> Why would you want to use dell-laptop despite the i915
>>>>> driver working fine ?
>>>>
>>>> i915 working only if I compile kernel without acpi video
>>>> dell- laptop support (distributions compile kernel with
>>>> these drivers) and i915 is not good for usage. First it
>>>> provides more then thousand brightness levels and lot of
>>>> (with low numbers) completely turn display off. And if
>>>> userspace map these thousand levels into 5-10 levels (as
>>>> nobody want to press brightness key up/down 1000x) two
>>>> lowest levels cause display off.
>>>
>>> More and more laptops will only have working backlight
>>> control at all when using i915, so userspace will need to
>>> learn to better deal with backlight controls with these
>>> ranges. And low pwm levels turning the backlight of is
>>> normal for raw interfaces, if userspace does not want this,
>>> then they should not go so low.
>>
>> So then kernel should report which low levels turn backlight
>> off so userspace will know which levels should avoid. But
>> this is not implemented yet.
>>
>>> I suggest that you file a bug against your desktop
>>> environment that it is not using the backlight controls in
>>> an optimal manner, from the kernel pov everything is
>>> working fine.
>>
>> Once I will know which levels should not DE use I can report
>> bug.
>>
>>>> With acpi
>>>> video and dell-laptop driver levels are mapped into small
>>>> level space in perfect way. So this is reason I want to
>>>> use dell-laptop for controlling brightness.
>>>
>>> If you want to use dell-laptop, specify
>>> acpi_backlight=vendor on the kernel commandline, that will
>>> give you dell_laptop + intel_backlight as backlight
>>> interfaces, and userspace should prefer dell_laptop.
>>
>> With acpi_backlight=vendor dell-laptop is not working, see my
>> previous mail. In this case intel i915 drm driver ignore bios
>> events for changing brightness. And userspace prefer to use
>> dell_laptop which do nothing!
>>
> 
> Ok, that problematic commit is:
> 
> ACPI / i915: ignore firmware requests for backlight change
> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
> 
> When I reverted it acpi_backlight=vendor started working again 
> (via dell_laptop). Without reverting that commit dell_laptop 
> simply do nothing.
> 
> Tested and on my laptop Dell Latitude E6440 driver dell_laptop 
> does not work with above commit.

Hmm, interesting, so when dell-laptop registers and makes a few
calls using the dell-laptop acpi interface, then you either
stop getting key press events through the acpi-video-bus, or
dell-laptop is just a shim around the i915 interface with the
firmware calling into itself on these models. Given that dell-laptop
is ancient, the shim story is not that far fetched.

Do you still get an on screen display showing the brightness when
using a kernel without this patch + acpi_backlight=vendor ?

Regards,

Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/24/2014 11:14 AM, Pali Rohár wrote:
> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
>> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
>>> Hi,
>>>
>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
>>>>> Hi,
>>>>>
>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>>>>> Hello,
>>>>>>
>>>>>> after big changes in acpi video/i915 code I cannot
>>>>>> change display brightness on my Dell Latitude E6440
>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
>>>>>> worked fine.
>>>>>>
>>>>>> More information about this problem:
>>>>>>
>>>>>> For configuring brightness on Dell laptops there are 4
>>>>>> ways: 1) via acpi video driver
>>>>>> 2) via dell-laptop driver
>>>>>> 3) via i915 drm driver
>>>>>> 4) from userspace with special dell SMI call
>>>>>>
>>>>>>     (e.g with program dellLcdBrightness from libsmbios
>>>>>>     package)
>>>>>>
>>>>>> Methods 2) and 4) are same, both making special SMI call
>>>>>> and Bios handing this request (just 2 is from kernel and
>>>>>> 4 from userspace)
>>>>>>
>>>>>> Method 1) via acpi video driver working, but is not
>>>>>> perfect. Driver can be used to change brightness (but
>>>>>> only some levels, probably this depends on acpi/DSDT
>>>>>> tables), but cannot be used to retrieve current
>>>>>> brightness (when BIOS/SMI change brightness acpi driver
>>>>>> report old incorrect value). So I prefer dell-laptop
>>>>>> driver instead acpi video.
>>>>>>
>>>>>> Method 3) working even with 3.17-rc6 kernel but because
>>>>>> that backlight device exported by i915 is marked as raw,
>>>>>> desktop programs prefer to use other devices.
>>>>>>
>>>>>> Moreover it looks like that methods 1) 2) and 4) just
>>>>>> forward request to method 3). So in any cased brightness
>>>>>> is changed by i915 drm driver.
>>>>>>
>>>>>> I'm not sure (correct me if I'm wrong!) but I think that
>>>>>> intel i915 drm driver accept changes (file
>>>>>> intel_opregion.c) only if acpi function
>>>>>> acpi_video_verify_backlight_support() returns true.
>>>>>>
>>>>>> Function acpi_video_verify_backlight_support() returns
>>>>>> true iff: function acpi_video_backlight_support()
>>>>>> returns true AND at least one of these functions
>>>>>> returns false: acpi_osi_is_win8()
>>>>>> acpi_video_use_native_backlight()
>>>>>> backlight_device_registered(BACKLIGHT_RAW)
>>>>>>
>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
>>>>>> win8 compliant),
>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns true
>>>>>> as I'm using intel i915 drm driver with raw backlight
>>>>>> device and acpi_video_use_native_backlight() returns
>>>>>> true/false depending on
>>>>>> "video.use_native_backlight" kernel param. Default is
>>>>>> true.
>>>>>>
>>>>>> So if I want to have working acpi video driver with
>>>>>> display brightness support I need to boot kernel with
>>>>>> param: "video.use_native_backlight=0". I tested it with
>>>>>> kernel 3.17-rc6 and this param really enabled display
>>>>>> brightness support via acpi video driver -- which is
>>>>>> good.
>>>>>>
>>>>>> Driver dell-laptop creating backligh device for
>>>>>> brightness control only if
>>>>>> acpi_video_backlight_support() returns false. There is
>>>>>> complicated condition for it and when kernel is booted
>>>>>> with "video.use_native_backlight=0" that function
>>>>>> returns true.
>>>>>>
>>>>>> So conclusion is: With current code in kernel 3.17-rc6
>>>>>> it is not possible to control brightness of display
>>>>>> with native driver dell-laptop on Dell Latitude E6440
>>>>>> (and probably on others too)!!!
>>>>>>
>>>>>> And Because laptop is win8 compliant and you create
>>>>>> decision to use native driver (instead acpi one) it is
>>>>>> not possible to control display brightness without
>>>>>> tweeks in kernel cmdline.
>>>>>>
>>>>>> As I wrote I would rather to use native dell-laptop
>>>>>> driver for controlling brightness, but it is not
>>>>>> possible.
>>>>>>
>>>>>> So how to solve this problem?
>>>>>>
>>>>>> Quick solution would be to set use_native_backlight
>>>>>> false for some Dell laptops which means, that acpi
>>>>>> video will be used and in this case intel i915 driver
>>>>>> will *not* drop backlight change request.
>>>>>>
>>>>>> Another solution could be to disable check in
>>>>>> dell_laptop driver and add use_native_backlight=0 to
>>>>>> hooks. But this create two backlight interfaces (which
>>>>>> is not good), but only way (for now) how to make
>>>>>> dell_laptop working again.
>>>>>>
>>>>>> Better and maybe only one proper solution would be to
>>>>>> teach intel drm i915 driver to not drop backlight change
>>>>>> request for Dell laptops (or all??). (This allows to
>>>>>> work both acpi video and dell_laptop drivers without
>>>>>> any change and with *any* value in param
>>>>>> use_native_backlight). I think that problematic code is
>>>>>> in function asle_set_backlight() in file
>>>>>> intel_opregion.c (but I'm not sure). My idea is that
>>>>>> "drop" event function it is caused by this commit
>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>>>>> sure).
>>>>>>
>>>>>> At least something must be done as I think that I'm not
>>>>>> only one who has Dell laptop and brightness
>>>>>> configuration is really important...
>>>>>
>>>>> I don't understand your problem, the kernel is selecting
>>>>> the i915 backlight driver, making that the only one
>>>>> available to userspace, so the one problem you state with
>>>>> the i915 driver, that it is "raw" is not an issue, as
>>>>> userspace will pick it when it is the only one.
>>>>
>>>> It is not only one.
>>>
>>> Are you sure, if you do not specify any commandline
>>> parameters, then neither dell-laptop nor acpi-video should
>>> register a backlight interface.
>>>
>>> dell-laptop.c has:
>>>
>>> #ifdef CONFIG_ACPI
>>>
>>>         /* In the event of an ACPI backlight being
>>>         available,
>>>
>>> don't * register the platform controller.
>>>
>>>          */
>>>         
>>>         if (acpi_video_backlight_support())
>>>         
>>>                 return 0;
>>>
>>> #endif
>>>
>>> And acpi_video_backlight_support() will (normally) return
>>> true on acpi-backlight capable systems independent of
>>> use_native_backlight.
>>>
>>> And drivers/acpi/video.c has this:
>>>         /* no warning message if acpi_backlight=vendor or a
>>>
>>> quirk is used */ if (!acpi_video_verify_backlight_support())
>>>
>>>                 return;
>>>
>>> ...
>>>
>>> bool acpi_video_verify_backlight_support(void)
>>> {
>>>
>>>         if (acpi_osi_is_win8() &&
>>>
>>> acpi_video_use_native_backlight() &&
>>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>>
>>>         return acpi_video_backlight_support();
>>>
>>> }
>>>
>>> So unlike the check in dell-laptop this one does depend on
>>> the use_native_backlight setting.
>>
>> It depends (see my previous mail). Here is code:
>>
>> 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;
>> }

Not sure what you're trying to say here, the default for
this is 1, so if you don't specify anything, then this:

bool acpi_video_verify_backlight_support(void)
{
        if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
            backlight_device_registered(BACKLIGHT_RAW))
                return false;
        return acpi_video_backlight_support();
}

Will return false, and you won't get an acpi_video0 backlight interface,
only the intel_backlight one, and everything should just work
(except for the turning off on low settings).

Have you tried not using any kernel commandline options? What happens?

>>
>>> I've just checked 3.17 on my E6430 and there this works as
>>> it should, I only get the intel_backlight in
>>> /sys/class/backlight
>>>
>>>> Also dell-laptop (or acpi video) backlight
>>>> interface is created (depends on use_native_backlight
>>>> param). And userspace will pick dell-laptop (or acpi
>>>> video) to use (which cannot change brightness).
>>>>
>>>>> Why would you want to use dell-laptop despite the i915
>>>>> driver working fine ?
>>>>
>>>> i915 working only if I compile kernel without acpi video
>>>> dell- laptop support (distributions compile kernel with
>>>> these drivers) and i915 is not good for usage. First it
>>>> provides more then thousand brightness levels and lot of
>>>> (with low numbers) completely turn display off. And if
>>>> userspace map these thousand levels into 5-10 levels (as
>>>> nobody want to press brightness key up/down 1000x) two
>>>> lowest levels cause display off.
>>>
>>> More and more laptops will only have working backlight
>>> control at all when using i915, so userspace will need to
>>> learn to better deal with backlight controls with these
>>> ranges. And low pwm levels turning the backlight of is
>>> normal for raw interfaces, if userspace does not want this,
>>> then they should not go so low.
>>
>> So then kernel should report which low levels turn backlight
>> off so userspace will know which levels should avoid. But
>> this is not implemented yet.
>>
>>> I suggest that you file a bug against your desktop
>>> environment that it is not using the backlight controls in
>>> an optimal manner, from the kernel pov everything is
>>> working fine.
>>
>> Once I will know which levels should not DE use I can report
>> bug.
>>
>>>> With acpi
>>>> video and dell-laptop driver levels are mapped into small
>>>> level space in perfect way. So this is reason I want to
>>>> use dell-laptop for controlling brightness.
>>>
>>> If you want to use dell-laptop, specify
>>> acpi_backlight=vendor on the kernel commandline, that will
>>> give you dell_laptop + intel_backlight as backlight
>>> interfaces, and userspace should prefer dell_laptop.
>>
>> With acpi_backlight=vendor dell-laptop is not working, see my
>> previous mail. In this case intel i915 drm driver ignore bios
>> events for changing brightness. And userspace prefer to use
>> dell_laptop which do nothing!
>>
> 
> Ok, that problematic commit is:
> 
> ACPI / i915: ignore firmware requests for backlight change
> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
> 
> When I reverted it acpi_backlight=vendor started working again 
> (via dell_laptop). Without reverting that commit dell_laptop 
> simply do nothing.
> 
> Tested and on my laptop Dell Latitude E6440 driver dell_laptop 
> does not work with above commit.

Hmm, interesting, so when dell-laptop registers and makes a few
calls using the dell-laptop acpi interface, then you either
stop getting key press events through the acpi-video-bus, or
dell-laptop is just a shim around the i915 interface with the
firmware calling into itself on these models. Given that dell-laptop
is ancient, the shim story is not that far fetched.

Do you still get an on screen display showing the brightness when
using a kernel without this patch + acpi_backlight=vendor ?

Regards,

Hans

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 13407 bytes --]

On Wednesday 24 September 2014 14:04:36 Hans de Goede wrote:
> Hi,
> 
> On 09/24/2014 11:14 AM, Pali Rohár wrote:
> > On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
> >> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
> >>> Hi,
> >>> 
> >>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
> >>>> On Tuesday 23 September 2014 22:31:31 you wrote:
> >>>>> Hi,
> >>>>> 
> >>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> >>>>>> Hello,
> >>>>>> 
> >>>>>> after big changes in acpi video/i915 code I cannot
> >>>>>> change display brightness on my Dell Latitude E6440
> >>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
> >>>>>> worked fine.
> >>>>>> 
> >>>>>> More information about this problem:
> >>>>>> 
> >>>>>> For configuring brightness on Dell laptops there are 4
> >>>>>> ways: 1) via acpi video driver
> >>>>>> 2) via dell-laptop driver
> >>>>>> 3) via i915 drm driver
> >>>>>> 4) from userspace with special dell SMI call
> >>>>>> 
> >>>>>>     (e.g with program dellLcdBrightness from libsmbios
> >>>>>>     package)
> >>>>>> 
> >>>>>> Methods 2) and 4) are same, both making special SMI
> >>>>>> call and Bios handing this request (just 2 is from
> >>>>>> kernel and 4 from userspace)
> >>>>>> 
> >>>>>> Method 1) via acpi video driver working, but is not
> >>>>>> perfect. Driver can be used to change brightness (but
> >>>>>> only some levels, probably this depends on acpi/DSDT
> >>>>>> tables), but cannot be used to retrieve current
> >>>>>> brightness (when BIOS/SMI change brightness acpi driver
> >>>>>> report old incorrect value). So I prefer dell-laptop
> >>>>>> driver instead acpi video.
> >>>>>> 
> >>>>>> Method 3) working even with 3.17-rc6 kernel but because
> >>>>>> that backlight device exported by i915 is marked as
> >>>>>> raw, desktop programs prefer to use other devices.
> >>>>>> 
> >>>>>> Moreover it looks like that methods 1) 2) and 4) just
> >>>>>> forward request to method 3). So in any cased
> >>>>>> brightness is changed by i915 drm driver.
> >>>>>> 
> >>>>>> I'm not sure (correct me if I'm wrong!) but I think
> >>>>>> that intel i915 drm driver accept changes (file
> >>>>>> intel_opregion.c) only if acpi function
> >>>>>> acpi_video_verify_backlight_support() returns true.
> >>>>>> 
> >>>>>> Function acpi_video_verify_backlight_support() returns
> >>>>>> true iff: function acpi_video_backlight_support()
> >>>>>> returns true AND at least one of these functions
> >>>>>> returns false: acpi_osi_is_win8()
> >>>>>> acpi_video_use_native_backlight()
> >>>>>> backlight_device_registered(BACKLIGHT_RAW)
> >>>>>> 
> >>>>>> On my notebook acpi_osi_is_win8() returns true (as is
> >>>>>> win8 compliant),
> >>>>>> backlight_device_registered(BACKLIGHT_RAW) returns true
> >>>>>> as I'm using intel i915 drm driver with raw backlight
> >>>>>> device and acpi_video_use_native_backlight() returns
> >>>>>> true/false depending on
> >>>>>> "video.use_native_backlight" kernel param. Default is
> >>>>>> true.
> >>>>>> 
> >>>>>> So if I want to have working acpi video driver with
> >>>>>> display brightness support I need to boot kernel with
> >>>>>> param: "video.use_native_backlight=0". I tested it with
> >>>>>> kernel 3.17-rc6 and this param really enabled display
> >>>>>> brightness support via acpi video driver -- which is
> >>>>>> good.
> >>>>>> 
> >>>>>> Driver dell-laptop creating backligh device for
> >>>>>> brightness control only if
> >>>>>> acpi_video_backlight_support() returns false. There is
> >>>>>> complicated condition for it and when kernel is booted
> >>>>>> with "video.use_native_backlight=0" that function
> >>>>>> returns true.
> >>>>>> 
> >>>>>> So conclusion is: With current code in kernel 3.17-rc6
> >>>>>> it is not possible to control brightness of display
> >>>>>> with native driver dell-laptop on Dell Latitude E6440
> >>>>>> (and probably on others too)!!!
> >>>>>> 
> >>>>>> And Because laptop is win8 compliant and you create
> >>>>>> decision to use native driver (instead acpi one) it is
> >>>>>> not possible to control display brightness without
> >>>>>> tweeks in kernel cmdline.
> >>>>>> 
> >>>>>> As I wrote I would rather to use native dell-laptop
> >>>>>> driver for controlling brightness, but it is not
> >>>>>> possible.
> >>>>>> 
> >>>>>> So how to solve this problem?
> >>>>>> 
> >>>>>> Quick solution would be to set use_native_backlight
> >>>>>> false for some Dell laptops which means, that acpi
> >>>>>> video will be used and in this case intel i915 driver
> >>>>>> will *not* drop backlight change request.
> >>>>>> 
> >>>>>> Another solution could be to disable check in
> >>>>>> dell_laptop driver and add use_native_backlight=0 to
> >>>>>> hooks. But this create two backlight interfaces (which
> >>>>>> is not good), but only way (for now) how to make
> >>>>>> dell_laptop working again.
> >>>>>> 
> >>>>>> Better and maybe only one proper solution would be to
> >>>>>> teach intel drm i915 driver to not drop backlight
> >>>>>> change request for Dell laptops (or all??). (This
> >>>>>> allows to work both acpi video and dell_laptop drivers
> >>>>>> without any change and with *any* value in param
> >>>>>> use_native_backlight). I think that problematic code is
> >>>>>> in function asle_set_backlight() in file
> >>>>>> intel_opregion.c (but I'm not sure). My idea is that
> >>>>>> "drop" event function it is caused by this commit
> >>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> >>>>>> sure).
> >>>>>> 
> >>>>>> At least something must be done as I think that I'm not
> >>>>>> only one who has Dell laptop and brightness
> >>>>>> configuration is really important...
> >>>>> 
> >>>>> I don't understand your problem, the kernel is selecting
> >>>>> the i915 backlight driver, making that the only one
> >>>>> available to userspace, so the one problem you state
> >>>>> with the i915 driver, that it is "raw" is not an issue,
> >>>>> as userspace will pick it when it is the only one.
> >>>> 
> >>>> It is not only one.
> >>> 
> >>> Are you sure, if you do not specify any commandline
> >>> parameters, then neither dell-laptop nor acpi-video should
> >>> register a backlight interface.
> >>> 
> >>> dell-laptop.c has:
> >>> 
> >>> #ifdef CONFIG_ACPI
> >>> 
> >>>         /* In the event of an ACPI backlight being
> >>>         available,
> >>> 
> >>> don't * register the platform controller.
> >>> 
> >>>          */
> >>>         
> >>>         if (acpi_video_backlight_support())
> >>>         
> >>>                 return 0;
> >>> 
> >>> #endif
> >>> 
> >>> And acpi_video_backlight_support() will (normally) return
> >>> true on acpi-backlight capable systems independent of
> >>> use_native_backlight.
> >>> 
> >>> And drivers/acpi/video.c has this:
> >>>         /* no warning message if acpi_backlight=vendor or
> >>>         a
> >>> 
> >>> quirk is used */ if
> >>> (!acpi_video_verify_backlight_support())
> >>> 
> >>>                 return;
> >>> 
> >>> ...
> >>> 
> >>> bool acpi_video_verify_backlight_support(void)
> >>> {
> >>> 
> >>>         if (acpi_osi_is_win8() &&
> >>> 
> >>> acpi_video_use_native_backlight() &&
> >>> backlight_device_registered(BACKLIGHT_RAW)) return false;
> >>> 
> >>>         return acpi_video_backlight_support();
> >>> 
> >>> }
> >>> 
> >>> So unlike the check in dell-laptop this one does depend on
> >>> the use_native_backlight setting.
> >> 
> >> It depends (see my previous mail). Here is code:
> >> 
> >> 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;
> >> 
> >> }
> 
> Not sure what you're trying to say here, the default for
> this is 1, so if you don't specify anything, then this:
> 
> bool acpi_video_verify_backlight_support(void)
> {
>         if (acpi_osi_is_win8() &&
> acpi_video_use_native_backlight() &&
> backlight_device_registered(BACKLIGHT_RAW)) return false;
>         return acpi_video_backlight_support();
> }
> 
> Will return false, and you won't get an acpi_video0 backlight
> interface, only the intel_backlight one, and everything
> should just work (except for the turning off on low
> settings).
> 

When I do not specify param acpi_video_verify_backlight_support() 
will return false because acpi_osi_is_win8() returns true.

> Have you tried not using any kernel commandline options? What
> happens?
> 

Yes, then kernel create two backlight devices: one raw from i915 
module and one normal from dell-laptop module. Userspace will 
pick dell-laptop for using. And due to commit 0b9f7d93 (which 
disabling events when acpi_video_verify_backlight_support returns 
false) dell-laptop backlight not working. So controlling 
brightness from userspace does not work.

> >>> I've just checked 3.17 on my E6430 and there this works as
> >>> it should, I only get the intel_backlight in
> >>> /sys/class/backlight
> >>> 
> >>>> Also dell-laptop (or acpi video) backlight
> >>>> interface is created (depends on use_native_backlight
> >>>> param). And userspace will pick dell-laptop (or acpi
> >>>> video) to use (which cannot change brightness).
> >>>> 
> >>>>> Why would you want to use dell-laptop despite the i915
> >>>>> driver working fine ?
> >>>> 
> >>>> i915 working only if I compile kernel without acpi video
> >>>> dell- laptop support (distributions compile kernel with
> >>>> these drivers) and i915 is not good for usage. First it
> >>>> provides more then thousand brightness levels and lot of
> >>>> (with low numbers) completely turn display off. And if
> >>>> userspace map these thousand levels into 5-10 levels (as
> >>>> nobody want to press brightness key up/down 1000x) two
> >>>> lowest levels cause display off.
> >>> 
> >>> More and more laptops will only have working backlight
> >>> control at all when using i915, so userspace will need to
> >>> learn to better deal with backlight controls with these
> >>> ranges. And low pwm levels turning the backlight of is
> >>> normal for raw interfaces, if userspace does not want
> >>> this, then they should not go so low.
> >> 
> >> So then kernel should report which low levels turn
> >> backlight off so userspace will know which levels should
> >> avoid. But this is not implemented yet.
> >> 
> >>> I suggest that you file a bug against your desktop
> >>> environment that it is not using the backlight controls in
> >>> an optimal manner, from the kernel pov everything is
> >>> working fine.
> >> 
> >> Once I will know which levels should not DE use I can
> >> report bug.
> >> 
> >>>> With acpi
> >>>> video and dell-laptop driver levels are mapped into small
> >>>> level space in perfect way. So this is reason I want to
> >>>> use dell-laptop for controlling brightness.
> >>> 
> >>> If you want to use dell-laptop, specify
> >>> acpi_backlight=vendor on the kernel commandline, that will
> >>> give you dell_laptop + intel_backlight as backlight
> >>> interfaces, and userspace should prefer dell_laptop.
> >> 
> >> With acpi_backlight=vendor dell-laptop is not working, see
> >> my previous mail. In this case intel i915 drm driver
> >> ignore bios events for changing brightness. And userspace
> >> prefer to use dell_laptop which do nothing!
> > 
> > Ok, that problematic commit is:
> > 
> > ACPI / i915: ignore firmware requests for backlight change
> > 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
> > 
> > When I reverted it acpi_backlight=vendor started working
> > again (via dell_laptop). Without reverting that commit
> > dell_laptop simply do nothing.
> > 
> > Tested and on my laptop Dell Latitude E6440 driver
> > dell_laptop does not work with above commit.
> 
> Hmm, interesting, so when dell-laptop registers and makes a
> few calls using the dell-laptop acpi interface,

No, dell-laptop is *not* acpi driver. See my first mail. It uses 
dell dcdbas driver which makes SMI calls for SMBIOS. But it (on 
my machine! no idea about other older once) just forward 
brightness change to intel driver. And it has useful brightness 
levels (no lot of levels which turning display off).

And making SMI calls can be done also from userspace. There is 
tool dellLcdBrightness in libsmbios package which for brightness. 
And commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe broke 
functionality of that tool.

> then you either stop getting key press events through the
> acpi-video-bus, or dell-laptop is just a shim around the i915
> interface with the firmware calling into itself on these
> models. Given that dell-laptop is ancient, the shim story is
> not that far fetched.
> 

Brightness Fn keys are reported by WMI (dell-wmi driver), so they 
working without dell-laptop and acpi video drivers perfectly.

> Do you still get an on screen display showing the brightness
> when using a kernel without this patch +
> acpi_backlight=vendor ?
> 

Brightness Fn keys are reported to userspace (from WMI input 
device) with any combination of video.use_native_backlight and 
acpi_backlight kernel params.

> Regards,
> 
> Hans

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/24/2014 02:53 PM, Pali Rohár wrote:
> On Wednesday 24 September 2014 14:04:36 Hans de Goede wrote:
>> Hi,
>>
>> On 09/24/2014 11:14 AM, Pali Rohár wrote:
>>> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
>>>> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
>>>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> after big changes in acpi video/i915 code I cannot
>>>>>>>> change display brightness on my Dell Latitude E6440
>>>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
>>>>>>>> worked fine.
>>>>>>>>
>>>>>>>> More information about this problem:
>>>>>>>>
>>>>>>>> For configuring brightness on Dell laptops there are 4
>>>>>>>> ways: 1) via acpi video driver
>>>>>>>> 2) via dell-laptop driver
>>>>>>>> 3) via i915 drm driver
>>>>>>>> 4) from userspace with special dell SMI call
>>>>>>>>
>>>>>>>>     (e.g with program dellLcdBrightness from libsmbios
>>>>>>>>     package)
>>>>>>>>
>>>>>>>> Methods 2) and 4) are same, both making special SMI
>>>>>>>> call and Bios handing this request (just 2 is from
>>>>>>>> kernel and 4 from userspace)
>>>>>>>>
>>>>>>>> Method 1) via acpi video driver working, but is not
>>>>>>>> perfect. Driver can be used to change brightness (but
>>>>>>>> only some levels, probably this depends on acpi/DSDT
>>>>>>>> tables), but cannot be used to retrieve current
>>>>>>>> brightness (when BIOS/SMI change brightness acpi driver
>>>>>>>> report old incorrect value). So I prefer dell-laptop
>>>>>>>> driver instead acpi video.
>>>>>>>>
>>>>>>>> Method 3) working even with 3.17-rc6 kernel but because
>>>>>>>> that backlight device exported by i915 is marked as
>>>>>>>> raw, desktop programs prefer to use other devices.
>>>>>>>>
>>>>>>>> Moreover it looks like that methods 1) 2) and 4) just
>>>>>>>> forward request to method 3). So in any cased
>>>>>>>> brightness is changed by i915 drm driver.
>>>>>>>>
>>>>>>>> I'm not sure (correct me if I'm wrong!) but I think
>>>>>>>> that intel i915 drm driver accept changes (file
>>>>>>>> intel_opregion.c) only if acpi function
>>>>>>>> acpi_video_verify_backlight_support() returns true.
>>>>>>>>
>>>>>>>> Function acpi_video_verify_backlight_support() returns
>>>>>>>> true iff: function acpi_video_backlight_support()
>>>>>>>> returns true AND at least one of these functions
>>>>>>>> returns false: acpi_osi_is_win8()
>>>>>>>> acpi_video_use_native_backlight()
>>>>>>>> backlight_device_registered(BACKLIGHT_RAW)
>>>>>>>>
>>>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
>>>>>>>> win8 compliant),
>>>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns true
>>>>>>>> as I'm using intel i915 drm driver with raw backlight
>>>>>>>> device and acpi_video_use_native_backlight() returns
>>>>>>>> true/false depending on
>>>>>>>> "video.use_native_backlight" kernel param. Default is
>>>>>>>> true.
>>>>>>>>
>>>>>>>> So if I want to have working acpi video driver with
>>>>>>>> display brightness support I need to boot kernel with
>>>>>>>> param: "video.use_native_backlight=0". I tested it with
>>>>>>>> kernel 3.17-rc6 and this param really enabled display
>>>>>>>> brightness support via acpi video driver -- which is
>>>>>>>> good.
>>>>>>>>
>>>>>>>> Driver dell-laptop creating backligh device for
>>>>>>>> brightness control only if
>>>>>>>> acpi_video_backlight_support() returns false. There is
>>>>>>>> complicated condition for it and when kernel is booted
>>>>>>>> with "video.use_native_backlight=0" that function
>>>>>>>> returns true.
>>>>>>>>
>>>>>>>> So conclusion is: With current code in kernel 3.17-rc6
>>>>>>>> it is not possible to control brightness of display
>>>>>>>> with native driver dell-laptop on Dell Latitude E6440
>>>>>>>> (and probably on others too)!!!
>>>>>>>>
>>>>>>>> And Because laptop is win8 compliant and you create
>>>>>>>> decision to use native driver (instead acpi one) it is
>>>>>>>> not possible to control display brightness without
>>>>>>>> tweeks in kernel cmdline.
>>>>>>>>
>>>>>>>> As I wrote I would rather to use native dell-laptop
>>>>>>>> driver for controlling brightness, but it is not
>>>>>>>> possible.
>>>>>>>>
>>>>>>>> So how to solve this problem?
>>>>>>>>
>>>>>>>> Quick solution would be to set use_native_backlight
>>>>>>>> false for some Dell laptops which means, that acpi
>>>>>>>> video will be used and in this case intel i915 driver
>>>>>>>> will *not* drop backlight change request.
>>>>>>>>
>>>>>>>> Another solution could be to disable check in
>>>>>>>> dell_laptop driver and add use_native_backlight=0 to
>>>>>>>> hooks. But this create two backlight interfaces (which
>>>>>>>> is not good), but only way (for now) how to make
>>>>>>>> dell_laptop working again.
>>>>>>>>
>>>>>>>> Better and maybe only one proper solution would be to
>>>>>>>> teach intel drm i915 driver to not drop backlight
>>>>>>>> change request for Dell laptops (or all??). (This
>>>>>>>> allows to work both acpi video and dell_laptop drivers
>>>>>>>> without any change and with *any* value in param
>>>>>>>> use_native_backlight). I think that problematic code is
>>>>>>>> in function asle_set_backlight() in file
>>>>>>>> intel_opregion.c (but I'm not sure). My idea is that
>>>>>>>> "drop" event function it is caused by this commit
>>>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>>>>>>> sure).
>>>>>>>>
>>>>>>>> At least something must be done as I think that I'm not
>>>>>>>> only one who has Dell laptop and brightness
>>>>>>>> configuration is really important...
>>>>>>>
>>>>>>> I don't understand your problem, the kernel is selecting
>>>>>>> the i915 backlight driver, making that the only one
>>>>>>> available to userspace, so the one problem you state
>>>>>>> with the i915 driver, that it is "raw" is not an issue,
>>>>>>> as userspace will pick it when it is the only one.
>>>>>>
>>>>>> It is not only one.
>>>>>
>>>>> Are you sure, if you do not specify any commandline
>>>>> parameters, then neither dell-laptop nor acpi-video should
>>>>> register a backlight interface.
>>>>>
>>>>> dell-laptop.c has:
>>>>>
>>>>> #ifdef CONFIG_ACPI
>>>>>
>>>>>         /* In the event of an ACPI backlight being
>>>>>         available,
>>>>>
>>>>> don't * register the platform controller.
>>>>>
>>>>>          */
>>>>>         
>>>>>         if (acpi_video_backlight_support())
>>>>>         
>>>>>                 return 0;
>>>>>
>>>>> #endif
>>>>>
>>>>> And acpi_video_backlight_support() will (normally) return
>>>>> true on acpi-backlight capable systems independent of
>>>>> use_native_backlight.
>>>>>
>>>>> And drivers/acpi/video.c has this:
>>>>>         /* no warning message if acpi_backlight=vendor or
>>>>>         a
>>>>>
>>>>> quirk is used */ if
>>>>> (!acpi_video_verify_backlight_support())
>>>>>
>>>>>                 return;
>>>>>
>>>>> ...
>>>>>
>>>>> bool acpi_video_verify_backlight_support(void)
>>>>> {
>>>>>
>>>>>         if (acpi_osi_is_win8() &&
>>>>>
>>>>> acpi_video_use_native_backlight() &&
>>>>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>>>>
>>>>>         return acpi_video_backlight_support();
>>>>>
>>>>> }
>>>>>
>>>>> So unlike the check in dell-laptop this one does depend on
>>>>> the use_native_backlight setting.
>>>>
>>>> It depends (see my previous mail). Here is code:
>>>>
>>>> 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;
>>>>
>>>> }
>>
>> Not sure what you're trying to say here, the default for
>> this is 1, so if you don't specify anything, then this:
>>
>> bool acpi_video_verify_backlight_support(void)
>> {
>>         if (acpi_osi_is_win8() &&
>> acpi_video_use_native_backlight() &&
>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>         return acpi_video_backlight_support();
>> }
>>
>> Will return false, and you won't get an acpi_video0 backlight
>> interface, only the intel_backlight one, and everything
>> should just work (except for the turning off on low
>> settings).
>>
> 
> When I do not specify param acpi_video_verify_backlight_support() 
> will return false because acpi_osi_is_win8() returns true.
> 
>> Have you tried not using any kernel commandline options? What
>> happens?
>>
> 
> Yes, then kernel create two backlight devices: one raw from i915 
> module and one normal from dell-laptop module.

Erm, no it won't, unless you've other kernel commandline options
active as well. dell-laptop contains this:

#ifdef CONFIG_ACPI
        /* In the event of an ACPI backlight being available, don't
         * register the platform controller.
         */

        if (acpi_video_backlight_support())
                return 0;
#endif

And acpi_video_backlight_support() will still report 1, so dell-laptop
will not register a backlight interface unless your meddling with things /
config options.


> Userspace will 
> pick dell-laptop for using. And due to commit 0b9f7d93 (which 
> disabling events when acpi_video_verify_backlight_support returns 
> false) dell-laptop backlight not working. So controlling 
> brightness from userspace does not work.

Right, so stop doing whatever you are doing which causes dell-laptop to
register a backlight interface as by default it will not do that, and
your problem is gone.

>>>>> I've just checked 3.17 on my E6430 and there this works as
>>>>> it should, I only get the intel_backlight in
>>>>> /sys/class/backlight
>>>>>
>>>>>> Also dell-laptop (or acpi video) backlight
>>>>>> interface is created (depends on use_native_backlight
>>>>>> param). And userspace will pick dell-laptop (or acpi
>>>>>> video) to use (which cannot change brightness).
>>>>>>
>>>>>>> Why would you want to use dell-laptop despite the i915
>>>>>>> driver working fine ?
>>>>>>
>>>>>> i915 working only if I compile kernel without acpi video
>>>>>> dell- laptop support (distributions compile kernel with
>>>>>> these drivers) and i915 is not good for usage. First it
>>>>>> provides more then thousand brightness levels and lot of
>>>>>> (with low numbers) completely turn display off. And if
>>>>>> userspace map these thousand levels into 5-10 levels (as
>>>>>> nobody want to press brightness key up/down 1000x) two
>>>>>> lowest levels cause display off.
>>>>>
>>>>> More and more laptops will only have working backlight
>>>>> control at all when using i915, so userspace will need to
>>>>> learn to better deal with backlight controls with these
>>>>> ranges. And low pwm levels turning the backlight of is
>>>>> normal for raw interfaces, if userspace does not want
>>>>> this, then they should not go so low.
>>>>
>>>> So then kernel should report which low levels turn
>>>> backlight off so userspace will know which levels should
>>>> avoid. But this is not implemented yet.
>>>>
>>>>> I suggest that you file a bug against your desktop
>>>>> environment that it is not using the backlight controls in
>>>>> an optimal manner, from the kernel pov everything is
>>>>> working fine.
>>>>
>>>> Once I will know which levels should not DE use I can
>>>> report bug.
>>>>
>>>>>> With acpi
>>>>>> video and dell-laptop driver levels are mapped into small
>>>>>> level space in perfect way. So this is reason I want to
>>>>>> use dell-laptop for controlling brightness.
>>>>>
>>>>> If you want to use dell-laptop, specify
>>>>> acpi_backlight=vendor on the kernel commandline, that will
>>>>> give you dell_laptop + intel_backlight as backlight
>>>>> interfaces, and userspace should prefer dell_laptop.
>>>>
>>>> With acpi_backlight=vendor dell-laptop is not working, see
>>>> my previous mail. In this case intel i915 drm driver
>>>> ignore bios events for changing brightness. And userspace
>>>> prefer to use dell_laptop which do nothing!
>>>
>>> Ok, that problematic commit is:
>>>
>>> ACPI / i915: ignore firmware requests for backlight change
>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
>>>
>>> When I reverted it acpi_backlight=vendor started working
>>> again (via dell_laptop). Without reverting that commit
>>> dell_laptop simply do nothing.
>>>
>>> Tested and on my laptop Dell Latitude E6440 driver
>>> dell_laptop does not work with above commit.
>>
>> Hmm, interesting, so when dell-laptop registers and makes a
>> few calls using the dell-laptop acpi interface,
> 
> No, dell-laptop is *not* acpi driver. See my first mail. It uses 
> dell dcdbas driver which makes SMI calls for SMBIOS. But it (on 
> my machine! no idea about other older once) just forward 
> brightness change to intel driver. And it has useful brightness 
> levels (no lot of levels which turning display off).
> 
> And making SMI calls can be done also from userspace. There is 
> tool dellLcdBrightness in libsmbios package which for brightness. 
> And commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe broke 
> functionality of that tool.
> 
>> then you either stop getting key press events through the
>> acpi-video-bus, or dell-laptop is just a shim around the i915
>> interface with the firmware calling into itself on these
>> models. Given that dell-laptop is ancient, the shim story is
>> not that far fetched.
>>
> 
> Brightness Fn keys are reported by WMI (dell-wmi driver), so they 
> working without dell-laptop and acpi video drivers perfectly.
> 
>> Do you still get an on screen display showing the brightness
>> when using a kernel without this patch +
>> acpi_backlight=vendor ?
>>
> 
> Brightness Fn keys are reported to userspace (from WMI input 
> device) with any combination of video.use_native_backlight and 
> acpi_backlight kernel params.

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Hans de Goede]

Hi,

On 09/24/2014 02:53 PM, Pali Rohár wrote:
> On Wednesday 24 September 2014 14:04:36 Hans de Goede wrote:
>> Hi,
>>
>> On 09/24/2014 11:14 AM, Pali Rohár wrote:
>>> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
>>>> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
>>>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> after big changes in acpi video/i915 code I cannot
>>>>>>>> change display brightness on my Dell Latitude E6440
>>>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
>>>>>>>> worked fine.
>>>>>>>>
>>>>>>>> More information about this problem:
>>>>>>>>
>>>>>>>> For configuring brightness on Dell laptops there are 4
>>>>>>>> ways: 1) via acpi video driver
>>>>>>>> 2) via dell-laptop driver
>>>>>>>> 3) via i915 drm driver
>>>>>>>> 4) from userspace with special dell SMI call
>>>>>>>>
>>>>>>>>     (e.g with program dellLcdBrightness from libsmbios
>>>>>>>>     package)
>>>>>>>>
>>>>>>>> Methods 2) and 4) are same, both making special SMI
>>>>>>>> call and Bios handing this request (just 2 is from
>>>>>>>> kernel and 4 from userspace)
>>>>>>>>
>>>>>>>> Method 1) via acpi video driver working, but is not
>>>>>>>> perfect. Driver can be used to change brightness (but
>>>>>>>> only some levels, probably this depends on acpi/DSDT
>>>>>>>> tables), but cannot be used to retrieve current
>>>>>>>> brightness (when BIOS/SMI change brightness acpi driver
>>>>>>>> report old incorrect value). So I prefer dell-laptop
>>>>>>>> driver instead acpi video.
>>>>>>>>
>>>>>>>> Method 3) working even with 3.17-rc6 kernel but because
>>>>>>>> that backlight device exported by i915 is marked as
>>>>>>>> raw, desktop programs prefer to use other devices.
>>>>>>>>
>>>>>>>> Moreover it looks like that methods 1) 2) and 4) just
>>>>>>>> forward request to method 3). So in any cased
>>>>>>>> brightness is changed by i915 drm driver.
>>>>>>>>
>>>>>>>> I'm not sure (correct me if I'm wrong!) but I think
>>>>>>>> that intel i915 drm driver accept changes (file
>>>>>>>> intel_opregion.c) only if acpi function
>>>>>>>> acpi_video_verify_backlight_support() returns true.
>>>>>>>>
>>>>>>>> Function acpi_video_verify_backlight_support() returns
>>>>>>>> true iff: function acpi_video_backlight_support()
>>>>>>>> returns true AND at least one of these functions
>>>>>>>> returns false: acpi_osi_is_win8()
>>>>>>>> acpi_video_use_native_backlight()
>>>>>>>> backlight_device_registered(BACKLIGHT_RAW)
>>>>>>>>
>>>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
>>>>>>>> win8 compliant),
>>>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns true
>>>>>>>> as I'm using intel i915 drm driver with raw backlight
>>>>>>>> device and acpi_video_use_native_backlight() returns
>>>>>>>> true/false depending on
>>>>>>>> "video.use_native_backlight" kernel param. Default is
>>>>>>>> true.
>>>>>>>>
>>>>>>>> So if I want to have working acpi video driver with
>>>>>>>> display brightness support I need to boot kernel with
>>>>>>>> param: "video.use_native_backlight=0". I tested it with
>>>>>>>> kernel 3.17-rc6 and this param really enabled display
>>>>>>>> brightness support via acpi video driver -- which is
>>>>>>>> good.
>>>>>>>>
>>>>>>>> Driver dell-laptop creating backligh device for
>>>>>>>> brightness control only if
>>>>>>>> acpi_video_backlight_support() returns false. There is
>>>>>>>> complicated condition for it and when kernel is booted
>>>>>>>> with "video.use_native_backlight=0" that function
>>>>>>>> returns true.
>>>>>>>>
>>>>>>>> So conclusion is: With current code in kernel 3.17-rc6
>>>>>>>> it is not possible to control brightness of display
>>>>>>>> with native driver dell-laptop on Dell Latitude E6440
>>>>>>>> (and probably on others too)!!!
>>>>>>>>
>>>>>>>> And Because laptop is win8 compliant and you create
>>>>>>>> decision to use native driver (instead acpi one) it is
>>>>>>>> not possible to control display brightness without
>>>>>>>> tweeks in kernel cmdline.
>>>>>>>>
>>>>>>>> As I wrote I would rather to use native dell-laptop
>>>>>>>> driver for controlling brightness, but it is not
>>>>>>>> possible.
>>>>>>>>
>>>>>>>> So how to solve this problem?
>>>>>>>>
>>>>>>>> Quick solution would be to set use_native_backlight
>>>>>>>> false for some Dell laptops which means, that acpi
>>>>>>>> video will be used and in this case intel i915 driver
>>>>>>>> will *not* drop backlight change request.
>>>>>>>>
>>>>>>>> Another solution could be to disable check in
>>>>>>>> dell_laptop driver and add use_native_backlight=0 to
>>>>>>>> hooks. But this create two backlight interfaces (which
>>>>>>>> is not good), but only way (for now) how to make
>>>>>>>> dell_laptop working again.
>>>>>>>>
>>>>>>>> Better and maybe only one proper solution would be to
>>>>>>>> teach intel drm i915 driver to not drop backlight
>>>>>>>> change request for Dell laptops (or all??). (This
>>>>>>>> allows to work both acpi video and dell_laptop drivers
>>>>>>>> without any change and with *any* value in param
>>>>>>>> use_native_backlight). I think that problematic code is
>>>>>>>> in function asle_set_backlight() in file
>>>>>>>> intel_opregion.c (but I'm not sure). My idea is that
>>>>>>>> "drop" event function it is caused by this commit
>>>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
>>>>>>>> sure).
>>>>>>>>
>>>>>>>> At least something must be done as I think that I'm not
>>>>>>>> only one who has Dell laptop and brightness
>>>>>>>> configuration is really important...
>>>>>>>
>>>>>>> I don't understand your problem, the kernel is selecting
>>>>>>> the i915 backlight driver, making that the only one
>>>>>>> available to userspace, so the one problem you state
>>>>>>> with the i915 driver, that it is "raw" is not an issue,
>>>>>>> as userspace will pick it when it is the only one.
>>>>>>
>>>>>> It is not only one.
>>>>>
>>>>> Are you sure, if you do not specify any commandline
>>>>> parameters, then neither dell-laptop nor acpi-video should
>>>>> register a backlight interface.
>>>>>
>>>>> dell-laptop.c has:
>>>>>
>>>>> #ifdef CONFIG_ACPI
>>>>>
>>>>>         /* In the event of an ACPI backlight being
>>>>>         available,
>>>>>
>>>>> don't * register the platform controller.
>>>>>
>>>>>          */
>>>>>         
>>>>>         if (acpi_video_backlight_support())
>>>>>         
>>>>>                 return 0;
>>>>>
>>>>> #endif
>>>>>
>>>>> And acpi_video_backlight_support() will (normally) return
>>>>> true on acpi-backlight capable systems independent of
>>>>> use_native_backlight.
>>>>>
>>>>> And drivers/acpi/video.c has this:
>>>>>         /* no warning message if acpi_backlight=vendor or
>>>>>         a
>>>>>
>>>>> quirk is used */ if
>>>>> (!acpi_video_verify_backlight_support())
>>>>>
>>>>>                 return;
>>>>>
>>>>> ...
>>>>>
>>>>> bool acpi_video_verify_backlight_support(void)
>>>>> {
>>>>>
>>>>>         if (acpi_osi_is_win8() &&
>>>>>
>>>>> acpi_video_use_native_backlight() &&
>>>>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>>>>
>>>>>         return acpi_video_backlight_support();
>>>>>
>>>>> }
>>>>>
>>>>> So unlike the check in dell-laptop this one does depend on
>>>>> the use_native_backlight setting.
>>>>
>>>> It depends (see my previous mail). Here is code:
>>>>
>>>> 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;
>>>>
>>>> }
>>
>> Not sure what you're trying to say here, the default for
>> this is 1, so if you don't specify anything, then this:
>>
>> bool acpi_video_verify_backlight_support(void)
>> {
>>         if (acpi_osi_is_win8() &&
>> acpi_video_use_native_backlight() &&
>> backlight_device_registered(BACKLIGHT_RAW)) return false;
>>         return acpi_video_backlight_support();
>> }
>>
>> Will return false, and you won't get an acpi_video0 backlight
>> interface, only the intel_backlight one, and everything
>> should just work (except for the turning off on low
>> settings).
>>
> 
> When I do not specify param acpi_video_verify_backlight_support() 
> will return false because acpi_osi_is_win8() returns true.
> 
>> Have you tried not using any kernel commandline options? What
>> happens?
>>
> 
> Yes, then kernel create two backlight devices: one raw from i915 
> module and one normal from dell-laptop module.

Erm, no it won't, unless you've other kernel commandline options
active as well. dell-laptop contains this:

#ifdef CONFIG_ACPI
        /* In the event of an ACPI backlight being available, don't
         * register the platform controller.
         */

        if (acpi_video_backlight_support())
                return 0;
#endif

And acpi_video_backlight_support() will still report 1, so dell-laptop
will not register a backlight interface unless your meddling with things /
config options.


> Userspace will 
> pick dell-laptop for using. And due to commit 0b9f7d93 (which 
> disabling events when acpi_video_verify_backlight_support returns 
> false) dell-laptop backlight not working. So controlling 
> brightness from userspace does not work.

Right, so stop doing whatever you are doing which causes dell-laptop to
register a backlight interface as by default it will not do that, and
your problem is gone.

>>>>> I've just checked 3.17 on my E6430 and there this works as
>>>>> it should, I only get the intel_backlight in
>>>>> /sys/class/backlight
>>>>>
>>>>>> Also dell-laptop (or acpi video) backlight
>>>>>> interface is created (depends on use_native_backlight
>>>>>> param). And userspace will pick dell-laptop (or acpi
>>>>>> video) to use (which cannot change brightness).
>>>>>>
>>>>>>> Why would you want to use dell-laptop despite the i915
>>>>>>> driver working fine ?
>>>>>>
>>>>>> i915 working only if I compile kernel without acpi video
>>>>>> dell- laptop support (distributions compile kernel with
>>>>>> these drivers) and i915 is not good for usage. First it
>>>>>> provides more then thousand brightness levels and lot of
>>>>>> (with low numbers) completely turn display off. And if
>>>>>> userspace map these thousand levels into 5-10 levels (as
>>>>>> nobody want to press brightness key up/down 1000x) two
>>>>>> lowest levels cause display off.
>>>>>
>>>>> More and more laptops will only have working backlight
>>>>> control at all when using i915, so userspace will need to
>>>>> learn to better deal with backlight controls with these
>>>>> ranges. And low pwm levels turning the backlight of is
>>>>> normal for raw interfaces, if userspace does not want
>>>>> this, then they should not go so low.
>>>>
>>>> So then kernel should report which low levels turn
>>>> backlight off so userspace will know which levels should
>>>> avoid. But this is not implemented yet.
>>>>
>>>>> I suggest that you file a bug against your desktop
>>>>> environment that it is not using the backlight controls in
>>>>> an optimal manner, from the kernel pov everything is
>>>>> working fine.
>>>>
>>>> Once I will know which levels should not DE use I can
>>>> report bug.
>>>>
>>>>>> With acpi
>>>>>> video and dell-laptop driver levels are mapped into small
>>>>>> level space in perfect way. So this is reason I want to
>>>>>> use dell-laptop for controlling brightness.
>>>>>
>>>>> If you want to use dell-laptop, specify
>>>>> acpi_backlight=vendor on the kernel commandline, that will
>>>>> give you dell_laptop + intel_backlight as backlight
>>>>> interfaces, and userspace should prefer dell_laptop.
>>>>
>>>> With acpi_backlight=vendor dell-laptop is not working, see
>>>> my previous mail. In this case intel i915 drm driver
>>>> ignore bios events for changing brightness. And userspace
>>>> prefer to use dell_laptop which do nothing!
>>>
>>> Ok, that problematic commit is:
>>>
>>> ACPI / i915: ignore firmware requests for backlight change
>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
>>>
>>> When I reverted it acpi_backlight=vendor started working
>>> again (via dell_laptop). Without reverting that commit
>>> dell_laptop simply do nothing.
>>>
>>> Tested and on my laptop Dell Latitude E6440 driver
>>> dell_laptop does not work with above commit.
>>
>> Hmm, interesting, so when dell-laptop registers and makes a
>> few calls using the dell-laptop acpi interface,
> 
> No, dell-laptop is *not* acpi driver. See my first mail. It uses 
> dell dcdbas driver which makes SMI calls for SMBIOS. But it (on 
> my machine! no idea about other older once) just forward 
> brightness change to intel driver. And it has useful brightness 
> levels (no lot of levels which turning display off).
> 
> And making SMI calls can be done also from userspace. There is 
> tool dellLcdBrightness in libsmbios package which for brightness. 
> And commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe broke 
> functionality of that tool.
> 
>> then you either stop getting key press events through the
>> acpi-video-bus, or dell-laptop is just a shim around the i915
>> interface with the firmware calling into itself on these
>> models. Given that dell-laptop is ancient, the shim story is
>> not that far fetched.
>>
> 
> Brightness Fn keys are reported by WMI (dell-wmi driver), so they 
> working without dell-laptop and acpi video drivers perfectly.
> 
>> Do you still get an on screen display showing the brightness
>> when using a kernel without this patch +
>> acpi_backlight=vendor ?
>>
> 
> Brightness Fn keys are reported to userspace (from WMI input 
> device) with any combination of video.use_native_backlight and 
> acpi_backlight kernel params.

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

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1.1: Type: Text/Plain, Size: 16207 bytes --]

On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
> Hi,
> 
> On 09/24/2014 02:53 PM, Pali Rohár wrote:
> > On Wednesday 24 September 2014 14:04:36 Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 09/24/2014 11:14 AM, Pali Rohár wrote:
> >>> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
> >>>> On Wednesday 24 September 2014 10:19:38 Hans de Goede 
wrote:
> >>>>> Hi,
> >>>>> 
> >>>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
> >>>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
> >>>>>>> Hi,
> >>>>>>> 
> >>>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> >>>>>>>> Hello,
> >>>>>>>> 
> >>>>>>>> after big changes in acpi video/i915 code I cannot
> >>>>>>>> change display brightness on my Dell Latitude E6440
> >>>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
> >>>>>>>> worked fine.
> >>>>>>>> 
> >>>>>>>> More information about this problem:
> >>>>>>>> 
> >>>>>>>> For configuring brightness on Dell laptops there are
> >>>>>>>> 4 ways: 1) via acpi video driver
> >>>>>>>> 2) via dell-laptop driver
> >>>>>>>> 3) via i915 drm driver
> >>>>>>>> 4) from userspace with special dell SMI call
> >>>>>>>> 
> >>>>>>>>     (e.g with program dellLcdBrightness from
> >>>>>>>>     libsmbios package)
> >>>>>>>> 
> >>>>>>>> Methods 2) and 4) are same, both making special SMI
> >>>>>>>> call and Bios handing this request (just 2 is from
> >>>>>>>> kernel and 4 from userspace)
> >>>>>>>> 
> >>>>>>>> Method 1) via acpi video driver working, but is not
> >>>>>>>> perfect. Driver can be used to change brightness (but
> >>>>>>>> only some levels, probably this depends on acpi/DSDT
> >>>>>>>> tables), but cannot be used to retrieve current
> >>>>>>>> brightness (when BIOS/SMI change brightness acpi
> >>>>>>>> driver report old incorrect value). So I prefer
> >>>>>>>> dell-laptop driver instead acpi video.
> >>>>>>>> 
> >>>>>>>> Method 3) working even with 3.17-rc6 kernel but
> >>>>>>>> because that backlight device exported by i915 is
> >>>>>>>> marked as raw, desktop programs prefer to use other
> >>>>>>>> devices.
> >>>>>>>> 
> >>>>>>>> Moreover it looks like that methods 1) 2) and 4) just
> >>>>>>>> forward request to method 3). So in any cased
> >>>>>>>> brightness is changed by i915 drm driver.
> >>>>>>>> 
> >>>>>>>> I'm not sure (correct me if I'm wrong!) but I think
> >>>>>>>> that intel i915 drm driver accept changes (file
> >>>>>>>> intel_opregion.c) only if acpi function
> >>>>>>>> acpi_video_verify_backlight_support() returns true.
> >>>>>>>> 
> >>>>>>>> Function acpi_video_verify_backlight_support()
> >>>>>>>> returns true iff: function
> >>>>>>>> acpi_video_backlight_support() returns true AND at
> >>>>>>>> least one of these functions returns false:
> >>>>>>>> acpi_osi_is_win8()
> >>>>>>>> acpi_video_use_native_backlight()
> >>>>>>>> backlight_device_registered(BACKLIGHT_RAW)
> >>>>>>>> 
> >>>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
> >>>>>>>> win8 compliant),
> >>>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns
> >>>>>>>> true as I'm using intel i915 drm driver with raw
> >>>>>>>> backlight device and
> >>>>>>>> acpi_video_use_native_backlight() returns true/false
> >>>>>>>> depending on
> >>>>>>>> "video.use_native_backlight" kernel param. Default is
> >>>>>>>> true.
> >>>>>>>> 
> >>>>>>>> So if I want to have working acpi video driver with
> >>>>>>>> display brightness support I need to boot kernel with
> >>>>>>>> param: "video.use_native_backlight=0". I tested it
> >>>>>>>> with kernel 3.17-rc6 and this param really enabled
> >>>>>>>> display brightness support via acpi video driver --
> >>>>>>>> which is good.
> >>>>>>>> 
> >>>>>>>> Driver dell-laptop creating backligh device for
> >>>>>>>> brightness control only if
> >>>>>>>> acpi_video_backlight_support() returns false. There
> >>>>>>>> is complicated condition for it and when kernel is
> >>>>>>>> booted with "video.use_native_backlight=0" that
> >>>>>>>> function returns true.
> >>>>>>>> 
> >>>>>>>> So conclusion is: With current code in kernel
> >>>>>>>> 3.17-rc6 it is not possible to control brightness of
> >>>>>>>> display with native driver dell-laptop on Dell
> >>>>>>>> Latitude E6440 (and probably on others too)!!!
> >>>>>>>> 
> >>>>>>>> And Because laptop is win8 compliant and you create
> >>>>>>>> decision to use native driver (instead acpi one) it
> >>>>>>>> is not possible to control display brightness
> >>>>>>>> without tweeks in kernel cmdline.
> >>>>>>>> 
> >>>>>>>> As I wrote I would rather to use native dell-laptop
> >>>>>>>> driver for controlling brightness, but it is not
> >>>>>>>> possible.
> >>>>>>>> 
> >>>>>>>> So how to solve this problem?
> >>>>>>>> 
> >>>>>>>> Quick solution would be to set use_native_backlight
> >>>>>>>> false for some Dell laptops which means, that acpi
> >>>>>>>> video will be used and in this case intel i915 driver
> >>>>>>>> will *not* drop backlight change request.
> >>>>>>>> 
> >>>>>>>> Another solution could be to disable check in
> >>>>>>>> dell_laptop driver and add use_native_backlight=0 to
> >>>>>>>> hooks. But this create two backlight interfaces
> >>>>>>>> (which is not good), but only way (for now) how to
> >>>>>>>> make dell_laptop working again.
> >>>>>>>> 
> >>>>>>>> Better and maybe only one proper solution would be to
> >>>>>>>> teach intel drm i915 driver to not drop backlight
> >>>>>>>> change request for Dell laptops (or all??). (This
> >>>>>>>> allows to work both acpi video and dell_laptop
> >>>>>>>> drivers without any change and with *any* value in
> >>>>>>>> param use_native_backlight). I think that
> >>>>>>>> problematic code is in function asle_set_backlight()
> >>>>>>>> in file
> >>>>>>>> intel_opregion.c (but I'm not sure). My idea is that
> >>>>>>>> "drop" event function it is caused by this commit
> >>>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> >>>>>>>> sure).
> >>>>>>>> 
> >>>>>>>> At least something must be done as I think that I'm
> >>>>>>>> not only one who has Dell laptop and brightness
> >>>>>>>> configuration is really important...
> >>>>>>> 
> >>>>>>> I don't understand your problem, the kernel is
> >>>>>>> selecting the i915 backlight driver, making that the
> >>>>>>> only one available to userspace, so the one problem
> >>>>>>> you state with the i915 driver, that it is "raw" is
> >>>>>>> not an issue, as userspace will pick it when it is
> >>>>>>> the only one.
> >>>>>> 
> >>>>>> It is not only one.
> >>>>> 
> >>>>> Are you sure, if you do not specify any commandline
> >>>>> parameters, then neither dell-laptop nor acpi-video
> >>>>> should register a backlight interface.
> >>>>> 
> >>>>> dell-laptop.c has:
> >>>>> 
> >>>>> #ifdef CONFIG_ACPI
> >>>>> 
> >>>>>         /* In the event of an ACPI backlight being
> >>>>>         available,
> >>>>> 
> >>>>> don't * register the platform controller.
> >>>>> 
> >>>>>          */
> >>>>>         
> >>>>>         if (acpi_video_backlight_support())
> >>>>>         
> >>>>>                 return 0;
> >>>>> 
> >>>>> #endif
> >>>>> 
> >>>>> And acpi_video_backlight_support() will (normally)
> >>>>> return true on acpi-backlight capable systems
> >>>>> independent of use_native_backlight.
> >>>>> 
> >>>>> And drivers/acpi/video.c has this:
> >>>>>         /* no warning message if acpi_backlight=vendor
> >>>>>         or a
> >>>>> 
> >>>>> quirk is used */ if
> >>>>> (!acpi_video_verify_backlight_support())
> >>>>> 
> >>>>>                 return;
> >>>>> 
> >>>>> ...
> >>>>> 
> >>>>> bool acpi_video_verify_backlight_support(void)
> >>>>> {
> >>>>> 
> >>>>>         if (acpi_osi_is_win8() &&
> >>>>> 
> >>>>> acpi_video_use_native_backlight() &&
> >>>>> backlight_device_registered(BACKLIGHT_RAW)) return
> >>>>> false;
> >>>>> 
> >>>>>         return acpi_video_backlight_support();
> >>>>> 
> >>>>> }
> >>>>> 
> >>>>> So unlike the check in dell-laptop this one does depend
> >>>>> on the use_native_backlight setting.
> >>>> 
> >>>> It depends (see my previous mail). Here is code:
> >>>> 
> >>>> 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;
> >>>> 
> >>>> }
> >> 
> >> Not sure what you're trying to say here, the default for
> >> this is 1, so if you don't specify anything, then this:
> >> 
> >> bool acpi_video_verify_backlight_support(void)
> >> {
> >> 
> >>         if (acpi_osi_is_win8() &&
> >> 
> >> acpi_video_use_native_backlight() &&
> >> backlight_device_registered(BACKLIGHT_RAW)) return false;
> >> 
> >>         return acpi_video_backlight_support();
> >> 
> >> }
> >> 
> >> Will return false, and you won't get an acpi_video0
> >> backlight interface, only the intel_backlight one, and
> >> everything should just work (except for the turning off on
> >> low settings).
> > 
> > When I do not specify param
> > acpi_video_verify_backlight_support() will return false
> > because acpi_osi_is_win8() returns true.
> > 
> >> Have you tried not using any kernel commandline options?
> >> What happens?
> > 
> > Yes, then kernel create two backlight devices: one raw from
> > i915 module and one normal from dell-laptop module.
> 
> Erm, no it won't, unless you've other kernel commandline
> options active as well. dell-laptop contains this:
> 
> #ifdef CONFIG_ACPI
>         /* In the event of an ACPI backlight being available,
> don't * register the platform controller.
>          */
> 
>         if (acpi_video_backlight_support())
>                 return 0;
> #endif
> 
> And acpi_video_backlight_support() will still report 1, so
> dell-laptop will not register a backlight interface unless
> your meddling with things / config options.
> 

Right, sorry for this. Without *any* kernel cmdline only intel 
backlight is registered and userspace will pick up it.

> > Userspace will
> > pick dell-laptop for using. And due to commit 0b9f7d93
> > (which disabling events when
> > acpi_video_verify_backlight_support returns false)
> > dell-laptop backlight not working. So controlling
> > brightness from userspace does not work.
> 
> Right, so stop doing whatever you are doing which causes
> dell-laptop to register a backlight interface as by default
> it will not do that, and your problem is gone.
> 
> >>>>> I've just checked 3.17 on my E6430 and there this works
> >>>>> as it should, I only get the intel_backlight in
> >>>>> /sys/class/backlight
> >>>>> 
> >>>>>> Also dell-laptop (or acpi video) backlight
> >>>>>> interface is created (depends on use_native_backlight
> >>>>>> param). And userspace will pick dell-laptop (or acpi
> >>>>>> video) to use (which cannot change brightness).
> >>>>>> 
> >>>>>>> Why would you want to use dell-laptop despite the i915
> >>>>>>> driver working fine ?
> >>>>>> 
> >>>>>> i915 working only if I compile kernel without acpi
> >>>>>> video dell- laptop support (distributions compile
> >>>>>> kernel with these drivers) and i915 is not good for
> >>>>>> usage. First it provides more then thousand brightness
> >>>>>> levels and lot of (with low numbers) completely turn
> >>>>>> display off. And if userspace map these thousand
> >>>>>> levels into 5-10 levels (as nobody want to press
> >>>>>> brightness key up/down 1000x) two lowest levels cause
> >>>>>> display off.
> >>>>> 
> >>>>> More and more laptops will only have working backlight
> >>>>> control at all when using i915, so userspace will need
> >>>>> to learn to better deal with backlight controls with
> >>>>> these ranges. And low pwm levels turning the backlight
> >>>>> of is normal for raw interfaces, if userspace does not
> >>>>> want this, then they should not go so low.
> >>>> 
> >>>> So then kernel should report which low levels turn
> >>>> backlight off so userspace will know which levels should
> >>>> avoid. But this is not implemented yet.
> >>>> 
> >>>>> I suggest that you file a bug against your desktop
> >>>>> environment that it is not using the backlight controls
> >>>>> in an optimal manner, from the kernel pov everything is
> >>>>> working fine.
> >>>> 
> >>>> Once I will know which levels should not DE use I can
> >>>> report bug.
> >>>> 
> >>>>>> With acpi
> >>>>>> video and dell-laptop driver levels are mapped into
> >>>>>> small level space in perfect way. So this is reason I
> >>>>>> want to use dell-laptop for controlling brightness.
> >>>>> 
> >>>>> If you want to use dell-laptop, specify
> >>>>> acpi_backlight=vendor on the kernel commandline, that
> >>>>> will give you dell_laptop + intel_backlight as
> >>>>> backlight interfaces, and userspace should prefer
> >>>>> dell_laptop.
> >>>> 
> >>>> With acpi_backlight=vendor dell-laptop is not working,
> >>>> see my previous mail. In this case intel i915 drm driver
> >>>> ignore bios events for changing brightness. And
> >>>> userspace prefer to use dell_laptop which do nothing!
> >>> 
> >>> Ok, that problematic commit is:
> >>> 
> >>> ACPI / i915: ignore firmware requests for backlight change
> >>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
> >>> 
> >>> When I reverted it acpi_backlight=vendor started working
> >>> again (via dell_laptop). Without reverting that commit
> >>> dell_laptop simply do nothing.
> >>> 
> >>> Tested and on my laptop Dell Latitude E6440 driver
> >>> dell_laptop does not work with above commit.
> >> 
> >> Hmm, interesting, so when dell-laptop registers and makes a
> >> few calls using the dell-laptop acpi interface,
> > 
> > No, dell-laptop is *not* acpi driver. See my first mail. It
> > uses dell dcdbas driver which makes SMI calls for SMBIOS.
> > But it (on my machine! no idea about other older once) just
> > forward brightness change to intel driver. And it has
> > useful brightness levels (no lot of levels which turning
> > display off).
> > 
> > And making SMI calls can be done also from userspace. There
> > is tool dellLcdBrightness in libsmbios package which for
> > brightness. And commit
> > 0b9f7d93ca6109048a4eb06332b666b6e29df4fe broke
> > functionality of that tool.
> > 
> >> then you either stop getting key press events through the
> >> acpi-video-bus, or dell-laptop is just a shim around the
> >> i915 interface with the firmware calling into itself on
> >> these models. Given that dell-laptop is ancient, the shim
> >> story is not that far fetched.
> > 
> > Brightness Fn keys are reported by WMI (dell-wmi driver), so
> > they working without dell-laptop and acpi video drivers
> > perfectly.
> > 
> >> Do you still get an on screen display showing the
> >> brightness when using a kernel without this patch +
> >> acpi_backlight=vendor ?
> > 
> > Brightness Fn keys are reported to userspace (from WMI input
> > device) with any combination of video.use_native_backlight
> > and acpi_backlight kernel params.
> 
> 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.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 16207 bytes --]

On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
> Hi,
> 
> On 09/24/2014 02:53 PM, Pali Rohár wrote:
> > On Wednesday 24 September 2014 14:04:36 Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 09/24/2014 11:14 AM, Pali Rohár wrote:
> >>> On Wednesday 24 September 2014 10:59:41 Pali Rohár wrote:
> >>>> On Wednesday 24 September 2014 10:19:38 Hans de Goede 
wrote:
> >>>>> Hi,
> >>>>> 
> >>>>> On 09/23/2014 10:44 PM, Pali Rohár wrote:
> >>>>>> On Tuesday 23 September 2014 22:31:31 you wrote:
> >>>>>>> Hi,
> >>>>>>> 
> >>>>>>> On 09/23/2014 10:06 PM, Pali Rohár wrote:
> >>>>>>>> Hello,
> >>>>>>>> 
> >>>>>>>> after big changes in acpi video/i915 code I cannot
> >>>>>>>> change display brightness on my Dell Latitude E6440
> >>>>>>>> with kernel 3.17-rc6. With kernel 3.13 everything
> >>>>>>>> worked fine.
> >>>>>>>> 
> >>>>>>>> More information about this problem:
> >>>>>>>> 
> >>>>>>>> For configuring brightness on Dell laptops there are
> >>>>>>>> 4 ways: 1) via acpi video driver
> >>>>>>>> 2) via dell-laptop driver
> >>>>>>>> 3) via i915 drm driver
> >>>>>>>> 4) from userspace with special dell SMI call
> >>>>>>>> 
> >>>>>>>>     (e.g with program dellLcdBrightness from
> >>>>>>>>     libsmbios package)
> >>>>>>>> 
> >>>>>>>> Methods 2) and 4) are same, both making special SMI
> >>>>>>>> call and Bios handing this request (just 2 is from
> >>>>>>>> kernel and 4 from userspace)
> >>>>>>>> 
> >>>>>>>> Method 1) via acpi video driver working, but is not
> >>>>>>>> perfect. Driver can be used to change brightness (but
> >>>>>>>> only some levels, probably this depends on acpi/DSDT
> >>>>>>>> tables), but cannot be used to retrieve current
> >>>>>>>> brightness (when BIOS/SMI change brightness acpi
> >>>>>>>> driver report old incorrect value). So I prefer
> >>>>>>>> dell-laptop driver instead acpi video.
> >>>>>>>> 
> >>>>>>>> Method 3) working even with 3.17-rc6 kernel but
> >>>>>>>> because that backlight device exported by i915 is
> >>>>>>>> marked as raw, desktop programs prefer to use other
> >>>>>>>> devices.
> >>>>>>>> 
> >>>>>>>> Moreover it looks like that methods 1) 2) and 4) just
> >>>>>>>> forward request to method 3). So in any cased
> >>>>>>>> brightness is changed by i915 drm driver.
> >>>>>>>> 
> >>>>>>>> I'm not sure (correct me if I'm wrong!) but I think
> >>>>>>>> that intel i915 drm driver accept changes (file
> >>>>>>>> intel_opregion.c) only if acpi function
> >>>>>>>> acpi_video_verify_backlight_support() returns true.
> >>>>>>>> 
> >>>>>>>> Function acpi_video_verify_backlight_support()
> >>>>>>>> returns true iff: function
> >>>>>>>> acpi_video_backlight_support() returns true AND at
> >>>>>>>> least one of these functions returns false:
> >>>>>>>> acpi_osi_is_win8()
> >>>>>>>> acpi_video_use_native_backlight()
> >>>>>>>> backlight_device_registered(BACKLIGHT_RAW)
> >>>>>>>> 
> >>>>>>>> On my notebook acpi_osi_is_win8() returns true (as is
> >>>>>>>> win8 compliant),
> >>>>>>>> backlight_device_registered(BACKLIGHT_RAW) returns
> >>>>>>>> true as I'm using intel i915 drm driver with raw
> >>>>>>>> backlight device and
> >>>>>>>> acpi_video_use_native_backlight() returns true/false
> >>>>>>>> depending on
> >>>>>>>> "video.use_native_backlight" kernel param. Default is
> >>>>>>>> true.
> >>>>>>>> 
> >>>>>>>> So if I want to have working acpi video driver with
> >>>>>>>> display brightness support I need to boot kernel with
> >>>>>>>> param: "video.use_native_backlight=0". I tested it
> >>>>>>>> with kernel 3.17-rc6 and this param really enabled
> >>>>>>>> display brightness support via acpi video driver --
> >>>>>>>> which is good.
> >>>>>>>> 
> >>>>>>>> Driver dell-laptop creating backligh device for
> >>>>>>>> brightness control only if
> >>>>>>>> acpi_video_backlight_support() returns false. There
> >>>>>>>> is complicated condition for it and when kernel is
> >>>>>>>> booted with "video.use_native_backlight=0" that
> >>>>>>>> function returns true.
> >>>>>>>> 
> >>>>>>>> So conclusion is: With current code in kernel
> >>>>>>>> 3.17-rc6 it is not possible to control brightness of
> >>>>>>>> display with native driver dell-laptop on Dell
> >>>>>>>> Latitude E6440 (and probably on others too)!!!
> >>>>>>>> 
> >>>>>>>> And Because laptop is win8 compliant and you create
> >>>>>>>> decision to use native driver (instead acpi one) it
> >>>>>>>> is not possible to control display brightness
> >>>>>>>> without tweeks in kernel cmdline.
> >>>>>>>> 
> >>>>>>>> As I wrote I would rather to use native dell-laptop
> >>>>>>>> driver for controlling brightness, but it is not
> >>>>>>>> possible.
> >>>>>>>> 
> >>>>>>>> So how to solve this problem?
> >>>>>>>> 
> >>>>>>>> Quick solution would be to set use_native_backlight
> >>>>>>>> false for some Dell laptops which means, that acpi
> >>>>>>>> video will be used and in this case intel i915 driver
> >>>>>>>> will *not* drop backlight change request.
> >>>>>>>> 
> >>>>>>>> Another solution could be to disable check in
> >>>>>>>> dell_laptop driver and add use_native_backlight=0 to
> >>>>>>>> hooks. But this create two backlight interfaces
> >>>>>>>> (which is not good), but only way (for now) how to
> >>>>>>>> make dell_laptop working again.
> >>>>>>>> 
> >>>>>>>> Better and maybe only one proper solution would be to
> >>>>>>>> teach intel drm i915 driver to not drop backlight
> >>>>>>>> change request for Dell laptops (or all??). (This
> >>>>>>>> allows to work both acpi video and dell_laptop
> >>>>>>>> drivers without any change and with *any* value in
> >>>>>>>> param use_native_backlight). I think that
> >>>>>>>> problematic code is in function asle_set_backlight()
> >>>>>>>> in file
> >>>>>>>> intel_opregion.c (but I'm not sure). My idea is that
> >>>>>>>> "drop" event function it is caused by this commit
> >>>>>>>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> >>>>>>>> sure).
> >>>>>>>> 
> >>>>>>>> At least something must be done as I think that I'm
> >>>>>>>> not only one who has Dell laptop and brightness
> >>>>>>>> configuration is really important...
> >>>>>>> 
> >>>>>>> I don't understand your problem, the kernel is
> >>>>>>> selecting the i915 backlight driver, making that the
> >>>>>>> only one available to userspace, so the one problem
> >>>>>>> you state with the i915 driver, that it is "raw" is
> >>>>>>> not an issue, as userspace will pick it when it is
> >>>>>>> the only one.
> >>>>>> 
> >>>>>> It is not only one.
> >>>>> 
> >>>>> Are you sure, if you do not specify any commandline
> >>>>> parameters, then neither dell-laptop nor acpi-video
> >>>>> should register a backlight interface.
> >>>>> 
> >>>>> dell-laptop.c has:
> >>>>> 
> >>>>> #ifdef CONFIG_ACPI
> >>>>> 
> >>>>>         /* In the event of an ACPI backlight being
> >>>>>         available,
> >>>>> 
> >>>>> don't * register the platform controller.
> >>>>> 
> >>>>>          */
> >>>>>         
> >>>>>         if (acpi_video_backlight_support())
> >>>>>         
> >>>>>                 return 0;
> >>>>> 
> >>>>> #endif
> >>>>> 
> >>>>> And acpi_video_backlight_support() will (normally)
> >>>>> return true on acpi-backlight capable systems
> >>>>> independent of use_native_backlight.
> >>>>> 
> >>>>> And drivers/acpi/video.c has this:
> >>>>>         /* no warning message if acpi_backlight=vendor
> >>>>>         or a
> >>>>> 
> >>>>> quirk is used */ if
> >>>>> (!acpi_video_verify_backlight_support())
> >>>>> 
> >>>>>                 return;
> >>>>> 
> >>>>> ...
> >>>>> 
> >>>>> bool acpi_video_verify_backlight_support(void)
> >>>>> {
> >>>>> 
> >>>>>         if (acpi_osi_is_win8() &&
> >>>>> 
> >>>>> acpi_video_use_native_backlight() &&
> >>>>> backlight_device_registered(BACKLIGHT_RAW)) return
> >>>>> false;
> >>>>> 
> >>>>>         return acpi_video_backlight_support();
> >>>>> 
> >>>>> }
> >>>>> 
> >>>>> So unlike the check in dell-laptop this one does depend
> >>>>> on the use_native_backlight setting.
> >>>> 
> >>>> It depends (see my previous mail). Here is code:
> >>>> 
> >>>> 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;
> >>>> 
> >>>> }
> >> 
> >> Not sure what you're trying to say here, the default for
> >> this is 1, so if you don't specify anything, then this:
> >> 
> >> bool acpi_video_verify_backlight_support(void)
> >> {
> >> 
> >>         if (acpi_osi_is_win8() &&
> >> 
> >> acpi_video_use_native_backlight() &&
> >> backlight_device_registered(BACKLIGHT_RAW)) return false;
> >> 
> >>         return acpi_video_backlight_support();
> >> 
> >> }
> >> 
> >> Will return false, and you won't get an acpi_video0
> >> backlight interface, only the intel_backlight one, and
> >> everything should just work (except for the turning off on
> >> low settings).
> > 
> > When I do not specify param
> > acpi_video_verify_backlight_support() will return false
> > because acpi_osi_is_win8() returns true.
> > 
> >> Have you tried not using any kernel commandline options?
> >> What happens?
> > 
> > Yes, then kernel create two backlight devices: one raw from
> > i915 module and one normal from dell-laptop module.
> 
> Erm, no it won't, unless you've other kernel commandline
> options active as well. dell-laptop contains this:
> 
> #ifdef CONFIG_ACPI
>         /* In the event of an ACPI backlight being available,
> don't * register the platform controller.
>          */
> 
>         if (acpi_video_backlight_support())
>                 return 0;
> #endif
> 
> And acpi_video_backlight_support() will still report 1, so
> dell-laptop will not register a backlight interface unless
> your meddling with things / config options.
> 

Right, sorry for this. Without *any* kernel cmdline only intel 
backlight is registered and userspace will pick up it.

> > Userspace will
> > pick dell-laptop for using. And due to commit 0b9f7d93
> > (which disabling events when
> > acpi_video_verify_backlight_support returns false)
> > dell-laptop backlight not working. So controlling
> > brightness from userspace does not work.
> 
> Right, so stop doing whatever you are doing which causes
> dell-laptop to register a backlight interface as by default
> it will not do that, and your problem is gone.
> 
> >>>>> I've just checked 3.17 on my E6430 and there this works
> >>>>> as it should, I only get the intel_backlight in
> >>>>> /sys/class/backlight
> >>>>> 
> >>>>>> Also dell-laptop (or acpi video) backlight
> >>>>>> interface is created (depends on use_native_backlight
> >>>>>> param). And userspace will pick dell-laptop (or acpi
> >>>>>> video) to use (which cannot change brightness).
> >>>>>> 
> >>>>>>> Why would you want to use dell-laptop despite the i915
> >>>>>>> driver working fine ?
> >>>>>> 
> >>>>>> i915 working only if I compile kernel without acpi
> >>>>>> video dell- laptop support (distributions compile
> >>>>>> kernel with these drivers) and i915 is not good for
> >>>>>> usage. First it provides more then thousand brightness
> >>>>>> levels and lot of (with low numbers) completely turn
> >>>>>> display off. And if userspace map these thousand
> >>>>>> levels into 5-10 levels (as nobody want to press
> >>>>>> brightness key up/down 1000x) two lowest levels cause
> >>>>>> display off.
> >>>>> 
> >>>>> More and more laptops will only have working backlight
> >>>>> control at all when using i915, so userspace will need
> >>>>> to learn to better deal with backlight controls with
> >>>>> these ranges. And low pwm levels turning the backlight
> >>>>> of is normal for raw interfaces, if userspace does not
> >>>>> want this, then they should not go so low.
> >>>> 
> >>>> So then kernel should report which low levels turn
> >>>> backlight off so userspace will know which levels should
> >>>> avoid. But this is not implemented yet.
> >>>> 
> >>>>> I suggest that you file a bug against your desktop
> >>>>> environment that it is not using the backlight controls
> >>>>> in an optimal manner, from the kernel pov everything is
> >>>>> working fine.
> >>>> 
> >>>> Once I will know which levels should not DE use I can
> >>>> report bug.
> >>>> 
> >>>>>> With acpi
> >>>>>> video and dell-laptop driver levels are mapped into
> >>>>>> small level space in perfect way. So this is reason I
> >>>>>> want to use dell-laptop for controlling brightness.
> >>>>> 
> >>>>> If you want to use dell-laptop, specify
> >>>>> acpi_backlight=vendor on the kernel commandline, that
> >>>>> will give you dell_laptop + intel_backlight as
> >>>>> backlight interfaces, and userspace should prefer
> >>>>> dell_laptop.
> >>>> 
> >>>> With acpi_backlight=vendor dell-laptop is not working,
> >>>> see my previous mail. In this case intel i915 drm driver
> >>>> ignore bios events for changing brightness. And
> >>>> userspace prefer to use dell_laptop which do nothing!
> >>> 
> >>> Ok, that problematic commit is:
> >>> 
> >>> ACPI / i915: ignore firmware requests for backlight change
> >>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe
> >>> 
> >>> When I reverted it acpi_backlight=vendor started working
> >>> again (via dell_laptop). Without reverting that commit
> >>> dell_laptop simply do nothing.
> >>> 
> >>> Tested and on my laptop Dell Latitude E6440 driver
> >>> dell_laptop does not work with above commit.
> >> 
> >> Hmm, interesting, so when dell-laptop registers and makes a
> >> few calls using the dell-laptop acpi interface,
> > 
> > No, dell-laptop is *not* acpi driver. See my first mail. It
> > uses dell dcdbas driver which makes SMI calls for SMBIOS.
> > But it (on my machine! no idea about other older once) just
> > forward brightness change to intel driver. And it has
> > useful brightness levels (no lot of levels which turning
> > display off).
> > 
> > And making SMI calls can be done also from userspace. There
> > is tool dellLcdBrightness in libsmbios package which for
> > brightness. And commit
> > 0b9f7d93ca6109048a4eb06332b666b6e29df4fe broke
> > functionality of that tool.
> > 
> >> then you either stop getting key press events through the
> >> acpi-video-bus, or dell-laptop is just a shim around the
> >> i915 interface with the firmware calling into itself on
> >> these models. Given that dell-laptop is ancient, the shim
> >> story is not that far fetched.
> > 
> > Brightness Fn keys are reported by WMI (dell-wmi driver), so
> > they working without dell-laptop and acpi video drivers
> > perfectly.
> > 
> >> Do you still get an on screen display showing the
> >> brightness when using a kernel without this patch +
> >> acpi_backlight=vendor ?
> > 
> > Brightness Fn keys are reported to userspace (from WMI input
> > device) with any combination of video.use_native_backlight
> > and acpi_backlight kernel params.
> 
> 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.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Aaron Lu]

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;
 	}

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Aaron Lu]

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;
 	}

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 3546 bytes --]

On Thursday 25 September 2014 05:15:35 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:
> 

Hi! this patch fixing this problem. With acpi_backlight=vendor 
dell-laptop will register backlight interface which is working 
again. Also userspace application dellLcdBrightness is working 
now without problem.

Can you going to send this patch also to stable 3.16 branch? As 
it fixing commit 0b9f7d93ca6109048a4eb06332b666b6e29df4fe.

> 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;
>  	}

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Rafael J. Wysocki]

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();

?

> +
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Rafael J. Wysocki]

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();

?

> +
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Aaron Lu]

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@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: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Aaron Lu]

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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Aaron Lu]

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 <pali.rohar@gmail.com>
Cc: <stable@vger.kernel.org> # v3.16 and later
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 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

[PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Aaron Lu]

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 <pali.rohar@gmail.com>
Cc: <stable@vger.kernel.org> # v3.16 and later
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 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

Re: [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Rafael J. Wysocki]

On Friday, September 26, 2014 10:30:08 AM Aaron Lu wrote:
> 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 <pali.rohar@gmail.com>
> Cc: <stable@vger.kernel.org> # v3.16 and later
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

Daniel, any objections?

> ---
>  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;
>  	}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Rafael J. Wysocki]

On Friday, September 26, 2014 10:30:08 AM Aaron Lu wrote:
> 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 <pali.rohar@gmail.com>
> Cc: <stable@vger.kernel.org> # v3.16 and later
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

Daniel, any objections?

> ---
>  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;
>  	}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Daniel Vetter]

On Fri, Sep 26, 2014 at 11:52:09PM +0200, Rafael J. Wysocki wrote:
> On Friday, September 26, 2014 10:30:08 AM Aaron Lu wrote:
> > 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 <pali.rohar@gmail.com>
> > Cc: <stable@vger.kernel.org> # v3.16 and later
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> 
> Daniel, any objections?

Nope, ack from my side.
-Daniel

> 
> > ---
> >  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;
> >  	}
> > 
> 
> -- 
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request

[Daniel Vetter]

On Fri, Sep 26, 2014 at 11:52:09PM +0200, Rafael J. Wysocki wrote:
> On Friday, September 26, 2014 10:30:08 AM Aaron Lu wrote:
> > 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 <pali.rohar@gmail.com>
> > Cc: <stable@vger.kernel.org> # v3.16 and later
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> 
> Daniel, any objections?

Nope, ack from my side.
-Daniel

> 
> > ---
> >  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;
> >  	}
> > 
> 
> -- 
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch